mbox series

[v7,0/8] tests/qtest/migration: Add tests for introducing 'channels' argument in migrate QAPIs

Message ID 20240312202634.63349-1-het.gala@nutanix.com
Headers show
Series tests/qtest/migration: Add tests for introducing 'channels' argument in migrate QAPIs | expand

Message

Het Gala March 12, 2024, 8:26 p.m. UTC
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.


v6->v7 Changelog:
----------------
1. Rebased the series withthe latest master
2. Added "tests/qtest/migration" prefix in all patches.

v5->v6 Changelog:
----------------
1. Pass args->connect_channels variable instead of NULL while calling
   migrate_qmp for .../multifd/channels/plain/none test. Passing NULL
   beats the whole purpose of the test. 

v4->v5 Changelog:
----------------
1. Remove redundant imports from migration-test.c after moving helper
   functions to migration-helpers.c
2. call migrate_get_socket_address(to) and internally let qdict_get() call
   “socket-address” parameter to make more sense to the reader.
3. qdict needs to be freed, other small fixups.

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):
  tests/qtest/migration: Add 'to' object into migrate_qmp()
  tests/qtest/migration: Replace connect_uri and move
    migrate_get_socket_address inside migrate_qmp
  tests/qtest/migration: Replace migrate_get_connect_uri inplace of
    migrate_get_socket_address
  tests/qtest/migration: Add channels parameter in migrate_qmp_fail
  tests/qtest/migration: Add migrate_set_ports into migrate_qmp to
    update migration port value
  tests/qtest/migration: Add channels parameter in migrate_qmp
  tests/qtest/migration: Add multifd_tcp_plain test using list of
    channels instead of uri
  tests/qtest/migration: Add negative tests to validate migration QAPIs

 tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
 tests/qtest/migration-helpers.h |  10 +-
 tests/qtest/migration-test.c    | 180 +++++++++++++++++---------------
 3 files changed, 257 insertions(+), 91 deletions(-)

Comments

Het Gala March 12, 2024, 8:59 p.m. UTC | #1
Can find the passed build here: 
https://gitlab.com/galahet/Qemu/-/pipelines/1210931136

On 13/03/24 1:56 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.
[...]
>
> Het Gala (8):
>    tests/qtest/migration: Add 'to' object into migrate_qmp()
>    tests/qtest/migration: Replace connect_uri and move
>      migrate_get_socket_address inside migrate_qmp
>    tests/qtest/migration: Replace migrate_get_connect_uri inplace of
>      migrate_get_socket_address
>    tests/qtest/migration: Add channels parameter in migrate_qmp_fail
>    tests/qtest/migration: Add migrate_set_ports into migrate_qmp to
>      update migration port value
>    tests/qtest/migration: Add channels parameter in migrate_qmp
>    tests/qtest/migration: Add multifd_tcp_plain test using list of
>      channels instead of uri
>    tests/qtest/migration: Add negative tests to validate migration QAPIs
>
>   tests/qtest/migration-helpers.c | 158 +++++++++++++++++++++++++++-
>   tests/qtest/migration-helpers.h |  10 +-
>   tests/qtest/migration-test.c    | 180 +++++++++++++++++---------------
>   3 files changed, 257 insertions(+), 91 deletions(-)
>
Regards,
Het Gala
Peter Xu March 12, 2024, 9:12 p.m. UTC | #2
On Tue, Mar 12, 2024 at 08:26:26PM +0000, 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.

queued for 9.1, thanks.