On 08/26/2014 09:54 AM, Peter Krempa wrote: > On 08/26/14 13:21, Eric Blake wrote: >> Upstream qemu 1.4 added some drive-mirror tunables not present >> when it was first introduced in 1.3. Management apps may want >> to set these in some cases (for example, without tuning >> granularity down to sector size, a copy may end up occupying >> more bytes than the original because an entire cluster is >> copied even when only a sector within the cluster is dirty, >> although tuning it down results in more CPU time to do the >> copy). I haven't personally needed to use the parameters, but >> since they exist, and since the new API supports virTypedParams, >> we might as well expose them. >> >> Since the tuning parameters aren't often used, and omitted from >> the QMP command when unspecified, I think it is safe to rely on >> qemu 1.3 to issue an error about them being unsupported, rather >> than trying to create a new capability bit in libvirt. >> >> Meanwhile, all versions of qemu from 1.4 to 2.1 have a bug where >> a bad granularity (such as non-power-of-2) gives a poor message: >> error: internal error: unable to execute QEMU command 'drive-mirror': Invalid parameter 'drive-virtio-disk0' >> Maybe I need to tweak libvirt's handler to report a nicer message >> if qemu reports an invalid parameter, since playing the qemu >> message verbatim isn't very informative. This 'internal error' is too easy to trigger; for v3, I'm trying to generate a nicer error message. >> @@ -15281,6 +15281,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr *vmptr, >> const char *path, >> virStorageSourcePtr dest, >> unsigned long bandwidth, >> + int granularity, >> + int buf_size, > > int ?? I know granularity is capped (qemu chokes if it is larger than 64M), but buf_size appears to have no limit in qemu. Hmm, maybe that means I should tweak the public API to have the virTypedParameter for buf_size be unsigned long long, before we bake it in too small. And yes, I'm inconsistent on signed/unsigned (evidence of several iterations of this patch). > There are a few singedness problems in this patch. Although trivial > enough to grant an > > ACK if you fix the issues above. At this point, I'd feel better getting a v3 reviewed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list