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

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(a)oracle.com&gt; Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I plan to push this for RC2.