2:31 p.m.
On 01/13/2012 11:17 AM, Peter Krempa wrote:
This patch adds a new command "desc" to show and modify
notes and
description for the domains using the new API.
This patch also adds a new flag for the "list" command to show notes in
the domain list, to allow easy identification of VMs by storing a short
description.
Example:
virsh # list --note
Id Name State Note
-----------------------------------------------
0 Domain-0 running Mailserver 1
2 fedora paused
---
tools/virsh.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
tools/virsh.pod | 30 +++++++-
2 files changed, 255 insertions(+), 21 deletions(-)
Looks like a great start!
+/*
+ * "desc" command for managing domain description and note
+ */
+static const vshCmdInfo info_desc[] = {
+ {"help", N_("show or set domain's description or note")},
+ {"desc", N_("Allows to show or modify description or note of a
domain.")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_desc[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
+ {"persistent", VSH_OT_BOOL, 0, N_("modify persistent state of
domain")},
+ {"live", VSH_OT_BOOL, 0, N_("modify note only for current
instance")},
Should we prefer the names --live/--config/--current, rather than just
--persistent/--live, for consistency with other recently added commands?
+ {"note", VSH_OT_BOOL, 0, N_("modify the note
instead of description")},
+ {"edit", VSH_OT_BOOL, 0, N_("open an editor to modify the
description")},
+ {"new_desc", VSH_OT_ARGV, 0, N_("message")},
Underscores are pain. I'd name this new-desc.
I like the slick trick to make quoting optional, where I can use either:
virsh desc domain --new_desc='my description'
virsh desc domain my description
+ if (live)
+ flags |= VIR_DOMAIN_DESCRIPTION_LIVE;
+ if (inactive)
+ flags |= VIR_DOMAIN_DESCRIPTION_CONFIG;
+ if (!(inactive || live)) {
+ flags |= VIR_DOMAIN_DESCRIPTION_CONFIG;
+ if (state == VIR_DOMAIN_RUNNING || state == VIR_DOMAIN_PAUSED)
+ flags |= VIR_DOMAIN_DESCRIPTION_LIVE;
+ }
This last step isn't needed; the drivers already convert _CURRENT into
the correct version, so virsh doesn't have to repeat that work.
+
+ if (virBufferError(&buf)) {
+ vshPrint(ctl, "%s", _("Failed to collect new
description/note"));
+ goto cleanup;
+ }
+ desc = virBufferContentAndReset(&buf);
+
+ if (edit || desc) {
+ if (!desc) {
+ desc = vshGetDomainDescription(ctl, dom, note,
+ inactive?VIR_DOMAIN_XML_INACTIVE:0);
+ if (!desc)
+ goto cleanup;
+ }
This ignored --live; more on that below.
+
+ if (edit) {
+ /* Create and open the temporary file. */
+ tmp = editWriteToTempFile (ctl, desc);
Format - no space before function call (. Several instances in this
function, and yes, I know it is from copy-and-paste.
+
+ if (virDomainSetDescription(dom, desc, flags) < 0) {
+ vshError(ctl, "%s",
+ _("Failed to set new domain description"));
+ goto cleanup;
+ }
+ } else {
+ desc = vshGetDomainDescription(ctl, dom, note,
+ inactive?VIR_DOMAIN_XML_INACTIVE:0);
If the user did 'virsh desc domain --live --persistent', then you
silently ignored --live; I'd almost rather have virsh error out (that
is, --live and --persistent together make sense only if the user is
providing a description - they don't even work together with --edit,
since it is not obvious whether the editor should start from the live or
from the config version).
+ if (desc)
+ vshPrint(ctl, "%s", desc);
+ else
+ vshPrint(ctl, _("No description for domain: %s"),
+ virDomainGetName(dom));
goto cleanup - this should be treated as an error condition (given that
vshGetDomainDescription returns "", rather than NULL, if it successfully
determined that the domain has no description).
@@ -15951,6 +16118,7 @@ static const vshCmdDef domManagementCmds[] =
{
{"migrate-getspeed", cmdMigrateGetMaxSpeed,
opts_migrate_getspeed, info_migrate_getspeed, 0},
{"numatune", cmdNumatune, opts_numatune, info_numatune, 0},
+ {"desc", cmdDesc, opts_desc, info_desc, 0},
Sort this line earlier.
+/* extract note from domain xml */
+static char *
+vshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool note,
+ unsigned int flags)
+{
+ char *desc = NULL;
+ char *domxml = NULL;
+ xmlDocPtr doc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
Here's where a getter function might make life easier (but you should
still fall back to GetXMLDesc in case you are talking to older servers).
+
+ /* get domains xml description and extract the note */
+ 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 (note)
+ desc = virXPathString("string(./description[1]/@note)", ctxt);
+ else
+ desc = virXPathString("string(./description[1])", ctxt);
If note is not present, but description is, should we fall back to a
truncated version of description rather than nothing at all?
@@ -426,6 +435,25 @@ 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<--persistent>] [I<--note>]
[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.
more on the characters vs. bytes distinction
+
+Flags I<--live> or I<--persistent> 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.
I'm not quite sure that matches the code. That is, the code has to pass
AFFECT_LIVE|AFFECT_CONFIG, and not 0, to affect both persistent and
running setting. And if you switch to --live/--config/--current, that
changes this paragraph even more.
+
+Flag I<--edit> specifies that an editor with the contents of current description
+or note should be opened and the contents save back afterwards.
s/save/saved/
+
+Flag I<--note> 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.
+
Overall, it sounds like a nice virsh addition. It will need some polish
for v2, but shouldn't be too hard.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org