On Mon, Apr 16, 2012 at 17:28:10 -0600, Eric Blake wrote: > 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). Makes sense, although we don't want to store the result of BLOCK_JOB_SPEED_INTERNAL to be stored in ret, then. Jirka -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list