On Tue, 23 Mar 2021 05:46:24 +0100 Thomas Huth <th...@redhat.com> wrote:
> On 22/03/2021 18.48, Lukas Straub wrote:
> > On Mon, 22 Mar 2021 17:00:23 +0100
> > Thomas Huth <th...@redhat.com> wrote:
> >
> >> On 22/03/2021 08.35, Lukas Straub wrote:
> >>> On Mon, 22 Mar 2021 06:20:50 +0100
> >>> Thomas Huth <th...@redhat.com> wrote:
> >>>
> >>>> On 22/03/2021 00.31, Lukas Straub wrote:
> >>>>> Use the normal yank code instead of stubs in relevant tests to
> >>>>> increase coverage and to ensure that registering and unregistering
> >>>>> of yank instances and functions is done correctly.
> >>>>>
> >>>>> Signed-off-by: Lukas Straub <lukasstra...@web.de>
> >>>>> ---
> >>>>> tests/qtest/meson.build | 6 +++---
> >>>>> tests/unit/meson.build | 4 ++--
> >>>>> 2 files changed, 5 insertions(+), 5 deletions(-)
> >>>>>
> >>>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> >>>>> index 66ee9fbf45..40e1f495f7 100644
> >>>>> --- a/tests/qtest/meson.build
> >>>>> +++ b/tests/qtest/meson.build
> >>>>> @@ -234,9 +234,9 @@ tpmemu_files = ['tpm-emu.c', 'tpm-util.c',
> >>>>> 'tpm-tests.c']
> >>>>> qtests = {
> >>>>> 'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c',
> >>>>> 'tpm-emu.c'],
> >>>>> 'cdrom-test': files('boot-sector.c'),
> >>>>> - 'dbus-vmstate-test': files('migration-helpers.c') + dbus_vmstate1,
> >>>>> + 'dbus-vmstate-test': ['migration-helpers.c', dbus_vmstate1,
> >>>>> '../../monitor/yank.c'],
> >>>>> 'ivshmem-test': [rt,
> >>>>> '../../contrib/ivshmem-server/ivshmem-server.c'],
> >>>>> - 'migration-test': files('migration-helpers.c'),
> >>>>> + 'migration-test': ['migration-helpers.c', io,
> >>>>> '../../monitor/yank.c'],
> >>>>> 'pxe-test': files('boot-sector.c'),
> >>>>> 'qos-test': [chardev, io, qos_test_ss.apply(config_host, strict:
> >>>>> false).sources()],
> >>>>> 'tpm-crb-swtpm-test': [io, tpmemu_files],
> >>>>
> >>>> Is this really necessary for the qtests? I can understand the change for
> >>>> the
> >>>> unit tests, but the qtests are separate programs where I could not
> >>>> imagine
> >>>> that they use the yank functions in any way?
> >>>
> >>> Yes, it is necessary. While the yank functions are not called in these
> >>> tests,
> >>> it still checks that registering and unregistering of yank instances and
> >>> functions is done correctly. I.e. That no yank functions are registered
> >>> before
> >>> the instance, that the yank instance is only unregistered after all
> >>> functions
> >>> where unregistered, that the same instance is not registered twice and
> >>> that
> >>> the yank instance actually exists before it is unregistered.
> >>
> >> Now you even confused me more. Could you elaborate a little bit? If none of
> >> the functions are called by the test, which part of yank.c is excercised
> >> here at all? Could you give a more detailed example? The only thing I could
> >> imagine is yank_init(), but that does not look like something we need to
> >> check in a qtest ?
> >
> > Oh, sorry. I meant yank's concept of a yank function here. It works this
> > way:
> > The different subsystems first register a yank instance. So in this case
> > when starting migration in the test, the migration code first registers a
> > yank instance. Then, it registers _yank functions_ with this instance, for
> > for example to shutdown a socket.
>
> But these are the qtest, separate stand-alone programs. The migration code
> of QEMU (i.e. the code in the main "migration" folder) is not linked into
> these binaries. Doing something like:
>
> grep -r yank tests/qtest/migration-test
>
> should give you zero results. Thus it IMHO does not make sense to add the
> yank.c to these tests here.
>
> Having said that, it seems like the qos-test is linking against the chardev
> code and thus might use indirectly the yank code there. So you maybe might
> want to add it to the qos-test instead?
Ok, now I understand. In that case it doesn't matter if full yank is linked
into qtest.
Regards,
Lukas Straub
> Thomas
>
--
pgpff9PyD8bXU.pgp
Description: OpenPGP digital signature
