With recent migrate QAPI changes, enabling the direct use of the
'channels' argument to avoid redundant URI string parsing is achieved.

To ensure backward compatibility, both 'uri' and 'channels' are kept as
optional parameters in migration QMP commands. However, they are mutually
exhaustive, requiring at least one for a successful migration connection.
This patchset adds qtests to validate 'uri' and 'channels' arguments'
mututally exhaustive behaviour.

Additionally, all migration qtests fail to employ 'channel' as the primary
method for validating migration QAPIs. This patchset also adds test to
enforce only use of 'channel' argument as the initial entry point for
migration QAPIs.

Patch Summary:
-------------
Patch 1-2:
---------
Introduce 'to' object inside migrate_qmp() so and move the calls to
migrate_get_socket_address() inside migrate_qmp. Also, replace connect_uri
with args->connect_uri everywhere.

Patch 3-6:
---------
Add channels argument to allow both migration QAPI arguments independently
into migrate_qmp and migrate_qmp_fail. migrate_qmp requires the port value to
be changed from 0 to port value coming from migrate_get_socket_address. Add
migrate_set_ports to address this change of port value.

Patch 7-8:
---------
Add 2 negative tests to validate mutually exhaustive behaviour of migration
QAPIs. Add a positive multifd_tcp_plain qtest with only channels as the
initial entry point for migration QAPIs.

v3->v4 Changelog:
----------------
1. introduced migrate_get_connect_uri and migrate_get_connect_qdict to
   both used migrate_get_socket_address to get dest uri in socket-
   address, and then use SokcketAddress_to_qdict to convert it into qdict.
2. Misc code changes.

v2->v3 Changelog:
-----------------
1. 'channels' introduction is not required now for migrate_qmp_incoming
2. Refactor the code into 7 different patches
3. 'channels' introduction is not required now for migrate_qmp_incoming
4. Remove custom function for converting string to MigrationChannelList
5. move calls for migrate_get_socket_address inside migrate_qmp so that
   migrate_set_ports can replace the QAPI's port with correct value.

Het Gala (8):
  Add 'to' object into migrate_qmp()
  Replace connect_uri and move migrate_get_socket_address inside
    migrate_qmp
  Replace migrate_get_connect_uri inplace of migrate_get_socket_address
  Add channels parameter in migrate_qmp_fail
  Add migrate_set_ports into migrate_qmp to update migration port value
  Add channels parameter in migrate_qmp
  Add multifd_tcp_plain test using list of channels instead of uri
  Add negative tests to validate migration QAPIs

 tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
 tests/qtest/migration-helpers.h |  10 +-
 tests/qtest/migration-test.c    | 177 ++++++++++++++++++--------------
 3 files changed, 258 insertions(+), 87 deletions(-)

-- 
2.22.3

Can find the reference to the githab pipeline (before patchset) : 
https://gitlab.com/galahet/Qemu/-/pipelines/1207185095

Can find the reference to the githab pipeline (after patchset) : 
https://gitlab.com/galahet/Qemu/-/pipelines/1207183673

On 09/03/24 2:29 am, Het Gala wrote:
> With recent migrate QAPI changes, enabling the direct use of the
> 'channels' argument to avoid redundant URI string parsing is achieved.
>
> To ensure backward compatibility, both 'uri' and 'channels' are kept as
> optional parameters in migration QMP commands. However, they are mutually
> exhaustive, requiring at least one for a successful migration connection.
> This patchset adds qtests to validate 'uri' and 'channels' arguments'
> mututally exhaustive behaviour.
>
> Additionally, all migration qtests fail to employ 'channel' as the primary
> method for validating migration QAPIs. This patchset also adds test to
> enforce only use of 'channel' argument as the initial entry point for
> migration QAPIs.
>
> Patch Summary:
> -------------
> Patch 1-2:
> ---------
> Introduce 'to' object inside migrate_qmp() so and move the calls to
> migrate_get_socket_address() inside migrate_qmp. Also, replace connect_uri
> with args->connect_uri everywhere.
>
> Patch 3-6:
> ---------
> Add channels argument to allow both migration QAPI arguments independently
> into migrate_qmp and migrate_qmp_fail. migrate_qmp requires the port value to
> be changed from 0 to port value coming from migrate_get_socket_address. Add
> migrate_set_ports to address this change of port value.
>
> Patch 7-8:
> ---------
> Add 2 negative tests to validate mutually exhaustive behaviour of migration
> QAPIs. Add a positive multifd_tcp_plain qtest with only channels as the
> initial entry point for migration QAPIs.
>
> v3->v4 Changelog:
> ----------------
> 1. introduced migrate_get_connect_uri and migrate_get_connect_qdict to
>     both used migrate_get_socket_address to get dest uri in socket-
>     address, and then use SokcketAddress_to_qdict to convert it into qdict.
> 2. Misc code changes.
>
> v2->v3 Changelog:
> -----------------
> 1. 'channels' introduction is not required now for migrate_qmp_incoming
> 2. Refactor the code into 7 different patches
> 3. 'channels' introduction is not required now for migrate_qmp_incoming
> 4. Remove custom function for converting string to MigrationChannelList
> 5. move calls for migrate_get_socket_address inside migrate_qmp so that
>     migrate_set_ports can replace the QAPI's port with correct value.
>
> Het Gala (8):
>    Add 'to' object into migrate_qmp()
>    Replace connect_uri and move migrate_get_socket_address inside
>      migrate_qmp
>    Replace migrate_get_connect_uri inplace of migrate_get_socket_address
>    Add channels parameter in migrate_qmp_fail
>    Add migrate_set_ports into migrate_qmp to update migration port value
>    Add channels parameter in migrate_qmp
>    Add multifd_tcp_plain test using list of channels instead of uri
>    Add negative tests to validate migration QAPIs
>
>   tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
>   tests/qtest/migration-helpers.h |  10 +-
>   tests/qtest/migration-test.c    | 177 ++++++++++++++++++--------------
>   3 files changed, 258 insertions(+), 87 deletions(-)
>

On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
> Can find the reference to the githab pipeline (before patchset) :
> https://gitlab.com/galahet/Qemu/-/pipelines/1207185095
> 
> Can find the reference to the githab pipeline (after patchset) :
> https://gitlab.com/galahet/Qemu/-/pipelines/1207183673

Het,

Please still copy me for any migration patches.  In this case Fabiano is
looking it'll be all fine, but it will still help me on marking the emails.

Thanks,

-- 
Peter Xu

On 12/03/24 2:55 am, Peter Xu wrote:
> On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
>> Can find the reference to the githab pipeline (before patchset) :
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
>>
>> Can find the reference to the githab pipeline (after patchset) :
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
> Het,
>
> Please still copy me for any migration patches.  In this case Fabiano is
> looking it'll be all fine, but it will still help me on marking the emails.
>
> Thanks,
So sorry about that Peter. I am aware that you and Fabiano are the go to 
migration
maintainers. I thought I emailed or cc'd all the stakeholders that 
should be involved
for this patchset series. Even in earlier series of this patchset, you 
were cc'ed,
but somehow I just forgot to cc you for this patchset. Sure, will take 
care from next
time. Again apologies for the mixup :)

Regards,
Het Gala

On Tue, Mar 12, 2024 at 03:01:51AM +0530, Het Gala wrote:
> 
> On 12/03/24 2:55 am, Peter Xu wrote:
> > On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
> > > Can find the reference to the githab pipeline (before patchset) :
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
> > > 
> > > Can find the reference to the githab pipeline (after patchset) :
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
> > Het,
> > 
> > Please still copy me for any migration patches.  In this case Fabiano is
> > looking it'll be all fine, but it will still help me on marking the emails.
> > 
> > Thanks,
> So sorry about that Peter. I am aware that you and Fabiano are the go to
> migration
> maintainers. I thought I emailed or cc'd all the stakeholders that should be
> involved
> for this patchset series. Even in earlier series of this patchset, you were
> cc'ed,
> but somehow I just forgot to cc you for this patchset. Sure, will take care
> from next
> time. Again apologies for the mixup :)

No problem at all.  As long as you have at least 1 maintainers copied,
logically nothing will get lost.  It's just that it helps me in the routines.

Are you managing cc list manually for each version?  In that case I suggest
you have a look at Stefan's tool:

https://github.com/stefanha/git-publish

It might help a great deal in patch managements at least to me, and it
definitely covers more than maintaining the cc list for a patchset.

-- 
Peter Xu

On 12/03/24 3:08 am, Peter Xu wrote:
> On Tue, Mar 12, 2024 at 03:01:51AM +0530, Het Gala wrote:
>> On 12/03/24 2:55 am, Peter Xu wrote:
>>> On Sat, Mar 09, 2024 at 01:11:45PM +0530, Het Gala wrote:
>>>> Can find the reference to the githab pipeline (before patchset) :
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207185095&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=vZRNX33_DuLO1TsfTpYR_s9bf_EMFm3oHHH_eg57zE0&e=
>>>>
>>>> Can find the reference to the githab pipeline (after patchset) :
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_galahet_Qemu_-2D_pipelines_1207183673&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=y2xUaOwvRVC5eTpFNEdxb37JYDdxN61W406HlCyx3CWIVyBRgLwjJhAYALZLinoi&s=C73ka3k3ouAuRJYNVLPIBQiWx3jDFDDvVYDiEYqfE04&e=
>>> Het,
>>>
>>> Please still copy me for any migration patches.  In this case Fabiano is
>>> looking it'll be all fine, but it will still help me on marking the emails.
>>>
>>> Thanks,
>> So sorry about that Peter. I am aware that you and Fabiano are the go to
>> migration
>> maintainers. I thought I emailed or cc'd all the stakeholders that should be
>> involved
>> for this patchset series. Even in earlier series of this patchset, you were
>> cc'ed,
>> but somehow I just forgot to cc you for this patchset. Sure, will take care
>> from next
>> time. Again apologies for the mixup :)
> No problem at all.  As long as you have at least 1 maintainers copied,
> logically nothing will get lost.  It's just that it helps me in the routines.
>
> Are you managing cc list manually for each version?  In that case I suggest
> you have a look at Stefan's tool:
I used to earlier. But lately markus introduced me to
scripts/get_maintainers.pl -f <file>
It gives list of all the maintainers handling that particular file.
So that helped me for this patchset.
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stefanha_git-2Dpublish&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=ydJfb02Wuk_NnlYl8-RkRkYXzWNpzlEht7yj5kakeAlz_WPoD6yvC7b-fVCeLzom&s=8KSe9MiMzmHda3uZ_uaGCIEjub4tSzpeDTpZZwq5knc&e=  
Thanks a lot Peter, looks cool. Will try to explore and use git-publish
and its different methods for next patchset.
> It might help a great deal in patch managements at least to me, and it
> definitely covers more than maintaining the cc list for a patchset.
>
Yes, it looks like there are a lot of useful methods that I can leverage
in future :)

Regards,
Het Gala