Re: [PATCH] tests/x86: Add 'q35' machine type to hotplug tests

[Michael Labiuk]

On 8/26/22 15:39, Denis V. Lunev wrote:
> On 26.08.2022 13:41, Michael Labiuk wrote:
> > +static void test_q35_pci_unplug_request(void)
> that seems a little bit wrong. we have pcie test and thus
> the naming should be appropriate.
>
> What about test_pcie_unplug_request()?

I don't think so. Device type remains PCI. Only bridge configuration to 
allow hotplug.

> > diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
> > index 5e6d58b4dd..3a2ddecf22 100644
> > --- a/tests/qtest/drive_del-test.c
> > +++ b/tests/qtest/drive_del-test.c
> > @@ -258,6 +258,27 @@ static void test_cli_device_del(void)
> >       qtest_quit(qts);
> >   }
>
> this patch seems trashes the internal structure of the test.
> originally it was unified for all archs through
>    qvirtio_get_dev_type(void)
> and this change makes the test non-uniform.
> This should be rethinked

We have to add test with explicit machine type. Existing test works fine 
but use default machine type. Type returned by 
qvirtio_get_dev_type(void) is the same for 'pc' and 'q35' machine types.
May be better to create a new test instead of extending drive-del-test. 
Can you make suggestion?

> > diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
> > index e23a97fa8e..c4ca7efc62 100644
> > --- a/tests/qtest/ivshmem-test.c
> > +++ b/tests/qtest/ivshmem-test.c
> > @@ -378,6 +378,32 @@ static void test_ivshmem_server(void)
> >       close(thread.pipe[0]);
> >   }
> > +static void device_del(QTestState *qtest, const char *id)
> > +{
> > +    QDict *resp;
> > +
> > +    resp = qtest_qmp(qtest,
> > +                     "{'execute': 'device_del',"
> > +                     " 'arguments': { 'id': %s } }", id);
> > +
> > +    g_assert(qdict_haskey(resp, "return"));
> > +    qobject_unref(resp);
> > +}
> > +
> > +static void test_ivshmem_hotplug_q35(void)
> > +{
> > +    QTestState *qts = qtest_init("-object 
> > memory-backend-ram,size=1M,id=mb1 "
> > +                                 "-device pcie-root-port,id=p1 "
> > +                                 "-device pcie-pci-bridge,bus=p1,id=b1 "
> > +                                 "-machine q35");
> > +
> > +    qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
> > +                         "{'memdev': 'mb1', 'bus': 'b1'}");
> > +    device_del(qts, "iv1");
> > +
> > +    qtest_quit(qts);
> > +}
> > +
> I think that we need something like we have observed in 
> tests/qtest/drive_del-test.c
> to avoid copy/pasted code
Could you explain where you find duplicate?
We can't select behavior because we do not want to replace test for 
default machine type. Needed new test for 'q35'.

Thanks,
Michael