Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On 04.09.2015 11:40, Daniel P. Berrange wrote:
> On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy wrote:
@@ -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?
>
> Yeah, that'd be fine as long as we still compile at each step it isn't
> a problem if the impl is not final.
Ok. Then I'll send patch supporting toURI{1, 2} in a different patchset as it
will
be a new thing that could lead to meaningless ping-ponging of parts
already areed upon.
>
>
> Regards,
> Daniel
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy wrote:
> >> @@ -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?
Yeah, that'd be fine as long as we still compile at each step it isn't
a problem if the impl is not final.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On 03.09.2015 19:45, Daniel P. Berrange wrote:
> On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote:
>> From: nshirokovs...@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
>> ---
>> 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
> th
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: nshirokovs...@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
> ---
> 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(pri
