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

Reply via email to