[libvirt] [PATCH v5 0/5] vz: add migration support
NOTE that minimal command to migrate vz domain is like next: virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p --live --compressed --persistent --undefinesource Difference from v4: 1. move preparation of the migration uri from src to dst as dst intended to do it. 2. change hypervisor migration scheme from 'tcp' to 'vzmigr' 3. declare this scheme in host caps src/vz/vz_driver.c | 356 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 86 +++++++++++-- src/vz/vz_sdk.h | 6 + src/vz/vz_utils.h | 4 +- 4 files changed, 437 insertions(+), 15 deletions(-)
This session uuid acts as authN token for different multihost vz operations one of which is migration. Unfortunately we can't get it from server at any time thus we need to save it at login. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 39 +++++++++++++++++++++++++++++---------- src/vz/vz_utils.h | 2 +- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 744b58a..f7253de 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -37,6 +37,9 @@ #define VIR_FROM_THIS VIR_FROM_PARALLELS #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX +static int +prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid); + VIR_LOG_INIT("parallels.sdk"); /* @@ -228,24 +231,40 @@ prlsdkDeinit(void) int prlsdkConnect(vzConnPtr privconn) { - PRL_RESULT ret; + int ret = -1; + PRL_RESULT pret; PRL_HANDLE job = PRL_INVALID_HANDLE; + PRL_HANDLE result = PRL_INVALID_HANDLE; + PRL_HANDLE response = PRL_INVALID_HANDLE; + char session_uuid[VIR_UUID_STRING_BUFLEN + 2]; + PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid); - ret = PrlSrv_Create(&privconn->server); - if (PRL_FAILED(ret)) { - logPrlError(ret); - return -1; - } + pret = PrlSrv_Create(&privconn->server); + prlsdkCheckRetGoto(pret, cleanup); job = PrlSrv_LoginLocalEx(privconn->server, NULL, 0, PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE); + if (PRL_FAILED(getJobResult(job, &result))) + goto cleanup; - if (waitJob(job)) { + pret = PrlResult_GetParam(result, &response); + prlsdkCheckRetGoto(pret, cleanup); + + pret = PrlLoginResponse_GetSessionUuid(response, session_uuid, &buflen); + prlsdkCheckRetGoto(pret, cleanup); + + if (prlsdkUUIDParse(session_uuid, privconn->session_uuid) < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (ret < 0) PrlHandle_Free(privconn->server); - return -1; - } + PrlHandle_Free(result); + PrlHandle_Free(response); - return 0; + return ret; } void diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index db09647..fe54b25 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -60,7 +60,7 @@ struct _vzConn { /* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; - + unsigned char session_uuid[VIR_UUID_BUFLEN]; PRL_HANDLE server; virStoragePoolObjList pools; virNetworkObjListPtr networks; -- 1.7.1
From: nshirokovskiy@virtuozzo.com <nshirokovskiy@virtuozzo.com> This patch makes basic vz migration possible. For example by virsh: virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p Vz migration is implemented as p2p migration. The reason is that vz sdk do all the job. The question may arise then why don't implement it as a direct migration. The reason is that we want to leverage rich libvirt authentication abilities we lack in vz sdk. We can do it because vz sdk can use tokens to factor out authentication from migration command. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 257 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 33 +++++++ src/vz/vz_sdk.h | 2 + 3 files changed, 292 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 8fa7957..36c64e9 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -146,6 +146,9 @@ vzBuildCapabilities(void) caps->host.cpu = cpu; + if (virCapabilitiesAddHostMigrateTransport(caps, "vzmigr") < 0) + goto error; + if (!(data = cpuNodeData(cpu->arch)) || cpuDecode(cpu, data, NULL, 0, NULL) < 0) { goto cleanup; @@ -1343,6 +1346,257 @@ vzDomainMemoryStats(virDomainPtr domain, return ret; } +static char* +vzFormatCookie(const unsigned char *session_uuid) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "<vz-migration1>\n"); + virUUIDFormat(session_uuid, uuidstr); + virBufferAsprintf(&buf, "<session_uuid>%s</session_uuid>\n", uuidstr); + virBufferAddLit(&buf, "</vz-migration1>\n"); + + if (virBufferCheckError(&buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} + +static int +vzParseCookie(const char *xml, unsigned char *session_uuid) +{ + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctx = NULL; + char *tmp = NULL; + int ret = -1; + + if (!(doc = virXMLParseStringCtxt(xml, _("(_migration_cookie)"), &ctx))) + goto cleanup; + + if (!(tmp = virXPathString("string(./session_uuid[1])", ctx))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing session_uuid element in migration data")); + goto cleanup; + } + if (virUUIDParse(tmp, session_uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed session_uuid element in migration data")); + goto cleanup; + } + ret = 0; + + cleanup: + xmlXPathFreeContext(ctx); + xmlFreeDoc(doc); + VIR_FREE(tmp); + + return ret; +} + +#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER) + +#define VZ_MIGRATION_PARAMETERS \ + VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ + NULL + +static char* +vzCreateMigrationURI(void) +{ + char *hostname = NULL; + char *uri = NULL; + + if ((hostname = virGetHostname()) == NULL) + goto cleanup; + + if (STRPREFIX(hostname, "localhost")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("hostname on destination resolved to localhost," + " but migration requires an FQDN")); + goto cleanup; + } + + if (virAsprintf(&uri, "vzmigr://%s", hostname) < 0) + goto cleanup; + + cleanup: + VIR_FREE(hostname); + return uri; +} + +static int +vzDomainMigratePrepare3Params(virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout, + int *cookieoutlen, + char **uri_out, + unsigned int flags) +{ + vzConnPtr privconn = conn->privateData; + const char *miguri = NULL; + int ret = -1; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &miguri) < 0) + goto cleanup; + + /* This check is of par with direct managed imlementation */ + if (miguri == NULL) + *uri_out = vzCreateMigrationURI(); + + if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) + goto cleanup; + *cookieoutlen = strlen(*cookieout) + 1; + ret = 0; + + cleanup: + if (ret != 0) { + VIR_FREE(*cookieout); + *cookieoutlen = 0; + VIR_FREE(*uri_out); + } + + return ret; +} + +static int +vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) +{ + switch (feature) { + case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_MIGRATION_P2P: + return 1; + default: + return 0; + } +} + +virURIPtr +vzParseVzURI(const char *uri_str) +{ + virURIPtr uri = NULL; + int ret = -1; + + if (!(uri = virURIParse(uri_str))) + goto cleanup; + + if (uri->scheme == NULL || uri->server == NULL) { + virReportError(VIR_ERR_INVALID_ARG, + _("scheme and host are mandatory vz migration URI: %s"), + uri_str); + goto cleanup; + } + + if (uri->user != NULL || uri->path != NULL || + uri->query != NULL || uri->fragment != NULL) { + virReportError(VIR_ERR_INVALID_ARG, + _("only scheme, host and port are supported in " + "vz migration URI: %s"), uri_str); + goto cleanup; + } + + if (STRNEQ(uri->scheme, "vzmigr")) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("unsupported scheme %s in migration URI %s"), + uri->scheme, uri_str); + goto cleanup; + } + + ret = 0; + + cleanup: + if (ret < 0) { + virURIFree(uri); + uri = NULL; + } + + return uri; +} + +static int +vzDomainMigratePerform3Params(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + int ret = -1; + virDomainObjPtr dom = NULL; + virConnectPtr dconn = NULL; + virURIPtr vzuri = NULL; + unsigned char session_uuid[VIR_UUID_BUFLEN]; + vzConnPtr privconn = domain->conn->privateData; + char *cookie = NULL; + int cookielen = 0; + const char *miguri = NULL; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (!(dom = vzDomObjFromDomain(domain))) + goto cleanup; + + dconn = virConnectOpen(dconnuri); + if (dconn == NULL) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to connect to remote libvirt URI %s: %s"), + dconnuri, virGetLastErrorMessage()); + goto cleanup; + } + + if (virDomainMigratePrepare3Params(dconn, params, nparams, NULL, 0, + &cookie, &cookielen, + (char**)&miguri, flags) < 0) + goto cleanup; + + if (vzParseCookie(cookie, session_uuid) < 0) + goto cleanup; + + if (miguri == NULL && + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &miguri) < 0) + goto cleanup; + + if (miguri == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domainMigratePrepare3Params did not set miguri")); + } + + if (!(vzuri = vzParseVzURI(miguri))) + goto cleanup; + + if (prlsdkMigrate(dom, vzuri, session_uuid) < 0) + goto cleanup; + + virDomainObjListRemove(privconn->domains, dom); + dom = NULL; + + ret = 0; + + cleanup: + if (dom) + virObjectUnlock(dom); + virObjectUnref(dconn); + virURIFree(vzuri); + VIR_FREE(cookie); + + return ret; +} + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1396,6 +1650,9 @@ static virHypervisorDriver vzDriver = { .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ + .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */ + .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */ + .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index f7253de..783438d 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4054,3 +4054,36 @@ prlsdkGetMemoryStats(virDomainObjPtr dom, return ret; } + +/* high security is default choice for 2 reasons: + 1. as this is the highest set security we can't get + reject from server with high security settings + 2. this is on par with security level of driver + connection to dispatcher */ + +#define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY) + +int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, + const unsigned char *session_uuid) +{ + int ret = -1; + vzDomObjPtr privdom = dom->privateData; + PRL_HANDLE job = PRL_INVALID_HANDLE; + char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + + prlsdkUUIDFormat(session_uuid, uuidstr); + job = PrlVm_MigrateEx(privdom->sdkdom, uri->server, uri->port, uuidstr, + "", /* use default dir for migrated instance bundle */ + PRLSDK_MIGRATION_FLAGS, + 0, /* reserved flags */ + PRL_TRUE /* don't ask for confirmations */ + ); + + if (PRL_FAILED(waitJob(job))) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index ebe4591..d3f0caf 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -76,3 +76,5 @@ int prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time); int prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int +prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned *session_uuid); -- 1.7.1
On Fri, Sep 04, 2015 at 05:18:03PM +0300, Nikolay Shirokovskiy wrote:
From: nshirokovskiy@virtuozzo.com <nshirokovskiy@virtuozzo.com>
This patch makes basic vz migration possible. For example by virsh:
virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
Vz migration is implemented as p2p migration. The reason is that vz sdk do all the job. The question may arise then why don't implement it as a direct migration. The reason is that we want to leverage rich libvirt authentication abilities we lack in vz sdk. We can do it because vz sdk can use tokens to factor out authentication from migration command.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 257 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 33 +++++++ src/vz/vz_sdk.h | 2 + 3 files changed, 292 insertions(+), 0 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 8fa7957..36c64e9 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c
+static int +vzDomainMigratePerform3Params(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + int ret = -1; + virDomainObjPtr dom = NULL; + virConnectPtr dconn = NULL; + virURIPtr vzuri = NULL; + unsigned char session_uuid[VIR_UUID_BUFLEN]; + vzConnPtr privconn = domain->conn->privateData; + char *cookie = NULL; + int cookielen = 0; + const char *miguri = NULL; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (!(dom = vzDomObjFromDomain(domain))) + goto cleanup; + + dconn = virConnectOpen(dconnuri); + if (dconn == NULL) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to connect to remote libvirt URI %s: %s"), + dconnuri, virGetLastErrorMessage()); + goto cleanup; + } + + if (virDomainMigratePrepare3Params(dconn, params, nparams, NULL, 0, + &cookie, &cookielen, + (char**)&miguri, flags) < 0) + goto cleanup; + + if (vzParseCookie(cookie, session_uuid) < 0) + goto cleanup; + + if (miguri == NULL && + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &miguri) < 0) + goto cleanup; + + if (miguri == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domainMigratePrepare3Params did not set miguri")); + } + + if (!(vzuri = vzParseVzURI(miguri))) + goto cleanup; + + if (prlsdkMigrate(dom, vzuri, session_uuid) < 0) + goto cleanup; + + virDomainObjListRemove(privconn->domains, dom); + dom = NULL; + + ret = 0; + + cleanup: + if (dom) + virObjectUnlock(dom); + virObjectUnref(dconn); + virURIFree(vzuri); + VIR_FREE(cookie); + + return ret; +} + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1396,6 +1650,9 @@ static virHypervisorDriver vzDriver = { .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ + .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */ + .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */ + .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */ };
As mentioned in the previous review, I really want to see VZ provide the full set of drive callbacks required by the V3 migration protocol, not just those you happen to need to have today. ie add Begin, Finish & Confirm. This in turn makes it quite easy to support non-P2P mode which is desirable because in P2P mode there is no practical way todo interactive auth for the destination connection, but in non-P2P mode the client app opens the destination connection, so can do auth if needed. I believe that the following changes to this patch should do all that is required. NB, I have only compile tested this, not actually run it, since I don't have an active VZ install, only the SDK install. This should work functionally identically with & without the PEER2PEER flag being set by the caller. diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 36c64e9..b160a3b 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1398,6 +1398,7 @@ vzParseCookie(const char *xml, unsigned char *session_uuid) #define VZ_MIGRATION_PARAMETERS \ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ NULL static char* @@ -1425,49 +1426,6 @@ vzCreateMigrationURI(void) } static int -vzDomainMigratePrepare3Params(virConnectPtr conn, - virTypedParameterPtr params, - int nparams, - const char *cookiein ATTRIBUTE_UNUSED, - int cookieinlen ATTRIBUTE_UNUSED, - char **cookieout, - int *cookieoutlen, - char **uri_out, - unsigned int flags) -{ - vzConnPtr privconn = conn->privateData; - const char *miguri = NULL; - int ret = -1; - - virCheckFlags(VZ_MIGRATION_FLAGS, -1); - - if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) - goto cleanup; - - if (virTypedParamsGetString(params, nparams, - VIR_MIGRATE_PARAM_URI, &miguri) < 0) - goto cleanup; - - /* This check is of par with direct managed imlementation */ - if (miguri == NULL) - *uri_out = vzCreateMigrationURI(); - - if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) - goto cleanup; - *cookieoutlen = strlen(*cookieout) + 1; - ret = 0; - - cleanup: - if (ret != 0) { - VIR_FREE(*cookieout); - *cookieoutlen = 0; - VIR_FREE(*uri_out); - } - - return ret; -} - -static int vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { switch (feature) { @@ -1479,7 +1437,7 @@ vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) } } -virURIPtr +static virURIPtr vzParseVzURI(const char *uri_str) { virURIPtr uri = NULL; @@ -1521,64 +1479,128 @@ vzParseVzURI(const char *uri_str) return uri; } +static char * +vzDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virDomainObjPtr privdom; + char *ret = NULL; + + if (!(privdom = vzDomObjFromDomain(domain))) + goto cleanup; + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (virTypedParamsGet(params, nparams, VIR_MIGRATE_PARAM_DEST_XML)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Changing destination XML is not supported")); + goto cleanup; + } + + /* We don't really need the XML, but the Begin API requires it + * be returned anyway + */ + ret = virDomainDefFormat(privdom->def, VIR_DOMAIN_XML_MIGRATABLE); + + cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; + +} + static int -vzDomainMigratePerform3Params(virDomainPtr domain, - const char *dconnuri, +vzDomainMigratePrepare3Params(virConnectPtr conn, virTypedParameterPtr params, int nparams, const char *cookiein ATTRIBUTE_UNUSED, int cookieinlen ATTRIBUTE_UNUSED, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, + char **cookieout, + int *cookieoutlen, + char **uri_out, unsigned int flags) { - int ret = -1; - virDomainObjPtr dom = NULL; - virConnectPtr dconn = NULL; - virURIPtr vzuri = NULL; - unsigned char session_uuid[VIR_UUID_BUFLEN]; - vzConnPtr privconn = domain->conn->privateData; - char *cookie = NULL; - int cookielen = 0; + vzConnPtr privconn = conn->privateData; const char *miguri = NULL; + int ret = -1; virCheckFlags(VZ_MIGRATION_FLAGS, -1); if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup; - if (!(dom = vzDomObjFromDomain(domain))) + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &miguri) < 0) goto cleanup; - dconn = virConnectOpen(dconnuri); - if (dconn == NULL) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s: %s"), - dconnuri, virGetLastErrorMessage()); + /* This check is of par with direct managed imlementation */ + if (miguri == NULL) + *uri_out = vzCreateMigrationURI(); + + if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) goto cleanup; + *cookieoutlen = strlen(*cookieout) + 1; + ret = 0; + + cleanup: + if (ret != 0) { + VIR_FREE(*cookieout); + *cookieoutlen = 0; + VIR_FREE(*uri_out); } - if (virDomainMigratePrepare3Params(dconn, params, nparams, NULL, 0, - &cookie, &cookielen, - (char**)&miguri, flags) < 0) + return ret; +} + + +static int +vzDomainMigratePerform3DirectParams(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + int ret = -1; + virDomainObjPtr dom = NULL; + virConnectPtr dconn = NULL; + virURIPtr vzuri = NULL; + unsigned char session_uuid[VIR_UUID_BUFLEN]; + vzConnPtr privconn = domain->conn->privateData; + char *defmiguri = NULL; + const char *miguri = NULL; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup; - if (vzParseCookie(cookie, session_uuid) < 0) + if (!(dom = vzDomObjFromDomain(domain))) goto cleanup; - if (miguri == NULL && - virTypedParamsGetString(params, nparams, + if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_URI, &miguri) < 0) - goto cleanup; + goto cleanup; if (miguri == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare3Params did not set miguri")); + goto cleanup; } if (!(vzuri = vzParseVzURI(miguri))) goto cleanup; + if (vzParseCookie(cookiein, session_uuid) < 0) + goto cleanup; + if (prlsdkMigrate(dom, vzuri, session_uuid) < 0) goto cleanup; @@ -1592,11 +1614,336 @@ vzDomainMigratePerform3Params(virDomainPtr domain, virObjectUnlock(dom); virObjectUnref(dconn); virURIFree(vzuri); - VIR_FREE(cookie); + VIR_FREE(defmiguri); + + return ret; +} + +static int +vzDomainMigratePerform3P2PParams(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virDomainPtr ddomain = NULL; + char *uri_out = NULL; + char *cookiein = NULL; + char *cookieout = NULL; + char *dom_xml = NULL; + int cookieinlen = 0; + int cookieoutlen = 0; + int ret; + virErrorPtr orig_err = NULL; + int cancelled = 1; + bool notify_source = true; + unsigned int destflags; + int state; + virTypedParameterPtr tmp; + const char *uri = NULL; + virConnectPtr dconn = NULL; + + VIR_DOMAIN_DEBUG(domain, + "params=%p, nparams=%d, flags=%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + if (virTypedParamsCopy(&tmp, params, nparams) < 0) + return NULL; + params = tmp; + + if ((dconn = virConnectOpen(dconnuri)) == NULL) + goto done; + + VIR_DEBUG("Begin3 %p", domain->conn); + dom_xml = domain->conn->driver->domainMigrateBegin3Params + (domain, params, nparams, &cookieout, &cookieoutlen, + flags); + if (!dom_xml) + goto done; + + ret = virDomainGetState(domain, &state, NULL, 0); + if (ret == 0 && state == VIR_DOMAIN_PAUSED) + flags |= VIR_MIGRATE_PAUSED; + + destflags = flags & ~(VIR_MIGRATE_ABORT_ON_ERROR | + VIR_MIGRATE_AUTO_CONVERGE); + + VIR_DEBUG("Prepare3 %p flags=%x", dconn, destflags); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + if (virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_DEST_XML, + dom_xml) < 0) + goto done; + ret = dconn->driver->domainMigratePrepare3Params + (dconn, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, &uri_out, destflags); + if (ret == -1) { + goto done; + } + + /* Did domainMigratePrepare3 change URI? */ + if (uri_out) { + uri = uri_out; + if (virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_URI, + uri_out) < 0) { + cancelled = 1; + orig_err = virSaveLastError(); + goto finish; + } + } else if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &uri) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domainMigratePrepare3 did not set uri")); + cancelled = 1; + orig_err = virSaveLastError(); + goto finish; + } + + if (flags & VIR_MIGRATE_OFFLINE) { + VIR_DEBUG("Offline migration, skipping Perform phase"); + VIR_FREE(cookieout); + cookieoutlen = 0; + cancelled = 0; + goto finish; + } + + /* Perform the migration. The driver isn't supposed to return + * until the migration is complete. The src VM should remain + * running, but in paused state until the destination can + * confirm migration completion. + */ + VIR_DEBUG("Perform3 %p uri=%s", domain->conn, uri); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + /* dconnuri not relevant in non-P2P modes, so left NULL here */ + ret = domain->conn->driver->domainMigratePerform3Params + (domain, NULL, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, flags & ~(VIR_MIGRATE_PEER2PEER)); + + /* Perform failed. Make sure Finish doesn't overwrite the error */ + if (ret < 0) { + orig_err = virSaveLastError(); + /* Perform failed so we don't need to call confirm to let source know + * about the failure. + */ + notify_source = false; + } + + /* If Perform returns < 0, then we need to cancel the VM + * startup on the destination + */ + cancelled = ret < 0 ? 1 : 0; + + finish: + /* + * The status code from the source is passed to the destination. + * The dest can cleanup if the source indicated it failed to + * send all migration data. Returns NULL for ddomain if + * the dest was unable to complete migration. + */ + VIR_DEBUG("Finish3 %p ret=%d", dconn, ret); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 && + virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + domain->name) < 0) { + ddomain = NULL; + } else { + ddomain = dconn->driver->domainMigrateFinish3Params + (dconn, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, destflags, cancelled); + } + + if (cancelled) { + if (ddomain) { + VIR_ERROR(_("finish step ignored that migration was cancelled")); + } else { + /* If Finish reported a useful error, use it instead of the + * original "migration unexpectedly failed" error. + * + * This is ugly but we can't do better with the APIs we have. We + * only replace the error if Finish was called with cancelled == 1 + * and reported a real error (old libvirt would report an error + * from RPC instead of MIGRATE_FINISH_OK), which only happens when + * the domain died on destination. To further reduce a possibility + * of false positives we also check that Perform returned + * VIR_ERR_OPERATION_FAILED. + */ + if (orig_err && + orig_err->domain == VIR_FROM_QEMU && + orig_err->code == VIR_ERR_OPERATION_FAILED) { + virErrorPtr err = virGetLastError(); + if (err && + err->domain == VIR_FROM_QEMU && + err->code != VIR_ERR_MIGRATE_FINISH_OK) { + virFreeError(orig_err); + orig_err = NULL; + } + } + } + } + + /* If ddomain is NULL, then we were unable to start + * the guest on the target, and must restart on the + * source. There is a small chance that the ddomain + * is NULL due to an RPC failure, in which case + * ddomain could in fact be running on the dest. + * The lock manager plugins should take care of + * safety in this scenario. + */ + cancelled = ddomain == NULL ? 1 : 0; + + /* If finish3 set an error, and we don't have an earlier + * one we need to preserve it in case confirm3 overwrites + */ + if (!orig_err) + orig_err = virSaveLastError(); + + /* + * If cancelled, then src VM will be restarted, else it will be killed. + * Don't do this if migration failed on source and thus it was already + * cancelled there. + */ + if (notify_source) { + VIR_DEBUG("Confirm3 %p ret=%d domain=%p", domain->conn, ret, domain); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + ret = domain->conn->driver->domainMigrateConfirm3Params + (domain, params, nparams, cookiein, cookieinlen, + flags, cancelled); + /* If Confirm3 returns -1, there's nothing more we can + * do, but fortunately worst case is that there is a + * domain left in 'paused' state on source. + */ + if (ret < 0) { + VIR_WARN("Guest %s probably left in 'paused' state on source", + domain->name); + } + } + + done: + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + VIR_FREE(dom_xml); + VIR_FREE(uri_out); + VIR_FREE(cookiein); + VIR_FREE(cookieout); + virTypedParamsFree(params, nparams); + virObjectUnref(dconn); + ret = ddomain ? 0 : -1; + virObjectUnref(ddomain); + return ret; +} + +static int +vzDomainMigratePerform3Params(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned int flags) +{ + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + return -1; + + if (flags & VIR_MIGRATE_PEER2PEER) { + return vzDomainMigratePerform3P2PParams(domain, + dconnuri, + params, + nparams, + flags); + } else { + return vzDomainMigratePerform3DirectParams(domain, + params, + nparams, + cookiein, + cookieinlen, + cookieout, + cookieoutlen, + flags); + } +} + + +static virDomainPtr +vzDomainMigrateFinish3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED, + int cancelled ATTRIBUTE_UNUSED) +{ + vzConnPtr privconn = dconn->privateData; + virDomainPtr ret = NULL; + virDomainObjPtr dom; + const char *name; + + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + &name) < 0) + return NULL; + + vzDriverLock(privconn); + dom = virDomainObjListFindByName(privconn->domains, name); + vzDriverUnlock(privconn); + + if (dom == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), name); + goto cleanup; + } + + ret = virGetDomain(dconn, dom->def->name, dom->def->uuid); + if (ret) + ret->id = dom->def->id; + + cleanup: + virDomainObjEndAPI(&dom); return ret; + } + +static int +vzDomainMigrateConfirm3Params(virDomainPtr domain ATTRIBUTE_UNUSED, + virTypedParameterPtr params ATTRIBUTE_UNUSED, + int nparams ATTRIBUTE_UNUSED, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED, + int cancelled ATTRIBUTE_UNUSED) +{ + return 0; +} + + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1651,8 +1998,11 @@ static virHypervisorDriver vzDriver = { .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */ + .domainMigrateBegin3Params = vzDomainMigrateBegin3Params, /* 1.2.20 */ .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */ .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */ + .domainMigrateFinish3Params = vzDomainMigrateFinish3Params, /* 1.2.20 */ + .domainMigrateConfirm3Params = vzDomainMigrateConfirm3Params, /* 1.2.20 */ }; static virConnectDriver vzConnectDriver = { 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 :|
As mentioned in the previous review, I really want to see VZ provide the full set of drive callbacks required by the V3 migration protocol, not just those you happen to need to have today. ie add Begin, Finish & Confirm. This in turn makes it quite easy to support non-P2P mode which is desirable because in P2P mode there is no practical way todo interactive auth for the destination connection, but in non-P2P mode the client app opens the destination connection, so can do auth if needed.
Ok, I just wanted this to be a matter of a different patchset.
I believe that the following changes to this patch should do all that is required. NB, I have only compile tested this, not actually run it, since I don't have an active VZ install, only the SDK install.
For me the patch you suggested is overkill in part of p2p. vzDomainMigratePerform3P2PParams follows virDomainMigrateVersion3Full too close, while vz p2p migration is much simplier that qemu or some abstract case. 1. Confirm phase is noop and this even won't change in future as vz migration is practically unmanaged direct one. So why call it in p2p at all? 2. Finish phase function is to return migrated domain only(in vz case). Thus it have meaning only for direct managed case due its API definition. 3. Calling begin phase in p2p is excessive too. We don't use generated dxml, so this phase is only a stub for direct managed case. Even if we support changing domain xml in future it is more practical to use specific functions and not general API which cases a lot of preparation work and has rather uncomfortable way of passing parameters. I'd better just add a small patch to existent vzDomainMigratePerform3Params which branches on how to get miguri and session_uuid. As to supporting all API versions. I resent a second version of the patch that should cover the case of toURI API so that implementing only version3 params protocol functions would be enough. But the patch does not cover the case of migration{N} API. So I can issue another patch of this sort or go the helpers way as you suggested to Joao Martins. Please take a look at mentioned patch.
This should work functionally identically with & without the PEER2PEER flag being set by the caller.
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 36c64e9..b160a3b 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1398,6 +1398,7 @@ vzParseCookie(const char *xml, unsigned char *session_uuid)
#define VZ_MIGRATION_PARAMETERS \ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ NULL
static char* @@ -1425,49 +1426,6 @@ vzCreateMigrationURI(void) }
static int -vzDomainMigratePrepare3Params(virConnectPtr conn, - virTypedParameterPtr params, - int nparams, - const char *cookiein ATTRIBUTE_UNUSED, - int cookieinlen ATTRIBUTE_UNUSED, - char **cookieout, - int *cookieoutlen, - char **uri_out, - unsigned int flags) -{ - vzConnPtr privconn = conn->privateData; - const char *miguri = NULL; - int ret = -1; - - virCheckFlags(VZ_MIGRATION_FLAGS, -1); - - if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) - goto cleanup; - - if (virTypedParamsGetString(params, nparams, - VIR_MIGRATE_PARAM_URI, &miguri) < 0) - goto cleanup; - - /* This check is of par with direct managed imlementation */ - if (miguri == NULL) - *uri_out = vzCreateMigrationURI(); - - if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) - goto cleanup; - *cookieoutlen = strlen(*cookieout) + 1; - ret = 0; - - cleanup: - if (ret != 0) { - VIR_FREE(*cookieout); - *cookieoutlen = 0; - VIR_FREE(*uri_out); - } - - return ret; -} - -static int vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { switch (feature) { @@ -1479,7 +1437,7 @@ vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) } }
-virURIPtr +static virURIPtr vzParseVzURI(const char *uri_str) { virURIPtr uri = NULL; @@ -1521,64 +1479,128 @@ vzParseVzURI(const char *uri_str) return uri; }
+static char * +vzDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virDomainObjPtr privdom; + char *ret = NULL; + + if (!(privdom = vzDomObjFromDomain(domain))) + goto cleanup; + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (virTypedParamsGet(params, nparams, VIR_MIGRATE_PARAM_DEST_XML)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Changing destination XML is not supported")); + goto cleanup; + } + + /* We don't really need the XML, but the Begin API requires it + * be returned anyway + */ + ret = virDomainDefFormat(privdom->def, VIR_DOMAIN_XML_MIGRATABLE); + + cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; + +} + static int -vzDomainMigratePerform3Params(virDomainPtr domain, - const char *dconnuri, +vzDomainMigratePrepare3Params(virConnectPtr conn, virTypedParameterPtr params, int nparams, const char *cookiein ATTRIBUTE_UNUSED, int cookieinlen ATTRIBUTE_UNUSED, - char **cookieout ATTRIBUTE_UNUSED, - int *cookieoutlen ATTRIBUTE_UNUSED, + char **cookieout, + int *cookieoutlen, + char **uri_out, unsigned int flags) { - int ret = -1; - virDomainObjPtr dom = NULL; - virConnectPtr dconn = NULL; - virURIPtr vzuri = NULL; - unsigned char session_uuid[VIR_UUID_BUFLEN]; - vzConnPtr privconn = domain->conn->privateData; - char *cookie = NULL; - int cookielen = 0; + vzConnPtr privconn = conn->privateData; const char *miguri = NULL; + int ret = -1;
virCheckFlags(VZ_MIGRATION_FLAGS, -1);
if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup;
- if (!(dom = vzDomObjFromDomain(domain))) + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &miguri) < 0) goto cleanup;
- dconn = virConnectOpen(dconnuri); - if (dconn == NULL) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s: %s"), - dconnuri, virGetLastErrorMessage()); + /* This check is of par with direct managed imlementation */ + if (miguri == NULL) + *uri_out = vzCreateMigrationURI(); + + if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) goto cleanup; + *cookieoutlen = strlen(*cookieout) + 1; + ret = 0; + + cleanup: + if (ret != 0) { + VIR_FREE(*cookieout); + *cookieoutlen = 0; + VIR_FREE(*uri_out); }
- if (virDomainMigratePrepare3Params(dconn, params, nparams, NULL, 0, - &cookie, &cookielen, - (char**)&miguri, flags) < 0) + return ret; +} + + +static int +vzDomainMigratePerform3DirectParams(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags) +{ + int ret = -1; + virDomainObjPtr dom = NULL; + virConnectPtr dconn = NULL; + virURIPtr vzuri = NULL; + unsigned char session_uuid[VIR_UUID_BUFLEN]; + vzConnPtr privconn = domain->conn->privateData; + char *defmiguri = NULL; + const char *miguri = NULL; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup;
- if (vzParseCookie(cookie, session_uuid) < 0) + if (!(dom = vzDomObjFromDomain(domain))) goto cleanup;
- if (miguri == NULL && - virTypedParamsGetString(params, nparams, + if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_URI, &miguri) < 0) - goto cleanup; + goto cleanup;
if (miguri == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare3Params did not set miguri")); + goto cleanup; }
if (!(vzuri = vzParseVzURI(miguri))) goto cleanup;
+ if (vzParseCookie(cookiein, session_uuid) < 0) + goto cleanup; + if (prlsdkMigrate(dom, vzuri, session_uuid) < 0) goto cleanup;
@@ -1592,11 +1614,336 @@ vzDomainMigratePerform3Params(virDomainPtr domain, virObjectUnlock(dom); virObjectUnref(dconn); virURIFree(vzuri); - VIR_FREE(cookie); + VIR_FREE(defmiguri); + + return ret; +} + +static int +vzDomainMigratePerform3P2PParams(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virDomainPtr ddomain = NULL; + char *uri_out = NULL; + char *cookiein = NULL; + char *cookieout = NULL; + char *dom_xml = NULL; + int cookieinlen = 0; + int cookieoutlen = 0; + int ret; + virErrorPtr orig_err = NULL; + int cancelled = 1; + bool notify_source = true; + unsigned int destflags; + int state; + virTypedParameterPtr tmp; + const char *uri = NULL; + virConnectPtr dconn = NULL; + + VIR_DOMAIN_DEBUG(domain, + "params=%p, nparams=%d, flags=%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + if (virTypedParamsCopy(&tmp, params, nparams) < 0) + return NULL; + params = tmp; + + if ((dconn = virConnectOpen(dconnuri)) == NULL) + goto done; + + VIR_DEBUG("Begin3 %p", domain->conn); + dom_xml = domain->conn->driver->domainMigrateBegin3Params + (domain, params, nparams, &cookieout, &cookieoutlen, + flags); + if (!dom_xml) + goto done; + + ret = virDomainGetState(domain, &state, NULL, 0); + if (ret == 0 && state == VIR_DOMAIN_PAUSED) + flags |= VIR_MIGRATE_PAUSED; + + destflags = flags & ~(VIR_MIGRATE_ABORT_ON_ERROR | + VIR_MIGRATE_AUTO_CONVERGE); + + VIR_DEBUG("Prepare3 %p flags=%x", dconn, destflags); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + if (virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_DEST_XML, + dom_xml) < 0) + goto done; + ret = dconn->driver->domainMigratePrepare3Params + (dconn, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, &uri_out, destflags); + if (ret == -1) { + goto done; + } + + /* Did domainMigratePrepare3 change URI? */ + if (uri_out) { + uri = uri_out; + if (virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_URI, + uri_out) < 0) { + cancelled = 1; + orig_err = virSaveLastError(); + goto finish; + } + } else if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_URI, &uri) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domainMigratePrepare3 did not set uri")); + cancelled = 1; + orig_err = virSaveLastError(); + goto finish; + } + + if (flags & VIR_MIGRATE_OFFLINE) { + VIR_DEBUG("Offline migration, skipping Perform phase"); + VIR_FREE(cookieout); + cookieoutlen = 0; + cancelled = 0; + goto finish; + } + + /* Perform the migration. The driver isn't supposed to return + * until the migration is complete. The src VM should remain + * running, but in paused state until the destination can + * confirm migration completion. + */ + VIR_DEBUG("Perform3 %p uri=%s", domain->conn, uri); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + /* dconnuri not relevant in non-P2P modes, so left NULL here */ + ret = domain->conn->driver->domainMigratePerform3Params + (domain, NULL, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, flags & ~(VIR_MIGRATE_PEER2PEER)); + + /* Perform failed. Make sure Finish doesn't overwrite the error */ + if (ret < 0) { + orig_err = virSaveLastError(); + /* Perform failed so we don't need to call confirm to let source know + * about the failure. + */ + notify_source = false; + } + + /* If Perform returns < 0, then we need to cancel the VM + * startup on the destination + */ + cancelled = ret < 0 ? 1 : 0; + + finish: + /* + * The status code from the source is passed to the destination. + * The dest can cleanup if the source indicated it failed to + * send all migration data. Returns NULL for ddomain if + * the dest was unable to complete migration. + */ + VIR_DEBUG("Finish3 %p ret=%d", dconn, ret); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 && + virTypedParamsReplaceString(¶ms, &nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + domain->name) < 0) { + ddomain = NULL; + } else { + ddomain = dconn->driver->domainMigrateFinish3Params + (dconn, params, nparams, cookiein, cookieinlen, + &cookieout, &cookieoutlen, destflags, cancelled); + } + + if (cancelled) { + if (ddomain) { + VIR_ERROR(_("finish step ignored that migration was cancelled")); + } else { + /* If Finish reported a useful error, use it instead of the + * original "migration unexpectedly failed" error. + * + * This is ugly but we can't do better with the APIs we have. We + * only replace the error if Finish was called with cancelled == 1 + * and reported a real error (old libvirt would report an error + * from RPC instead of MIGRATE_FINISH_OK), which only happens when + * the domain died on destination. To further reduce a possibility + * of false positives we also check that Perform returned + * VIR_ERR_OPERATION_FAILED. + */ + if (orig_err && + orig_err->domain == VIR_FROM_QEMU && + orig_err->code == VIR_ERR_OPERATION_FAILED) { + virErrorPtr err = virGetLastError(); + if (err && + err->domain == VIR_FROM_QEMU && + err->code != VIR_ERR_MIGRATE_FINISH_OK) { + virFreeError(orig_err); + orig_err = NULL; + } + } + } + } + + /* If ddomain is NULL, then we were unable to start + * the guest on the target, and must restart on the + * source. There is a small chance that the ddomain + * is NULL due to an RPC failure, in which case + * ddomain could in fact be running on the dest. + * The lock manager plugins should take care of + * safety in this scenario. + */ + cancelled = ddomain == NULL ? 1 : 0; + + /* If finish3 set an error, and we don't have an earlier + * one we need to preserve it in case confirm3 overwrites + */ + if (!orig_err) + orig_err = virSaveLastError(); + + /* + * If cancelled, then src VM will be restarted, else it will be killed. + * Don't do this if migration failed on source and thus it was already + * cancelled there. + */ + if (notify_source) { + VIR_DEBUG("Confirm3 %p ret=%d domain=%p", domain->conn, ret, domain); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + ret = domain->conn->driver->domainMigrateConfirm3Params + (domain, params, nparams, cookiein, cookieinlen, + flags, cancelled); + /* If Confirm3 returns -1, there's nothing more we can + * do, but fortunately worst case is that there is a + * domain left in 'paused' state on source. + */ + if (ret < 0) { + VIR_WARN("Guest %s probably left in 'paused' state on source", + domain->name); + } + } + + done: + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + VIR_FREE(dom_xml); + VIR_FREE(uri_out); + VIR_FREE(cookiein); + VIR_FREE(cookieout); + virTypedParamsFree(params, nparams); + virObjectUnref(dconn); + ret = ddomain ? 0 : -1; + virObjectUnref(ddomain); + return ret; +}
+ +static int +vzDomainMigratePerform3Params(virDomainPtr domain, + const char *dconnuri, + virTypedParameterPtr params, + int nparams, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned int flags) +{ + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + return -1; + + if (flags & VIR_MIGRATE_PEER2PEER) { + return vzDomainMigratePerform3P2PParams(domain, + dconnuri, + params, + nparams, + flags); + } else { + return vzDomainMigratePerform3DirectParams(domain, + params, + nparams, + cookiein, + cookieinlen, + cookieout, + cookieoutlen, + flags); + } +} + + +static virDomainPtr +vzDomainMigrateFinish3Params(virConnectPtr dconn, + virTypedParameterPtr params, + int nparams, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout ATTRIBUTE_UNUSED, + int *cookieoutlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED, + int cancelled ATTRIBUTE_UNUSED) +{ + vzConnPtr privconn = dconn->privateData; + virDomainPtr ret = NULL; + virDomainObjPtr dom; + const char *name; + + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + &name) < 0) + return NULL; + + vzDriverLock(privconn); + dom = virDomainObjListFindByName(privconn->domains, name); + vzDriverUnlock(privconn); + + if (dom == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), name); + goto cleanup; + } + + ret = virGetDomain(dconn, dom->def->name, dom->def->uuid); + if (ret) + ret->id = dom->def->id; + + cleanup: + virDomainObjEndAPI(&dom); return ret; + }
+ +static int +vzDomainMigrateConfirm3Params(virDomainPtr domain ATTRIBUTE_UNUSED, + virTypedParameterPtr params ATTRIBUTE_UNUSED, + int nparams ATTRIBUTE_UNUSED, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED, + int cancelled ATTRIBUTE_UNUSED) +{ + return 0; +} + + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1651,8 +1998,11 @@ static virHypervisorDriver vzDriver = { .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */ + .domainMigrateBegin3Params = vzDomainMigrateBegin3Params, /* 1.2.20 */ .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */ .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */ + .domainMigrateFinish3Params = vzDomainMigrateFinish3Params, /* 1.2.20 */ + .domainMigrateConfirm3Params = vzDomainMigrateConfirm3Params, /* 1.2.20 */ };
static virConnectDriver vzConnectDriver = {
Regards, Daniel
On Fri, Sep 11, 2015 at 12:05:54PM +0300, Nikolay Shirokovskiy wrote:
As mentioned in the previous review, I really want to see VZ provide the full set of drive callbacks required by the V3 migration protocol, not just those you happen to need to have today. ie add Begin, Finish & Confirm. This in turn makes it quite easy to support non-P2P mode which is desirable because in P2P mode there is no practical way todo interactive auth for the destination connection, but in non-P2P mode the client app opens the destination connection, so can do auth if needed.
Ok, I just wanted this to be a matter of a different patchset.
I believe that the following changes to this patch should do all that is required. NB, I have only compile tested this, not actually run it, since I don't have an active VZ install, only the SDK install.
For me the patch you suggested is overkill in part of p2p. vzDomainMigratePerform3P2PParams follows virDomainMigrateVersion3Full too close, while vz p2p migration is much simplier that qemu or some abstract case.
As you can see from the fact that we have many versions of the migration protocol, what appears obviously sufficient when first implmented, has frequently turned out to be insufficient. So while you only need 2 calls today, I would not bet on that always being sufficient in the future. Implementing all phases today provides better future proofing, should we need to make use of them in the future, as it allows the potential to implement fixes / workarounds on the dest, without relying on also lock-step upgrading the host to add the callers you didn't make to start with. It would also be desirable in the future, to provide the means to share the code impl for P2P migration across all the hypervisor drivers. This would again imply supporting all stages of the v3 migration protocol. If we don't do this now we will create problems in the future if we wish to change to shared code, because any shared impl would be calling functions that were not implemented on older versions of VZ.
1. Confirm phase is noop and this even won't change in future as vz migration is practically unmanaged direct one. So why call it in p2p at all?
2. Finish phase function is to return migrated domain only(in vz case). Thus it have meaning only for direct managed case due its API definition.
3. Calling begin phase in p2p is excessive too. We don't use generated dxml, so this phase is only a stub for direct managed case.
Even if we support changing domain xml in future it is more practical to use specific functions and not general API which cases a lot of preparation work and has rather uncomfortable way of passing parameters.
I'd better just add a small patch to existent vzDomainMigratePerform3Params which branches on how to get miguri and session_uuid.
As to supporting all API versions. I resent a second version of the patch that should cover the case of toURI API so that implementing only version3 params protocol functions would be enough. But the patch does not cover the case of migration{N} API.
This patch I have provided covers the normal virDomainMigrate API, in addition to the virDomainMigrateToURI method. As I mentioned, supporting virDomainMigrate is desirable, because it allows the calling application to perform arbitrary authentication with the destination libvirtd, which is not possible when using P2P migration. Supporting the non-P2P migration mode requires that we provide all the callbacks defined for version 3, and once we do this, it is sensible to support all the callbacks in the P2P case too, to ensure we have identical functional behaviour in both cases. So in summary, I want to see the full V3 migration protocol implemented in VZ before this is acceptable for merge. This patch I supplied would be sufficient to achieve this. 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 11.09.2015 12:28, Daniel P. Berrange wrote:
On Fri, Sep 11, 2015 at 12:05:54PM +0300, Nikolay Shirokovskiy wrote:
As mentioned in the previous review, I really want to see VZ provide the full set of drive callbacks required by the V3 migration protocol, not just those you happen to need to have today. ie add Begin, Finish & Confirm. This in turn makes it quite easy to support non-P2P mode which is desirable because in P2P mode there is no practical way todo interactive auth for the destination connection, but in non-P2P mode the client app opens the destination connection, so can do auth if needed.
Ok, I just wanted this to be a matter of a different patchset.
I believe that the following changes to this patch should do all that is required. NB, I have only compile tested this, not actually run it, since I don't have an active VZ install, only the SDK install.
For me the patch you suggested is overkill in part of p2p. vzDomainMigratePerform3P2PParams follows virDomainMigrateVersion3Full too close, while vz p2p migration is much simplier that qemu or some abstract case.
As you can see from the fact that we have many versions of the migration protocol, what appears obviously sufficient when first implmented, has frequently turned out to be insufficient. So while you only need 2 calls today, I would not bet on that always being sufficient in the future. Implementing all phases today provides better future proofing, should we need to make use of them in the future, as it allows the potential to implement fixes / workarounds on the dest, without relying on also lock-step upgrading the host to add the callers you didn't make to start with.
It would also be desirable in the future, to provide the means to share the code impl for P2P migration across all the hypervisor drivers. This would again imply supporting all stages of the v3 migration protocol. If we don't do this now we will create problems in the future if we wish to change to shared code, because any shared impl would be calling functions that were not implemented on older versions of VZ.
Well I'm not against implementing the full set of protocol functions. This will be done anyway as a part of supporting direct managed migrating scheme as you recommend. So it is not a point of dispute. The point of dispute is what p2p migration should look like. Well I understand the argument of ease of fixes/workaronds. But if there is even a disire in future share p2p code why not to do it from the start? Look vzDomainMigratePerform3P2PParams is identical to virDomainMigrateVersion3Full except for that last one supports plain version 3 protocol. Can we reuse virDomainMigrateVersion3Full? Thus at least for vz p2p and direct managed will be identical from procedural POV, the only difference is who executes the migration procedure - client or daemon. So the last statement will be not only a declaration but implemented in code.
1. Confirm phase is noop and this even won't change in future as vz migration is practically unmanaged direct one. So why call it in p2p at all?
2. Finish phase function is to return migrated domain only(in vz case). Thus it have meaning only for direct managed case due its API definition.
3. Calling begin phase in p2p is excessive too. We don't use generated dxml, so this phase is only a stub for direct managed case.
Even if we support changing domain xml in future it is more practical to use specific functions and not general API which cases a lot of preparation work and has rather uncomfortable way of passing parameters.
I'd better just add a small patch to existent vzDomainMigratePerform3Params which branches on how to get miguri and session_uuid.
As to supporting all API versions. I resent a second version of the patch that should cover the case of toURI API so that implementing only version3 params protocol functions would be enough. But the patch does not cover the case of migration{N} API.
Looks like you misunderstood me. Here i just wanted you to review https://www.redhat.com/archives/libvir-list/2015-September/msg00397.html patch. and estimate it as an alternative to helpers methods you suggested in https://www.redhat.com/archives/libvir-list/2015-September/msg00372.html.
This patch I have provided covers the normal virDomainMigrate API, in addition to the virDomainMigrateToURI method. As I mentioned, supporting virDomainMigrate is desirable, because it allows the calling application to perform arbitrary authentication with the destination libvirtd, which is not possible when using P2P migration. Supporting the non-P2P migration mode requires that we provide all the callbacks defined for version 3, and once we do this, it is sensible to support all the callbacks in the P2P case too, to ensure we have identical functional behaviour in both cases.
So in summary, I want to see the full V3 migration protocol implemented in VZ before this is acceptable for merge. This patch I supplied would be sufficient to achieve this.
Regards, Daniel
In short, may be factor out common p2p code that now resides in libvirt-domain.c and reuse it for vz? Looks like vz driver is not a good place for such common and kinda complicated code. On 11.09.2015 14:06, Nikolay Shirokovskiy wrote:
On 11.09.2015 12:28, Daniel P. Berrange wrote:
On Fri, Sep 11, 2015 at 12:05:54PM +0300, Nikolay Shirokovskiy wrote:
As mentioned in the previous review, I really want to see VZ provide the full set of drive callbacks required by the V3 migration protocol, not just those you happen to need to have today. ie add Begin, Finish & Confirm. This in turn makes it quite easy to support non-P2P mode which is desirable because in P2P mode there is no practical way todo interactive auth for the destination connection, but in non-P2P mode the client app opens the destination connection, so can do auth if needed.
Ok, I just wanted this to be a matter of a different patchset.
I believe that the following changes to this patch should do all that is required. NB, I have only compile tested this, not actually run it, since I don't have an active VZ install, only the SDK install.
For me the patch you suggested is overkill in part of p2p. vzDomainMigratePerform3P2PParams follows virDomainMigrateVersion3Full too close, while vz p2p migration is much simplier that qemu or some abstract case.
As you can see from the fact that we have many versions of the migration protocol, what appears obviously sufficient when first implmented, has frequently turned out to be insufficient. So while you only need 2 calls today, I would not bet on that always being sufficient in the future. Implementing all phases today provides better future proofing, should we need to make use of them in the future, as it allows the potential to implement fixes / workarounds on the dest, without relying on also lock-step upgrading the host to add the callers you didn't make to start with.
It would also be desirable in the future, to provide the means to share the code impl for P2P migration across all the hypervisor drivers. This would again imply supporting all stages of the v3 migration protocol. If we don't do this now we will create problems in the future if we wish to change to shared code, because any shared impl would be calling functions that were not implemented on older versions of VZ.
Well I'm not against implementing the full set of protocol functions. This will be done anyway as a part of supporting direct managed migrating scheme as you recommend. So it is not a point of dispute.
The point of dispute is what p2p migration should look like. Well I understand the argument of ease of fixes/workaronds. But if there is even a disire in future share p2p code why not to do it from the start?
Look vzDomainMigratePerform3P2PParams is identical to virDomainMigrateVersion3Full except for that last one supports plain version 3 protocol. Can we reuse virDomainMigrateVersion3Full? Thus at least for vz p2p and direct managed will be identical from procedural POV, the only difference is who executes the migration procedure - client or daemon. So the last statement will be not only a declaration but implemented in code.
1. Confirm phase is noop and this even won't change in future as vz migration is practically unmanaged direct one. So why call it in p2p at all?
2. Finish phase function is to return migrated domain only(in vz case). Thus it have meaning only for direct managed case due its API definition.
3. Calling begin phase in p2p is excessive too. We don't use generated dxml, so this phase is only a stub for direct managed case.
Even if we support changing domain xml in future it is more practical to use specific functions and not general API which cases a lot of preparation work and has rather uncomfortable way of passing parameters.
I'd better just add a small patch to existent vzDomainMigratePerform3Params which branches on how to get miguri and session_uuid.
As to supporting all API versions. I resent a second version of the patch that should cover the case of toURI API so that implementing only version3 params protocol functions would be enough. But the patch does not cover the case of migration{N} API.
Looks like you misunderstood me. Here i just wanted you to review https://www.redhat.com/archives/libvir-list/2015-September/msg00397.html patch. and estimate it as an alternative to helpers methods you suggested in https://www.redhat.com/archives/libvir-list/2015-September/msg00372.html.
This patch I have provided covers the normal virDomainMigrate API, in addition to the virDomainMigrateToURI method. As I mentioned, supporting virDomainMigrate is desirable, because it allows the calling application to perform arbitrary authentication with the destination libvirtd, which is not possible when using P2P migration. Supporting the non-P2P migration mode requires that we provide all the callbacks defined for version 3, and once we do this, it is sensible to support all the callbacks in the P2P case too, to ensure we have identical functional behaviour in both cases.
So in summary, I want to see the full V3 migration protocol implemented in VZ before this is acceptable for merge. This patch I supplied would be sufficient to achieve this.
Regards, Daniel
From: nshirokovskiy@virtuozzo.com <nshirokovskiy@virtuozzo.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 9 ++++++++- src/vz/vz_sdk.c | 16 +++++++++------- src/vz/vz_sdk.h | 5 ++++- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 36c64e9..e9ca611 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1398,6 +1398,7 @@ vzParseCookie(const char *xml, unsigned char *session_uuid) #define VZ_MIGRATION_PARAMETERS \ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \ NULL static char* @@ -1541,12 +1542,18 @@ vzDomainMigratePerform3Params(virDomainPtr domain, char *cookie = NULL; int cookielen = 0; const char *miguri = NULL; + const char *dname = NULL; virCheckFlags(VZ_MIGRATION_FLAGS, -1); if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup; + if (virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_NAME, + &dname) < 0) + goto cleanup; + if (!(dom = vzDomObjFromDomain(domain))) goto cleanup; @@ -1579,7 +1586,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain, if (!(vzuri = vzParseVzURI(miguri))) goto cleanup; - if (prlsdkMigrate(dom, vzuri, session_uuid) < 0) + if (prlsdkMigrate(dom, vzuri, session_uuid, dname) < 0) goto cleanup; virDomainObjListRemove(privconn->domains, dom); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 783438d..89a2429 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4064,7 +4064,8 @@ prlsdkGetMemoryStats(virDomainObjPtr dom, #define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY) int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, - const unsigned char *session_uuid) + const unsigned char *session_uuid, + const char *dname) { int ret = -1; vzDomObjPtr privdom = dom->privateData; @@ -4072,12 +4073,13 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; prlsdkUUIDFormat(session_uuid, uuidstr); - job = PrlVm_MigrateEx(privdom->sdkdom, uri->server, uri->port, uuidstr, - "", /* use default dir for migrated instance bundle */ - PRLSDK_MIGRATION_FLAGS, - 0, /* reserved flags */ - PRL_TRUE /* don't ask for confirmations */ - ); + job = PrlVm_MigrateWithRenameEx(privdom->sdkdom, uri->server, uri->port, uuidstr, + dname == NULL ? "" : dname, + "", /* use default dir for migrated instance bundle */ + PRLSDK_MIGRATION_FLAGS, + 0, /* reserved flags */ + PRL_TRUE /* don't ask for confirmations */ + ); if (PRL_FAILED(waitJob(job))) goto cleanup; diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index d3f0caf..0aa70b3 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -77,4 +77,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time); int prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats); int -prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned *session_uuid); +prlsdkMigrate(virDomainObjPtr dom, + virURIPtr uri, + const char unsigned *session_uuid, + const char *dname); -- 1.7.1
From: nshirokovskiy@virtuozzo.com <nshirokovskiy@virtuozzo.com> Migration API has a lot of options. This patch intention is to provide support for those options that can be trivially supported and give estimation for other options support in this commit message. I. Supported. 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain memory'. It is supported but quite uncommon way: vz migration demands that this option should be set. This is due to vz is hardcoded to moving VMs memory using compression. So anyone who wants to migrate vz domain should set this option thus declaring it knows it uses compression. Why bother? May be just support this option and ignore if it is not set or don't support at all as we can't change behaviour in this aspect. Well I believe that this option is, first, inherent to hypervisor implementation as we have a task of moving domain memory to different place and, second, we have a tradeoff here between cpu and network resources and some managment should choose the stratery via this option. If we choose ignoring or unsupporting implementation than this option has a too vague meaning. Let's go into more detail. First if we ignore situation where option is not set than we put user into fallacy that vz hypervisor don't use compression and thus have lower cpu usage. Second approach is to not support the option. The main reason not to follow this way is that 'not supported and not set' is indistinguishable from 'supported and not set' and thus again fool the user. 2. VIR_MIGRATE_LIVE. Means 'don't pause domain before migration'. Vz doesn give this option but we can emulate it easily making pause/unpause in libvirt itself. However as this is rather unnatural addon its implementation is delayed until futher request. So this option is demanded. One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these flags and always use live migration. 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes together. Vz domain are always persistent so we have to demand that options VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set. 4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on destination'. This is trivially supported as we have a corresponding option in vz migration. 5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It is a forcing option that is it is ignored if domain is running and must be set to migrate stopped domain. Vz implemenation follows this unformal definition with one exception: non-shared disks will be migrated too. This desicion is on par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this notes). All that said the minimal command to migrate vz domain looks like next: migrate $DOMAIN $DESTINATION --compressed --live --persistent --undefinesource Not good. Say if you want to just migrate a domain without further details you will get error messages until you add these options to command line. I think there is a lack of notion 'default' behaviour in all these aspects. If we have it we could just issue: migrate $DOMAIN $DESTINATION For vz this would give default compression for example, for qemu - default no-compression. Then we could have flags --compressed and -no-compressed and for vz the latter would give unsupported error. II. Unsupported. 1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this is not reflected in current version of vz driver and will be fixed soon). So we need not to support this option. 2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no appopriate support from vz sdk. Although we have locks they are advisory and cant help us. 3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd connection to pass hypervisor migration traffic'. Unsupported as not among vz hypervisor usecases. 4. Direct migration. Which is exposed via *toURI* interface with VIR_MIGRATE_PEER2PEER flag unset. Means 'migrate without using libvirtd on the other side'. To support it we should add authN means to vz driver as mentioned in 'backbone patch' which looks ugly. 5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE, VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC, VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH, VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_MIGRATE_PARAM_MIGRATE_DISKS. Without further discussion. They are just not usecases of vz hypevisor. III. Incompatible options. 6. VIR_MIGRATE_NON_SHARED_DISK. Means 'force to migrate if domain has non-shared disks'. Without this option we should refuse to migrate with non-shared disks. Unfortunately this behaviour is incompatible with vz as vz doesn't demand any user awareness of disk sharedness. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++-- src/vz/vz_sdk.c | 8 +++- src/vz/vz_sdk.h | 3 +- 3 files changed, 104 insertions(+), 7 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index e9ca611..d708ca4 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1394,7 +1394,14 @@ vzParseCookie(const char *xml, unsigned char *session_uuid) return ret; } -#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER) +#define VZ_MIGRATION_FLAGS \ + (VIR_MIGRATE_PEER2PEER | \ + VIR_MIGRATE_OFFLINE | \ + VIR_MIGRATE_LIVE | \ + VIR_MIGRATE_COMPRESSED | \ + VIR_MIGRATE_PERSIST_DEST | \ + VIR_MIGRATE_UNDEFINE_SOURCE | \ + VIR_MIGRATE_PAUSED) #define VZ_MIGRATION_PARAMETERS \ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ @@ -1522,6 +1529,82 @@ vzParseVzURI(const char *uri_str) return uri; } +/* TODO this code should be in common place as these + rules follows from options (informal) definitions. + Qemu makes some of these checks on begin phase but not all. */ +int vzCheckOfflineFlags(int flags) +{ + if (!(flags & VIR_MIGRATE_OFFLINE)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + return -1; + } + + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("live offline migration does not " + "make sense")); + return -1; + } + + if (flags & VIR_MIGRATE_COMPRESSED) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("compressed offline migration does not " + "make sense")); + return -1; + } + + if (flags & VIR_MIGRATE_PAUSED) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("paused offline migration does not " + "make sense")); + return -1; + } + + return 0; +} + +int vzCheckCommonFlags(int flags) +{ + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (!(flags & VIR_MIGRATE_PERSIST_DEST)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flags VIR_MIGRATE_PERSIST_DEST must be set" + " for vz migration")); + return -1; + } + + if (!(flags & VIR_MIGRATE_UNDEFINE_SOURCE)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flags VIR_MIGRATE_UNDEFINE_SOURCE must be set" + " for vz migration")); + return -1; + } + + return 0; +} + +int vzCheckOnlineFlags(int flags) +{ + + if (!(flags & VIR_MIGRATE_LIVE)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flags VIR_MIGRATE_LIVE must be set" + " for online vz migration")); + return -1; + } + + if (!(flags & VIR_MIGRATE_COMPRESSED)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flags VIR_MIGRATE_COMPRESSED must be set" + " for online vz migration")); + return -1; + } + + return 0; +} + static int vzDomainMigratePerform3Params(virDomainPtr domain, const char *dconnuri, @@ -1533,7 +1616,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain, int *cookieoutlen ATTRIBUTE_UNUSED, unsigned int flags) { - int ret = -1; + int ret = -1, tmpret; virDomainObjPtr dom = NULL; virConnectPtr dconn = NULL; virURIPtr vzuri = NULL; @@ -1544,7 +1627,8 @@ vzDomainMigratePerform3Params(virDomainPtr domain, const char *miguri = NULL; const char *dname = NULL; - virCheckFlags(VZ_MIGRATION_FLAGS, -1); + if (vzCheckCommonFlags(flags)) + goto cleanup; if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) goto cleanup; @@ -1557,6 +1641,14 @@ vzDomainMigratePerform3Params(virDomainPtr domain, if (!(dom = vzDomObjFromDomain(domain))) goto cleanup; + if (virDomainObjIsActive(dom)) + tmpret = vzCheckOnlineFlags(flags); + else + tmpret = vzCheckOfflineFlags(flags); + + if (tmpret < 0) + goto cleanup; + dconn = virConnectOpen(dconnuri); if (dconn == NULL) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -1586,7 +1678,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain, if (!(vzuri = vzParseVzURI(miguri))) goto cleanup; - if (prlsdkMigrate(dom, vzuri, session_uuid, dname) < 0) + if (prlsdkMigrate(dom, vzuri, session_uuid, dname, flags) < 0) goto cleanup; virDomainObjListRemove(privconn->domains, dom); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 89a2429..9a2b5df 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4065,18 +4065,22 @@ prlsdkGetMemoryStats(virDomainObjPtr dom, int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const unsigned char *session_uuid, - const char *dname) + const char *dname, unsigned int flags) { int ret = -1; vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + PRL_UINT32 vzflags = PRLSDK_MIGRATION_FLAGS; + + if (flags & VIR_MIGRATE_PAUSED) + vzflags |= PVMT_DONT_RESUME_VM; prlsdkUUIDFormat(session_uuid, uuidstr); job = PrlVm_MigrateWithRenameEx(privdom->sdkdom, uri->server, uri->port, uuidstr, dname == NULL ? "" : dname, "", /* use default dir for migrated instance bundle */ - PRLSDK_MIGRATION_FLAGS, + vzflags, 0, /* reserved flags */ PRL_TRUE /* don't ask for confirmations */ ); diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 0aa70b3..5b26b70 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -80,4 +80,5 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned *session_uuid, - const char *dname); + const char *dname, + unsigned int flags); -- 1.7.1
vz puts uuids into curly braces. Simply introduce new contstant to reflect this and get rid of magic +2 in code. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_sdk.c | 12 ++++++------ src/vz/vz_utils.h | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 9a2b5df..bafc6e4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -236,7 +236,7 @@ prlsdkConnect(vzConnPtr privconn) PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; PRL_HANDLE response = PRL_INVALID_HANDLE; - char session_uuid[VIR_UUID_STRING_BUFLEN + 2]; + char session_uuid[VZ_UUID_STRING_BUFLEN]; PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid); pret = PrlSrv_Create(&privconn->server); @@ -316,7 +316,7 @@ prlsdkUUIDFormat(const unsigned char *uuid, char *uuidstr) static PRL_HANDLE prlsdkSdkDomainLookupByUUID(vzConnPtr privconn, const unsigned char *uuid) { - char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + char uuidstr[VZ_UUID_STRING_BUFLEN]; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; prlsdkUUIDFormat(uuid, uuidstr); @@ -365,7 +365,7 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom, char **name, unsigned char *uuid) { - char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + char uuidstr[VZ_UUID_STRING_BUFLEN]; PRL_UINT32 len; PRL_RESULT pret; @@ -1722,7 +1722,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) vzConnPtr privconn = opaque; PRL_RESULT pret = PRL_ERR_FAILURE; PRL_HANDLE_TYPE handleType; - char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + char uuidstr[VZ_UUID_STRING_BUFLEN]; unsigned char uuid[VIR_UUID_BUFLEN]; PRL_UINT32 bufsize = ARRAY_CARDINALITY(uuidstr); PRL_EVENT_TYPE prlEventType; @@ -3480,7 +3480,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, { PRL_RESULT pret; size_t i; - char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + char uuidstr[VZ_UUID_STRING_BUFLEN]; bool needBoot = true; char *mask = NULL; @@ -4070,7 +4070,7 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, int ret = -1; vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; - char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; + char uuidstr[VZ_UUID_STRING_BUFLEN]; PRL_UINT32 vzflags = PRLSDK_MIGRATION_FLAGS; if (flags & VIR_MIGRATE_PAUSED) diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index fe54b25..2a59426 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -55,6 +55,8 @@ # define PARALLELS_REQUIRED_BRIDGED_NETWORK "Bridged" # define PARALLELS_BRIDGED_NETWORK_TYPE "bridged" +# define VZ_UUID_STRING_BUFLEN (VIR_UUID_STRING_BUFLEN + 2) + struct _vzConn { virMutex lock; -- 1.7.1
04.09.2015 17:18, Nikolay Shirokovskiy пишет:
NOTE that minimal command to migrate vz domain is like next:
virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p --live --compressed --persistent --undefinesource
Difference from v4:
1. move preparation of the migration uri from src to dst as dst intended to do it. 2. change hypervisor migration scheme from 'tcp' to 'vzmigr' 3. declare this scheme in host caps
src/vz/vz_driver.c | 356 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 86 +++++++++++-- src/vz/vz_sdk.h | 6 + src/vz/vz_utils.h | 4 +- 4 files changed, 437 insertions(+), 15 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Have you forgotten to send the #2 patch of the series?
04.09.2015 17:48, Maxim Nestratov пишет:
04.09.2015 17:18, Nikolay Shirokovskiy пишет:
NOTE that minimal command to migrate vz domain is like next:
virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p --live --compressed --persistent --undefinesource
Difference from v4:
1. move preparation of the migration uri from src to dst as dst intended to do it. 2. change hypervisor migration scheme from 'tcp' to 'vzmigr' 3. declare this scheme in host caps
src/vz/vz_driver.c | 356 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 86 +++++++++++-- src/vz/vz_sdk.h | 6 + src/vz/vz_utils.h | 4 +- 4 files changed, 437 insertions(+), 15 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Have you forgotten to send the #2 patch of the series?
Please disregard. Just got it, though with a long delay.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (4)
-
Daniel P. Berrange -
Maxim Nestratov -
Nikolay Shirokovskiy -
Nikolay Shirokovskiy