[libvirt] [PATCHv2 1/2] virsh: add virsh snapshot-current --name
Sometimes, full XML is too much; since most snapshot commands operate on a snapshot name, there should be an easy way to get at the current snapshot's name. * tools/virsh.c (cmdSnapshotCurrent): Add an option. * tools/virsh.pod (snapshot-current): Document it. --- v2: use xpath instead of strstr; replaces: https://www.redhat.com/archives/libvir-list/2011-August/msg00329.html tools/virsh.c | 29 ++++++++++++++++++++++++++--- tools/virsh.pod | 4 +++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bffec1d..fd64020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12034,6 +12034,7 @@ static const vshCmdInfo info_snapshot_current[] = { static const vshCmdOptDef opts_snapshot_current[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"name", VSH_OT_BOOL, 0, N_("list the name, rather than the full xml")}, {NULL, 0, 0, NULL} }; @@ -12044,6 +12045,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) bool ret = false; int current; virDomainSnapshotPtr snapshot = NULL; + char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -12056,7 +12058,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (current < 0) goto cleanup; else if (current) { - char *xml; + char *name = NULL; if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) goto cleanup; @@ -12065,13 +12067,34 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (!xml) goto cleanup; - vshPrint(ctl, "%s", xml); - VIR_FREE(xml); + if (vshCommandOptBool(cmd, "name")) { + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + + xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", + NULL, XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + if (!xmldoc) + goto cleanup; + ctxt = xmlXPathNewContext(xmldoc); + if (!ctxt) { + xmlFreeDoc(xmldoc); + } + + name = virXPathString("string(/domainsnapshot/name)", ctxt); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + if (!name) + goto cleanup; + } + + vshPrint(ctl, "%s", name ? name : xml); } ret = true; cleanup: + VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); if (dom) diff --git a/tools/virsh.pod b/tools/virsh.pod index ec778bf..53376d6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1569,9 +1569,11 @@ Create a snapshot for domain I<domain> with the given <name> and value. If I<--print-xml> is specified, then XML appropriate for I<snapshot-create> is output, rather than actually creating a snapshot. -=item B<snapshot-current> I<domain> +=item B<snapshot-current> I<domain> [I<--name>] Output the snapshot XML for the domain's current snapshot (if any). +If I<--name> is specified, just list the snapshot name instead of the +full xml. =item B<snapshot-list> I<domain> -- 1.7.4.4
Found this while revising the previous patch to use xpath rather than strstr for undoing the escapse. * tools/virsh.c (cmdSnapshotCreateAs): Escape user input. --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index fd64020..52a15bc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11963,9 +11963,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "<domainsnapshot>\n"); if (name) - virBufferAsprintf(&buf, " <name>%s</name>\n", name); + virBufferEscapeString(&buf, " <name>%s</name>\n", name); if (desc) - virBufferAsprintf(&buf, " <description>%s</description>\n", desc); + virBufferEscapeString(&buf, " <description>%s</description>\n", desc); virBufferAddLit(&buf, "</domainsnapshot>\n"); buffer = virBufferContentAndReset(&buf); -- 1.7.4.4
On Wed, Aug 10, 2011 at 05:02:37PM -0600, Eric Blake wrote:
Found this while revising the previous patch to use xpath rather than strstr for undoing the escapse.
typo, "escape"
* tools/virsh.c (cmdSnapshotCreateAs): Escape user input. --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index fd64020..52a15bc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11963,9 +11963,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
virBufferAddLit(&buf, "<domainsnapshot>\n"); if (name) - virBufferAsprintf(&buf, " <name>%s</name>\n", name); + virBufferEscapeString(&buf, " <name>%s</name>\n", name); if (desc) - virBufferAsprintf(&buf, " <description>%s</description>\n", desc); + virBufferEscapeString(&buf, " <description>%s</description>\n", desc); virBufferAddLit(&buf, "</domainsnapshot>\n");
buffer = virBufferContentAndReset(&buf);
Ah, right ! 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/
On 08/10/2011 08:33 PM, Daniel Veillard wrote:
On Wed, Aug 10, 2011 at 05:02:37PM -0600, Eric Blake wrote:
Found this while revising the previous patch to use xpath rather than strstr for undoing the escapse.
typo, "escape"
I adjusted the message to fix that, then pushed this one first after all.
virBufferAddLit(&buf, "<domainsnapshot>\n"); if (name) - virBufferAsprintf(&buf, "<name>%s</name>\n", name); + virBufferEscapeString(&buf, "<name>%s</name>\n", name); if (desc) - virBufferAsprintf(&buf, "<description>%s</description>\n", desc); + virBufferEscapeString(&buf, "<description>%s</description>\n", desc); virBufferAddLit(&buf, "</domainsnapshot>\n");
buffer = virBufferContentAndReset(&buf);
Ah, right ! ACK
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/10/2011 10:02 PM, Eric Blake wrote:
On 08/10/2011 08:33 PM, Daniel Veillard wrote:
On Wed, Aug 10, 2011 at 05:02:37PM -0600, Eric Blake wrote:
Found this while revising the previous patch to use xpath rather than strstr for undoing the escapse.
typo, "escape"
I adjusted the message to fix that, then pushed this one first after all.
virBufferAddLit(&buf, "<domainsnapshot>\n"); if (name) - virBufferAsprintf(&buf, "<name>%s</name>\n", name); + virBufferEscapeString(&buf, "<name>%s</name>\n", name); if (desc) - virBufferAsprintf(&buf, "<description>%s</description>\n", desc); + virBufferEscapeString(&buf, "<description>%s</description>\n", desc); virBufferAddLit(&buf, "</domainsnapshot>\n");
buffer = virBufferContentAndReset(&buf);
Ah, right ! ACK
Shame on me for not testing this further. snapshot-create-as can now create weird names, but until virDomainSnapshotGetXMLDesc is fixed to re-output those names correctly, it's a one-way trip and still broken. domain_conf.c needs the same fix. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, Aug 10, 2011 at 05:02:36PM -0600, Eric Blake wrote:
Sometimes, full XML is too much; since most snapshot commands operate on a snapshot name, there should be an easy way to get at the current snapshot's name.
* tools/virsh.c (cmdSnapshotCurrent): Add an option. * tools/virsh.pod (snapshot-current): Document it. ---
v2: use xpath instead of strstr; replaces: https://www.redhat.com/archives/libvir-list/2011-August/msg00329.html
tools/virsh.c | 29 ++++++++++++++++++++++++++--- tools/virsh.pod | 4 +++- 2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index bffec1d..fd64020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12034,6 +12034,7 @@ static const vshCmdInfo info_snapshot_current[] = {
static const vshCmdOptDef opts_snapshot_current[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"name", VSH_OT_BOOL, 0, N_("list the name, rather than the full xml")}, {NULL, 0, 0, NULL} };
@@ -12044,6 +12045,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) bool ret = false; int current; virDomainSnapshotPtr snapshot = NULL; + char *xml = NULL;
if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -12056,7 +12058,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (current < 0) goto cleanup; else if (current) { - char *xml; + char *name = NULL;
if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) goto cleanup; @@ -12065,13 +12067,34 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (!xml) goto cleanup;
- vshPrint(ctl, "%s", xml); - VIR_FREE(xml); + if (vshCommandOptBool(cmd, "name")) { + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + + xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", + NULL, XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + if (!xmldoc) + goto cleanup; + ctxt = xmlXPathNewContext(xmldoc); + if (!ctxt) { + xmlFreeDoc(xmldoc); + } + + name = virXPathString("string(/domainsnapshot/name)", ctxt); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + if (!name) + goto cleanup; + } + + vshPrint(ctl, "%s", name ? name : xml); }
ret = true;
cleanup: + VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); if (dom) diff --git a/tools/virsh.pod b/tools/virsh.pod index ec778bf..53376d6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1569,9 +1569,11 @@ Create a snapshot for domain I<domain> with the given <name> and value. If I<--print-xml> is specified, then XML appropriate for I<snapshot-create> is output, rather than actually creating a snapshot.
-=item B<snapshot-current> I<domain> +=item B<snapshot-current> I<domain> [I<--name>]
Output the snapshot XML for the domain's current snapshot (if any). +If I<--name> is specified, just list the snapshot name instead of the +full xml.
=item B<snapshot-list> I<domain>
ACK, this is really more convenient in most cases, 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 08/10/2011 08:32 PM, Daniel Veillard wrote:
On Wed, Aug 10, 2011 at 05:02:36PM -0600, Eric Blake wrote:
Sometimes, full XML is too much; since most snapshot commands operate on a snapshot name, there should be an easy way to get at the current snapshot's name.
* tools/virsh.c (cmdSnapshotCurrent): Add an option. * tools/virsh.pod (snapshot-current): Document it. ---
+ xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", + NULL, XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + if (!xmldoc) + goto cleanup; + ctxt = xmlXPathNewContext(xmldoc); + if (!ctxt) { + xmlFreeDoc(xmldoc); + }
I missed a goto cleanup here.
+ + name = virXPathString("string(/domainsnapshot/name)", ctxt); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + if (!name) + goto cleanup; + } + + vshPrint(ctl, "%s", name ? name : xml);
as well as a VIR_FREE(name) here to avoid a leak.
ACK, this is really more convenient in most cases,
Pushed with those fixes, and also mentioning a common use case in the commit message: virsh snapshot-revert dom `virsh snapshot-current dom --name` for reverting back to the most recently made snapshot. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel Veillard -
Eric Blake