[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/4] virsh: Add support for modifying domain description and titles



On 01/27/2012 10:22 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.

Are your plans to have "desc" also modify metadata, or would that be
better suited to a new command, probably named "metadata"?

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

> +/* 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) {

Ah, so we are using the new error (hmm, I should re-read the description
- we even documented it that way).

> +            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, "");

Looks like a reasonable use of the API.

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

Again, careful with the wording.  We may want to s/note/title/ to match
the XML, and be careful in describing the differences between them.

I think this patch is mostly there; but may have more fallout if we make
any more tweaks to the public API or description of it.  I'm okay if we
save <metadata> manipulation from virsh for another day.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]