[libvirt] [PATCH 00/14] Add ability to handle the <metadata> element using the API
This series adds the initialy omitted functionality of changing the <metadata> element using the virDomain[Get|Set]Metadata API. First few patches are cleanup of some code noticed during implementation of the other stuff. Peter Krempa (14): virsh-domain: Remove spurious ATTRIBUTE_UNUSED from cmdDesc virsh-domain: Line up signal names array qemu: Factor out body of qemuDomainGetMetadata for universal use qemu: Factor out body of qemuDomainSetMetadata for universal use conf: Factor out setting of metadata to simplify code util: Add helper to convert libxml2 nodes to a string virsh-domain: use virXMLNodeDump instead of xmlNodeDump virsh-domain: Add command to allow modifications of XML metadata conf: Add support for requesting of XML metadata via the API conf: allow to add XML metadata using the virDomainSetMetadata api lib: Don't force the key argument when deleting metadata lxc: Add metadata modification APIs test: Add <metadata> support into the test driver tests: Add metadata tests src/conf/domain_conf.c | 171 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 +++ src/libvirt.c | 3 +- src/libvirt_private.syms | 4 + src/lxc/lxc_driver.c | 70 +++++++++++ src/qemu/qemu_driver.c | 122 ++----------------- src/test/test_driver.c | 68 +++++++++++ src/util/virxml.c | 187 +++++++++++++++++++++++++++++ src/util/virxml.h | 13 ++ tests/Makefile.am | 7 ++ tests/metadatatest.c | 245 ++++++++++++++++++++++++++++++++++++++ tools/virsh-domain.c | 304 ++++++++++++++++++++++++++++++++--------------- 12 files changed, 1001 insertions(+), 210 deletions(-) create mode 100644 tests/metadatatest.c -- 1.8.3.2
The "cmd" variable is actually used so remove the attribute. --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 74feca1..f6bfad0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6707,7 +6707,7 @@ static const vshCmdOptDef opts_desc[] = { }; static bool -cmdDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdDesc(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool config = vshCommandOptBool(cmd, "config"); -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The "cmd" variable is actually used so remove the attribute. --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 74feca1..f6bfad0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6707,7 +6707,7 @@ static const vshCmdOptDef opts_desc[] = { };
static bool -cmdDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdDesc(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool config = vshCommandOptBool(cmd, "config");
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Line up the array so that the grid is visible. --- Notes: Not an useful change but it tripped my OCD while looking at the file. tools/virsh-domain.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f6bfad0..727a42a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7009,19 +7009,19 @@ static const vshCmdOptDef opts_send_process_signal[] = { VIR_ENUM_DECL(virDomainProcessSignal) VIR_ENUM_IMPL(virDomainProcessSignal, VIR_DOMAIN_PROCESS_SIGNAL_LAST, - "nop", "hup", "int", "quit", "ill", /* 0-4 */ - "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */ - "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */ - "term", "stkflt", "chld", "cont", "stop", /* 15-19 */ - "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */ + "nop", "hup", "int", "quit", "ill", /* 0-4 */ + "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */ + "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */ + "term", "stkflt", "chld", "cont", "stop", /* 15-19 */ + "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */ "xfsz", "vtalrm", "prof", "winch", "poll", /* 25-29 */ - "pwr", "sys", "rt0","rt1", "rt2", /* 30-34 */ - "rt3", "rt4", "rt5", "rt6", "rt7", /* 35-39 */ - "rt8", "rt9", "rt10", "rt11", "rt12", /* 40-44 */ - "rt13", "rt14", "rt15", "rt16", "rt17", /* 45-49 */ - "rt18", "rt19", "rt20", "rt21", "rt22", /* 50-54 */ - "rt23", "rt24", "rt25", "rt26", "rt27", /* 55-59 */ - "rt28", "rt29", "rt30", "rt31", "rt32") /* 60-64 */ + "pwr", "sys", "rt0", "rt1", "rt2", /* 30-34 */ + "rt3", "rt4", "rt5", "rt6", "rt7", /* 35-39 */ + "rt8", "rt9", "rt10", "rt11", "rt12", /* 40-44 */ + "rt13", "rt14", "rt15", "rt16", "rt17", /* 45-49 */ + "rt18", "rt19", "rt20", "rt21", "rt22", /* 50-54 */ + "rt23", "rt24", "rt25", "rt26", "rt27", /* 55-59 */ + "rt28", "rt29", "rt30", "rt31", "rt32") /* 60-64 */ static int getSignalNumber(vshControl *ctl, const char *signame) { -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
Line up the array so that the grid is visible. ---
Notes: Not an useful change but it tripped my OCD while looking at the file.
tools/virsh-domain.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f6bfad0..727a42a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7009,19 +7009,19 @@ static const vshCmdOptDef opts_send_process_signal[] = { VIR_ENUM_DECL(virDomainProcessSignal) VIR_ENUM_IMPL(virDomainProcessSignal, VIR_DOMAIN_PROCESS_SIGNAL_LAST, - "nop", "hup", "int", "quit", "ill", /* 0-4 */ - "trap", "abrt", "bus", "fpe", "kill", /* 5-9 */ - "usr1", "segv", "usr2", "pipe", "alrm", /* 10-14 */ - "term", "stkflt", "chld", "cont", "stop", /* 15-19 */ - "tstp", "ttin", "ttou", "urg", "xcpu", /* 20-24 */ + "nop", "hup", "int", "quit", "ill", /* 0-4 */
As long as you're lining it up... How about even spacing between columns, rather than minimal spacing for the widest entry in a given column: "nop", "hup", "int", "quit", "ill", /* 0-4 */ At any rate, it's no impact to the code, so ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The function impelemnted common behavior that can be reused for other hypervisor drivers that use the virDomainObj data structures. Factor out the core into a separate helper func. --- src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 49 +++++------------------------------------- 4 files changed, 68 insertions(+), 44 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aed2a9d..d9ed985 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18527,3 +18527,58 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) } return false; } + + +char * +virDomainObjGetMetadata(virDomainObjPtr vm, + int type, + const char *uri ATTRIBUTE_UNUSED, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + virDomainDefPtr def; + char *field = NULL; + char *ret = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, NULL); + + if (virDomainLiveConfigHelperMethod(caps, xmlopt, vm, &flags, &def) < 0) + goto cleanup; + + /* use correct domain definition according to flags */ + if (flags & VIR_DOMAIN_AFFECT_LIVE) + def = vm->def; + + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + field = def->description; + break; + + case VIR_DOMAIN_METADATA_TITLE: + field = def->title; + break; + + case VIR_DOMAIN_METADATA_ELEMENT: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("<metadata> element is not yet supported")); + goto cleanup; + break; + + default: + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + + if (!field) + virReportError(VIR_ERR_NO_DOMAIN_METADATA, "%s", + _("Requested metadata element is not present")); + + ignore_value(VIR_STRDUP(ret, field)); + +cleanup: + return ret; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 539bc1c..173d302 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2738,4 +2738,11 @@ bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); +char *virDomainObjGetMetadata(virDomainObjPtr vm, + int type, + const char *uri, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..2516b9c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -309,6 +309,7 @@ virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainObjAssignDef; virDomainObjCopyPersistentDef; +virDomainObjGetMetadata; virDomainObjGetPersistentDef; virDomainObjGetState; virDomainObjListAdd; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9b5d126..9d5e825 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15144,21 +15144,16 @@ cleanup: static char * qemuDomainGetMetadata(virDomainPtr dom, int type, - const char *uri ATTRIBUTE_UNUSED, + const char *uri, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; + virCapsPtr caps = NULL; virDomainObjPtr vm; - virDomainDefPtr def; char *ret = NULL; - char *field = NULL; - virCapsPtr caps = NULL; - - virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, NULL); if (!(vm = qemuDomObjFromDomain(dom))) - goto cleanup; + return NULL; if (virDomainGetMetadataEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -15166,44 +15161,10 @@ qemuDomainGetMetadata(virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &def) < 0) - goto cleanup; - - /* use correct domain definition according to flags */ - if (flags & VIR_DOMAIN_AFFECT_LIVE) - def = vm->def; - - switch ((virDomainMetadataType) type) { - case VIR_DOMAIN_METADATA_DESCRIPTION: - field = def->description; - break; - case VIR_DOMAIN_METADATA_TITLE: - field = def->title; - break; - case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("QEMU driver does not support " - "<metadata> element")); - goto cleanup; - break; - default: - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("unknown metadata type")); - goto cleanup; - break; - } - - if (!field) { - virReportError(VIR_ERR_NO_DOMAIN_METADATA, "%s", - _("Requested metadata element is not present")); - goto cleanup; - } - - ignore_value(VIR_STRDUP(ret, field)); + ret = virDomainObjGetMetadata(vm, type, uri, caps, driver->xmlopt, flags); cleanup: - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); virObjectUnref(caps); return ret; } -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The function impelemnted common behavior that can be reused for other
s/impelemnted/implemented/
hypervisor drivers that use the virDomainObj data structures. Factor out the core into a separate helper func. --- src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 49 +++++------------------------------------- 4 files changed, 68 insertions(+), 44 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aed2a9d..d9ed985 100644 --- a/src/conf/domain_conf.c
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The function impelemnted common behavior that can be reused for other hypervisor drivers that use the virDomainObj data structures. Factor out the core into a separate helper func. --- src/conf/domain_conf.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 73 ++++------------------------------------- 4 files changed, 103 insertions(+), 66 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d9ed985..e157d49 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18582,3 +18582,88 @@ virDomainObjGetMetadata(virDomainObjPtr vm, cleanup: return ret; } + +int +virDomainObjSetMetadata(virDomainObjPtr vm, + int type, + const char *metadata, + const char *key ATTRIBUTE_UNUSED, + const char *uri ATTRIBUTE_UNUSED, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + const char *configDir, + unsigned int flags) +{ + virDomainDefPtr persistentDef; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (virDomainLiveConfigHelperMethod(caps, xmlopt, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(vm->def->description); + if (VIR_STRDUP(vm->def->description, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(vm->def->title); + if (VIR_STRDUP(vm->def->title, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_ELEMENT: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("<metadata> element is not supported")); + goto cleanup; + break; + + default: + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(persistentDef->description); + if (VIR_STRDUP(persistentDef->description, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(persistentDef->title); + if (VIR_STRDUP(persistentDef->title, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_ELEMENT: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("<metadata> element is not supported")); + goto cleanup; + + default: + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + + if (virDomainSaveConfig(configDir, persistentDef) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 173d302..e4dfee9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2745,4 +2745,14 @@ char *virDomainObjGetMetadata(virDomainObjPtr vm, virDomainXMLOptionPtr xmlopt, unsigned int flags); +int virDomainObjSetMetadata(virDomainObjPtr vm, + int type, + const char *metadata, + const char *key, + const char *uri, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + const char *configDir, + unsigned int flags); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2516b9c..0631941 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -327,6 +327,7 @@ virDomainObjListRemove; virDomainObjListRemoveLocked; virDomainObjNew; virDomainObjSetDefTransient; +virDomainObjSetMetadata; virDomainObjSetState; virDomainObjTaint; virDomainPausedReasonTypeFromString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9d5e825..2468310 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15048,22 +15048,21 @@ static int qemuDomainSetMetadata(virDomainPtr dom, int type, const char *metadata, - const char *key ATTRIBUTE_UNUSED, - const char *uri ATTRIBUTE_UNUSED, + const char *key, + const char *uri, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virDomainDefPtr persistentDef; - int ret = -1; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = qemuDomObjFromDomain(dom))) - goto cleanup; + return -1; cfg = virQEMUDriverGetConfig(driver); @@ -15073,69 +15072,11 @@ qemuDomainSetMetadata(virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) - goto cleanup; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - switch ((virDomainMetadataType) type) { - case VIR_DOMAIN_METADATA_DESCRIPTION: - VIR_FREE(vm->def->description); - if (VIR_STRDUP(vm->def->description, metadata) < 0) - goto cleanup; - break; - case VIR_DOMAIN_METADATA_TITLE: - VIR_FREE(vm->def->title); - if (VIR_STRDUP(vm->def->title, metadata) < 0) - goto cleanup; - break; - case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("QEmu driver does not support modifying " - "<metadata> element")); - goto cleanup; - break; - default: - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("unknown metadata type")); - goto cleanup; - break; - } - } - - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - switch ((virDomainMetadataType) type) { - case VIR_DOMAIN_METADATA_DESCRIPTION: - VIR_FREE(persistentDef->description); - if (VIR_STRDUP(persistentDef->description, metadata) < 0) - goto cleanup; - break; - case VIR_DOMAIN_METADATA_TITLE: - VIR_FREE(persistentDef->title); - if (VIR_STRDUP(persistentDef->title, metadata) < 0) - goto cleanup; - break; - case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("QEMU driver does not support " - "<metadata> element")); - goto cleanup; - default: - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("unknown metadata type")); - goto cleanup; - break; - } - - if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) - goto cleanup; - } - - ret = 0; + ret = virDomainObjSetMetadata(vm, type, metadata, key, uri, caps, + driver->xmlopt, cfg->configDir, flags); cleanup: - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); return ret; -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The function impelemnted common behavior that can be reused for other
s/impelemnted/implemented/
hypervisor drivers that use the virDomainObj data structures. Factor out the core into a separate helper func. --- src/conf/domain_conf.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 73 ++++------------------------------------- 4 files changed, 103 insertions(+), 66 deletions(-)
+ + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(vm->def->description); + if (VIR_STRDUP(vm->def->description, metadata) < 0) + goto cleanup;
Pre-existing - but should we do the VIR_STRDUP into a temporary before freeing the original, and only replace on success, so that at least on an OOM situation we still leave the older string in vm->def rather than losing both strings? On the other hand, OOM recovery is best effort, and someone should already be prepared for weird things to have happened to their guest. Certainly not something to change in this patch. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The code to set the metadata in a domain definition is common to live and inactive domains. Factor it out into a common func. --- src/conf/domain_conf.c | 113 +++++++++++++++++++++++-------------------------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e157d49..aa07056 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18583,87 +18583,80 @@ cleanup: return ret; } + +static int +virDomainDefSetMetadata(virDomainDefPtr def, + int type, + const char *metadata, + const char *key ATTRIBUTE_UNUSED, + const char *uri ATTRIBUTE_UNUSED) +{ + int ret = -1; + + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(def->description); + if (VIR_STRDUP(def->description, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(def->title); + if (VIR_STRDUP(def->title, metadata) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_METADATA_ELEMENT: + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("<metadata> element is not supported")); + goto cleanup; + break; + + default: + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + + ret = 0; + +cleanup: + return ret; +} + + int virDomainObjSetMetadata(virDomainObjPtr vm, int type, const char *metadata, - const char *key ATTRIBUTE_UNUSED, - const char *uri ATTRIBUTE_UNUSED, + const char *key, + const char *uri, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *configDir, unsigned int flags) { virDomainDefPtr persistentDef; - int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); if (virDomainLiveConfigHelperMethod(caps, xmlopt, vm, &flags, &persistentDef) < 0) - goto cleanup; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - switch ((virDomainMetadataType) type) { - case VIR_DOMAIN_METADATA_DESCRIPTION: - VIR_FREE(vm->def->description); - if (VIR_STRDUP(vm->def->description, metadata) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_METADATA_TITLE: - VIR_FREE(vm->def->title); - if (VIR_STRDUP(vm->def->title, metadata) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("<metadata> element is not supported")); - goto cleanup; - break; + return -1; - default: - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("unknown metadata type")); - goto cleanup; - break; - } - } + if (flags & VIR_DOMAIN_AFFECT_LIVE) + if (virDomainDefSetMetadata(vm->def, type, metadata, key, uri) < 0) + return -1; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - switch ((virDomainMetadataType) type) { - case VIR_DOMAIN_METADATA_DESCRIPTION: - VIR_FREE(persistentDef->description); - if (VIR_STRDUP(persistentDef->description, metadata) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_METADATA_TITLE: - VIR_FREE(persistentDef->title); - if (VIR_STRDUP(persistentDef->title, metadata) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("<metadata> element is not supported")); - goto cleanup; - - default: - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("unknown metadata type")); - goto cleanup; - break; - } + if (virDomainDefSetMetadata(persistentDef, type, metadata, key, uri) < 0) + return -1; if (virDomainSaveConfig(configDir, persistentDef) < 0) - goto cleanup; + return -1; } - ret = 0; - -cleanup: - return ret; + return 0; } -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The code to set the metadata in a domain definition is common to live and inactive domains. Factor it out into a common func. --- src/conf/domain_conf.c | 113 +++++++++++++++++++++++-------------------------- 1 file changed, 53 insertions(+), 60 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/libvirt_private.syms | 1 + src/util/virxml.c | 33 +++++++++++++++++++++++++++++++++ src/util/virxml.h | 2 ++ 3 files changed, 36 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0631941..18e9a4b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,6 +2091,7 @@ virUUIDParse; # util/virxml.h virXMLChildElementCount; +virXMLNodeDump; virXMLParseHelper; virXMLPickShellSafeComment; virXMLPropString; diff --git a/src/util/virxml.c b/src/util/virxml.c index f652ee0..44d6f27 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -895,3 +895,36 @@ virXMLChildElementCount(xmlNodePtr node) } return ret; } + + +/** + * virXMLNodeDump: convert a XML node ptr to a XML string + * + * Returns the XML string of the document or NULL on error. + * The caller has to free the string. + */ +char * +virXMLNodeDump(xmlDocPtr doc, + xmlNodePtr node) +{ + xmlBufferPtr xmlbuf = NULL; + char *ret = NULL; + + if (!(xmlbuf = xmlBufferCreate())) { + virReportOOMError(); + return NULL; + } + + if (xmlNodeDump(xmlbuf, doc, node, 0, 1) == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to dump XML node tree")); + goto cleanup; + } + + ignore_value(VIR_STRDUP(ret, (const char *)xmlBufferContent(xmlbuf))); + +cleanup: + xmlBufferFree(xmlbuf); + + return ret; +} diff --git a/src/util/virxml.h b/src/util/virxml.h index 364288d..9165cb1 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -163,4 +163,6 @@ int virXMLSaveFile(const char *path, const char *warnCommand, const char *xml); +char *virXMLNodeDump(xmlDocPtr doc, xmlNodePtr node); + #endif /* __VIR_XML_H__ */ -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
--- src/libvirt_private.syms | 1 + src/util/virxml.c | 33 +++++++++++++++++++++++++++++++++ src/util/virxml.h | 2 ++ 3 files changed, 36 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0631941..18e9a4b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,6 +2091,7 @@ virUUIDParse;
# util/virxml.h virXMLChildElementCount; +virXMLNodeDump;
Interesting name; I don't know that I would come up with much better (ToString is longer to type than Dump).
+ +/** + * virXMLNodeDump: convert a XML node ptr to a XML string
Silly English. The rule is "an" before a leading vowel sound, and since "XML" is pronounced "ex-em-ell", there's a leading vowel sound. s/a XML/an XML/2 ACK with comment fix. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/16/13 23:42, Eric Blake wrote:
On 09/10/2013 04:15 AM, Peter Krempa wrote:
--- src/libvirt_private.syms | 1 + src/util/virxml.c | 33 +++++++++++++++++++++++++++++++++ src/util/virxml.h | 2 ++ 3 files changed, 36 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0631941..18e9a4b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,6 +2091,7 @@ virUUIDParse;
# util/virxml.h virXMLChildElementCount; +virXMLNodeDump;
Interesting name; I don't know that I would come up with much better (ToString is longer to type than Dump).
I went with virXMLNodeToString despite it's longer I liked it better.
+ +/** + * virXMLNodeDump: convert a XML node ptr to a XML string
Silly English. The rule is "an" before a leading vowel sound, and since "XML" is pronounced "ex-em-ell", there's a leading vowel sound.
Good to know that it's based on the sound and not on the actual letter. That's what they don't teach at school here.
s/a XML/an XML/2
ACK with comment fix.
Fixed && pushed; Thanks Peter
--- tools/virsh-domain.c | 117 ++++++++++++++------------------------------------- 1 file changed, 31 insertions(+), 86 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 727a42a..0d00440 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2359,7 +2359,7 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) xmlXPathContextPtr ctxt = NULL; xmlXPathObjectPtr obj = NULL; xmlNodePtr cur = NULL; - xmlBufferPtr xml_buf = NULL; + char *xml_buf = NULL; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -2464,18 +2464,13 @@ hit: goto cleanup; } - xml_buf = xmlBufferCreate(); - if (!xml_buf) { - vshError(ctl, _("Failed to allocate memory")); - goto cleanup; - } - - if (xmlNodeDump(xml_buf, xml, obj->nodesetval->nodeTab[i], 0, 0) < 0) { + if (!(xml_buf = virXMLNodeDump(xml, obj->nodesetval->nodeTab[i]))) { + vshSaveLibvirtError(); vshError(ctl, _("Failed to create XML")); goto cleanup; } - if (virDomainUpdateDeviceFlags(dom, (char *)xmlBufferContent(xml_buf), flags) < 0) { + if (virDomainUpdateDeviceFlags(dom, xml_buf, flags) < 0) { vshError(ctl, _("Failed to update interface link state")); goto cleanup; } else { @@ -2487,10 +2482,8 @@ cleanup: xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); - xmlBufferFree(xml_buf); - - if (dom) - virDomainFree(dom); + VIR_FREE(xml_buf); + virDomainFree(dom); return ret; } @@ -6080,11 +6073,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) bool ret = false; char *buffer; int result; - const char *snippet; + char *snippet = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; - xmlBufferPtr xml_buf = NULL; xmlNodePtr node; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) @@ -6100,17 +6092,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) if ((node = virXPathNode("/cpu|" "/domain/cpu|" "/capabilities/host/cpu", ctxt))) { - if (!(xml_buf = xmlBufferCreate())) { - vshError(ctl, _("Can't create XML buffer to extract CPU element.")); - goto cleanup; - } - - if (xmlNodeDump(xml_buf, xml, node, 0, 0) < 0) { - vshError(ctl, _("Failed to extract CPU element snippet from domain XML.")); + if (!(snippet = virXMLNodeDump(xml, node))) { + vshSaveLibvirtError(); goto cleanup; } - - snippet = (const char *) xmlBufferContent(xml_buf); } else { vshError(ctl, _("File '%s' does not contain a <cpu> element or is not " "a valid domain or capabilities XML"), from); @@ -6146,7 +6131,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(buffer); - xmlBufferFree(xml_buf); + VIR_FREE(snippet); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); @@ -6193,7 +6178,6 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) xmlDocPtr xml = NULL; xmlNodePtr *node_list = NULL; xmlXPathContextPtr ctxt = NULL; - xmlBufferPtr xml_buf = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; @@ -6229,18 +6213,11 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list = vshCalloc(ctl, count, sizeof(const char *)); - if (!(xml_buf = xmlBufferCreate())) - goto no_memory; - for (i = 0; i < count; i++) { - xmlBufferEmpty(xml_buf); - - if (xmlNodeDump(xml_buf, xml, node_list[i], 0, 0) < 0) { - vshError(ctl, _("Failed to extract <cpu> element")); + if (!(list[i] = virXMLNodeDump(xml, node_list[i]))) { + vshSaveLibvirtError(); goto cleanup; } - - list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); } result = virConnectBaselineCPU(ctl->conn, @@ -6254,7 +6231,6 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) cleanup: xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); - xmlBufferFree(xml_buf); VIR_FREE(result); if (list != NULL && count > 0) { for (i = 0; i < count; i++) @@ -9630,7 +9606,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) xmlXPathObjectPtr obj=NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr cur = NULL, matchNode = NULL; - xmlBufferPtr xml_buf = NULL; + char *detach_xml = NULL; const char *mac =NULL, *type = NULL; char *doc = NULL; char buf[64]; @@ -9723,25 +9699,16 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - hit: - xml_buf = xmlBufferCreate(); - if (!xml_buf) { - vshError(ctl, "%s", _("Failed to allocate memory")); - goto cleanup; - } - - if (xmlNodeDump(xml_buf, xml, matchNode, 0, 0) < 0) { - vshError(ctl, "%s", _("Failed to create XML")); +hit: + if (!(detach_xml = virXMLNodeDump(xml, matchNode))) { + vshSaveLibvirtError(); goto cleanup; } - if (flags != 0) { - ret = virDomainDetachDeviceFlags(dom, - (char *)xmlBufferContent(xml_buf), - flags); - } else { - ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf)); - } + if (flags != 0) + ret = virDomainDetachDeviceFlags(dom, detach_xml, flags); + else + ret = virDomainDetachDevice(dom, detach_xml); if (ret != 0) { vshError(ctl, "%s", _("Failed to detach interface")); @@ -9750,13 +9717,13 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) functionReturn = true; } - cleanup: +cleanup: VIR_FREE(doc); + VIR_FREE(detach_xml); virDomainFree(dom); xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); - xmlBufferFree(xml_buf); return functionReturn; } @@ -9874,21 +9841,14 @@ vshPrepareDiskXML(xmlNodePtr disk_node, int type) { xmlNodePtr cur = NULL; - xmlBufferPtr xml_buf = NULL; - char *disk_type = NULL; - char *device_type = NULL; + const char *disk_type = NULL; + const char *device_type = NULL; xmlNodePtr new_node = NULL; char *ret = NULL; if (!disk_node) return NULL; - xml_buf = xmlBufferCreate(); - if (!xml_buf) { - vshError(NULL, "%s", _("Failed to allocate memory")); - return NULL; - } - device_type = virXMLPropString(disk_node, "device"); if (STREQ_NULLABLE(device_type, "cdrom") || @@ -9910,7 +9870,7 @@ vshPrepareDiskXML(xmlNodePtr disk_node, if (type == VSH_PREPARE_DISK_XML_EJECT) { vshError(NULL, _("The disk device '%s' doesn't have media"), path); - goto error; + goto cleanup; } if (source) { @@ -9922,10 +9882,10 @@ vshPrepareDiskXML(xmlNodePtr disk_node, xmlAddChild(disk_node, new_node); } else if (type == VSH_PREPARE_DISK_XML_INSERT) { vshError(NULL, _("No source is specified for inserting media")); - goto error; + goto cleanup; } else if (type == VSH_PREPARE_DISK_XML_UPDATE) { vshError(NULL, _("No source is specified for updating media")); - goto error; + goto cleanup; } } @@ -9933,7 +9893,7 @@ vshPrepareDiskXML(xmlNodePtr disk_node, if (type == VSH_PREPARE_DISK_XML_INSERT) { vshError(NULL, _("The disk device '%s' already has media"), path); - goto error; + goto cleanup; } /* Remove the source if it tends to eject/update media. */ @@ -9949,30 +9909,15 @@ vshPrepareDiskXML(xmlNodePtr disk_node, } } - if (xmlNodeDump(xml_buf, NULL, disk_node, 0, 0) < 0) { - vshError(NULL, "%s", _("Failed to create XML")); - goto error; + if (!(ret = virXMLNodeDump(NULL, disk_node))) { + vshSaveLibvirtError(); + goto cleanup; } - goto cleanup; - cleanup: VIR_FREE(device_type); VIR_FREE(disk_type); - if (xml_buf) { - int len = xmlBufferLength(xml_buf); - if (VIR_ALLOC_N(ret, len + 1) < 0) - return NULL; - memcpy(ret, (char *)xmlBufferContent(xml_buf), len); - ret[len] = '\0'; - xmlBufferFree(xml_buf); - } return ret; - -error: - xmlBufferFree(xml_buf); - xml_buf = NULL; - goto cleanup; } -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
--- tools/virsh-domain.c | 117 ++++++++++++++------------------------------------- 1 file changed, 31 insertions(+), 86 deletions(-)
ACK; nice cleanup. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The metadata modification functions will support modification of the XML metadata. Add a virsh command to allow using this approach. --- tools/virsh-domain.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0d00440..c880394 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6810,6 +6810,161 @@ cleanup: return ret; } + +static const vshCmdInfo info_metadata[] = { + {.name = "help", + .data = N_("show or set domain's custom XML metadata") + }, + {.name = "desc", + .data = N_("Allows to show or modify the XML metadata of a domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_metadata[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("modify/get running state") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("modify/get persistent configuration") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("modify/get current state configuration") + }, + {.name = "edit", + .type = VSH_OT_BOOL, + .help = N_("use an editor to change the metadata") + }, + {.name = "uri", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("URI of the namespace") + }, + {.name = "key", + .type = VSH_OT_DATA, + .help = N_("key to be used as a namespace identificatior"), + }, + {.name = "set", + .type = VSH_OT_DATA, + .help = N_("new metadata to set"), + }, + {.name = "remove", + .type = VSH_OT_BOOL, + .help = N_("remove the metadata corresponding to an uri") + }, + {.name = NULL} +}; + + +/* helper to add new metadata using the --edit option */ +static char * +vshDomainGetEditMetadata(vshControl *ctl, + virDomainPtr dom, + const char *uri, + unsigned int flags) +{ + char *ret; + + if (!(ret = virDomainGetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, + uri, flags))) { + vshResetLibvirtError(); + ret = vshStrdup(ctl, "\n"); + } + + return ret; +} + + +static bool +cmdMetadata(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool edit = vshCommandOptBool(cmd, "edit"); + bool remove = vshCommandOptBool(cmd, "remove"); + const char *set = NULL; + const char *uri = NULL; + const char *key = NULL; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + VSH_EXCLUSIVE_OPTIONS("edit", "set"); + VSH_EXCLUSIVE_OPTIONS("remove", "set"); + VSH_EXCLUSIVE_OPTIONS("remove", "edit"); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "uri", &uri) < 0 || + vshCommandOptStringReq(ctl, cmd, "key", &key) < 0 || + vshCommandOptStringReq(ctl, cmd, "set", &set) < 0) + goto cleanup; + + if ((set || edit) && !key) { + vshError(ctl, "%s", + _("namespace key is required when modifying metadata")); + goto cleanup; + } + + if (set || remove) { + if (virDomainSetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, + set, key, uri, flags)) + goto cleanup; + + if (remove) + vshPrint("%s\n", _("Metadata removed")); + else + vshPrint("%s\n", _("Metadata modified")); + } else if (edit) { +#define EDIT_GET_XML \ + vshDomainGetEditMetadata(ctl, dom, uri, flags) +#define EDIT_NOT_CHANGED \ + vshPrint(ctl, "%s", _("Metadata not changed")); \ + ret = true; \ + goto edit_cleanup; +#define EDIT_DEFINE \ + (virDomainSetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, doc_edited, \ + key, uri, flags) == 0) +#define EDIT_FREE /* nothing */ +#include "virsh-edit.c" + + vshPrint("%s\n", _("Metadata modified")); + } else { + char *data; + /* get */ + if (!(data = virDomainGetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, + uri, flags))) + goto cleanup; + + vshPrint(ctl, "%s\n", data); + VIR_FREE(data); + } + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + + /* * "inject-nmi" command */ @@ -10595,6 +10750,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_memtune, .flags = 0 }, + {.name = "metadata", + .handler = cmdMetadata, + .opts = opts_metadata, + .info = info_metadata, + .flags = 0 + }, {.name = "migrate", .handler = cmdMigrate, .opts = opts_migrate, -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The metadata modification functions will support modification of the XML metadata. Add a virsh command to allow using this approach. --- tools/virsh-domain.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+)
+ {.name = "desc", + .data = N_("Allows to show or modify the XML metadata of a domain.")
Sounds awkward; maybe: Shows or modifies the XML metadata of a domain.
+ {.name = "key", + .type = VSH_OT_DATA, + .help = N_("key to be used as a namespace identificatior"),
s/identificatior/identifier/
+ {.name = "metadata", + .handler = cmdMetadata, + .opts = opts_metadata, + .info = info_metadata, + .flags = 0
Not your fault, but we ought to be using trailing commas in C99 struct initializers. ACK with the grammar fixes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The virDomainGetMetadata function was designed to support also retrieval of app specific metadata from the <metadata> element. This functionality was never implemented originally. --- src/conf/domain_conf.c | 19 ++++---- src/libvirt_private.syms | 1 + src/util/virxml.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 7 +++ 4 files changed, 140 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aa07056..4269690 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18538,7 +18538,6 @@ virDomainObjGetMetadata(virDomainObjPtr vm, unsigned int flags) { virDomainDefPtr def; - char *field = NULL; char *ret = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -18553,17 +18552,21 @@ virDomainObjGetMetadata(virDomainObjPtr vm, switch ((virDomainMetadataType) type) { case VIR_DOMAIN_METADATA_DESCRIPTION: - field = def->description; + if (VIR_STRDUP(ret, def->description) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_TITLE: - field = def->title; + if (VIR_STRDUP(ret, def->title) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("<metadata> element is not yet supported")); - goto cleanup; + if (!def->metadata) + break; + + if (virXMLExtractNamespaceXML(def->metadata, uri, &ret) < 0) + goto cleanup; break; default: @@ -18573,12 +18576,10 @@ virDomainObjGetMetadata(virDomainObjPtr vm, break; } - if (!field) + if (!ret) virReportError(VIR_ERR_NO_DOMAIN_METADATA, "%s", _("Requested metadata element is not present")); - ignore_value(VIR_STRDUP(ret, field)); - cleanup: return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 18e9a4b..b643c51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2091,6 +2091,7 @@ virUUIDParse; # util/virxml.h virXMLChildElementCount; +virXMLExtractNamespaceXML; virXMLNodeDump; virXMLParseHelper; virXMLPickShellSafeComment; diff --git a/src/util/virxml.c b/src/util/virxml.c index 44d6f27..0fac931 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -928,3 +928,125 @@ cleanup: return ret; } + +typedef int (*virXMLForeachCallback)(xmlNodePtr node, + void *opaque); + +static int +virXMLForeachNode(xmlNodePtr root, + virXMLForeachCallback cb, + void *opaque); + +static int +virXMLForeachNode(xmlNodePtr root, + virXMLForeachCallback cb, + void *opaque) +{ + xmlNodePtr next; + int ret; + + for (next = root; next; next = next->next) { + if ((ret = cb(next, opaque)) != 0) + return ret; + + /* recurse into children */ + if (next->children) { + if ((ret = virXMLForeachNode(next->children, cb, opaque)) != 0) + return ret; + } + } + + return 0; +} + + +static int +virXMLRemoveElementNamespace(xmlNodePtr node, + void *opaque) +{ + const char *uri = opaque; + + if (node->ns && + STREQ_NULLABLE((const char *)node->ns->href, uri)) + xmlSetNs(node, NULL); + return 0; +} + + +xmlNodePtr +virXMLFindChildNodeByNs(xmlNodePtr root, + const char *uri) +{ + xmlNodePtr next; + + for (next = root->children; next; next = next->next) { + if (next->ns && + STREQ_NULLABLE((const char *) next->ns->href, uri)) + return next; + } + + return NULL; +} + + +/** + * virXMLExtractNamespaceXML: extract a sub-namespace of a XML as string + */ +int +virXMLExtractNamespaceXML(xmlNodePtr root, + const char *uri, + char **doc) +{ + xmlNodePtr node; + xmlNodePtr nodeCopy = NULL; + xmlNsPtr actualNs; + xmlNsPtr prevNs = NULL; + char *xmlstr = NULL; + int ret = -1; + + if (!(node = virXMLFindChildNodeByNs(root, uri))) { + /* node not found */ + ret = 1; + goto cleanup; + } + + /* copy the node so that we can modify the namespace */ + if (!(nodeCopy = xmlCopyNode(node, 1))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to copy XML node")); + goto cleanup; + } + + virXMLForeachNode(nodeCopy, virXMLRemoveElementNamespace, + (void *)uri); + + /* remove the namespace declaration + * - it's only a single linked list ... doh */ + for (actualNs = nodeCopy->nsDef; actualNs; actualNs = actualNs->next) { + if (STREQ_NULLABLE((const char *)actualNs->href, uri)) { + + /* unlink */ + if (prevNs) + prevNs->next = actualNs->next; + else + nodeCopy->nsDef = actualNs->next; + + /* discard */ + xmlFreeNs(actualNs); + break; + } + + prevNs = actualNs; + } + + if (!(xmlstr = virXMLNodeDump(nodeCopy->doc, nodeCopy))) + goto cleanup; + + ret = 0; + +cleanup: + if (doc) + *doc = xmlstr; + xmlFreeNode(nodeCopy); + return ret; +} diff --git a/src/util/virxml.h b/src/util/virxml.h index 9165cb1..aab29fb 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -165,4 +165,11 @@ int virXMLSaveFile(const char *path, char *virXMLNodeDump(xmlDocPtr doc, xmlNodePtr node); +xmlNodePtr virXMLFindChildNodeByNs(xmlNodePtr root, + const char *uri); + +int virXMLExtractNamespaceXML(xmlNodePtr root, + const char *uri, + char **doc); + #endif /* __VIR_XML_H__ */ -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The virDomainGetMetadata function was designed to support also retrieval of app specific metadata from the <metadata> element. This functionality was never implemented originally. ---
+/** + * virXMLExtractNamespaceXML: extract a sub-namespace of a XML as string
s/a XML/XML/ ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The functionality wasn't originally implemented. This patch adds the ability to modify domain's XML metadata using the API. --- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- src/util/virxml.c | 32 ++++++++++++++++++++++++++++++++ src/util/virxml.h | 4 ++++ 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4269690..69a8748 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18589,9 +18589,12 @@ static int virDomainDefSetMetadata(virDomainDefPtr def, int type, const char *metadata, - const char *key ATTRIBUTE_UNUSED, - const char *uri ATTRIBUTE_UNUSED) + const char *key, + const char *uri) { + xmlDocPtr doc = NULL; + xmlNodePtr old; + xmlNodePtr new; int ret = -1; switch ((virDomainMetadataType) type) { @@ -18608,9 +18611,42 @@ virDomainDefSetMetadata(virDomainDefPtr def, break; case VIR_DOMAIN_METADATA_ELEMENT: - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("<metadata> element is not supported")); - goto cleanup; + if (metadata) { + /* parse and modify the xml from the user */ + if (!(doc = virXMLParseString(metadata, _("(metadata_xml)")))) + goto cleanup; + + if (virXMLInjectNamespace(doc->children, uri, key) < 0) + goto cleanup; + + /* create the root node if needed */ + if (!def->metadata && + !(def->metadata = xmlNewNode(NULL, (unsigned char *)"metadata"))) { + virReportOOMError(); + goto cleanup; + } + + if (!(new = xmlCopyNode(doc->children, 1))) { + virReportOOMError(); + goto cleanup; + } + } + + /* remove possible other nodes sharing the namespace */ + while ((old = virXMLFindChildNodeByNs(def->metadata, uri))) { + xmlUnlinkNode(old); + xmlFreeNode(old); + } + + /* just delete the metadata */ + if (!metadata) + break; + + if (!(xmlAddChild(def->metadata, new))) { + xmlFreeNode(new); + virReportOOMError(); + goto cleanup; + } break; default: @@ -18623,6 +18659,7 @@ virDomainDefSetMetadata(virDomainDefPtr def, ret = 0; cleanup: + xmlFreeDoc(doc); return ret; } diff --git a/src/util/virxml.c b/src/util/virxml.c index 0fac931..59e04f5 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1050,3 +1050,35 @@ cleanup: xmlFreeNode(nodeCopy); return ret; } + + +static int +virXMLAddElementNamespace(xmlNodePtr node, + void *opaque) +{ + xmlNsPtr ns = opaque; + + if (!node->ns) + xmlSetNs(node, ns); + + return 0; +} + + +int +virXMLInjectNamespace(xmlNodePtr node, + const char *uri, + const char *key) +{ + xmlNsPtr ns; + + if (!(ns = xmlNewNs(node, (const unsigned char *)uri, (const unsigned char *)key))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create a new XML namespace")); + return -1; + } + + virXMLForeachNode(node, virXMLAddElementNamespace, ns); + + return 0; +} diff --git a/src/util/virxml.h b/src/util/virxml.h index aab29fb..ff0265b 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -172,4 +172,8 @@ int virXMLExtractNamespaceXML(xmlNodePtr root, const char *uri, char **doc); +int virXMLInjectNamespace(xmlNodePtr node, + const char *uri, + const char *key); + #endif /* __VIR_XML_H__ */ -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
The functionality wasn't originally implemented. This patch adds the ability to modify domain's XML metadata using the API. --- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- src/util/virxml.c | 32 ++++++++++++++++++++++++++++++++ src/util/virxml.h | 4 ++++ 3 files changed, 78 insertions(+), 5 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
virDomainSetMetadata when operating on the metadata element was requesting the @key argument to be passed even if @metadata was NULL used to delete the corresponding metadata element. This is not needed as the key is only used when adding the element and matching is done via the XML namespace. --- src/libvirt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..2c94950 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -10734,7 +10734,8 @@ virDomainSetMetadata(virDomainPtr domain, break; case VIR_DOMAIN_METADATA_ELEMENT: virCheckNonNullArgGoto(uri, error); - virCheckNonNullArgGoto(key, error); + if (metadata) + virCheckNonNullArgGoto(key, error); break; default: /* For future expansion */ -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
virDomainSetMetadata when operating on the metadata element was requesting the @key argument to be passed even if @metadata was NULL used to delete the corresponding metadata element. This is not needed as the key is only used when adding the element and matching is done via the XML namespace. --- src/libvirt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 07a3fd5..2c94950 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -10734,7 +10734,8 @@ virDomainSetMetadata(virDomainPtr domain, break; case VIR_DOMAIN_METADATA_ELEMENT: virCheckNonNullArgGoto(uri, error); - virCheckNonNullArgGoto(key, error); + if (metadata) + virCheckNonNullArgGoto(key, error);
ACK - if we delete ALL metadata (rather than just the metadata tied to a particular key), then it doesn't matter what key we pass. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/lxc/lxc_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b587c22..d49f524 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4591,6 +4591,74 @@ lxcNodeSuspendForDuration(virConnectPtr conn, } +static int +lxcDomainSetMetadata(virDomainPtr dom, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virLXCDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + virLXCDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = lxcDomObjFromDomain(dom))) + return -1; + + cfg = virLXCDriverGetConfig(driver); + + if (virDomainSetMetadataEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (!(caps = virLXCDriverGetCapabilities(driver, false))) + goto cleanup; + + ret = virDomainObjSetMetadata(vm, type, metadata, key, uri, caps, + driver->xmlopt, cfg->configDir, flags); + +cleanup: + virObjectUnlock(vm); + virObjectUnref(caps); + virObjectUnref(cfg); + return ret; +} + + +static char * +lxcDomainGetMetadata(virDomainPtr dom, + int type, + const char *uri, + unsigned int flags) +{ + virLXCDriverPtr driver = dom->conn->privateData; + virCapsPtr caps = NULL; + virDomainObjPtr vm; + char *ret = NULL; + + if (!(vm = lxcDomObjFromDomain(dom))) + return NULL; + + if (virDomainGetMetadataEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!(caps = virLXCDriverGetCapabilities(driver, false))) + goto cleanup; + + ret = virDomainObjGetMetadata(vm, type, uri, caps, driver->xmlopt, flags); + +cleanup: + virObjectUnlock(vm); + virObjectUnref(caps); + return ret; +} + + /* Function Tables */ static virDriver lxcDriver = { .no = VIR_DRV_LXC, @@ -4665,6 +4733,8 @@ static virDriver lxcDriver = { .domainOpenConsole = lxcDomainOpenConsole, /* 0.8.6 */ .connectIsAlive = lxcConnectIsAlive, /* 0.9.8 */ .nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */ + .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */ + .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */ .nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */ .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */ -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
--- src/lxc/lxc_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/test/test_driver.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c225618..23f7b2e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2585,6 +2585,72 @@ cleanup: return ret; } +static char *testDomainGetMetadata(virDomainPtr dom, + int type, + const char *uri, + unsigned int flags) +{ + testConnPtr privconn = dom->conn->privateData; + virDomainObjPtr privdom; + char *ret = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, NULL); + + testDriverLock(privconn); + privdom = virDomainObjListFindByName(privconn->domains, + dom->name); + testDriverUnlock(privconn); + + if (privdom == NULL) { + virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + ret = virDomainObjGetMetadata(privdom, type, uri, privconn->caps, + privconn->xmlopt, flags); + +cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; +} + +static int testDomainSetMetadata(virDomainPtr dom, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + testConnPtr privconn = dom->conn->privateData; + virDomainObjPtr privdom; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + testDriverLock(privconn); + privdom = virDomainObjListFindByName(privconn->domains, + dom->name); + testDriverUnlock(privconn); + + if (privdom == NULL) { + virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto cleanup; + } + + ret = virDomainObjSetMetadata(privdom, type, metadata, key, uri, + privconn->caps, privconn->xmlopt, + NULL, flags); + +cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; +} + + static int testNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freemems, int startCell, int maxCells) { @@ -5872,6 +5938,8 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .domainGetMetadata = testDomainGetMetadata, /* 1.1.3 */ + .domainSetMetadata = testDomainSetMetadata, /* 1.1.3 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
--- src/test/test_driver.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
ACK. Isn't shared code nice :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This test excercises the virDomain[Get|Set]Metadata API and tests it for regressions --- tests/Makefile.am | 7 ++ tests/metadatatest.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 tests/metadatatest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 1a24367..617b5be 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -220,6 +220,8 @@ test_programs += interfacexml2xmltest test_programs += cputest +test_programs += metadatatest + test_scripts = \ capabilityschematest \ interfaceschematest \ @@ -574,6 +576,11 @@ cputest_SOURCES = \ testutils.c testutils.h cputest_LDADD = $(LDADDS) $(LIBXML_LIBS) +metadatatest_SOURCES = \ + metadatatest.c \ + testutils.c testutils.h +metadatatest_LDADD = $(LDADDS) $(LIBXML_LIBS) + virshtest_SOURCES = \ virshtest.c \ testutils.c testutils.h diff --git a/tests/metadatatest.c b/tests/metadatatest.c new file mode 100644 index 0000000..a910f68 --- /dev/null +++ b/tests/metadatatest.c @@ -0,0 +1,245 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ + +#include <config.h> + +#include "testutils.h" + +#include "virerror.h" +#include "virxml.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static const char metadata1[] = +"<derp xmlns:foobar='http://foo.bar/'>\n" +" <bar>foobar</bar>\n" +" <foo fooish='blurb'>foofoo</foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</derp>"; + + +static const char metadata1_ns[] = +"<herp:derp xmlns:foobar='http://foo.bar/' xmlns:herp='http://herp.derp/'>\n" +" <herp:bar>foobar</herp:bar>\n" +" <herp:foo fooish='blurb'>foofoo</herp:foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</herp:derp>"; + + +static const char metadata2[] = +"<foo>\n" +" <bar>baz</bar>\n" +"</foo>"; + + +static const char metadata2_ns[] = +"<blurb:foo xmlns:blurb='http://herp.derp/'>\n" +" <blurb:bar>baz</blurb:bar>\n" +"</blurb:foo>"; + + +static char * +getMetadataFromXML(virDomainPtr dom) +{ + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr node; + + char *xml = NULL; + char *ret = NULL; + + if (!(xml = virDomainGetXMLDesc(dom, 0))) + goto cleanup; + + if (!(doc = virXMLParseStringCtxt(xml, "(domain_definition)", &ctxt))) + goto cleanup; + + if (!(node = virXPathNode("//metadata/*", ctxt))) + goto cleanup; + + ret = virXMLNodeDump(node->doc, node); + +cleanup: + VIR_FREE(xml); + xmlFreeDoc(doc); + xmlXPathFreeContext(ctxt); + + return ret; +} + + +static void +metadataXMLConvertApostrophe(char *str) +{ + do { + if (*str == '\"') + *str = '\''; + } while ((*++str) != '\0'); +} + + +static bool +verifyMetadata(virDomainPtr dom, + const char *expectXML, + const char *expectAPI, + const char *uri) +{ + bool ret = false; + char *metadataXML = NULL; + char *metadataAPI = NULL; + + if (!expectAPI) { + if ((metadataAPI = virDomainGetMetadata(dom, + VIR_DOMAIN_METADATA_ELEMENT, + uri, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected no metadata in API, but got:\n[%s]", + metadataAPI); + goto cleanup; + } + } else { + if (!(metadataAPI = virDomainGetMetadata(dom, + VIR_DOMAIN_METADATA_ELEMENT, + uri, 0))) + goto cleanup; + + metadataXMLConvertApostrophe(metadataAPI); + + if (STRNEQ(metadataAPI, expectAPI)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "XML metadata in API doesn't match expected metadata: " + "expected:\n[%s]\ngot:\n[%s]", + expectAPI, metadataAPI); + goto cleanup; + } + + } + + if (!expectXML) { + if ((metadataXML = getMetadataFromXML(dom))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected no metadata in XML, but got:\n[%s]", + metadataXML); + goto cleanup; + } + } else { + if (!(metadataXML = getMetadataFromXML(dom))) + goto cleanup; + + metadataXMLConvertApostrophe(metadataXML); + + if (STRNEQ(metadataXML, expectXML)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "XML in dump doesn't match expected metadata: " + "expected:\n[%s]\ngot:\n[%s]", + expectXML, metadataXML); + goto cleanup; + } + } + + ret = true; + +cleanup: + VIR_FREE(metadataXML); + VIR_FREE(metadataAPI); + + return ret; +} + + +struct metadataTest { + virConnectPtr conn; + virDomainPtr dom; +}; + + +static int +testAssignMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virDomainSetMetadata(test->dom, VIR_DOMAIN_METADATA_ELEMENT, + metadata1, "herp", "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->dom, metadata1_ns, metadata1, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +testRewriteMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virDomainSetMetadata(test->dom, VIR_DOMAIN_METADATA_ELEMENT, + metadata2, "blurb", "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->dom, metadata2_ns, metadata2, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +testEraseMetadata(const void *data) +{ + const struct metadataTest *test = data; + + if (virDomainSetMetadata(test->dom, VIR_DOMAIN_METADATA_ELEMENT, + NULL, NULL, "http://herp.derp/", 0) < 0) + return -1; + + if (!verifyMetadata(test->dom, NULL, NULL, "http://herp.derp/")) + return -1; + + return 0; +} + +static int +mymain(void) +{ + struct metadataTest test; + int ret = EXIT_SUCCESS; + + if (!(test.conn = virConnectOpen("test:///default"))) + return EXIT_FAILURE; + + if (!(test.dom = virDomainLookupByName(test.conn, "test"))) { + virConnectClose(test.conn); + return EXIT_FAILURE; + } + + if (virtTestRun("Assign metadata ", 1, testAssignMetadata, &test) < 0) + ret = EXIT_FAILURE; + if (virtTestRun("Rewrite Metadata ", 1, testRewriteMetadata, &test) < 0) + ret = EXIT_FAILURE; + if (virtTestRun("Erase metadata ", 1, testEraseMetadata, &test) < 0) + ret = EXIT_FAILURE; + + virDomainFree(test.dom); + virConnectClose(test.conn); + + return ret; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.3.2
On 09/10/2013 04:15 AM, Peter Krempa wrote:
This test excercises the virDomain[Get|Set]Metadata API and tests it for
s/excercises/exercises/
regressions --- tests/Makefile.am | 7 ++ tests/metadatatest.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 tests/metadatatest.c
ACK.
+static const char metadata1[] = +"<derp xmlns:foobar='http://foo.bar/'>\n" +" <bar>foobar</bar>\n" +" <foo fooish='blurb'>foofoo</foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</derp>";
For a good laugh, try reading this aloud while someone else transcribes it :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/17/13 05:01, Eric Blake wrote:
On 09/10/2013 04:15 AM, Peter Krempa wrote:
This test excercises the virDomain[Get|Set]Metadata API and tests it for
s/excercises/exercises/
regressions --- tests/Makefile.am | 7 ++ tests/metadatatest.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 tests/metadatatest.c
ACK.
Thanks for the reviews. I've fixed the grammar nits in the patches and pushed the series. Peter
+static const char metadata1[] = +"<derp xmlns:foobar='http://foo.bar/'>\n" +" <bar>foobar</bar>\n" +" <foo fooish='blurb'>foofoo</foo>\n" +" <foobar:baz>zomg</foobar:baz>\n" +"</derp>";
For a good laugh, try reading this aloud while someone else transcribes it :)
:) :)
On 09/10/2013 04:15 AM, Peter Krempa wrote:
This test excercises the virDomain[Get|Set]Metadata API and tests it for regressions --- tests/Makefile.am | 7 ++ tests/metadatatest.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 tests/metadatatest.c
Your new test is noisy: $ make check ... TEST: metadatatest ..libvirt: Domain Config error : metadata not found: Requested metadata element is not present . 3 OK ... Any chance you can post a followup to reset the (expected) error message? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/10/13 12:15, Peter Krempa wrote:
This series adds the initialy omitted functionality of changing the <metadata> element using the virDomain[Get|Set]Metadata API.
First few patches are cleanup of some code noticed during implementation of the other stuff.
Peter Krempa (14): virsh-domain: Remove spurious ATTRIBUTE_UNUSED from cmdDesc virsh-domain: Line up signal names array qemu: Factor out body of qemuDomainGetMetadata for universal use qemu: Factor out body of qemuDomainSetMetadata for universal use conf: Factor out setting of metadata to simplify code util: Add helper to convert libxml2 nodes to a string virsh-domain: use virXMLNodeDump instead of xmlNodeDump virsh-domain: Add command to allow modifications of XML metadata conf: Add support for requesting of XML metadata via the API conf: allow to add XML metadata using the virDomainSetMetadata api lib: Don't force the key argument when deleting metadata lxc: Add metadata modification APIs test: Add <metadata> support into the test driver tests: Add metadata tests
Ping? Any volunteer for the horrendous task of reviewing this? :) Peter
participants (2)
-
Eric Blake -
Peter Krempa