[libvirt] [PATCH 0/2] snapshot: add getName API
I know I've missed the 0.9.5 RC1 freeze, but think that this new API is worth adding, if only because I noticed that 'virsh snapshot-create dom --no-metadata' currently creates a snapshot with a generated name but then fails in attempting to express that name to the user. Eric Blake (2): snapshot: new APIs for inspecting snapshot object snapshot: use new API for less work include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++- tools/virsh.c | 41 +++++------------------ 4 files changed, 90 insertions(+), 33 deletions(-) -- 1.7.4.4
These functions access internals of the opaque object, and do not need any rpc counterpart. It could be argued that we should have provided these when snapshot objects were first introduced, since all the other vir*Ptr objects have at least a GetName accessor. * include/libvirt/libvirt.h.in (virDomainSnapshotGetName) (virDomainSnapshotGetDomain, virDomainSnapshotGetConnect): Declare. * src/libvirt.c (virDomainSnapshotGetName) (virDomainSnapshotGetDomain, virDomainSnapshotGetConnect): New functions. * src/libvirt_public.syms: Export them. --- include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++- 3 files changed, 81 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5fa489e..ea7b3fc 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2656,6 +2656,10 @@ typedef struct _virDomainSnapshot virDomainSnapshot; */ typedef virDomainSnapshot *virDomainSnapshotPtr; +const char *virDomainSnapshotGetName(virDomainSnapshotPtr snapshot); +virDomainPtr virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot); +virConnectPtr virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot); + typedef enum { VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE = (1 << 0), /* Restore or alter metadata */ diff --git a/src/libvirt.c b/src/libvirt.c index dc082a6..c32c7a6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -15665,6 +15665,79 @@ error: } /** + * virDomainSnapshotGetName: + * @snapshot: a snapshot object + * + * Get the public name for that snapshot + * + * Returns a pointer to the name or NULL, the string need not be deallocated + * as its lifetime will be the same as the snapshot object. + */ +const char * +virDomainSnapshotGetName(virDomainSnapshotPtr snapshot) +{ + VIR_DEBUG("snapshot=%p", snapshot); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + return snapshot->name; +} + +/** + * virDomainSnapshotGetDomain: + * @snapshot: a snapshot object + * + * Get the domain that a snapshot was created for + * + * Returns the domain or NULL. + */ +virDomainPtr +virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot) +{ + VIR_DEBUG("snapshot=%p", snapshot); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + return snapshot->domain; +} + +/** + * virDomainSnapshotGetConnect: + * @snapshot: a snapshot object + * + * Get the connection that owns the domain that a snapshot was created for + * + * Returns the connection or NULL. + */ +virConnectPtr +virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) +{ + VIR_DEBUG("snapshot=%p", snapshot); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, + __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + return snapshot->domain->conn; +} + +/** * virDomainSnapshotCreateXML: * @domain: a domain object * @xmlDesc: string containing an XML description of the domain diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index dc5a80b..8a6d55a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -482,8 +482,11 @@ LIBVIRT_0.9.4 { LIBVIRT_0.9.5 { global: - virDomainMigrateGetMaxSpeed; virDomainBlockStatsFlags; + virDomainMigrateGetMaxSpeed; + virDomainSnapshotGetConnect; + virDomainSnapshotGetDomain; + virDomainSnapshotGetName; } LIBVIRT_0.9.4; # .... define new API here using predicted next version number .... -- 1.7.4.4
On 09/08/2011 01:21 PM, Eric Blake wrote:
These functions access internals of the opaque object, and do not need any rpc counterpart. It could be argued that we should have provided these when snapshot objects were first introduced, since all the other vir*Ptr objects have at least a GetName accessor.
* include/libvirt/libvirt.h.in (virDomainSnapshotGetName) (virDomainSnapshotGetDomain, virDomainSnapshotGetConnect): Declare. * src/libvirt.c (virDomainSnapshotGetName) (virDomainSnapshotGetDomain, virDomainSnapshotGetConnect): New functions. * src/libvirt_public.syms: Export them. --- include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++- 3 files changed, 81 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5fa489e..ea7b3fc 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2656,6 +2656,10 @@ typedef struct _virDomainSnapshot virDomainSnapshot; */ typedef virDomainSnapshot *virDomainSnapshotPtr;
+const char *virDomainSnapshotGetName(virDomainSnapshotPtr snapshot); +virDomainPtr virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot); +virConnectPtr virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot);
By the way, it is intentional that these 3 APIs do _not_ have an unsigned int flags argument - we are consistent that none of our accessor functions have a flags argument, and since there is no rpc call involved, we know up front that these APIs will never be extended. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This has the added benefit of making 'snapshot-create dom --no-metadata' now able to tell you the name of the just-generated snapshot. * tools/virsh.c (vshSnapshotCreate, cmdSnapshotCurrent): Don't get XML just for name. --- tools/virsh.c | 41 +++++++++-------------------------------- 1 files changed, 9 insertions(+), 32 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cf3e816..5e74947 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12424,7 +12424,7 @@ vshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer, char *doc = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; - char *name = NULL; + const char *name = NULL; snapshot = virDomainSnapshotCreateXML(dom, buffer, flags); @@ -12459,21 +12459,9 @@ vshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer, goto cleanup; } - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA) - doc = vshStrdup(ctl, buffer); - else - doc = virDomainSnapshotGetXMLDesc(snapshot, 0); - if (!doc) - goto cleanup; - - xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt); - if (!xml) - goto cleanup; - - name = virXPathString("string(/domainsnapshot/name)", ctxt); + name = virDomainSnapshotGetName(snapshot); if (!name) { - vshError(ctl, "%s", - _("Could not find 'name' element in domain snapshot XML")); + vshError(ctl, "%s", _("Could not get snapshot name")); goto cleanup; } @@ -12485,7 +12473,6 @@ vshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer, ret = true; cleanup: - VIR_FREE(name); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); if (snapshot) @@ -12891,32 +12878,22 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (current < 0) goto cleanup; else if (current) { - char *name = NULL; + const char *name = NULL; if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) goto cleanup; - xml = virDomainSnapshotGetXMLDesc(snapshot, flags); - if (!xml) - goto cleanup; - if (vshCommandOptBool(cmd, "name")) { - xmlDocPtr xmldoc = NULL; - xmlXPathContextPtr ctxt = NULL; - - xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt); - if (!xmldoc) - goto cleanup; - - name = virXPathString("string(/domainsnapshot/name)", ctxt); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(xmldoc); + name = virDomainSnapshotGetName(snapshot); if (!name) goto cleanup; + } else { + xml = virDomainSnapshotGetXMLDesc(snapshot, flags); + if (!xml) + goto cleanup; } vshPrint(ctl, "%s", name ? name : xml); - VIR_FREE(name); } ret = true; -- 1.7.4.4
On Thu, Sep 08, 2011 at 01:21:09PM +0100, Eric Blake wrote:
I know I've missed the 0.9.5 RC1 freeze, but think that this new API is worth adding, if only because I noticed that 'virsh snapshot-create dom --no-metadata' currently creates a snapshot with a generated name but then fails in attempting to express that name to the user.
Eric Blake (2): snapshot: new APIs for inspecting snapshot object snapshot: use new API for less work
include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++- tools/virsh.c | 41 +++++------------------ 4 files changed, 90 insertions(+), 33 deletions(-)
Okay, the accessors make sense and the amount of changes are minimal. The improvement of virsh.c make their need very clear ACK to the 2 patches 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/08/2011 02:22 PM, Daniel Veillard wrote:
On Thu, Sep 08, 2011 at 01:21:09PM +0100, Eric Blake wrote:
I know I've missed the 0.9.5 RC1 freeze, but think that this new API is worth adding, if only because I noticed that 'virsh snapshot-create dom --no-metadata' currently creates a snapshot with a generated name but then fails in attempting to express that name to the user.
Eric Blake (2): snapshot: new APIs for inspecting snapshot object snapshot: use new API for less work
include/libvirt/libvirt.h.in | 4 ++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++- tools/virsh.c | 41 +++++------------------ 4 files changed, 90 insertions(+), 33 deletions(-)
Okay, the accessors make sense and the amount of changes are minimal. The improvement of virsh.c make their need very clear
ACK to the 2 patches
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel Veillard -
Eric Blake