Re: [PATCH v9 1/7] include: add zoned device structs

[Sam Li <faithilikerun@gmail.com>]

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

Damien Le Moal <damien.lemoal@opensource.wdc.com>于2022年9月19日 周一16:04写道：

> On 9/19/22 09:50, Sam Li wrote:
> > Stefan Hajnoczi <stefanha@redhat.com> 于2022年9月18日周日 04:17写道：
> >>
> >> On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote:
> >>> Eric Blake <eblake@redhat.com> 于2022年9月15日周四 16:05写道：
> >>>>
> >>>> On Sat, Sep 10, 2022 at 01:27:53PM +0800, Sam Li wrote:
> >>>>> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> >>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>>>> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> >>>>> ---
> >>>>>  include/block/block-common.h | 43
> ++++++++++++++++++++++++++++++++++++
> >>>>>  1 file changed, 43 insertions(+)
> >>>>>
> >>>>> diff --git a/include/block/block-common.h
> b/include/block/block-common.h
> >>>>> index fdb7306e78..36bd0e480e 100644
> >>>>> --- a/include/block/block-common.h
> >>>>> +++ b/include/block/block-common.h
> >>>>> @@ -49,6 +49,49 @@ typedef struct BlockDriver BlockDriver;
> >>>>>  typedef struct BdrvChild BdrvChild;
> >>>>>  typedef struct BdrvChildClass BdrvChildClass;
> >>>>>
> >>>>> +typedef enum BlockZoneOp {
> >>>>> +    BLK_ZO_OPEN,
> >>>>> +    BLK_ZO_CLOSE,
> >>>>> +    BLK_ZO_FINISH,
> >>>>> +    BLK_ZO_RESET,
> >>>>> +} BlockZoneOp;
> >>>>> +
> >>>>> +typedef enum BlockZoneModel {
> >>>>> +    BLK_Z_NONE = 0x0, /* Regular block device */
> >>>>> +    BLK_Z_HM = 0x1, /* Host-managed zoned block device */
> >>>>> +    BLK_Z_HA = 0x2, /* Host-aware zoned block device */
> >>>>> +} BlockZoneModel;
> >>>>> +
> >>>>> +typedef enum BlockZoneCondition {
> >>>>> +    BLK_ZS_NOT_WP = 0x0,
> >>>>> +    BLK_ZS_EMPTY = 0x1,
> >>>>> +    BLK_ZS_IOPEN = 0x2,
> >>>>> +    BLK_ZS_EOPEN = 0x3,
> >>>>> +    BLK_ZS_CLOSED = 0x4,
> >>>>> +    BLK_ZS_RDONLY = 0xD,
> >>>>> +    BLK_ZS_FULL = 0xE,
> >>>>> +    BLK_ZS_OFFLINE = 0xF,
> >>>>> +} BlockZoneCondition;
> >>>>> +
> >>>>> +typedef enum BlockZoneType {
> >>>>> +    BLK_ZT_CONV = 0x1, /* Conventional random writes supported */
> >>>>> +    BLK_ZT_SWR = 0x2, /* Sequential writes required */
> >>>>> +    BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> >>>>> +} BlockZoneType;
> >>>>> +
> >>>>> +/*
> >>>>> + * Zone descriptor data structure.
> >>>>> + * Provides information on a zone with all position and size values
> in bytes.
> >>>>
> >>>> I'm glad that you chose bytes here for use in qemu.  But since the
> >>>> kernel struct blk_zone uses sectors instead of bytes, is it worth
> >>>> adding a sentence that we intentionally use bytes here, different from
> >>>> Linux, to make it easier for reviewers to realize that scaling when
> >>>> translating between qemu and kernel is necessary?
> >>>
> >>> Sorry about the unit mistake. The zone information is in sectors which
> >>> is the same as kernel struct blk_zone. I think adding a sentence to
> >>> inform the sector unit makes it clear what the zone descriptor is.
> >>
> >> I'd make the units bytes for consistency with the rest of the QEMU block
> >> layer. For example, the MapEntry structure that "qemu-img map" reports
> >> has names with similar fields and they are in bytes:
> >>
> >>   struct MapEntry {
> >>       int64_t start;
> >>       int64_t length;
> >>
> >
> > I think the zone descriptor uses sector units because ioctl() will
> > report zones in sector units. Making blk_zone.offset =
> > zone_descriptor.offset is more convenient than using byte units where
> > it needs make conversions twice(sector -> byte -> sector in zone
> > descriptors and offset argument in bdrv_co_zone_report). The MapEntry
> > uses byte units because lseek() in bdrv_co_block_status suggests the
> > file offset is set to bytes and I think it may be why the rest of the
> > block layer uses bytes(not sure).
> >
> > I do not object to using bytes here but it would require some
> > compromises. If I was wrong about anything, please let me know.
>
> The conversion can be done using 9-bits left and right shifts, which are
> cheap to do. I think it is important to be consistent with qemu block API,
> so using for the API bytes is preferred. That will avoid confusions.


Ok, will change it. Thanks!


>
> >
> >
> > Sam
>
> --
> Damien Le Moal
> Western Digital Research
>
>

[-- Attachment #2: Type: text/html, Size: 6367 bytes --]