[Qemu-devel] [PATCH v6 0/8] virtio-blk refactoring.

[Qemu-devel] [PATCH v6 0/8] virtio-blk refactoring.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This is the next part of virtio-refactoring.

Basically it creates virtio-blk device which extends virtio-device.
Then a virtio-blk can be connected on a virtio-bus.
virtio-blk-pci, virtio-blk-s390x, virtio-blk-ccw are created too, they extend
respectively virtio-pci, virtio-s390-device, virtio-ccw-device and have a
virtio-blk.

It is on top of "virtio: make virtio device's structures public" (V4) I posted
before, but you can checkout my branch here:

git://project.greensocs.com/qemu-virtio.git virtio-blk-v6

I made basic tests (with linux guests) on:
 * qemu-system-i386
 * qemu-system-s390x

Cornelia made virtio-ccw test, and Stefan tried dataplane.

There are no changes, just rebased on the current git.

Thanks.

Fred

KONRAD Frederic (8):
  virtio-blk: don't use pointer for configuration.
  virtio-blk: add the virtio-blk device.
  virtio-blk-pci: switch to new API.
  virtio-blk-s390: switch to the new API.
  virtio-blk-ccw switch to new API.
  virtio-blk: cleanup: init and exit functions.
  virtio-blk: cleanup: QOM cast
  virtio-blk: cleanup: remove qdev field.

 hw/s390x/s390-virtio-bus.c |  30 +++++++----
 hw/s390x/s390-virtio-bus.h |  13 ++++-
 hw/s390x/virtio-ccw.c      |  37 ++++++++-----
 hw/s390x/virtio-ccw.h      |  14 ++++-
 hw/virtio-blk.c            | 131 +++++++++++++++++++++++++++++----------------
 hw/virtio-blk.h            |  33 ++++++++++--
 hw/virtio-pci.c            | 124 +++++++++++++++++++-----------------------
 hw/virtio-pci.h            |  15 +++++-
 hw/virtio.h                |   2 -
 9 files changed, 250 insertions(+), 149 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v6 1/8] virtio-blk: don't use pointer for configuration.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

The configuration field must not be a pointer as it will be used for virtio-blk
properties. So *blk is replaced by blk in VirtIOBlock structure.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 8 ++++----
 hw/virtio-blk.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 6714b01..908c316 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
      */
     req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
 
-    if (!req->dev->blk->scsi) {
+    if (!req->dev->blk.scsi) {
         status = VIRTIO_BLK_S_UNSUPP;
         goto fail;
     }
@@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
          * terminated by '\0' only when shorter than buffer.
          */
         strncpy(req->elem.in_sg[0].iov_base,
-                s->blk->serial ? s->blk->serial : "",
+                s->blk.serial ? s->blk.serial : "",
                 MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
         g_free(req);
@@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
     features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
     features |= (1 << VIRTIO_BLK_F_SCSI);
 
-    if (s->blk->config_wce) {
+    if (s->blk.config_wce) {
         features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
     }
     if (bdrv_enable_write_cache(s->bs))
@@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->vdev.reset = virtio_blk_reset;
     s->bs = blk->conf.bs;
     s->conf = &blk->conf;
-    s->blk = blk;
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 19ec569..b704d50 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -118,7 +118,7 @@ typedef struct VirtIOBlock {
     void *rq;
     QEMUBH *bh;
     BlockConf *conf;
-    VirtIOBlkConf *blk;
+    VirtIOBlkConf blk;
     unsigned short sector_mask;
     DeviceState *qdev;
     VMChangeStateEntry *change;
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Create virtio-blk which extends virtio-device, so it can be connected on
virtio-bus.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/virtio-blk.h | 28 +++++++++++++++++
 hw/virtio-pci.c | 11 +------
 3 files changed, 122 insertions(+), 15 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 908c316..a0d0679 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -21,7 +21,11 @@
 #ifdef __linux__
 # include <scsi/sg.h>
 #endif
+#include "hw/virtio-bus.h"
 
+/*
+ * Moving to QOM later in this series.
+ */
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 {
     return (VirtIOBlock *)vdev;
@@ -620,9 +624,16 @@ static const BlockDevOps virtio_block_ops = {
     .resize_cb = virtio_blk_resize,
 };
 
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
 {
-    VirtIOBlock *s;
+    VirtIOBlock *s = VIRTIO_BLK(dev);
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
+}
+
+static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
+                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
+{
+    VirtIOBlock *s = *ps;
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
@@ -639,9 +650,20 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
         return NULL;
     }
 
-    s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                          sizeof(struct virtio_blk_config),
-                                          sizeof(VirtIOBlock));
+    /*
+     * We have two cases here: the old virtio-blk-pci device, and the
+     * refactored virtio-blk.
+     */
+    if (s == NULL) {
+        /* virtio-blk-pci */
+        s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
+                                              sizeof(struct virtio_blk_config),
+                                              sizeof(VirtIOBlock));
+    } else {
+        /* virtio-blk */
+        virtio_init(VIRTIO_DEVICE(s), "virtio-blk", VIRTIO_ID_BLOCK,
+                    sizeof(struct virtio_blk_config));
+    }
 
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.set_config = virtio_blk_set_config;
@@ -675,6 +697,12 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     return &s->vdev;
 }
 
+VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+{
+    VirtIOBlock *s = NULL;
+    return virtio_blk_common_init(dev, blk, &s);
+}
+
 void virtio_blk_exit(VirtIODevice *vdev)
 {
     VirtIOBlock *s = to_virtio_blk(vdev);
@@ -688,3 +716,63 @@ void virtio_blk_exit(VirtIODevice *vdev)
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
 }
+
+
+static int virtio_blk_device_init(VirtIODevice *vdev)
+{
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
+    VirtIOBlkConf *blk = &(s->blk);
+    if (virtio_blk_common_init(qdev, blk, &s) == NULL) {
+        return -1;
+    }
+    return 0;
+}
+
+static int virtio_blk_device_exit(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOBlock *s = VIRTIO_BLK(dev);
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    virtio_blk_data_plane_destroy(s->dataplane);
+    s->dataplane = NULL;
+#endif
+    qemu_del_vm_change_state_handler(s->change);
+    unregister_savevm(s->qdev, "virtio-blk", s);
+    blockdev_mark_auto_del(s->bs);
+    virtio_common_cleanup(vdev);
+    return 0;
+}
+
+static Property virtio_blk_properties[] = {
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk)
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_blk_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    dc->exit = virtio_blk_device_exit;
+    dc->props = virtio_blk_properties;
+    vdc->init = virtio_blk_device_init;
+    vdc->get_config = virtio_blk_update_config;
+    vdc->set_config = virtio_blk_set_config;
+    vdc->get_features = virtio_blk_get_features;
+    vdc->set_status = virtio_blk_set_status;
+    vdc->reset = virtio_blk_reset;
+}
+
+static const TypeInfo virtio_device_info = {
+    .name = TYPE_VIRTIO_BLK,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOBlock),
+    .class_init = virtio_blk_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_device_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index b704d50..5479424 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -20,6 +20,10 @@
 #include "dataplane/virtio-blk.h"
 #endif
 
+#define TYPE_VIRTIO_BLK "virtio-blk"
+#define VIRTIO_BLK(obj) \
+        OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
+
 /* from Linux's linux/virtio_blk.h */
 
 /* The ID for virtio_block */
@@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
 
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)                          \
+        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0, false),
+#else
+#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
+#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
+
+#ifdef __linux__
+#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
+        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
+#else
+#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
+#endif /* __linux__ */
+
+#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                          \
+        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                         \
+        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                     \
+        DEFINE_PROP_STRING("serial", _state, _field.serial),                  \
+        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true),    \
+        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
+        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
+
+void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
+
 #endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 39c1966..9ed0228 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
 
 static Property virtio_blk_properties[] = {
     DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
-    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
-#ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
-#endif
-    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
-#endif
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
     DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 3/8] virtio-blk-pci: switch to new API.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-blk-pci is modified for the new API. The device
virtio-blk-pci extends virtio-pci. It creates and connects a virtio-blk
during the init. The properties are not changed.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-pci.c | 115 ++++++++++++++++++++++++++------------------------------
 hw/virtio-pci.h |  15 +++++++-
 2 files changed, 68 insertions(+), 62 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9ed0228..f76cae1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -924,26 +924,6 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
     proxy->host_features = vdev->get_features(vdev, proxy->host_features);
 }
 
-static int virtio_blk_init_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-    VirtIODevice *vdev;
-
-    if (proxy->class_code != PCI_CLASS_STORAGE_SCSI &&
-        proxy->class_code != PCI_CLASS_STORAGE_OTHER)
-        proxy->class_code = PCI_CLASS_STORAGE_SCSI;
-
-    vdev = virtio_blk_init(&pci_dev->qdev, &proxy->blk);
-    if (!vdev) {
-        return -1;
-    }
-    vdev->nvectors = proxy->nvectors;
-    virtio_init_pci(proxy, vdev);
-    /* make the actual value visible */
-    proxy->nvectors = vdev->nvectors;
-    return 0;
-}
-
 static void virtio_exit_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -952,15 +932,6 @@ static void virtio_exit_pci(PCIDevice *pci_dev)
     msix_uninit_exclusive_bar(pci_dev);
 }
 
-static void virtio_blk_exit_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
-    virtio_pci_stop_ioeventfd(proxy);
-    virtio_blk_exit(proxy->vdev);
-    virtio_exit_pci(pci_dev);
-}
-
 static int virtio_serial_init_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -1082,37 +1053,6 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
     virtio_exit_pci(pci_dev);
 }
 
-static Property virtio_blk_properties[] = {
-    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void virtio_blk_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->init = virtio_blk_init_pci;
-    k->exit = virtio_blk_exit_pci;
-    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
-    k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
-    k->revision = VIRTIO_PCI_ABI_VERSION;
-    k->class_id = PCI_CLASS_STORAGE_SCSI;
-    dc->reset = virtio_pci_reset;
-    dc->props = virtio_blk_properties;
-}
-
-static const TypeInfo virtio_blk_info = {
-    .name          = "virtio-blk-pci",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(VirtIOPCIProxy),
-    .class_init    = virtio_blk_class_init,
-};
-
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
@@ -1467,6 +1407,59 @@ static const TypeInfo virtio_pci_info = {
     .abstract      = true,
 };
 
+/* virtio-blk-pci */
+
+static Property virtio_blk_pci_properties[] = {
+    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk)
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static int virtio_blk_pci_init(VirtIOPCIProxy *vpci_dev)
+{
+    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+    if (qdev_init(vdev) < 0) {
+        return -1;
+    }
+    return 0;
+}
+
+static void virtio_blk_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+    dc->props = virtio_blk_pci_properties;
+    k->init = virtio_blk_pci_init;
+    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
+    pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+    pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
+}
+
+static void virtio_blk_pci_instance_init(Object *obj)
+{
+    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+}
+
+static const TypeInfo virtio_blk_pci_info = {
+    .name          = TYPE_VIRTIO_BLK_PCI,
+    .parent        = TYPE_VIRTIO_PCI,
+    .instance_size = sizeof(VirtIOBlkPCI),
+    .instance_init = virtio_blk_pci_instance_init,
+    .class_init    = virtio_blk_pci_class_init,
+};
+
 /* virtio-pci-bus */
 
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
@@ -1506,7 +1499,6 @@ static const TypeInfo virtio_pci_bus_info = {
 
 static void virtio_pci_register_types(void)
 {
-    type_register_static(&virtio_blk_info);
     type_register_static(&virtio_net_info);
     type_register_static(&virtio_serial_info);
     type_register_static(&virtio_balloon_info);
@@ -1517,6 +1509,7 @@ static void virtio_pci_register_types(void)
 #ifdef CONFIG_VIRTFS
     type_register_static(&virtio_9p_info);
 #endif
+    type_register_static(&virtio_blk_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index 2ae96f8..a9dbfff 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -25,6 +25,7 @@
 #include "hw/9pfs/virtio-9p-device.h"
 
 typedef struct VirtIOPCIProxy VirtIOPCIProxy;
+typedef struct VirtIOBlkPCI VirtIOBlkPCI;
 
 /* virtio-pci-bus */
 
@@ -73,7 +74,6 @@ struct VirtIOPCIProxy {
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features;
 #ifdef CONFIG_VIRTFS
@@ -90,6 +90,19 @@ struct VirtIOPCIProxy {
     VirtioBusState bus;
 };
 
+/*
+ * virtio-blk-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci"
+#define VIRTIO_BLK_PCI(obj) \
+        OBJECT_CHECK(VirtIOBlkPCI, (obj), TYPE_VIRTIO_BLK_PCI)
+
+struct VirtIOBlkPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+};
+
 void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 4/8] virtio-blk-s390: switch to the new API.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-blk-s390 is modified for the new API. The device
virtio-blk-s390 extends virtio-s390-device as before. It creates and
connects a virtio-blk during the init. The properties are not modified.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/s390-virtio-bus.c | 30 +++++++++++++++++++-----------
 hw/s390x/s390-virtio-bus.h | 13 ++++++++++++-
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d9b7f83..10c1fbe 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -162,16 +162,23 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
     return s390_virtio_device_init(dev, vdev);
 }
 
-static int s390_virtio_blk_init(VirtIOS390Device *dev)
+static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
 {
-    VirtIODevice *vdev;
-
-    vdev = virtio_blk_init((DeviceState *)dev, &dev->blk);
-    if (!vdev) {
+    VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
+    if (qdev_init(vdev) < 0) {
         return -1;
     }
+    return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+}
 
-    return s390_virtio_device_init(dev, vdev);
+static void s390_virtio_blk_instance_init(Object *obj)
+{
+    VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int s390_virtio_serial_init(VirtIOS390Device *dev)
@@ -428,11 +435,11 @@ static const TypeInfo s390_virtio_net = {
 };
 
 static Property s390_virtio_blk_properties[] = {
-    DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
-    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
+    DEFINE_BLOCK_PROPERTIES(VirtIOBlkS390, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlkS390, blk.conf),
+    DEFINE_PROP_STRING("serial", VirtIOBlkS390, blk.serial),
 #ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
+    DEFINE_PROP_BIT("scsi", VirtIOBlkS390, blk.scsi, 0, true),
 #endif
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -449,7 +456,8 @@ static void s390_virtio_blk_class_init(ObjectClass *klass, void *data)
 static const TypeInfo s390_virtio_blk = {
     .name          = "virtio-blk-s390",
     .parent        = TYPE_VIRTIO_S390_DEVICE,
-    .instance_size = sizeof(VirtIOS390Device),
+    .instance_size = sizeof(VirtIOBlkS390),
+    .instance_init = s390_virtio_blk_instance_init,
     .class_init    = s390_virtio_blk_class_init,
 };
 
diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h
index 4aacf83..1a63411 100644
--- a/hw/s390x/s390-virtio-bus.h
+++ b/hw/s390x/s390-virtio-bus.h
@@ -89,7 +89,6 @@ struct VirtIOS390Device {
     ram_addr_t feat_offs;
     uint8_t feat_len;
     VirtIODevice *vdev;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features;
     virtio_serial_conf serial;
@@ -120,5 +119,17 @@ VirtIOS390Device *s390_virtio_bus_find_mem(VirtIOS390Bus *bus, ram_addr_t mem);
 void s390_virtio_device_sync(VirtIOS390Device *dev);
 void s390_virtio_reset_idx(VirtIOS390Device *dev);
 
+/* virtio-blk-s390 */
+
+#define TYPE_VIRTIO_BLK_S390 "virtio-blk-s390"
+#define VIRTIO_BLK_S390(obj) \
+        OBJECT_CHECK(VirtIOBlkS390, (obj), TYPE_VIRTIO_BLK_S390)
+
+typedef struct VirtIOBlkS390 {
+    VirtIOS390Device parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+} VirtIOBlkS390;
+
 
 #endif
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 5/8] virtio-blk-ccw switch to new API.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-ccw-s390 is modified for the new API. The device
virtio-ccw-s390 extends virtio-ccw-device as before. It creates and
connects a virtio-ccw during the init. The properties are not modified.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/virtio-ccw.c | 37 +++++++++++++++++++++++--------------
 hw/s390x/virtio-ccw.h | 14 +++++++++++++-
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a9cf703..401ee57 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -570,22 +570,30 @@ static int virtio_ccw_net_exit(VirtioCcwDevice *dev)
     return virtio_ccw_exit(dev);
 }
 
-static int virtio_ccw_blk_init(VirtioCcwDevice *dev)
+static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
 {
-    VirtIODevice *vdev;
-
-    vdev = virtio_blk_init((DeviceState *)dev, &dev->blk);
-    if (!vdev) {
+    VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
+    if (qdev_init(vdev) < 0) {
         return -1;
     }
 
-    return virtio_ccw_device_init(dev, vdev);
+    return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
 }
 
-static int virtio_ccw_blk_exit(VirtioCcwDevice *dev)
+static int virtio_ccw_blk_exit(VirtioCcwDevice *ccw_dev)
 {
-    virtio_blk_exit(dev->vdev);
-    return virtio_ccw_exit(dev);
+    virtio_bus_destroy_device(&ccw_dev->bus);
+    return virtio_ccw_exit(ccw_dev);
+}
+
+static void virtio_ccw_blk_instance_init(Object *obj)
+{
+    VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int virtio_ccw_serial_init(VirtioCcwDevice *dev)
@@ -730,10 +738,10 @@ static const TypeInfo virtio_ccw_net = {
 
 static Property virtio_ccw_blk_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_BLOCK_PROPERTIES(VirtioCcwDevice, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtioCcwDevice, blk.serial),
+    DEFINE_BLOCK_PROPERTIES(VirtIOBlkCcw, blk.conf),
+    DEFINE_PROP_STRING("serial", VirtIOBlkCcw, blk.serial),
 #ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtioCcwDevice, blk.scsi, 0, true),
+    DEFINE_PROP_BIT("scsi", VirtIOBlkCcw, blk.scsi, 0, true),
 #endif
     DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]),
     DEFINE_PROP_END_OF_LIST(),
@@ -751,9 +759,10 @@ static void virtio_ccw_blk_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo virtio_ccw_blk = {
-    .name          = "virtio-blk-ccw",
+    .name          = TYPE_VIRTIO_BLK_CCW,
     .parent        = TYPE_VIRTIO_CCW_DEVICE,
-    .instance_size = sizeof(VirtioCcwDevice),
+    .instance_size = sizeof(VirtIOBlkCcw),
+    .instance_init = virtio_ccw_blk_instance_init,
     .class_init    = virtio_ccw_blk_class_init,
 };
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 48474b3..9ab0b0d 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -71,7 +71,6 @@ struct VirtioCcwDevice {
     SubchDev *sch;
     VirtIODevice *vdev;
     char *bus_id;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
     virtio_serial_conf serial;
@@ -92,6 +91,19 @@ typedef struct VirtualCssBus {
 #define VIRTUAL_CSS_BUS(obj) \
      OBJECT_CHECK(VirtualCssBus, (obj), TYPE_VIRTUAL_CSS_BUS)
 
+/* virtio-blk-ccw */
+
+#define TYPE_VIRTIO_BLK_CCW "virtio-blk-ccw"
+#define VIRTIO_BLK_CCW(obj) \
+        OBJECT_CHECK(VirtIOBlkCcw, (obj), TYPE_VIRTIO_BLK_CCW)
+
+typedef struct VirtIOBlkCcw {
+    VirtioCcwDevice parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+} VirtIOBlkCcw;
+
+
 VirtualCssBus *virtual_css_bus_init(void);
 void virtio_ccw_device_update_status(SubchDev *sch);
 VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch);
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 6/8] virtio-blk: cleanup: init and exit functions.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

As all virtio-blk-* are switched to the new API, we can remove the separate
init/exit for the old API.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 85 ++++++++++++++-------------------------------------------
 hw/virtio.h     |  2 --
 2 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a0d0679..935aad4 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -630,102 +630,59 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
     memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
 }
 
-static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
-                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
+static int virtio_blk_device_init(VirtIODevice *vdev)
 {
-    VirtIOBlock *s = *ps;
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
+    VirtIOBlkConf *blk = &(s->blk);
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
         error_report("drive property not set");
-        return NULL;
+        return -1;
     }
     if (!bdrv_is_inserted(blk->conf.bs)) {
         error_report("Device needs media, but drive is empty");
-        return NULL;
+        return -1;
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
-        return NULL;
+        return -1;
     }
 
-    /*
-     * We have two cases here: the old virtio-blk-pci device, and the
-     * refactored virtio-blk.
-     */
-    if (s == NULL) {
-        /* virtio-blk-pci */
-        s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                              sizeof(struct virtio_blk_config),
-                                              sizeof(VirtIOBlock));
-    } else {
-        /* virtio-blk */
-        virtio_init(VIRTIO_DEVICE(s), "virtio-blk", VIRTIO_ID_BLOCK,
-                    sizeof(struct virtio_blk_config));
-    }
+    virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
+                sizeof(struct virtio_blk_config));
 
-    s->vdev.get_config = virtio_blk_update_config;
-    s->vdev.set_config = virtio_blk_set_config;
-    s->vdev.get_features = virtio_blk_get_features;
-    s->vdev.set_status = virtio_blk_set_status;
-    s->vdev.reset = virtio_blk_reset;
+    vdev->get_config = virtio_blk_update_config;
+    vdev->set_config = virtio_blk_set_config;
+    vdev->get_features = virtio_blk_get_features;
+    vdev->set_status = virtio_blk_set_status;
+    vdev->reset = virtio_blk_reset;
     s->bs = blk->conf.bs;
     s->conf = &blk->conf;
     memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
-    s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
+    s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    if (!virtio_blk_data_plane_create(&s->vdev, blk, &s->dataplane)) {
-        virtio_cleanup(&s->vdev);
-        return NULL;
+    if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) {
+        virtio_cleanup(vdev);
+        return -1;
     }
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-    s->qdev = dev;
-    register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
+    s->qdev = qdev;
+    register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
     bdrv_set_buffer_alignment(s->bs, s->conf->logical_block_size);
 
     bdrv_iostatus_enable(s->bs);
-    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
 
-    return &s->vdev;
-}
-
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
-{
-    VirtIOBlock *s = NULL;
-    return virtio_blk_common_init(dev, blk, &s);
-}
-
-void virtio_blk_exit(VirtIODevice *vdev)
-{
-    VirtIOBlock *s = to_virtio_blk(vdev);
-
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    virtio_blk_data_plane_destroy(s->dataplane);
-    s->dataplane = NULL;
-#endif
-    qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(s->qdev, "virtio-blk", s);
-    blockdev_mark_auto_del(s->bs);
-    virtio_cleanup(vdev);
-}
-
-
-static int virtio_blk_device_init(VirtIODevice *vdev)
-{
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOBlock *s = VIRTIO_BLK(vdev);
-    VirtIOBlkConf *blk = &(s->blk);
-    if (virtio_blk_common_init(qdev, blk, &s) == NULL) {
-        return -1;
-    }
+    add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0");
     return 0;
 }
 
diff --git a/hw/virtio.h b/hw/virtio.h
index ca43fd7..fdbe931 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -240,7 +240,6 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
 
 /* Base devices.  */
 typedef struct VirtIOBlkConf VirtIOBlkConf;
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk);
 struct virtio_net_conf;
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
                               struct virtio_net_conf *net,
@@ -258,7 +257,6 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
 
 
 void virtio_net_exit(VirtIODevice *vdev);
-void virtio_blk_exit(VirtIODevice *vdev);
 void virtio_serial_exit(VirtIODevice *vdev);
 void virtio_balloon_exit(VirtIODevice *vdev);
 void virtio_scsi_exit(VirtIODevice *vdev);
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 7/8] virtio-blk: cleanup: QOM cast

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Use QOM casts inside virtio-blk.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 33 ++++++++++++++-------------------
 hw/virtio-blk.h |  2 +-
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 935aad4..556d6d9 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -23,14 +23,6 @@
 #endif
 #include "hw/virtio-bus.h"
 
-/*
- * Moving to QOM later in this series.
- */
-static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
-{
-    return (VirtIOBlock *)vdev;
-}
-
 typedef struct VirtIOBlockReq
 {
     VirtIOBlock *dev;
@@ -46,12 +38,13 @@ typedef struct VirtIOBlockReq
 static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
     trace_virtio_blk_req_complete(req, status);
 
     stb_p(&req->in->status, status);
     virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
-    virtio_notify(&s->vdev, s->vq);
+    virtio_notify(vdev, s->vq);
 }
 
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
@@ -396,7 +389,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
 
 static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     VirtIOBlockReq *req;
     MultiReqBuffer mrb = {
         .num_writes = 0,
@@ -464,7 +457,7 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running,
 static void virtio_blk_reset(VirtIODevice *vdev)
 {
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 
     if (s->dataplane) {
         virtio_blk_data_plane_stop(s->dataplane);
@@ -482,7 +475,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
  */
 static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     struct virtio_blk_config blkcfg;
     uint64_t capacity;
     int blk_size = s->conf->logical_block_size;
@@ -521,7 +514,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 
 static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     struct virtio_blk_config blkcfg;
 
     memcpy(&blkcfg, config, sizeof(blkcfg));
@@ -530,7 +523,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
 
 static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 
     features |= (1 << VIRTIO_BLK_F_SEG_MAX);
     features |= (1 << VIRTIO_BLK_F_GEOMETRY);
@@ -552,7 +545,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 
 static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     uint32_t features;
 
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
@@ -573,9 +566,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 static void virtio_blk_save(QEMUFile *f, void *opaque)
 {
     VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     VirtIOBlockReq *req = s->rq;
 
-    virtio_save(&s->vdev, f);
+    virtio_save(vdev, f);
     
     while (req) {
         qemu_put_sbyte(f, 1);
@@ -588,12 +582,13 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
 static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     int ret;
 
     if (version_id != 2)
         return -EINVAL;
 
-    ret = virtio_load(&s->vdev, f);
+    ret = virtio_load(vdev, f);
     if (ret) {
         return ret;
     }
@@ -615,9 +610,9 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 
 static void virtio_blk_resize(void *opaque)
 {
-    VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
 
-    virtio_notify_config(&s->vdev);
+    virtio_notify_config(vdev);
 }
 
 static const BlockDevOps virtio_block_ops = {
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 5479424..aa933ce 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -116,7 +116,7 @@ struct VirtIOBlkConf
 };
 
 typedef struct VirtIOBlock {
-    VirtIODevice vdev;
+    VirtIODevice parent_obj;
     BlockDriverState *bs;
     VirtQueue *vq;
     void *rq;
-- 
1.7.11.7

[Qemu-devel] [PATCH v6 8/8] virtio-blk: cleanup: remove qdev field.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

The qdev field is no longer needed, just drop it.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-blk.c | 3 +--
 hw/virtio-blk.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 556d6d9..fffeeae 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -669,7 +669,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-    s->qdev = qdev;
     register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
@@ -690,7 +689,7 @@ static int virtio_blk_device_exit(DeviceState *dev)
     s->dataplane = NULL;
 #endif
     qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(s->qdev, "virtio-blk", s);
+    unregister_savevm(dev, "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_common_cleanup(vdev);
     return 0;
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index aa933ce..efa0ebf 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -124,7 +124,6 @@ typedef struct VirtIOBlock {
     BlockConf *conf;
     VirtIOBlkConf blk;
     unsigned short sector_mask;
-    DeviceState *qdev;
     VMChangeStateEntry *change;
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     VirtIOBlockDataPlane *dataplane;
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v6 1/8] virtio-blk: don't use pointer for configuration.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> The configuration field must not be a pointer as it will be used for virtio-blk
> properties. So *blk is replaced by blk in VirtIOBlock structure.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
>  /* The ID for virtio_block */
> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
>  #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>
> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)                          \
> +        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0, false),
> +#else
> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
> +
> +#ifdef __linux__
> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
> +        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
> +#else
> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
> +#endif /* __linux__ */
> +
> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                          \
> +        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                         \
> +        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                     \
> +        DEFINE_PROP_STRING("serial", _state, _field.serial),                  \
> +        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true),    \
> +        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
> +        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
> +
> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
> +
>  #endif
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 39c1966..9ed0228 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
>
>  static Property virtio_blk_properties[] = {
>      DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
> -    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
> -    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
> -    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
> -#ifdef __linux__
> -    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
> -#endif
> -    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
> -#endif
>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>      DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
>      DEFINE_PROP_END_OF_LIST(),

You need to tweak your macro definitions so that the user can
put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare
DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES].
Otherwise it's going to get confusing and somebody's going
to add one without noticing that that gets you an extra
empty properties array element.

>  };
>
> --
> 1.7.11.7
>

-- PMM

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 12/03/2013 15:28, Peter Maydell wrote:
> On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
>>   /* The ID for virtio_block */
>> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
>>   #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>>           DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>>
>> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)                          \
>> +        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0, false),
>> +#else
>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
>> +
>> +#ifdef __linux__
>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
>> +        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
>> +#else
>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>> +#endif /* __linux__ */
>> +
>> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                          \
>> +        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                         \
>> +        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                     \
>> +        DEFINE_PROP_STRING("serial", _state, _field.serial),                  \
>> +        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true),    \
>> +        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                       \
>> +        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>> +
>> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
>> +
>>   #endif
>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>> index 39c1966..9ed0228 100644
>> --- a/hw/virtio-pci.c
>> +++ b/hw/virtio-pci.c
>> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
>>
>>   static Property virtio_blk_properties[] = {
>>       DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>> -    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
>> -    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
>> -    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
>> -#ifdef __linux__
>> -    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
>> -#endif
>> -    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
>>       DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
>> -#endif
>>       DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>>       DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
>> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
>>       DEFINE_PROP_END_OF_LIST(),
> You need to tweak your macro definitions so that the user can
> put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare
> DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES].
> Otherwise it's going to get confusing and somebody's going
> to add one without noticing that that gets you an extra
> empty properties array element.
Do you have any idea for that?

It's a little tricky with the two conditions 
CONFIG_VIRTIO_BLK_DATA_PLANE and __linux__,

I can't have #define with an #ifdef inside..

Do you see what I mean?
>
>>   };
>>
>> --
>> 1.7.11.7
>>
> -- PMM

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Peter Maydell]

On 12 March 2013 14:37, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> On 12/03/2013 15:28, Peter Maydell wrote:
>>
>> On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
>>>
>>>   /* The ID for virtio_block */
>>> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
>>>   #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>>>           DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>>>
>>> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>> \
>>> +        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0,
>>> false),
>>> +#else
>>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
>>> +
>>> +#ifdef __linux__
>>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>> \
>>> +        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
>>> +#else
>>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>> +#endif /* __linux__ */
>>> +
>>> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)
>>> \
>>> +        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
>>> \
>>> +        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
>>> \
>>> +        DEFINE_PROP_STRING("serial", _state, _field.serial),
>>> \
>>> +        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0,
>>> true),    \
>>> +        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>> \
>>> +        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>> +
>>> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
>>> +
>>>   #endif
>>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>>> index 39c1966..9ed0228 100644
>>> --- a/hw/virtio-pci.c
>>> +++ b/hw/virtio-pci.c
>>> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice
>>> *pci_dev)
>>>
>>>   static Property virtio_blk_properties[] = {
>>>       DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>>> -    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
>>> -    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
>>> -    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
>>> -#ifdef __linux__
>>> -    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
>>> -#endif
>>> -    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0,
>>> true),
>>>       DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>>> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>>> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>>> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0,
>>> false),
>>> -#endif
>>>       DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>>>       DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
>>> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
>>>       DEFINE_PROP_END_OF_LIST(),
>>
>> You need to tweak your macro definitions so that the user can
>> put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare
>> DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES].
>> Otherwise it's going to get confusing and somebody's going
>> to add one without noticing that that gets you an extra
>> empty properties array element.
>
> Do you have any idea for that?
>
> It's a little tricky with the two conditions CONFIG_VIRTIO_BLK_DATA_PLANE
> and __linux__,
>
> I can't have #define with an #ifdef inside..
>
> Do you see what I mean?

Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
ones that are expected to be used by other code, right? So you can
define them with commas (and name them something so it's obvious
they're not intended for wider use as property array elements),
and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
doesn't end with a comma. (You can do that by putting the macros
that expand to maybe-comma-or-not at the front, not the end.)

-- PMM

Re: [Qemu-devel] [PATCH v6 3/8] virtio-blk-pci: switch to new API.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Here the virtio-blk-pci is modified for the new API. The device
> virtio-blk-pci extends virtio-pci. It creates and connects a virtio-blk
> during the init. The properties are not changed.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 4/8] virtio-blk-s390: switch to the new API.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Here the virtio-blk-s390 is modified for the new API. The device
> virtio-blk-s390 extends virtio-s390-device as before. It creates and
> connects a virtio-blk during the init. The properties are not modified.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 5/8] virtio-blk-ccw switch to new API.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Here the virtio-ccw-s390 is modified for the new API. The device
> virtio-ccw-s390 extends virtio-ccw-device as before. It creates and
> connects a virtio-ccw during the init. The properties are not modified.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 6/8] virtio-blk: cleanup: init and exit functions.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> As all virtio-blk-* are switched to the new API, we can remove the separate
> init/exit for the old API.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 7/8] virtio-blk: cleanup: QOM cast

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Use QOM casts inside virtio-blk.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 8/8] virtio-blk: cleanup: remove qdev field.

[Peter Maydell]

On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> The qdev field is no longer needed, just drop it.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 12/03/2013 15:42, Peter Maydell wrote:
> On 12 March 2013 14:37, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>> On 12/03/2013 15:28, Peter Maydell wrote:
>>> On 12 March 2013 09:22,  <fred.konrad@greensocs.com> wrote:
>>>>    /* The ID for virtio_block */
>>>> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
>>>>    #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>>>>            DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>>>>
>>>> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>>>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>>> \
>>>> +        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0,
>>>> false),
>>>> +#else
>>>> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>>> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
>>>> +
>>>> +#ifdef __linux__
>>>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>>> \
>>>> +        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
>>>> +#else
>>>> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>>> +#endif /* __linux__ */
>>>> +
>>>> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)
>>>> \
>>>> +        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
>>>> \
>>>> +        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
>>>> \
>>>> +        DEFINE_PROP_STRING("serial", _state, _field.serial),
>>>> \
>>>> +        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0,
>>>> true),    \
>>>> +        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
>>>> \
>>>> +        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
>>>> +
>>>> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
>>>> +
>>>>    #endif
>>>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>>>> index 39c1966..9ed0228 100644
>>>> --- a/hw/virtio-pci.c
>>>> +++ b/hw/virtio-pci.c
>>>> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice
>>>> *pci_dev)
>>>>
>>>>    static Property virtio_blk_properties[] = {
>>>>        DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>>>> -    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
>>>> -    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
>>>> -    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
>>>> -#ifdef __linux__
>>>> -    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
>>>> -#endif
>>>> -    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0,
>>>> true),
>>>>        DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>>>> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>>>> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
>>>> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0,
>>>> false),
>>>> -#endif
>>>>        DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>>>>        DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
>>>> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
>>>>        DEFINE_PROP_END_OF_LIST(),
>>> You need to tweak your macro definitions so that the user can
>>> put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare
>>> DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES].
>>> Otherwise it's going to get confusing and somebody's going
>>> to add one without noticing that that gets you an extra
>>> empty properties array element.
>> Do you have any idea for that?
>>
>> It's a little tricky with the two conditions CONFIG_VIRTIO_BLK_DATA_PLANE
>> and __linux__,
>>
>> I can't have #define with an #ifdef inside..
>>
>> Do you see what I mean?
> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
> ones that are expected to be used by other code, right? So you can
> define them with commas (and name them something so it's obvious
> they're not intended for wider use as property array elements),
> and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
> doesn't end with a comma. (You can do that by putting the macros
> that expand to maybe-comma-or-not at the front, not the end.)
>
> -- PMM
ok, I can put a comment which say not to use them?

Thanks,

Fred

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Peter Maydell]

On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> On 12/03/2013 15:42, Peter Maydell wrote:
>>
>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>> ones that are expected to be used by other code, right? So you can
>> define them with commas (and name them something so it's obvious
>> they're not intended for wider use as property array elements),
>> and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
>> doesn't end with a comma. (You can do that by putting the macros
>> that expand to maybe-comma-or-not at the front, not the end.)
>>
>> -- PMM
>
> ok, I can put a comment which say not to use them?

And suitable macro names (ie not ones which look like all
the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
macro's only used once as far as I can see, you could just not
bother to abstract it out. The virtio-ccw blk properties still
just have inline #ifdefs for the scsi prop for instance.

-- PMM

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 12/03/2013 16:12, Peter Maydell wrote:
> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>> On 12/03/2013 15:42, Peter Maydell wrote:
>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>>> ones that are expected to be used by other code, right? So you can
>>> define them with commas (and name them something so it's obvious
>>> they're not intended for wider use as property array elements),
>>> and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
>>> doesn't end with a comma. (You can do that by putting the macros
>>> that expand to maybe-comma-or-not at the front, not the end.)
>>>
>>> -- PMM
>> ok, I can put a comment which say not to use them?
> And suitable macro names (ie not ones which look like all
> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
> macro's only used once as far as I can see, you could just not
> bother to abstract it out. The virtio-ccw blk properties still
> just have inline #ifdefs for the scsi prop for instance.
>
> -- PMM

The macro is used for virtio-blk device and virtio-blk-pci.
s390x devices don't use the same properties.

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Cornelia Huck]

On Tue, 12 Mar 2013 16:22:22 +0100
KONRAD Frédéric <fred.konrad@greensocs.com> wrote:

> On 12/03/2013 16:12, Peter Maydell wrote:
> > On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> >> On 12/03/2013 15:42, Peter Maydell wrote:
> >>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
> >>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
> >>> ones that are expected to be used by other code, right? So you can
> >>> define them with commas (and name them something so it's obvious
> >>> they're not intended for wider use as property array elements),
> >>> and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
> >>> doesn't end with a comma. (You can do that by putting the macros
> >>> that expand to maybe-comma-or-not at the front, not the end.)
> >>>
> >>> -- PMM
> >> ok, I can put a comment which say not to use them?
> > And suitable macro names (ie not ones which look like all
> > the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
> > macro's only used once as far as I can see, you could just not
> > bother to abstract it out. The virtio-ccw blk properties still
> > just have inline #ifdefs for the scsi prop for instance.
> >
> > -- PMM
> 
> The macro is used for virtio-blk device and virtio-blk-pci.
> s390x devices don't use the same properties.
> 

Looking at the s390 devices, the difference seems to be the following:

- CHS - missing on virtio-ccw, I'll do a patch.
- config_wce - missing on s390-virtio and virtio-ccw, should probably
  be added.
- x-data-plane - we plan to add this eventually to virtio-ccw, but not
  to s390-virtio. Could that be split out from the generic properties?

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 12/03/2013 17:31, Cornelia Huck wrote:
> On Tue, 12 Mar 2013 16:22:22 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> On 12/03/2013 16:12, Peter Maydell wrote:
>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>>> On 12/03/2013 15:42, Peter Maydell wrote:
>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>>>>> ones that are expected to be used by other code, right? So you can
>>>>> define them with commas (and name them something so it's obvious
>>>>> they're not intended for wider use as property array elements),
>>>>> and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
>>>>> doesn't end with a comma. (You can do that by putting the macros
>>>>> that expand to maybe-comma-or-not at the front, not the end.)
>>>>>
>>>>> -- PMM
>>>> ok, I can put a comment which say not to use them?
>>> And suitable macro names (ie not ones which look like all
>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
>>> macro's only used once as far as I can see, you could just not
>>> bother to abstract it out. The virtio-ccw blk properties still
>>> just have inline #ifdefs for the scsi prop for instance.
>>>
>>> -- PMM
>> The macro is used for virtio-blk device and virtio-blk-pci.
>> s390x devices don't use the same properties.
>>
> Looking at the s390 devices, the difference seems to be the following:
>
> - CHS - missing on virtio-ccw, I'll do a patch.
> - config_wce - missing on s390-virtio and virtio-ccw, should probably
>    be added.
> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
>    to s390-virtio. Could that be split out from the generic properties?
>
ok, so what I can do is:

- split up x-data-plane property (so it will be only in virtio-pci.c).
- fix this comma thing.

Then when you put these two missing properties you can just replace all 
of them
  with the macro.

Is that ok for everybody? Peter? Stefan?

Thanks for replies,

Fred

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 13/03/2013 09:24, KONRAD Frédéric wrote:
> On 12/03/2013 17:31, Cornelia Huck wrote:
>> On Tue, 12 Mar 2013 16:22:22 +0100
>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>
>>> On 12/03/2013 16:12, Peter Maydell wrote:
>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> 
>>>> wrote:
>>>>> On 12/03/2013 15:42, Peter Maydell wrote:
>>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>>>>>> ones that are expected to be used by other code, right? So you can
>>>>>> define them with commas (and name them something so it's obvious
>>>>>> they're not intended for wider use as property array elements),
>>>>>> and then just make sure your public-facing 
>>>>>> DEFINE_VIRTIO_BLK_PROPERTIES
>>>>>> doesn't end with a comma. (You can do that by putting the macros
>>>>>> that expand to maybe-comma-or-not at the front, not the end.)
>>>>>>
>>>>>> -- PMM
>>>>> ok, I can put a comment which say not to use them?
>>>> And suitable macro names (ie not ones which look like all
>>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
>>>> macro's only used once as far as I can see, you could just not
>>>> bother to abstract it out. The virtio-ccw blk properties still
>>>> just have inline #ifdefs for the scsi prop for instance.
>>>>
>>>> -- PMM
>>> The macro is used for virtio-blk device and virtio-blk-pci.
>>> s390x devices don't use the same properties.
>>>
>> Looking at the s390 devices, the difference seems to be the following:
>>
>> - CHS - missing on virtio-ccw, I'll do a patch.
>> - config_wce - missing on s390-virtio and virtio-ccw, should probably
>>    be added.
>> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
>>    to s390-virtio. Could that be split out from the generic properties?
>>
> ok, so what I can do is:
>
> - split up x-data-plane property (so it will be only in virtio-pci.c).
> - fix this comma thing.
>
> Then when you put these two missing properties you can just replace 
> all of them
>  with the macro.
>
> Is that ok for everybody? Peter? Stefan?
>
Any other suggestion?

Thanks,

Fred

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Cornelia Huck]

On Wed, 13 Mar 2013 16:32:31 +0100
KONRAD Frédéric <fred.konrad@greensocs.com> wrote:

> On 13/03/2013 09:24, KONRAD Frédéric wrote:
> > On 12/03/2013 17:31, Cornelia Huck wrote:
> >> On Tue, 12 Mar 2013 16:22:22 +0100
> >> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> >>
> >>> On 12/03/2013 16:12, Peter Maydell wrote:
> >>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> 
> >>>> wrote:
> >>>>> On 12/03/2013 15:42, Peter Maydell wrote:
> >>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
> >>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
> >>>>>> ones that are expected to be used by other code, right? So you can
> >>>>>> define them with commas (and name them something so it's obvious
> >>>>>> they're not intended for wider use as property array elements),
> >>>>>> and then just make sure your public-facing 
> >>>>>> DEFINE_VIRTIO_BLK_PROPERTIES
> >>>>>> doesn't end with a comma. (You can do that by putting the macros
> >>>>>> that expand to maybe-comma-or-not at the front, not the end.)
> >>>>>>
> >>>>>> -- PMM
> >>>>> ok, I can put a comment which say not to use them?
> >>>> And suitable macro names (ie not ones which look like all
> >>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
> >>>> macro's only used once as far as I can see, you could just not
> >>>> bother to abstract it out. The virtio-ccw blk properties still
> >>>> just have inline #ifdefs for the scsi prop for instance.
> >>>>
> >>>> -- PMM
> >>> The macro is used for virtio-blk device and virtio-blk-pci.
> >>> s390x devices don't use the same properties.
> >>>
> >> Looking at the s390 devices, the difference seems to be the following:
> >>
> >> - CHS - missing on virtio-ccw, I'll do a patch.
> >> - config_wce - missing on s390-virtio and virtio-ccw, should probably
> >>    be added.
> >> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
> >>    to s390-virtio. Could that be split out from the generic properties?
> >>
> > ok, so what I can do is:
> >
> > - split up x-data-plane property (so it will be only in virtio-pci.c).
> > - fix this comma thing.
> >
> > Then when you put these two missing properties you can just replace 
> > all of them
> >  with the macro.
> >
> > Is that ok for everybody? Peter? Stefan?
> >
> Any other suggestion?

I currently have the following two patches sitting in my pending queue
(git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably submit
them once my current pull request is through.

On top of this, s390-virtio and virtio-ccw could use the generic macro
for the virtio-blk properties from the start if x-data-plane is split
out (I can add it to virtio-ccw once we support it).

From 763c1a8ff61faaef5b488072cc9965bd29f8a1fd Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cornelia.huck@de.ibm.com>
Date: Wed, 13 Mar 2013 14:43:22 +0100
Subject: [PATCH 1/2] virtio-ccw: Add missing blk chs properties.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d4361f6..70aba41 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -755,6 +755,7 @@ static const TypeInfo virtio_ccw_net = {
 static Property virtio_ccw_blk_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_BLOCK_PROPERTIES(VirtioCcwDevice, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtioCcwDevice, blk.conf),
     DEFINE_PROP_STRING("serial", VirtioCcwDevice, blk.serial),
 #ifdef __linux__
     DEFINE_PROP_BIT("scsi", VirtioCcwDevice, blk.scsi, 0, true),
-- 
1.7.9.5

From 9e60f80b0ca9943ce3e6d11630c3b5a8bf0c3cbd Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cornelia.huck@de.ibm.com>
Date: Wed, 13 Mar 2013 15:20:07 +0100
Subject: [PATCH 2/2] s390-virtio, virtio-ccw: Add config_wce for virtio-blk.

There's no reason why we wouldn't want to make the cache mode
configurable.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-bus.c |    1 +
 hw/s390x/virtio-ccw.c      |    1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d9b7f83..18f1292 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -434,6 +434,7 @@ static Property s390_virtio_blk_properties[] = {
 #ifdef __linux__
     DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
 #endif
+    DEFINE_PROP_BIT("config-wce", VirtIOS390Device, blk.config_wce, 0, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 70aba41..5795bdd 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -761,6 +761,7 @@ static Property virtio_ccw_blk_properties[] = {
     DEFINE_PROP_BIT("scsi", VirtioCcwDevice, blk.scsi, 0, true),
 #endif
     DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]),
+    DEFINE_PROP_BIT("config-wce", VirtioCcwDevice, blk.config_wce, 0, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 14/03/2013 08:25, Cornelia Huck wrote:
> On Wed, 13 Mar 2013 16:32:31 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> On 13/03/2013 09:24, KONRAD Frédéric wrote:
>>> On 12/03/2013 17:31, Cornelia Huck wrote:
>>>> On Tue, 12 Mar 2013 16:22:22 +0100
>>>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>>>
>>>>> On 12/03/2013 16:12, Peter Maydell wrote:
>>>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com>
>>>>>> wrote:
>>>>>>> On 12/03/2013 15:42, Peter Maydell wrote:
>>>>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>>>>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>>>>>>>> ones that are expected to be used by other code, right? So you can
>>>>>>>> define them with commas (and name them something so it's obvious
>>>>>>>> they're not intended for wider use as property array elements),
>>>>>>>> and then just make sure your public-facing
>>>>>>>> DEFINE_VIRTIO_BLK_PROPERTIES
>>>>>>>> doesn't end with a comma. (You can do that by putting the macros
>>>>>>>> that expand to maybe-comma-or-not at the front, not the end.)
>>>>>>>>
>>>>>>>> -- PMM
>>>>>>> ok, I can put a comment which say not to use them?
>>>>>> And suitable macro names (ie not ones which look like all
>>>>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
>>>>>> macro's only used once as far as I can see, you could just not
>>>>>> bother to abstract it out. The virtio-ccw blk properties still
>>>>>> just have inline #ifdefs for the scsi prop for instance.
>>>>>>
>>>>>> -- PMM
>>>>> The macro is used for virtio-blk device and virtio-blk-pci.
>>>>> s390x devices don't use the same properties.
>>>>>
>>>> Looking at the s390 devices, the difference seems to be the following:
>>>>
>>>> - CHS - missing on virtio-ccw, I'll do a patch.
>>>> - config_wce - missing on s390-virtio and virtio-ccw, should probably
>>>>     be added.
>>>> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
>>>>     to s390-virtio. Could that be split out from the generic properties?
>>>>
>>> ok, so what I can do is:
>>>
>>> - split up x-data-plane property (so it will be only in virtio-pci.c).
>>> - fix this comma thing.
>>>
>>> Then when you put these two missing properties you can just replace
>>> all of them
>>>   with the macro.
>>>
>>> Is that ok for everybody? Peter? Stefan?
>>>
>> Any other suggestion?
> I currently have the following two patches sitting in my pending queue
> (git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably submit
> them once my current pull request is through.
>
> On top of this, s390-virtio and virtio-ccw could use the generic macro
> for the virtio-blk properties from the start if x-data-plane is split
> out (I can add it to virtio-ccw once we support it).
>
Ok good,

And what about config-wce property for virtio-blk-s390x? It is missing too.

Fred

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Cornelia Huck]

On Thu, 14 Mar 2013 09:37:54 +0100
KONRAD Frédéric <fred.konrad@greensocs.com> wrote:

> On 14/03/2013 08:25, Cornelia Huck wrote:
> > On Wed, 13 Mar 2013 16:32:31 +0100
> > KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> >
> >> On 13/03/2013 09:24, KONRAD Frédéric wrote:
> >>> On 12/03/2013 17:31, Cornelia Huck wrote:
> >>>> On Tue, 12 Mar 2013 16:22:22 +0100
> >>>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> >>>>
> >>>>> On 12/03/2013 16:12, Peter Maydell wrote:
> >>>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com>
> >>>>>> wrote:
> >>>>>>> On 12/03/2013 15:42, Peter Maydell wrote:
> >>>>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
> >>>>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
> >>>>>>>> ones that are expected to be used by other code, right? So you can
> >>>>>>>> define them with commas (and name them something so it's obvious
> >>>>>>>> they're not intended for wider use as property array elements),
> >>>>>>>> and then just make sure your public-facing
> >>>>>>>> DEFINE_VIRTIO_BLK_PROPERTIES
> >>>>>>>> doesn't end with a comma. (You can do that by putting the macros
> >>>>>>>> that expand to maybe-comma-or-not at the front, not the end.)
> >>>>>>>>
> >>>>>>>> -- PMM
> >>>>>>> ok, I can put a comment which say not to use them?
> >>>>>> And suitable macro names (ie not ones which look like all
> >>>>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
> >>>>>> macro's only used once as far as I can see, you could just not
> >>>>>> bother to abstract it out. The virtio-ccw blk properties still
> >>>>>> just have inline #ifdefs for the scsi prop for instance.
> >>>>>>
> >>>>>> -- PMM
> >>>>> The macro is used for virtio-blk device and virtio-blk-pci.
> >>>>> s390x devices don't use the same properties.
> >>>>>
> >>>> Looking at the s390 devices, the difference seems to be the following:
> >>>>
> >>>> - CHS - missing on virtio-ccw, I'll do a patch.
> >>>> - config_wce - missing on s390-virtio and virtio-ccw, should probably
> >>>>     be added.
> >>>> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
> >>>>     to s390-virtio. Could that be split out from the generic properties?
> >>>>
> >>> ok, so what I can do is:
> >>>
> >>> - split up x-data-plane property (so it will be only in virtio-pci.c).
> >>> - fix this comma thing.
> >>>
> >>> Then when you put these two missing properties you can just replace
> >>> all of them
> >>>   with the macro.
> >>>
> >>> Is that ok for everybody? Peter? Stefan?
> >>>
> >> Any other suggestion?
> > I currently have the following two patches sitting in my pending queue
> > (git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably submit
> > them once my current pull request is through.
> >
> > On top of this, s390-virtio and virtio-ccw could use the generic macro
> > for the virtio-blk properties from the start if x-data-plane is split
> > out (I can add it to virtio-ccw once we support it).
> >
> Ok good,
> 
> And what about config-wce property for virtio-blk-s390x? It is missing too.

See the second patch :)

Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[KONRAD Frédéric]

On 14/03/2013 09:42, Cornelia Huck wrote:
> On Thu, 14 Mar 2013 09:37:54 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> On 14/03/2013 08:25, Cornelia Huck wrote:
>>> On Wed, 13 Mar 2013 16:32:31 +0100
>>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>>
>>>> On 13/03/2013 09:24, KONRAD Frédéric wrote:
>>>>> On 12/03/2013 17:31, Cornelia Huck wrote:
>>>>>> On Tue, 12 Mar 2013 16:22:22 +0100
>>>>>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>>>>>
>>>>>>> On 12/03/2013 16:12, Peter Maydell wrote:
>>>>>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com>
>>>>>>>> wrote:
>>>>>>>>> On 12/03/2013 15:42, Peter Maydell wrote:
>>>>>>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
>>>>>>>>>> and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
>>>>>>>>>> ones that are expected to be used by other code, right? So you can
>>>>>>>>>> define them with commas (and name them something so it's obvious
>>>>>>>>>> they're not intended for wider use as property array elements),
>>>>>>>>>> and then just make sure your public-facing
>>>>>>>>>> DEFINE_VIRTIO_BLK_PROPERTIES
>>>>>>>>>> doesn't end with a comma. (You can do that by putting the macros
>>>>>>>>>> that expand to maybe-comma-or-not at the front, not the end.)
>>>>>>>>>>
>>>>>>>>>> -- PMM
>>>>>>>>> ok, I can put a comment which say not to use them?
>>>>>>>> And suitable macro names (ie not ones which look like all
>>>>>>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
>>>>>>>> macro's only used once as far as I can see, you could just not
>>>>>>>> bother to abstract it out. The virtio-ccw blk properties still
>>>>>>>> just have inline #ifdefs for the scsi prop for instance.
>>>>>>>>
>>>>>>>> -- PMM
>>>>>>> The macro is used for virtio-blk device and virtio-blk-pci.
>>>>>>> s390x devices don't use the same properties.
>>>>>>>
>>>>>> Looking at the s390 devices, the difference seems to be the following:
>>>>>>
>>>>>> - CHS - missing on virtio-ccw, I'll do a patch.
>>>>>> - config_wce - missing on s390-virtio and virtio-ccw, should probably
>>>>>>      be added.
>>>>>> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
>>>>>>      to s390-virtio. Could that be split out from the generic properties?
>>>>>>
>>>>> ok, so what I can do is:
>>>>>
>>>>> - split up x-data-plane property (so it will be only in virtio-pci.c).
>>>>> - fix this comma thing.
>>>>>
>>>>> Then when you put these two missing properties you can just replace
>>>>> all of them
>>>>>    with the macro.
>>>>>
>>>>> Is that ok for everybody? Peter? Stefan?
>>>>>
>>>> Any other suggestion?
>>> I currently have the following two patches sitting in my pending queue
>>> (git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably submit
>>> them once my current pull request is through.
>>>
>>> On top of this, s390-virtio and virtio-ccw could use the generic macro
>>> for the virtio-blk properties from the start if x-data-plane is split
>>> out (I can add it to virtio-ccw once we support it).
>>>
>> Ok good,
>>
>> And what about config-wce property for virtio-blk-s390x? It is missing too.
> See the second patch :)
>
>
>
good, sorry for the noise :).

Thanks,

Fred