On 03.09.2015 19:45, Daniel P. Berrange wrote:
> On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote:
>> From: nshirokovskiy@xxxxxxxxxxxxx <nshirokovskiy@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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
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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list