[libvirt] [PATCH 0/4] virsh snapshot-info
I'm working on a patch series to allow offline disk snapshots, and in the process, I found that I either don't have enough information ('virsh snapshot-list' is great for a summary, but not for an individual snapshot) or too much ('virsh snapshot-dumpxml' means I have to wade through the XML). This provides an intermediate view, designed for ease of use. Eric Blake (4): snapshot: new query APIs snapshot: add 'virsh snapshot-info' snapshot: RPC for new query APIs snapshot: implement new APIs for qemu, esx, and vbox include/libvirt/libvirt.h.in | 9 +++ src/driver.h | 10 +++ src/esx/esx_driver.c | 71 ++++++++++++++++++++++++ src/libvirt.c | 85 ++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_driver.c | 83 ++++++++++++++++++++++++++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 23 +++++++- src/remote_protocol-structs | 16 +++++ src/vbox/vbox_tmpl.c | 97 ++++++++++++++++++++++++++++++++ tools/virsh.c | 125 ++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++ 12 files changed, 531 insertions(+), 1 deletions(-) -- 1.7.7.6
Right now, starting from just a virDomainSnapshotPtr, and wanting to know if it is the current snapshot for its respective domain, you have to use virDomainSnapshotGetDomain(), then virDomainSnapshotCurrent(), then compare the two names returned by virDomainSnapshotGetName(). It is a bit easier if we can directly query this information from the snapshot itself. Right now, it is possible to filter a snapshot listing based on whether snapshots have metadata that would prevent domain deletion, but the only way to learn if an individual snapshot has metadata is to see if that snapshot appears in the list returned by a listing. Additionally, I hope to expand the qemu driver in a future patch to use qemu-img to reconstruct snapshot XML corresponding to internal qcow2 snapshot names not otherwise tracked by libvirt (in part, so that libvirt can guarantee that new snapshots are not created with a name that would silently corrupt the existing portion of the qcow2 file); if I ever get that in, then it would no longer be an all-or-none decision on whether snapshots have metadata, and becomes all the more important to be able to directly determine that information from a particular snapshot. Other query functions (such as virDomainIsActive) do not have a flags argument, but since virDomainHasCurrentSnapshot takes a flags argument, I figured it was safer to provide a flags argument here as well. * include/libvirt/libvirt.h.in (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New declarations. * src/libvirt.c (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New functions. * src/libvirt_public.syms (LIBVIRT_0.9.13): Export them. * src/driver.h (virDrvDomainSnapshotIsCurrent) (virDrvDomainSnapshotHasMetadata): New driver callbacks. --- include/libvirt/libvirt.h.in | 9 ++++ src/driver.h | 10 +++++ src/libvirt.c | 85 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 110 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index da3ce29..dde5c4c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3401,6 +3401,15 @@ virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain, virDomainSnapshotPtr virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags); +/* Determine if a snapshot is the current snapshot of its domain. */ +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags); + +/* Determine if a snapshot has associated libvirt metadata that would + * prevent the deletion of its domain. */ +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags); + typedef enum { VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED = 1 << 1, /* Pause after revert */ diff --git a/src/driver.h b/src/driver.h index aa7a377..94cc851 100644 --- a/src/driver.h +++ b/src/driver.h @@ -650,6 +650,14 @@ typedef virDomainSnapshotPtr unsigned int flags); typedef int + (*virDrvDomainSnapshotIsCurrent)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef int + (*virDrvDomainSnapshotHasMetadata)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef int (*virDrvDomainRevertToSnapshot)(virDomainSnapshotPtr snapshot, unsigned int flags); @@ -986,6 +994,8 @@ struct _virDriver { virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot; virDrvDomainSnapshotGetParent domainSnapshotGetParent; virDrvDomainSnapshotCurrent domainSnapshotCurrent; + virDrvDomainSnapshotIsCurrent domainSnapshotIsCurrent; + virDrvDomainSnapshotHasMetadata domainSnapshotHasMetadata; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; virDrvDomainQemuMonitorCommand qemuDomainMonitorCommand; diff --git a/src/libvirt.c b/src/libvirt.c index 8900011..41ca840 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17621,6 +17621,91 @@ error: } /** + * virDomainSnapshotIsCurrent: + * @snapshot: a snapshot object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Determine if the given snapshot is the domain's current snapshot. See + * also virDomainHasCurrentSnapshot(). + * + * Returns 1 if current, 0 if not current, or -1 on error. + */ +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotIsCurrent) { + int ret; + ret = conn->driver->domainSnapshotIsCurrent(snapshot, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** + * virDomainSnapshotHasMetadata: + * @snapshot: a snapshot object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Determine if the given snapshot is associated with libvirt metadata + * that would prevent the deletion of the domain. + * + * Returns 1 if the snapshot has metadata, 0 if the snapshot exists without + * help from libvirt, or -1 on error. + */ +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotHasMetadata) { + int ret; + ret = conn->driver->domainSnapshotHasMetadata(snapshot, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainRevertToSnapshot: * @snapshot: a domain snapshot object * @flags: bitwise-OR of virDomainSnapshotRevertFlags diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 46c13fb..ebcbd94 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -534,4 +534,10 @@ LIBVIRT_0.9.11 { virDomainPMWakeup; } LIBVIRT_0.9.10; +LIBVIRT_0.9.13 { + global: + virDomainSnapshotHasMetadata; + virDomainSnapshotIsCurrent; +} LIBVIRT_0.9.11; + # .... define new API here using predicted next version number .... -- 1.7.7.6
On 2012年05月25日 11:33, Eric Blake wrote:
Right now, starting from just a virDomainSnapshotPtr, and wanting to know if it is the current snapshot for its respective domain, you have to use virDomainSnapshotGetDomain(), then virDomainSnapshotCurrent(), then compare the two names returned by virDomainSnapshotGetName(). It is a bit easier if we can directly query this information from the snapshot itself.
Right now, it is possible to filter a snapshot listing based on whether snapshots have metadata that would prevent domain deletion, but the only way to learn if an individual snapshot has metadata is to see if that snapshot appears in the list returned by a listing. Additionally, I hope to expand the qemu driver in a future patch to use qemu-img to reconstruct snapshot XML corresponding to internal qcow2 snapshot names not otherwise tracked by libvirt (in part, so that libvirt can guarantee that new snapshots are not created with a name that would silently corrupt the existing portion of the qcow2 file); if I ever get that in, then it would no longer be an all-or-none decision on whether snapshots have metadata, and becomes all the more important to be able to directly determine that information from a particular snapshot.
Other query functions (such as virDomainIsActive) do not have a flags argument, but since virDomainHasCurrentSnapshot takes a flags argument, I figured it was safer to provide a flags argument here as well.
* include/libvirt/libvirt.h.in (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New declarations. * src/libvirt.c (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New functions. * src/libvirt_public.syms (LIBVIRT_0.9.13): Export them. * src/driver.h (virDrvDomainSnapshotIsCurrent) (virDrvDomainSnapshotHasMetadata): New driver callbacks. --- include/libvirt/libvirt.h.in | 9 ++++ src/driver.h | 10 +++++ src/libvirt.c | 85 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 4 files changed, 110 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index da3ce29..dde5c4c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3401,6 +3401,15 @@ virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain, virDomainSnapshotPtr virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags);
+/* Determine if a snapshot is the current snapshot of its domain. */ +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags); + +/* Determine if a snapshot has associated libvirt metadata that would + * prevent the deletion of its domain. */ +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags); + typedef enum { VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1<< 0, /* Run after revert */ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED = 1<< 1, /* Pause after revert */ diff --git a/src/driver.h b/src/driver.h index aa7a377..94cc851 100644 --- a/src/driver.h +++ b/src/driver.h @@ -650,6 +650,14 @@ typedef virDomainSnapshotPtr unsigned int flags);
typedef int + (*virDrvDomainSnapshotIsCurrent)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef int + (*virDrvDomainSnapshotHasMetadata)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef int (*virDrvDomainRevertToSnapshot)(virDomainSnapshotPtr snapshot, unsigned int flags);
@@ -986,6 +994,8 @@ struct _virDriver { virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot; virDrvDomainSnapshotGetParent domainSnapshotGetParent; virDrvDomainSnapshotCurrent domainSnapshotCurrent; + virDrvDomainSnapshotIsCurrent domainSnapshotIsCurrent; + virDrvDomainSnapshotHasMetadata domainSnapshotHasMetadata; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; virDrvDomainQemuMonitorCommand qemuDomainMonitorCommand; diff --git a/src/libvirt.c b/src/libvirt.c index 8900011..41ca840 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17621,6 +17621,91 @@ error: }
/** + * virDomainSnapshotIsCurrent: + * @snapshot: a snapshot object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Determine if the given snapshot is the domain's current snapshot. See + * also virDomainHasCurrentSnapshot(). + * + * Returns 1 if current, 0 if not current, or -1 on error. + */ +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotIsCurrent) { + int ret; + ret = conn->driver->domainSnapshotIsCurrent(snapshot, flags); + if (ret< 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** + * virDomainSnapshotHasMetadata: + * @snapshot: a snapshot object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Determine if the given snapshot is associated with libvirt metadata + * that would prevent the deletion of the domain. + * + * Returns 1 if the snapshot has metadata, 0 if the snapshot exists without + * help from libvirt, or -1 on error. + */ +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotHasMetadata) { + int ret; + ret = conn->driver->domainSnapshotHasMetadata(snapshot, flags); + if (ret< 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainRevertToSnapshot: * @snapshot: a domain snapshot object * @flags: bitwise-OR of virDomainSnapshotRevertFlags diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 46c13fb..ebcbd94 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -534,4 +534,10 @@ LIBVIRT_0.9.11 { virDomainPMWakeup; } LIBVIRT_0.9.10;
+LIBVIRT_0.9.13 { + global: + virDomainSnapshotHasMetadata; + virDomainSnapshotIsCurrent; +} LIBVIRT_0.9.11; + # .... define new API here using predicted next version number ....
I'm not hep in the overall snapshot design, but the APIs looks quite straightforward, so ACK. Osier
On 06/11/2012 12:48 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
Right now, starting from just a virDomainSnapshotPtr, and wanting to know if it is the current snapshot for its respective domain, you have to use virDomainSnapshotGetDomain(), then virDomainSnapshotCurrent(), then compare the two names returned by virDomainSnapshotGetName(). It is a bit easier if we can directly query this information from the snapshot itself.
* include/libvirt/libvirt.h.in (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New declarations. * src/libvirt.c (virDomainSnapshotIsCurrent) (virDomainSnapshotHasMetadata): New functions. * src/libvirt_public.syms (LIBVIRT_0.9.13): Export them. * src/driver.h (virDrvDomainSnapshotIsCurrent) (virDrvDomainSnapshotHasMetadata): New driver callbacks.
+LIBVIRT_0.9.13 { + global: + virDomainSnapshotHasMetadata; + virDomainSnapshotIsCurrent; +} LIBVIRT_0.9.11; + # .... define new API here using predicted next version number ....
I'm not hep in the overall snapshot design, but the APIs looks quite straightforward, so ACK.
Thanks; I've gone ahead and pushed this patch (which means we are now committed to providing it before 0.9.13); this of course means that other API additions probably have to rebase, but that should be relatively straightforward conflict resolution. I will split and repost patch 4/4 into two patches, one for qemu (which appeared to be non-controversial) and one for esx/vbox (which may depend on list consensus on whether blind success is desirable). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Expose the recent API additions in virsh. Borrows ideas from 'dominfo' for the general type of information to display. Output looks like: $ tools/virsh snapshot-info fedora-local tmp Name: tmp Domain: fedora-local Current: no State: disk-snapshot Parent: - Children: 1 Descendants: 2 Metadata: yes possibly with fewer lines when talking to older servers. * tools/virsh.c (cmdSnapshotInfo): New command. * tools/virsh.pod (snapshot-info): Document it. --- tools/virsh.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++ 2 files changed, 130 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ffe6ed2..6a85338 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -16601,6 +16601,129 @@ cleanup: } /* + * "snapshot-info" command + */ +static const vshCmdInfo info_snapshot_info[] = { + {"help", N_("snapshot information")}, + {"desc", N_("Returns basic information about a snapshot.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_snapshot_info[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")}, + {"current", VSH_OT_BOOL, 0, N_("info on current snapshot")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virDomainSnapshotPtr snapshot = NULL; + const char *name; + char *doc = NULL; + char *tmp; + char *parent = NULL; + bool ret = false; + int count; + unsigned int flags; + int current; + int metadata; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + dom = vshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + return false; + + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, + &snapshot, &name) < 0) + goto cleanup; + + vshPrint(ctl, "%-15s %s\n", _("Name:"), name); + vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom)); + + /* Determine if snapshot is current; this is useful enough that we + * attempt a fallback. */ + current = virDomainSnapshotIsCurrent(snapshot, 0); + if (current < 0) { + virDomainSnapshotPtr other = virDomainSnapshotCurrent(dom, 0); + + virResetLastError(); + current = 0; + if (other) { + if (STREQ(name, virDomainSnapshotGetName(other))) + current = 1; + virDomainSnapshotFree(other); + } + } + vshPrint(ctl, "%-15s %s\n", _("Current:"), + current > 0 ? _("yes") : _("no")); + + /* Get the XML configuration of the snapshot to determine the + * state of the machine at the time of the snapshot. */ + doc = virDomainSnapshotGetXMLDesc(snapshot, 0); + if (!doc) + goto cleanup; + + tmp = strstr(doc, "<state>"); + if (!tmp) { + vshError(ctl, "%s", + _("unexpected problem reading snapshot xml")); + goto cleanup; + } + tmp += strlen("<state>"); + vshPrint(ctl, "%-15s %.*s\n", _("State:"), + (int) (strchr(tmp, '<') - tmp), tmp); + + if (vshGetSnapshotParent(ctl, snapshot, &parent) < 0) + goto cleanup; + vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent : "-"); + + /* Children, Descendants. After this point, the fallback to + * compute children is too expensive, so we gracefully quit if the + * APIs don't exist. */ + if (ctl->useSnapshotOld) { + ret = true; + goto cleanup; + } + flags = 0; + count = virDomainSnapshotNumChildren(snapshot, 0); + if (count < 0) + goto cleanup; + vshPrint(ctl, "%-15s %d\n", _("Children:"), count); + flags = VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + count = virDomainSnapshotNumChildren(snapshot, flags); + if (count < 0) + goto cleanup; + vshPrint(ctl, "%-15s %d\n", _("Descendants:"), count); + + /* Metadata; the fallback here relies on the fact that metadata + * used to have an all-or-nothing effect on snapshot count. */ + metadata = virDomainSnapshotHasMetadata(snapshot, 0); + if (metadata < 0) { + virResetLastError(); + count = virDomainSnapshotNum(dom, + VIR_DOMAIN_SNAPSHOT_LIST_METADATA); + metadata = count != 0; + } + vshPrint(ctl, "%-15s %s\n", _("Metadata:"), + metadata > 0 ? _("yes") : _("no")); + + ret = true; + +cleanup: + VIR_FREE(doc); + VIR_FREE(parent); + if (snapshot) + virDomainSnapshotFree(snapshot); + virDomainFree(dom); + return ret; +} + +/* * "snapshot-list" command */ static const vshCmdInfo info_snapshot_list[] = { @@ -17702,6 +17825,8 @@ static const vshCmdDef snapshotCmds[] = { info_snapshot_dumpxml, 0}, {"snapshot-edit", cmdSnapshotEdit, opts_snapshot_edit, info_snapshot_edit, 0}, + {"snapshot-info", cmdSnapshotInfo, opts_snapshot_info, + info_snapshot_info, 0}, {"snapshot-list", cmdSnapshotList, opts_snapshot_list, info_snapshot_list, 0}, {"snapshot-parent", cmdSnapshotParent, opts_snapshot_parent, diff --git a/tools/virsh.pod b/tools/virsh.pod index ef71717..6553825 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2504,6 +2504,11 @@ a snapshot name must be done with care, since the contents of some snapshots, such as internal snapshots within a single qcow2 file, are accessible only from the original name. +=item B<snapshot-info> I<domain> {I<snapshot> | I<--current>} + +Output basic information about a named <snapshot>, or the current snapshot +with I<--current>. + =item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}] [{[I<--from>] B<snapshot> | I<--current>} [I<--descendants>]] [I<--metadata>] [I<--leaves>] -- 1.7.7.6
On 2012年05月25日 11:33, Eric Blake wrote:
Expose the recent API additions in virsh. Borrows ideas from 'dominfo' for the general type of information to display. Output looks like:
$ tools/virsh snapshot-info fedora-local tmp Name: tmp Domain: fedora-local Current: no State: disk-snapshot Parent: - Children: 1 Descendants: 2 Metadata: yes
possibly with fewer lines when talking to older servers.
* tools/virsh.c (cmdSnapshotInfo): New command. * tools/virsh.pod (snapshot-info): Document it.
--- tools/virsh.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++ 2 files changed, 130 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index ffe6ed2..6a85338 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -16601,6 +16601,129 @@ cleanup: }
/* + * "snapshot-info" command + */ +static const vshCmdInfo info_snapshot_info[] = { + {"help", N_("snapshot information")}, + {"desc", N_("Returns basic information about a snapshot.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_snapshot_info[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")}, + {"current", VSH_OT_BOOL, 0, N_("info on current snapshot")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virDomainSnapshotPtr snapshot = NULL; + const char *name; + char *doc = NULL; + char *tmp; + char *parent = NULL; + bool ret = false; + int count; + unsigned int flags; + int current; + int metadata; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + dom = vshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + return false; + + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, +&snapshot,&name)< 0) + goto cleanup; + + vshPrint(ctl, "%-15s %s\n", _("Name:"), name); + vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom)); + + /* Determine if snapshot is current; this is useful enough that we + * attempt a fallback. */ + current = virDomainSnapshotIsCurrent(snapshot, 0); + if (current< 0) { + virDomainSnapshotPtr other = virDomainSnapshotCurrent(dom, 0); + + virResetLastError(); + current = 0; + if (other) { + if (STREQ(name, virDomainSnapshotGetName(other))) + current = 1; + virDomainSnapshotFree(other); + } + } + vshPrint(ctl, "%-15s %s\n", _("Current:"), + current> 0 ? _("yes") : _("no")); + + /* Get the XML configuration of the snapshot to determine the + * state of the machine at the time of the snapshot. */ + doc = virDomainSnapshotGetXMLDesc(snapshot, 0); + if (!doc) + goto cleanup; + + tmp = strstr(doc, "<state>"); + if (!tmp) { + vshError(ctl, "%s", + _("unexpected problem reading snapshot xml")); + goto cleanup; + } + tmp += strlen("<state>"); + vshPrint(ctl, "%-15s %.*s\n", _("State:"), + (int) (strchr(tmp, '<') - tmp), tmp); + + if (vshGetSnapshotParent(ctl, snapshot,&parent)< 0) + goto cleanup; + vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent : "-"); + + /* Children, Descendants. After this point, the fallback to + * compute children is too expensive, so we gracefully quit if the + * APIs don't exist. */ + if (ctl->useSnapshotOld) { + ret = true; + goto cleanup; + } + flags = 0; + count = virDomainSnapshotNumChildren(snapshot, 0); + if (count< 0) + goto cleanup; + vshPrint(ctl, "%-15s %d\n", _("Children:"), count); + flags = VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + count = virDomainSnapshotNumChildren(snapshot, flags); + if (count< 0) + goto cleanup; + vshPrint(ctl, "%-15s %d\n", _("Descendants:"), count); + + /* Metadata; the fallback here relies on the fact that metadata + * used to have an all-or-nothing effect on snapshot count. */ + metadata = virDomainSnapshotHasMetadata(snapshot, 0); + if (metadata< 0) { + virResetLastError(); + count = virDomainSnapshotNum(dom, + VIR_DOMAIN_SNAPSHOT_LIST_METADATA); + metadata = count != 0; + } + vshPrint(ctl, "%-15s %s\n", _("Metadata:"), + metadata> 0 ? _("yes") : _("no")); + + ret = true; + +cleanup: + VIR_FREE(doc); + VIR_FREE(parent); + if (snapshot) + virDomainSnapshotFree(snapshot); + virDomainFree(dom); + return ret; +} + +/* * "snapshot-list" command */ static const vshCmdInfo info_snapshot_list[] = { @@ -17702,6 +17825,8 @@ static const vshCmdDef snapshotCmds[] = { info_snapshot_dumpxml, 0}, {"snapshot-edit", cmdSnapshotEdit, opts_snapshot_edit, info_snapshot_edit, 0}, + {"snapshot-info", cmdSnapshotInfo, opts_snapshot_info, + info_snapshot_info, 0}, {"snapshot-list", cmdSnapshotList, opts_snapshot_list, info_snapshot_list, 0}, {"snapshot-parent", cmdSnapshotParent, opts_snapshot_parent, diff --git a/tools/virsh.pod b/tools/virsh.pod index ef71717..6553825 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2504,6 +2504,11 @@ a snapshot name must be done with care, since the contents of some snapshots, such as internal snapshots within a single qcow2 file, are accessible only from the original name.
+=item B<snapshot-info> I<domain> {I<snapshot> | I<--current>} + +Output basic information about a named<snapshot>, or the current snapshot +with I<--current>. + =item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}] [{[I<--from>] B<snapshot> | I<--current>} [I<--descendants>]] [I<--metadata>] [I<--leaves>]
ACK Osier
On 06/11/2012 12:49 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
Expose the recent API additions in virsh. Borrows ideas from 'dominfo' for the general type of information to display. Output looks like:
$ tools/virsh snapshot-info fedora-local tmp Name: tmp Domain: fedora-local Current: no State: disk-snapshot Parent: - Children: 1 Descendants: 2 Metadata: yes
possibly with fewer lines when talking to older servers.
+ /* Metadata; the fallback here relies on the fact that metadata + * used to have an all-or-nothing effect on snapshot count. */ + metadata = virDomainSnapshotHasMetadata(snapshot, 0); + if (metadata< 0) { + virResetLastError(); + count = virDomainSnapshotNum(dom, + VIR_DOMAIN_SNAPSHOT_LIST_METADATA); + metadata = count != 0; + } + vshPrint(ctl, "%-15s %s\n", _("Metadata:"), + metadata> 0 ? _("yes") : _("no"));
I tweaked this part slightly (if virDomainSnapshotNum() fails due to the LIST_METADATA flag being unsupported, then that should omit the Metadata: line as we don't have an accurate answer).
ACK
Thanks; pushed now, with this squashed in. diff --git c/tools/virsh.c w/tools/virsh.c index 5e6fe6d..744b629 100644 --- c/tools/virsh.c +++ w/tools/virsh.c @@ -16657,7 +16657,7 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } flags = 0; - count = virDomainSnapshotNumChildren(snapshot, 0); + count = virDomainSnapshotNumChildren(snapshot, flags); if (count < 0) goto cleanup; vshPrint(ctl, "%-15s %d\n", _("Children:"), count); @@ -16671,13 +16671,13 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) * used to have an all-or-nothing effect on snapshot count. */ metadata = virDomainSnapshotHasMetadata(snapshot, 0); if (metadata < 0) { + metadata = virDomainSnapshotNum(dom, + VIR_DOMAIN_SNAPSHOT_LIST_METADATA); virResetLastError(); - count = virDomainSnapshotNum(dom, - VIR_DOMAIN_SNAPSHOT_LIST_METADATA); - metadata = count != 0; } - vshPrint(ctl, "%-15s %s\n", _("Metadata:"), - metadata > 0 ? _("yes") : _("no")); + if (metadata >= 0) + vshPrint(ctl, "%-15s %s\n", _("Metadata:"), + metadata ? _("yes") : _("no")); ret = true; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Pretty straightforward. * src/remote/remote_protocol.x (remote_domain_snapshot_is_current_args) (remote_domain_snapshot_is_current_ret) (remote_domain_snapshot_has_metadata_args) (remote_domain_snapshot_has_metadata_ret): New structs. (REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT) (REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA): New RPC calls. * src/remote/remote_driver.c (remote_driver): Call them. * src/remote_protocol-structs: Regenerate. --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 23 ++++++++++++++++++++++- src/remote_protocol-structs | 16 ++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c87561..fc9babd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5082,6 +5082,8 @@ static virDriver remote_driver = { .domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = remoteDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = remoteDomainRevertToSnapshot, /* 0.8.0 */ + .domainSnapshotIsCurrent = remoteDomainSnapshotIsCurrent, /* 0.9.13 */ + .domainSnapshotHasMetadata = remoteDomainSnapshotHasMetadata, /* 0.9.13 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ .qemuDomainMonitorCommand = remoteQemuDomainMonitorCommand, /* 0.8.3 */ .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2d57247..9982963 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2305,6 +2305,24 @@ struct remote_domain_snapshot_current_ret { remote_nonnull_domain_snapshot snap; }; +struct remote_domain_snapshot_is_current_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_is_current_ret { + int current; +}; + +struct remote_domain_snapshot_has_metadata_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_has_metadata_ret { + int metadata; +}; + struct remote_domain_revert_to_snapshot_args { remote_nonnull_domain_snapshot snap; unsigned int flags; @@ -2782,7 +2800,10 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_WAKEUP = 267, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_TRAY_CHANGE = 268, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_PMWAKEUP = 269, /* autogen autogen */ - REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270 /* autogen autogen */ + REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270, /* autogen autogen */ + + REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT = 271, /* autogen autogen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 9b2414f..8e00b0e 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1756,6 +1756,20 @@ struct remote_domain_snapshot_current_args { struct remote_domain_snapshot_current_ret { remote_nonnull_domain_snapshot snap; }; +struct remote_domain_snapshot_is_current_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_is_current_ret { + int current; +}; +struct remote_domain_snapshot_has_metadata_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_has_metadata_ret { + int metadata; +}; struct remote_domain_revert_to_snapshot_args { remote_nonnull_domain_snapshot snap; u_int flags; @@ -2192,4 +2206,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_TRAY_CHANGE = 268, REMOTE_PROC_DOMAIN_EVENT_PMWAKEUP = 269, REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270, + REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT = 271, + REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272, }; -- 1.7.7.6
On 2012年05月25日 11:33, Eric Blake wrote:
Pretty straightforward.
* src/remote/remote_protocol.x (remote_domain_snapshot_is_current_args) (remote_domain_snapshot_is_current_ret) (remote_domain_snapshot_has_metadata_args) (remote_domain_snapshot_has_metadata_ret): New structs. (REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT) (REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA): New RPC calls. * src/remote/remote_driver.c (remote_driver): Call them. * src/remote_protocol-structs: Regenerate. --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 23 ++++++++++++++++++++++- src/remote_protocol-structs | 16 ++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c87561..fc9babd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5082,6 +5082,8 @@ static virDriver remote_driver = { .domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = remoteDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = remoteDomainRevertToSnapshot, /* 0.8.0 */ + .domainSnapshotIsCurrent = remoteDomainSnapshotIsCurrent, /* 0.9.13 */ + .domainSnapshotHasMetadata = remoteDomainSnapshotHasMetadata, /* 0.9.13 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ .qemuDomainMonitorCommand = remoteQemuDomainMonitorCommand, /* 0.8.3 */ .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2d57247..9982963 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2305,6 +2305,24 @@ struct remote_domain_snapshot_current_ret { remote_nonnull_domain_snapshot snap; };
+struct remote_domain_snapshot_is_current_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_is_current_ret { + int current; +}; + +struct remote_domain_snapshot_has_metadata_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_has_metadata_ret { + int metadata; +}; + struct remote_domain_revert_to_snapshot_args { remote_nonnull_domain_snapshot snap; unsigned int flags; @@ -2782,7 +2800,10 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_WAKEUP = 267, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_TRAY_CHANGE = 268, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_PMWAKEUP = 269, /* autogen autogen */ - REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270 /* autogen autogen */ + REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270, /* autogen autogen */ + + REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT = 271, /* autogen autogen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272 /* autogen autogen */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 9b2414f..8e00b0e 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1756,6 +1756,20 @@ struct remote_domain_snapshot_current_args { struct remote_domain_snapshot_current_ret { remote_nonnull_domain_snapshot snap; }; +struct remote_domain_snapshot_is_current_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_is_current_ret { + int current; +}; +struct remote_domain_snapshot_has_metadata_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_has_metadata_ret { + int metadata; +}; struct remote_domain_revert_to_snapshot_args { remote_nonnull_domain_snapshot snap; u_int flags; @@ -2192,4 +2206,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_TRAY_CHANGE = 268, REMOTE_PROC_DOMAIN_EVENT_PMWAKEUP = 269, REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270, + REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT = 271, + REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272, };
ACK. Osier
On 06/11/2012 12:50 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
Pretty straightforward.
* src/remote/remote_protocol.x (remote_domain_snapshot_is_current_args) (remote_domain_snapshot_is_current_ret) (remote_domain_snapshot_has_metadata_args) (remote_domain_snapshot_has_metadata_ret): New structs. (REMOTE_PROC_DOMAIN_SNAPSHOT_IS_CURRENT) (REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA): New RPC calls. * src/remote/remote_driver.c (remote_driver): Call them. * src/remote_protocol-structs: Regenerate.
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists. * src/qemu/qemu_driver.c (qemuDomainSnapshotIsCurrent) (qemuDomainSnapshotHasMetadata): New functions. * src/esx/esx_driver.c (esxDomainSnapshotIsCurrent) (esxDomainSnapshotHasMetadata): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotIsCurrent) (vboxDomainSnapshotHasMetadata): Likewise. --- src/esx/esx_driver.c | 71 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 83 +++++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index b3f1948..7de86ab 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4707,6 +4707,75 @@ esxDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) } +static int +esxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) +{ + esxPrivate *priv = snapshot->domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL; + esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (esxVI_EnsureSession(priv->primary) < 0) { + return -1; + } + + /* Check that snapshot exists. */ + if (esxVI_LookupRootSnapshotTreeList(priv->primary, snapshot->domain->uuid, + &rootSnapshotList) < 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name, + &snapshotTree, NULL, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + if (esxVI_LookupCurrentSnapshotTree(priv->primary, snapshot->domain->uuid, + ¤tSnapshotTree, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + ret = STREQ(snapshot->name, currentSnapshotTree->name); + +cleanup: + esxVI_VirtualMachineSnapshotTree_Free(¤tSnapshotTree); + esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); + return ret; +} + + +static int +esxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, unsigned int flags) +{ + esxPrivate *priv = snapshot->domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (esxVI_EnsureSession(priv->primary) < 0) { + return -1; + } + + /* Check that snapshot exists. If so, there is no metadata. */ + if (esxVI_LookupRootSnapshotTreeList(priv->primary, snapshot->domain->uuid, + &rootSnapshotList) < 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name, + &snapshotTree, NULL, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + ret = 0; + +cleanup: + esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); + return ret; +} + static int esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) @@ -5021,6 +5090,8 @@ static virDriver esxDriver = { .domainSnapshotGetParent = esxDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = esxDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = esxDomainRevertToSnapshot, /* 0.8.0 */ + .domainSnapshotIsCurrent = esxDomainSnapshotIsCurrent, /* 0.9.13 */ + .domainSnapshotHasMetadata = esxDomainSnapshotHasMetadata, /* 0.9.13 */ .domainSnapshotDelete = esxDomainSnapshotDelete, /* 0.8.0 */ .isAlive = esxIsAlive, /* 0.9.8 */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39b27b1..fadf804 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10916,6 +10916,87 @@ cleanup: return xml; } +static int +qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + virDomainSnapshotObjPtr snap = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + virUUIDFormat(snapshot->domain->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); + if (!snap) { + qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("no domain snapshot with matching name '%s'"), + snapshot->name); + goto cleanup; + } + + ret = (vm->current_snapshot && + STREQ(snapshot->name, vm->current_snapshot->def->name)); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + + +static int +qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + virDomainSnapshotObjPtr snap = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + virUUIDFormat(snapshot->domain->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name); + if (!snap) { + qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("no domain snapshot with matching name '%s'"), + snapshot->name); + goto cleanup; + } + + /* XXX Someday, we should recognize internal snapshots in qcow2 + * images that are not tied to a libvirt snapshot; if we ever do + * that, then we would have a reason to return 0 here. */ + ret = 1; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotRevertInactive(struct qemud_driver *driver, @@ -13073,6 +13154,8 @@ static virDriver qemuDriver = { .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */ .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = qemuDomainSnapshotCurrent, /* 0.8.0 */ + .domainSnapshotIsCurrent = qemuDomainSnapshotIsCurrent, /* 0.9.13 */ + .domainSnapshotHasMetadata = qemuDomainSnapshotHasMetadata, /* 0.9.13 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */ .qemuDomainMonitorCommand = qemuDomainMonitorCommand, /* 0.8.3 */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 4b0ee2e..d7ea73b 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6426,6 +6426,101 @@ cleanup: return ret; } +static int +vboxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainPtr dom = snapshot->domain; + VBOX_OBJECT_CHECK(dom->conn, int, -1); + vboxIID iid = VBOX_IID_INITIALIZER; + IMachine *machine = NULL; + ISnapshot *snap = NULL; + ISnapshot *current = NULL; + PRUnichar *nameUtf16 = NULL; + char *name = NULL; + nsresult rc; + + virCheckFlags(0, -1); + + vboxIIDFromUUID(&iid, dom->uuid); + rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); + if (NS_FAILED(rc)) { + vboxError(VIR_ERR_NO_DOMAIN, "%s", + _("no domain with matching UUID")); + goto cleanup; + } + + if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + goto cleanup; + + rc = machine->vtbl->GetCurrentSnapshot(machine, ¤t); + if (NS_FAILED(rc)) { + vboxError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not get current snapshot")); + goto cleanup; + } + if (!current) { + ret = 0; + goto cleanup; + } + + rc = current->vtbl->GetName(current, &nameUtf16); + if (NS_FAILED(rc) || !nameUtf16) { + vboxError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not get current snapshot name")); + goto cleanup; + } + + VBOX_UTF16_TO_UTF8(nameUtf16, &name); + if (!name) { + virReportOOMError(); + goto cleanup; + } + + ret = STREQ(snapshot->name, name); + +cleanup: + VBOX_UTF8_FREE(name); + VBOX_UTF16_FREE(nameUtf16); + VBOX_RELEASE(current); + VBOX_RELEASE(machine); + vboxIIDUnalloc(&iid); + return ret; +} + +static int +vboxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainPtr dom = snapshot->domain; + VBOX_OBJECT_CHECK(dom->conn, int, -1); + vboxIID iid = VBOX_IID_INITIALIZER; + IMachine *machine = NULL; + ISnapshot *snap = NULL; + nsresult rc; + + virCheckFlags(0, -1); + + vboxIIDFromUUID(&iid, dom->uuid); + rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); + if (NS_FAILED(rc)) { + vboxError(VIR_ERR_NO_DOMAIN, "%s", + _("no domain with matching UUID")); + goto cleanup; + } + + /* Check that snapshot exists. If so, there is no metadata. */ + if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) + goto cleanup; + + ret = 0; + +cleanup: + VBOX_RELEASE(machine); + vboxIIDUnalloc(&iid); + return ret; +} + #if VBOX_API_VERSION < 3001 static int vboxDomainSnapshotRestore(virDomainPtr dom, @@ -9172,6 +9267,8 @@ virDriver NAME(Driver) = { .domainHasCurrentSnapshot = vboxDomainHasCurrentSnapshot, /* 0.8.0 */ .domainSnapshotGetParent = vboxDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = vboxDomainSnapshotCurrent, /* 0.8.0 */ + .domainSnapshotIsCurrent = vboxDomainSnapshotIsCurrent, /* 0.9.13 */ + .domainSnapshotHasMetadata = vboxDomainSnapshotHasMetadata, /* 0.9.13 */ .domainRevertToSnapshot = vboxDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = vboxDomainSnapshotDelete, /* 0.8.0 */ .isAlive = vboxIsAlive, /* 0.9.8 */ -- 1.7.7.6
On 2012年05月25日 11:33, Eric Blake wrote:
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists.
* src/qemu/qemu_driver.c (qemuDomainSnapshotIsCurrent) (qemuDomainSnapshotHasMetadata): New functions. * src/esx/esx_driver.c (esxDomainSnapshotIsCurrent) (esxDomainSnapshotHasMetadata): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotIsCurrent) (vboxDomainSnapshotHasMetadata): Likewise. --- src/esx/esx_driver.c | 71 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 83 +++++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 0 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index b3f1948..7de86ab 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4707,6 +4707,75 @@ esxDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) }
+static int +esxDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) +{ + esxPrivate *priv = snapshot->domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL; + esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (esxVI_EnsureSession(priv->primary)< 0) { + return -1; + } + + /* Check that snapshot exists. */ + if (esxVI_LookupRootSnapshotTreeList(priv->primary, snapshot->domain->uuid, +&rootSnapshotList)< 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name, +&snapshotTree, NULL, + esxVI_Occurrence_RequiredItem)< 0) { + goto cleanup; + } + + if (esxVI_LookupCurrentSnapshotTree(priv->primary, snapshot->domain->uuid, +¤tSnapshotTree, + esxVI_Occurrence_RequiredItem)< 0) { + goto cleanup; + } + + ret = STREQ(snapshot->name, currentSnapshotTree->name); + +cleanup: + esxVI_VirtualMachineSnapshotTree_Free(¤tSnapshotTree); + esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); + return ret; +} + + +static int +esxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, unsigned int flags) +{ + esxPrivate *priv = snapshot->domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (esxVI_EnsureSession(priv->primary)< 0) { + return -1; + } + + /* Check that snapshot exists. If so, there is no metadata. */ + if (esxVI_LookupRootSnapshotTreeList(priv->primary, snapshot->domain->uuid, +&rootSnapshotList)< 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name, +&snapshotTree, NULL, + esxVI_Occurrence_RequiredItem)< 0) { + goto cleanup; + } + + ret = 0; +
Looks like it doesn't have metadata anyway for esx and vbox driver, is it useful then? Osier
On 06/11/2012 01:14 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists.
Looks like it doesn't have metadata anyway for esx and vbox driver, is it useful then?
Right now, we have: qemu: always uses metadata [*] esx, vbox: never uses metadata (instead, the driver queries the hypervisor every time) [*] but this may change - I can see adding a flag to let the qemu driver report snapshots without metadata, corresponding to qcow2 internal images Furthermore, the existence of metadata is useful to know; with qemu, a snapshot prevents deletion of a persistent domain, with esx and vbox, all domains are persistent but the existence of a snapshot has no effect on the ability to delete the domain. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年06月11日 20:41, Eric Blake wrote:
On 06/11/2012 01:14 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists.
Looks like it doesn't have metadata anyway for esx and vbox driver, is it useful then?
Right now, we have:
qemu: always uses metadata [*] esx, vbox: never uses metadata (instead, the driver queries the hypervisor every time)
[*] but this may change - I can see adding a flag to let the qemu driver report snapshots without metadata, corresponding to qcow2 internal images
Furthermore, the existence of metadata is useful to know; with qemu, a snapshot prevents deletion of a persistent domain, with esx and vbox, all domains are persistent but the existence of a snapshot has no effect on the ability to delete the domain.
It's fine for qemu driver, as there will be changes in future. But not sure if checking whether the snapshot exists or not in hasMetadata is good or not for esx and vbox driver. It seems to me duplicate with domainSnapshotLookupByName, in case of it's very likely no changes (support to have metadata) for vbox and esx driver in future. Perhaps simply returns 0 is better. Osier
On 06/11/2012 08:31 AM, Osier Yang wrote:
On 2012年06月11日 20:41, Eric Blake wrote:
On 06/11/2012 01:14 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists.
But not sure if checking whether the snapshot exists or not in hasMetadata is good or not for esx and vbox driver.
I think it _is_ good - we are returning 0 or -1 (snapshot exists, or snapshot does not exist), as opposed to blindly returning 0 (snapshot exists, even when it doesn't).
It seems to me duplicate with domainSnapshotLookupByName, in case of it's very likely no changes (support to have metadata) for vbox and esx driver in future. Perhaps simply returns 0 is better.
I don't see the point in taking the shortcut of always returning 0, as that is no longer correct when the snapshot does not exist. And yes, that means I think our current implementation of esxDomainIsPersistent() is broken (it always returns 1 instead of checking for existence). But I guess that means I should either write a followup patch that fixes the other broken functions that don't check for existence, or that I can be convinced to change HasMetadata to always return 0 because it is no more broken than the existing functions. Anyone else have an opinion on whether it is better to do existence checks rather than blindly succeeding? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/11/2012 08:31 AM, Osier Yang wrote:
On 2012年06月11日 20:41, Eric Blake wrote:
On 06/11/2012 01:14 AM, Osier Yang wrote:
On 2012年05月25日 11:33, Eric Blake wrote:
The two APIs are rather trivial; based on bits and pieces of other existing APIs. Rather than blindly return 0 or 1 for HasMetadata, I chose to first validate that the snapshot in question in fact exists.
Furthermore, the existence of metadata is useful to know; with qemu, a snapshot prevents deletion of a persistent domain, with esx and vbox, all domains are persistent but the existence of a snapshot has no effect on the ability to delete the domain.
It's fine for qemu driver, as there will be changes in future.
I've gone ahead and pushed the qemu changes. I'll post two alternatives for a v2 for the esx and vbox patch. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Osier Yang