Patchwork [v2,3/3] block: enforce constraints on block size properties

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 14, 2012, 3:57 p.m.
Message ID <1331740626-5053-4-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/146657/
State New
Headers show

Comments

Stefan Hajnoczi - March 14, 2012, 3:57 p.m.
Nicolae Mogoreanu <mogo@google.com> noticed that I/O requests can lead
to QEMU crashes when the logical_block_size property is smaller than 512
bytes.

Using the new "blocksize" property we can properly enforce constraints
on the block size such that QEMU's block layer is able to operate
correctly.

Reported-by: Nicolae Mogoreanu <mogo@google.com>
Reported-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Paolo Bonzini - March 14, 2012, 4:04 p.m.
Il 14/03/2012 16:57, Stefan Hajnoczi ha scritto:
> Nicolae Mogoreanu <mogo@google.com> noticed that I/O requests can lead
> to QEMU crashes when the logical_block_size property is smaller than 512
> bytes.
> 
> Using the new "blocksize" property we can properly enforce constraints
> on the block size such that QEMU's block layer is able to operate
> correctly.
> 
> Reported-by: Nicolae Mogoreanu <mogo@google.com>
> Reported-by: Michael Halcrow <mhalcrow@google.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.h |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block.h b/block.h
> index 415bb17..3df6c30 100644
> --- a/block.h
> +++ b/block.h
> @@ -437,10 +437,10 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
>  
>  #define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
>      DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
> -    DEFINE_PROP_UINT16("logical_block_size", _state,                    \
> -                       _conf.logical_block_size, 512),                  \
> -    DEFINE_PROP_UINT16("physical_block_size", _state,                   \
> -                       _conf.physical_block_size, 512),                 \
> +    DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
> +                          _conf.logical_block_size, 512),               \
> +    DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
> +                          _conf.physical_block_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),    \
>      DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Kevin Wolf - March 14, 2012, 4:13 p.m.
Am 14.03.2012 17:04, schrieb Paolo Bonzini:
> Il 14/03/2012 16:57, Stefan Hajnoczi ha scritto:
>> Nicolae Mogoreanu <mogo@google.com> noticed that I/O requests can lead
>> to QEMU crashes when the logical_block_size property is smaller than 512
>> bytes.
>>
>> Using the new "blocksize" property we can properly enforce constraints
>> on the block size such that QEMU's block layer is able to operate
>> correctly.
>>
>> Reported-by: Nicolae Mogoreanu <mogo@google.com>
>> Reported-by: Michael Halcrow <mhalcrow@google.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>> ---
>>  block.h |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.h b/block.h
>> index 415bb17..3df6c30 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -437,10 +437,10 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
>>  
>>  #define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
>>      DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
>> -    DEFINE_PROP_UINT16("logical_block_size", _state,                    \
>> -                       _conf.logical_block_size, 512),                  \
>> -    DEFINE_PROP_UINT16("physical_block_size", _state,                   \
>> -                       _conf.physical_block_size, 512),                 \
>> +    DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
>> +                          _conf.logical_block_size, 512),               \
>> +    DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
>> +                          _conf.physical_block_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),    \
>>      DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied all.

Kevin

Patch

diff --git a/block.h b/block.h
index 415bb17..3df6c30 100644
--- a/block.h
+++ b/block.h
@@ -437,10 +437,10 @@  static inline unsigned int get_physical_block_exp(BlockConf *conf)
 
 #define DEFINE_BLOCK_PROPERTIES(_state, _conf)                          \
     DEFINE_PROP_DRIVE("drive", _state, _conf.bs),                       \
-    DEFINE_PROP_UINT16("logical_block_size", _state,                    \
-                       _conf.logical_block_size, 512),                  \
-    DEFINE_PROP_UINT16("physical_block_size", _state,                   \
-                       _conf.physical_block_size, 512),                 \
+    DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
+                          _conf.logical_block_size, 512),               \
+    DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
+                          _conf.physical_block_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),    \
     DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \