Re: [libvirt] [PATCH] snapshot: affect persistent xml after disk snapshot
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org