Re: Problem with migration/rdma

[Yu Zhang]

Hello Peter and Zhijian,

I created a MR in gitlab. You may have a look and let me know whether
it's fine for you.
    https://gitlab.com/qemu/qemu/-/merge_requests/4

Best regards,
Yu Zhang @ IONOS Compute Platform
11.03.2024

On Fri, Mar 8, 2024 at 10:13 AM Yu Zhang <yu.zhang@ionos.com> wrote:
>
> Hallo Alexei,
>
> vielen Dank! Habe probiert, aber auch an Permission Problem gestoßen.
> Ich versuche, Google Application-specific password zu erstellen für Zukunft.
>
> QEMU Upstream bietet diese Lösung für kleinen Patch an im Notfall.
> Sie werden es behanden.
>
> Viele Grüße
> Yu
>
> On Fri, Mar 8, 2024 at 8:09 AM Alexei Pastuchov
> <alexei.pastuchov@ionos.com> wrote:
> >
> > Hi Yu, I think you could create a pull request to the project
> > https://github.com/qemu/qemu for the purpose.
> > Best,
> > Alexei
> >
> > On Fri, Mar 8, 2024 at 7:28 AM Yu Zhang <yu.zhang@ionos.com> wrote:
> > >
> > > Hello Zhijian and Peter,
> > >
> > > Thank you so much for testing and confirming it.
> > > I created a patch in the email format, unfortunately got an issue for
> > > setting up the
> > > "Application-specific Password" in Gmail. It seems that in my gmail
> > > account there
> > > is no option at all for selecting "mail" before creating the
> > > application password.
> > >
> > > As it's tiny, I attached it in this email at this time (not elegant.),
> > > so that it can get
> > > included before the soft freezing.
> > >
> > > Really sorry for this inconvenience.
> > > --------------
> > > From c9fb6a6debfbd5e103aa90f30e9a028316449104 Mon Sep 17 00:00:00 2001
> > > From: Yu Zhang <yu.zhang@ionos.com>
> > > Date: Wed, 6 Mar 2024 09:06:54 +0100
> > > Subject: [PATCH] migration/rdma: Fix a memory issue for migration
> > >
> > > In commit 3fa9642ff7 change was made to convert the RDMA backend to
> > > accept MigrateAddress struct. However, the assignment of "host" leads
> > > to data corruption on the target host and the failure of migration.
> > >
> > >     isock->host = rdma->host;
> > >
> > > By allocating the memory explicitly for it with g_strdup_printf(), the
> > > issue is fixed and the migration doesn't fail any more.
> > >
> > > Fixes: 3fa9642ff7 ("migration: convert rdma backend to accept 
> > > MigrateAddress")
> > > Cc: qemu-stable <qemu-stable@nongnu.org>
> > > Cc: Li Zhijian <lizhijian@fujitsu.com>
> > > Cc: Peter Xu <peterx@redhat.com>
> > > Signed-off-by: Yu Zhang <yu.zhang@ionos.com>
> > > ---
> > >  migration/rdma.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > index a355dcea89..d6abe718b5 100644
> > > --- a/migration/rdma.c
> > > +++ b/migration/rdma.c
> > > @@ -3357,7 +3357,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
> > >          goto err_rdma_dest_wait;
> > >      }
> > >
> > > -    isock->host = rdma->host;
> > > +    isock->host = g_strdup_printf("%s", rdma->host);
> > >      isock->port = g_strdup_printf("%d", rdma->port);
> > >
> > >      /*
> > > --
> > > 2.25.1
> > > --------------
> > >
> > > Best regards,
> > > Yu Zhang @ IONOS Compute Platform
> > > 08.03.2024
> > >
> > > On Thu, Mar 7, 2024 at 4:37 AM Peter Xu <peterx@redhat.com> wrote:
> > > >
> > > > On Thu, Mar 07, 2024 at 02:41:37AM +0000, Zhijian Li (Fujitsu) via 
> > > > wrote:
> > > > > Yu,
> > > > >
> > > > >
> > > > > On 07/03/2024 00:30, Philippe Mathieu-Daudé wrote:
> > > > > > Cc'ing RDMA migration reviewers/maintainers:
> > > > > >
> > > > > > $ ./scripts/get_maintainer.pl -f migration/rdma.c
> > > > > > Li Zhijian <lizhijian@fujitsu.com> (reviewer:RDMA Migration)
> > > > > > Peter Xu <peterx@redhat.com> (maintainer:Migration)
> > > > > > Fabiano Rosas <farosas@suse.de> (maintainer:Migration)
> > > > > >
> > > > > > On 5/3/24 22:32, Yu Zhang wrote:
> > > > > >> Hello Het and all,
> > > > > >>
> > > > > >> while I was testing qemu-8.2, I saw a lot of our migration test 
> > > > > >> cases failed.
> > > > > >> After debugging the commits of the 8.2 branch, I saw the issue and 
> > > > > >> mad a diff:
> > > > > >>
> > > > > >> diff --git a/migration/rdma.c b/migration/rdma.c
> > > > > >> index 6a29e53daf..f10d56f556 100644
> > > > > >> --- a/migration/rdma.c
> > > > > >> +++ b/migration/rdma.c
> > > > > >> @@ -3353,9 +3353,9 @@ static int qemu_rdma_accept(RDMAContext 
> > > > > >> *rdma)
> > > > > >>           goto err_rdma_dest_wait;
> > > > > >>       }
> > > > > >>
> > > > > >> -    isock->host = rdma->host;
> > > > > >> +    isock->host = g_strdup_printf("%s", rdma->host);
> > > > > >>       isock->port = g_strdup_printf("%d", rdma->port);
> > > > >
> > > > >
> > > > > Thanks for your analysis.
> > > > >
> > > > > It will be great if you send this as a patch.
> > > > >
> > > > >
> > > > > isock is defined as a _autoptr VVV
> > > > > 3333 _autoptr(InetSocketAddress) isock = g_new0(InetSocketAddress, 1);
> > > > >
> > > > > I'm surprised that it seems the auto free scheme will free the member 
> > > > > of isock as well
> > > > > see below valrind log. That will cause a double free.
> > > >
> > > > Right, all the QAPI-free is a deep one.  Thanks for checking this up,
> > > > Zhijian.
> > > >
> > > > Yu, would you please send a formal patch (better before this week ends) 
> > > > so
> > > > that I can include it for the last pull for 9.0 soft-freeze (March 
> > > > 12th)?
> > > > As 8.2 affected, please also attach proper tags:
> > > >
> > > > Cc: qemu-stable <qemu-stable@nongnu.org>
> > > > Fixes: 3fa9642ff7 ("migration: convert rdma backend to accept 
> > > > MigrateAddress")
> > > >
> > > > >
> > > > > ==809138== Invalid free() / delete / delete[] / realloc()
> > > > > ==809138==    at 0x483A9F5: free (vg_replace_malloc.c:538)
> > > > > ==809138==    by 0x598F70C: g_free (in 
> > > > > /usr/lib64/libglib-2.0.so.0.6600.8)
> > > > > ==809138==    by 0x79B6AD: qemu_rdma_cleanup (rdma.c:2432)
> > > > > ==809138==    by 0x79CEE6: qio_channel_rdma_close_rcu (rdma.c:3108)
> > > > > ==809138==    by 0xC2E339: call_rcu_thread (rcu.c:301)
> > > > > ==809138==    by 0xC2116A: qemu_thread_start (qemu-thread-posix.c:541)
> > > > > ==809138==    by 0x72683F8: ??? (in /usr/lib64/libpthread-2.32.so)
> > > > > ==809138==    by 0x73824C2: clone (in /usr/lib64/libc-2.32.so)
> > > > > ==809138==  Address 0x13daa070 is 0 bytes inside a block of size 14 
> > > > > free'd
> > > > > ==809138==    at 0x483A9F5: free (vg_replace_malloc.c:538)
> > > > > ==809138==    by 0x598F70C: g_free (in 
> > > > > /usr/lib64/libglib-2.0.so.0.6600.8)
> > > > > ==809138==    by 0xC058CF: qapi_dealloc_type_str 
> > > > > (qapi-dealloc-visitor.c:68)
> > > > > ==809138==    by 0xC09EF3: visit_type_str (qapi-visit-core.c:349)
> > > > > ==809138==    by 0xBDDECC: visit_type_InetSocketAddressBase_members 
> > > > > (qapi-visit-sockets.c:29)
> > > > > ==809138==    by 0xBDE055: visit_type_InetSocketAddress_members 
> > > > > (qapi-visit-sockets.c:67)
> > > > > ==809138==    by 0xBDE30D: visit_type_InetSocketAddress 
> > > > > (qapi-visit-sockets.c:119)
> > > > > ==809138==    by 0xBDDB38: qapi_free_InetSocketAddress 
> > > > > (qapi-types-sockets.c:51)
> > > > > ==809138==    by 0x792351: glib_autoptr_clear_InetSocketAddress 
> > > > > (qapi-types-sockets.h:109)
> > > > > ==809138==    by 0x79236F: glib_autoptr_cleanup_InetSocketAddress 
> > > > > (qapi-types-sockets.h:109)
> > > > > ==809138==    by 0x79D956: qemu_rdma_accept (rdma.c:3341)
> > > > > ==809138==    by 0x79F05A: rdma_accept_incoming_migration 
> > > > > (rdma.c:4041)
> > > > > ==809138==  Block was alloc'd at
> > > > > ==809138==    at 0x4839809: malloc (vg_replace_malloc.c:307)
> > > > > ==809138==    by 0x5992BB8: g_malloc (in 
> > > > > /usr/lib64/libglib-2.0.so.0.6600.8)
> > > > > ==809138==    by 0x59A7FE3: g_strdup (in 
> > > > > /usr/lib64/libglib-2.0.so.0.6600.8)
> > > > > ==809138==    by 0x79C2A8: qemu_rdma_data_init (rdma.c:2731)
> > > > > ==809138==    by 0x79F183: rdma_start_incoming_migration (rdma.c:4081)
> > > > > ==809138==    by 0x76F200: qemu_start_incoming_migration 
> > > > > (migration.c:581)
> > > > > ==809138==    by 0x77193A: qmp_migrate_incoming (migration.c:1735)
> > > > > ==809138==    by 0x74B3D3: qmp_x_exit_preconfig (vl.c:2718)
> > > > > ==809138==    by 0x74DB6F: qemu_init (vl.c:3753)
> > > > > ==809138==    by 0xA14F3F: main (main.c:47)
> > > >
> > > > --
> > > > Peter Xu
> > > >
> >
> >
> >
> > --
> >
> > Alexei Pastuchov
> >
> > Senior Linux Systems Developer
> > Compute Platform Development Cloud
> >
> > IONOS SE | Revaler Str. 28-31 | 10245 Berlin | Deutschland
> > E-Mail: alexei.pastuchov@ionos.com | Web: www.ionos.de
> >
> > Hauptsitz Montabaur, Amtsgericht Montabaur, HRB 24498
> >
> > Vorstand: Hüseyin Dogan, Claudia Frese, Arthur Mai, Dr. Markus Noga,
> > Dr. Jens-Christian Reich, Britta Schmidt, Achim Weiß
> > Aufsichtsratsvorsitzender: Sven Fritz
> >
> > Member of United Internet
> >
> > Diese E-Mail kann vertrauliche und/oder gesetzlich geschützte
> > Informationen enthalten. Wenn Sie nicht der bestimmungsgemäße Adressat
> > sind oder diese E-Mail irrtümlich erhalten haben, unterrichten Sie
> > bitte den Absender und vernichten Sie diese E-Mail. Anderen als dem
> > bestimmungsgemäßen Adressaten ist untersagt, diese E-Mail zu
> > speichern, weiterzuleiten oder ihren Inhalt auf welche Weise auch
> > immer zu verwenden.
> >
> > This e-mail may contain confidential and/or privileged information. If
> > you are not the intended recipient of this e-mail, you are hereby
> > notified that saving, distribution or use of the content of this
> > e-mail in any way is prohibited. If you have received this e-mail in
> > error, please notify the sender and delete the e-mail.