[libvirt] [PATCH 2/3] snapshot: prepare to remove transient snapshot metadata

This patch is mostly code motion - moving some functions out of qemu_driver and into qemu_domain so they can be reused by multiple qemu_* files (since qemu_driver.h must not grow). It also adds a new helper function, qemuDomainRemoveInactive, which will be used in the next patch. * src/qemu/qemu_domain.h (qemuFindQemuImgBinary) (qemuDomainSnapshotWriteMetadata, qemuDomainSnapshotForEachQcow2) (qemuDomainSnapshotDiscard, qemuDomainSnapshotDiscardAll) (qemuDomainRemoveInactive): New prototypes. (struct qemu_snap_remove): New struct. * src/qemu/qemu_domain.c (qemuDomainRemoveInactive) (qemuDomainSnapshotDiscardAllMetadata): New functions. (qemuFindQemuImgBinary, qemuDomainSnapshotWriteMetadata) (qemuDomainSnapshotForEachQcow2, qemuDomainSnapshotDiscard) (qemuDomainSnapshotDiscardAll): Move here... * src/qemu/qemu_driver.c (qemuFindQemuImgBinary) (qemuDomainSnapshotWriteMetadata, qemuDomainSnapshotForEachQcow2) (qemuDomainSnapshotDiscard, qemuDomainSnapshotDiscardAll): ...from here. (qemuDomainUndefineFlags): Update caller. * src/conf/domain_conf.c (virDomainRemoveInactive): Doc fixes. --- src/conf/domain_conf.c | 4 +- src/qemu/qemu_domain.c | 260 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 36 +++++++ src/qemu/qemu_driver.c | 249 +-------------------------------------------- src/qemu/qemu_process.c | 2 +- 5 files changed, 301 insertions(+), 250 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eebcba0..7463d7c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1438,9 +1438,9 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* - * The caller must hold a lock on the driver owning 'doms', + * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else - * is either waiting for 'dom' or still usingn it + * is either waiting for 'dom' or still using it */ void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d2cc2f0..9436245 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1296,3 +1296,263 @@ cleanup: VIR_FREE(message); return ret; } + +/* Locate an appropriate 'qemu-img' binary. */ +const char * +qemuFindQemuImgBinary(struct qemud_driver *driver) +{ + if (!driver->qemuImgBinary) { + driver->qemuImgBinary = virFindFileInPath("kvm-img"); + if (!driver->qemuImgBinary) + driver->qemuImgBinary = virFindFileInPath("qemu-img"); + if (!driver->qemuImgBinary) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + } + + return driver->qemuImgBinary; +} + +int +qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir) +{ + int fd = -1; + char *newxml = NULL; + int ret = -1; + char *snapDir = NULL; + char *snapFile = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *tmp; + + virUUIDFormat(vm->def->uuid, uuidstr); + newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, + VIR_DOMAIN_XML_SECURE, 1); + if (newxml == NULL) { + virReportOOMError(); + return -1; + } + + if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virFileMakePath(snapDir) < 0) { + virReportSystemError(errno, _("cannot create snapshot directory '%s'"), + snapDir); + goto cleanup; + } + + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + fd = open(snapFile, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); + if (fd < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to create snapshot file '%s'"), snapFile); + goto cleanup; + } + + if (virAsprintf(&tmp, "snapshot-edit %s", vm->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + virEmitXMLWarning(fd, snapshot->def->name, tmp); + VIR_FREE(tmp); + + if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) { + virReportSystemError(errno, _("Failed to write snapshot data to %s"), + snapFile); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(snapFile); + VIR_FREE(snapDir); + VIR_FREE(newxml); + VIR_FORCE_CLOSE(fd); + return ret; +} + +/* The domain is expected to be locked and inactive. Return -1 on normal + * failure, 1 if we skipped a disk due to try_all. */ +int +qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + const char *op, + bool try_all) +{ + const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL }; + int i; + bool skipped = false; + + qemuimgarg[0] = qemuFindQemuImgBinary(driver); + if (qemuimgarg[0] == NULL) { + /* qemuFindQemuImgBinary set the error */ + return -1; + } + + qemuimgarg[2] = op; + qemuimgarg[3] = snap->def->name; + + for (i = 0; i < vm->def->ndisks; i++) { + /* FIXME: we also need to handle LVM here */ + /* FIXME: if we fail halfway through this loop, we are in an + * inconsistent state. I'm not quite sure what to do about that + */ + if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + if (!vm->def->disks[i]->driverType || + STRNEQ(vm->def->disks[i]->driverType, "qcow2")) { + if (try_all) { + /* Continue on even in the face of error, since other + * disks in this VM may have the same snapshot name. + */ + VIR_WARN("skipping snapshot action on %s", + vm->def->disks[i]->dst); + skipped = true; + continue; + } + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Disk device '%s' does not support" + " snapshotting"), + vm->def->disks[i]->dst); + return -1; + } + + qemuimgarg[4] = vm->def->disks[i]->src; + + if (virRun(qemuimgarg, NULL) < 0) { + if (try_all) { + VIR_WARN("skipping snapshot action on %s", + vm->def->disks[i]->info.alias); + skipped = true; + continue; + } + return -1; + } + } + } + + return skipped ? 1 : 0; +} + +/* Discard one snapshot (or its metadata), without reparenting any children. */ +int +qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool update_current, + bool metadata_only) +{ + char *snapFile = NULL; + int ret = -1; + qemuDomainObjPrivatePtr priv; + virDomainSnapshotObjPtr parentsnap = NULL; + + if (!metadata_only) { + if (!virDomainObjIsActive(vm)) { + /* Ignore any skipped disks */ + if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d", + true) < 0) + goto cleanup; + } else { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* we continue on even in the face of error */ + qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + } + + if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, + vm->def->name, snap->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (snap == vm->current_snapshot) { + if (update_current && snap->def->parent) { + parentsnap = virDomainSnapshotFindByName(&vm->snapshots, + snap->def->parent); + if (!parentsnap) { + VIR_WARN("missing parent snapshot matching name '%s'", + snap->def->parent); + } else { + parentsnap->def->current = true; + if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, + driver->snapshotDir) < 0) { + VIR_WARN("failed to set parent snapshot '%s' as current", + snap->def->parent); + parentsnap->def->current = false; + parentsnap = NULL; + } + } + } + vm->current_snapshot = parentsnap; + } + + if (unlink(snapFile) < 0) + VIR_WARN("Failed to unlink %s", snapFile); + virDomainSnapshotObjListRemove(&vm->snapshots, snap); + + ret = 0; + +cleanup: + VIR_FREE(snapFile); + + return ret; +} + +/* Hash iterator callback to discard multiple snapshots. */ +void qemuDomainSnapshotDiscardAll(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainSnapshotObjPtr snap = payload; + struct qemu_snap_remove *curr = data; + int err; + + if (snap->def->current) + curr->current = true; + err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false, + curr->metadata_only); + if (err && !curr->err) + curr->err = err; +} + +int +qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver, + virDomainObjPtr vm) +{ + struct qemu_snap_remove rem; + + rem.driver = driver; + rem.vm = vm; + rem.metadata_only = true; + rem.err = 0; + virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem); + + /* XXX also do rmdir ? */ + return rem.err; +} + +/* + * The caller must hold a lock on both driver and vm, and there must + * be no remaining references to vm. + */ +void +qemuDomainRemoveInactive(struct qemud_driver *driver, + virDomainObjPtr vm) +{ + /* Remove any snapshot metadata prior to removing the domain */ + if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) { + VIR_WARN("unable to remove all snapshots for domain %s", + vm->def->name); + } + virDomainRemoveInactive(&driver->domains, vm); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cde3ada..00cfa3a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -237,4 +237,40 @@ int qemuDomainAppendLog(struct qemud_driver *driver, int logFD, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(4, 5); +const char *qemuFindQemuImgBinary(struct qemud_driver *driver); + +int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir); + +int qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + const char *op, + bool try_all); + +int qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool update_current, + bool metadata_only); + +struct qemu_snap_remove { + struct qemud_driver *driver; + virDomainObjPtr vm; + int err; + bool metadata_only; + bool current; +}; + +void qemuDomainSnapshotDiscardAll(void *payload, + const void *name, + void *data); + +int qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver, + virDomainObjPtr vm); + +void qemuDomainRemoveInactive(struct qemud_driver *driver, + virDomainObjPtr vm); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a24019..4ff3281 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1596,243 +1596,6 @@ cleanup: } -/* Locate an appropriate 'qemu-img' binary. */ -static const char * -qemuFindQemuImgBinary(struct qemud_driver *driver) -{ - if (!driver->qemuImgBinary) { - driver->qemuImgBinary = virFindFileInPath("kvm-img"); - if (!driver->qemuImgBinary) - driver->qemuImgBinary = virFindFileInPath("qemu-img"); - if (!driver->qemuImgBinary) - qemuReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); - } - - return driver->qemuImgBinary; -} - -static int -qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, - virDomainSnapshotObjPtr snapshot, - char *snapshotDir) -{ - int fd = -1; - char *newxml = NULL; - int ret = -1; - char *snapDir = NULL; - char *snapFile = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; - char *tmp; - - virUUIDFormat(vm->def->uuid, uuidstr); - newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, - VIR_DOMAIN_XML_SECURE, 1); - if (newxml == NULL) { - virReportOOMError(); - return -1; - } - - if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - if (virFileMakePath(snapDir) < 0) { - virReportSystemError(errno, _("cannot create snapshot directory '%s'"), - snapDir); - goto cleanup; - } - - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - fd = open(snapFile, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); - if (fd < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("failed to create snapshot file '%s'"), snapFile); - goto cleanup; - } - - if (virAsprintf(&tmp, "snapshot-edit %s", vm->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - virEmitXMLWarning(fd, snapshot->def->name, tmp); - VIR_FREE(tmp); - - if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) { - virReportSystemError(errno, _("Failed to write snapshot data to %s"), - snapFile); - goto cleanup; - } - - ret = 0; - -cleanup: - VIR_FREE(snapFile); - VIR_FREE(snapDir); - VIR_FREE(newxml); - VIR_FORCE_CLOSE(fd); - return ret; -} - -/* The domain is expected to be locked and inactive. Return -1 on normal - * failure, 1 if we skipped a disk due to try_all. */ -static int -qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, - const char *op, - bool try_all) -{ - const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL }; - int i; - bool skipped = false; - - qemuimgarg[0] = qemuFindQemuImgBinary(driver); - if (qemuimgarg[0] == NULL) { - /* qemuFindQemuImgBinary set the error */ - return -1; - } - - qemuimgarg[2] = op; - qemuimgarg[3] = snap->def->name; - - for (i = 0; i < vm->def->ndisks; i++) { - /* FIXME: we also need to handle LVM here */ - /* FIXME: if we fail halfway through this loop, we are in an - * inconsistent state. I'm not quite sure what to do about that - */ - if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (!vm->def->disks[i]->driverType || - STRNEQ(vm->def->disks[i]->driverType, "qcow2")) { - if (try_all) { - /* Continue on even in the face of error, since other - * disks in this VM may have the same snapshot name. - */ - VIR_WARN("skipping snapshot action on %s", - vm->def->disks[i]->dst); - skipped = true; - continue; - } - qemuReportError(VIR_ERR_OPERATION_INVALID, - _("Disk device '%s' does not support" - " snapshotting"), - vm->def->disks[i]->dst); - return -1; - } - - qemuimgarg[4] = vm->def->disks[i]->src; - - if (virRun(qemuimgarg, NULL) < 0) { - if (try_all) { - VIR_WARN("skipping snapshot action on %s", - vm->def->disks[i]->info.alias); - skipped = true; - continue; - } - return -1; - } - } - } - - return skipped ? 1 : 0; -} - -/* Discard one snapshot (or its metadata), without reparenting any children. */ -static int -qemuDomainSnapshotDiscard(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, - bool update_current, - bool metadata_only) -{ - char *snapFile = NULL; - int ret = -1; - qemuDomainObjPrivatePtr priv; - virDomainSnapshotObjPtr parentsnap = NULL; - - if (!metadata_only) { - if (!virDomainObjIsActive(vm)) { - /* Ignore any skipped disks */ - if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d", - true) < 0) - goto cleanup; - } else { - priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); - /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - } - - if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, - vm->def->name, snap->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - - if (snap == vm->current_snapshot) { - if (update_current && snap->def->parent) { - parentsnap = virDomainSnapshotFindByName(&vm->snapshots, - snap->def->parent); - if (!parentsnap) { - VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); - } else { - parentsnap->def->current = true; - if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, - driver->snapshotDir) < 0) { - VIR_WARN("failed to set parent snapshot '%s' as current", - snap->def->parent); - parentsnap->def->current = false; - parentsnap = NULL; - } - } - } - vm->current_snapshot = parentsnap; - } - - if (unlink(snapFile) < 0) - VIR_WARN("Failed to unlink %s", snapFile); - virDomainSnapshotObjListRemove(&vm->snapshots, snap); - - ret = 0; - -cleanup: - VIR_FREE(snapFile); - - return ret; -} - -struct snap_remove { - struct qemud_driver *driver; - virDomainObjPtr vm; - int err; - bool metadata_only; - bool current; -}; - -/* Hash iterator callback to discard multiple snapshots. */ -static void -qemuDomainSnapshotDiscardAll(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) -{ - virDomainSnapshotObjPtr snap = payload; - struct snap_remove *curr = data; - int err; - - if (snap->def->current) - curr->current = true; - err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false, - curr->metadata_only); - if (err && !curr->err) - curr->err = err; -} - /* Count how many snapshots in a set have external disk snapshots. */ static void qemuDomainSnapshotCountExternal(void *payload, @@ -5126,8 +4889,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, if (!virDomainObjIsActive(vm) && (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) { - struct snap_remove rem; - if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { qemuReportError(VIR_ERR_OPERATION_INVALID, _("cannot delete inactive domain with %d " @@ -5135,13 +4896,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, nsnapshots); goto cleanup; } - - rem.driver = driver; - rem.vm = vm; - rem.metadata_only = true; - rem.err = 0; - virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem); - if (rem.err < 0) + if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) goto cleanup; } @@ -10132,7 +9887,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, int ret = -1; virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - struct snap_remove rem; + struct qemu_snap_remove rem; struct snap_reparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); int external = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd49b21..8797a56 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -106,7 +106,7 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver; /* - * This is a callback registered with a qemuMonitorPtr instance, + * This is a callback registered with a qemuMonitorPtr instance, * and to be invoked when the monitor console hits an end of file * condition, or error, thus indicating VM shutdown should be * performed -- 1.7.4.4