[libvirt] [PATCH 0/5] snapshot-list --tree
I liked 'nodedev-list --tree' so much that I wanted the same for 'snapshot-list domain --tree'. Eric Blake (5): snapshot: new virDomainSnapshotGetParent API snapshot: remote protocol for getparent snapshot: refactor virsh snapshot parent computation snapshot: add virsh snapshot-list --tree snapshot: implement getparent in qemu include/libvirt/libvirt.h.in | 4 + src/driver.h | 5 ++ src/libvirt.c | 44 +++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 46 +++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 8 ++ src/rpc/gendispatch.pl | 4 +- tools/virsh.c | 147 ++++++++++++++++++++++++++++++++++-------- tools/virsh.pod | 14 ++-- 11 files changed, 254 insertions(+), 36 deletions(-) -- 1.7.4.4
Although a client can already obtain a snapshot's parent by dumping and parsing the xml, then doing a snapshot lookup by name, it is more efficient to get the parent in one step, which in turn will make operations that must traverse a snapshot hierarchy easier to perform. * include/libvirt/libvirt.h.in (virDomainSnapshotGetParent): Declare. * src/libvirt.c (virDomainSnapshotGetParent): New function. * src/libvirt_public.syms: Export it. * src/driver.h (virDrvDomainSnapshotGetParent): New callback. --- include/libvirt/libvirt.h.in | 4 +++ src/driver.h | 5 ++++ src/libvirt.c | 44 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 58 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 39155a6..afeb83f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2711,6 +2711,10 @@ int virDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags); virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags); +/* Get a handle to the parent snapshot, if one exists */ +virDomainSnapshotPtr virDomainSnapshotGetParent(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 3792003..7dcab8f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -590,6 +590,10 @@ typedef int (*virDrvDomainHasCurrentSnapshot)(virDomainPtr domain, unsigned int flags); typedef virDomainSnapshotPtr + (*virDrvDomainSnapshotGetParent)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef virDomainSnapshotPtr (*virDrvDomainSnapshotCurrent)(virDomainPtr domain, unsigned int flags); @@ -854,6 +858,7 @@ struct _virDriver { virDrvDomainSnapshotListNames domainSnapshotListNames; virDrvDomainSnapshotLookupByName domainSnapshotLookupByName; virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot; + virDrvDomainSnapshotGetParent domainSnapshotGetParent; virDrvDomainSnapshotCurrent domainSnapshotCurrent; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; diff --git a/src/libvirt.c b/src/libvirt.c index 8f94b11..38fcfbc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16150,6 +16150,50 @@ error: } /** + * virDomainSnapshotGetParent: + * @snapshot: a snapshot object + * @flags: unused flag parameters; callers should pass 0 + * + * Get the parent snapshot for @snapshot, if any. + * + * Returns a domain snapshot object or NULL in case of failure. If the + * given snapshot is a root (no parent), then the VIR_ERR_NO_DOMAIN_SNAPSHOT + * error is raised. + */ +virDomainSnapshotPtr +virDomainSnapshotGetParent(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 NULL; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotGetParent) { + virDomainSnapshotPtr snap; + snap = conn->driver->domainSnapshotGetParent(snapshot, flags); + if (!snap) + goto error; + return snap; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return NULL; +} + +/** * virDomainRevertToSnapshot: * @snapshot: a domain snapshot object * @flags: bitwise-OR of virDomainSnapshotRevertFlags diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 8a6d55a..cef14f0 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -489,4 +489,9 @@ LIBVIRT_0.9.5 { virDomainSnapshotGetName; } LIBVIRT_0.9.4; +LIBVIRT_0.9.7 { + global: + virDomainSnapshotGetParent; +} LIBVIRT_0.9.5; + # .... define new API here using predicted next version number .... -- 1.7.4.4
On Sat, Sep 24, 2011 at 06:30:02PM -0600, Eric Blake wrote:
Although a client can already obtain a snapshot's parent by dumping and parsing the xml, then doing a snapshot lookup by name, it is more efficient to get the parent in one step, which in turn will make operations that must traverse a snapshot hierarchy easier to perform.
* include/libvirt/libvirt.h.in (virDomainSnapshotGetParent): Declare. * src/libvirt.c (virDomainSnapshotGetParent): New function. * src/libvirt_public.syms: Export it. * src/driver.h (virDrvDomainSnapshotGetParent): New callback. --- include/libvirt/libvirt.h.in | 4 +++ src/driver.h | 5 ++++ src/libvirt.c | 44 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 39155a6..afeb83f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2711,6 +2711,10 @@ int virDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags); virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags);
+/* Get a handle to the parent snapshot, if one exists */ +virDomainSnapshotPtr virDomainSnapshotGetParent(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 3792003..7dcab8f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -590,6 +590,10 @@ typedef int (*virDrvDomainHasCurrentSnapshot)(virDomainPtr domain, unsigned int flags);
typedef virDomainSnapshotPtr + (*virDrvDomainSnapshotGetParent)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef virDomainSnapshotPtr (*virDrvDomainSnapshotCurrent)(virDomainPtr domain, unsigned int flags);
@@ -854,6 +858,7 @@ struct _virDriver { virDrvDomainSnapshotListNames domainSnapshotListNames; virDrvDomainSnapshotLookupByName domainSnapshotLookupByName; virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot; + virDrvDomainSnapshotGetParent domainSnapshotGetParent; virDrvDomainSnapshotCurrent domainSnapshotCurrent; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; diff --git a/src/libvirt.c b/src/libvirt.c index 8f94b11..38fcfbc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16150,6 +16150,50 @@ error: }
/** + * virDomainSnapshotGetParent: + * @snapshot: a snapshot object + * @flags: unused flag parameters; callers should pass 0 + * + * Get the parent snapshot for @snapshot, if any. + * + * Returns a domain snapshot object or NULL in case of failure. If the + * given snapshot is a root (no parent), then the VIR_ERR_NO_DOMAIN_SNAPSHOT + * error is raised. + */ +virDomainSnapshotPtr +virDomainSnapshotGetParent(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 NULL; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotGetParent) { + virDomainSnapshotPtr snap; + snap = conn->driver->domainSnapshotGetParent(snapshot, flags); + if (!snap) + goto error; + return snap; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return NULL; +} + +/** * virDomainRevertToSnapshot: * @snapshot: a domain snapshot object * @flags: bitwise-OR of virDomainSnapshotRevertFlags diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 8a6d55a..cef14f0 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -489,4 +489,9 @@ LIBVIRT_0.9.5 { virDomainSnapshotGetName; } LIBVIRT_0.9.4;
+LIBVIRT_0.9.7 { + global: + virDomainSnapshotGetParent; +} LIBVIRT_0.9.5; + # .... define new API here using predicted next version number .... -- 1.7.4.4
ACK, interface and intent looks fine ! I have seen another patch today touching the libvirt_public.syms though, tweaking may be needed by someone :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Mostly straight-forward, although this is the first API that returns a new snapshot based on a snapshot rather than a domain. * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT): New rpc. (remote_domain_snapshot_get_parent_args) (remote_domain_snapshot_get_parent_ret): New structs. * src/rpc/gendispatch.pl: Adjust generator. * src/remote/remote_driver.c (remote_driver): Use it. * src/remote_protocol-structs: Update. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 8 ++++++++ src/rpc/gendispatch.pl | 4 +++- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1217d94..740dd75 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4412,6 +4412,7 @@ static virDriver remote_driver = { .domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */ .domainSnapshotLookupByName = remoteDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = remoteDomainHasCurrentSnapshot, /* 0.8.0 */ + .domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = remoteDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = remoteDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 455e324..3504e34 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2081,6 +2081,15 @@ struct remote_domain_has_current_snapshot_ret { int result; }; +struct remote_domain_snapshot_get_parent_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_get_parent_ret { + remote_nonnull_domain_snapshot snap; +}; + struct remote_domain_snapshot_current_args { remote_nonnull_domain dom; unsigned int flags; @@ -2509,7 +2518,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ - REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244 /* 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 810b19c..53705bf 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1568,6 +1568,13 @@ struct remote_domain_has_current_snapshot_args { struct remote_domain_has_current_snapshot_ret { int result; }; +struct remote_domain_snapshot_get_parent_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_get_parent_ret { + remote_nonnull_domain_snapshot snap; +}; struct remote_domain_snapshot_current_args { remote_nonnull_domain dom; u_int flags; @@ -1958,4 +1965,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, + REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index fcd1a26..039d785 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1273,7 +1273,9 @@ elsif ($opt_k) { $single_ret_type = "int"; } else { if ($name eq "domain_snapshot") { - push(@ret_list, "rv = get_nonnull_$name(dom, ret.$arg_name);"); + my $dom = "$priv_src"; + $dom =~ s/->conn//; + push(@ret_list, "rv = get_nonnull_$name($dom, ret.$arg_name);"); } else { push(@ret_list, "rv = get_nonnull_$name($priv_src, ret.$arg_name);"); } -- 1.7.4.4
On Sat, Sep 24, 2011 at 06:30:03PM -0600, Eric Blake wrote:
Mostly straight-forward, although this is the first API that returns a new snapshot based on a snapshot rather than a domain.
* src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT): New rpc. (remote_domain_snapshot_get_parent_args) (remote_domain_snapshot_get_parent_ret): New structs. * src/rpc/gendispatch.pl: Adjust generator. * src/remote/remote_driver.c (remote_driver): Use it. * src/remote_protocol-structs: Update. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 8 ++++++++ src/rpc/gendispatch.pl | 4 +++- 4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1217d94..740dd75 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4412,6 +4412,7 @@ static virDriver remote_driver = { .domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */ .domainSnapshotLookupByName = remoteDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = remoteDomainHasCurrentSnapshot, /* 0.8.0 */ + .domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = remoteDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = remoteDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 455e324..3504e34 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2081,6 +2081,15 @@ struct remote_domain_has_current_snapshot_ret { int result; };
+struct remote_domain_snapshot_get_parent_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_get_parent_ret { + remote_nonnull_domain_snapshot snap; +}; + struct remote_domain_snapshot_current_args { remote_nonnull_domain dom; unsigned int flags; @@ -2509,7 +2518,8 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */ - REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244 /* 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 810b19c..53705bf 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1568,6 +1568,13 @@ struct remote_domain_has_current_snapshot_args { struct remote_domain_has_current_snapshot_ret { int result; }; +struct remote_domain_snapshot_get_parent_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_get_parent_ret { + remote_nonnull_domain_snapshot snap; +}; struct remote_domain_snapshot_current_args { remote_nonnull_domain dom; u_int flags; @@ -1958,4 +1965,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, + REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index fcd1a26..039d785 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1273,7 +1273,9 @@ elsif ($opt_k) { $single_ret_type = "int"; } else { if ($name eq "domain_snapshot") { - push(@ret_list, "rv = get_nonnull_$name(dom, ret.$arg_name);"); + my $dom = "$priv_src"; + $dom =~ s/->conn//; + push(@ret_list, "rv = get_nonnull_$name($dom, ret.$arg_name);"); } else { push(@ret_list, "rv = get_nonnull_$name($priv_src, ret.$arg_name);"); }
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Make parent computation reusable, using virDomainSnapshotGetParent when possible. * tools/virsh.c (vshGetSnapshotParent): New helper. (cmdSnapshotParent): Use it. --- tools/virsh.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 51 insertions(+), 15 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7b0533d..035b209 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -246,6 +246,8 @@ typedef struct __vshControl { char *historyfile; /* readline history file name */ bool useGetInfo; /* must use virDomainGetInfo, since virDomainGetState is not supported */ + bool useSnapshotGetXML; /* must use virDomainSnapshotGetXMLDesc, since + virDomainSnapshotGetParent is missing */ } __vshControl; typedef struct vshCmdGrp { @@ -613,6 +615,7 @@ vshReconnect(vshControl *ctl) vshError(ctl, "%s", _("Reconnected to the hypervisor")); disconnected = 0; ctl->useGetInfo = false; + ctl->useSnapshotGetXML = false; } /* --------------- @@ -760,6 +763,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) ctl->name = vshStrdup(ctl, name); ctl->useGetInfo = false; + ctl->useSnapshotGetXML = false; ctl->readonly = ro; ctl->conn = virConnectOpenAuth(ctl->name, virConnectAuthPtrDefault, @@ -12967,6 +12971,52 @@ cleanup: return ret; } +/* Helper function to get the name of a snapshot's parent. Caller + * must free the result. */ +static char * +vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) +{ + virDomainSnapshotPtr parent = NULL; + char *xml = NULL; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + char *parent_name = NULL; + + /* Try new API, since it is faster. */ + if (!ctl->useSnapshotGetXML) { + parent = virDomainSnapshotGetParent(snapshot, 0); + if (parent) { + /* API works, and virDomainSnapshotGetName will succeed */ + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + goto cleanup; + } + if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) { + /* API works, and we found a root with no parent */ + goto cleanup; + } + /* API didn't work, fall back to XML scraping. */ + ctl->useSnapshotGetXML = true; + } + + xml = virDomainSnapshotGetXMLDesc(snapshot, 0); + if (!xml) + goto cleanup; + + xmldoc = virXMLParseStringCtxt(xml, _("(domain_snapshot)"), &ctxt); + if (!xmldoc) + goto cleanup; + + parent_name = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + +cleanup: + if (parent) + virDomainSnapshotFree(parent); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + VIR_FREE(xml); + return parent_name; +} + /* * "snapshot-list" command */ @@ -13220,10 +13270,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) bool ret = false; const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; - char *xml = NULL; char *parent = NULL; - xmlDocPtr xmldoc = NULL; - xmlXPathContextPtr ctxt = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -13239,15 +13286,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup; - xml = virDomainSnapshotGetXMLDesc(snapshot, 0); - if (!xml) - goto cleanup; - - xmldoc = virXMLParseStringCtxt(xml, _("(domain_snapshot)"), &ctxt); - if (!xmldoc) - goto cleanup; - - parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + parent = vshGetSnapshotParent(ctl, snapshot); if (!parent) goto cleanup; @@ -13257,9 +13296,6 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(parent); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(xmldoc); - VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); if (dom) -- 1.7.4.4
On Sat, Sep 24, 2011 at 06:30:04PM -0600, Eric Blake wrote:
Make parent computation reusable, using virDomainSnapshotGetParent when possible.
* tools/virsh.c (vshGetSnapshotParent): New helper. (cmdSnapshotParent): Use it. --- tools/virsh.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 51 insertions(+), 15 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 7b0533d..035b209 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -246,6 +246,8 @@ typedef struct __vshControl { char *historyfile; /* readline history file name */ bool useGetInfo; /* must use virDomainGetInfo, since virDomainGetState is not supported */ + bool useSnapshotGetXML; /* must use virDomainSnapshotGetXMLDesc, since + virDomainSnapshotGetParent is missing */ } __vshControl;
typedef struct vshCmdGrp { @@ -613,6 +615,7 @@ vshReconnect(vshControl *ctl) vshError(ctl, "%s", _("Reconnected to the hypervisor")); disconnected = 0; ctl->useGetInfo = false; + ctl->useSnapshotGetXML = false; }
/* --------------- @@ -760,6 +763,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) ctl->name = vshStrdup(ctl, name);
ctl->useGetInfo = false; + ctl->useSnapshotGetXML = false; ctl->readonly = ro;
ctl->conn = virConnectOpenAuth(ctl->name, virConnectAuthPtrDefault, @@ -12967,6 +12971,52 @@ cleanup: return ret; }
+/* Helper function to get the name of a snapshot's parent. Caller + * must free the result. */ +static char * +vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) +{ + virDomainSnapshotPtr parent = NULL; + char *xml = NULL; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + char *parent_name = NULL; + + /* Try new API, since it is faster. */ + if (!ctl->useSnapshotGetXML) { + parent = virDomainSnapshotGetParent(snapshot, 0); + if (parent) { + /* API works, and virDomainSnapshotGetName will succeed */ + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + goto cleanup; + } + if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) { + /* API works, and we found a root with no parent */ + goto cleanup; + } + /* API didn't work, fall back to XML scraping. */ + ctl->useSnapshotGetXML = true; + } + + xml = virDomainSnapshotGetXMLDesc(snapshot, 0); + if (!xml) + goto cleanup; + + xmldoc = virXMLParseStringCtxt(xml, _("(domain_snapshot)"), &ctxt); + if (!xmldoc) + goto cleanup; + + parent_name = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + +cleanup: + if (parent) + virDomainSnapshotFree(parent); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + VIR_FREE(xml); + return parent_name; +} + /* * "snapshot-list" command */ @@ -13220,10 +13270,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) bool ret = false; const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; - char *xml = NULL; char *parent = NULL; - xmlDocPtr xmldoc = NULL; - xmlXPathContextPtr ctxt = NULL;
if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -13239,15 +13286,7 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup;
- xml = virDomainSnapshotGetXMLDesc(snapshot, 0); - if (!xml) - goto cleanup; - - xmldoc = virXMLParseStringCtxt(xml, _("(domain_snapshot)"), &ctxt); - if (!xmldoc) - goto cleanup; - - parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + parent = vshGetSnapshotParent(ctl, snapshot); if (!parent) goto cleanup;
@@ -13257,9 +13296,6 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd)
cleanup: VIR_FREE(parent); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(xmldoc); - VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); if (dom)
Okay, that works even if one reopen a different connection in shell mode ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Reuse the tree listing of nodedev-list, coupled with the new helper function to efficiently grab snapshot parent names, to produce tree output for a snapshot hierarchy. For example: $ virsh snapshot-list dom --tree root1 | +- sibling1 +- sibling2 | | | +- grandchild | +- sibling3 root2 | +- child * tools/virsh.c (cmdSnapshotList): Add --tree. * tools/virsh.pod (snapshot-list): Document it. --- tools/virsh.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------- tools/virsh.pod | 14 +++++---- 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 035b209..7e7b65d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) { @@ -13032,6 +13032,7 @@ static const vshCmdOptDef opts_snapshot_list[] = { {"roots", VSH_OT_BOOL, 0, N_("list only snapshots without parents")}, {"metadata", VSH_OT_BOOL, 0, N_("list only snapshots that have metadata that would prevent undefine")}, + {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")}, {NULL, 0, 0, NULL} }; @@ -13057,6 +13058,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) time_t creation_time_t; char timestr[100]; struct tm time_info; + bool tree = vshCommandOptBool(cmd, "tree"); if (vshCommandOptBool(cmd, "parent")) { if (vshCommandOptBool(cmd, "roots")) { @@ -13064,8 +13066,18 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) _("--parent and --roots are mutually exlusive")); return false; } + if (tree) { + vshError(ctl, "%s", + _("--parent and --tree are mutually exlusive")); + return false; + } parent_filter = 1; } else if (vshCommandOptBool(cmd, "roots")) { + if (tree) { + vshError(ctl, "%s", + _("--roots and --tree are mutually exlusive")); + return false; + } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; } @@ -13095,23 +13107,66 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (numsnaps < 0) goto cleanup; - if (parent_filter > 0) - vshPrintExtra(ctl, " %-20s %-25s %-15s %s", - _("Name"), _("Creation Time"), _("State"), _("Parent")); - else - vshPrintExtra(ctl, " %-20s %-25s %s", - _("Name"), _("Creation Time"), _("State")); - vshPrintExtra(ctl, "\n\ + if (!tree) { + if (parent_filter > 0) + vshPrintExtra(ctl, " %-20s %-25s %-15s %s", + _("Name"), _("Creation Time"), _("State"), + _("Parent")); + else + vshPrintExtra(ctl, " %-20s %-25s %s", + _("Name"), _("Creation Time"), _("State")); + vshPrintExtra(ctl, "\n\ ------------------------------------------------------------\n"); + } - if (numsnaps) { - if (VIR_ALLOC_N(names, numsnaps) < 0) - goto cleanup; + if (!numsnaps) { + ret = true; + goto cleanup; + } - actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); - if (actual < 0) - goto cleanup; + if (VIR_ALLOC_N(names, numsnaps) < 0) + goto cleanup; + + actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); + if (actual < 0) + goto cleanup; + if (tree) { + char indentBuf[INDENT_BUFLEN]; + char **parents = vshMalloc(ctl, sizeof(char *) * actual); + for (i = 0; i < actual; i++) { + /* free up memory from previous iterations of the loop */ + if (snapshot) + virDomainSnapshotFree(snapshot); + snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); + if (!snapshot) { + while (--i >= 0) + VIR_FREE(parents[i]); + VIR_FREE(parents); + goto cleanup; + } + parents[i] = vshGetSnapshotParent(ctl, snapshot); + } + for (i = 0 ; i < actual ; i++) { + memset(indentBuf, '\0', sizeof indentBuf); + if (parents[i] == NULL) + cmdNodeListDevicesPrint(ctl, + names, + parents, + actual, + i, + i, + 0, + 0, + indentBuf); + } + for (i = 0 ; i < actual ; i++) + VIR_FREE(parents[i]); + VIR_FREE(parents); + + ret = true; + goto cleanup; + } else { qsort(&names[0], actual, sizeof(char*), namesorter); for (i = 0; i < actual; i++) { diff --git a/tools/virsh.pod b/tools/virsh.pod index ddf7578..49aa42a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1934,15 +1934,17 @@ except that it does some error checking. The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment variables, and defaults to C<vi>. -=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots>}] [I<--metadata>] +=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}] +[I<--metadata>] -List all of the available snapshots for the given domain. +List all of the available snapshots for the given domain, defaulting +to show columns for the snapshot name, creation time, and domain state. If I<--parent> is specified, add a column to the output table giving -the name of the parent of each snapshot. - -If I<--roots> is specified, the list will be filtered to just snapshots -that have no parents; this option is not compatible with I<--parent>. +the name of the parent of each snapshot. If I<--roots> is specified, +the list will be filtered to just snapshots that have no parents. +If I<--tree> is specified, the output will be in a tree format, listing +just snapshot names. These three options are mutually exclusive. If I<--metadata> is specified, the list will be filtered to just snapshots that involve libvirt metadata, and thus would prevent -- 1.7.4.4
On Sat, Sep 24, 2011 at 06:30:05PM -0600, Eric Blake wrote:
Reuse the tree listing of nodedev-list, coupled with the new helper function to efficiently grab snapshot parent names, to produce tree output for a snapshot hierarchy. For example:
$ virsh snapshot-list dom --tree root1 | +- sibling1 +- sibling2 | | | +- grandchild | +- sibling3
root2 | +- child
* tools/virsh.c (cmdSnapshotList): Add --tree. * tools/virsh.pod (snapshot-list): Document it. --- tools/virsh.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------- tools/virsh.pod | 14 +++++---- 2 files changed, 77 insertions(+), 20 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 035b209..7e7b65d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {
That looks like a bug fix squashed in here instead of previous patch, what am I missing ?
@@ -13032,6 +13032,7 @@ static const vshCmdOptDef opts_snapshot_list[] = { {"roots", VSH_OT_BOOL, 0, N_("list only snapshots without parents")}, {"metadata", VSH_OT_BOOL, 0, N_("list only snapshots that have metadata that would prevent undefine")}, + {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")}, {NULL, 0, 0, NULL} };
@@ -13057,6 +13058,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) time_t creation_time_t; char timestr[100]; struct tm time_info; + bool tree = vshCommandOptBool(cmd, "tree");
if (vshCommandOptBool(cmd, "parent")) { if (vshCommandOptBool(cmd, "roots")) { @@ -13064,8 +13066,18 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) _("--parent and --roots are mutually exlusive")); return false; } + if (tree) { + vshError(ctl, "%s", + _("--parent and --tree are mutually exlusive")); + return false; + } parent_filter = 1; } else if (vshCommandOptBool(cmd, "roots")) { + if (tree) { + vshError(ctl, "%s", + _("--roots and --tree are mutually exlusive")); + return false; + } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; }
@@ -13095,23 +13107,66 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (numsnaps < 0) goto cleanup;
- if (parent_filter > 0) - vshPrintExtra(ctl, " %-20s %-25s %-15s %s", - _("Name"), _("Creation Time"), _("State"), _("Parent")); - else - vshPrintExtra(ctl, " %-20s %-25s %s", - _("Name"), _("Creation Time"), _("State")); - vshPrintExtra(ctl, "\n\ + if (!tree) { + if (parent_filter > 0) + vshPrintExtra(ctl, " %-20s %-25s %-15s %s", + _("Name"), _("Creation Time"), _("State"), + _("Parent")); + else + vshPrintExtra(ctl, " %-20s %-25s %s", + _("Name"), _("Creation Time"), _("State")); + vshPrintExtra(ctl, "\n\ ------------------------------------------------------------\n"); + }
- if (numsnaps) { - if (VIR_ALLOC_N(names, numsnaps) < 0) - goto cleanup; + if (!numsnaps) { + ret = true; + goto cleanup; + }
- actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); - if (actual < 0) - goto cleanup; + if (VIR_ALLOC_N(names, numsnaps) < 0) + goto cleanup; + + actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); + if (actual < 0) + goto cleanup;
+ if (tree) { + char indentBuf[INDENT_BUFLEN]; + char **parents = vshMalloc(ctl, sizeof(char *) * actual); + for (i = 0; i < actual; i++) { + /* free up memory from previous iterations of the loop */ + if (snapshot) + virDomainSnapshotFree(snapshot); + snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); + if (!snapshot) { + while (--i >= 0) + VIR_FREE(parents[i]); + VIR_FREE(parents); + goto cleanup; + } + parents[i] = vshGetSnapshotParent(ctl, snapshot); + } + for (i = 0 ; i < actual ; i++) { + memset(indentBuf, '\0', sizeof indentBuf); + if (parents[i] == NULL) + cmdNodeListDevicesPrint(ctl, + names, + parents, + actual, + i, + i, + 0, + 0, + indentBuf); + } + for (i = 0 ; i < actual ; i++) + VIR_FREE(parents[i]); + VIR_FREE(parents); + + ret = true; + goto cleanup; + } else { qsort(&names[0], actual, sizeof(char*), namesorter);
for (i = 0; i < actual; i++) { diff --git a/tools/virsh.pod b/tools/virsh.pod index ddf7578..49aa42a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1934,15 +1934,17 @@ except that it does some error checking. The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment variables, and defaults to C<vi>.
-=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots>}] [I<--metadata>] +=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}] +[I<--metadata>]
-List all of the available snapshots for the given domain. +List all of the available snapshots for the given domain, defaulting +to show columns for the snapshot name, creation time, and domain state.
If I<--parent> is specified, add a column to the output table giving -the name of the parent of each snapshot. - -If I<--roots> is specified, the list will be filtered to just snapshots -that have no parents; this option is not compatible with I<--parent>. +the name of the parent of each snapshot. If I<--roots> is specified, +the list will be filtered to just snapshots that have no parents. +If I<--tree> is specified, the output will be in a tree format, listing +just snapshot names. These three options are mutually exclusive.
If I<--metadata> is specified, the list will be filtered to just snapshots that involve libvirt metadata, and thus would prevent
ACK, great improvement, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/28/2011 07:21 AM, Daniel Veillard wrote:
On Sat, Sep 24, 2011 at 06:30:05PM -0600, Eric Blake wrote:
Reuse the tree listing of nodedev-list, coupled with the new helper function to efficiently grab snapshot parent names, to produce tree output for a snapshot hierarchy. For example:
$ virsh snapshot-list dom --tree root1 | +- sibling1 +- sibling2
@@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {
That looks like a bug fix squashed in here instead of previous patch, what am I missing ?
Nothing. It was indeed a fix that I had made after the fact, then merely squashed to the wrong patch when rebasing it back in. I've floated this hunk up to patch 3/5.
ACK, great improvement,
Code reuse is nice, too - I was dreading writing the tree display code, until I realized with my recent doc efforts that nodedev-list already did it for me :) I've now pushed the amended series (well, through patch 5; patches to esx and vbox still need to be written and reviewed). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, Sep 28, 2011 at 10:07:46AM -0600, Eric Blake wrote:
On 09/28/2011 07:21 AM, Daniel Veillard wrote:
On Sat, Sep 24, 2011 at 06:30:05PM -0600, Eric Blake wrote:
Reuse the tree listing of nodedev-list, coupled with the new helper function to efficiently grab snapshot parent names, to produce tree output for a snapshot hierarchy. For example:
$ virsh snapshot-list dom --tree root1 | +- sibling1 +- sibling2
@@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot)); + parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {
That looks like a bug fix squashed in here instead of previous patch, what am I missing ?
Nothing. It was indeed a fix that I had made after the fact, then merely squashed to the wrong patch when rebasing it back in. I've floated this hunk up to patch 3/5.
Okay
ACK, great improvement,
Code reuse is nice, too - I was dreading writing the tree display code, until I realized with my recent doc efforts that nodedev-list already did it for me :)
I've now pushed the amended series (well, through patch 5; patches to esx and vbox still need to be written and reviewed).
Yeah no hurry, maybe Matthias can have a look at the ESX part, all the infrastructire is there now :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
First hypervisor implementation of the new API. Allows 'virsh snapshot-list --tree' to be more efficient. * src/qemu/qemu_driver.c (qemuDomainSnapshotGetParent): New function. --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 46 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 08310b4..47dde3f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9451,6 +9451,51 @@ cleanup: return ret; } +static virDomainSnapshotPtr +qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotPtr parent = NULL; + + virCheckFlags(0, NULL); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(snapshot->domain->uuid, uuidstr); + 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; + } + + if (!snap->def->parent) { + qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("snapshot '%s' does not have a parent"), + snap->def->name); + goto cleanup; + } + + parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return parent; +} + static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) { @@ -10432,6 +10477,7 @@ static virDriver qemuDriver = { .domainSnapshotListNames = qemuDomainSnapshotListNames, /* 0.8.0 */ .domainSnapshotLookupByName = qemuDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */ + .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = qemuDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */ -- 1.7.4.4
On Sat, Sep 24, 2011 at 06:30:06PM -0600, Eric Blake wrote:
First hypervisor implementation of the new API. Allows 'virsh snapshot-list --tree' to be more efficient.
* src/qemu/qemu_driver.c (qemuDomainSnapshotGetParent): New function. --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 08310b4..47dde3f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9451,6 +9451,51 @@ cleanup: return ret; }
+static virDomainSnapshotPtr +qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotPtr parent = NULL; + + virCheckFlags(0, NULL); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(snapshot->domain->uuid, uuidstr); + 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; + } + + if (!snap->def->parent) { + qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("snapshot '%s' does not have a parent"), + snap->def->name); + goto cleanup; + } + + parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return parent; +} + static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) { @@ -10432,6 +10477,7 @@ static virDriver qemuDriver = { .domainSnapshotListNames = qemuDomainSnapshotListNames, /* 0.8.0 */ .domainSnapshotLookupByName = qemuDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */ + .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = qemuDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */
ACK, any change to get this implemented for other drivers ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/28/2011 07:22 AM, Daniel Veillard wrote:
@@ -10432,6 +10477,7 @@ static virDriver qemuDriver = { .domainSnapshotListNames = qemuDomainSnapshotListNames, /* 0.8.0 */ .domainSnapshotLookupByName = qemuDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */ + .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */ .domainSnapshotCurrent = qemuDomainSnapshotCurrent, /* 0.8.0 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */
ACK, any change to get this implemented for other drivers ?
I don't have ESX or VBox set up to test things, but I'll give the code a shot if someone else can review the actual implementation. Those are the only two other hypervisors with snapshots at the moment, and since they already have the ability to report parent information into the generated xml, I believe it should be doable to implement this new function. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel Veillard -
Eric Blake