[v1] tests/cdrom-test: Add cdrom test for LoongArch virt machine

[PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by Bibo Mao 9 months, 3 weeks ago

The cdrom test skips to execute on LoongArch system with command
"make check", this patch enables cdrom test for LoongArch virt
machine platform.

With this patch, cdrom test passes to run on LoongArch virt
machine type.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 tests/qtest/cdrom-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 0945383789..c8b97d8d9a 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -271,6 +271,9 @@ int main(int argc, char **argv)
             const char *virtmachine[] = { "virt", NULL };
             add_cdrom_param_tests(virtmachine);
         }
+    } else if (g_str_equal(arch, "loongarch64")) {
+        const char *virtmachine[] = { "virt", NULL };
+        add_cdrom_param_tests(virtmachine);
     } else {
         const char *nonemachine[] = { "none", NULL };
         add_cdrom_param_tests(nonemachine);
-- 
2.39.3

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago

Hi Bibo,

On 5/2/24 03:13, Bibo Mao wrote:
> The cdrom test skips to execute on LoongArch system with command
> "make check", this patch enables cdrom test for LoongArch virt
> machine platform.
> 
> With this patch, cdrom test passes to run on LoongArch virt
> machine type.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   tests/qtest/cdrom-test.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 0945383789..c8b97d8d9a 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>               const char *virtmachine[] = { "virt", NULL };
>               add_cdrom_param_tests(virtmachine);
>           }
> +    } else if (g_str_equal(arch, "loongarch64")) {
> +        const char *virtmachine[] = { "virt", NULL };
> +        add_cdrom_param_tests(virtmachine);

What is the default device used, virtio-blk-pci?

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by maobibo 9 months, 3 weeks ago

Hi Philippe,

On 2024/2/5 下午8:58, Philippe Mathieu-Daudé wrote:
> Hi Bibo,
> 
> On 5/2/24 03:13, Bibo Mao wrote:
>> The cdrom test skips to execute on LoongArch system with command
>> "make check", this patch enables cdrom test for LoongArch virt
>> machine platform.
>>
>> With this patch, cdrom test passes to run on LoongArch virt
>> machine type.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   tests/qtest/cdrom-test.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>> index 0945383789..c8b97d8d9a 100644
>> --- a/tests/qtest/cdrom-test.c
>> +++ b/tests/qtest/cdrom-test.c
>> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>>               const char *virtmachine[] = { "virt", NULL };
>>               add_cdrom_param_tests(virtmachine);
>>           }
>> +    } else if (g_str_equal(arch, "loongarch64")) {
>> +        const char *virtmachine[] = { "virt", NULL };
>> +        add_cdrom_param_tests(virtmachine);
> 
> What is the default device used, virtio-blk-pci?
> 
yes, it is. For virt machine type, the default type for block device is
virtio interface, and it is defined at function loongarch_class_init().
    mc->block_default_type = IF_VIRTIO

Regards
Bibo Mao

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by Thomas Huth 9 months, 3 weeks ago

On 06/02/2024 03.29, maobibo wrote:
> Hi Philippe,
> 
> On 2024/2/5 下午8:58, Philippe Mathieu-Daudé wrote:
>> Hi Bibo,
>>
>> On 5/2/24 03:13, Bibo Mao wrote:
>>> The cdrom test skips to execute on LoongArch system with command
>>> "make check", this patch enables cdrom test for LoongArch virt
>>> machine platform.
>>>
>>> With this patch, cdrom test passes to run on LoongArch virt
>>> machine type.
>>>
>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>> ---
>>>   tests/qtest/cdrom-test.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>>> index 0945383789..c8b97d8d9a 100644
>>> --- a/tests/qtest/cdrom-test.c
>>> +++ b/tests/qtest/cdrom-test.c
>>> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>>>               const char *virtmachine[] = { "virt", NULL };
>>>               add_cdrom_param_tests(virtmachine);
>>>           }
>>> +    } else if (g_str_equal(arch, "loongarch64")) {
>>> +        const char *virtmachine[] = { "virt", NULL };
>>> +        add_cdrom_param_tests(virtmachine);
>>
>> What is the default device used, virtio-blk-pci?
>>
> yes, it is. For virt machine type, the default type for block device is
> virtio interface, and it is defined at function loongarch_class_init().
>     mc->block_default_type = IF_VIRTIO

Ok, then you might need to check whether your patch still works when you run 
"configure" with "--without-default-devices". You might need to check with 
'if (qtest_has_device("virtio-blk-pci"))' whether the device is really 
available in the binary, like it is done some lines earlier in the arm case.

  Thomas

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by maobibo 9 months, 1 week ago


On 2024/2/6 下午5:20, Thomas Huth wrote:
> On 06/02/2024 03.29, maobibo wrote:
>> Hi Philippe,
>>
>> On 2024/2/5 下午8:58, Philippe Mathieu-Daudé wrote:
>>> Hi Bibo,
>>>
>>> On 5/2/24 03:13, Bibo Mao wrote:
>>>> The cdrom test skips to execute on LoongArch system with command
>>>> "make check", this patch enables cdrom test for LoongArch virt
>>>> machine platform.
>>>>
>>>> With this patch, cdrom test passes to run on LoongArch virt
>>>> machine type.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>>   tests/qtest/cdrom-test.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>>>> index 0945383789..c8b97d8d9a 100644
>>>> --- a/tests/qtest/cdrom-test.c
>>>> +++ b/tests/qtest/cdrom-test.c
>>>> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>>>>               const char *virtmachine[] = { "virt", NULL };
>>>>               add_cdrom_param_tests(virtmachine);
>>>>           }
>>>> +    } else if (g_str_equal(arch, "loongarch64")) {
>>>> +        const char *virtmachine[] = { "virt", NULL };
>>>> +        add_cdrom_param_tests(virtmachine);
>>>
>>> What is the default device used, virtio-blk-pci?
>>>
>> yes, it is. For virt machine type, the default type for block device is
>> virtio interface, and it is defined at function loongarch_class_init().
>>     mc->block_default_type = IF_VIRTIO
> 
> Ok, then you might need to check whether your patch still works when you 
> run "configure" with "--without-default-devices". You might need to 
> check with 'if (qtest_has_device("virtio-blk-pci"))' whether the device 
> is really available in the binary, like it is done some lines earlier in 
> the arm case.
Sorry for the late response. Yes cdrom test case will report failure 
with it is compiled with --without-default-devices option.

I will refresh the patch like arm case.

Regards
Bibo Mao
> 
>   Thomas
>

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by Thomas Huth 9 months, 3 weeks ago

On 05/02/2024 03.13, Bibo Mao wrote:
> The cdrom test skips to execute on LoongArch system with command
> "make check"

Are you sure the test is marked with "skip"? ... it should at least test 
with the "none" machine...?

> this patch enables cdrom test for LoongArch virt
> machine platform.
> 
> With this patch, cdrom test passes to run on LoongArch virt
> machine type.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   tests/qtest/cdrom-test.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 0945383789..c8b97d8d9a 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>               const char *virtmachine[] = { "virt", NULL };
>               add_cdrom_param_tests(virtmachine);
>           }
> +    } else if (g_str_equal(arch, "loongarch64")) {
> +        const char *virtmachine[] = { "virt", NULL };
> +        add_cdrom_param_tests(virtmachine);
>       } else {
>           const char *nonemachine[] = { "none", NULL };
>           add_cdrom_param_tests(nonemachine);

Anyway, using the virt machine is certainly better than the "none" machine, so:
Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by maobibo 9 months, 3 weeks ago

Hi Thomas,

On 2024/2/5 下午3:47, Thomas Huth wrote:
> On 05/02/2024 03.13, Bibo Mao wrote:
>> The cdrom test skips to execute on LoongArch system with command
>> "make check"
> 
> Are you sure the test is marked with "skip"? ... it should at least test 
> with the "none" machine...?
>
I check again, cdrom test case passes to run with "none" machine. And 
the root cause is that xorriso rpm package is missing so it is skipped.

Regards
Bibo Mao

>> this patch enables cdrom test for LoongArch virt
>> machine platform.
>>
>> With this patch, cdrom test passes to run on LoongArch virt
>> machine type.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   tests/qtest/cdrom-test.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>> index 0945383789..c8b97d8d9a 100644
>> --- a/tests/qtest/cdrom-test.c
>> +++ b/tests/qtest/cdrom-test.c
>> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>>               const char *virtmachine[] = { "virt", NULL };
>>               add_cdrom_param_tests(virtmachine);
>>           }
>> +    } else if (g_str_equal(arch, "loongarch64")) {
>> +        const char *virtmachine[] = { "virt", NULL };
>> +        add_cdrom_param_tests(virtmachine);
>>       } else {
>>           const char *nonemachine[] = { "none", NULL };
>>           add_cdrom_param_tests(nonemachine);
> 
> Anyway, using the virt machine is certainly better than the "none" 
> machine, so:
> Acked-by: Thomas Huth <thuth@redhat.com>
>

Re: [PATCH] tests/cdrom-test: Add cdrom test for LoongArch virt machine

Posted by maobibo 9 months, 3 weeks ago


On 2024/2/5 下午3:47, Thomas Huth wrote:
> On 05/02/2024 03.13, Bibo Mao wrote:
>> The cdrom test skips to execute on LoongArch system with command
>> "make check"
> 
> Are you sure the test is marked with "skip"? ... it should at least test 
> with the "none" machine...?
With the latest code, cdrom testcase passes to run. It is strange that 
about two weeks ago, the result displays SKIP, so I post this patch -:)

Regards
Bibo Mao

> 
>> this patch enables cdrom test for LoongArch virt
>> machine platform.
>>
>> With this patch, cdrom test passes to run on LoongArch virt
>> machine type.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   tests/qtest/cdrom-test.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
>> index 0945383789..c8b97d8d9a 100644
>> --- a/tests/qtest/cdrom-test.c
>> +++ b/tests/qtest/cdrom-test.c
>> @@ -271,6 +271,9 @@ int main(int argc, char **argv)
>>               const char *virtmachine[] = { "virt", NULL };
>>               add_cdrom_param_tests(virtmachine);
>>           }
>> +    } else if (g_str_equal(arch, "loongarch64")) {
>> +        const char *virtmachine[] = { "virt", NULL };
>> +        add_cdrom_param_tests(virtmachine);
>>       } else {
>>           const char *nonemachine[] = { "none", NULL };
>>           add_cdrom_param_tests(nonemachine);
> 
> Anyway, using the virt machine is certainly better than the "none" 
> machine, so:
> Acked-by: Thomas Huth <thuth@redhat.com>