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

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 <jfehlig@suse.com> --- 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

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 <jfehlig@suse.com> --- 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

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 <jfehlig@suse.com> --- 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

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 <joao.m.martins@oracle.com>

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 <joao.m.martins@oracle.com>
Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I plan to push this for RC2. Regards, Jim

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 <joao.m.martins@oracle.com>
Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I plan to push this for RC2.
Cool. Regards, Joao
participants (3)
-
Cedric Bosdonnat
-
Jim Fehlig
-
Joao Martins