[libvirt] [PATCH 2/3] snapshot: prepare to remove transient snapshot metadata
Eric Blake
eblake at redhat.com
Wed Sep 21 19:08:51 UTC 2011
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
More information about the libvir-list
mailing list