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

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



On 02/01/2012 06:03 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

s/retireval/retrieval/

> does not add support for modifying the <metadata> element.

That's fair, as the primary goal for 0.9.10 is getting the API in place,
since even if full use of the API is post-0.9.10, it can be backported
by distros where new API cannot be backported.  But since we haven't
frozen for 0.9.10 quite yet, it may still be worth working up the
<metadata> integration for a followup patch, or coordinating with
Zeeshan to get that up and running.

> ---
>  src/qemu/qemu_driver.c |  174 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 174 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 79ad06f..d1104df 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11813,6 +11813,178 @@ cleanup:
>      return ret;
>  }
> 
> +static int
> +qemuDomainSetMetadata(virDomainPtr dom,
> +                      int type,
> +                      const char *metadata,
> +                      const char *key ATTRIBUTE_UNUSED,
> +                      const char *uri ATTRIBUTE_UNUSED,
> +                      unsigned int flags)
> +{

> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        switch ((virDomainMetadataType) type) {
> +        case VIR_DOMAIN_METADATA_DESCRIPTION:
> +            VIR_FREE(vm->def->description);
> +            if (metadata &&
> +                !(vm->def->description = strdup(metadata)))
> +                goto no_memory;
> +            break;
> +        case VIR_DOMAIN_METADATA_TITLE:
> +            VIR_FREE(vm->def->title);
> +            if (metadata &&
> +                !(vm->def->title = strdup(metadata)))
> +                goto no_memory;
> +            break;

Looks sane.

> +        case VIR_DOMAIN_METADATA_ELEMENT:
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",

I'd use VIR_ERR_ARGUMENT_UNSUPPORTED (the call is valid per the
documented API, but we don't yet support it).

> +                            _("QEmu driver does not support modifying"
> +                              "<metadata> element"));
> +            goto cleanup;
> +            break;
> +        default:
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                            _("unknown metadata type"));

Whereas this is a correct error (the value does not match documented API).

> +            goto cleanup;
> +            break;
> +        }
> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        switch ((virDomainMetadataType) type) {
> +        case VIR_DOMAIN_METADATA_DESCRIPTION:
> +            VIR_FREE(persistentDef->description);
> +            if (metadata &&
> +                !(persistentDef->description = strdup(metadata)))
> +                goto no_memory;
> +            break;
> +        case VIR_DOMAIN_METADATA_TITLE:
> +            VIR_FREE(persistentDef->title);
> +            if (metadata &&
> +                !(persistentDef->title = strdup(metadata)))
> +                goto no_memory;
> +            break;
> +        case VIR_DOMAIN_METADATA_ELEMENT:
> +            qemuReportError(VIR_ERR_INVALID_ARG, "%s",

And again, VIR_ERR_ARGUMENT_UNSUPPORTED.

> +static char *
> +qemuDomainGetMetadata(virDomainPtr dom,
> +                      int type,
> +                      const char *uri ATTRIBUTE_UNUSED,
> +                      unsigned int flags)
> +{

> +    switch ((virDomainMetadataType) type) {
> +    case VIR_DOMAIN_METADATA_DESCRIPTION:
> +        field = def->description;
> +        break;
> +    case VIR_DOMAIN_METADATA_TITLE:
> +        field = def->title;
> +        break;
> +    case VIR_DOMAIN_METADATA_ELEMENT:
> +        qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                        _("QEMU driver does not support"
> +                          "<metadata> element"));
> +        goto cleanup;
> +        break;
> +    default:
> +        qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                        _("unknown metadata type"));

Here, you got it right for the two error types.

ACK with those two tweaks.

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