Re: [libvirt] [PATCH] libxl: advertise support for migration V3

[Joao Martins]

On 08/30/2016 06:21 PM, Jim Fehlig wrote:
> On 08/30/2016 08:45 AM, Joao Martins wrote:
>> On 08/29/2016 06:20 PM, Jim Fehlig wrote:
>>> The libxl driver has long supported migration V3 but has never
>>> indicated so in the connectSupportsFeature API. 
>> Hmm, but IIRC it was only since "recent" commit 8db77b3 right?
> 
> The libxl driver has supported the VIR_DRV_FEATURE_MIGRATION_PARAMS V3 family 
> of
> migration functions since commit 9b8d6e1e in May 2014.
Indeed.

>>  Or rather Nikolay's
>> reworking top-level migration code, which effectively would convert to the 
>> params
>> versions accordingly. Before that rework I think one had to implement both 
>> params and
>> non-params variants. Would it be worth adding a comment mainly to help the 
>> reader?
> 
> I think you are right, but support for V3 is independent of the params vs
> non-params variants. virt-manager uses the generic virDomainMigrate() function
> in src/libvirt-domain.c, and at least as far back as libvirt 1.2.5 that 
> function
> tests whether both source and destination support 
> VIR_DRV_FEATURE_MIGRATION_V{1,2,3}. If the driver doesn't advertise support 
> for
> any of the versions, virDomainMigrate() returns VIR_ERR_NO_SUPPORT.
Hmm, OK. IIRC the problems I once observed where more around 
virDomainMigrateToURI -
which got addressed in the series containing the commit above. Anyhow sorry for 
the
noise!

> 
>>
>> (I vaguely remember this as I dropped my v3 patch as being no longer 
>> necessary)
>>
>>> As a result, apps
>>> such as virt-manager that use the more generic virDomainMigrate API
>>> fail with
>>>
>>> libvirtError: this function is not supported by the connection driver:
>>> virDomainMigrate
>>>
>>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>>> supported in the connectSupportsFeature API.
>> FWIW and irrespective of the comment above:
>>
>> Reviewed-by: Joao Martins 
> 
> Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I 
> plan
> to push this for RC2.

Cool.

Regards,
Joao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

[Jim Fehlig]

On 08/30/2016 12:52 AM, Cedric Bosdonnat wrote:
> On Mon, 2016-08-29 at 11:20 -0600, Jim Fehlig wrote:
>> The libxl driver has long supported migration V3 but has never
>> indicated so in the connectSupportsFeature API. As a result, apps
>> such as virt-manager that use the more generic virDomainMigrate API
>> fail with
>>
>> libvirtError: this function is not supported by the connection driver:
>> virDomainMigrate
>>
>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>> supported in the connectSupportsFeature API.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>  src/libxl/libxl_driver.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index a573c82..3ffaa74 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -5677,6 +5677,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
>> feature)
>>  return -1;
>>  
>>  switch (feature) {
>> +case VIR_DRV_FEATURE_MIGRATION_V3:
>>  case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>>  case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>>  case VIR_DRV_FEATURE_MIGRATION_P2P:
> ACK

Thanks. Since this is a trivial bug fix, and given a second review by Joao, I've
pushed this for RC2.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

[Jim Fehlig]

On 08/30/2016 08:45 AM, Joao Martins wrote:
> On 08/29/2016 06:20 PM, Jim Fehlig wrote:
>> The libxl driver has long supported migration V3 but has never
>> indicated so in the connectSupportsFeature API. 
> Hmm, but IIRC it was only since "recent" commit 8db77b3 right?

The libxl driver has supported the VIR_DRV_FEATURE_MIGRATION_PARAMS V3 family of
migration functions since commit 9b8d6e1e in May 2014.

>  Or rather Nikolay's
> reworking top-level migration code, which effectively would convert to the 
> params
> versions accordingly. Before that rework I think one had to implement both 
> params and
> non-params variants. Would it be worth adding a comment mainly to help the 
> reader?

I think you are right, but support for V3 is independent of the params vs
non-params variants. virt-manager uses the generic virDomainMigrate() function
in src/libvirt-domain.c, and at least as far back as libvirt 1.2.5 that function
tests whether both source and destination support 
VIR_DRV_FEATURE_MIGRATION_V{1,2,3}. If the driver doesn't advertise support for
any of the versions, virDomainMigrate() returns VIR_ERR_NO_SUPPORT.

>
> (I vaguely remember this as I dropped my v3 patch as being no longer 
> necessary)
>
>> As a result, apps
>> such as virt-manager that use the more generic virDomainMigrate API
>> fail with
>>
>> libvirtError: this function is not supported by the connection driver:
>> virDomainMigrate
>>
>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>> supported in the connectSupportsFeature API.
> FWIW and irrespective of the comment above:
>
> Reviewed-by: Joao Martins 

Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I plan
to push this for RC2.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

[Joao Martins]

On 08/29/2016 06:20 PM, Jim Fehlig wrote:
> The libxl driver has long supported migration V3 but has never
> indicated so in the connectSupportsFeature API. 

Hmm, but IIRC it was only since "recent" commit 8db77b3 right? Or rather 
Nikolay's
reworking top-level migration code, which effectively would convert to the 
params
versions accordingly. Before that rework I think one had to implement both 
params and
non-params variants. Would it be worth adding a comment mainly to help the 
reader?

(I vaguely remember this as I dropped my v3 patch as being no longer necessary)

> As a result, apps
> such as virt-manager that use the more generic virDomainMigrate API
> fail with
> 
> libvirtError: this function is not supported by the connection driver:
> virDomainMigrate
> 
> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
> supported in the connectSupportsFeature API.

FWIW and irrespective of the comment above:

Reviewed-by: Joao Martins 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

[Cedric Bosdonnat]

On Mon, 2016-08-29 at 11:20 -0600, Jim Fehlig wrote:
> The libxl driver has long supported migration V3 but has never
> indicated so in the connectSupportsFeature API. As a result, apps
> such as virt-manager that use the more generic virDomainMigrate API
> fail with
> 
> libvirtError: this function is not supported by the connection driver:
> virDomainMigrate
> 
> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
> supported in the connectSupportsFeature API.
> 
> Signed-off-by: Jim Fehlig 
> ---
>  src/libxl/libxl_driver.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a573c82..3ffaa74 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5677,6 +5677,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
> feature)
>  return -1;
>  
>  switch (feature) {
> +case VIR_DRV_FEATURE_MIGRATION_V3:
>  case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>  case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>  case VIR_DRV_FEATURE_MIGRATION_P2P:

ACK
--
Cedric

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] libxl: advertise support for migration V3

[Jim Fehlig]

The libxl driver has long supported migration V3 but has never
indicated so in the connectSupportsFeature API. As a result, apps
such as virt-manager that use the more generic virDomainMigrate API
fail with

libvirtError: this function is not supported by the connection driver:
virDomainMigrate

Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
supported in the connectSupportsFeature API.

Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a573c82..3ffaa74 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5677,6 +5677,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
feature)
 return -1;
 
 switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_V3:
 case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
 case VIR_DRV_FEATURE_MIGRATION_PARAMS:
 case VIR_DRV_FEATURE_MIGRATION_P2P:
-- 
2.9.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list