Re: test regression - qemu:block-slow+slow / io-raw-055

[Pierrick Bouvier]

On 11/5/24 08:41, Peter Xu wrote:
> On Mon, Nov 04, 2024 at 07:41:44PM -0800, Pierrick Bouvier wrote:
> > Hi,
> >
> > this test was recently broken by 34a889 (migration: Drop
> > migration_is_idle()).
> >
> > Reproduce with:
> > meson test -C build -t 1 --setup slow  --num-processes 1 --print-errorlogs
> > io-raw-055 --verbose
> >
> > 1/1 qemu:block-slow+slow / io-raw-055        RUNNING
> > > > > ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 
> > > > > UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
> > MALLOC_PERTURB_=29 MESON_TEST_ITERATION=1
> > PYTHON=/home/user/.work/qemu/build/pyvenv/bin/python3 G_TEST_SLOW=1
> > SPEED=slow 
> > MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
> > /home/user/.work/qemu/build/pyvenv/bin/python3
> > /home/user/.work/qemu/build/../tests/qemu-iotests/check -tap -raw 055
> > --source-dir /home/user/.work/qemu/tests/qemu-iotests --build-dir
> > /home/user/.work/qemu/build/tests/qemu-iotests
> > ▶ 1/1 raw 055                                FAIL
> > 1/1 qemu:block-slow+slow / io-raw-055        ERROR           11.06s exit
> > status 1
> > ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> > ✀ 
> > ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> > stderr:
> > --- /home/user/.work/qemu/tests/qemu-iotests/055.out
> > +++ /home/user/.work/qemu/build/scratch/raw-file-055/055.out.bad
> > @@ -1,5 +1,215 @@
> > -........................................
> > +......ERROR:qemu.qmp.qmp_client.qemu-1856388:Failed to receive Greeting:
> > EOFError
> > +ERROR:qemu.qmp.qmp_client.qemu-1856388:Failed to establish session:
> > EOFError
>
> Thanks for the report.
>
> I don't know how this was triggered, looks like current_migration wasn't
> set at all for some reason..
>
> Could you try below change to see whether it can fix there too (it worked
> here)?
>
> ===8<===
>  From d33ad33854d36c04260150ee817b984f48da46c6 Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Tue, 5 Nov 2024 11:29:07 -0500
> Subject: [PATCH] migration: Check current_migration in migration_is_running()
>
> Report shows that commit 34a8892dec broke iotest 055:
>
> b8806360-a2b6-4608-83a3-db67e264c733@linaro.org">https://lore.kernel.org/r/b8806360-a2b6-4608-83a3-db67e264c733@linaro.org
>
> When replacing migration_is_idle() with "!migration_is_running()", it was
> overlooked that the idle helper also checks for current_migration being
> available first.
>
> The check would be there if the whole series was applied, but since the
> last patches in the previous series rely on some other patches to land
> first, we need to recover the behavior of migration_is_idle() first before
> that whole set will be merged.
>
> I left migration_is_active / migration_is_device alone, as I don't think
> it's possible for them to hit his case (current_migration not initialized).
> Also they're prone to removal soon from VFIO side.
>
> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Cc: Fabiano Rosas <farosas@suse.de>
> Fixes: 34a8892dec ("migration: Drop migration_is_idle()")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   migration/migration.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index aedf7f0751..8c5bd0a75c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1117,6 +1117,10 @@ bool migration_is_running(void)
>   {
>       MigrationState *s = current_migration;
>   
> +    if (!s) {
> +        return false;
> +    }
> +
>       switch (s->state) {
>       case MIGRATION_STATUS_ACTIVE:
>       case MIGRATION_STATUS_POSTCOPY_ACTIVE:

Hi Peter,
I confirm this fix the test mentioned. You can post this patch.

Regards,
Pierrick