Re: [libvirt] [PATCHv2 4/3] snapshot: make offline qemu snapshots atomic

On 03/17/2012 05:33 PM, Eric Blake wrote: > Offline internal snapshots can be rolled back with just a little > bit of refactoring, meaning that we are now automatically atomic. > > * src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Move > guts... > (qemuDomainSnapshotForEachQcow2Raw): ...to new helper, to allow > rollbacks. > * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Offline > snapshots are now atomic. > --- You rollback changes to disks if other disks are not snapshotable, but later on, when the actual qemu-img command is run and fails the rollback is not performed.

I's suggest squashing in:

if (virRun(qemuimgarg, NULL) < 0) { if (try_all) { VIR_WARN("skipping snapshot action on %s", def->disks[i]->dst); skipped = true; continue; + } else if (STREQ(op, "-c") && i) { + /* We must roll back partial creation by deleting + * all earlier snapshots. */ + qemuDomainSnapshotForEachQcow2Raw(driver, def, name, + "-d", false, i); }

Otherwise looks good. ACK with that suggested change.