Re: [Qemu-devel] [PATCH v6 0/5] Geometry and blocksize detection for backing devices.

[Kevin Wolf <kwolf@redhat.com>]

Am 12.02.2015 um 17:42 hat Christian Borntraeger geschrieben:
> Am 12.02.2015 um 16:46 schrieb Stefan Hajnoczi:
> > On Mon, Jan 19, 2015 at 03:34:56PM +0100, Ekaterina Tumanova wrote:
> >> Updates v5 -> v6:
> >>
> >> Minor Updates according the last review from Stefan Hajnoczi:
> >> 1. Do not change the flow of code, factored out of raw_probe_alignment.
> >> 2. added #ifdef __linux__ in 2 places of raw-posix.c, mentioned by reviewer.
> >> 3. adjusted the comment hdev_probe_geometry according suggestment.
> >> 4. use bdrv_nb_sectors(bs) instead of bs->total_sectors.
> >> 5. do not discard error blk_probe_blocksizes(). now has rc.
> >> 6. put the 512-byte default blocksize value in blkconf_blocksizes.
> >> 7. drop the default parameter from the DEFINE_PROP_BLOCKSIZE() macro.
> > 
> > Unfortunately this series breaks "make check" so it cannot be merged:
> > 
> > GTESTER check-qtest-x86_64
> > qemu-system-x86_64: logical_block_size must be 512 for IDE
> > qemu-system-x86_64: Device initialization failed.
> > qemu-system-x86_64: Initialization of device ide-cd failed
> > Broken pipe
> > GTester: last random seed: R02S942fac7e56eff09e8ab7a7f7fecf847e
> > 
> 
> This particular message came in with 
> 
> commit d20051856cd2fa8f10fed2d2a0b2751de5f7b20d
> Author: Kevin Wolf <kwolf@redhat.com>
> Date:   Wed Dec 3 13:21:32 2014 +0100
> 
>     ide: Check validity of logical block size
> 
> so something like
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 353854c..2680275 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -163,7 +163,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>          return -1;
>      }
>  
> -    if (dev->conf.logical_block_size != 512) {
> +    if (dev->conf.logical_block_size != 512 &&
> +        dev->conf.logical_block_size != 0) {
>          error_report("logical_block_size must be 512 for IDE");
>          return -1;
>      }
> 
> 
> will fix this.

It would probably be better to set the default first and then make sure
that the final value, no matter whether explicitly specified or default,
is 512.

Kevin