[libvirt] [PATCH v2 02/12] migration: refactor: reuse p2p url check

[John Ferlan]

On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
> As promised in previous patch.

Update commit message to say what's being done from patch 1:

"Common uri check we factor out in different patch for clarity."

Or for me ;-)  "Refactor dconnuri local server URI check to common API"


> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/libvirt-domain.c |   43 +++++++++++++++++++++++--------------------
>  1 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 1a00485..07e342f 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3293,14 +3293,33 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
>  }
>  

I missed nothing this with the previous change - keep to two blank lines
between functions...

>  static int
> +virDomainMigrateCheckNotLocal(const char *dconnuri)
> +{
> +    virURIPtr tempuri = NULL;
> +    int ret = -1;
> +
> +    if (!(tempuri = virURIParse(dconnuri)))

^^ This will get us into trouble later because 'dconnuri' cannot be NULL
when being passed to virURIParse; however, by patch 10 there's a call to
virDomainMigrateUnmanaged with a NULL dconnuri "if (!(flags &
VIR_MIGRATE_PEER2PEER))"

I haven't yet followed all the future code motion, although you could
add a ATTRIBUTE_NONNULL(1) after the "static int" to be more clear even
though yes, it's perhaps not something that was in the "existing" code.


Other than spacing and missing commit message, this seems fine.

John

> +        goto cleanup;
> +    if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
> +        virReportInvalidArg(dconnuri, "%s",
> +                            _("Attempt to migrate guest to the same host %s"));
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +
> +    virURIFree(tempuri);
> +    return ret;
> +}
> +
> +static int
>  virDomainMigratePeer2PeerParams(virDomainPtr domain,
>                                  const char *dconnuri,
>                                  virTypedParameterPtr params,
>                                  int nparams,
>                                  unsigned int flags)
>  {
> -    virURIPtr tempuri = NULL;
> -
>      VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
>                       dconnuri, params, nparams, flags);
>      VIR_TYPED_PARAMS_DEBUG(params, nparams);
> @@ -3310,15 +3329,8 @@ virDomainMigratePeer2PeerParams(virDomainPtr domain,
>          return -1;
>      }
>  
> -    if (!(tempuri = virURIParse(dconnuri)))
> +    if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
>          return -1;
> -    if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
> -        virReportInvalidArg(dconnuri, "%s",
> -                            _("unable to parse server from dconnuri"));
> -        virURIFree(tempuri);
> -        return -1;
> -    }
> -    virURIFree(tempuri);
>  
>      VIR_DEBUG("Using migration protocol 3 with extensible parameters");
>      return domain->conn->driver->domainMigratePerform3Params
> @@ -3335,8 +3347,6 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
>                                 const char *uri,
>                                 unsigned long long bandwidth)
>  {
> -    virURIPtr tempuri = NULL;
> -
>      VIR_DOMAIN_DEBUG(domain,
>                       "dconnuri=%s, xmlin=%s, dname=%s, uri=%s, bandwidth=%llu "
>                       "flags=%x",
> @@ -3349,15 +3359,8 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
>          return -1;
>      }
>  
> -    if (!(tempuri = virURIParse(dconnuri)))
> -        return -1;
> -    if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
> -        virReportInvalidArg(dconnuri, "%s",
> -                            _("unable to parse server from dconnuri"));
> -        virURIFree(tempuri);
> +    if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
>          return -1;
> -    }
> -    virURIFree(tempuri);
>  
>      if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>                                          VIR_DRV_FEATURE_MIGRATION_V3)) {
>