Re: [libvirt] [PATCH v2 8/8] blockcopy: add qemu implementation of new tunables

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.

> @@ -15281,6 +15281,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr *vmptr, > const char *path, > virStorageSourcePtr dest, > unsigned long bandwidth, > + int granularity, > + int buf_size, int ??

There are a few singedness problems in this patch. Although trivial enough to grant an ACK if you fix the issues above.