[libvirt] [PATCH 2/4] API: Add api to set and get domain metadata

[Eric Blake]

On 01/27/2012 10:22 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
> ---
> This is a major rework based on Eric's and Daniels comments.

I agree that we want to settle on the API, and get that in before the
0.9.10 freeze, even if you don't have <metadata> manipulation wired up
by the freeze deadline.

I'm still reluctant on the 40-byte limit; maybe a better formulation
would be:

<description> is free-form text, with no limitations on newlines or length
<title> is free-form, but no newlines permitted, and should be short
(although the length is not enforced)

> 
>  /**
> + * 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
> + * length-limited to 40 bytes. For these two options @key and @uri are
> + * irelevant and can be set to NULL.

s/irelevant/irrelevant/

> + *
> + * For type VIR_DOMAIN_METADATA_ELEMENT @metadata  must be well-formed
> + * XML belonging to namespace defined by @uri with local name @key.

Our tests give this as an example of metadata:

  <metadata>
    <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo>
    <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar>
  </metadata>

So based on your description, it looks like I would call:

virDomainSetMetadata(domain, VIR_DOMAIN_METADATA_ELEMENT,
"<bar maman=\"baz\">barish</bar>", "app2", "http://bar.com/", 0);

and that libvirt then renames the top-level element <bar> into
<KEY:bar>, then adds the xmlns:KEY=URI attribute to that element, so
that it ties the entire XML argument to that namespace?

It's probably worth giving an example of this in the documentation, so
that users know what their incoming @metadata argument must look like.

> +   if (conn->flags & VIR_CONNECT_RO) {
> +        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> +        goto error;
> +    }

It might be nice to guarantee that if type is DESCRIPTION or TITLE, then
key and uri are NULL; while if type is ELEMENT, then key and uri are not
NULL.

> +/**
> + * 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.
> + *
> + * 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=%p, flags=%x",
> +                     type, uri, flags);
> +
> +    if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> +        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> +        goto error;
> +    }
> +

Probably worth checking that uri is either NULL or not NULL, as required
by type.

Check that flags does not contain both AFFECT_LIVE and AFFECT_CONFIG at
once.

> +++ b/src/libvirt_public.syms
> @@ -520,6 +520,8 @@ LIBVIRT_0.9.10 {
>      global:
>          virDomainShutdownFlags;
>          virStorageVolWipePattern;
> +        virDomainSetMetadata;
> +        virDomainGetMetadata;

I like to keep this sorted.  And you'll have to rebase, since another
API went in today.

> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 85eec8d..5bcfe79 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -1220,6 +1220,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;

Do we use this new error yet?  Maybe it's not worth introducing it until
we have a use for it.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120127/7ed4e964/attachment-0001.sig>