[libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks

[Jiri Denemark]

On Fri, Sep 18, 2015 at 18:05:47 +0300, Nikolay Shirokovskiy wrote:
> Move virDomainMigrateUnmanagedProto* expected params list check into
> function itself and use common virTypedParamsCheck for this purpose.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/libvirt-domain.c |   56 ++++++++++++++++++++-----------------------------
>  1 files changed, 23 insertions(+), 33 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index f87a22d..abed9d6 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3322,30 +3322,31 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
>                                  int nparams,
>                                  unsigned int flags)
>  {
> +    /* uri parameter is added for direct case */
> +    const char *compatParams[] = { VIR_MIGRATE_PARAM_DEST_NAME,
> +                                   VIR_MIGRATE_PARAM_BANDWIDTH,
> +                                   VIR_MIGRATE_PARAM_URI };
>      const char *uri = NULL;
>      const char *miguri = NULL;
>      const char *dname = NULL;
> -    const char *xmlin = NULL;
>      unsigned long long bandwidth = 0;
>  
> +    if (!virTypedParamsCheck(params, nparams, compatParams,
> +                             ARRAY_CARDINALITY(compatParams))) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                       _("Migration does not support some of parameters."));

How about "Some parameters are not supported by migration protocol 2"?

> +        return -1;
> +    }
> +
>      if (virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
>          virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
> -        virTypedParamsGetString(params, nparams,
> -                                VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
>          virTypedParamsGetULLong(params, nparams,
>                                  VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
>          return -1;
>      }
>  
> -    if (xmlin) {
> -        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                       _("Unable to change target guest XML during "
> -                         "migration"));
> -        return -1;
> -    }
> -
>      if (flags & VIR_MIGRATE_PEER2PEER) {
>          if (miguri) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                              _("Unable to override peer2peer migration URI"));
               return -1;
           }

Can we merge this check with with the others by adding
VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
passed? I think it would be a bit cleaner, although it depends on how
dirty way of doing that you choose :-)

> @@ -3369,11 +3370,22 @@ virDomainMigrateUnmanagedProto3(virDomainPtr domain,
>                                  int nparams,
>                                  unsigned int flags)
>  {
> +    const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
> +                                   VIR_MIGRATE_PARAM_DEST_NAME,
> +                                   VIR_MIGRATE_PARAM_DEST_XML,
> +                                   VIR_MIGRATE_PARAM_BANDWIDTH };
>      const char *miguri = NULL;
>      const char *dname = NULL;
>      const char *xmlin = NULL;
>      unsigned long long bandwidth = 0;
>  
> +    if (!virTypedParamsCheck(params, nparams, compatParams,
> +                             ARRAY_CARDINALITY(compatParams))) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                       _("Migration does not support some of parameters."));

How about "Some parameters are not supported by migration protocol 3"?

> +        return -1;
> +    }
> +
>      if (virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
>          virTypedParamsGetString(params, nparams,

Jirka