[libvirt] [PATCH] snapshot: affect persistent xml after disk snapshot

[Daniel Veillard]

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.
> >---
> >
> >This is worth including in 0.9.5 - without it, the new feature of
> >disk snapshots on a persistent domain are lost the moment the domain
> >stops running, which is likely to cause data corruption for guests.
> 
> I'm squashing this in, so that the persistent changes are preserved
> over libvirtd restarts.
> 
> diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
> index cbe28d8..2a1e5ea 100644
> --- i/src/qemu/qemu_driver.c
> +++ w/src/qemu/qemu_driver.c
> @@ -9221,6 +9221,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>      bool resume = false;
>      int ret = -1;
>      int i;
> +    bool persist = false;
> 
>      if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
>          return -1;
> @@ -9259,8 +9260,10 @@
> qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>              int indx = virDomainDiskIndexByName(vm->newDef,
>                                                  vm->def->disks[i]->dst,
>                                                  false);
> -            if (indx >= 0)
> +            if (indx >= 0) {
>                  persistDisk = vm->newDef->disks[indx];
> +                persist = true;
> +            }
>          }
> 
>          ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
> @@ -9301,7 +9304,9 @@ cleanup:
>      }
> 
>      if (vm) {
> -        if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> +        if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0 ||
> +            (persist &&
> +             virDomainSaveConfig(driver->configDir, vm->newDef) < 0))
>              ret = -1;
>          if (qemuDomainObjEndJob(driver, vm) == 0) {
>              /* Only possible if a transient vm quit while our locks
> were down,
> 

  ACK, though I don't fully undestand the index issue, isn't there other
  ways to find matching devices ?

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/