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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list