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

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(a)cn.fujitsu.com&gt; > --- > 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.

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

Regards, Daniel