Re: [PATCH] Make 'uri' optional for migrate QAPI

[Het Gala]

On 24/01/24 10:03 am, Het Gala wrote:

On 24/01/24 7:13 am, Peter Xu wrote:

On Tue, Jan 23, 2024 at 08:21:55AM +0000, Daniel P. Berrangé wrote:

On Tue, Jan 23, 2024 at 06:42:19AM +0000, Het Gala wrote:

'uri' argument should be optional, as 'uri' and 'channels'
arguments are mutally exclusive in nature.

Fixes: 074dbce5fcce (migration: New migrate and
migrate-incoming argument 'channels')
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index eb2f883513..197d3faa43 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1757,7 +1757,7 @@
 #
 ##
 { 'command': 'migrate',
-  'data': {'uri': 'str',
+  'data': {'*uri': 'str',
            '*channels': [ 'MigrationChannel' ],
            '*blk': { 'type': 'bool', 'features': [ 'deprecated' ] },
            '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] },

Hmm, this mistake shows a lack of coverage in migration-test.c for
the 'channels' argument. I thought the original series adding 'channels'
included the tests for this. Either way, this needs to come with test
coverage for use of 'channels', with 'uri' omitted.

Agreed.  Het, do you plan to provide a test case?

Yes, will provide a test case patch for 'channels' soon.

Hi everyone, I was trying to wrap around on how to write a migration test or to mock migration.
I see there are a couple of migration tests already written, but most of them focuses on just getting the uri and parsing uri to start the migration.
I have a couple of questions for starters like me who is attempting to write test cases for the first time:
1. Do I need to make a whole new test or just edit one of the tests that is using uri, and instead send in 'MigrateChannel' struct and parse the necessary information out of it ?
2. Do I need to add tests for unix, fd too with the modified syntax ?
3. Do I also need to add test to ensure - uri and channels both cannot be used simultaneously ? (based on the above patch)
4. Is there updated document in Qemu to follow latest practices on how to write migration tests?
Regards,
Het Gala
>  
>