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

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. 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).