On 09/17/2011 08:48 AM, Daniel Veillard wrote:
On Sat, Sep 17, 2011 at 05:56:22AM -0600, Eric Blake wrote:
On 09/16/2011 10:11 PM, Eric Blake wrote:
For external snapshots to be useful on persistent domains, we must alter the persistent definition alongside the running definition. Thanks to the possibility of disk hotplug as well as of edits that only affect the persistent xml, we can't assume that vm->def and vm->newDef have the same disk at the same index, so we can only update the persistent copy if the device destination matches up.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive) (qemuDomainSnapshotCreateSingleDiskActive): Also affect newDef, if present. ---
ACK,
Pushed.
though I don't fully undestand the index issue, isn't there other ways to find matching devices ?
if (snap->def->disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO) continue; + if (vm->newDef) { + int indx = virDomainDiskIndexByName(vm->newDef, + vm->def->disks[i]->dst, + false); + if (indx >= 0) + persistDisk = vm->newDef->disks[indx]; + }
The bit about finding the index is for situations like: vm->def->disks[0] is vda, disks[1] is vdb but we have pending changes to the persistent config, so that: vm->newDef->disks[0] is vdb, disks[1] is vdc We are guaranteed that disks[n]->dst is a valid string, so the index lookup says find the disk (if any) in vm->newDef with the same dst as what we are about to modify in vm->def; if both definitions have a disk by the same dst, then we edit both. In the above example, that would be def->disks[0] gives indx -1, so no edit to vm->newDef; while def->disks[1] gives indx 0, and 'vdb' is edited in both definitions. If we had just blindly updated disks[1] in both def and newDef, we would have been updating vdc instead of vdb in newDef. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org