[libvirt] [PATCH v2 08/12] migration: refactor: gather parameters compatibility checks

[John Ferlan]

$subj:  "migration: refactor parameter compatibility checks

On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
> Checks for migration's parameter set and support by protocol are slightly

s/migration's parameter set/migration parameter sets
> scattered in code. Let's put it in one place, namely every protocol function
> should check it's parameter set.
> 

Perhaps more simply said - "Use the common virTypedParamsCheck for each
virDomainMigrate* function to check its expected params list prior to
the API making the driver call.  Thi"


> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/libvirt-domain.c |   56 ++++++++++++++++++++-----------------------------
>  1 files changed, 23 insertions(+), 33 deletions(-)
> 

I think the "one" (perhaps naive) mental note I make is that you add a
virTypedParamsCheck to virDomainMigrateUnmanagedProto3; however, it
seems it's unnecessary for virDomainMigratePeer2PeerParams... Yes, I see
one calls Perform3 and one calls Perform3Params

Still seems sane to me

John
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 55efd49..8b4171e 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3345,30 +3345,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."));
> +        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",
> @@ -3391,11 +3392,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."));
> +        return -1;
> +    }
> +
>      if (virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
>          virTypedParamsGetString(params, nparams,
> @@ -4439,12 +4451,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
>                         unsigned int nparams,
>                         unsigned int flags)
>  {
> -    bool compat;
> -    const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
> -                                   VIR_MIGRATE_PARAM_DEST_NAME,
> -                                   VIR_MIGRATE_PARAM_DEST_XML,
> -                                   VIR_MIGRATE_PARAM_BANDWIDTH };
> -
>      VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparms=%u flags=%x",
>                       NULLSTR(dconnuri), params, nparams, flags);
>      VIR_TYPED_PARAMS_DEBUG(params, nparams);
> @@ -4459,9 +4465,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
>                               VIR_MIGRATE_NON_SHARED_INC,
>                               error);
>  
> -    compat = virTypedParamsCheck(params, nparams, compatParams,
> -                                 ARRAY_CARDINALITY(compatParams));
> -
>      if (flags & VIR_MIGRATE_PEER2PEER) {
>          if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>                                        VIR_DRV_FEATURE_MIGRATION_P2P)) {
> @@ -4477,17 +4480,11 @@ virDomainMigrateToURI3(virDomainPtr domain,
>              if (virDomainMigratePeer2PeerParams(domain, dconnuri, params,
>                                                  nparams, flags) < 0)
>                  goto error;
> -        } else if (compat) {
> +        } else {
>              VIR_DEBUG("Using peer2peer migration");
>              if (virDomainMigrateUnmanagedParams(domain, dconnuri, params,
>                                                  nparams, flags) < 0)
>                  goto error;
> -        } else {
> -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                           _("Peer-to-peer migration with extensible "
> -                             "parameters is not supported but extended "
> -                             "parameters were passed"));
> -            goto error;
>          }
>      } else {
>          if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> @@ -4499,13 +4496,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
>              goto error;
>          }
>  
> -        if (!compat) {
> -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                           _("Direct migration does not support extensible "
> -                             "parameters"));
> -            goto error;
> -        }
> -
>          VIR_DEBUG("Using direct migration");
>          if (virDomainMigrateUnmanagedParams(domain, NULL, params,
>                                              nparams, flags) < 0)
>