Re: [PATCH v2 2/4] qcow2: add configurations for zoned format extension

[Sam Li]

Damien Le Moal <dlemoal@kernel.org> 于2023年8月28日周一 18:22写道：
>
> On 8/28/23 19:18, Sam Li wrote:
> > Damien Le Moal <dlemoal@kernel.org> 于2023年8月28日周一 18:13写道：
> >>
> >> On 8/28/23 18:22, Sam Li wrote:
> >>> Stefan Hajnoczi <stefanha@redhat.com> 于2023年8月21日周一 21:31写道：
> >>>>
> >>>> On Mon, Aug 14, 2023 at 04:58:00PM +0800, Sam Li wrote:
> >>>>> diff --git a/block/qcow2.h b/block/qcow2.h
> >>>>> index f789ce3ae0..3694c8d217 100644
> >>>>> --- a/block/qcow2.h
> >>>>> +++ b/block/qcow2.h
> >>>>> @@ -236,6 +236,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> >>>>>      uint64_t length;
> >>>>>  } QEMU_PACKED Qcow2CryptoHeaderExtension;
> >>>>>
> >>>>> +typedef struct Qcow2ZonedHeaderExtension {
> >>>>> +    /* Zoned device attributes */
> >>>>> +    uint8_t zoned_profile;
> >>>>> +    uint8_t zoned;
> >>>>> +    uint16_t reserved16;
> >>>>> +    uint32_t zone_size;
> >>>>> +    uint32_t zone_capacity;
> >>>>
> >>>> Should zone capacity be stored individually for each zone (alongside the
> >>>> write pointer and other per zone metadata) instead of as a global value
> >>>> for all zones? My understanding is that NVMe ZNS does not have a global
> >>>> value and each zone could have a different zone capacity value.
> >>>
> >>> Though zone capacity is per-zone attribute, it remains same for all
> >>> zones in most cases. Referring to the NVMe ZNS spec, zone capacity
> >>> changes associate to RESET_ZONE op when the variable zone capacity bit
> >>> is '1'. It hasn't specifically tell what it is changed to. Current ZNS
> >>> emulation doesn't change zone capacity as well.
> >>>
> >>> If the Variable Zone Capacity bit is cleared to ‘0’ in the Zone
> >>> Operation Characteristics field in the Zoned
> >>> Namespace Command Set specific Identify Namespace data structure, then
> >>> this field does not change without a change to the format of the zoned
> >>> namespace.
> >>>
> >>> If the Variable Zone Capacity bit is set to ‘1’ in the Zone Operation
> >>> Characteristics field in the Zoned
> >>> Namespace Command Set specific Identify Namespace data structure, then
> >>> the zone capacity may
> >>> change upon successful completion of a Zone Management Send command
> >>> specifying the Zone Send
> >>> Action of Reset Zone.
> >>
> >> Regardless of the variable zone capacity feature, zone capacity is per 
> >> zone and
> >> may be different between zones. That is why it is reported per zone in zone
> >> report. The IO path code should not assume that the zone capacity is the 
> >> same
> >> for all zones.
> >
> > How is zone capacity changed, by devices or commands? Can you give
> > some example please?
>
> If the device does not support variable zone capacity, the zone capacity is
> fixed at device manufacturing time and never changes. It is reported per zone
> and you have to make things work with whatever value you see. The user cannot
> change device zone capacity.
>
> For you qcow2 zoned image, the equivalent is to fix the zone capacity when the
> image is created and not allowing to change it. And for simplicity, the same
> zone capacity value can be used for all zones, so having the zone capacity
> value in the header is OK.

Thanks!

>
> >
> >>
> >> For this particular case though, given that this is QCow2 emulation, 
> >> limiting
> >> ourselves to the same zone capacity for all zones is I think fine. But that
> >> should be clearly stated somewhere may be...
> >
> > I see. The qcow2 documentaion can add that.
> >
> >>
> >>>
> >>>>
> >>>>> +    uint32_t nr_zones;
> >>>>
> >>>> Is this field necessary since it can be derived from other image
> >>>> options: nr_zones = DIV_ROUND_UP(total_length, zone_capacity)?
> >>>
> >>> It can be dropped. I added this for reducing duplication. Thanks!
> >>
> >> --
> >> Damien Le Moal
> >> Western Digital Research
> >>
>
> --
> Damien Le Moal
> Western Digital Research
>