Re: [Qemu-devel] [PATCH v2] tests: Enable the drive_del test also on s390x

[David Hildenbrand]

>  static void test_drive_del_device_del(void)
>  {
> +    char *args;
> +
>      /* Start with a drive used by a device that unplugs instantaneously */
> -    qtest_start("-drive if=none,id=drive0,file=null-co://,format=raw"
> -                " -device virtio-scsi-pci"
> -                " -device scsi-hd,drive=drive0,id=dev0");
> +    args = g_strdup_printf("-drive 
> if=none,id=drive0,file=null-co://,format=raw"
> +                           " -device virtio-scsi-%s"
> +                           " -device scsi-hd,drive=drive0,id=dev0",

Would look better with the spaces at the end of the previous line (so
all "-device" are aligned), but just my taste.

> +                           qvirtio_get_dev_type());
> +    qtest_start(args);
>  
>      /*
>       * Delete the drive, and then the device
> @@ -104,6 +109,7 @@ static void test_drive_del_device_del(void)
>      device_del();
>  
>      qtest_end();
> +    g_free(args);
>  }
>  
>  int main(int argc, char **argv)
> @@ -114,9 +120,10 @@ int main(int argc, char **argv)
>  
>      qtest_add_func("/drive_del/without-dev", test_drive_without_dev);
>  
> -    /* TODO I guess any arch with PCI would do */
> +    /* TODO I guess any arch with a hot-pluggable virtio bus would do */
>      if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64") ||
> -        !strcmp(arch, "ppc") || !strcmp(arch, "ppc64")) {
> +        !strcmp(arch, "ppc") || !strcmp(arch, "ppc64") ||
> +        !strcmp(arch, "s390x")) {
>          qtest_add_func("/drive_del/after_failed_device_add",
>                         test_after_failed_device_add);
>          qtest_add_func("/blockdev/drive_del_device_del",
> diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
> index 9880a69..0879a62 100644
> --- a/tests/libqos/virtio.c
> +++ b/tests/libqos/virtio.c
> @@ -339,3 +339,20 @@ void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t 
> idx)
>      /* vq->avail->used_event */
>      writew(vq->avail + 4 + (2 * vq->size), idx);
>  }
> +
> +/*
> + * qvirtio_get_dev_type:
> + * Returns: the preferred virtio bus/device type for the current 
> architecture.
> + */
> +const char *qvirtio_get_dev_type(void)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
> +        return "device";  /* for virtio-mmio */
> +    } else if (g_str_equal(arch, "s390x")) {
> +        return "ccw";
> +    } else {
> +        return "pci";
> +    }

You could drop the else case and do it unconditionally.

> +}
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> index 8fbcd18..0a04740 100644
> --- a/tests/libqos/virtio.h
> +++ b/tests/libqos/virtio.h
> @@ -143,4 +143,7 @@ void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, 
> uint32_t free_head);
>  bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx);
>  
>  void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx);
> +
> +const char *qvirtio_get_dev_type(void);
> +
>  #endif
> 


Looks good to me!

Reviewed-by: David Hildenbrand <address@hidden>

-- 

Thanks,

David