[libvirt] [PATCH v2 0/8] Add ability to store notes with domains
I've reworked this patches to use a separate element for storing the short note. This v2 also contains som new patches, especially added support for the LXC driver, and optionaly API to get the description. (See patches marked as optional). These are not required, just add a helper api to get the description and it's usage in virsh. v2 patches: xml: Add element <title> to allow short description of domains api: Add api to set domain title and description in runtime qemu: Implement note API in qemu driver virsh: Add support for modifying domain description and titles v1 patches: lxc: implement description API in LXC driver. api: Add API to get domain title and description (optional) lxc, qemu: Add support for virDomainGetDescription (optional) virsh: Add support for virDomainGetDescription api (optional) docs/formatdomain.html.in | 6 + docs/schemas/domaincommon.rng | 13 +- include/libvirt/libvirt.h.in | 13 + src/conf/domain_conf.c | 19 ++ src/conf/domain_conf.h | 1 + src/driver.h | 9 + src/libvirt.c | 92 +++++++ src/libvirt_public.syms | 6 + src/lxc/lxc_driver.c | 139 ++++++++++ src/qemu/qemu_driver.c | 139 ++++++++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 19 ++- .../qemu-simple-description-title.xml | 27 ++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 + tools/virsh.c | 268 ++++++++++++++++++-- tools/virsh.pod | 34 +++- 16 files changed, 769 insertions(+), 23 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 is limited to 40 bytes and can't contain newlines. *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 --- Changes to v1: - use a new element <title> in the XML instead of attribute to domain - Tweaked commit message - Tweaked docs - Added restrictions into the RNG grammar for the new element - Added tests! (hopefuly right) docs/formatdomain.html.in | 6 ++++ docs/schemas/domaincommon.rng | 13 +++++++++- src/conf/domain_conf.c | 19 ++++++++++++++ src/conf/domain_conf.h | 1 + .../qemu-simple-description-title.xml | 27 ++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml | 5 +++ 6 files changed, 70 insertions(+), 1 deletions(-) create mode 100644 tests/domainschemadata/qemu-simple-description-title.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de9b480..2945868 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -33,6 +33,7 @@ <name>fv0</name> <uuid>4dea22b31d52d8f32516782e98ab3fa0</uuid> <description>Some human readable description</description> + <title>A short description - title - of the domain</title> ...</pre> <dl> @@ -59,6 +60,11 @@ human readable description of the virtual machine. This data is not used by libvirt in any way, it can contain any information the user wants. <span class="since">Since 0.7.2</span></dd> + + <dt><code>title</code></dt> + <dd>The optional element <code>title</code> provides space for a + shorter description, capped at 40 bytes and with no newline, + <span class="since">since 0.9.10</span>.</dd> </dl> <h3><a name="elementsOS">Operating system booting</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2041dfb..bd40e42 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,14 @@ </element> </define> + <define name="title"> + <element name="title"> + <data type="string"> + <param name='pattern'>[^\n]{0,40}</param> + </data> + </element> + </define> + <!-- We handle only document defining a domain --> @@ -26,6 +34,9 @@ <ref name="description"/> </optional> <optional> + <ref name="title"/> + </optional> + <optional> <ref name="cpu"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f97014e..c04a4c0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1478,6 +1478,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); @@ -7090,6 +7091,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(tmp); } + /* Extract short description of domain (title) */ + def->title = virXPathString("string(./title[1])", ctxt); + if (def->title) { + if (strchr(def->title, '\n')) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("Domain title can't contain newlines")); + goto error; + } + + if (strlen(def->title) > 40) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("Domain title too long")); + goto error; + } + } + /* Extract documentation if present */ def->description = virXPathString("string(./description[1])", ctxt); @@ -11420,6 +11437,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferEscapeString(buf, " <description>%s</description>\n", def->description); + virBufferEscapeString(buf, " <title>%s</title>\n", def->title); + virBufferAsprintf(buf, " <memory>%lu</memory>\n", def->mem.max_balloon); virBufferAsprintf(buf, " <currentMemory>%lu</currentMemory>\n", def->mem.cur_balloon); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b121f9c..cbaa2df 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1420,6 +1420,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..6cb0b31 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> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> + <title>A description of the test machine.</title> <memory>219100</memory> <currentMemory>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu> -- 1.7.3.4
The <description> and <title> elements are accessible only through the XML, that makes it hard to work with. Atomic operations and modification of the live domain are impossible. This patch adds a function to the public API to set arbitrary descriptions for live and persistent domains. *include/libvirt/libvirt.h.in - add api function virDomainSetDescription - add flags for this new api *src/driver.h *src/libvirt.c - add driver support - implement the public api *src/libvirt_public.syms - export the new function *src/remote/remote_driver.c *src/remote/remote_protocol.x - wire up the remote protocol --- Changes to v1: - tweaked spellings - left out "renaming" of flags. Now there's just a reference to VIR_DOMAIN_AFFECT_* - reject setting flags on read-only connections include/libvirt/libvirt.h.in | 10 ++++++++ src/driver.h | 5 ++++ src/libvirt.c | 52 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 ++++++- 6 files changed, 81 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e436f3c..40e0032 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1405,6 +1405,16 @@ int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +typedef enum { + /* bits 0 and 1 are reserverd for @virDomainModificationImpact */ + VIR_DOMAIN_DESCRIPTION_TITLE = (1 << 2), /* Operate on title instead of + the domain's description */ +} virDomainDescriptionFlags; + +int virDomainSetDescription(virDomainPtr domain, + const char *description, + unsigned int flags); + /* * XML domain description */ diff --git a/src/driver.h b/src/driver.h index 24636a4..afb8f3c 100644 --- a/src/driver.h +++ b/src/driver.h @@ -793,6 +793,10 @@ typedef int virTypedParameterPtr params, int *nparams, unsigned int flags); +typedef int + (*virDrvDomainSetDescription)(virDomainPtr dom, + const char *description, + unsigned int flags); /** * _virDriver: @@ -962,6 +966,7 @@ struct _virDriver { virDrvNodeSuspendForDuration nodeSuspendForDuration; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; + virDrvDomainSetDescription domainSetDescription; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 7b8adf7..177d52d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8746,6 +8746,58 @@ error: } /** + * virDomainSetDescription: + * @domain: a domain object + * @description: new description text + * @flags: bitwise-OR of virDomainDescriptionFlags + * + * Sets the domain description field or note field depending on the flags + * parameter. + * + * Returns 0 on success, -1 in case of failure; + */ +int +virDomainSetDescription(virDomainPtr domain, + const char *description, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "description=%p, flags=%x", description, flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + if (description == NULL) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainSetDescription) { + int ret; + ret = conn->driver->domainSetDescription(domain, description, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virNodeGetSecurityModel: * @conn: a connection object * @secmodel: pointer to a virSecurityModel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..882b746 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virDomainSetDescription; +} LIBVIRT_0.9.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..4d64bc9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4750,6 +4750,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainSetDescription = remoteDomainSetDescription, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ca739ff..ad3e12f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1095,6 +1095,12 @@ struct remote_domain_set_autostart_args { int autostart; }; +struct remote_domain_set_description_args { + remote_nonnull_domain dom; + remote_nonnull_string description; + unsigned int flags; +}; + struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2653,7 +2659,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_DESCRIPTION = 258 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3.4
This patch adds support for the new api to change domain descriptions to the qemu driver. --- Diff to v1: - use consistent goto labels - simplified error path - add flush of config to disk after it's changed - fixed flag checking logic operators src/qemu/qemu_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 70 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c920bfd..10dd98d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11857,6 +11857,75 @@ cleanup: return ret; } +static int +qemuDomainSetDescription(virDomainPtr dom, const char *description, + 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 | + VIR_DOMAIN_DESCRIPTION_TITLE, -1); + + bool title = (flags & VIR_DOMAIN_DESCRIPTION_TITLE) > 0; + + 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) { + if (title) { + VIR_FREE(vm->def->title); + if (!(vm->def->title = strdup(description))) + goto no_memory; + } else { + VIR_FREE(vm->def->description); + if (!(vm->def->description = strdup(description))) + goto no_memory; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (title) { + VIR_FREE(persistentDef->title); + if (!(persistentDef->title = strdup(description))) + goto no_memory; + } else { + VIR_FREE(persistentDef->description); + if (!(persistentDef->description = strdup(description))) + goto no_memory; + } + + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +no_memory: + virReportOOMError(); + goto cleanup; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -12009,6 +12078,7 @@ static virDriver qemuDriver = { .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ + .domainSetDescription = qemuDomainSetDescription, /* 0.9.10 */ }; -- 1.7.3.4
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 --- Diff to v1: - change flag naming to consist with new scheme/api flag names - use dashes instead of underscores in command flags - remove unneeded flag checks - document behavior if both --live and --config are specified - enter error paths if helper function returns NULL - fix inapropriate sorting of function registration (I renamed it quite a few times now) - tweak spelling in man page tools/virsh.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++----- tools/virsh.pod | 34 +++++++- 2 files changed, 261 insertions(+), 21 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c511e2a..976c2d9 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -312,6 +312,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) @@ -885,6 +888,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} }; @@ -899,7 +903,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; @@ -917,8 +924,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); @@ -928,37 +934,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]); @@ -974,17 +995,167 @@ 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; + char *desc = NULL; + char *desc_edited = NULL; + char *tmp = NULL; + 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) + flags |= VIR_DOMAIN_DESCRIPTION_TITLE; + + 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; + } + + if (virDomainSetDescription(dom, desc, 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; } /* @@ -15917,6 +16088,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}, @@ -17672,6 +17844,42 @@ 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; + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + + /* 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 c88395b..190a5b9 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 note message] + +Show or modify description and note for a domain. These values are user +fields that allow to store arbitrary textual data to allow easy identifiaction +of domains. Note is a short (maximum 40 characters) field. + +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
This patch adds support for the new api to change domain descriptions to the lxc driver. --- src/lxc/lxc_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 70 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3baff19..d26bfe9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3802,6 +3802,75 @@ cleanup: } static int +lxcDomainSetDescription(virDomainPtr dom, const char *description, + unsigned int flags) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_DESCRIPTION_TITLE, -1); + + bool title = (flags & VIR_DOMAIN_DESCRIPTION_TITLE) > 0; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + lxcDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(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) { + if (title) { + VIR_FREE(vm->def->title); + if (!(vm->def->title = strdup(description))) + goto no_memory; + } else { + VIR_FREE(vm->def->description); + if (!(vm->def->description = strdup(description))) + goto no_memory; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (title) { + VIR_FREE(persistentDef->title); + if (!(persistentDef->title = strdup(description))) + goto no_memory; + } else { + VIR_FREE(persistentDef->description); + if (!(persistentDef->description = strdup(description))) + goto no_memory; + } + + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +no_memory: + virReportOOMError(); + goto cleanup; +} + +static int lxcVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, virHashIterator iter, void *data) { @@ -3891,6 +3960,7 @@ static virDriver lxcDriver = { .domainOpenConsole = lxcDomainOpenConsole, /* 0.8.6 */ .isAlive = lxcIsAlive, /* 0.9.8 */ .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ + .domainSetDescription = lxcDomainSetDescription, /* 0.9.10 */ }; static virStateDriver lxcStateDriver = { -- 1.7.3.4
This patch add a helper API as a counterpart to virDomainSetDescription to allow easy getting of the <description> and <title> fields from domain's config. --- include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 4 ++++ src/libvirt.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- 6 files changed, 60 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 40e0032..04a2050 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1415,6 +1415,9 @@ int virDomainSetDescription(virDomainPtr domain, const char *description, unsigned int flags); +char *virDomainGetDescription(virDomainPtr domain, + unsigned int flags); + /* * XML domain description */ diff --git a/src/driver.h b/src/driver.h index afb8f3c..59718cb 100644 --- a/src/driver.h +++ b/src/driver.h @@ -797,6 +797,9 @@ typedef int (*virDrvDomainSetDescription)(virDomainPtr dom, const char *description, unsigned int flags); +typedef char * + (*virDrvDomainGetDescription)(virDomainPtr dom, + unsigned int flags); /** * _virDriver: @@ -967,6 +970,7 @@ struct _virDriver { virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; virDrvDomainSetDescription domainSetDescription; + virDrvDomainGetDescription domainGetDescription; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 177d52d..99514b7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8798,6 +8798,46 @@ error: } /** + * virDomainGetDescription: + * @domain: a domain object + * @flags: bitwise-OR of virDomainDescriptionFlags + * + * Gets the domain description field or title field depending on the flags + * parameter. + * + * Returns NULL on error or the requested description field on success. The + * caller must free the returned pointer. + */ +char * +virDomainGetDescription(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetDescription) { + char *ret; + if (!(ret = conn->driver->domainGetDescription(domain, 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 882b746..62d19a2 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -519,6 +519,7 @@ LIBVIRT_0.9.9 { LIBVIRT_0.9.10 { global: virDomainSetDescription; + virDomainGetDescription; } LIBVIRT_0.9.9; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 4d64bc9..c3ff1c7 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4751,6 +4751,7 @@ static virDriver remote_driver = { .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ .domainSetDescription = remoteDomainSetDescription, /* 0.9.10 */ + .domainGetDescription = remoteDomainGetDescription, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ad3e12f..c2fe40a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1101,6 +1101,15 @@ struct remote_domain_set_description_args { unsigned int flags; }; +struct remote_domain_get_description_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_description_ret { + remote_string xml; +}; + struct remote_domain_block_job_abort_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2660,7 +2669,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_SET_DESCRIPTION = 258 /* autogen autogen */ + REMOTE_PROC_DOMAIN_SET_DESCRIPTION = 258, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_DESCRIPTION = 259 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3.4
This patch adds support for the newly added api for the LXC and qemu drivers. --- src/lxc/lxc_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d26bfe9..1df7b45 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3870,6 +3870,74 @@ no_memory: goto cleanup; } +static char * +lxcDomainGetDescription(virDomainPtr dom, unsigned int flags) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef; + char *ret = NULL; + char *field; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_DESCRIPTION_TITLE, NULL); + + bool title = (flags & VIR_DOMAIN_DESCRIPTION_TITLE) > 0; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + lxcDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE && + flags & VIR_DOMAIN_AFFECT_CONFIG) { + lxcError(VIR_ERR_INVALID_ARG, + _("Can't specify both LIVE and CONFIG flags")); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (title) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) + field = vm->def->title; + else + field = persistentDef->title; + } else { + /* description */ + if (flags & VIR_DOMAIN_AFFECT_LIVE) + field = vm->def->description; + else + field = persistentDef->description; + } + + if (!field) { + lxcError(VIR_ERR_OPERATION_FAILED, + _("No title or description found")); + goto cleanup; + } + + if (!(ret = strdup(field))) { + virReportOOMError(); + goto cleanup; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static int lxcVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, virHashIterator iter, void *data) @@ -3961,6 +4029,7 @@ static virDriver lxcDriver = { .isAlive = lxcIsAlive, /* 0.9.8 */ .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ .domainSetDescription = lxcDomainSetDescription, /* 0.9.10 */ + .domainGetDescription = lxcDomainGetDescription, /* 0.9.10 */ }; static virStateDriver lxcStateDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10dd98d..e4082df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11926,6 +11926,74 @@ no_memory: goto cleanup; } +static char * +qemuDomainGetDescription(virDomainPtr dom, unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDefPtr persistentDef; + char *ret = NULL; + char *field=NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_DESCRIPTION_TITLE, NULL); + + bool title = (flags & VIR_DOMAIN_DESCRIPTION_TITLE) > 0; + + 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 (flags & VIR_DOMAIN_AFFECT_LIVE && + flags & VIR_DOMAIN_AFFECT_CONFIG) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Can't specify both LIVE and CONFIG flags")); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (title) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) + field = vm->def->title; + else + field = persistentDef->title; + } else { + /* description */ + if (flags & VIR_DOMAIN_AFFECT_LIVE) + field = vm->def->description; + else + field = persistentDef->description; + } + + if (!field) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("No title or description found")); + 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", @@ -12079,6 +12147,7 @@ static virDriver qemuDriver = { .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ .domainSetDescription = qemuDomainSetDescription, /* 0.9.10 */ + .domainGetDescription = qemuDomainGetDescription, /* 0.9.10 */ }; -- 1.7.3.4
This patch modifies the description parser to use the new API if available with option to fall back to XML parsing. --- tools/virsh.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 976c2d9..08bfb27 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -17851,9 +17851,29 @@ vshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title, { char *desc = NULL; char *domxml = NULL; + virErrorPtr err = NULL; xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; + int apiflags = flags; + if (title) + apiflags |= VIR_DOMAIN_DESCRIPTION_TITLE; + + if ((desc = virDomainGetDescription(dom, apiflags))) { + return desc; + } else { + err = virGetLastError(); + + if (err && err->code == VIR_ERR_OPERATION_FAILED) { + desc = vshStrdup(ctl, ""); + 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")); -- 1.7.3.4
On 01/18/2012 03:23 PM, Peter Krempa wrote:
I've reworked this patches to use a separate element for storing the short note. This v2 also contains som new patches, especially added support for the LXC driver, and optionaly API to get the description. (See patches marked as optional). These are not required, just add a helper api to get the description and it's usage in virsh.
I noticed a discussion [1] about adding metadata to domains definition and methods of accessing it. As these patches can also be considered to deal with metadata and we probably shouldn't add a ton of API's for accessing them we could as well as add an API to support various formats of metadata. What are your opinions on this? Should we add such a general function (virDomain[Get|Set]Metadata)? Is it a good idea to use the virTypedParam to do this? Is the string support on virTypedParam complete? Thanks Peter P.S.: I'm self-NACKing this patchset until this is settled. [1] http://www.redhat.com/archives/libvir-list/2012-January/msg00833.html
On 01/23/2012 08:50 AM, Peter Krempa wrote:
On 01/18/2012 03:23 PM, Peter Krempa wrote:
I've reworked this patches to use a separate element for storing the short note. This v2 also contains som new patches, especially added support for the LXC driver, and optionaly API to get the description. (See patches marked as optional). These are not required, just add a helper api to get the description and it's usage in virsh.
I noticed a discussion [1] about adding metadata to domains definition and methods of accessing it.
As these patches can also be considered to deal with metadata and we probably shouldn't add a ton of API's for accessing them we could as well as add an API to support various formats of metadata.
What are your opinions on this? Should we add such a general function (virDomain[Get|Set]Metadata)? Is it a good idea to use the virTypedParam to do this? Is the string support on virTypedParam complete?
Changing <title> or <description> of a transient domain is a nice-to-have, but not the end of the world. Changing <metadata> of a transient domain is a must-have, so we need at least one new API. Setting is important, while getting is only a shortcut (we can make the user call virDomainGetXMLDesc, and do an XPath query), but symmetry is nice. Meanwhile, libvirt shouldn't care about the contents of <metadata>, other than that it is a well-formed XML string. We do _not_ need virTypedParams, but can stick with just an enum for which piece of metadata to be modifying. We can get by with just one API for all three elements, as well as leaving the door open for any future metadata. So I'm starting to think: new error type VIR_ERR_NO_DOMAIN_METADATA = 79, /* Metadata element not present */ 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; /** * virDomainSetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType * @description: new description text * @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 * length-limited to 40 bytes, and VIR_DOMAIN_METADATA_ELEMENT must * be well-formed XML but is otherwise uninterpreted by libvirt. * Passing NULL for @description 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 *description, unsigned int flags); /** * virDomainGetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType * @flags: bitwise-OR of virDomainModificationImpact * * Retrieves the appropriate domain element given by @type. * 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, unsigned int flags); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Jan 23, 2012 at 09:17:51AM -0700, Eric Blake wrote:
On 01/23/2012 08:50 AM, Peter Krempa wrote:
On 01/18/2012 03:23 PM, Peter Krempa wrote:
I've reworked this patches to use a separate element for storing the short note. This v2 also contains som new patches, especially added support for the LXC driver, and optionaly API to get the description. (See patches marked as optional). These are not required, just add a helper api to get the description and it's usage in virsh.
I noticed a discussion [1] about adding metadata to domains definition and methods of accessing it.
As these patches can also be considered to deal with metadata and we probably shouldn't add a ton of API's for accessing them we could as well as add an API to support various formats of metadata.
What are your opinions on this? Should we add such a general function (virDomain[Get|Set]Metadata)? Is it a good idea to use the virTypedParam to do this? Is the string support on virTypedParam complete?
Changing <title> or <description> of a transient domain is a nice-to-have, but not the end of the world. Changing <metadata> of a transient domain is a must-have, so we need at least one new API. Setting is important, while getting is only a shortcut (we can make the user call virDomainGetXMLDesc, and do an XPath query), but symmetry is nice. Meanwhile, libvirt shouldn't care about the contents of <metadata>, other than that it is a well-formed XML string. We do _not_ need virTypedParams, but can stick with just an enum for which piece of metadata to be modifying. We can get by with just one API for all three elements, as well as leaving the door open for any future metadata. So
For the <metadata> element we want more tha njust a well-formed XML string. The intention is that every top level element inside <metadata> *must* declare its own private XML namespace. The default namespace is to be reserved for any libvirt official metadata elements we might introduce in the future.
I'm starting to think:
new error type VIR_ERR_NO_DOMAIN_METADATA = 79, /* Metadata element not present */
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;
/** * virDomainSetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType * @description: new description text * @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 * length-limited to 40 bytes, and VIR_DOMAIN_METADATA_ELEMENT must * be well-formed XML but is otherwise uninterpreted by libvirt. * Passing NULL for @description 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 *description, unsigned int flags);
/** * virDomainGetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType * @flags: bitwise-OR of virDomainModificationImpact * * Retrieves the appropriate domain element given by @type. * 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, unsigned int flags);
While fine for title/description, I don't think this really works for <metadata>. When setting the metadata we'd want to specify an XML namespace key and URI, and when getting the metadata we'd really want to specify a namespace URI Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/23/2012 09:23 AM, Daniel P. Berrange wrote:
Changing <title> or <description> of a transient domain is a nice-to-have, but not the end of the world. Changing <metadata> of a transient domain is a must-have, so we need at least one new API. Setting is important, while getting is only a shortcut (we can make the user call virDomainGetXMLDesc, and do an XPath query), but symmetry is nice. Meanwhile, libvirt shouldn't care about the contents of <metadata>, other than that it is a well-formed XML string. We do _not_ need virTypedParams, but can stick with just an enum for which piece of metadata to be modifying. We can get by with just one API for all three elements, as well as leaving the door open for any future metadata. So
For the <metadata> element we want more tha njust a well-formed XML string. The intention is that every top level element inside <metadata> *must* declare its own private XML namespace. The default namespace is to be reserved for any libvirt official metadata elements we might introduce in the future.
Do we want to start out with <description> or even <title> as our first libvirt-official metadata (that is, in the spirit of backcompat, output: <domain ...> <description>desc</description> <metadata> <description>desc</description> </metadata> </domain>
While fine for title/description, I don't think this really works for <metadata>. When setting the metadata we'd want to specify an XML namespace key and URI, and when getting the metadata we'd really want to specify a namespace URI
Ah, so you're thinking of getting at an individual sub-element within <metadata>, where I was thinking of grabbing the entire <metadata> element and making the user sift through the sub-elements to the ones they want. Your proposal does indeed make a bit more sense, in saving the user some effort for filtering to just the metadata elements they care about, and when setting things, they can set just the ones they care about while leaving all other existing metadata elements untouched (instead of having to do a read-modify-write cycle). Your proposal also makes it easier to force the user to specify a namespace for each element that goes into metadata. But it also makes it sound like we are imposing one additional constraint on <metadata> - each subelement must be a distinct namespace, so you cannot have multiple top-level metadata elements from the same namespace. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Jan 23, 2012 at 09:46:57AM -0700, Eric Blake wrote:
On 01/23/2012 09:23 AM, Daniel P. Berrange wrote:
Changing <title> or <description> of a transient domain is a nice-to-have, but not the end of the world. Changing <metadata> of a transient domain is a must-have, so we need at least one new API. Setting is important, while getting is only a shortcut (we can make the user call virDomainGetXMLDesc, and do an XPath query), but symmetry is nice. Meanwhile, libvirt shouldn't care about the contents of <metadata>, other than that it is a well-formed XML string. We do _not_ need virTypedParams, but can stick with just an enum for which piece of metadata to be modifying. We can get by with just one API for all three elements, as well as leaving the door open for any future metadata. So
For the <metadata> element we want more tha njust a well-formed XML string. The intention is that every top level element inside <metadata> *must* declare its own private XML namespace. The default namespace is to be reserved for any libvirt official metadata elements we might introduce in the future.
Do we want to start out with <description> or even <title> as our first libvirt-official metadata (that is, in the spirit of backcompat, output:
<domain ...> <description>desc</description> <metadata> <description>desc</description> </metadata> </domain>
Nah, I feel that's just overkill - it is harmless having description and title outside the block IMHO.
While fine for title/description, I don't think this really works for <metadata>. When setting the metadata we'd want to specify an XML namespace key and URI, and when getting the metadata we'd really want to specify a namespace URI
Ah, so you're thinking of getting at an individual sub-element within <metadata>, where I was thinking of grabbing the entire <metadata> element and making the user sift through the sub-elements to the ones they want. Your proposal does indeed make a bit more sense, in saving the user some effort for filtering to just the metadata elements they care about, and when setting things, they can set just the ones they care about while leaving all other existing metadata elements untouched (instead of having to do a read-modify-write cycle). Your proposal also makes it easier to force the user to specify a namespace for each element that goes into metadata. But it also makes it sound like we are imposing one additional constraint on <metadata> - each subelement must be a distinct namespace, so you cannot have multiple top-level metadata elements from the same namespace.
That is correct - basically the top level element would be a container inside which all the app's metadata would live, so I'd expect apps todo <metadata> <virtman:guest xmlns:virtman="http://virt-manager.org/guets/1.0"> <ostype>linux</ostype> <osvariant>fedora16</osvariant> </virtman:guest> </metadata> And *not* <metadata> <virtman:ostype xmlns:virtman="http://virt-manager.org/guets/1.0">linux</virtman:ostype> <virtman:osvariant xmlns:virtman="http://virt-manager.org/guets/1.0">fedora16</virtman:osvariant> </metadata> Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Peter Krempa