| Message ID | 1410891148-28849-13-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <armbru@redhat.com> > --- > 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, "/disk@0,0"); > + add_boot_device_path(s->blk.conf.bootindex, dev, "/disk@0,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 <benoit.canet@nodalink.com>
On 16.09.2014 20:12, Markus Armbruster wrote: > Commit 12c5674 turned it into a pointer to member blk.conf. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > 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); Is there a reason for you not using "conf" instead of "&s->blk.conf" here? Of course, it's not wrong, so with the one or the other: Reviewed-by: Max Reitz <mreitz@redhat.com>
Max Reitz <mreitz@redhat.com> writes: > On 16.09.2014 20:12, Markus Armbruster wrote: >> Commit 12c5674 turned it into a pointer to member blk.conf. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> 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); > > Is there a reason for you not using "conf" instead of "&s->blk.conf" here? > > Of course, it's not wrong, so with the one or the other: Looks like an editing accident to me. I'll clean it up in v4. > Reviewed-by: Max Reitz <mreitz@redhat.com> Thanks!
Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: > Commit 12c5674 turned it into a pointer to member blk.conf. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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, "/disk@0,0"); + add_boot_device_path(s->blk.conf.bootindex, dev, "/disk@0,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;
Commit 12c5674 turned it into a pointer to member blk.conf. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/block/virtio-blk.c | 28 ++++++++++++++-------------- include/hw/virtio/virtio-blk.h | 1 - 2 files changed, 14 insertions(+), 15 deletions(-)