[v1] qtests vs. default devices

[PATCH 06/12] tests/qtest: Skip unplug tests that use missing devices

Posted by Fabiano Rosas 3 years ago

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 5a6afa2b57..931acbdf50 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -67,6 +67,11 @@ static void test_pci_unplug_request(void)
     const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
+    if (!qtest_has_device("virtio-mouse-pci")) {
+        g_test_skip("Device virtio-mouse-pci not available");
+        return;
+    }
+
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         machine_addition = "-machine pc";
     }
@@ -81,6 +86,10 @@ static void test_pci_unplug_request(void)
 
 static void test_q35_pci_unplug_request(void)
 {
+    if (!qtest_has_device("virtio-mouse-pci")) {
+        g_test_skip("Device virtio-mouse-pci not available");
+        return;
+    }
 
     QTestState *qtest = qtest_initf("-machine q35 "
                                     "-device pcie-root-port,id=p1 "
@@ -97,6 +106,11 @@ static void test_pci_unplug_json_request(void)
     const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
+    if (!qtest_has_device("virtio-mouse-pci")) {
+        g_test_skip("Device virtio-mouse-pci not available");
+        return;
+    }
+
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         machine_addition = "-machine pc";
     }
@@ -123,6 +137,11 @@ static void test_q35_pci_unplug_json_request(void)
                                     "'bus': 'b1', "
                                     "'id': 'dev0'}\"";
 
+    if (!qtest_has_device("virtio-mouse-pci")) {
+        g_test_skip("Device virtio-mouse-pci not available");
+        return;
+    }
+
     QTestState *qtest = qtest_initf("-machine q35 %s %s %s",
                                     port, bridge, device);
 
-- 
2.35.3

Re: [PATCH 06/12] tests/qtest: Skip unplug tests that use missing devices

Posted by Thomas Huth 3 years ago

On 06/02/2023 16.04, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
> index 5a6afa2b57..931acbdf50 100644
> --- a/tests/qtest/device-plug-test.c
> +++ b/tests/qtest/device-plug-test.c
> @@ -67,6 +67,11 @@ static void test_pci_unplug_request(void)
>       const char *arch = qtest_get_arch();
>       const char *machine_addition = "";
>   
> +    if (!qtest_has_device("virtio-mouse-pci")) {
> +        g_test_skip("Device virtio-mouse-pci not available");
> +        return;
> +    }
> +
>       if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>           machine_addition = "-machine pc";
>       }
> @@ -81,6 +86,10 @@ static void test_pci_unplug_request(void)
>   
>   static void test_q35_pci_unplug_request(void)
>   {
> +    if (!qtest_has_device("virtio-mouse-pci")) {
> +        g_test_skip("Device virtio-mouse-pci not available");
> +        return;
> +    }
>   
>       QTestState *qtest = qtest_initf("-machine q35 "
>                                       "-device pcie-root-port,id=p1 "

This seems to break the QEMU coding style ("Mixed declarations (interleaving 
statements and declarations within blocks) are generally not allowed; 
declarations should be at the beginning
of blocks.") ... could you separate the declaration of qtest from its 
initialization now, please?

  Thomas

Re: [PATCH 06/12] tests/qtest: Skip unplug tests that use missing devices

Posted by Fabiano Rosas 3 years ago

Thomas Huth <thuth@redhat.com> writes:

> On 06/02/2023 16.04, Fabiano Rosas wrote:
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>> 
>> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
>> index 5a6afa2b57..931acbdf50 100644
>> --- a/tests/qtest/device-plug-test.c
>> +++ b/tests/qtest/device-plug-test.c
>> @@ -67,6 +67,11 @@ static void test_pci_unplug_request(void)
>>       const char *arch = qtest_get_arch();
>>       const char *machine_addition = "";
>>   
>> +    if (!qtest_has_device("virtio-mouse-pci")) {
>> +        g_test_skip("Device virtio-mouse-pci not available");
>> +        return;
>> +    }
>> +
>>       if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>           machine_addition = "-machine pc";
>>       }
>> @@ -81,6 +86,10 @@ static void test_pci_unplug_request(void)
>>   
>>   static void test_q35_pci_unplug_request(void)
>>   {
>> +    if (!qtest_has_device("virtio-mouse-pci")) {
>> +        g_test_skip("Device virtio-mouse-pci not available");
>> +        return;
>> +    }
>>   
>>       QTestState *qtest = qtest_initf("-machine q35 "
>>                                       "-device pcie-root-port,id=p1 "
>
> This seems to break the QEMU coding style ("Mixed declarations (interleaving 
> statements and declarations within blocks) are generally not allowed; 
> declarations should be at the beginning
> of blocks.") ... could you separate the declaration of qtest from its 
> initialization now, please?

Ah well spotted, I got thrown off because some of these tests already
have a:

    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
        machine_addition = "-machine pc";
    }

    QTestState *qtest = qtest_initf...

I'll fix those as well.

Re: [PATCH 06/12] tests/qtest: Skip unplug tests that use missing devices

Posted by Thomas Huth 3 years ago

On 07/02/2023 15.17, Fabiano Rosas wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 06/02/2023 16.04, Fabiano Rosas wrote:
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> ---
>>>    tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
>>>    1 file changed, 19 insertions(+)
>>>
>>> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
>>> index 5a6afa2b57..931acbdf50 100644
>>> --- a/tests/qtest/device-plug-test.c
>>> +++ b/tests/qtest/device-plug-test.c
>>> @@ -67,6 +67,11 @@ static void test_pci_unplug_request(void)
>>>        const char *arch = qtest_get_arch();
>>>        const char *machine_addition = "";
>>>    
>>> +    if (!qtest_has_device("virtio-mouse-pci")) {
>>> +        g_test_skip("Device virtio-mouse-pci not available");
>>> +        return;
>>> +    }
>>> +
>>>        if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>>            machine_addition = "-machine pc";
>>>        }
>>> @@ -81,6 +86,10 @@ static void test_pci_unplug_request(void)
>>>    
>>>    static void test_q35_pci_unplug_request(void)
>>>    {
>>> +    if (!qtest_has_device("virtio-mouse-pci")) {
>>> +        g_test_skip("Device virtio-mouse-pci not available");
>>> +        return;
>>> +    }
>>>    
>>>        QTestState *qtest = qtest_initf("-machine q35 "
>>>                                        "-device pcie-root-port,id=p1 "
>>
>> This seems to break the QEMU coding style ("Mixed declarations (interleaving
>> statements and declarations within blocks) are generally not allowed;
>> declarations should be at the beginning
>> of blocks.") ... could you separate the declaration of qtest from its
>> initialization now, please?
> 
> Ah well spotted, I got thrown off because some of these tests already
> have a:
> 
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          machine_addition = "-machine pc";
>      }
> 
>      QTestState *qtest = qtest_initf...
> 
> I'll fix those as well.

Yes, please.

Actually, I wonder whether we could remove those "-machine pc" lines again, 
since "pc" is the default machine anyway. I think the original idea here was 
to get rid of the default machine on x86 or to switch it to q35, but that 
never happened, so this code seems superfluous now. Anyway, maybe rather 
something for a separate patch later...

  Thomas