| Message ID | 1344847212-5047-2-git-send-email-jfrei@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 13, 2012 at 8:40 AM, Jens Freimann <jfrei@de.ibm.com> wrote: > From: Einar Lueck <elelueck@linux.vnet.ibm.com> > > Virtio-blk does not impose fixed block sizes for access to > backing devices. This patch introduces support for auto > lookup of the block sizes of the backing block device. This > automatic lookup needs to be enabled explicitly. Users may > do this by specifying (physical|logical)_block_size=0. > Machine types may do this for their defaults, too. > To achieve this, a new function blkconf_blocksizes is > implemented. If physical or logical block size are zero > a corresponding ioctl tries to find an appropriate value. > If this does not work, 512 is used. blkconf_blocksizes > is therefore only called w/in the virtio-blk context. > For s390-virtio, this patch configures auto lookup as > default. > > Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > --- > hw/block-common.c | 23 +++++++++++++++++++++++ > hw/block-common.h | 12 +++++++++--- > hw/qdev-properties.c | 4 +++- > hw/s390-virtio-bus.c | 2 +- > hw/virtio-blk.c | 1 + > 5 files changed, 37 insertions(+), 5 deletions(-) > > diff --git a/hw/block-common.c b/hw/block-common.c > index f0196d7..444edd2 100644 > --- a/hw/block-common.c > +++ b/hw/block-common.c > @@ -10,6 +10,9 @@ > #include "blockdev.h" > #include "hw/block-common.h" > #include "qemu-error.h" > +#ifdef __linux__ > +#include <linux/fs.h> > +#endif > > void blkconf_serial(BlockConf *conf, char **serial) > { > @@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial) > } > } > > +void blkconf_blocksizes(BlockConf *conf) > +{ > + int block_size; > + > + if (!conf->physical_block_size) { > + if (bdrv_ioctl(conf->bs, BLKPBSZGET, &block_size) == 0) { This use of BLKPBSZGET is not protected by #ifdef __linux__ (or BLKPBSZGET), so this will probably fail when the host OS is not Linux and BLKPBSZGET is not defined. > + conf->physical_block_size = (uint16_t) block_size; > + } else { > + conf->physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; > + } > + } > + if (!conf->logical_block_size) { > + if (bdrv_ioctl(conf->bs, BLKSSZGET, &block_size) == 0) { Also here. > + conf->logical_block_size = (uint16_t) block_size; > + } else { > + conf->logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; > + } > + } > +} > + > int blkconf_geometry(BlockConf *conf, int *ptrans, > unsigned cyls_max, unsigned heads_max, unsigned secs_max) > { > diff --git a/hw/block-common.h b/hw/block-common.h > index bb808f7..d593128 100644 > --- a/hw/block-common.h > +++ b/hw/block-common.h > @@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > return exp; > } > > -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > +#define BLOCK_PROPERTY_STD_BLKSIZE 512 > +#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize) \ > DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ > DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ > - _conf.logical_block_size, 512), \ > + _conf.logical_block_size, _blksize), \ > DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ > - _conf.physical_block_size, 512), \ > + _conf.physical_block_size, _blksize), \ > DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ > DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ > DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ > DEFINE_PROP_UINT32("discard_granularity", _state, \ > _conf.discard_granularity, 0) > > +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > + DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, \ > + BLOCK_PROPERTY_STD_BLKSIZE) > + > #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ > DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ > DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ > @@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > /* Configuration helpers */ > > void blkconf_serial(BlockConf *conf, char **serial); > +void blkconf_blocksizes(BlockConf *conf); > int blkconf_geometry(BlockConf *conf, int *trans, > unsigned cyls_max, unsigned heads_max, unsigned secs_max); > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 8aca0d4..e99dee4 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, > error_propagate(errp, local_err); > return; > } > - if (value < min || value > max) { > + > + /* value == 0 indicates that block size should be sensed later on */ > + if ((value < min || value > max) && value > 0) { > error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, > dev->id?:"", name, (int64_t)value, min, max); > return; > diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > index a245684..562964e 100644 > --- a/hw/s390-virtio-bus.c > +++ b/hw/s390-virtio-bus.c > @@ -401,7 +401,7 @@ static TypeInfo s390_virtio_net = { > }; > > static Property s390_virtio_blk_properties[] = { > - DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf), > + DEFINE_BLOCK_PROPERTIES_EXTENDED(VirtIOS390Device, blk.conf, 0), > DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf), > DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial), > #ifdef __linux__ > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index f21757e..7686433 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -600,6 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) > } > > blkconf_serial(&blk->conf, &blk->serial); > + blkconf_blocksizes(&blk->conf); > if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) { > return NULL; > } > -- > 1.7.11.4 > >
On Mon, Aug 13, 2012 at 07:50:39PM +0000, Blue Swirl wrote: > On Mon, Aug 13, 2012 at 8:40 AM, Jens Freimann <jfrei@de.ibm.com> wrote: > > From: Einar Lueck <elelueck@linux.vnet.ibm.com> > > > > Virtio-blk does not impose fixed block sizes for access to > > backing devices. This patch introduces support for auto > > lookup of the block sizes of the backing block device. This > > automatic lookup needs to be enabled explicitly. Users may > > do this by specifying (physical|logical)_block_size=0. > > Machine types may do this for their defaults, too. > > To achieve this, a new function blkconf_blocksizes is > > implemented. If physical or logical block size are zero > > a corresponding ioctl tries to find an appropriate value. > > If this does not work, 512 is used. blkconf_blocksizes > > is therefore only called w/in the virtio-blk context. > > For s390-virtio, this patch configures auto lookup as > > default. > > > > Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com> > > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > > --- > > hw/block-common.c | 23 +++++++++++++++++++++++ > > hw/block-common.h | 12 +++++++++--- > > hw/qdev-properties.c | 4 +++- > > hw/s390-virtio-bus.c | 2 +- > > hw/virtio-blk.c | 1 + > > 5 files changed, 37 insertions(+), 5 deletions(-) > > > > diff --git a/hw/block-common.c b/hw/block-common.c > > index f0196d7..444edd2 100644 > > --- a/hw/block-common.c > > +++ b/hw/block-common.c > > @@ -10,6 +10,9 @@ > > #include "blockdev.h" > > #include "hw/block-common.h" > > #include "qemu-error.h" > > +#ifdef __linux__ > > +#include <linux/fs.h> > > +#endif > > > > void blkconf_serial(BlockConf *conf, char **serial) > > { > > @@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial) > > } > > } > > > > +void blkconf_blocksizes(BlockConf *conf) > > +{ > > + int block_size; > > + > > + if (!conf->physical_block_size) { > > + if (bdrv_ioctl(conf->bs, BLKPBSZGET, &block_size) == 0) { > > This use of BLKPBSZGET is not protected by #ifdef __linux__ (or > BLKPBSZGET), so this will probably fail when the host OS is not Linux > and BLKPBSZGET is not defined. Yes, you're right. > > + conf->physical_block_size = (uint16_t) block_size; > > + } else { > > + conf->physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; > > + } > > + } > > + if (!conf->logical_block_size) { > > + if (bdrv_ioctl(conf->bs, BLKSSZGET, &block_size) == 0) { > > Also here. Ok, will fix. Thanks for the review! Jens > > + conf->logical_block_size = (uint16_t) block_size; > > + } else { > > + conf->logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; > > + } > > + } > > +} > > + > > int blkconf_geometry(BlockConf *conf, int *ptrans, > > unsigned cyls_max, unsigned heads_max, unsigned secs_max) > > { > > diff --git a/hw/block-common.h b/hw/block-common.h > > index bb808f7..d593128 100644 > > --- a/hw/block-common.h > > +++ b/hw/block-common.h > > @@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > > return exp; > > } > > > > -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > > +#define BLOCK_PROPERTY_STD_BLKSIZE 512 > > +#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize) \ > > DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ > > DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ > > - _conf.logical_block_size, 512), \ > > + _conf.logical_block_size, _blksize), \ > > DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ > > - _conf.physical_block_size, 512), \ > > + _conf.physical_block_size, _blksize), \ > > DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ > > DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ > > DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ > > DEFINE_PROP_UINT32("discard_granularity", _state, \ > > _conf.discard_granularity, 0) > > > > +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > > + DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, \ > > + BLOCK_PROPERTY_STD_BLKSIZE) > > + > > #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ > > DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ > > DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ > > @@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > > /* Configuration helpers */ > > > > void blkconf_serial(BlockConf *conf, char **serial); > > +void blkconf_blocksizes(BlockConf *conf); > > int blkconf_geometry(BlockConf *conf, int *trans, > > unsigned cyls_max, unsigned heads_max, unsigned secs_max); > > > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > > index 8aca0d4..e99dee4 100644 > > --- a/hw/qdev-properties.c > > +++ b/hw/qdev-properties.c > > @@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, > > error_propagate(errp, local_err); > > return; > > } > > - if (value < min || value > max) { > > + > > + /* value == 0 indicates that block size should be sensed later on */ > > + if ((value < min || value > max) && value > 0) { > > error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, > > dev->id?:"", name, (int64_t)value, min, max); > > return; > > diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > > index a245684..562964e 100644 > > --- a/hw/s390-virtio-bus.c > > +++ b/hw/s390-virtio-bus.c > > @@ -401,7 +401,7 @@ static TypeInfo s390_virtio_net = { > > }; > > > > static Property s390_virtio_blk_properties[] = { > > - DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf), > > + DEFINE_BLOCK_PROPERTIES_EXTENDED(VirtIOS390Device, blk.conf, 0), > > DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf), > > DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial), > > #ifdef __linux__ > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > > index f21757e..7686433 100644 > > --- a/hw/virtio-blk.c > > +++ b/hw/virtio-blk.c > > @@ -600,6 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) > > } > > > > blkconf_serial(&blk->conf, &blk->serial); > > + blkconf_blocksizes(&blk->conf); > > if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) { > > return NULL; > > } > > -- > > 1.7.11.4 > > > > >
diff --git a/hw/block-common.c b/hw/block-common.c index f0196d7..444edd2 100644 --- a/hw/block-common.c +++ b/hw/block-common.c @@ -10,6 +10,9 @@ #include "blockdev.h" #include "hw/block-common.h" #include "qemu-error.h" +#ifdef __linux__ +#include <linux/fs.h> +#endif void blkconf_serial(BlockConf *conf, char **serial) { @@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial) } } +void blkconf_blocksizes(BlockConf *conf) +{ + int block_size; + + if (!conf->physical_block_size) { + if (bdrv_ioctl(conf->bs, BLKPBSZGET, &block_size) == 0) { + conf->physical_block_size = (uint16_t) block_size; + } else { + conf->physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; + } + } + if (!conf->logical_block_size) { + if (bdrv_ioctl(conf->bs, BLKSSZGET, &block_size) == 0) { + conf->logical_block_size = (uint16_t) block_size; + } else { + conf->logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; + } + } +} + int blkconf_geometry(BlockConf *conf, int *ptrans, unsigned cyls_max, unsigned heads_max, unsigned secs_max) { diff --git a/hw/block-common.h b/hw/block-common.h index bb808f7..d593128 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) return exp; } -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ +#define BLOCK_PROPERTY_STD_BLKSIZE 512 +#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize) \ DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ - _conf.logical_block_size, 512), \ + _conf.logical_block_size, _blksize), \ DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ - _conf.physical_block_size, 512), \ + _conf.physical_block_size, _blksize), \ DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ DEFINE_PROP_UINT32("discard_granularity", _state, \ _conf.discard_granularity, 0) +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ + DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, \ + BLOCK_PROPERTY_STD_BLKSIZE) + #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ @@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ void blkconf_serial(BlockConf *conf, char **serial); +void blkconf_blocksizes(BlockConf *conf); int blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max); diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..e99dee4 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } - if (value < min || value > max) { + + /* value == 0 indicates that block size should be sensed later on */ + if ((value < min || value > max) && value > 0) { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, dev->id?:"", name, (int64_t)value, min, max); return; diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index a245684..562964e 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -401,7 +401,7 @@ static TypeInfo s390_virtio_net = { }; static Property s390_virtio_blk_properties[] = { - DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf), + DEFINE_BLOCK_PROPERTIES_EXTENDED(VirtIOS390Device, blk.conf, 0), DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf), DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial), #ifdef __linux__ diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index f21757e..7686433 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -600,6 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk) } blkconf_serial(&blk->conf, &blk->serial); + blkconf_blocksizes(&blk->conf); if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) { return NULL; }