Patchwork [RFC,V8,08/15] virtio-blk : Add the virtio-blk device.

login
register
mail settings
Submitter fred.konrad@greensocs.com
Date Dec. 19, 2012, 9:53 a.m.
Message ID <1355910821-21302-9-git-send-email-fred.konrad@greensocs.com>
Download mbox | patch
Permalink /patch/207298/
State New
Headers show

Comments

fred.konrad@greensocs.com - Dec. 19, 2012, 9:53 a.m.
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 | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/virtio-blk.h |  19 +++++++++++
 hw/virtio-pci.c |   7 ++--
 3 files changed, 116 insertions(+), 11 deletions(-)
Blue Swirl - Dec. 19, 2012, 8 p.m.
On Wed, Dec 19, 2012 at 9:53 AM,  <fred.konrad@greensocs.com> wrote:
> 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 | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  hw/virtio-blk.h |  19 +++++++++++
>  hw/virtio-pci.c |   7 ++--
>  3 files changed, 116 insertions(+), 11 deletions(-)
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index e25cc96..5511999 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -21,6 +21,7 @@
>  #ifdef __linux__
>  # include <scsi/sg.h>
>  #endif
> +#include "virtio-bus.h"
>
>  typedef struct VirtIOBlock
>  {
> @@ -30,11 +31,14 @@ typedef struct VirtIOBlock
>      void *rq;
>      QEMUBH *bh;
>      BlockConf *conf;
> -    VirtIOBlkConf *blk;
> +    VirtIOBlkConf blk;
>      unsigned short sector_mask;
>      DeviceState *qdev;
>  } VirtIOBlock;
>
> +/*
> + * Moving to QOM later in this series.
> + */
>  static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
>  {
>      return (VirtIOBlock *)vdev;
> @@ -164,7 +168,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;
>      }
> @@ -384,7 +388,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);
> @@ -600,9 +604,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 = VIRTIO_BLK(dev);
> +    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
> +}
> +
> +static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
> +                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
>  {
> -    VirtIOBlock *s;
> +    VirtIOBlock *s = *ps;
>      static int virtio_blk_id;
>
>      if (!blk->conf.bs) {
> @@ -619,9 +630,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;
> @@ -630,7 +652,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;
> +    virtio_blk_set_conf(dev, blk);
>      s->rq = NULL;
>      s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
>
> @@ -649,6 +671,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);
> @@ -656,3 +684,58 @@ 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);
> +    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 651a000..e89c88e 100644
> --- a/hw/virtio-blk.h
> +++ b/hw/virtio-blk.h
> @@ -17,6 +17,10 @@
>  #include "virtio.h"
>  #include "hw/block-common.h"
>
> +#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 */
> @@ -110,4 +114,19 @@ struct VirtIOBlkConf
>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
>          DEFINE_PROP_BIT("config-wce", _state, _field, VIRTIO_BLK_F_CONFIG_WCE, true)
>
> +#ifdef __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("scsi", _state, _field.scsi, 0, true)
> +#else
> +#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)

Does the different property set mean that it would not be possible to
migrate VMs which use virtio from non-Linux host to Linux and vice
versa?

> +#endif /* __linux__ */
> +
> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
> +
>  #endif
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 916ed7c..877bf38 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -1132,8 +1132,11 @@ static void virtio_pci_device_plugged(void *opaque)
>
>      /* Put the PCI IDs */
>      switch (get_virtio_device_id(proxy->bus)) {
> -
> -
> +    case VIRTIO_ID_BLOCK:
> +        pci_config_set_device_id(proxy->pci_dev.config,
> +                                 PCI_DEVICE_ID_VIRTIO_BLOCK);
> +        pci_config_set_class(proxy->pci_dev.config, PCI_CLASS_STORAGE_SCSI);
> +    break;

Again wrong indentation.

>      default:
>          error_report("unknown device id\n");
>      break;
> --
> 1.7.11.7
>
>
Peter Maydell - Dec. 19, 2012, 11:22 p.m.
On 19 December 2012 20:00, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Wed, Dec 19, 2012 at 9:53 AM,  <fred.konrad@greensocs.com> wrote:
>> +#ifdef __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("scsi", _state, _field.scsi, 0, true)
>> +#else
>> +#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)
>
> Does the different property set mean that it would not be possible to
> migrate VMs which use virtio from non-Linux host to Linux and vice
> versa?

Probably, but note that this is just reflecting an existing ifdef in
the non-refactored implementation.

(I looked at that existing ifdef and thought "smells dubious", but
presumably there is a reason.)

-- PMM

Patch

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index e25cc96..5511999 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -21,6 +21,7 @@ 
 #ifdef __linux__
 # include <scsi/sg.h>
 #endif
+#include "virtio-bus.h"
 
 typedef struct VirtIOBlock
 {
@@ -30,11 +31,14 @@  typedef struct VirtIOBlock
     void *rq;
     QEMUBH *bh;
     BlockConf *conf;
-    VirtIOBlkConf *blk;
+    VirtIOBlkConf blk;
     unsigned short sector_mask;
     DeviceState *qdev;
 } VirtIOBlock;
 
+/*
+ * Moving to QOM later in this series.
+ */
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 {
     return (VirtIOBlock *)vdev;
@@ -164,7 +168,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;
     }
@@ -384,7 +388,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);
@@ -600,9 +604,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 = VIRTIO_BLK(dev);
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
+}
+
+static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
+                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
 {
-    VirtIOBlock *s;
+    VirtIOBlock *s = *ps;
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
@@ -619,9 +630,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;
@@ -630,7 +652,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;
+    virtio_blk_set_conf(dev, blk);
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
@@ -649,6 +671,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);
@@ -656,3 +684,58 @@  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);
+    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 651a000..e89c88e 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -17,6 +17,10 @@ 
 #include "virtio.h"
 #include "hw/block-common.h"
 
+#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 */
@@ -110,4 +114,19 @@  struct VirtIOBlkConf
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
         DEFINE_PROP_BIT("config-wce", _state, _field, VIRTIO_BLK_F_CONFIG_WCE, true)
 
+#ifdef __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("scsi", _state, _field.scsi, 0, true)
+#else
+#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)
+#endif /* __linux__ */
+
+void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
+
 #endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 916ed7c..877bf38 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1132,8 +1132,11 @@  static void virtio_pci_device_plugged(void *opaque)
 
     /* Put the PCI IDs */
     switch (get_virtio_device_id(proxy->bus)) {
-
-
+    case VIRTIO_ID_BLOCK:
+        pci_config_set_device_id(proxy->pci_dev.config,
+                                 PCI_DEVICE_ID_VIRTIO_BLOCK);
+        pci_config_set_class(proxy->pci_dev.config, PCI_CLASS_STORAGE_SCSI);
+    break;
     default:
         error_report("unknown device id\n");
     break;