On Wed, Sep 02, 2015 at 03:09:23PM +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 | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 33 +++++++++ src/vz/vz_sdk.h | 2 + 3 files changed, 227 insertions(+), 0 deletions(-)
+static int +vzDomainMigratePrepare3Params(virConnectPtr conn, + virTypedParameterPtr params ATTRIBUTE_UNUSED, + int nparams ATTRIBUTE_UNUSED, + const char *cookiein ATTRIBUTE_UNUSED, + int cookieinlen ATTRIBUTE_UNUSED, + char **cookieout, + int *cookieoutlen, + char **uri_out ATTRIBUTE_UNUSED, + unsigned int flags) +{ + vzConnPtr privconn = conn->privateData; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(*cookieout = vzFormatCookie(privconn->session_uuid))) + goto cleanup; + *cookieoutlen = strlen(*cookieout) + 1;
As mentioned before, you should fill in uri_out here with the hypervisor URI to use for migration, by filling in the URI of the current host (ie dest host). eg
char *thishost = virGetHostname(); virAsprintf(uri_out, "tcp://%s", thishost); VIR_FREE(thishost);
+ ret = 0; + + cleanup: + if (ret != 0) { + VIR_FREE(*cookieout); + *cookieoutlen = 0; + } + + return ret; +} +
+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; + + virCheckFlags(VZ_MIGRATION_FLAGS, -1); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (!(vzuri = vzMakeVzUri(dconnuri))) + goto cleanup;
This is not right - you can't use the libvirt URI to form the hypervisor migration URI, since the libvirt URI may not in fact refer to the hypervisor host.
eg people may be accessing libvirt(d) via a SSH tunnel in which case the dconnuri would include 'localhost' and not the actual target host. This is why you must fill in the uri_out parameter in the Prepare() method and use that, if the "migrateuri" parameter is not provided in the virTypedParams array.
+ + 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; + } + + /* NULL and zero elements are unused */ + if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0, + &cookie, &cookielen, NULL, 0) < 0) + goto cleanup;
Since this is implementing v3, I'd prefer to see you provide the full set of 5 callbacks, even though they will currently be no-ops. This provides better future proofing for the migration impl in case those become neccessary later.
You can also then trivially implement the non-p2p plain migration in this method, by checking whether or not the PEER2PEER flag is set or not. If it is not set, you can just skip the connect open & prepare calls on the basis that libvirt will have done them for you. Ok.
+ + if (vzParseCookie(cookie, session_uuid) < 0) + 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 +1585,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 */
Somewhat annoyingly you also need to implement the callbacks for .domainMigratePrepare3 and .domainMigratePerform3, as we don't automatically convert non-params usage to the params based method AFAICT.
Your impl of .domainMigratePerform3 could pack the values into a virTypedParams array and then call .domainMigratePerform3Params, or do the reverse. Yes, without plain(non-params) callbacks we get working only toURI3 API function and I create a patch not included in this patchset to make toURI{1,2} work too. I take this approach of converting
On 03.09.2015 19:45, Daniel P. Berrange wrote: parameters and use one common worker function but patch a different place - API implementaion itself. So I'll include this patch in next version of the set. As in this case I need to patch 2 different sets of API implementation *migrate{N} and *migrateURI{N} I'd rather put direct managed support to a different patchset. Is it ok?
Regards, Daniel