[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v15 3/8] block: add block layer APIs resembling Linux ZonedBl
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v15 3/8] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls |
Date: |
Mon, 27 Feb 2023 14:14:22 -0500 |
On Mon, Feb 27, 2023 at 07:20:14PM +0100, Kevin Wolf wrote:
> Am 29.01.2023 um 11:28 hat Sam Li geschrieben:
> > Add zoned device option to host_device BlockDriver. It will be presented
> > only
> > for zoned host block devices. By adding zone management operations to the
> > host_block_device BlockDriver, users can use the new block layer APIs
> > including Report Zone and four zone management operations
> > (open, close, finish, reset, reset_all).
> >
> > Qemu-io uses the new APIs to perform zoned storage commands of the device:
> > zone_report(zrp), zone_open(zo), zone_close(zc), zone_reset(zrs),
> > zone_finish(zf).
> >
> > For example, to test zone_report, use following command:
> > $ ./build/qemu-io --image-opts -n driver=host_device, filename=/dev/nullb0
> > -c "zrp offset nr_zones"
> >
> > Signed-off-by: Sam Li <faithilikerun@gmail.com>
> > Reviewed-by: Hannes Reinecke <hare@suse.de>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > block/block-backend.c | 147 ++++++++++++++
> > block/file-posix.c | 323 ++++++++++++++++++++++++++++++
> > block/io.c | 41 ++++
> > include/block/block-io.h | 7 +
> > include/block/block_int-common.h | 21 ++
> > include/block/raw-aio.h | 6 +-
> > include/sysemu/block-backend-io.h | 18 ++
> > meson.build | 4 +
> > qemu-io-cmds.c | 149 ++++++++++++++
> > 9 files changed, 715 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index ba7bf1d6bc..a4847b9131 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -1451,6 +1451,15 @@ typedef struct BlkRwCo {
> > void *iobuf;
> > int ret;
> > BdrvRequestFlags flags;
> > + union {
> > + struct {
> > + unsigned int *nr_zones;
> > + BlockZoneDescriptor *zones;
> > + } zone_report;
> > + struct {
> > + unsigned long op;
> > + } zone_mgmt;
> > + };
> > } BlkRwCo;
>
> Should we use a different struct for blk_aio_zone_*() so that we don't
> need to touch the one for the normal I/O path? My concern is that
> increasing the size of the struct (currently 32 bytes) might negatively
> impact the performance even of non-zoned devices. Maybe it turns out
> that it wasn't really necessary in the end (have we done any
> benchmarks?), but I don't think it can hurt anyway.
>
> With this changed, you can add to the series:
> Acked-by: Kevin Wolf <kwolf@redhat.com>
There are unused fields in BlkRwCo and BlkAioEmAIOCB, so changing the
size of the struct isn't necessary. ioctl/flush/pdiscard already use
BlkAioEmAIOCB/BlkRwCo for non-read/write operations, including using the
iobuf field for different types, so it wouldn't be weird:
typedef struct BlkRwCo {
BlockBackend *blk;
int64_t offset;
void *iobuf;
^^^^^ used for preadv/pwritev qiov, ioctl buf, and NULL for
other request types. zone_report could put the
BlockZoneDescriptor pointer here. zone_mgmt could put
op here.
int ret;
BdrvRequestFlags flags;
} BlkRwCo;
typedef struct BlkAioEmAIOCB {
BlockAIOCB common;
BlkRwCo rwco;
int64_t bytes;
^^^^^ zone_report could put the nr_zones pointer here
bool has_returned;
} BlkAioEmAIOCB;
Does that sound okay?
Stefan
signature.asc
Description: PGP signature