[PATCH] migration: Simplify initial conditionals in migration for better readability

[PATCH] migration: Simplify initial conditionals in migration for better readability

[Het Gala]

The inital conditional statements in qmp migration functions is harder
to understand than necessary. It is better to get all errors out of
the way in the beginning itself to have better readability and error
handling.

Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
---
 migration/migration.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 3ce04b2aaf..962ee7564c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -523,28 +523,26 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
     /*
      * Having preliminary checks for uri and channel
      */
-    if (uri && has_channels) {
-        error_setg(errp, "'uri' and 'channels' arguments are mutually "
-                   "exclusive; exactly one of the two should be present in "
-                   "'migrate-incoming' qmp command ");
+    if (!uri == !channels) {
+        error_setg(errp, "need either 'uri' or 'channels' argument");
         return;
-    } else if (channels) {
+    }
+
+    if (channels) {
         /* To verify that Migrate channel list has only item */
         if (channels->next) {
             error_setg(errp, "Channel list has more than one entries");
             return;
         }
         addr = channels->value->addr;
-    } else if (uri) {
+    }
+
+    if (uri) {
         /* caller uses the old URI syntax */
         if (!migrate_uri_parse(uri, &channel, errp)) {
             return;
         }
         addr = channel->addr;
-    } else {
-        error_setg(errp, "neither 'uri' or 'channels' argument are "
-                   "specified in 'migrate-incoming' qmp command ");
-        return;
     }
 
     /* transport mechanism not suitable for migration? */
@@ -1939,28 +1937,26 @@ void qmp_migrate(const char *uri, bool has_channels,
     /*
      * Having preliminary checks for uri and channel
      */
-    if (uri && has_channels) {
-        error_setg(errp, "'uri' and 'channels' arguments are mutually "
-                   "exclusive; exactly one of the two should be present in "
-                   "'migrate' qmp command ");
+    if (!uri == !channels) {
+        error_setg(errp, "need either 'uri' or 'channels' argument");
         return;
-    } else if (channels) {
+    }
+
+    if (channels) {
         /* To verify that Migrate channel list has only item */
         if (channels->next) {
             error_setg(errp, "Channel list has more than one entries");
             return;
         }
         addr = channels->value->addr;
-    } else if (uri) {
+    }
+
+    if (uri) {
         /* caller uses the old URI syntax */
         if (!migrate_uri_parse(uri, &channel, errp)) {
             return;
         }
         addr = channel->addr;
-    } else {
-        error_setg(errp, "neither 'uri' or 'channels' argument are "
-                   "specified in 'migrate' qmp command ");
-        return;
     }
 
     /* transport mechanism not suitable for migration? */
-- 
2.22.3

Re: [PATCH] migration: Simplify initial conditionals in migration for better readability

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]


On 05/12/23 1:30 pm, Het Gala wrote:
> The inital conditional statements in qmp migration functions is harder
> to understand than necessary. It is better to get all errors out of
> the way in the beginning itself to have better readability and error
> handling.
>
> Signed-off-by: Het Gala<het.gala@nutanix.com>
> Suggested-by: Markus Armbruster<armbru@redhat.com>

I have also tested this patch on the Qemu CI/CD, ref :https://gitlab.com/galahet/Qemu/-/pipelines/1094304060

Regards,

Het Gala

[-- Attachment #2: Type: text/html, Size: 1149 bytes --]

Re: [PATCH] migration: Simplify initial conditionals in migration for better readability

[Fabiano Rosas]

Het Gala <het.gala@nutanix.com> writes:

> The inital conditional statements in qmp migration functions is harder
> to understand than necessary. It is better to get all errors out of
> the way in the beginning itself to have better readability and error
> handling.
>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Suggested-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH] migration: Simplify initial conditionals in migration for better readability

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Ping? Waiting for other maintainers to respond on this patch

On 05/12/23 6:28 pm, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> The inital conditional statements in qmp migration functions is harder
>> to understand than necessary. It is better to get all errors out of
>> the way in the beginning itself to have better readability and error
>> handling.
>>
>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>> Suggested-by: Markus Armbruster<armbru@redhat.com>
> Reviewed-by: Fabiano Rosas<farosas@suse.de>
>
Regards,

Het Gala

[-- Attachment #2: Type: text/html, Size: 1750 bytes --]

Re: [PATCH] migration: Simplify initial conditionals in migration for better readability

[Het Gala]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

Ping. In-case this patch has been missed out. Waiting for other maintainers to respond. Thanks!

On 11/12/23 6:43 pm, Het Gala wrote:
> Ping? Waiting for other maintainers to respond on this patch
> On 05/12/23 6:28 pm, Fabiano Rosas wrote:
>> Het Gala<het.gala@nutanix.com>  writes:
>>
>>> The inital conditional statements in qmp migration functions is harder
>>> to understand than necessary. It is better to get all errors out of
>>> the way in the beginning itself to have better readability and error
>>> handling.
>>>
>>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>>> Suggested-by: Markus Armbruster<armbru@redhat.com>
>> Reviewed-by: Fabiano Rosas<farosas@suse.de>
>>
> Regards,
> Het Gala

Regards,

Het Gala

[-- Attachment #2: Type: text/html, Size: 2214 bytes --]

Re: [PATCH] migration: Simplify initial conditionals in migration for better readability

[Peter Xu]

On Thu, Jan 04, 2024 at 01:13:48PM +0530, Het Gala wrote:
> Ping. In-case this patch has been missed out. Waiting for other maintainers to respond. Thanks!
> 
> On 11/12/23 6:43 pm, Het Gala wrote:
> > Ping? Waiting for other maintainers to respond on this patch
> > On 05/12/23 6:28 pm, Fabiano Rosas wrote:
> > > Het Gala<het.gala@nutanix.com>  writes:
> > > 
> > > > The inital conditional statements in qmp migration functions is harder
> > > > to understand than necessary. It is better to get all errors out of
> > > > the way in the beginning itself to have better readability and error
> > > > handling.
> > > > 
> > > > Signed-off-by: Het Gala<het.gala@nutanix.com>
> > > > Suggested-by: Markus Armbruster<armbru@redhat.com>
> > > Reviewed-by: Fabiano Rosas<farosas@suse.de>
> > > 
> > Regards,
> > Het Gala

queued in staging.  will be in the next pull.

https://gitlab.com/peterx/qemu/-/tree/migration-staging

Thanks,

-- 
Peter Xu