On 09/10/2013 04:15 AM, Peter Krempa wrote: ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/10/2013 04:15 AM, Peter Krempa wrote: 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

On 09/10/2013 04:15 AM, Peter Krempa wrote: Interesting name; I don't know that I would come up with much better (ToString is longer to type than Dump). 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

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

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

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

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

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

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

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

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(a)redhat.com&gt; + */ + +#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