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