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

Eric Blake eblake at redhat.com
Tue Jul 8 15:50:50 UTC 2014


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

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


More information about the libvir-list mailing list