| Message ID | 20240823-san-v4-7-a24c6dfa4ceb@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix check-qtest-ppc64 sanitizer errors | expand |
On Fri, Aug 23, 2024 at 03:13:12PM +0900, Akihiko Odaki wrote: > A test run may create boot files several times. Delete the previous boot > file before creating a new one. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Acked-by: Thomas Huth <thuth@redhat.com> I didn't track which came early, but I think Fabiano has queued the other one here: https://lore.kernel.org/r/20240820144912.320744-2-peter.maydell@linaro.org https://gitlab.com/farosas/qemu/-/commits/migration-staging/ So we should be good.
On 26/08/2024 17.26, Peter Xu wrote: > On Fri, Aug 23, 2024 at 03:13:12PM +0900, Akihiko Odaki wrote: >> A test run may create boot files several times. Delete the previous boot >> file before creating a new one. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Acked-by: Thomas Huth <thuth@redhat.com> > > I didn't track which came early, but I think Fabiano has queued the other > one here: > > https://lore.kernel.org/r/20240820144912.320744-2-peter.maydell@linaro.org > https://gitlab.com/farosas/qemu/-/commits/migration-staging/ > > So we should be good. Ooops, sorry, I think I might have jumped the gun and put this patch here in my final PR for 9.1 ... I hope it's ok to do modifications on top of that later if it is still necessary. Thomas
Thomas Huth <thuth@redhat.com> writes: > On 26/08/2024 17.26, Peter Xu wrote: >> On Fri, Aug 23, 2024 at 03:13:12PM +0900, Akihiko Odaki wrote: >>> A test run may create boot files several times. Delete the previous boot >>> file before creating a new one. >>> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >>> Acked-by: Thomas Huth <thuth@redhat.com> >> >> I didn't track which came early, but I think Fabiano has queued the other >> one here: >> >> https://lore.kernel.org/r/20240820144912.320744-2-peter.maydell@linaro.org >> https://gitlab.com/farosas/qemu/-/commits/migration-staging/ >> >> So we should be good. > > Ooops, sorry, I think I might have jumped the gun and put this patch here in > my final PR for 9.1 ... I hope it's ok to do modifications on top of that > later if it is still necessary. It's fine, don't worry. I'll just drop the other one, they're basically the same.
On Tue, 27 Aug 2024 at 15:03, Fabiano Rosas <farosas@suse.de> wrote: > > Thomas Huth <thuth@redhat.com> writes: > > > On 26/08/2024 17.26, Peter Xu wrote: > >> On Fri, Aug 23, 2024 at 03:13:12PM +0900, Akihiko Odaki wrote: > >>> A test run may create boot files several times. Delete the previous boot > >>> file before creating a new one. > >>> > >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > >>> Acked-by: Thomas Huth <thuth@redhat.com> > >> > >> I didn't track which came early, but I think Fabiano has queued the other > >> one here: > >> > >> https://lore.kernel.org/r/20240820144912.320744-2-peter.maydell@linaro.org > >> https://gitlab.com/farosas/qemu/-/commits/migration-staging/ > >> > >> So we should be good. > > > > Ooops, sorry, I think I might have jumped the gun and put this patch here in > > my final PR for 9.1 ... I hope it's ok to do modifications on top of that > > later if it is still necessary. > > It's fine, don't worry. I'll just drop the other one, they're basically > the same. They're not quite the same -- my one also fixes the bug where if we run no tests we call bootfile_delete() without ever having called bootfile_create() and as a result try to unlink(NULL). So if we take Akihiko's patch I'll respin my patch to be just that fix. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 27 Aug 2024 at 15:03, Fabiano Rosas <farosas@suse.de> wrote: >> >> Thomas Huth <thuth@redhat.com> writes: >> >> > On 26/08/2024 17.26, Peter Xu wrote: >> >> On Fri, Aug 23, 2024 at 03:13:12PM +0900, Akihiko Odaki wrote: >> >>> A test run may create boot files several times. Delete the previous boot >> >>> file before creating a new one. >> >>> >> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> >>> Acked-by: Thomas Huth <thuth@redhat.com> >> >> >> >> I didn't track which came early, but I think Fabiano has queued the other >> >> one here: >> >> >> >> https://lore.kernel.org/r/20240820144912.320744-2-peter.maydell@linaro.org >> >> https://gitlab.com/farosas/qemu/-/commits/migration-staging/ >> >> >> >> So we should be good. >> > >> > Ooops, sorry, I think I might have jumped the gun and put this patch here in >> > my final PR for 9.1 ... I hope it's ok to do modifications on top of that >> > later if it is still necessary. >> >> It's fine, don't worry. I'll just drop the other one, they're basically >> the same. > > They're not quite the same -- my one also fixes the bug > where if we run no tests we call bootfile_delete() > without ever having called bootfile_create() and as > a result try to unlink(NULL). So if we take Akihiko's > patch I'll respin my patch to be just that fix. Ah right, there's the other call at the end of main(). I can touch that up in migration-staging if it comes to it. > > thanks > -- PMM
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 70b606b88864..6c06100d91e2 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -144,12 +144,23 @@ static char *bootpath; #include "tests/migration/ppc64/a-b-kernel.h" #include "tests/migration/s390x/a-b-bios.h" +static void bootfile_delete(void) +{ + unlink(bootpath); + g_free(bootpath); + bootpath = NULL; +} + static void bootfile_create(char *dir, bool suspend_me) { const char *arch = qtest_get_arch(); unsigned char *content; size_t len; + if (bootpath) { + bootfile_delete(); + } + bootpath = g_strdup_printf("%s/bootsect", dir); if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { /* the assembled x86 boot sector should be exactly one sector large */ @@ -177,13 +188,6 @@ static void bootfile_create(char *dir, bool suspend_me) fclose(bootfile); } -static void bootfile_delete(void) -{ - unlink(bootpath); - g_free(bootpath); - bootpath = NULL; -} - /* * Wait for some output in the serial output file, * we get an 'A' followed by an endless string of 'B's