[libvirt] [PATCHv3 0/4] API for modification of domain metadata

This third version incorporates fixes of Eric's review and a rebase on the top of Jirka's new API. Differences to v2: xml: Add element <title> to allow short description of domains - Move <title> before <description> - fix tests to accomodate this - remove forgotten length limits from docs - fixed XML schema to work around libxml2 newline escape bug API: Add api to set and get domain metadata - added new remote procedures to remote_protocol-structs ( and installed tools to detect it in the future :( ) - Fixed function docs to accomodate new code semantics - Changed error message to sync with other API functions - Changed debug prints to print strings instead of pointers - Removed conditions forbidding expansion/work with newer versins - Changed RPC return type to nonnull_string virsh: Add support for modifying domain description and titles qemu: Add support for virDomainGetMetadata and virDomainSetMetadata - no changes docs/formatdomain.html.in | 8 +- docs/schemas/domaincommon.rng | 15 +- include/libvirt/libvirt.h.in | 24 ++ include/libvirt/virterror.h | 1 + src/conf/domain_conf.c | 11 + src/conf/domain_conf.h | 1 + src/driver.h | 16 ++ src/libvirt.c | 178 ++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_driver.c | 174 ++++++++++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 24 ++- src/remote_protocol-structs | 19 ++ src/util/virterror.c | 6 + .../qemu-simple-description-title.xml | 27 ++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 + tools/virsh.c | 283 ++++++++++++++++++-- tools/virsh.pod | 34 +++- 18 files changed, 806 insertions(+), 24 deletions(-) create mode 100644 tests/domainschemadata/qemu-simple-description-title.xml -- 1.7.3.4

This patch adds a new element <title> to the domain XML. This attribute can hold a short title defined by the user to ease the identification of domains. The title may not contain newlines and should be reasonably short. *docs/formatdomain.html.in *docs/schemas/domaincommon.rng - add schema grammar for the new element and documentation *src/conf/domain_conf.c *src/conf/domain_conf.h - add field to hold the new attribute - add code to parse and create XML with the new attribute --- docs/formatdomain.html.in | 8 +++++- docs/schemas/domaincommon.rng | 15 ++++++++++- src/conf/domain_conf.c | 11 ++++++++ src/conf/domain_conf.h | 1 + .../qemu-simple-description-title.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 +++ 6 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/domainschemadata/qemu-simple-description-title.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d58a5e1..99152b6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -32,6 +32,7 @@ <domain type='xen' id='3'> <name>fv0</name> <uuid>4dea22b31d52d8f32516782e98ab3fa0</uuid> + <title>A short description - title - of the domain</title> <description>Some human readable description</description> <metadata> <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo> @@ -58,6 +59,11 @@ specification. <span class="since">Since 0.0.1, sysinfo since 0.8.7</span></dd> + <dt><code>title</code></dt> + <dd>The optional element <code>title</code> provides space for a + short description of the domain. The title should not contain + any newlines. <span class="since">Since 0.9.10</span>.</dd> + <dt><code>description</code></dt> <dd>The content of the <code>description</code> element provides a human readable description of the virtual machine. This data is not @@ -72,7 +78,7 @@ (if the application needs structure, they should have sub-elements to their namespace element). <span class="since">Since 0.9.10</span></dd> - </dl> + </dl> <h3><a name="elementsOS">Operating system booting</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2423154..1576233 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6,7 +6,7 @@ <include href='networkcommon.rng'/> <!-- - description element, may be placed anywhere under the root + description and title element, may be placed anywhere under the root --> <define name="description"> <element name="description"> @@ -14,6 +14,16 @@ </element> </define> + <define name="title"> + <element name="title"> + <data type="string"> + <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> + <param name="pattern">[^ +]+</param> + </data> + </element> + </define> + <!-- We handle only document defining a domain --> @@ -23,6 +33,9 @@ <ref name="ids"/> <interleave> <optional> + <ref name="title"/> + </optional> + <optional> <ref name="description"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 35cb7a4..072fcc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1481,6 +1481,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->cpumask); VIR_FREE(def->emulator); VIR_FREE(def->description); + VIR_FREE(def->title); virBlkioDeviceWeightArrayClear(def->blkio.devices, def->blkio.ndevices); @@ -7158,6 +7159,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(tmp); } + /* Extract short description of domain (title) */ + def->title = virXPathString("string(./title[1])", ctxt); + if (def->title && strchr(def->title, '\n')) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Domain title can't contain newlines")); + goto error; + } + /* Extract documentation if present */ def->description = virXPathString("string(./description[1])", ctxt); @@ -11487,6 +11496,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferEscapeString(buf, " <title>%s</title>\n", def->title); + virBufferEscapeString(buf, " <description>%s</description>\n", def->description); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 503684f..acb936e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1425,6 +1425,7 @@ struct _virDomainDef { int id; unsigned char uuid[VIR_UUID_BUFLEN]; char *name; + char *title; char *description; struct { diff --git a/tests/domainschemadata/qemu-simple-description-title.xml b/tests/domainschemadata/qemu-simple-description-title.xml new file mode 100644 index 0000000..a8a9cac --- /dev/null +++ b/tests/domainschemadata/qemu-simple-description-title.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>qemu-demo</name> + <uuid>603cc28c-9841-864e-0949-8cc7d3bae9f8</uuid> + <memory>65536</memory> + <currentMemory>65536</currentMemory> + <title>A short description of this domain</title> + <description> + A longer explanation that this domain is a test domain + for validating domain schemas. + </description> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.14'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml index 2f13d46..51eb59a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml @@ -1,6 +1,11 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> <memory>219100</memory> <currentMemory>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu> -- 1.7.3.4

On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds a new element <title> to the domain XML. This attribute can hold a short title defined by the user to ease the identification of domains. The title may not contain newlines and should be reasonably short.
*docs/formatdomain.html.in *docs/schemas/domaincommon.rng - add schema grammar for the new element and documentation *src/conf/domain_conf.c *src/conf/domain_conf.h - add field to hold the new attribute - add code to parse and create XML with the new attribute --- docs/formatdomain.html.in | 8 +++++- docs/schemas/domaincommon.rng | 15 ++++++++++- src/conf/domain_conf.c | 11 ++++++++ src/conf/domain_conf.h | 1 + .../qemu-simple-description-title.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 +++ 6 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/domainschemadata/qemu-simple-description-title.xml
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Feb 01, 2012 at 02:03:49PM +0100, Peter Krempa wrote:
This patch adds a new element <title> to the domain XML. This attribute can hold a short title defined by the user to ease the identification of domains. The title may not contain newlines and should be reasonably short.
*docs/formatdomain.html.in *docs/schemas/domaincommon.rng - add schema grammar for the new element and documentation *src/conf/domain_conf.c *src/conf/domain_conf.h - add field to hold the new attribute - add code to parse and create XML with the new attribute --- docs/formatdomain.html.in | 8 +++++- docs/schemas/domaincommon.rng | 15 ++++++++++- src/conf/domain_conf.c | 11 ++++++++ src/conf/domain_conf.h | 1 + .../qemu-simple-description-title.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 +++ 6 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/domainschemadata/qemu-simple-description-title.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d58a5e1..99152b6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -32,6 +32,7 @@ <domain type='xen' id='3'> <name>fv0</name> <uuid>4dea22b31d52d8f32516782e98ab3fa0</uuid> + <title>A short description - title - of the domain</title> <description>Some human readable description</description> <metadata> <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo> @@ -58,6 +59,11 @@ specification. <span class="since">Since 0.0.1, sysinfo since 0.8.7</span></dd>
+ <dt><code>title</code></dt> + <dd>The optional element <code>title</code> provides space for a + short description of the domain. The title should not contain + any newlines. <span class="since">Since 0.9.10</span>.</dd> + <dt><code>description</code></dt> <dd>The content of the <code>description</code> element provides a human readable description of the virtual machine. This data is not @@ -72,7 +78,7 @@ (if the application needs structure, they should have sub-elements to their namespace element). <span class="since">Since 0.9.10</span></dd> - </dl> + </dl>
<h3><a name="elementsOS">Operating system booting</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2423154..1576233 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6,7 +6,7 @@ <include href='networkcommon.rng'/>
<!-- - description element, may be placed anywhere under the root + description and title element, may be placed anywhere under the root --> <define name="description"> <element name="description"> @@ -14,6 +14,16 @@ </element> </define>
+ <define name="title"> + <element name="title"> + <data type="string"> + <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> + <param name="pattern">[^ +]+</param>
Hum, using [^ ]+ should do the trick too I think, since rng is XML it would be first interpreted by the parser and replaced by the equivalent character i.e. \n.
+ </data> + </element> + </define> + <!-- We handle only document defining a domain --> @@ -23,6 +33,9 @@ <ref name="ids"/> <interleave> <optional> + <ref name="title"/> + </optional> + <optional> <ref name="description"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 35cb7a4..072fcc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1481,6 +1481,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->cpumask); VIR_FREE(def->emulator); VIR_FREE(def->description); + VIR_FREE(def->title);
virBlkioDeviceWeightArrayClear(def->blkio.devices, def->blkio.ndevices); @@ -7158,6 +7159,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(tmp); }
+ /* Extract short description of domain (title) */ + def->title = virXPathString("string(./title[1])", ctxt); + if (def->title && strchr(def->title, '\n')) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Domain title can't contain newlines")); + goto error; + } + /* Extract documentation if present */ def->description = virXPathString("string(./description[1])", ctxt);
@@ -11487,6 +11496,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr);
+ virBufferEscapeString(buf, " <title>%s</title>\n", def->title); + virBufferEscapeString(buf, " <description>%s</description>\n", def->description);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 503684f..acb936e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1425,6 +1425,7 @@ struct _virDomainDef { int id; unsigned char uuid[VIR_UUID_BUFLEN]; char *name; + char *title; char *description;
struct { diff --git a/tests/domainschemadata/qemu-simple-description-title.xml b/tests/domainschemadata/qemu-simple-description-title.xml new file mode 100644 index 0000000..a8a9cac --- /dev/null +++ b/tests/domainschemadata/qemu-simple-description-title.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>qemu-demo</name> + <uuid>603cc28c-9841-864e-0949-8cc7d3bae9f8</uuid> + <memory>65536</memory> + <currentMemory>65536</currentMemory> + <title>A short description of this domain</title> + <description> + A longer explanation that this domain is a test domain + for validating domain schemas. + </description> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.14'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml index 2f13d46..51eb59a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml @@ -1,6 +1,11 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> <memory>219100</memory> <currentMemory>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu>
ACK, fairly close to what I wrote too, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch adds API to modify domain metadata for running and stopped domains. The api supports changing description, title as well as the newly added <metadata> element. The API has support for storing data in the metadata element using xml namespaces. * include/libvirt/libvirt.h.in * src/libvirt_public.syms - add function headers - add enum to select metadata to operate on - export functions * src/libvirt.c - add public api implementation * src/driver.h - add driver support * src/remote/remote_driver.c * src/remote/remote_protocol.x - wire up the remote protocol * include/libvirt/virterror.h * src/util/virterror.c - add a new error message note that metadata for domain are missing --- include/libvirt/libvirt.h.in | 24 ++++++ include/libvirt/virterror.h | 1 + src/driver.h | 16 ++++ src/libvirt.c | 178 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 24 ++++++- src/remote_protocol-structs | 19 +++++ src/util/virterror.c | 6 ++ 9 files changed, 271 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cf53cf2..07d03fa 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1496,6 +1496,30 @@ int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +typedef enum { + VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */ + VIR_DOMAIN_METADATA_TITLE = 1, /* Operate on <title> */ + VIR_DOMAIN_METADATA_ELEMENT = 2, /* Operate on <metadata> */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_METADATA_LAST +#endif +} virDomainMetadataType; + +int +virDomainSetMetadata(virDomainPtr domain, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +char * +virDomainGetMetadata(virDomainPtr domain, + int type, + const char *uri, + unsigned int flags); + /* * XML domain description */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 9844cbe..9dbadfe 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -244,6 +244,7 @@ typedef enum { VIR_ERR_OPERATION_ABORTED = 78, /* operation on a domain was canceled/aborted by user */ VIR_ERR_AUTH_CANCELLED = 79, /* authentication cancelled */ + VIR_ERR_NO_DOMAIN_METADATA = 80, /* The metadata is not present */ } virErrorNumber; /** diff --git a/src/driver.h b/src/driver.h index 9ff5edf..d6ee60f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -816,6 +816,20 @@ typedef int unsigned int maxerrors, unsigned int flags); +typedef int + (*virDrvDomainSetMetadata)(virDomainPtr dom, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +typedef char * + (*virDrvDomainGetMetadata)(virDomainPtr dom, + int type, + const char *uri, + unsigned int flags); + /** * _virDriver: * @@ -988,6 +1002,8 @@ struct _virDriver { virDrvDomainGetBlockIoTune domainGetBlockIoTune; virDrvDomainGetCPUStats domainGetCPUStats; virDrvDomainGetDiskErrors domainGetDiskErrors; + virDrvDomainSetMetadata domainSetMetadata; + virDrvDomainGetMetadata domainGetMetadata; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index fc41a3f..659e5db 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8887,6 +8887,184 @@ error: } /** + * virDomainSetMetadata: + * @domain: a domain object + * @type: type of description, from virDomainMetadataType + * @metadata: new metadata text + * @key: XML namespace key, or NULL + * @uri: XML namespace URI, or NULL + * @flags: bitwise-OR of virDomainModificationImpact + * + * Sets the appropriate domain element given by @type to the + * value of @description. A @type of VIR_DOMAIN_METADATA_DESCRIPTION + * is free-form text; VIR_DOMAIN_METADATA_TITLE is free-form, but no + * newlines are permitted, and should be short (although the length is + * not enforced). For these two options @key and @uri are irrelevant and + * must be set to NULL. + * + * For type VIR_DOMAIN_METADATA_ELEMENT @metadata must be well-formed + * XML belonging to namespace defined by @uri with local name @key. + * + * Passing NULL for @metadata says to remove that element from the + * domain XML (passing the empty string leaves the element present). + * + * The resulting metadata will be present in virDomainGetXMLDesc(), + * as well as quick access through virDomainGetMetadata(). + * + * @flags controls whether the live domain, persistent configuration, + * or both will be modified. + * + * Returns 0 on success, -1 in case of failure. + */ +int +virDomainSetMetadata(virDomainPtr domain, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, + "type=%d, metadata='%s', key='%s', uri='%s', flags=%x", + type, NULLSTR(metadata), NULLSTR(key), NULLSTR(uri), + flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + switch (type) { + case VIR_DOMAIN_METADATA_TITLE: + if (metadata && strchr(metadata, '\n')) { + virLibDomainError(VIR_ERR_INVALID_ARG, "%s", + _("Domain title can't contain newlines")); + goto error; + } + /* fallthrough */ + case VIR_DOMAIN_METADATA_DESCRIPTION: + if (uri || key) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + case VIR_DOMAIN_METADATA_ELEMENT: + if (!uri || !key) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + default: + /* For future expansion */ + break; + } + + if (conn->driver->domainSetMetadata) { + int ret; + ret = conn->driver->domainSetMetadata(domain, type, metadata, key, uri, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** + * virDomainGetMetadata: + * @domain: a domain object + * @type: type of description, from virDomainMetadataType + * @uri: XML namespace identifier + * @flags: bitwise-OR of virDomainModificationImpact + * + * Retrieves the appropriate domain element given by @type. + * If VIR_DOMAIN_METADATA_ELEMENT is requested parameter @uri + * must be set to the name of the namespace the requested elements + * belong to, otherwise must be NULL. + * + * If an element of the domain XML is not present, the resulting + * error will be VIR_ERR_NO_DOMAIN_METADATA. This method forms + * a shortcut for seeing information from virDomainSetMetadata() + * without having to go through virDomainGetXMLDesc(). + * + * @flags controls whether the live domain or persistent + * configuration will be queried. + * + * Returns the metadata string on success (caller must free), + * or NULL in case of failure. + */ +char * +virDomainGetMetadata(virDomainPtr domain, + int type, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "type=%d, uri='%s', flags=%x", + type, NULLSTR(uri), flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + switch (type) { + case VIR_DOMAIN_METADATA_TITLE: + case VIR_DOMAIN_METADATA_DESCRIPTION: + if (uri) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + case VIR_DOMAIN_METADATA_ELEMENT: + if (!uri) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + default: + /* For future expansion */ + break; + } + + conn = domain->conn; + + if (conn->driver->domainGetMetadata) { + char *ret; + if (!(ret = conn->driver->domainGetMetadata(domain, type, uri, flags))) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return NULL; +} + +/** * virNodeGetSecurityModel: * @conn: a connection object * @secmodel: pointer to a virSecurityModel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ced9fb3..9bbd308 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -520,7 +520,9 @@ LIBVIRT_0.9.10 { global: virDomainGetCPUStats; virDomainGetDiskErrors; + virDomainGetMetadata; virDomainPMSuspendForDuration; + virDomainSetMetadata; virDomainShutdownFlags; virStorageVolResize; virStorageVolWipePattern; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 09b5ace..34816ad 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4917,6 +4917,8 @@ static virDriver remote_driver = { .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ .domainGetCPUStats = remoteDomainGetCPUStats, /* 0.9.10 */ .domainGetDiskErrors = remoteDomainGetDiskErrors, /* 0.9.10 */ + .domainSetMetadata = remoteDomainSetMetadata, /* 0.9.10 */ + .domainGetMetadata = remoteDomainGetMetadata, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 10fd294..efb209a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1123,6 +1123,26 @@ struct remote_domain_set_autostart_args { int autostart; }; +struct remote_domain_set_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + unsigned int flags; +}; + +struct remote_domain_get_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string uri; + unsigned int flags; +}; + +struct remote_domain_get_metadata_ret { + remote_nonnull_string metadata; +}; + struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2729,7 +2749,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_METADATA = 264, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_METADATA = 265 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ee2207c..8b66a5f 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -788,6 +788,23 @@ struct remote_domain_set_autostart_args { remote_nonnull_domain dom; int autostart; }; +struct remote_domain_set_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_domain_get_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string uri; + u_int flags; +}; +struct remote_domain_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2146,4 +2163,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261, REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262, REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263, + REMOTE_PROC_DOMAIN_SET_METADATA = 264, + REMOTE_PROC_DOMAIN_GET_METADATA = 265, }; diff --git a/src/util/virterror.c b/src/util/virterror.c index 31ddd9d..640f5d8 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1226,6 +1226,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("operation aborted: %s"); break; + case VIR_ERR_NO_DOMAIN_METADATA: + if (info == NULL) + errmsg = _("metadata not found"); + else + errmsg = _("metadata not found"); + break; } return (errmsg); } -- 1.7.3.4

On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds API to modify domain metadata for running and stopped domains. The api supports changing description, title as well as the newly added <metadata> element. The API has support for storing data in the metadata element using xml namespaces.
* include/libvirt/libvirt.h.in * src/libvirt_public.syms - add function headers - add enum to select metadata to operate on - export functions * src/libvirt.c - add public api implementation * src/driver.h - add driver support * src/remote/remote_driver.c * src/remote/remote_protocol.x - wire up the remote protocol * include/libvirt/virterror.h * src/util/virterror.c - add a new error message note that metadata for domain are missing --- include/libvirt/libvirt.h.in | 24 ++++++ include/libvirt/virterror.h | 1 + src/driver.h | 16 ++++ src/libvirt.c | 178 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 24 ++++++- src/remote_protocol-structs | 19 +++++ src/util/virterror.c | 6 ++ 9 files changed, 271 insertions(+), 1 deletions(-)
ACK with one nit:
+++ b/src/util/virterror.c @@ -1226,6 +1226,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("operation aborted: %s"); break; + case VIR_ERR_NO_DOMAIN_METADATA: + if (info == NULL) + errmsg = _("metadata not found"); + else + errmsg = _("metadata not found");
The else branch should be: "metadata not found: %s" -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Feb 01, 2012 at 02:03:50PM +0100, Peter Krempa wrote:
This patch adds API to modify domain metadata for running and stopped domains. The api supports changing description, title as well as the newly added <metadata> element. The API has support for storing data in the metadata element using xml namespaces.
* include/libvirt/libvirt.h.in * src/libvirt_public.syms - add function headers - add enum to select metadata to operate on - export functions * src/libvirt.c - add public api implementation * src/driver.h - add driver support * src/remote/remote_driver.c * src/remote/remote_protocol.x - wire up the remote protocol * include/libvirt/virterror.h * src/util/virterror.c - add a new error message note that metadata for domain are missing --- include/libvirt/libvirt.h.in | 24 ++++++ include/libvirt/virterror.h | 1 + src/driver.h | 16 ++++ src/libvirt.c | 178 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 24 ++++++- src/remote_protocol-structs | 19 +++++ src/util/virterror.c | 6 ++ 9 files changed, 271 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cf53cf2..07d03fa 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1496,6 +1496,30 @@ int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel);
+typedef enum { + VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */ + VIR_DOMAIN_METADATA_TITLE = 1, /* Operate on <title> */ + VIR_DOMAIN_METADATA_ELEMENT = 2, /* Operate on <metadata> */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_METADATA_LAST +#endif +} virDomainMetadataType; + +int +virDomainSetMetadata(virDomainPtr domain, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +char * +virDomainGetMetadata(virDomainPtr domain, + int type, + const char *uri, + unsigned int flags); + /* * XML domain description */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 9844cbe..9dbadfe 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -244,6 +244,7 @@ typedef enum { VIR_ERR_OPERATION_ABORTED = 78, /* operation on a domain was canceled/aborted by user */ VIR_ERR_AUTH_CANCELLED = 79, /* authentication cancelled */ + VIR_ERR_NO_DOMAIN_METADATA = 80, /* The metadata is not present */ } virErrorNumber;
/** diff --git a/src/driver.h b/src/driver.h index 9ff5edf..d6ee60f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -816,6 +816,20 @@ typedef int unsigned int maxerrors, unsigned int flags);
+typedef int + (*virDrvDomainSetMetadata)(virDomainPtr dom, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags); + +typedef char * + (*virDrvDomainGetMetadata)(virDomainPtr dom, + int type, + const char *uri, + unsigned int flags); + /** * _virDriver: * @@ -988,6 +1002,8 @@ struct _virDriver { virDrvDomainGetBlockIoTune domainGetBlockIoTune; virDrvDomainGetCPUStats domainGetCPUStats; virDrvDomainGetDiskErrors domainGetDiskErrors; + virDrvDomainSetMetadata domainSetMetadata; + virDrvDomainGetMetadata domainGetMetadata; };
typedef int diff --git a/src/libvirt.c b/src/libvirt.c index fc41a3f..659e5db 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8887,6 +8887,184 @@ error: }
/** + * virDomainSetMetadata: + * @domain: a domain object + * @type: type of description, from virDomainMetadataType + * @metadata: new metadata text + * @key: XML namespace key, or NULL + * @uri: XML namespace URI, or NULL + * @flags: bitwise-OR of virDomainModificationImpact + * + * Sets the appropriate domain element given by @type to the + * value of @description. A @type of VIR_DOMAIN_METADATA_DESCRIPTION + * is free-form text; VIR_DOMAIN_METADATA_TITLE is free-form, but no + * newlines are permitted, and should be short (although the length is + * not enforced). For these two options @key and @uri are irrelevant and + * must be set to NULL. + * + * For type VIR_DOMAIN_METADATA_ELEMENT @metadata must be well-formed + * XML belonging to namespace defined by @uri with local name @key. + * + * Passing NULL for @metadata says to remove that element from the + * domain XML (passing the empty string leaves the element present). + * + * The resulting metadata will be present in virDomainGetXMLDesc(), + * as well as quick access through virDomainGetMetadata(). + * + * @flags controls whether the live domain, persistent configuration, + * or both will be modified. + * + * Returns 0 on success, -1 in case of failure. + */ +int +virDomainSetMetadata(virDomainPtr domain, + int type, + const char *metadata, + const char *key, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, + "type=%d, metadata='%s', key='%s', uri='%s', flags=%x", + type, NULLSTR(metadata), NULLSTR(key), NULLSTR(uri), + flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + switch (type) { + case VIR_DOMAIN_METADATA_TITLE: + if (metadata && strchr(metadata, '\n')) { + virLibDomainError(VIR_ERR_INVALID_ARG, "%s", + _("Domain title can't contain newlines")); + goto error; + } + /* fallthrough */ + case VIR_DOMAIN_METADATA_DESCRIPTION: + if (uri || key) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + case VIR_DOMAIN_METADATA_ELEMENT: + if (!uri || !key) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + default: + /* For future expansion */ + break;
Okay we let the driver error directly on unsupported metadata type
+ } + + if (conn->driver->domainSetMetadata) { + int ret; + ret = conn->driver->domainSetMetadata(domain, type, metadata, key, uri, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** + * virDomainGetMetadata: + * @domain: a domain object + * @type: type of description, from virDomainMetadataType + * @uri: XML namespace identifier + * @flags: bitwise-OR of virDomainModificationImpact + * + * Retrieves the appropriate domain element given by @type. + * If VIR_DOMAIN_METADATA_ELEMENT is requested parameter @uri + * must be set to the name of the namespace the requested elements + * belong to, otherwise must be NULL. + * + * If an element of the domain XML is not present, the resulting + * error will be VIR_ERR_NO_DOMAIN_METADATA. This method forms + * a shortcut for seeing information from virDomainSetMetadata() + * without having to go through virDomainGetXMLDesc(). + * + * @flags controls whether the live domain or persistent + * configuration will be queried. + * + * Returns the metadata string on success (caller must free), + * or NULL in case of failure. + */ +char * +virDomainGetMetadata(virDomainPtr domain, + int type, + const char *uri, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "type=%d, uri='%s', flags=%x", + type, NULLSTR(uri), flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + switch (type) { + case VIR_DOMAIN_METADATA_TITLE: + case VIR_DOMAIN_METADATA_DESCRIPTION: + if (uri) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + case VIR_DOMAIN_METADATA_ELEMENT: + if (!uri) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + break; + default: + /* For future expansion */ + break; + } + + conn = domain->conn; + + if (conn->driver->domainGetMetadata) { + char *ret; + if (!(ret = conn->driver->domainGetMetadata(domain, type, uri, flags))) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return NULL; +} + +/** * virNodeGetSecurityModel: * @conn: a connection object * @secmodel: pointer to a virSecurityModel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ced9fb3..9bbd308 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -520,7 +520,9 @@ LIBVIRT_0.9.10 { global: virDomainGetCPUStats; virDomainGetDiskErrors; + virDomainGetMetadata; virDomainPMSuspendForDuration; + virDomainSetMetadata; virDomainShutdownFlags; virStorageVolResize; virStorageVolWipePattern; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 09b5ace..34816ad 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4917,6 +4917,8 @@ static virDriver remote_driver = { .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ .domainGetCPUStats = remoteDomainGetCPUStats, /* 0.9.10 */ .domainGetDiskErrors = remoteDomainGetDiskErrors, /* 0.9.10 */ + .domainSetMetadata = remoteDomainSetMetadata, /* 0.9.10 */ + .domainGetMetadata = remoteDomainGetMetadata, /* 0.9.10 */ };
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 10fd294..efb209a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1123,6 +1123,26 @@ struct remote_domain_set_autostart_args { int autostart; };
+struct remote_domain_set_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + unsigned int flags; +}; + +struct remote_domain_get_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string uri; + unsigned int flags; +}; + +struct remote_domain_get_metadata_ret { + remote_nonnull_string metadata; +}; + struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2729,7 +2749,9 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_METADATA = 264, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_METADATA = 265 /* autogen autogen */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ee2207c..8b66a5f 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -788,6 +788,23 @@ struct remote_domain_set_autostart_args { remote_nonnull_domain dom; int autostart; }; +struct remote_domain_set_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string metadata; + remote_string key; + remote_string uri; + u_int flags; +}; +struct remote_domain_get_metadata_args { + remote_nonnull_domain dom; + int type; + remote_string uri; + u_int flags; +}; +struct remote_domain_get_metadata_ret { + remote_nonnull_string metadata; +}; struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2146,4 +2163,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_SUSPEND_FOR_DURATION = 261, REMOTE_PROC_DOMAIN_GET_CPU_STATS = 262, REMOTE_PROC_DOMAIN_GET_DISK_ERRORS = 263, + REMOTE_PROC_DOMAIN_SET_METADATA = 264, + REMOTE_PROC_DOMAIN_GET_METADATA = 265, }; diff --git a/src/util/virterror.c b/src/util/virterror.c index 31ddd9d..640f5d8 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1226,6 +1226,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("operation aborted: %s"); break; + case VIR_ERR_NO_DOMAIN_METADATA: + if (info == NULL) + errmsg = _("metadata not found");
%s missing as Eric pointed out,
+ else + errmsg = _("metadata not found"); + break; } return (errmsg); }
ACK, better than what I had in mind :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch adds a new command "desc" to show and modify titles and description for the domains using the new API. This patch also adds a new flag for the "list" command to show titles in the domain list, to allow easy identification of VMs by storing a short description. Example: virsh # list --title Id Name State Title ----------------------------------------------- 0 Domain-0 running Mailserver 1 2 fedora paused --- tools/virsh.c | 283 +++++++++++++++++++++++++++++++++++++++++++++++++++---- tools/virsh.pod | 34 +++++++- 2 files changed, 296 insertions(+), 21 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 23962bf..cb5b88a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -313,6 +313,9 @@ static int vshCommandOptULongLong(const vshCmd *cmd, const char *name, static bool vshCommandOptBool(const vshCmd *cmd, const char *name); static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt); +static char *vshGetDomainDescription(vshControl *ctl, virDomainPtr dom, + bool title, unsigned int flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; #define VSH_BYID (1 << 1) #define VSH_BYUUID (1 << 2) @@ -886,6 +889,7 @@ static const vshCmdOptDef opts_list[] = { {"all", VSH_OT_BOOL, 0, N_("list inactive & active domains")}, {"managed-save", VSH_OT_BOOL, 0, N_("mark domains with managed save state")}, + {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, {NULL, 0, 0, NULL} }; @@ -900,7 +904,10 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) char **names = NULL; int maxname = 0; bool managed = vshCommandOptBool(cmd, "managed-save"); + bool desc = vshCommandOptBool(cmd, "title"); + char *title; int state; + bool ret = false; inactive |= all; @@ -918,8 +925,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if ((maxid = virConnectListDomains(ctl->conn, &ids[0], maxid)) < 0) { vshError(ctl, "%s", _("Failed to list active domains")); - VIR_FREE(ids); - return false; + goto cleanup; } qsort(&ids[0], maxid, sizeof(int), idsorter); @@ -929,37 +935,52 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) maxname = virConnectNumOfDefinedDomains(ctl->conn); if (maxname < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); - VIR_FREE(ids); - return false; + goto cleanup; } if (maxname) { names = vshMalloc(ctl, sizeof(char *) * maxname); if ((maxname = virConnectListDefinedDomains(ctl->conn, names, maxname)) < 0) { vshError(ctl, "%s", _("Failed to list inactive domains")); - VIR_FREE(ids); - VIR_FREE(names); - return false; + goto cleanup; } qsort(&names[0], maxname, sizeof(char*), namesorter); } } - vshPrintExtra(ctl, " %-5s %-30s %s\n", _("Id"), _("Name"), _("State")); - vshPrintExtra(ctl, "----------------------------------------------------\n"); + + if (desc) { + vshPrintExtra(ctl, "%-5s %-30s %-10s %s\n", _("Id"), _("Name"), _("State"), _("Title")); + vshPrintExtra(ctl, "-----------------------------------------------------------\n"); + } else { + vshPrintExtra(ctl, " %-5s %-30s %s\n", _("Id"), _("Name"), _("State")); + vshPrintExtra(ctl, "----------------------------------------------------\n"); + } for (i = 0; i < maxid; i++) { - virDomainPtr dom = virDomainLookupByID(ctl->conn, ids[i]); + virDomainPtr dom = virDomainLookupByID(ctl->conn, ids[i]); /* this kind of work with domains is not atomic operation */ if (!dom) continue; - vshPrint(ctl, " %-5d %-30s %s\n", - virDomainGetID(dom), - virDomainGetName(dom), - _(vshDomainStateToString(vshDomainState(ctl, dom, NULL)))); - virDomainFree(dom); + if (desc) { + if (!(title = vshGetDomainDescription(ctl, dom, true, 0))) + goto cleanup; + + vshPrint(ctl, "%-5d %-30s %-10s %s\n", + virDomainGetID(dom), + virDomainGetName(dom), + _(vshDomainStateToString(vshDomainState(ctl, dom, NULL))), + title); + VIR_FREE(title); + } else { + vshPrint(ctl, " %-5d %-30s %s\n", + virDomainGetID(dom), + virDomainGetName(dom), + _(vshDomainStateToString(vshDomainState(ctl, dom, NULL)))); + } + virDomainFree(dom); } for (i = 0; i < maxname; i++) { virDomainPtr dom = virDomainLookupByName(ctl->conn, names[i]); @@ -975,17 +996,179 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virDomainHasManagedSaveImage(dom, 0) > 0) state = -2; - vshPrint(ctl, " %-5s %-30s %s\n", - "-", - names[i], - state == -2 ? _("saved") : _(vshDomainStateToString(state))); + if (desc) { + if (!(title = vshGetDomainDescription(ctl, dom, true, 0))) + goto cleanup; + + vshPrint(ctl, "%-5s %-30s %-10s %s\n", + "-", + names[i], + state == -2 ? _("saved") : _(vshDomainStateToString(state)), + title); + VIR_FREE(title); + } else { + vshPrint(ctl, " %-5s %-30s %s\n", + "-", + names[i], + state == -2 ? _("saved") : _(vshDomainStateToString(state))); virDomainFree(dom); VIR_FREE(names[i]); + } } + + ret = true; +cleanup: VIR_FREE(ids); VIR_FREE(names); - return true; + return ret; +} + +/* + * "desc" command for managing domain description and title + */ +static const vshCmdInfo info_desc[] = { + {"help", N_("show or set domain's description or title")}, + {"desc", N_("Allows to show or modify description or title of a domain.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_desc[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"live", VSH_OT_BOOL, 0, N_("modify/get running state")}, + {"config", VSH_OT_BOOL, 0, N_("modify/get persistent configuration")}, + {"current", VSH_OT_BOOL, 0, N_("modify/get current state configuration")}, + {"title", VSH_OT_BOOL, 0, N_("modify the title instead of description")}, + {"edit", VSH_OT_BOOL, 0, N_("open an editor to modify the description")}, + {"new-desc", VSH_OT_ARGV, 0, N_("message")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + virDomainPtr dom; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + /* current is ignored */ + + bool title = vshCommandOptBool(cmd, "title"); + bool edit = vshCommandOptBool(cmd, "edit"); + + int state; + int type; + char *desc = NULL; + char *desc_edited = NULL; + char *tmp = NULL; + char *tmpstr; + const vshCmdOpt *opt = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool pad = false; + bool ret = false; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if ((state = vshDomainState(ctl, dom, NULL)) < 0) { + ret = false; + goto cleanup; + } + + while ((opt = vshCommandOptArgv(cmd, opt))) { + if (pad) + virBufferAddChar(&buf, ' '); + pad = true; + virBufferAdd(&buf, opt->data, -1); + } + + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (title) + type = VIR_DOMAIN_METADATA_TITLE; + else + type = VIR_DOMAIN_METADATA_DESCRIPTION; + + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to collect new description/title")); + goto cleanup; + } + desc = virBufferContentAndReset(&buf); + + if (edit || desc) { + if (!desc) { + desc = vshGetDomainDescription(ctl, dom, title, + config?VIR_DOMAIN_XML_INACTIVE:0); + if (!desc) + goto cleanup; + } + + if (edit) { + /* Create and open the temporary file. */ + if (!(tmp = editWriteToTempFile(ctl, desc))) + goto cleanup; + + /* Start the editor. */ + if (editFile(ctl, tmp) == -1) + goto cleanup; + + /* Read back the edited file. */ + if (!(desc_edited = editReadBackFile(ctl, tmp))) + goto cleanup; + + /* Compare original XML with edited. Has it changed at all? */ + if (STREQ(desc, desc_edited)) { + vshPrint(ctl, _("Domain description not changed.\n")); + ret = true; + goto cleanup; + } + + VIR_FREE(desc); + desc = desc_edited; + desc_edited = NULL; + } + + /* strip a possible newline at the end of file */ + /* some editors enforce a newline, this makes editing the title + * more convinient */ + if (title && + (tmpstr = strrchr(desc, '\n')) && + *(tmpstr+1) == '\0') + *tmpstr = '\0'; + + if (virDomainSetMetadata(dom, type, desc, NULL, NULL, flags) < 0) { + vshError(ctl, "%s", + _("Failed to set new domain description")); + goto cleanup; + } + vshPrint(ctl, "%s", _("Domain description updated successfuly")); + } else { + desc = vshGetDomainDescription(ctl, dom, title, + config?VIR_DOMAIN_XML_INACTIVE:0); + if (!desc) + goto cleanup; + + if (strlen(desc) > 0) + vshPrint(ctl, "%s", desc); + else + vshPrint(ctl, _("No description for domain: %s"), + virDomainGetName(dom)); + } + + ret = true; +cleanup: + VIR_FREE(desc_edited); + VIR_FREE(desc); + if (tmp) { + unlink(tmp); + VIR_FREE(tmp); + } + return ret; } /* @@ -16245,6 +16428,7 @@ static const vshCmdDef domManagementCmds[] = { {"cpu-compare", cmdCPUCompare, opts_cpu_compare, info_cpu_compare, 0}, {"create", cmdCreate, opts_create, info_create, 0}, {"define", cmdDefine, opts_define, info_define, 0}, + {"desc", cmdDesc, opts_desc, info_desc, 0}, {"destroy", cmdDestroy, opts_destroy, info_destroy, 0}, {"detach-device", cmdDetachDevice, opts_detach_device, info_detach_device, 0}, @@ -18011,6 +18195,65 @@ vshDomainStateReasonToString(int state, int reason) return N_("unknown"); } +/* extract description or title from domain xml */ +static char * +vshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title, + unsigned int flags) +{ + char *desc = NULL; + char *domxml = NULL; + virErrorPtr err = NULL; + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + int type; + + if (title) + type = VIR_DOMAIN_METADATA_TITLE; + else + type = VIR_DOMAIN_METADATA_DESCRIPTION; + + if ((desc = virDomainGetMetadata(dom, type, NULL, flags))) { + return desc; + } else { + err = virGetLastError(); + + if (err && err->code == VIR_ERR_NO_DOMAIN_METADATA) { + desc = vshStrdup(ctl, ""); + virResetLastError(); + return desc; + } + + if (err && err->code != VIR_ERR_NO_SUPPORT) + return desc; + } + + /* fall back to xml */ + /* get domains xml description and extract the title/description */ + if (!(domxml = virDomainGetXMLDesc(dom, flags))) { + vshError(ctl, "%s", _("Failed to retrieve domain XML")); + goto cleanup; + } + doc = virXMLParseStringCtxt(domxml, _("(domain_definition)"), &ctxt); + if (!doc) { + vshError(ctl, "%s", _("Couldn't parse domain XML")); + goto cleanup; + } + if (title) + desc = virXPathString("string(./title[1])", ctxt); + else + desc = virXPathString("string(./description[1])", ctxt); + + if (!desc) + desc = vshStrdup(ctl, ""); + +cleanup: + VIR_FREE(domxml); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); + + return desc; +} + /* Return a non-NULL string representation of a typed parameter; exit * if we are out of memory. */ static char * diff --git a/tools/virsh.pod b/tools/virsh.pod index e9598ac..71dea2c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -279,7 +279,7 @@ The XML also show the NUMA topology information if available. Inject NMI to the guest. -=item B<list> [I<--inactive> | I<--all>] [I<--managed-save>] +=item B<list> [I<--inactive> | I<--all>] [I<--managed-save>] [I<--title>] Prints information about existing domains. If no options are specified it prints out information about running domains. @@ -350,6 +350,15 @@ If I<--managed-save> is specified, then domains that have managed save state (only possible if they are in the B<shut off> state) will instead show as B<saved> in the listing. +If I<--title> is specified, then the domain note is printed. The output then +the output looks as follows. + +B<virsh> list --note + Id Name State Title +----------------------------------------------- + 0 Domain-0 running Mailserver 1 + 2 fedora paused + =item B<freecell> [B<cellno> | I<--all>] Prints the available amount of memory on the machine or within a @@ -426,6 +435,29 @@ Define a domain from an XML <file>. The domain definition is registered but not started. If domain is already running, the changes will take effect on the next boot. +=item B<desc> [I<--live> | I<--config>] [I<--title>] [I<--edit>] + [I<--new-desc> New description or title message] + +Show or modify description and title of a domain. These values are user +fields that allow to store arbitrary textual data to allow easy identifiaction +of domains. Note should be short, although it's not enforced. + +Flags I<--live> or I<--config> select wether this command works on live +or persistent definitions of the domain. By default both are infuenced, while +modifying and running definition is used while reading the note. + +If both I<--live> and I<--config> are specified, the I<--config> option takes +predcedence on getting the current description and both live configuration +and config are updated while setting the description. + +Flag I<--edit> specifies that an editor with the contents of current description +or note should be opened and the contents saved back afterwards. + +Flag I<--title> selects operation on the note field instead of description. + +If neither of I<--edit> and I<--new_desc> are specified the note or description +is displayed instead of being modified. + =item B<destroy> I<domain-id> Immediately terminate the domain domain-id. This doesn't give the domain -- 1.7.3.4

On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds a new command "desc" to show and modify titles and description for the domains using the new API.
This patch also adds a new flag for the "list" command to show titles in the domain list, to allow easy identification of VMs by storing a short description.
Example: virsh # list --title Id Name State Title ----------------------------------------------- 0 Domain-0 running Mailserver 1 2 fedora paused --- tools/virsh.c | 283 +++++++++++++++++++++++++++++++++++++++++++++++++++---- tools/virsh.pod | 34 +++++++- 2 files changed, 296 insertions(+), 21 deletions(-)
+static bool +cmdDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + virDomainPtr dom; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + /* current is ignored */
We could copy some other commands that make sure --current is mutually-exclusive with either --config or --live; but I'm not too worried about that.
+ + bool title = vshCommandOptBool(cmd, "title"); + bool edit = vshCommandOptBool(cmd, "edit"); + + int state; + int type; + char *desc = NULL; + char *desc_edited = NULL; + char *tmp = NULL; + char *tmpstr; + const vshCmdOpt *opt = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool pad = false; + bool ret = false; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if ((state = vshDomainState(ctl, dom, NULL)) < 0) { + ret = false;
Redundant assignment, since ret started false.
+ /* strip a possible newline at the end of file */ + /* some editors enforce a newline, this makes editing the title + * more convinient */
s/convinient/convenient/
+ if (title && + (tmpstr = strrchr(desc, '\n')) && + *(tmpstr+1) == '\0') + *tmpstr = '\0'; + + if (virDomainSetMetadata(dom, type, desc, NULL, NULL, flags) < 0) { + vshError(ctl, "%s", + _("Failed to set new domain description")); + goto cleanup; + } + vshPrint(ctl, "%s", _("Domain description updated successfuly"));
s/successfuly/successfully/
+ } else { + desc = vshGetDomainDescription(ctl, dom, title, + config?VIR_DOMAIN_XML_INACTIVE:0); + if (!desc) + goto cleanup; + + if (strlen(desc) > 0) + vshPrint(ctl, "%s", desc); + else + vshPrint(ctl, _("No description for domain: %s"), + virDomainGetName(dom));
This is, of course, ambiguous output (since I can give my domain the <description>No description for domain: self</description>). Perhaps we should consider returning false when there is no description? But the ambiguity is a corner case, so I'm okay if you don't change anything here.
+ if (err && err->code == VIR_ERR_NO_DOMAIN_METADATA) { + desc = vshStrdup(ctl, ""); + virResetLastError(); + return desc; + }
Nice - makes it slightly easier to use throughout the rest of virsh.
+ + if (err && err->code != VIR_ERR_NO_SUPPORT)
Is the double space intentional?
+ return desc; + } + + /* fall back to xml */ + /* get domains xml description and extract the title/description */
s/domains/domain's/
+=item B<desc> [I<--live> | I<--config>] [I<--title>] [I<--edit>] + [I<--new-desc> New description or title message]
This didn't document --current; then again, since you ignore it on parsing, I don't quite care (but you've been warned if QE files a bug report about inconsistency in the future :)
+ +Show or modify description and title of a domain. These values are user +fields that allow to store arbitrary textual data to allow easy identifiaction
s/identifiaction/identification/
+of domains. Note should be short, although it's not enforced. + +Flags I<--live> or I<--config> select wether this command works on live
s/wether/whether/
+or persistent definitions of the domain. By default both are infuenced, while
s/infuenced/influenced/
+modifying and running definition is used while reading the note. + +If both I<--live> and I<--config> are specified, the I<--config> option takes +predcedence on getting the current description and both live configuration
s/predcedence/precedence/
+and config are updated while setting the description. + +Flag I<--edit> specifies that an editor with the contents of current description +or note should be opened and the contents saved back afterwards. + +Flag I<--title> selects operation on the note field instead of description. + +If neither of I<--edit> and I<--new_desc> are specified the note or description +is displayed instead of being modified.
ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/01/2012 09:49 AM, Eric Blake wrote:
On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds a new command "desc" to show and modify titles and description for the domains using the new API.
+ /* strip a possible newline at the end of file */ + /* some editors enforce a newline, this makes editing the title + * more convinient */
s/convinient/convenient/
+ if (title && + (tmpstr = strrchr(desc, '\n')) && + *(tmpstr+1) == '\0') + *tmpstr = '\0';
Also, this hunk should be moved up to just after editing the file finishes, since otherwise, our shortcut of comparing the editor results with the input string might needlessly differ by the added newline. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Dňa 1.2.2012 23:09, Eric Blake wrote / napísal(a):
On 02/01/2012 09:49 AM, Eric Blake wrote:
On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds a new command "desc" to show and modify titles and description for the domains using the new API.
+ /* strip a possible newline at the end of file */ + /* some editors enforce a newline, this makes editing the title + * more convinient */
s/convinient/convenient/
+ if (title&& + (tmpstr = strrchr(desc, '\n'))&& + *(tmpstr+1) == '\0') + *tmpstr = '\0';
Also, this hunk should be moved up to just after editing the file finishes, since otherwise, our shortcut of comparing the editor results with the input string might needlessly differ by the added newline.
Oh, yeah, you are right. And you managed to point it out literally seconds before pushing :D. Peter

This patch adds support for the new api into the qemu driver to support modification and retireval of domain description and title. This patch does not add support for modifying the <metadata> element. --- src/qemu/qemu_driver.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 174 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 79ad06f..d1104df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11813,6 +11813,178 @@ cleanup: return ret; } +static int +qemuDomainSetMetadata(virDomainPtr dom, + int type, + const char *metadata, + const char *key ATTRIBUTE_UNUSED, + const char *uri ATTRIBUTE_UNUSED, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, 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 (metadata && + !(vm->def->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(vm->def->title); + if (metadata && + !(vm->def->title = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("QEmu driver does not support modifying" + "<metadata> element")); + goto cleanup; + break; + default: + qemuReportError(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 (metadata && + !(persistentDef->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(persistentDef->title); + if (metadata && + !(persistentDef->title = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("QEMU driver does not support" + "<metadata> element")); + goto cleanup; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +no_memory: + virReportOOMError(); + goto cleanup; +} + +static char * +qemuDomainGetMetadata(virDomainPtr dom, + int type, + const char *uri ATTRIBUTE_UNUSED, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr def; + char *ret = NULL; + char *field = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, NULL); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, 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: + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU driver does not support" + "<metadata> element")); + goto cleanup; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type")); + goto cleanup; + break; + } + + if (!field) { + qemuReportError(VIR_ERR_NO_DOMAIN_METADATA, "%s", + _("Requested metadata element is not present")); + goto cleanup; + } + + if (!(ret = strdup(field))) { + virReportOOMError(); + goto cleanup; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -11967,6 +12139,8 @@ static virDriver qemuDriver = { .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ .domainGetDiskErrors = qemuDomainGetDiskErrors, /* 0.9.10 */ + .domainSetMetadata = qemuDomainSetMetadata, /* 0.9.10 */ + .domainGetMetadata = qemuDomainGetMetadata, /* 0.9.10 */ }; -- 1.7.3.4

On 02/01/2012 06:03 AM, Peter Krempa wrote:
This patch adds support for the new api into the qemu driver to support modification and retireval of domain description and title. This patch
s/retireval/retrieval/
does not add support for modifying the <metadata> element.
That's fair, as the primary goal for 0.9.10 is getting the API in place, since even if full use of the API is post-0.9.10, it can be backported by distros where new API cannot be backported. But since we haven't frozen for 0.9.10 quite yet, it may still be worth working up the <metadata> integration for a followup patch, or coordinating with Zeeshan to get that up and running.
--- src/qemu/qemu_driver.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 79ad06f..d1104df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11813,6 +11813,178 @@ cleanup: return ret; }
+static int +qemuDomainSetMetadata(virDomainPtr dom, + int type, + const char *metadata, + const char *key ATTRIBUTE_UNUSED, + const char *uri ATTRIBUTE_UNUSED, + unsigned int flags) +{
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(vm->def->description); + if (metadata && + !(vm->def->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(vm->def->title); + if (metadata && + !(vm->def->title = strdup(metadata))) + goto no_memory; + break;
Looks sane.
+ case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
I'd use VIR_ERR_ARGUMENT_UNSUPPORTED (the call is valid per the documented API, but we don't yet support it).
+ _("QEmu driver does not support modifying" + "<metadata> element")); + goto cleanup; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type"));
Whereas this is a correct error (the value does not match documented API).
+ goto cleanup; + break; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(persistentDef->description); + if (metadata && + !(persistentDef->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(persistentDef->title); + if (metadata && + !(persistentDef->title = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
And again, VIR_ERR_ARGUMENT_UNSUPPORTED.
+static char * +qemuDomainGetMetadata(virDomainPtr dom, + int type, + const char *uri ATTRIBUTE_UNUSED, + unsigned int flags) +{
+ 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: + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU driver does not support" + "<metadata> element")); + goto cleanup; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("unknown metadata type"));
Here, you got it right for the two error types. ACK with those two tweaks. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/01/2012 06:03 AM, Peter Krempa wrote:
This third version incorporates fixes of Eric's review and a rebase on the top of Jirka's new API.
In the interest of sparing you further rebase headaches (and in part because I had already rebased my series on top of yours, and didn't want to do yet another rebase on my end), I've gone ahead and pushed this series with my review comments folded in as fixes. Hopefully, I didn't cause you too many headaches in doing so :) -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Dňa 1.2.2012 23:35, Eric Blake wrote / napísal(a):
On 02/01/2012 06:03 AM, Peter Krempa wrote:
This third version incorporates fixes of Eric's review and a rebase on the top of Jirka's new API.
In the interest of sparing you further rebase headaches (and in part because I had already rebased my series on top of yours, and didn't want to do yet another rebase on my end), I've gone ahead and pushed this series with my review comments folded in as fixes. Hopefully, I didn't cause you too many headaches in doing so :)
Oh, thanks :) (I had one rebase nightmare today in the morning with Jirka's patches :D) I got stuck in trying to find the cause for virsh complaining about leaking references when the daemon hypervisor driver does not yet support the new API. I'll look into that tomorrow. It's pretty late. Peter
participants (3)
-
Daniel Veillard
-
Eric Blake
-
Peter Krempa