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@xxxxxxxxxx +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