Eric Blake | 1 Oct 2011 01:05
Picon
Favicon
Gravatar

Re: [libvirt] [PATCH 2.6/4] snapshot: virsh fallback for snapshot-list --from children

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 <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 3/7] snapshot: virsh fallback for snapshot-list --tree --from

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 (__vshControl): Rename member.
(vshReconnect, cmdConnect, vshGetSnapshotParent): Update clients.
(cmdSnapshotList): Add fallback.
---

v2: reuse existing ctl flag, so that virsh in shell mode doesn't
repeatedly try non-working commands

 tools/virsh.c |   43 +++++++++++++++++++++++++++++--------------
 1 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index adafe86..e3bc364 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
 <at>  <at>  -246,8 +246,8  <at>  <at>  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 */
+    bool useSnapshotOld;        /* cannot use virDomainSnapshotGetParent or
+                                   virDomainSnapshotNumChildren */
 } __vshControl;

(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 0/7] snapshot: listing children

Cleaned up the rebase goofs present throughout my v1:
https://www.redhat.com/archives/libvir-list/2011-September/msg01270.html

Eric Blake (7):
  snapshot: new virDomainSnapshotListChildrenNames API
  snapshot: virsh snapshot-list and children
  snapshot: virsh fallback for snapshot-list --tree --from
  snapshot: virsh fallback for snapshot-list --from children
  snapshot: virsh fallback for snapshot-list --descendants --from
  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                   |  190 ++++++++++++++++++++++++++++++++------
 tools/virsh.pod                 |    9 ++-
 16 files changed, 564 insertions(+), 42 deletions(-)

(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 1/7] snapshot: new virDomainSnapshotListChildrenNames API

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.
---

v2: no change

 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 +
(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 4/7] snapshot: virsh fallback for snapshot-list --from children

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.
---

v2: fix compilation error, rebase around ctl flag

 tools/virsh.c |   48 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index e3bc364..b2523d3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
 <at>  <at>  -13117,6 +13117,7  <at>  <at>  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;
 <at>  <at>  -13132,6 +13133,7  <at>  <at>  cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
     bool tree = vshCommandOptBool(cmd, "tree");
     const char *from = NULL;
     virDomainSnapshotPtr start = NULL;
+    bool descendants = false;

(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 2/7] snapshot: virsh snapshot-list and children

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.
---

v2: minor rebase cleanups

 tools/virsh.c   |   76 ++++++++++++++++++++++++++++++++++++++++++++-----------
 tools/virsh.pod |    9 ++++++-
 2 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 955b8df..adafe86 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
 <at>  <at>  -13102,6 +13102,8  <at>  <at>  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}
 };

(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 5/7] snapshot: virsh fallback for snapshot-list --descendants --from

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.
---

v2: rebase around ctl flag

 tools/virsh.c |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index b2523d3..ec6f854 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
 <at>  <at>  -13133,6 +13133,7  <at>  <at>  cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
     bool tree = vshCommandOptBool(cmd, "tree");
     const char *from = NULL;
     virDomainSnapshotPtr start = NULL;
+    int start_index = -1;
     bool descendants = false;

     if (vshCommandOptString(cmd, "from", &from) < 0) {
 <at>  <at>  -13187,10 +13188,8  <at>  <at>  cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         numsnaps = ctl->useSnapshotOld ? -1 :
             virDomainSnapshotNumChildren(start, flags);
         if (numsnaps < 0) {
(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 6/7] snapshot: remote protocol for snapshot children

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.
---

v2: fix typo in remote_protocol-structs

 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
 <at>  <at>  -4411,6 +4411,8  <at>  <at>  static virDriver remote_driver = {
     .domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */
     .domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */
     .domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */
+    .domainSnapshotNumChildren = remoteDomainSnapshotNumChildren, /* 0.9.7 */
(Continue reading)

Eric Blake | 1 Oct 2011 01:09
Picon
Favicon
Gravatar

[libvirt] [PATCHv2 7/7] snapshot: implement snapshot children listing in qemu

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.
---

v2: no change, but now virsh changes have been tested both with
and without this patch

 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 c141982..438e3b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
 <at>  <at>  -12138,6 +12138,37  <at>  <at>  cleanup:
     return -1;
(Continue reading)

Marc-André Lureau | 1 Oct 2011 03:05
Picon
Gravatar

[libvirt] [PATCH] virsh: do not unlink NULL file

error:could not take a screenshot of xp
==6216== Syscall param unlink(pathname) points to unaddressable byte(s)
==6216==    at 0x373A0D4937: unlink (syscall-template.S:82)
==6216==    by 0x40FD73: cmdScreenshot (virsh.c:3070)
==6216==    by 0x42BA0D: vshCommandRun (virsh.c:14920)
==6216==    by 0x42EC97: main (virsh.c:16379)
==6216==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==6216==
error:Requested operation is not valid: domain is not running
---
 tools/virsh.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 1909dce..89d355f 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
 <at>  <at>  -3004,7 +3004,7  <at>  <at>  cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
     unsigned int screen = 0;
     unsigned int flags = 0; /* currently unused */
     int ret = false;
-    bool created = true;
+    bool created = false;
     bool generated = false;
     char *mime = NULL;

 <at>  <at>  -3039,13 +3039,13  <at>  <at>  cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
     }

     if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) {
(Continue reading)


Gmane