[Qemu-devel] [PATCH] Change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Qemu-devel] [PATCH] Change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Dor Laor]

Windows virtio driver cannot pass DTM (certification) tests
while the storage class is PCI_CLASS_STORAGE_UNKNOWN.

Signed-off-by: Dor Laor <dor@redhat.com>
---
 hw/virtio-blk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 51a8e22..6c23ec6 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -250,7 +250,7 @@ void *virtio_blk_init(PCIBus *bus, BlockDriverState *bs)
                                        PCI_DEVICE_ID_VIRTIO_BLOCK,
                                        PCI_VENDOR_ID_REDHAT_QUMRANET,
                                        VIRTIO_ID_BLOCK,
-                                       PCI_CLASS_STORAGE_OTHER, 0x00,
+                                       PCI_CLASS_STORAGE_SCSI, 0x00,
                                        sizeof(struct 
virtio_blk_config), sizeof(VirtIOBlock));
     if (!s)
         return NULL;
-- 
1.5.6.6

Re: [Qemu-devel] [PATCH] Change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Christoph Hellwig]

On Thu, Apr 30, 2009 at 10:13:48PM +0300, Dor Laor wrote:
> Windows virtio driver cannot pass DTM (certification) tests
> while the storage class is PCI_CLASS_STORAGE_UNKNOWN.

Well, it isn't scsi so this is wrong.  Tell these certification people
to fix their test.

Re: [Qemu-devel] [PATCH] Change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Dor Laor]

Christoph Hellwig wrote:
> On Thu, Apr 30, 2009 at 10:13:48PM +0300, Dor Laor wrote:
>   
>> Windows virtio driver cannot pass DTM (certification) tests
>> while the storage class is PCI_CLASS_STORAGE_UNKNOWN.
>>     
>
> Well, it isn't scsi so this is wrong.  Tell these certification people
> to fix their test.
>   
It's also not unknown... someone I know implemented scsi pass through so 
it is
more close to scsi than to unknown..
These certification people are M$ so it is simpler to do this harmless 
change instead
of waiting some light years.

Re: [Qemu-devel] [PATCH] Change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Christoph Hellwig]

On Thu, Apr 30, 2009 at 11:18:16PM +0300, Dor Laor wrote:
> Christoph Hellwig wrote:
> >On Thu, Apr 30, 2009 at 10:13:48PM +0300, Dor Laor wrote:
> >  
> >>Windows virtio driver cannot pass DTM (certification) tests
> >>while the storage class is PCI_CLASS_STORAGE_UNKNOWN.
> >>    
> >
> >Well, it isn't scsi so this is wrong.  Tell these certification people
> >to fix their test.
> >  
> It's also not unknown... someone I know implemented scsi pass through so 
> it is
> more close to scsi than to unknown..
> These certification people are M$ so it is simpler to do this harmless 
> change instead
> of waiting some light years.

It only supports limited scsi cdb passthrough _if_ it's using a raw host
device supporting the SG_IO ioctl on Linux.  That's a lot of ifs and
whens and definitively not anything like a SCSI HBA.  Now fortunately
the pci class IDs don't matter too much in real life, but I'd rather
not change it from the correct to the wrong one.  Especially as there
might be all kinds of weird breakages in existing setups when the pci id
changes underneath on a migration / upgrade of the host.

So if you really need this to please a broken testcase add an option to
qemu to use a different pci id just for this broken certification test,
similarly to various other OS workarounds.

[Qemu-devel] [PATCH]Optionally change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

[Dor Laor]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

Here is a new patch that allows a configurable option to be passed to qemu.
The default will be PCI_CLASS_STORAGE_SCSI. To be able to sustain 
migration from older
qemu's, a parameter - driver ...,class=128 can be passed.
While it's not the most beautiful piece of code, it aligned with current 
qemu code.


[-- Attachment #2: 0001-Optionaly-change-the-virtio-block-storage-class-to-P.patch --]
[-- Type: text/plain, Size: 5293 bytes --]

>From a19863b191e5e32812ddcbd63f88826ff16d2298 Mon Sep 17 00:00:00 2001
From: Dor Laor <dor@redhat.com>
Date: Sun, 24 May 2009 18:01:39 +0300
Subject: [PATCH] Optionally change the virtio-block storage class to PCI_CLASS_STORAGE_SCSI

Windows whql tests does not recognizes the virtio device since its storage
class is PCI_CLASS_STORAGE_OTHER.
Changing the default and allow and option to pass the class as
-drive file=...,class=128.

Signed-off-by: Dor Laor <dor@redhat.com>
---
 hw/virtio-blk.c |    7 +++++++
 hw/virtio-pci.c |    6 +++++-
 hw/virtio.h     |    1 +
 sysemu.h        |    2 ++
 vl.c            |   23 ++++++++++++++++++++++-
 5 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 8dd3c7a..7f89e0d 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -348,6 +348,13 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+int virtio_blk_get_class(DeviceState *dev)
+{
+    VirtIOBlock *s = (VirtIOBlock *)dev;
+
+    return drive_get_class_code(s->bs);
+}
+
 VirtIODevice *virtio_blk_init(DeviceState *dev)
 {
     VirtIOBlock *s;
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c072423..77d4a54 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -14,6 +14,7 @@
  */
 
 #include <inttypes.h>
+#include <sysemu.h>
 
 #include "virtio.h"
 #include "pci.h"
@@ -286,12 +287,15 @@ static void virtio_blk_init_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
     VirtIODevice *vdev;
+    int class_code;
 
     vdev = virtio_blk_init(&pci_dev->qdev);
+    if ((class_code = virtio_blk_get_class(vdev)) == -1)
+        class_code = PCI_CLASS_STORAGE_SCSI;
     virtio_init_pci(proxy, vdev,
                     PCI_VENDOR_ID_REDHAT_QUMRANET,
                     PCI_DEVICE_ID_VIRTIO_BLOCK,
-                    PCI_CLASS_STORAGE_OTHER,
+                    (uint16_t)class_code,
                     0x00);
 }
 
diff --git a/hw/virtio.h b/hw/virtio.h
index 425727e..fd8b7a7 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -151,5 +151,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev);
 VirtIODevice *virtio_net_init(DeviceState *dev);
 VirtIODevice *virtio_console_init(DeviceState *dev);
 VirtIODevice *virtio_balloon_init(DeviceState *dev);
+int virtio_blk_get_class(DeviceState *dev);
 
 #endif
diff --git a/sysemu.h b/sysemu.h
index 92501ed..659e222 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -154,6 +154,7 @@ typedef struct DriveInfo {
     BlockDriverState *bdrv;
     BlockInterfaceType type;
     int bus;
+    int class;
     int unit;
     int used;
     int drive_opt_idx;
@@ -174,6 +175,7 @@ extern void drive_uninit(BlockDriverState *bdrv);
 extern void drive_remove(int index);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
 extern BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv);
+extern int drive_get_class_code(BlockDriverState *bdrv);
 
 BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
 
diff --git a/vl.c b/vl.c
index 2c1f0e0..5c8ac6f 100644
--- a/vl.c
+++ b/vl.c
@@ -2181,6 +2181,17 @@ BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv)
     return BLOCK_ERR_STOP_ENOSPC;
 }
 
+extern int drive_get_class_code(BlockDriverState *bdrv)
+{
+    int index;
+
+    for (index = 0; index < nb_drives; index++)
+        if (drives_table[index].bdrv == bdrv)
+            return drives_table[index].class;
+
+    return -1;
+}
+
 static void bdrv_format_print(void *opaque, const char *name)
 {
     fprintf(stderr, " %s", name);
@@ -2218,13 +2229,14 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
     int index;
     int cache;
     int bdrv_flags, onerror;
+    int class = -1;
     int drives_table_idx;
     char *str = arg->opt;
     static const char * const params[] = { "bus", "unit", "if", "index",
                                            "cyls", "heads", "secs", "trans",
                                            "media", "snapshot", "file",
                                            "cache", "format", "serial", "werror",
-                                           NULL };
+                                           "class", NULL };
 
     if (check_params(params, str) < 0) {
          fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n",
@@ -2355,6 +2367,14 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
 	}
     }
 
+    if (get_param_value(buf, sizeof(buf), "class", str)) {
+        if (type != IF_VIRTIO) {
+            fprintf(stderr, "class is no supported by this format\n");
+            return -1;
+        }
+        class =  strtol(buf, NULL, 0);
+    }
+
     if (get_param_value(buf, sizeof(buf), "media", str)) {
         if (!strcmp(buf, "disk")) {
 	    media = MEDIA_DISK;
@@ -2502,6 +2522,7 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
     drives_table[drives_table_idx].bus = bus_id;
     drives_table[drives_table_idx].unit = unit_id;
     drives_table[drives_table_idx].onerror = onerror;
+    drives_table[drives_table_idx].class = class;
     drives_table[drives_table_idx].drive_opt_idx = arg - drives_opt;
     strncpy(drives_table[drives_table_idx].serial, serial, sizeof(serial));
     nb_drives++;
-- 
1.5.6.6