On 10/9/2023 8:05 PM, Fabiano Rosas wrote:
Het Gala<het.g...@nutanix.com> writes:
On 10/4/2023 8:55 PM, Fabiano Rosas wrote:
Het Gala<het.g...@nutanix.com> writes:
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.retnaka...@nutanix.com>
Signed-off-by: Het Gala<het.g...@nutanix.com>
Reviewed-by: Daniel P. Berrangé<berra...@redhat.com>
---
migration/migration-hmp-cmds.c | 15 +++++++++++++--
migration/migration.c | 5 ++---
migration/migration.h | 3 ++-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index a2e6a5c51e..a1657f3d37 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -441,9 +441,14 @@ 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 = g_new0(MigrationChannel, 1);
Just the pointer here. If I remember correctly the g_autoptr here would
cause a double free when freeing the caps.
Yes, we'll just have 'g_autoptr(MigrationChannel) channel = NULL'.
Is it because inside QAPI_LIST_PREPEND, caps will be refrencing to the
same memory as 'channel', we don't need to free channel ?
Slightly different scenario here. Here the issue is that we will free
the caps with qapi_free_MigrationChannel() before returning. Then, at
function exit the g_autoptr will try to free 'channel', which has
already been freed along with 'caps'. That's a double free, I think it
hits an assert inside glib, if I remember correctly.
I am still not
sure what is the right place to use g_steal_pointer(), is this a right
place to use (non-error paths) ?
It doesn't look like we need it here. As long as the qapi list has a
reference and we're freeing the caps, then channel should be freed by
that function already.
Ack. Yes, with the discussion in earlier patches, I also don't think we
need g_autoptr too here. Normal pointer is enough as we are freeing the
memory after the function is returned.
- qmp_migrate_incoming(uri, false, NULL, &err);
+ migrate_uri_parse(uri, &channel, &err);
+ QAPI_LIST_PREPEND(caps, channel);
+ qmp_migrate_incoming(NULL, true, caps, &err);
+ qapi_free_MigrationChannelList(caps);
hmp_handle_error(mon, err);
}
@@ -730,9 +735,15 @@ 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 = g_new0(MigrationChannel, 1);
Same here. We free the channel via caps and we attribute it below, no
need to allocate.
Ack.
- qmp_migrate(uri, false, NULL, !!blk, blk, !!inc, inc,
+ migrate_uri_parse(uri, &channel, &err);
+ QAPI_LIST_PREPEND(caps, channel);
+
+ qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc,
false, false, true, resume, &err);
+ qapi_free_MigrationChannelList(caps);
if (hmp_handle_error(mon, err)) {
Regards,
Het Gala
Regards,
Het Gala
