[libvirt] [PATCHv4 12/18] blockjob: implement block copy for qemu

[Eric Blake]

On 04/13/2012 09:15 AM, Eric Blake wrote:
> On 04/13/2012 08:46 AM, Jiri Denemark wrote:
>> On Mon, Apr 09, 2012 at 21:52:21 -0600, Eric Blake wrote:
>>> Minimal patch to wire up all the pieces in the previous patches
>>> to actually enable a block copy job.  By minimal, I mean that
>>> qemu creates the file (that is, no REUSE_EXT flag support yet),
>>> SELinux must be disabled, a lock manager is not informed, and
>>> the audit logs aren't updated.  But those will be added as
>>> improvements in future patches.
>>>

>>> +    qemuDomainObjEnterMonitorWithDriver(driver, vm);
>>> +    ret = qemuMonitorDriveMirror(priv->mon, NULL, device, dest, format, flags);
>>> +    if (ret == 0 && bandwidth != 0)
>>> +        ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
>>> +                                  BLOCK_JOB_SPEED_INTERNAL);
>>> +    qemuDomainObjExitMonitorWithDriver(driver, vm);
>>
>> Can BLOCK_JOB_SPEED_INTERNAL fail while block copy remains running? If so, we
>> should try to abort the job in case we fail to set the speed, otherwise we
>> will report an error and forget about the job while qemu will keep copying
>> data.
> 
> Good catch, and virDomainBlockPull() suffers from the same
> ramifications.  I think both code paths need the same fix.

Actually, thinking about this a bit more:

qemu documents that block-job-set-speed will fail if:
DeviceNotActive: streaming is not active (but we already ignore this for
BLOCK_JOB_SPEED_INTERNAL, thanks to commit a9d3495)
NotSupported: throttling not supported

It doesn't document any other errors; there are probably errors such as
DeviceNotFound if an invalid device is specified, but we are less likely
to hit those, since we would have hit that same error with the initial
drive-mirror.

Meanwhile, libvirt.c currently documents:

 * The maximum bandwidth (in Mbps) that will be used to do the copy can be
 * specified with the bandwidth parameter.  If set to 0, libvirt will
choose a
 * suitable default.  Some hypervisors do not support this feature and will
 * return an error if bandwidth is not 0.

But trying to cancel the job is risky - we've already modified the
destination file; it would be nicer if we could detect the throttling
limitation before even starting the job, but qemu doesn't let us do
that.  I will see about requesting the ability to set bandwidth as part
of starting a job, rather than our current limitation of a guaranteed
race, as a qemu patch.

In the meantime, I think I will patch this instead by documentation:
state that non-zero bandwidth in virDomainBlockRebase() may be silently
ignored; if error checking is needed, the user should use a separate
call to virDomainBlockJobSetSpeed(); and to see if a request was
honored, use virDomainGetBlockJobInfo() (with the known race that the
job might already be complete).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120416/251244fb/attachment-0001.sig>