Re: [PATCH 2/2] add default migrate uri in definition file

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

 



Hi Daniel,

On Tue, 2014-04-15 at 12:04 +0100, Daniel P. Berrange wrote: 
> On Tue, Apr 15, 2014 at 06:31:09PM +0800, Chen Fan wrote:
> > Current virsh migrate command require specfying migration URI with
> > command option.
> > 
> > Here is current step.
> > 1) If user specifies --migrateuri on virsh migrate command, then the command
> >    transfers the data to specified host.
> > 2) If --migrateuri is not specified, the command transfers the data to host
> >    whose name is resolved by DNS or /etc/hosts.
> > 
> >    but we are able to use virsh migrate command more usefull.
> >    User can specify a constant destination by definition file.
> >    if user want to specify other temporary destination, command option
> >    is good for it.
> > 
> > Signed-off-by: Chen Fan <chen.fan.fnst@xxxxxxxxxxxxxx>
> > ---
> >  daemon/remote.c              | 11 ++++++++++-
> >  src/driver.h                 |  1 +
> >  src/libvirt.c                | 12 +++++++++++-
> >  src/libvirt.conf             |  7 +++++++
> >  src/libvirt_internal.h       |  1 +
> >  src/qemu/qemu_driver.c       | 37 ++++++++++++++++++++++++++++++++++---
> >  src/remote/remote_driver.c   | 13 +++++++++++++
> >  src/remote/remote_protocol.x |  1 +
> >  8 files changed, 78 insertions(+), 5 deletions(-)
> > 
> > diff --git a/daemon/remote.c b/daemon/remote.c
> > index 8476961..693f460 100644
> > --- a/daemon/remote.c
> > +++ b/daemon/remote.c
> > @@ -5331,6 +5331,7 @@ remoteDispatchDomainMigrateBegin3Params(virNetServerPtr server ATTRIBUTE_UNUSED,
> >      int nparams = 0;
> >      char *cookieout = NULL;
> >      int cookieoutlen = 0;
> > +    char **uri_out = NULL;
> >      int rv = -1;
> >      struct daemonClientPrivate *priv =
> >          virNetServerClientGetPrivateData(client);
> > @@ -5355,21 +5356,29 @@ remoteDispatchDomainMigrateBegin3Params(virNetServerPtr server ATTRIBUTE_UNUSED,
> >                                                      0, &nparams)))
> >          goto cleanup;
> >  
> > +    /* Wacky world of XDR ... */
> > +    if (VIR_ALLOC(uri_out) < 0)
> > +        goto cleanup;
> > +
> >      if (!(xml = virDomainMigrateBegin3Params(dom, params, nparams,
> >                                               &cookieout, &cookieoutlen,
> > +                                             uri_out,
> >                                               args->flags)))
> >          goto cleanup;
> >  
> >      ret->cookie_out.cookie_out_len = cookieoutlen;
> >      ret->cookie_out.cookie_out_val = cookieout;
> > +    ret->uri_out = !*uri_out ? NULL : uri_out;
> >      ret->xml = xml;
> >  
> >      rv = 0;
> >  
> >   cleanup:
> >      virTypedParamsFree(params, nparams);
> > -    if (rv < 0)
> > +    if (rv < 0) {
> >          virNetMessageSaveError(rerr);
> > +        VIR_FREE(uri_out);
> > +     }
> >      if (dom)
> >          virDomainFree(dom);
> >      return rv;
> > diff --git a/src/driver.h b/src/driver.h
> > index e66fc7a..738ab3a 100644
> > --- a/src/driver.h
> > +++ b/src/driver.h
> > @@ -1094,6 +1094,7 @@ typedef char *
> >                                     int nparams,
> >                                     char **cookieout,
> >                                     int *cookieoutlen,
> > +                                   char **uri_out,
> >                                     unsigned int flags);
> >  
> >  typedef int
> > diff --git a/src/libvirt.c b/src/libvirt.c
> > index f8d5240..257adbd 100644
> > --- a/src/libvirt.c
> > +++ b/src/libvirt.c
> > @@ -4738,7 +4738,7 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
> >      VIR_DEBUG("Begin3 %p", domain->conn);
> >      if (useParams) {
> >          dom_xml = domain->conn->driver->domainMigrateBegin3Params
> > -            (domain, params, nparams, &cookieout, &cookieoutlen,
> > +            (domain, params, nparams, &cookieout, &cookieoutlen, &uri_out,
> >               flags | protection);
> >      } else {
> >          dom_xml = domain->conn->driver->domainMigrateBegin3
> > @@ -4748,6 +4748,14 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
> >      if (!dom_xml)
> >          goto done;
> >  
> > +    /* Does domainMigrateBegin3Params() change URI? */
> > +    if (uri_out) {
> > +        if (virTypedParamsReplaceString(&params, &nparams,
> > +                                        VIR_MIGRATE_PARAM_URI,
> > +                                        uri_out) < 0)
> > +            goto done;
> > +    }
> > +
> >      if (useParams) {
> >          /* If source is new enough to support extensible migration parameters,
> >           * it's certainly new enough to support virDomainGetState. */
> > @@ -6778,6 +6786,7 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
> >                               int nparams,
> >                               char **cookieout,
> >                               int *cookieoutlen,
> > +                             char **uri_out,
> >                               unsigned int flags)
> >  {
> >      virConnectPtr conn;
> > @@ -6798,6 +6807,7 @@ virDomainMigrateBegin3Params(virDomainPtr domain,
> >          char *xml;
> >          xml = conn->driver->domainMigrateBegin3Params(domain, params, nparams,
> >                                                        cookieout, cookieoutlen,
> > +                                                      uri_out,
> >                                                        flags);
> >          VIR_DEBUG("xml %s", NULLSTR(xml));
> >          if (!xml)
> > diff --git a/src/libvirt.conf b/src/libvirt.conf
> > index 016cd24..9cef343 100644
> > --- a/src/libvirt.conf
> > +++ b/src/libvirt.conf
> > @@ -16,3 +16,10 @@
> >  # driver when no URI is supplied by the application.
> >  
> >  #uri_default = "qemu:///system"
> > +
> > +#
> > +# This can be used to provide the default migrate URI when
> > +# migrate to target host. if migrate URI had been specified
> > +# in command line, this URI was ignored.
> > +
> > +#uri_migrate = "tcp://dest-uri-example"
> 
> This is a client side configuration file...
> 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 1d08951..c82fbca 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> 
> 
> > +
> > +    if (!uri_in) {
> > +        if (virConnectGetConfigFile(&conf) < 0) {
> > +            goto cleanup;
> > +        }
> > +
> > +        if ((value = virConfGetValue(conf, "uri_migrate"))) {
> > +            if (value->type != VIR_CONF_STRING) {
> > +                VIR_WARN("Expected a string for 'uri_migrate' config parameter");
> > +            } else {
> > +                if (VIR_STRDUP(*uri_out, value->str) < 0)
> > +                    goto cleanup;
> > +            }
> > +        }
> > +        virConfFree(conf);
> 
> ...which you're attempting to read from the server side.
Oh, I'm sorry for this low-level mistake.

> 
> > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> > index ed7dde6..3df59da 100644
> > --- a/src/remote/remote_driver.c
> > +++ b/src/remote/remote_driver.c
> > @@ -6970,6 +6970,7 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain,
> >                                  int nparams,
> >                                  char **cookieout,
> >                                  int *cookieoutlen,
> > +                                char **uri_out,
> >                                  unsigned int flags)
> >  {
> >      char *rv = NULL;
> > @@ -7017,15 +7018,27 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain,
> >          *cookieoutlen = ret.cookie_out.cookie_out_len;
> >      }
> >  
> > +    if (ret.uri_out) {
> > +        if (!uri_out) {
> > +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                           _("caller ignores uri_out"));
> > +            goto error;
> > +        }
> > +        *uri_out = *ret.uri_out; /* Caller frees. */
> > +    }
> > +
> >      rv = ret.xml; /* caller frees */
> >  
> >   cleanup:
> >      remoteFreeTypedParameters(args.params.params_val, args.params.params_len);
> > +    VIR_FREE(ret.uri_out);
> >      remoteDriverUnlock(priv);
> >      return rv;
> >  
> >   error:
> >      VIR_FREE(ret.cookie_out.cookie_out_val);
> > +    if (ret.uri_out)
> > +        VIR_FREE(*ret.uri_out);
> >      goto cleanup;
> >  }
> >  
> > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> > index 6c445cc..202a0eb 100644
> > --- a/src/remote/remote_protocol.x
> > +++ b/src/remote/remote_protocol.x
> > @@ -2860,6 +2860,7 @@ struct remote_domain_migrate_begin3_params_args {
> >  
> >  struct remote_domain_migrate_begin3_params_ret {
> >      opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>;
> > +    remote_string uri_out;
> >      remote_nonnull_string xml;
> >  };
> 
> NACK, this breaks wire protocol compatibility. Changing any existing
> APIs is absolutely forbidden.
> 
> 
> IMHO the idea of storing the 'migration_uri' parameter in a configuration
> file is just plain wrong. This value is inherantly associated with the
> host that you're migrating to. So if you set 'migration_uri' to one host
> in the config, but then invoke virDomainMigrate with a virConnectPtr that
> is associated with a different host, this just crashes and burns. 

how about add a optional 'migrate_uri'(or 'data_migrate_uri') in
libvirtd.conf as secondary network interface?
if so, when user add a new NIC in host A, then user can store this NIC
address to 'migrate_uri' parameter in the configuration file, then when 
doing migration from other host B to this host A, we can get the
'migrate_uri' address in host A and pass this uri back to host B as the
new 'uri_out' value at domainMigratePrepare3Params(). then we don't need
to change any existing APIs. and the new NIC used to transfer migrate
data will be more useful.

Thanks,
Chen


> 
> Regards,
> Daniel


--
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]