Re: [PATCH v4 2/3] block: consolidate blocksize properties consistency checks

[Kevin Wolf <kwolf@redhat.com>]

Am 20.05.2020 um 10:59 hat Philippe Mathieu-Daudé geschrieben:
> On Wed, May 20, 2020 at 10:57 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > Hi Roman,
> >
> > On 5/20/20 10:06 AM, Roman Kagan wrote:
> > > Several block device properties related to blocksize configuration must
> > > be in certain relationship WRT each other: physical block must be no
> > > smaller than logical block; min_io_size, opt_io_size, and
> > > discard_granularity must be a multiple of a logical block.
> > >
> > > To ensure these requirements are met, add corresponding consistency
> > > checks to blkconf_blocksizes, adjusting its signature to communicate
> > > possible error to the caller.  Also remove the now redundant consistency
> > > checks from the specific devices.
> > >
> > > Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
> > > ---
> > > v4: new patch
> > >
> > >   include/hw/block/block.h   |  2 +-
> > >   hw/block/block.c           | 29 ++++++++++++++++++++++++++++-
> > >   hw/block/fdc.c             |  5 ++++-
> > >   hw/block/nvme.c            |  5 ++++-
> > >   hw/block/virtio-blk.c      |  7 +------
> > >   hw/ide/qdev.c              |  5 ++++-
> > >   hw/scsi/scsi-disk.c        | 10 +++-------
> > >   hw/usb/dev-storage.c       |  5 ++++-
> > >   tests/qemu-iotests/172.out |  2 +-
> > >   9 files changed, 50 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/hw/block/block.h b/include/hw/block/block.h
> > > index d7246f3862..784953a237 100644
> > > --- a/include/hw/block/block.h
> > > +++ b/include/hw/block/block.h
> > > @@ -87,7 +87,7 @@ bool blk_check_size_and_read_all(BlockBackend *blk, void *buf, hwaddr size,
> > >   bool blkconf_geometry(BlockConf *conf, int *trans,
> > >                         unsigned cyls_max, unsigned heads_max, unsigned secs_max,
> > >                         Error **errp);
> > > -void blkconf_blocksizes(BlockConf *conf);
> > > +bool blkconf_blocksizes(BlockConf *conf, Error **errp);
> 
> Maybe rename blkconf_validate_blocksizes()?

It does more than validating. It also assigns defaults or guesses
settings from the host file.

Kevin