On Thu, Nov 06, 2025 at 12:27:49PM +0100, Juraj Marcin wrote:
> On 2025-11-05 14:52, Peter Xu wrote:
> > On Tue, Nov 04, 2025 at 01:35:53PM +0100, Juraj Marcin wrote:
> > > Hi Peter,
> > > 
> > > On 2025-10-31 12:49, Peter Xu wrote:
> > > > error-desc should present on dest QEMU after migration failed on dest 
> > > > when
> > > > exit-on-error is set to FALSE.  Check the error message.
> > > > 
> > > > Signed-off-by: Peter Xu <[email protected]>
> > > > ---
> > > >  tests/qtest/migration/precopy-tests.c | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > > 
> > > > diff --git a/tests/qtest/migration/precopy-tests.c 
> > > > b/tests/qtest/migration/precopy-tests.c
> > > > index 57ca623de5..5f02e35324 100644
> > > > --- a/tests/qtest/migration/precopy-tests.c
> > > > +++ b/tests/qtest/migration/precopy-tests.c
> > > > @@ -759,6 +759,14 @@ static void test_cancel_src_after_none(QTestState 
> > > > *from, QTestState *to,
> > > >      wait_for_migration_complete(to);
> > > >  }
> > > >  
> > > > +static void assert_migration_error(QTestState *vm)
> > > > +{
> > > > +    QDict *rep = migrate_query(vm);
> > > > +
> > > > +    g_assert(qdict_get_str(rep, "error-desc"));
> > > 
> > > I think it would be beneficial to also check if there even is
> > > "error-desc". That way if the "error-desc" is missing, it fails on
> > > assert with SIGABRT instead of SIGSEGV inside qdict_get_str().
> > 
> > IMHO it doesn't matter on how the test would crash.
> > 
> > > 
> > > With this change you can add my:
> > > 
> > > Reviewed-by: Juraj Marcin <[email protected]>
> > 
> > I would go ahead and merge a test patch if it had both lines, definitely
> > not a huge deal.
> > 
> > However strictly speaking, qdict_get_str() is actually pretty efficient to
> > make sure both that exists && is_string when used in testings. Would you
> > agree?
> 
> It is an efficient way, I just thought the less efficient might be a
> little bit easier to deduce why the test failed. But if nobody else
> opposes, you can also keep it as proposed,
> 
> > 
> > I definitely still want your R-b one way or another!
> 
> and also add my R-b.

Thanks.  I wanted to have this coverage asap, so I collected it for -rc.

-- 
Peter Xu


Reply via email to