On 09/04/2014 09:11 AM, Peter Krempa wrote:
modify command. Technically, there is one case where getting block job info can modify domain XML - we do snooping to see if a 2-phase job has transitioned into the second phase, for an optimization in the case of old qemu that lacked an event for the transition. But I think that optimization is safe; and if it proves to be a problem, we could use the difference between QEMU_CAPS_BLOCKJOB_{ASYNC,SYNC} to determine whether we even need snooping, and request a modifying job in that case.
+ + if (info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && + disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) + info->type = disk->mirrorJob; + + /* Snoop block copy operations, so future cancel operations can + * avoid checking if pivot is safe. Save the change to XML, but + * we can ignore failure because it is only an optimization. */ + if (ret == 1 && disk->mirror && + info->cur == info->end && !disk->mirrorState) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
Atomicity is guaranteed by holding the domain lock here. We should document that the mirrorState field can change even when the _MODIFY job is held though.
Otherwise I don't see a problem currently with this as long as it is stated somewhere. Possibly even in a comment here.
ACK
Here's what I squashed in. diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index e7bd504..fb13169 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -15289,7 +15289,9 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path, /* Snoop block copy operations, so future cancel operations can * avoid checking if pivot is safe. Save the change to XML, but - * we can ignore failure because it is only an optimization. */ + * we can ignore failure because it is only an optimization. We + * hold the vm lock, so modifying the in-memory representation + * even though we are a query rather than a modify job, is safe. */ if (ret == 1 && disk->mirror && info->cur == info->end && !disk->mirrorState) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org