| Message ID | 20231023182053.8711-14-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration | expand |
Fabiano Rosas <farosas@suse.de> wrote: > From: Het Gala <het.gala@nutanix.com> > > Integrate MigrateChannelList with all transport backends > (socket, exec and rdma) for both src and dest migration > endpoints for hmp migration. > > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > Signed-off-by: Het Gala <het.gala@nutanix.com> > Signed-off-by: Fabiano Rosas <farosas@suse.de> > bool resume = qdict_get_try_bool(qdict, "resume", false); > const char *uri = qdict_get_str(qdict, "uri"); > Error *err = NULL; > + MigrationChannelList *caps = NULL; > + g_autoptr(MigrationChannel) channel = NULL; > > - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc, > + if (!migrate_uri_parse(uri, &channel, &err)) { > + goto end; > + } > + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); > + > + qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc, > false, false, true, resume, &err); > - if (hmp_handle_error(mon, err)) { > - return; > - } I think that dropping this chunk is wrong. What assures that qmp_migrate will not give an error? > + qapi_free_MigrationChannelList(caps); > > if (!detach) { > HMPMigrationStatus *status; > @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) > status); > timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); > } > + > +end: > + hmp_handle_error(mon, err); Oh, you put it here, but you enter in the detach case even if there is one error. I think it is easier to just repeat the hmp_mhandle_error() inplace of the goto. Later, Juan.
On 31/10/23 11:12 pm, Juan Quintela wrote: > Fabiano Rosas <farosas@suse.de> wrote: >> From: Het Gala <het.gala@nutanix.com> >> >> Integrate MigrateChannelList with all transport backends >> (socket, exec and rdma) for both src and dest migration >> endpoints for hmp migration. >> >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> >> Signed-off-by: Het Gala <het.gala@nutanix.com> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > >> bool resume = qdict_get_try_bool(qdict, "resume", false); >> const char *uri = qdict_get_str(qdict, "uri"); >> Error *err = NULL; >> + MigrationChannelList *caps = NULL; >> + g_autoptr(MigrationChannel) channel = NULL; >> >> - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc, >> + if (!migrate_uri_parse(uri, &channel, &err)) { >> + goto end; >> + } >> + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); >> + >> + qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc, >> false, false, true, resume, &err); > >> - if (hmp_handle_error(mon, err)) { >> - return; >> - } > I think that dropping this chunk is wrong. What assures that > qmp_migrate will not give an error? > >> + qapi_free_MigrationChannelList(caps); > > > > >> >> if (!detach) { >> HMPMigrationStatus *status; >> @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) >> status); >> timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); >> } >> + >> +end: >> + hmp_handle_error(mon, err); > > Oh, you put it here, but you enter in the detach case even if there is > one error. > > I think it is easier to just repeat the hmp_mhandle_error() inplace of > the goto. Okay Juan, will add hmp_handle_error() instead of goto statement if (!migrate_uri_parse(uri, &channel, &err)) { hmp_handle_error(mon, err); return; } I will send new patchset, squash previous commits and add the tags wherever required. > Later, Juan. Regards, Het Gala
I intrehated al parches until this forma next pull. On Tue, Oct 31, 2023, 19:51 Het Gala <het.gala@nutanix.com> wrote: > > On 31/10/23 11:12 pm, Juan Quintela wrote: > > Fabiano Rosas <farosas@suse.de> wrote: > >> From: Het Gala <het.gala@nutanix.com> > >> > >> Integrate MigrateChannelList with all transport backends > >> (socket, exec and rdma) for both src and dest migration > >> endpoints for hmp migration. > >> > >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > >> Signed-off-by: Het Gala <het.gala@nutanix.com> > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > >> bool resume = qdict_get_try_bool(qdict, "resume", false); > >> const char *uri = qdict_get_str(qdict, "uri"); > >> Error *err = NULL; > >> + MigrationChannelList *caps = NULL; > >> + g_autoptr(MigrationChannel) channel = NULL; > >> > >> - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc, > >> + if (!migrate_uri_parse(uri, &channel, &err)) { > >> + goto end; > >> + } > >> + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); > >> + > >> + qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc, > >> false, false, true, resume, &err); > > > >> - if (hmp_handle_error(mon, err)) { > >> - return; > >> - } > > I think that dropping this chunk is wrong. What assures that > > qmp_migrate will not give an error? > > > >> + qapi_free_MigrationChannelList(caps); > > > > > > > > > >> > >> if (!detach) { > >> HMPMigrationStatus *status; > >> @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) > >> status); > >> timer_mod(status->timer, > qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); > >> } > >> + > >> +end: > >> + hmp_handle_error(mon, err); > > > > Oh, you put it here, but you enter in the detach case even if there is > > one error. > > > > I think it is easier to just repeat the hmp_mhandle_error() inplace of > > the goto. > > Okay Juan, will add hmp_handle_error() instead of goto statement > > if (!migrate_uri_parse(uri, &channel, &err)) { > hmp_handle_error(mon, err); > return; > } > > I will send new patchset, squash previous commits and add the tags > wherever required. > > > Later, Juan. > Regards, > Het Gala > >
On Tue, Oct 31, 2023 at 06:42:43PM +0100, Juan Quintela wrote: > Fabiano Rosas <farosas@suse.de> wrote: > > From: Het Gala <het.gala@nutanix.com> > > > > Integrate MigrateChannelList with all transport backends > > (socket, exec and rdma) for both src and dest migration > > endpoints for hmp migration. > > > > Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > > Signed-off-by: Het Gala <het.gala@nutanix.com> > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > > bool resume = qdict_get_try_bool(qdict, "resume", false); > > const char *uri = qdict_get_str(qdict, "uri"); > > Error *err = NULL; > > + MigrationChannelList *caps = NULL; > > + g_autoptr(MigrationChannel) channel = NULL; > > > > - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc, > > + if (!migrate_uri_parse(uri, &channel, &err)) { > > + goto end; > > + } > > + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); > > + > > + qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc, > > false, false, true, resume, &err); > > > > - if (hmp_handle_error(mon, err)) { > > - return; > > - } > > I think that dropping this chunk is wrong. What assures that > qmp_migrate will not give an error? > > > + qapi_free_MigrationChannelList(caps); > > > > > > > > > if (!detach) { > > HMPMigrationStatus *status; > > @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) > > status); > > timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); > > } > > + > > +end: > > + hmp_handle_error(mon, err); > > > Oh, you put it here, but you enter in the detach case even if there is > one error. > > I think it is easier to just repeat the hmp_mhandle_error() inplace of > the goto. Agreed. We don't want to kick the timer even if error..
On 01/11/23 12:25 am, Juan Quintela wrote: > I intrehated al parches until this forma next pull. I should just add the last 2 patches as individual ones, is that what you mean ? > > On Tue, Oct 31, 2023, 19:51 Het Gala <het.gala@nutanix.com> wrote: > > > On 31/10/23 11:12 pm, Juan Quintela wrote: > > Fabiano Rosas <farosas@suse.de> wrote: > >> From: Het Gala <het.gala@nutanix.com> > >> > >> Integrate MigrateChannelList with all transport backends > >> (socket, exec and rdma) for both src and dest migration > >> endpoints for hmp migration. > >> > >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> > >> Signed-off-by: Het Gala <het.gala@nutanix.com> > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > > > Later, Juan. > Regards, > Het Gala > Regards, Het Gala
Het Gala <het.gala@nutanix.com> wrote: > On 01/11/23 12:25 am, Juan Quintela wrote: >> I intrehated al parches until this forma next pull. > I should just add the last 2 patches as individual ones, is that what > you mean ? Take a look at: https://gitlab.com/juan.quintela/qemu/-/commits/migration-next This is my next PULL request on top of the one already on the list. https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg10281.html I just did that O:-) Later, Juan. >> On Tue, Oct 31, 2023, 19:51 Het Gala <het.gala@nutanix.com> wrote: >> >> >> On 31/10/23 11:12 pm, Juan Quintela wrote: >> > Fabiano Rosas <farosas@suse.de> wrote: >> >> From: Het Gala <het.gala@nutanix.com> >> >> >> >> Integrate MigrateChannelList with all transport backends >> >> (socket, exec and rdma) for both src and dest migration >> >> endpoints for hmp migration. >> >> >> >> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com> >> >> Signed-off-by: Het Gala <het.gala@nutanix.com> >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> > >> >> > Later, Juan. >> Regards, >> Het Gala >> > Regards, > Het Gala
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index c1a73df7c0..ffe32e3b9b 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -446,9 +446,18 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) { Error *err = NULL; const char *uri = qdict_get_str(qdict, "uri"); + MigrationChannelList *caps = NULL; + g_autoptr(MigrationChannel) channel = NULL; - qmp_migrate_incoming(uri, false, NULL, &err); + if (!migrate_uri_parse(uri, &channel, &err)) { + goto end; + } + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); + qmp_migrate_incoming(NULL, true, caps, &err); + qapi_free_MigrationChannelList(caps); + +end: hmp_handle_error(mon, err); } @@ -744,12 +753,17 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) bool resume = qdict_get_try_bool(qdict, "resume", false); const char *uri = qdict_get_str(qdict, "uri"); Error *err = NULL; + MigrationChannelList *caps = NULL; + g_autoptr(MigrationChannel) channel = NULL; - qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc, + if (!migrate_uri_parse(uri, &channel, &err)) { + goto end; + } + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); + + qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc, false, false, true, resume, &err); - if (hmp_handle_error(mon, err)) { - return; - } + qapi_free_MigrationChannelList(caps); if (!detach) { HMPMigrationStatus *status; @@ -766,6 +780,9 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) status); timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); } + +end: + hmp_handle_error(mon, err); } void migrate_set_capability_completion(ReadLineState *rs, int nb_args, diff --git a/migration/migration.c b/migration/migration.c index c4c2f3a266..427965099b 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -432,9 +432,8 @@ void migrate_add_address(SocketAddress *address) QAPI_CLONE(SocketAddress, address)); } -static bool migrate_uri_parse(const char *uri, - MigrationChannel **channel, - Error **errp) +bool migrate_uri_parse(const char *uri, MigrationChannel **channel, + Error **errp) { g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1); g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1); diff --git a/migration/migration.h b/migration/migration.h index ae82004892..c2a5333e0e 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -520,7 +520,8 @@ bool check_dirty_bitmap_mig_alias_map(const BitmapMigrationNodeAliasList *bbm, Error **errp); void migrate_add_address(SocketAddress *address); - +bool migrate_uri_parse(const char *uri, MigrationChannel **channel, + Error **errp); int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque); #define qemu_ram_foreach_block \