[Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Christoph Hellwig]

Currently we set them to 512 bytes unless manually specified.  Unforuntaly
some brain-dead partitioning tools create unaligned partitions if they
get low enough optiomal I/O size values, so don't report any at all
unless explicitly set.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/block_int.h
===================================================================
--- qemu.orig/block_int.h	2010-07-23 09:26:07.660494681 +0200
+++ qemu/block_int.h	2010-07-23 09:26:20.323494685 +0200
@@ -243,7 +243,7 @@ static inline unsigned int get_physical_
                        _conf.logical_block_size, 512),                  \
     DEFINE_PROP_UINT16("physical_block_size", _state,                   \
                        _conf.physical_block_size, 512),                 \
-    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512),  \
-    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512)
+    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
+    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
 
 #endif /* BLOCK_INT_H */

Re: [Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Kevin Wolf]

Am 23.07.2010 09:35, schrieb Christoph Hellwig:
> Currently we set them to 512 bytes unless manually specified.  Unforuntaly
> some brain-dead partitioning tools create unaligned partitions if they
> get low enough optiomal I/O size values, so don't report any at all
> unless explicitly set.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: qemu/block_int.h
> ===================================================================
> --- qemu.orig/block_int.h	2010-07-23 09:26:07.660494681 +0200
> +++ qemu/block_int.h	2010-07-23 09:26:20.323494685 +0200
> @@ -243,7 +243,7 @@ static inline unsigned int get_physical_
>                         _conf.logical_block_size, 512),                  \
>      DEFINE_PROP_UINT16("physical_block_size", _state,                   \
>                         _conf.physical_block_size, 512),                 \
> -    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512),  \
> -    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512)
> +    DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
> +    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
>  
>  #endif /* BLOCK_INT_H */

This isn't reverting to the state before we reported anything, but it
reports values of 0 now. Is this defined for both virtio-blk and SCSI to
mean the same as no report at all? Or should we rather not advertise
VIRTIO_BLK_F_TOPOLOGY (and the SCSI equivalent) in this case?

Kevin

Re: [Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Christoph Hellwig]

On Fri, Jul 23, 2010 at 12:50:12PM +0200, Kevin Wolf wrote:
> This isn't reverting to the state before we reported anything, but it
> reports values of 0 now. Is this defined for both virtio-blk and SCSI to
> mean the same as no report at all? Or should we rather not advertise
> VIRTIO_BLK_F_TOPOLOGY (and the SCSI equivalent) in this case?

For scsi it's explicitly defined that way, and there is not equivalent
to the VIRTIO_BLK_F_TOPOLOGY because it will just use the VPD data if
the page is long enough.  virtio is a bit underspecified, but the
Linux guest driver has the same behaviour as scsi in that respect.

Re: [Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Kevin Wolf]

Am 23.07.2010 13:30, schrieb Christoph Hellwig:
> On Fri, Jul 23, 2010 at 12:50:12PM +0200, Kevin Wolf wrote:
>> This isn't reverting to the state before we reported anything, but it
>> reports values of 0 now. Is this defined for both virtio-blk and SCSI to
>> mean the same as no report at all? Or should we rather not advertise
>> VIRTIO_BLK_F_TOPOLOGY (and the SCSI equivalent) in this case?
> 
> For scsi it's explicitly defined that way, and there is not equivalent
> to the VIRTIO_BLK_F_TOPOLOGY because it will just use the VPD data if
> the page is long enough.  virtio is a bit underspecified, but the
> Linux guest driver has the same behaviour as scsi in that respect.

Ok, then the right fix is to clarify the spec probably.

I've applied the patch to the block branch. This is something for 0.13,
too, I suppose?

Kevin

Re: [Qemu-devel] [PATCH] block: default to 0 minimal / optiomal I/O size

[Christoph Hellwig]

On Fri, Jul 23, 2010 at 01:56:50PM +0200, Kevin Wolf wrote:
> I've applied the patch to the block branch. This is something for 0.13,
> too, I suppose?

Yes, I think so.  It's easy enough and the broken guest already are in
the wild.