Re: [PATCH 1/3] conf: Verify metadata type right away

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

 



On 07/08/2014 09:29 AM, Peter Krempa wrote:
> Verify the desired metadata type in the libvirt.c dispatcher rather than
> in the bottom level executor function.
> ---
>  src/conf/domain_conf.c | 10 ++--------
>  src/libvirt.c          | 10 ++++++----
>  2 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 70f1103..a2b0f23 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19601,10 +19601,7 @@ virDomainObjGetMetadata(virDomainObjPtr vm,
>              goto cleanup;
>          break;
> 
> -    default:
> -        virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                       _("unknown metadata type"));
> -        goto cleanup;
> +    case VIR_DOMAIN_METADATA_LAST:
>          break;

This part is nice for one reason:  it cleans up a dead code issue (the
break was previously unreachable).  But I'm not sure if it is right to
hoist the validation into the caller...

> +++ b/src/libvirt.c
> @@ -10178,8 +10178,9 @@ virDomainSetMetadata(virDomainPtr domain,
>              virCheckNonNullArgGoto(key, error);
>          break;
>      default:
> -        /* For future expansion */
> -        break;
> +        virReportInvalidArg(type,
> +                            _("unsupported metadata type '%d'"), type);
> +        goto error;
>      }

...because doing this makes it impossible for an older client to
manually set a newer key of a newer server.  That is, we intentionally
don't reject unknown flags in libvirt.c, so that:

func(0x4) -> old libvirt.so that doesn't know flag 4 -> new libvirtd
that does

works, rather than dying client-side.  On the same vein,
virTypedParameterValidateSet rejects strings when the server is too old,
but does NOT reject unknown parameter type extensions (which would allow
an extension that does not affect wire protocol to go through an old
client to a new server).

I'd like Dan to chime in on this one.

-- 
Eric Blake   eblake redhat com    +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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]