[libvirt] [PATCH 4/4] qemu: Add support for virDomainGetMetadata and virDomainSetMetadata

[Eric Blake]

On 01/27/2012 10:22 AM, Peter Krempa wrote:
> This patch adds support for the new api into the qemu driver to support
> modification and retireval of domain description and title. This patch
> does not add support for modifying the <metadata> element.
> ---

> +
> +    /* validate title */
> +    if (type == VIR_DOMAIN_METADATA_TITLE && metadata) {
> +        if (strchr(metadata, '\n')) {
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                            _("Domain title can't contain newlines"));
> +            goto cleanup;
> +        }
> +        if (strlen(metadata) > VIR_DOMAIN_TITLE_LENGTH) {
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                             _("Domain title too long"));
> +            goto cleanup;
> +        }
> +    }

Since title is relatively new, we may want to do this validation in both
domain_conf (when parsing XML) and in libvirt.c (when calling
virDomainSetMetadata), so that hypervisors don't have to repeat this code.

> +        case VIR_DOMAIN_METADATA_ELEMENT:
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                            _("QEmu driver does not support modifying"
> +                              "<metadata> element"));

I'd use VIR_ERR_ARGUMENT_UNSUPPORTED here.  All of the arguments were
valid, we just don't know how to act on them.

> +static char *
> +qemuDomainGetMetadata(virDomainPtr dom,
> +                      int type,
> +                      const char *uri ATTRIBUTE_UNUSED,
> +                      unsigned int flags)
> +{
> +    struct qemud_driver *driver = dom->conn->privateData;

> +
> +    if (flags &
> +        (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) {
> +        qemuReportError(VIR_ERR_INVALID_ARG,
> +                        _("Can't specify both LIVE and CONFIG flags"));
> +        goto cleanup;
> +    }

Drop this check.  It should live in libvirt.c.  And as written, you
didn't do the check you wanted - you are rejecting any use of LIVE or
CONFIG, rather than the intended simultaneous use of both flags.

Looks like a good implementation for 2 of the 3 fields, and again, I'm
okay if we don't get <metadata> implemented until a later patch, as long
as we're happy that the API will let us do it right.

-- 
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/f092cfa5/attachment-0001.sig>