[libvirt] [PATCH 0/4] snapshot: listing children
My previous API added parent traversal; this one adds child traversal. virsh snapshot-list domain snapshot --tree is a nice end result for limiting the output to a particular subset of the overall hierarchy. Again, same story about not wiring up ESX and VBox support yet, but I'll play with that next. Eric Blake (4): snapshot: new virDomainSnapshotListChildrenNames API snapshot: virsh snapshot-list and children snapshot: remote protocol for snapshot children snapshot: implement snapshot children listing in qemu include/libvirt/libvirt.h.in | 27 +++++++-- python/generator.py | 4 ++ python/libvirt-override-api.xml | 12 +++- python/libvirt-override.c | 45 ++++++++++++++++ src/conf/domain_conf.c | 51 ++++++++++++++++++ src/conf/domain_conf.h | 7 +++ src/driver.h | 12 ++++ src/libvirt.c | 111 +++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 87 ++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 25 ++++++++- src/remote_protocol-structs | 20 +++++++ tools/virsh.c | 64 +++++++++++++++++++--- tools/virsh.pod | 9 +++- 16 files changed, 459 insertions(+), 21 deletions(-) -- 1.7.4.4
The previous API addition allowed traversal up the hierarchy; this one makes it easier to traverse down the hierarchy. In the python bindings, virDomainSnapshotNumChildren can be generated, but virDomainSnapshotListChildrenNames had to copy from the hand-written example of virDomainSnapshotListNames. * include/libvirt/libvirt.h.in (virDomainSnapshotNumChildren) (virDomainSnapshotListChildrenNames): New prototypes. (VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS): New flag alias. * src/libvirt.c (virDomainSnapshotNumChildren) (virDomainSnapshotListChildrenNames): New functions. * src/libvirt_public.syms: Export them. * src/driver.h (virDrvDomainSnapshotNumChildren) (virDrvDomainSnapshotListChildrenNames): New callbacks. * python/generator.py (skip_impl, nameFixup): Update lists. * python/libvirt-override-api.xml: Likewise. * python/libvirt-override.c (libvirt_virDomainSnapshotListChildrenNames): New wrapper function. --- include/libvirt/libvirt.h.in | 27 +++++++-- python/generator.py | 4 ++ python/libvirt-override-api.xml | 12 +++- python/libvirt-override.c | 45 ++++++++++++++++ src/driver.h | 12 ++++ src/libvirt.c | 111 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 7 files changed, 204 insertions(+), 9 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a3c581d..3c7f278 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2686,13 +2686,19 @@ virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain, char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags); -/* Flags valid for both virDomainSnapshotNum() and - * virDomainSnapshotListNames(). */ +/* Flags valid for virDomainSnapshotNum(), + * virDomainSnapshotListNames(), virDomainSnapshotNumChildren(), and + * virDomainSnapshotListChildrenNames(). Note that the interpretation + * of flag (1<<0) depends on which function it is passed to. */ typedef enum { - VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 << 0), /* Filter by snapshots which - have no parents */ - VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1 << 1), /* Filter by snapshots which - have metadata */ + VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 << 0), /* Filter by snapshots + with no parents, when + listing a domain */ + VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = (1 << 0), /* List all descendants, + not just children, when + listing a snapshot */ + VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1 << 1), /* Filter by snapshots + which have metadata */ } virDomainSnapshotListFlags; /* Return the number of snapshots for this domain */ @@ -2702,6 +2708,15 @@ int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags); int virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, unsigned int flags); +/* Return the number of child snapshots for this snapshot */ +int virDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, + unsigned int flags); + +/* Get the names of all child snapshots for this snapshot */ +int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, + char **names, int nameslen, + unsigned int flags); + /* Get a handle to a named snapshot */ virDomainSnapshotPtr virDomainSnapshotLookupByName(virDomainPtr domain, const char *name, diff --git a/python/generator.py b/python/generator.py index 79558dd..71afdb7 100755 --- a/python/generator.py +++ b/python/generator.py @@ -352,6 +352,7 @@ skip_impl = ( 'virConnectListDefinedInterfaces', 'virConnectListNWFilters', 'virDomainSnapshotListNames', + 'virDomainSnapshotListChildrenNames', 'virConnGetLastError', 'virGetLastError', 'virDomainGetInfo', @@ -963,6 +964,9 @@ def nameFixup(name, classe, type, file): elif name[0:26] == "virDomainSnapshotListNames": func = name[9:] func = string.lower(func[0:1]) + func[1:] + elif name[0:28] == "virDomainSnapshotNumChildren": + func = name[17:] + func = string.lower(func[0:1]) + func[1:] elif name[0:20] == "virDomainSnapshotNum": func = name[9:] func = string.lower(func[0:1]) + func[1:] diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 3013e46..ef02f34 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -346,14 +346,20 @@ <function name='virDomainSnapshotListNames' file='python'> <info>collect the list of snapshots for the given domain</info> <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> - <arg name='flags' type='unsigned int' info='flags, curently unused'/> - <return type='str *' info='the list of Names of None in case of error'/> + <arg name='flags' type='unsigned int' info='flags'/> + <return type='str *' info='the list of Names or None in case of error'/> + </function> + <function name='virDomainSnapshotListChildrenNames' file='python'> + <info>collect the list of child snapshots for the given snapshot</info> + <arg name='snapshot' type='virDomainSnapshotPtr' info='pointer to the snapshot'/> + <arg name='flags' type='unsigned int' info='flags'/> + <return type='str *' info='the list of Names or None in case of error'/> </function> <function name='virDomainRevertToSnapshot' file='python'> <info>revert the domain to the given snapshot</info> <arg name='dom' type='virDomainPtr' info='dummy domain pointer'/> <arg name='snap' type='virDomainSnapshotPtr' info='pointer to the snapshot'/> - <arg name='flags' type='unsigned int' info='flags, curently unused'/> + <arg name='flags' type='unsigned int' info='flags'/> <return type='int' info="0 on success, -1 on error"/> </function> <function name='virDomainGetBlockJobInfo' file='python'> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d65423d..523c03b 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1727,6 +1727,51 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virDomainSnapshotListChildrenNames(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + PyObject *py_retval; + char **names = NULL; + int c_retval, i; + virDomainSnapshotPtr snap; + PyObject *pyobj_snap; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virDomainSnapshotListChildrenNames", &pyobj_snap, &flags)) + return(NULL); + snap = (virDomainSnapshotPtr) PyvirDomainSnapshot_Get(pyobj_snap); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainSnapshotNumChildren(snap, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (c_retval) { + names = malloc(sizeof(*names) * c_retval); + if (!names) + return VIR_PY_NONE; + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainSnapshotListChildrenNames(snap, names, c_retval, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) { + free(names); + return VIR_PY_NONE; + } + } + py_retval = PyList_New(c_retval); + + if (names) { + for (i = 0;i < c_retval;i++) { + PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); + free(names[i]); + } + free(names); + } + + return(py_retval); +} + +static PyObject * libvirt_virDomainRevertToSnapshot(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { int c_retval; diff --git a/src/driver.h b/src/driver.h index f85a1b1..b899d0e 100644 --- a/src/driver.h +++ b/src/driver.h @@ -584,6 +584,16 @@ typedef int int nameslen, unsigned int flags); +typedef int + (*virDrvDomainSnapshotNumChildren)(virDomainSnapshotPtr snapshot, + unsigned int flags); + +typedef int + (*virDrvDomainSnapshotListChildrenNames)(virDomainSnapshotPtr snapshot, + char **names, + int nameslen, + unsigned int flags); + typedef virDomainSnapshotPtr (*virDrvDomainSnapshotLookupByName)(virDomainPtr domain, const char *name, @@ -860,6 +870,8 @@ struct _virDriver { virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc; virDrvDomainSnapshotNum domainSnapshotNum; virDrvDomainSnapshotListNames domainSnapshotListNames; + virDrvDomainSnapshotNumChildren domainSnapshotNumChildren; + virDrvDomainSnapshotListChildrenNames domainSnapshotListChildrenNames; virDrvDomainSnapshotLookupByName domainSnapshotLookupByName; virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot; virDrvDomainSnapshotGetParent domainSnapshotGetParent; diff --git a/src/libvirt.c b/src/libvirt.c index 9080b2f..2b2f6be 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16068,6 +16068,117 @@ error: } /** + * virDomainSnapshotNumChildren: + * @snapshot: a domain snapshot object + * @flags: bitwise-or of supported virDomainSnapshotListFlags + * + * Provides the number of child snapshots for this domain snapshot. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, then the result + * includes all descendants, otherwise it is limited to direct children. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is + * the number of snapshots that also include metadata that would prevent + * the removal of the last reference to a domain; this value will either + * be 0 or the same value as if the flag were not given. + * + * Returns the number of domain snapshots found or -1 in case of error. + */ +int +virDomainSnapshotNumChildren(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->domainSnapshotNumChildren) { + int ret = conn->driver->domainSnapshotNumChildren(snapshot, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** + * virDomainSnapshotListChildrenNames: + * @snapshot: a domain snapshot object + * @names: array to collect the list of names of snapshots + * @nameslen: size of @names + * @flags: bitwise-or of supported virDomainSnapshotListFlags + * + * Collect the list of domain snapshots that are children of the given + * snapshot, and store their names in @names. Caller is responsible for + * freeing each member of the array. The value to use for @nameslen can + * be determined by virDomainSnapshotNumChildren() with the same @flags. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, then the result + * includes all descendants, otherwise it is limited to direct children. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is + * the number of snapshots that also include metadata that would prevent + * the removal of the last reference to a domain; this value will either + * be 0 or the same value as if the flag were not given. + * + * Returns the number of domain snapshots found or -1 in case of error. + */ +int +virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, + char **names, int nameslen, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("snapshot=%p, names=%p, nameslen=%d, flags=%x", + snapshot, names, nameslen, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if ((names == NULL) || (nameslen < 0)) { + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->driver->domainSnapshotListChildrenNames) { + int ret = conn->driver->domainSnapshotListChildrenNames(snapshot, + names, + nameslen, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainSnapshotLookupByName: * @domain: a domain object * @name: name for the domain snapshot diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index afea29b..9762fc4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -493,6 +493,8 @@ LIBVIRT_0.9.7 { global: virDomainReset; virDomainSnapshotGetParent; + virDomainSnapshotListChildrenNames; + virDomainSnapshotNumChildren; } LIBVIRT_0.9.5; # .... define new API here using predicted next version number .... -- 1.7.4.4
Sometimes, we only care about one branch of the snapshot hierarchy. Make it easier to list a single branch, by using the new APIs. Technically, I could emulate these new virsh options on old servers by doing a complete dump, then scraping xml to filter out just the snapshots that I care about, but I didn't want to do that in this patch. * tools/virsh.c (cmdSnapshotList): Add --from, --descendants. * tools/virsh.pod (snapshot-list): Document them. --- tools/virsh.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++------- tools/virsh.pod | 9 +++++++- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 1909dce..f43af7e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13081,6 +13081,8 @@ static const vshCmdOptDef opts_snapshot_list[] = { {"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")}, + {"from", VSH_OT_DATA, 0, N_("limit list to children of given snapshot")}, + {"descendants", VSH_OT_BOOL, 0, N_("with --from, list all descendants")}, {NULL, 0, 0, NULL} }; @@ -13107,25 +13109,36 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) char timestr[100]; struct tm time_info; bool tree = vshCommandOptBool(cmd, "tree"); + const char *from = NULL; + virDomainSnapshotPtr start = NULL; + + if (vshCommandOptString(cmd, "from", &from) < 0) { + vshError(ctl, _("invalid from argument '%s'"), from); + goto cleanup; + } if (vshCommandOptBool(cmd, "parent")) { if (vshCommandOptBool(cmd, "roots")) { vshError(ctl, "%s", - _("--parent and --roots are mutually exlusive")); + _("--parent and --roots are mutually exclusive")); return false; } if (tree) { vshError(ctl, "%s", - _("--parent and --tree are mutually exlusive")); + _("--parent and --tree are mutually exclusive")); return false; } parent_filter = 1; } else if (vshCommandOptBool(cmd, "roots")) { if (tree) { vshError(ctl, "%s", - _("--roots and --tree are mutually exlusive")); + _("--roots and --tree are mutually exclusive")); return false; } + if (from) { + vshError(ctl, "%s", + _("--roots and --from are mutually exclusive")); + } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; } @@ -13140,10 +13153,23 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - numsnaps = virDomainSnapshotNum(dom, flags); + if (from) { + start = virDomainSnapshotLookupByName(dom, from, 0); + if (!start) + goto cleanup; + if (vshCommandOptBool(cmd, "descendants") || tree) { + flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + } + numsnaps = virDomainSnapshotNumChildren(start, flags); + if (tree) + numsnaps++; + } else { + numsnaps = virDomainSnapshotNum(dom, flags); + } - /* Fall back to simulation if --roots was unsupported. */ - if (numsnaps < 0 && last_error->code == VIR_ERR_INVALID_ARG && + /* Fall back to simulation if --roots was unsupported. + * XXX Is it worth emulating --from on older servers? */ + if (numsnaps < 0 && last_error->code == VIR_ERR_INVALID_ARG && !from && (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) { virFreeError(last_error); last_error = NULL; @@ -13175,14 +13201,32 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (VIR_ALLOC_N(names, numsnaps) < 0) goto cleanup; - actual = virDomainSnapshotListNames(dom, names, numsnaps, flags); + if (from) { + /* When mixing --from and --tree, we want to start the tree at the + * given snapshot. Without --tree, only list the children. */ + if (tree) { + if (numsnaps) + actual = virDomainSnapshotListChildrenNames(start, names + 1, + numsnaps - 1, + flags); + if (actual >= 0) { + actual++; + names[0] = vshStrdup(ctl, from); + } + } else { + actual = virDomainSnapshotListChildrenNames(start, names, + numsnaps, flags); + } + } else { + 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++) { + char **parents = vshCalloc(ctl, sizeof(char *), actual); + for (i = (from ? 1 : 0); i < actual; i++) { /* free up memory from previous iterations of the loop */ if (snapshot) virDomainSnapshotFree(snapshot); @@ -13277,6 +13321,8 @@ cleanup: VIR_FREE(state); if (snapshot) virDomainSnapshotFree(snapshot); + if (start) + virDomainSnapshotFree(start); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); VIR_FREE(doc); diff --git a/tools/virsh.pod b/tools/virsh.pod index be81afc..7c91d75 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1969,7 +1969,7 @@ 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<--tree>}] -[I<--metadata>] +[I<--metadata>] [[I<--from>] B<snapshot> [I<--descendants>]] List all of the available snapshots for the given domain, defaulting to show columns for the snapshot name, creation time, and domain state. @@ -1980,6 +1980,13 @@ 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<--from> is provided, filter the list to snapshots which are +children of the given B<snapshot>. When used in isolation or with +I<--parent>, the list is limited to direct children unless +I<--descendants> is also present. When used with I<--tree>, the +use of I<--descendants> is implied. This option is not compatible +with I<--roots>. + If I<--metadata> is specified, the list will be filtered to just snapshots that involve libvirt metadata, and thus would prevent B<undefine> of a persistent domain, or be lost on B<destroy> of -- 1.7.4.4
Very mechanical. I'm so glad we've automated the generation of things, compared to what it was in 0.8.x days, where this would be much longer. * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN) (REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES): New rpcs. (remote_domain_snapshot_num_children_args) (remote_domain_snapshot_num_children_ret) (remote_domain_snapshot_list_children_names_args) (remote_domain_snapshot_list_children_names_ret): New structs. * src/remote/remote_driver.c (remote_driver): Use it. * src/remote_protocol-structs: Update. --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 25 +++++++++++++++++++++++-- src/remote_protocol-structs | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 83f4f3c..0e303df 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4411,6 +4411,8 @@ static virDriver remote_driver = { .domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */ .domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */ + .domainSnapshotNumChildren = remoteDomainSnapshotNumChildren, /* 0.9.7 */ + .domainSnapshotListChildrenNames = remoteDomainSnapshotListChildrenNames, /* 0.9.7 */ .domainSnapshotLookupByName = remoteDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = remoteDomainHasCurrentSnapshot, /* 0.8.0 */ .domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c8a92fd..f95253e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2067,6 +2067,25 @@ struct remote_domain_snapshot_list_names_ret { remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert@1 */ }; +struct remote_domain_snapshot_num_children_args { + remote_nonnull_domain_snapshot snap; + unsigned int flags; +}; + +struct remote_domain_snapshot_num_children_ret { + int num; +}; + +struct remote_domain_snapshot_list_children_names_args { + remote_nonnull_domain_snapshot snap; + int maxnames; + unsigned int flags; +}; + +struct remote_domain_snapshot_list_children_names_ret { + remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert@1 */ +}; + struct remote_domain_snapshot_lookup_by_name_args { remote_nonnull_domain dom; remote_nonnull_string name; @@ -2524,8 +2543,10 @@ 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_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */ - REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */ + REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247 /* autogen autogen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 69175cc..fc11893 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1557,6 +1557,24 @@ struct remote_domain_snapshot_list_names_ret { remote_nonnull_string * names_val; } names; }; +struct remote_domain_snapshot_num_children_args { + remote_nonnull_domain_snapshot snap; + u_int flags; +}; +struct remote_domain_snapshot_num_children_ret { + int num; +}; +struct remote_domain_snapshot_list_children_names_args { + remote_nonnull_domain_snapshot dom; + int maxnames; + u_int flags; +}; +struct remote_domain_snapshot_list_children_names_ret { + struct { + u_int names_len; + remote_nonnull_string * names_val; + } names; +}; struct remote_domain_snapshot_lookup_by_name_args { remote_nonnull_domain dom; remote_nonnull_string name; @@ -1971,4 +1989,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, REMOTE_PROC_DOMAIN_RESET = 245, + REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, }; -- 1.7.4.4
Not too hard to wire up. The trickiest part is realizing that listing children of a snapshot cannot use SNAPSHOT_LIST_ROOTS, and that we overloaded that bit to also mean SNAPSHOT_LIST_DESCENDANTS; we use that bit to decide which iteration to use, but don't want the existing counting/listing functions to see that bit. * src/conf/domain_conf.h (virDomainSnapshotObjListNumFrom) (virDomainSnapshotObjListGetNamesFrom): New prototypes. * src/conf/domain_conf.c (virDomainSnapshotObjListNumFrom) (virDomainSnapshotObjListGetNamesFrom): New functions. * src/libvirt_private.syms (domain_conf.h): Export them. * src/qemu/qemu_driver.c (qemuDomainSnapshotNumChildren) (qemuDomainSnapshotListChildrenNames): New functions. --- src/conf/domain_conf.c | 51 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1871974..40b3589 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12132,6 +12132,37 @@ cleanup: return -1; } +int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjListPtr snapshots, + char **const names, int maxnames, + unsigned int flags) +{ + struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 }; + int i; + + data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + + if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) + virDomainSnapshotForEachDescendant(snapshots, snapshot, + virDomainSnapshotObjListCopyNames, + &data); + else + virDomainSnapshotForEachChild(snapshots, snapshot, + virDomainSnapshotObjListCopyNames, &data); + + if (data.oom) { + virReportOOMError(); + goto cleanup; + } + + return data.numnames; + +cleanup: + for (i = 0; i < data.numnames; i++) + VIR_FREE(data.names[i]); + return -1; +} + struct virDomainSnapshotNumData { int count; unsigned int flags; @@ -12159,6 +12190,26 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, return data.count; } +int +virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjListPtr snapshots, + unsigned int flags) +{ + struct virDomainSnapshotNumData data = { 0, 0 }; + + data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + + if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) + virDomainSnapshotForEachDescendant(snapshots, snapshot, + virDomainSnapshotObjListCount, + &data); + else + virDomainSnapshotForEachChild(snapshots, snapshot, + virDomainSnapshotObjListCount, &data); + + return data.count; +} + virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots, const char *name) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4126d58..1d490d0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1488,6 +1488,13 @@ int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, unsigned int flags); int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, unsigned int flags); +int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjListPtr snapshots, + char **const names, int maxnames, + unsigned int flags); +int virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjListPtr snapshots, + unsigned int flags); virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots, const char *name); void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6505eae..3c7e44d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -419,7 +419,9 @@ virDomainSnapshotForEachChild; virDomainSnapshotForEachDescendant; virDomainSnapshotHasChildren; virDomainSnapshotObjListGetNames; +virDomainSnapshotObjListGetNamesFrom; virDomainSnapshotObjListNum; +virDomainSnapshotObjListNumFrom; virDomainSnapshotObjListRemove; virDomainSnapshotStateTypeFromString; virDomainSnapshotStateTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b459b73..eccb48d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9448,6 +9448,91 @@ cleanup: return n; } +static int +qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, + char **names, + int nameslen, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | + VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); + + 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; + } + + n = virDomainSnapshotObjListGetNamesFrom(snap, &vm->snapshots, + names, nameslen, flags); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return n; +} + +static int +qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + struct qemud_driver *driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | + VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); + + 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; + } + + /* All qemu snapshots have libvirt metadata, so + * VIR_DOMAIN_SNAPSHOT_LIST_METADATA makes no difference to our + * answer. */ + + n = virDomainSnapshotObjListNumFrom(snap, &vm->snapshots, flags); + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return n; +} + static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain, const char *name, unsigned int flags) @@ -10538,6 +10623,8 @@ static virDriver qemuDriver = { .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */ .domainSnapshotListNames = qemuDomainSnapshotListNames, /* 0.8.0 */ + .domainSnapshotNumChildren = qemuDomainSnapshotNumChildren, /* 0.9.7 */ + .domainSnapshotListChildrenNames = qemuDomainSnapshotListChildrenNames, /* 0.9.7 */ .domainSnapshotLookupByName = qemuDomainSnapshotLookupByName, /* 0.8.0 */ .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */ .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */ -- 1.7.4.4
Emulating --from requires grabbing the entire list of snapshots and their parents, and recursively iterating over the list from the point of interest - but we already do that for --tree. This turns on emulation for that situation. * tools/virsh.c (cmdSnapshotList): Add fallback. --- Can be applied anywhere after 2/4, but by inserting it before 3/4, you can actually test it against the same version of libvirtd (instead of having to run an older server with newer client). I'm also looking at implementing the fallback even when not in --tree, but that's a bit more complex. tools/virsh.c | 24 +++++++++++++++++------- 1 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 778725c..95baca5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13111,6 +13111,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) bool tree = vshCommandOptBool(cmd, "tree"); const char *from = NULL; virDomainSnapshotPtr start = NULL; + bool emulate_from = false; if (vshCommandOptString(cmd, "from", &from) < 0) { vshError(ctl, _("invalid from argument '%s'"), from); @@ -13161,14 +13162,23 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; } numsnaps = virDomainSnapshotNumChildren(start, flags); - if (numsnaps >= 0 && tree) - numsnaps++; + if (tree) { + if (numsnaps >= 0) { + numsnaps++; + } else if (last_error->code == VIR_ERR_NO_SUPPORT) { + /* We can emulate --tree --from. */ + virFreeError(last_error); + last_error = NULL; + emulate_from = true; + flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; + numsnaps = virDomainSnapshotNum(dom, flags); + } + } } else { numsnaps = virDomainSnapshotNum(dom, flags); } - /* Fall back to simulation if --roots was unsupported. - * XXX Is it worth emulating --from on older servers? */ + /* Fall back to simulation if --roots was unsupported. */ if (numsnaps < 0 && last_error->code == VIR_ERR_INVALID_ARG && !from && (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) { virFreeError(last_error); @@ -13201,7 +13211,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (VIR_ALLOC_N(names, numsnaps) < 0) goto cleanup; - if (from) { + if (from && !emulate_from) { /* When mixing --from and --tree, we want to start the tree at the * given snapshot. Without --tree, only list the children. */ if (tree) { @@ -13226,7 +13236,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (tree) { char indentBuf[INDENT_BUFLEN]; char **parents = vshCalloc(ctl, sizeof(char *), actual); - for (i = (from ? 1 : 0); i < actual; i++) { + for (i = (from && !emulate_from ? 1 : 0); i < actual; i++) { /* free up memory from previous iterations of the loop */ if (snapshot) virDomainSnapshotFree(snapshot); @@ -13241,7 +13251,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } for (i = 0 ; i < actual ; i++) { memset(indentBuf, '\0', sizeof indentBuf); - if (parents[i] == NULL) + if (emulate_from ? STREQ(names[i], from) : parents[i] == NULL) cmdNodeListDevicesPrint(ctl, names, parents, -- 1.7.4.4
Iterating over one level of children requires parsing all snapshots and their parents; a bit of code shuffling makes it pretty easy to do this as well. * tools/virsh.c (cmdSnapshotList): Add another fallback. --- Applies anywhere after 2.5/4, but again, easiest to test against the same version of libvirtd if applied before 3/4. tools/virsh.c | 46 +++++++++++++++++++++++++++++++--------------- 1 files changed, 31 insertions(+), 15 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 93e4528..232c9b8 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13096,6 +13096,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) information needed, 1 for parent column */ int numsnaps; char **names = NULL; + char **parents = NULL; int actual = 0; int i; xmlDocPtr xml = NULL; @@ -13112,6 +13113,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; virDomainSnapshotPtr start = NULL; bool emulate_from = false; + bool descendants = false; if (vshCommandOptString(cmd, "from", &from) < 0) { vshError(ctl, _("invalid from argument '%s'"), from); @@ -13155,25 +13157,27 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (from) { + descendants = vshCommandOptBool(cmd, "descendants"); start = virDomainSnapshotLookupByName(dom, from, 0); if (!start) goto cleanup; - if (vshCommandOptBool(cmd, "descendants") || tree) { + if (descendants || tree) { flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; } numsnaps = virDomainSnapshotNumChildren(start, flags); - /* XXX Is it worth emulating --from without --tree on older servers? */ - if (tree) { - if (numsnaps >= 0) { - numsnaps++; - } else if (last_error->code == VIR_ERR_NO_SUPPORT) { - /* We can emulate --tree --from. */ + if (numsnaps < 0) { + /* XXX also want to emulate --descendants without --tree */ + if ((!descendants || tree) + last_error->code == VIR_ERR_NO_SUPPORT) { + /* We can emulate --from. */ virFreeError(last_error); last_error = NULL; emulate_from = true; flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; numsnaps = virDomainSnapshotNum(dom, flags); } + } else if (tree) { + numsnaps++; } } else { numsnaps = virDomainSnapshotNum(dom, flags); @@ -13234,9 +13238,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (actual < 0) goto cleanup; - if (tree) { - char indentBuf[INDENT_BUFLEN]; - char **parents = vshCalloc(ctl, sizeof(char *), actual); + if (!tree) + qsort(&names[0], actual, sizeof(char*), namesorter); + + if (tree || emulate_from) { + parents = vshCalloc(ctl, sizeof(char *), actual); for (i = (from && !emulate_from); i < actual; i++) { /* free up memory from previous iterations of the loop */ if (snapshot) @@ -13250,6 +13256,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } parents[i] = vshGetSnapshotParent(ctl, snapshot); } + } + if (tree) { + char indentBuf[INDENT_BUFLEN]; for (i = 0 ; i < actual ; i++) { memset(indentBuf, '\0', sizeof indentBuf); if (emulate_from ? STREQ(names[i], from) : !parents[i]) @@ -13263,16 +13272,19 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) 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); + if (emulate_from && descendants) { + /* XXX emulate --descendants as well */ + goto cleanup; + } for (i = 0; i < actual; i++) { + if (emulate_from && STRNEQ_NULLABLE(parents[i], from)) + continue; + /* free up memory from previous iterations of the loop */ VIR_FREE(parent); VIR_FREE(state); @@ -13337,9 +13349,13 @@ cleanup: xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); VIR_FREE(doc); - for (i = 0; i < actual; i++) + for (i = 0; i < actual; i++) { VIR_FREE(names[i]); + if (parents) + VIR_FREE(parents[i]); + } VIR_FREE(names); + VIR_FREE(parents); if (dom) virDomainFree(dom); -- 1.7.4.4
On 09/29/2011 04:02 PM, Eric Blake wrote:
Iterating over one level of children requires parsing all snapshots and their parents; a bit of code shuffling makes it pretty easy to do this as well.
* tools/virsh.c (cmdSnapshotList): Add another fallback. ---
Applies anywhere after 2.5/4, but again, easiest to test against the same version of libvirtd if applied before 3/4.
+ if (numsnaps< 0) { + /* XXX also want to emulate --descendants without --tree */ + if ((!descendants || tree) + last_error->code == VIR_ERR_NO_SUPPORT) {
This doesn't even compile. Let me send a cleaned up v2. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Given a list of snapshots and their parents, finding all descendants requires a hairy traversal. This code is O(n^3); it could maybe be made to scale O(n^2) with the use of a hash table, but that costs more memory. Hopefully there aren't too many people with a hierarchy so large as to approach REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX (1024). * tools/virsh.c (cmdSnapshotList): Add final fallback. --- Applies anywhere after 2.6/4; same story about testing before 3/4. Since I couldn't implement virDomainSnapshotNumChildren for ESX, I figured I'd better go ahead and implement the fallbacks, so that the new snapshot-list features are useful for more than just qemu. tools/virsh.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 48 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 232c9b8..4fbd1a8 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13113,6 +13113,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; virDomainSnapshotPtr start = NULL; bool emulate_from = false; + int start_index = -1; bool descendants = false; if (vshCommandOptString(cmd, "from", &from) < 0) { @@ -13166,9 +13167,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } numsnaps = virDomainSnapshotNumChildren(start, flags); if (numsnaps < 0) { - /* XXX also want to emulate --descendants without --tree */ - if ((!descendants || tree) - last_error->code == VIR_ERR_NO_SUPPORT) { + if (last_error->code == VIR_ERR_NO_SUPPORT) { /* We can emulate --from. */ virFreeError(last_error); last_error = NULL; @@ -13244,6 +13243,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (tree || emulate_from) { parents = vshCalloc(ctl, sizeof(char *), actual); for (i = (from && !emulate_from); i < actual; i++) { + if (emulate_from && STREQ(names[i], from)) { + start_index = i; + continue; + } + /* free up memory from previous iterations of the loop */ if (snapshot) virDomainSnapshotFree(snapshot); @@ -13277,12 +13281,50 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) goto cleanup; } else { if (emulate_from && descendants) { - /* XXX emulate --descendants as well */ - goto cleanup; + bool changed = true; + + /* Make multiple passes over the list - first pass NULLs + * out all roots except start, remaining passes NULL out + * any entry whose parent is not still in list. + * Sorry, this is O(n^3) - hope your hierarchy isn't huge. */ + if (start_index < 0) { + vshError(ctl, _("snapshot %s disappeared from list"), from); + goto cleanup; + } + for (i = 0; i < actual; i++) { + if (i == start_index) + continue; + if (!parents[i]) + VIR_FREE(names[i]); + } + while (changed) { + changed = false; + for (i = 0; i < actual; i++) { + bool found = false; + int j; + + if (!names[i] || i == start_index) + continue; + for (j = 0; j < actual; j++) { + if (!names[j] || i == j) + continue; + if (STREQ(parents[i], names[j])) { + found = true; + break; + } + } + if (!found) { + changed = true; + VIR_FREE(names[i]); + } + } + } + VIR_FREE(names[start_index]); } for (i = 0; i < actual; i++) { - if (emulate_from && STRNEQ_NULLABLE(parents[i], from)) + if (emulate_from && + (descendants ? !names[i] : STRNEQ_NULLABLE(parents[i], from))) continue; /* free up memory from previous iterations of the loop */ -- 1.7.4.4
On 09/29/2011 04:39 PM, Eric Blake wrote:
Given a list of snapshots and their parents, finding all descendants requires a hairy traversal. This code is O(n^3); it could maybe be made to scale O(n^2) with the use of a hash table, but that costs more memory. Hopefully there aren't too many people with a hierarchy so large as to approach REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX (1024).
* tools/virsh.c (cmdSnapshotList): Add final fallback. ---
Applies anywhere after 2.6/4; same story about testing before 3/4.
I realized after sending the email that I can squash this in for fewer passes of the outer loop for slightly faster performance (borrowing ideas from how virDomainSnapshotForEachDescendant tackled the same problem of finding descendants by using two separate marks). diff --git i/tools/virsh.c w/tools/virsh.c index 4fbd1a8..9f3b4dc 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -13281,11 +13281,12 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) goto cleanup; } else { if (emulate_from && descendants) { - bool changed = true; + bool changed = false; /* Make multiple passes over the list - first pass NULLs * out all roots except start, remaining passes NULL out - * any entry whose parent is not still in list. + * any entry whose parent is not still in list. Also, we + * NULL out parent when name is known to be in list. * Sorry, this is O(n^3) - hope your hierarchy isn't huge. */ if (start_index < 0) { vshError(ctl, _("snapshot %s disappeared from list"), from); @@ -13294,8 +13295,16 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) for (i = 0; i < actual; i++) { if (i == start_index) continue; - if (!parents[i]) + if (!parents[i]) { VIR_FREE(names[i]); + } else if (STREQ(parents[i], from)) { + VIR_FREE(parents[i]); + changed = true; + } + } + if (!changed) { + ret = true; + goto cleanup; } while (changed) { changed = false; @@ -13303,13 +13312,15 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) bool found = false; int j; - if (!names[i] || i == start_index) + if (!names[i] || !parents[i]) continue; for (j = 0; j < actual; j++) { if (!names[j] || i == j) continue; if (STREQ(parents[i], names[j])) { found = true; + if (!parents[j]) + VIR_FREE(parents[i]); break; } } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (1)
-
Eric Blake