Re: [Qemu-devel] [PATCH v3 12/23] virtio-blk: Drop redundant VirtIOBlock member conf

[Benoît Canet]

On Tue, Sep 16, 2014 at 08:12:17PM +0200, Markus Armbruster wrote:
> Commit 12c5674 turned it into a pointer to member blk.conf.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  hw/block/virtio-blk.c          | 28 ++++++++++++++--------------
>  include/hw/virtio/virtio-blk.h |  1 -
>  2 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 38ad38f..5943af5 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -298,7 +298,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
>      if (sector & dev->sector_mask) {
>          return false;
>      }
> -    if (size % dev->conf->logical_block_size) {
> +    if (size % dev->blk.conf.logical_block_size) {
>          return false;
>      }
>      bdrv_get_geometry(dev->bs, &total_sectors);
> @@ -519,19 +519,20 @@ static void virtio_blk_reset(VirtIODevice *vdev)
>  static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>  {
>      VirtIOBlock *s = VIRTIO_BLK(vdev);
> +    BlockConf *conf = &s->blk.conf;
>      struct virtio_blk_config blkcfg;
>      uint64_t capacity;
> -    int blk_size = s->conf->logical_block_size;
> +    int blk_size = conf->logical_block_size;
>  
>      bdrv_get_geometry(s->bs, &capacity);
>      memset(&blkcfg, 0, sizeof(blkcfg));
>      virtio_stq_p(vdev, &blkcfg.capacity, capacity);
>      virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2);
> -    virtio_stw_p(vdev, &blkcfg.cylinders, s->conf->cyls);
> +    virtio_stw_p(vdev, &blkcfg.cylinders, conf->cyls);
>      virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> -    virtio_stw_p(vdev, &blkcfg.min_io_size, s->conf->min_io_size / blk_size);
> -    virtio_stw_p(vdev, &blkcfg.opt_io_size, s->conf->opt_io_size / blk_size);
> -    blkcfg.heads = s->conf->heads;
> +    virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> +    virtio_stw_p(vdev, &blkcfg.opt_io_size, conf->opt_io_size / blk_size);
> +    blkcfg.heads = conf->heads;
>      /*
>       * We must ensure that the block device capacity is a multiple of
>       * the logical block size. If that is not the case, let's use
> @@ -543,13 +544,13 @@ static void virtio_blk_update_config(VirtIODevice 
> *vdev, uint8_t *config)
>       * divided by 512 - instead it is the amount of blk_size blocks
>       * per track (cylinder).
>       */
> -    if (bdrv_getlength(s->bs) /  s->conf->heads / s->conf->secs % blk_size) {
> -        blkcfg.sectors = s->conf->secs & ~s->sector_mask;
> +    if (bdrv_getlength(s->bs) /  conf->heads / conf->secs % blk_size) {
> +        blkcfg.sectors = conf->secs & ~s->sector_mask;
>      } else {
> -        blkcfg.sectors = s->conf->secs;
> +        blkcfg.sectors = conf->secs;
>      }
>      blkcfg.size_max = 0;
> -    blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
> +    blkcfg.physical_block_exp = get_physical_block_exp(&s->blk.conf);
>      blkcfg.alignment_offset = 0;
>      blkcfg.wce = bdrv_enable_write_cache(s->bs);
>      memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
> @@ -756,9 +757,8 @@ static void virtio_blk_device_realize(DeviceState *dev, 
> Error **errp)
>                  sizeof(struct virtio_blk_config));
>  
>      s->bs = blk->conf.bs;
> -    s->conf = &blk->conf;
>      s->rq = NULL;
> -    s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
> +    s->sector_mask = (s->blk.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1;
>  
>      s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
>      s->complete_request = virtio_blk_complete_request;
> @@ -777,11 +777,11 @@ static void virtio_blk_device_realize(DeviceState *dev, 
> Error **errp)
>      register_savevm(dev, "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_guest_block_size(s->bs, s->conf->logical_block_size);
> +    bdrv_set_guest_block_size(s->bs, s->blk.conf.logical_block_size);
>  
>      bdrv_iostatus_enable(s->bs);
>  
> -    add_boot_device_path(s->conf->bootindex, dev, "/address@hidden,0");
> +    add_boot_device_path(s->blk.conf.bootindex, dev, "/address@hidden,0");
>  }
>  
>  static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index cf61154..18967c8 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -125,7 +125,6 @@ typedef struct VirtIOBlock {
>      VirtQueue *vq;
>      void *rq;
>      QEMUBH *bh;
> -    BlockConf *conf;
>      VirtIOBlkConf blk;
>      unsigned short sector_mask;
>      bool original_wce;
> -- 
> 1.9.3
> 

Look good

Reviewed-by: Benoit Canet <address@hidden>