Damien Le Moal <damien.lem...@opensource.wdc.com>于2022年9月19日 周一16:04写道:
> On 9/19/22 09:50, Sam Li wrote: > > Stefan Hajnoczi <stefa...@redhat.com> 于2022年9月18日周日 04:17写道: > >> > >> On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote: > >>> Eric Blake <ebl...@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 <faithilike...@gmail.com> > >>>>> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > >>>>> Reviewed-by: Damien Le Moal <damien.lem...@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 > >