On Fri, Feb 08, 2013 at 03:52:35PM +0100, Einar Lueck wrote:
> @@ -22,6 +25,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) {
#ifdef __linux__
> + conf->physical_block_size = (uint16_t) block_size;
4 spaces for indentation.
> + } 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) \
Detecting the underlying block size is a generally useful configuration
option. This should not be s390-specific, so no need to rename
DEFINE_BLOCK_PROPERTIES().
If you set the default to 0 then QEMU will do extra ioctls. Perhaps
BlockDriverState needs a function to probe block sizes. This way
block/raw-posix.c could do the probing only on hdev (host block
devices).
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index a8a31f5..73b6da0 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -650,7 +650,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;
In the current patch this allows through the 0 value. Since the patch
only adds a blkconf_blocksizes() call to hw/virtio-blk.c, who knows what
will happen to ide/scsi/etc -drives that are configured with 0.
We need to make sure that 0 is really handled for all emulated storage
controllers.
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 34913ee..cd25712 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -654,6 +654,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> VirtIOBlkConf *blk)
> }
>
> blkconf_serial(&blk->conf, &blk->serial);
> + blkconf_blocksizes(&blk->conf);
Shouldn't be virtio-specific, all -drives should be able to autodetect
block size.
