| Message ID | 20231205080039.197615-1-het.gala@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Simplify initial conditionals in migration for better readability | expand |
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
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>
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
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
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,
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? */
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(-)