Re: [Qemu-devel] [RFC PATCH V8 08/15] virtio-blk : Add the virtio-blk device.

[Blue Swirl]

On Wed, Dec 19, 2012 at 9:53 AM,  <address@hidden> wrote:
> From: KONRAD Frederic <address@hidden>
>
> Create virtio-blk which extends virtio-device, so it can be connected on
> virtio-bus.
>
> Signed-off-by: KONRAD Frederic <address@hidden>
> ---
>  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
>
>