[libvirt] [PATCH 0/2] specify migration URI on setting file
Hi all, Current virsh migrate command require specfying migration URI with command option. 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 would like to specify it on setting file like file libvirt.conf of libvirt host. the motivation is: If user would like to increase their system's resource, and user add one more NIC for each host. (One is for management LAN, another is for data LAN). in this case, user would like to use "data LAN" for transfer migrate data. Then, user can specify the NIC address for data LAN by migrateuri. Though the migrateuri is usually stable, user must specify migrateuri every migrate time. So, setting file for migrateuri is desirable. in addition, if there are more KVM host, to change the migrateuri will be too troublesome for user. Chen Fan (2): move virConnectGetConfigFile() to virconf.h add default migrate uri in definition file daemon/remote.c | 11 +++++++- src/driver.h | 1 + src/libvirt.c | 66 ++++++++------------------------------------ src/libvirt.conf | 7 +++++ src/libvirt_internal.h | 1 + src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 37 +++++++++++++++++++++++-- src/remote/remote_driver.c | 13 +++++++++ src/remote/remote_protocol.x | 1 + src/util/virconf.c | 54 ++++++++++++++++++++++++++++++++++++ src/util/virconf.h | 1 + 11 files changed, 134 insertions(+), 60 deletions(-) -- 1.8.1.4
Currently, function virConnectGetConfigFile() is in src/libvirt.c, but this function is to manipulate configration file, so we move it to virconf.c could make it more generic. Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/libvirt.c | 54 ------------------------------------------------ src/libvirt_private.syms | 2 +- src/util/virconf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virconf.h | 1 + 4 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 4454829..f8d5240 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -51,7 +51,6 @@ #include "viruuid.h" #include "viralloc.h" -#include "configmake.h" #include "intprops.h" #include "virconf.h" #if WITH_GNUTLS @@ -875,59 +874,6 @@ virGetVersion(unsigned long *libVer, const char *type ATTRIBUTE_UNUSED, return -1; } - -static char * -virConnectGetConfigFilePath(void) -{ - char *path; - if (geteuid() == 0) { - if (virAsprintf(&path, "%s/libvirt/libvirt.conf", - SYSCONFDIR) < 0) - return NULL; - } else { - char *userdir = virGetUserConfigDirectory(); - if (!userdir) - return NULL; - - if (virAsprintf(&path, "%s/libvirt.conf", - userdir) < 0) { - VIR_FREE(userdir); - return NULL; - } - VIR_FREE(userdir); - } - - return path; -} - - -static int -virConnectGetConfigFile(virConfPtr *conf) -{ - char *filename = NULL; - int ret = -1; - - *conf = NULL; - - if (!(filename = virConnectGetConfigFilePath())) - goto cleanup; - - if (!virFileExists(filename)) { - ret = 0; - goto cleanup; - } - - VIR_DEBUG("Loading config file '%s'", filename); - if (!(*conf = virConfReadFile(filename, 0))) - goto cleanup; - - ret = 0; - - cleanup: - VIR_FREE(filename); - return ret; -} - #define URI_ALIAS_CHARS "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-" diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2d12105..4fdde15 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1142,7 +1142,7 @@ virConfSetValue; virConfWalk; virConfWriteFile; virConfWriteMem; - +virConnectGetConfigFile; # util/vircrypto.h virCryptoHashString; diff --git a/src/util/virconf.c b/src/util/virconf.c index 55de0e9..a06e72d 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -38,6 +38,7 @@ #include "viralloc.h" #include "virfile.h" #include "virstring.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_CONF @@ -1054,3 +1055,56 @@ virConfWriteMem(char *memory, int *len, virConfPtr conf) *len = use; return use; } + + +static char * +virConnectGetConfigFilePath(void) +{ + char *path; + if (geteuid() == 0) { + if (virAsprintf(&path, "%s/libvirt/libvirt.conf", + SYSCONFDIR) < 0) + return NULL; + } else { + char *userdir = virGetUserConfigDirectory(); + if (!userdir) + return NULL; + + if (virAsprintf(&path, "%s/libvirt.conf", + userdir) < 0) { + VIR_FREE(userdir); + return NULL; + } + VIR_FREE(userdir); + } + + return path; +} + + +int +virConnectGetConfigFile(virConfPtr *conf) +{ + char *filename = NULL; + int ret = -1; + + *conf = NULL; + + if (!(filename = virConnectGetConfigFilePath())) + goto cleanup; + + if (!virFileExists(filename)) { + ret = 0; + goto cleanup; + } + + VIR_DEBUG("Loading config file '%s'", filename); + if (!(*conf = virConfReadFile(filename, 0))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(filename); + return ret; +} diff --git a/src/util/virconf.h b/src/util/virconf.h index 2a6b050..aacd1c1 100644 --- a/src/util/virconf.h +++ b/src/util/virconf.h @@ -106,5 +106,6 @@ int virConfWriteFile (const char *filename, int virConfWriteMem (char *memory, int *len, virConfPtr conf); +int virConnectGetConfigFile(virConfPtr *conf); #endif /* __VIR_CONF_H__ */ -- 1.8.1.4
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@cn.fujitsu.com> --- 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(¶ms, &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" diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index ebf2acf..c4b92b2 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -236,6 +236,7 @@ char *virDomainMigrateBegin3Params(virDomainPtr domain, int nparams, char **cookieout, int *cookieoutlen, + char **uri_out, unsigned int flags); int virDomainMigratePrepare3Params(virConnectPtr dconn, 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 @@ -95,6 +95,7 @@ #include "viraccessapicheckqemu.h" #include "storage/storage_driver.h" #include "virhostdev.h" +#include "virconf.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -10801,12 +10802,17 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, int nparams, char **cookieout, int *cookieoutlen, + char **uri_out, unsigned int flags) { const char *xmlin = NULL; const char *dname = NULL; virDomainObjPtr vm; + const char *uri_in = NULL; + virConfPtr conf = NULL; + virConfValuePtr value = NULL; + *uri_out = NULL; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) return NULL; @@ -10816,19 +10822,44 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, &xmlin) < 0 || virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, - &dname) < 0) + &dname) < 0 || + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, + &uri_in) < 0) return NULL; if (!(vm = qemuDomObjFromDomain(domain))) return NULL; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { - virObjectUnlock(vm); - return NULL; + goto cleanup; + } + + 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); } return qemuMigrationBegin(domain->conn, vm, xmlin, dname, cookieout, cookieoutlen, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + virConfFree(conf); + + return NULL; } 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; }; -- 1.8.1.4
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@cn.fujitsu.com> --- 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(¶ms, &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 -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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@cn.fujitsu.com> --- 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(¶ms, &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
On Wed, Apr 16, 2014 at 04:19:05AM +0000, chen.fan.fnst@cn.fujitsu.com wrote:
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.
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.
The problem is that the migrate_uri is tied to a specific target host, while the API can be told to migrate to any host. I just dont see how it makes sense to store this URI in any configuration file. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, 2014-04-16 at 10:08 +0100, Daniel P. Berrange wrote:
On Wed, Apr 16, 2014 at 04:19:05AM +0000, chen.fan.fnst@cn.fujitsu.com wrote:
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.
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.
The problem is that the migrate_uri is tied to a specific target host, while the API can be told to migrate to any host. I just dont see how it makes sense to store this URI in any configuration file.
I'm sorry if I confused you. My new Idea is that: If we have 2 NIC or more in our target host, we can configure the secondary NIC address as the migrate data transfer's address on this host, then when other hosts need to migrate VM to this target host, they could through the dest_uri to get this secondary address from the target host. thus the secondary NIC can be used to transfer migrate data. certainly, the default configuration should be disabled. and this uri just was tied to its host. if the API want to migrate to any host, it should be not affected I guess. :) Thanks, Chen
Regards, Daniel
On Wed, Apr 16, 2014 at 10:12:18 +0000, chen.fan.fnst@cn.fujitsu.com wrote:
On Wed, 2014-04-16 at 10:08 +0100, Daniel P. Berrange wrote:
On Wed, Apr 16, 2014 at 04:19:05AM +0000, chen.fan.fnst@cn.fujitsu.com wrote:
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.
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.
The problem is that the migrate_uri is tied to a specific target host, while the API can be told to migrate to any host. I just dont see how it makes sense to store this URI in any configuration file.
I'm sorry if I confused you. My new Idea is that: If we have 2 NIC or more in our target host, we can configure the secondary NIC address as the migrate data transfer's address on this host, then when other hosts need to migrate VM to this target host, they could through the dest_uri to get this secondary address from the target host. thus the secondary NIC can be used to transfer migrate data. certainly, the default configuration should be disabled. and this uri just was tied to its host. if the API want to migrate to any host, it should be not affected I guess. :)
During migration destination libvirtd sends the URI which it thinks is the one that can be used to contact the daemon from other hosts. Currently we construct the URI from hostname and IIUC this request is to make it configurable. In other words, instead of using the hostname, libvirtd would use the address specified in the configuration file. That is, an admin would be able to explicitly configure which of the host's addresses should be used for incoming migrations unless overridden by migrate URI passed to the Prepare migration API. Am I correct? If so, I believe this can be useful. Jirka
On Wed, 2014-04-16 at 13:42 +0200, Jiri Denemark wrote:
On Wed, Apr 16, 2014 at 10:12:18 +0000, chen.fan.fnst@cn.fujitsu.com wrote:
On Wed, 2014-04-16 at 10:08 +0100, Daniel P. Berrange wrote:
On Wed, Apr 16, 2014 at 04:19:05AM +0000, chen.fan.fnst@cn.fujitsu.com wrote:
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.
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.
The problem is that the migrate_uri is tied to a specific target host, while the API can be told to migrate to any host. I just dont see how it makes sense to store this URI in any configuration file.
I'm sorry if I confused you. My new Idea is that: If we have 2 NIC or more in our target host, we can configure the secondary NIC address as the migrate data transfer's address on this host, then when other hosts need to migrate VM to this target host, they could through the dest_uri to get this secondary address from the target host. thus the secondary NIC can be used to transfer migrate data. certainly, the default configuration should be disabled. and this uri just was tied to its host. if the API want to migrate to any host, it should be not affected I guess. :)
During migration destination libvirtd sends the URI which it thinks is the one that can be used to contact the daemon from other hosts. Currently we construct the URI from hostname and IIUC this request is to make it configurable. In other words, instead of using the hostname, libvirtd would use the address specified in the configuration file. That is, an admin would be able to explicitly configure which of the host's addresses should be used for incoming migrations unless overridden by migrate URI passed to the Prepare migration API.
Am I correct? If so, I believe this can be useful. Yes, your understanding is correct. Thanks for your generous explanation.
I would like to make patches to implement it subsequently. Thanks, Chen
Jirka
participants (4)
-
Chen Fan -
chen.fan.fnst＠cn.fujitsu.com
-
Daniel P. Berrange -
Jiri Denemark