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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list