On Thu, Feb 09, 2023 at 07:08:13PM +0530, Het Gala wrote:
On 09/02/23 5:35 pm, Daniel P. Berrangé wrote:
On Wed, Feb 08, 2023 at 09:35:57AM +0000, Het Gala wrote:
hmp_migrate() stores modified QAPI 'migrate' arguments from qdict
into well defined MigrateChannel struct with help of
migrate_channel_from_qdict().
hmp_migrate() also accepts uri string as modified QAPI a 'migrate'
argument (for backward compatibility).
Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Suggested-by: Manish Mishra <manish.mishra@nutanix.com>
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
migration/migration-hmp-cmds.c | 105 ++++++++++++++++++++++++++++++++-
migration/migration.c | 15 ++++-
2 files changed, 116 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 7a14aa98d8..f6dd8dbb03 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2463,9 +2463,9 @@ static bool migrate_prepare(MigrationState *s, bool blk,
bool blk_inc,
return true;
}
-void qmp_migrate(const char *uri, bool has_blk, bool blk,
- bool has_inc, bool inc, bool has_detach, bool detach,
- bool has_resume, bool resume, Error **errp)
+void qmp_migrate(const char *uri, MigrateChannel *channel, bool has_blk,
+ bool blk, bool has_inc, bool inc, bool has_detach,
+ bool detach, bool has_resume, bool resume, Error **errp)
{
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
@@ -2483,6 +2483,15 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
}
}
+ /*
+ * Having preliminary checks for uri and channel
+ */
+ if (uri && channel) {
+ error_setg(errp, "uri and channels options should be"
s/should be/are/, also best to quote parameter names, eg
error_setg(errp,
"'uri' and 'channels' options are mutually exclusive");
Ack.
+ "mutually exclusive");
+ return;
+ }
+
This change for qmp_migrate will need to be in patch 2.
QEMU needs to compile & pass tests successfully after each individual
patch. Currently it'll fail on patch 2, because the code generator
wil emit the new qmp_migrate API declaration signature, but the change
to the implementation signature is here in patch 3.
Yes Daniel, it will fail on patch 2. My understanding was that, even if
sometimes individual patches dont compile properly, the final series of all
the patches should be compiled properly. But I understand your point.
No, unfortunately we require the strict behaviour, where *every* individual
commit must compile and pass unit tests.
The reason for this is that when chasing regression bugs, it is common for
developers to use 'git bisect' to test compilation across a range of
releases. 'git bisect' will land on completely arbitrary commits, so it
is critical that every QEMU commit in the repo must compile and pass
tests. It isn't sufficient for just the end of the series to compile.
I have a small concern here Daniel, if you could help me resolve it?
- There is a similar issue in patch 4. Where some function parameters are to
be changed. But that function responds to both source and destination side
changes. So though patch 4 contains all the source side changes, it does not
take into account destination side changes and it does not compile entirely.
And after destination side changes are inside patch 6, the dependencies are
resolved. How is it possible to compile individual patches in this case,
because then each patch should also have some significant meaning to all the
changes. So, in that way, source side changes in one commit and destination
side changes in another commit makes more sense right ?
If there is code that is shared between src + dst, that may put constraints
on how you split up the patches.
Possibly a split like this may avoid having dependancy problems:
* Patch intoduces the 'MigrateAddress' struct and related child
objects, but *not* the MigrateChannel struct.
* Patch introduces code that can parse a 'uri' and spit out a
'MigrateAddress' struct.
* Patch converts internal socket backend to accept MigrateAddress,
with 'migrate/migrate_incoming' impl convert from uri -> MigrateAddress
* Patch converts internal exec backend to accept MigrateAddress
with 'migrate/migrate_incoming' impl convert from uri -> MigrateAddress
* Patch converts internal rdma backend to accept MigrateAddress
with 'migrate/migrate_incoming' impl convert from uri -> MigrateAddress
* Patch converts 'migrate' command to accept MigrateChannel param
directly
* Patch converts 'migrate_incoming' command to accept MigrateChannel
param directly.