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

[Christian Borntraeger]

Am 12.02.2015 um 17:42 schrieb Christian Borntraeger:
> 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 <address@hidden>
> 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.
> 
> Now we have 
> block.c:582: bdrv_probe_blocksizes: Assertion `drv != ((void *)0)' failed
> 
> so
> we need something like this on top - I guess.
> 
> diff --git a/block.c b/block.c
> index dbc2519..8bbcc6d 100644
> --- a/block.c
> +++ b/block.c
> @@ -579,8 +579,7 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, 
> BlockSizes *bsz)
>  {
>      BlockDriver *drv = bs->drv;
>  
> -    assert(drv != NULL);
> -    if (drv->bdrv_probe_blocksizes) {
> +    if (drv && drv->bdrv_probe_blocksizes) {
>          return drv->bdrv_probe_blocksizes(bs, bsz);
>      }
>  
> @@ -597,8 +596,7 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry 
> *geo)
>  {
>      BlockDriver *drv = bs->drv;
>  
> -    assert(drv != NULL);
> -    if (drv->bdrv_probe_geometry) {
> +    if (drv && drv->bdrv_probe_geometry) {
>          return drv->bdrv_probe_geometry(bs, geo);
>      }
>  
> 
> Kate, I think its time for a v7 :-(

Of course, please double check my quick hacks. I still see some broken pipe 
messages, so I must have missed something.