[libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.

john cooper john.cooper at redhat.com
Fri Jan 30 15:10:34 UTC 2009


Jim Meyering wrote:
>
> Hi John,
>
> I tried to apply that, but failed miserably,
> since all of the following was recently redone to
> use virBufferVSprintf rather than snprintf.
Yea I suspected the code was likely seeing some
motion. Thanks for bringing it forward.
> And it's
> a good thing, because with the changes below, there was
> potential buffer overrun: nc could end up larger than PATH_MAX.
You are correct. I mistook the return value of
snprintf() in the case of truncation.

> One remaining bit that I haven't done:
> add tests for this, exercising e.g.,
>   - cachemode
>   - !cachemode && (disk->shared && !disk->readonly)
>   - !cachemode && (!disk->shared || disk->readonly)
>   
That logic should be correct as it exists in the
patch, namely we'll generate a "cache=*" flag in
the case cachemode was specified explicitly in
the xml definition or for the preexisting case of
(shared && !readonly). Here if both happen to be
true the explicit xml cache tag takes precedence.

But with the existing code we need to remove the
clause of:

@@ -1007,18 +1025,34 @@ int qemudBuildCommandLine(virConnectPtr
if (bootable &&
disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(&opt, ",boot=on");
- if (disk->shared && !disk->readonly)
- virBufferAddLit(&opt, ",cache=off");
if (disk->driverType)
virBufferVSprintf(&opt, ",fmt=%s", disk->driverType);

As this results in generation of a "cache=*"
twice on the qemu cmdline, and possibly of
different dispositions.

With this change I think it is good to go.
The attached patch incorporates the above
modification.

-john

-- 
john.cooper at redhat.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: libvirt-drive-cache-09013000.patch
Type: text/x-patch
Size: 7983 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090130/787ef406/attachment-0001.bin>


More information about the libvir-list mailing list