Re: [PATCH v13 3/8] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

On Mon, Dec 05, 2022 at 09:24:28PM +0800, Sam Li wrote:
> Stefan Hajnoczi <stefanha@redhat.com> 于2022年12月5日周一 20:20写道：
> >
> > On Wed, Nov 30, 2022 at 10:24:10AM +0800, Sam Li wrote:
> > > Stefan Hajnoczi <stefanha@gmail.com> 于2022年11月30日周三 10:01写道：
> > > > On Thu, 27 Oct 2022 at 11:46, Sam Li <faithilikerun@gmail.com> wrote:
> > > > > @@ -1374,9 +1428,11 @@ static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> > > > >      int ret;
> > > > >
> > > > >      /* If DASD, get blocksizes */
> > > > > +#ifndef CONFIG_BLKZONED
> > > > >      if (check_for_dasd(s->fd) < 0) {
> > > > >          return -ENOTSUP;
> > > > >      }
> > > > > +#endif
> > > >
> > > > What is the purpose of this #ifndef? .bdrv_probe_blocksizes() should
> > > > only return block sizes for s390 DASD devices. I don't think zoned
> > > > storage needs block size probing here.
> > >
> > > Zoned storage needs to be virtualized with the correct physical block
> > > size and logical block size. And the probing here can guarantee that.
> > > Or virtio-blk may send wrong block size to the guest. If manually set
> > > block size in the command line as before, it is somewhat inaccurate.
> >
> > I see. I/O won't work if the guest block size differs from the physical
> > zoned device's block size.
> >
> > However, we must not do this for regular host_device BlockDriverStates.
> > The block size is manually controlled from those devices and defaults to
> > 512B. That way the blocksize doesn't change across live migration and
> > break the guest.
> >
> > Please use a run-time check instead of an #ifdef. Only probe blocksizes
> > for dasd and zoned devices.
> 
> I see. Like this?
> 
> #ifndef CONFIG_BLKZONED
> static int hdev_probe_zbd_blocksizes(BlockDriverState *bs, BlockSizes *bsz){
>     int ret;
>     /* check zbd */
>     ...
>     /* probe zbd */
>     ....    }
> +#endif

Yes, I think that's the cleanest option.

You don't need to check if it's a ZBD. Only the zoned_host_device driver
will use this .bdrv_probe_blocksizes() callback. The regular host_device
driver will use hdev_probe_blocksizes().

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]