Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code

On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote: > From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com&gt; > > 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(a)virtuozzo.com&gt; > --- > 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.

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

Regards, Daniel