[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC 2/4] qcow2: add configurations for zoned format extension
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC 2/4] qcow2: add configurations for zoned format extension |
Date: |
Tue, 20 Jun 2023 16:44:25 +0200 |
On Mon, Jun 19, 2023 at 10:50:31PM +0800, Sam Li wrote:
> Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 22:42写道:
> >
> > On Mon, Jun 19, 2023 at 06:32:52PM +0800, Sam Li wrote:
> > > Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道:
> > > > On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > > > > diff --git a/block/qcow2.h b/block/qcow2.h
> > > > > index 4f67eb912a..fe18dc4d97 100644
> > > > > --- a/block/qcow2.h
> > > > > +++ b/block/qcow2.h
> > > > > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > > > > uint64_t length;
> > > > > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> > > > >
> > > > > +typedef struct Qcow2ZonedHeaderExtension {
> > > > > + /* Zoned device attributes */
> > > > > + BlockZonedProfile zoned_profile;
> > > > > + BlockZoneModel zoned;
> > > > > + uint32_t zone_size;
> > > > > + uint32_t zone_capacity;
> > > > > + uint32_t nr_zones;
> > > > > + uint32_t zone_nr_conv;
> > > > > + uint32_t max_active_zones;
> > > > > + uint32_t max_open_zones;
> > > > > + uint32_t max_append_sectors;
> > > > > + uint8_t padding[3];
> > > >
> > > > This looks strange. Why is there 3 bytes of padding at the end? Normally
> > > > padding would align to an even power-of-two number of bytes like 2, 4,
> > > > 8, etc.
> > >
> > > It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
> > > 16, the padding is 2.
> >
> > I don't understand. Can you explain why there is padding at the end of
> > this struct?
>
> The overall size should be aligned with 64 bit, which leaves use one
> uint32_t and two fields zoned, zoned_profile. I am not sure the size
> of macros here and it used 4 for each. So it makes 3 (*8) + 32 + 8 =
> 64 in the end. If the macro size is wrong, then the padding will
> change as well.
The choice of the type (char or int) representing an enum is
implementation-defined according to the C17 standard (see "6.7.2.2
Enumeration specifiers").
Therefore it's not portable to use enums in structs exposed to the
outside world (on-disk formats or network protocols).
Please use uint8_t for the zoned_profile and zoned fields and move them
to the end of the struct so the uint32_t fields are naturally aligned.
I think only 2 bytes of padding will be required to align the struct to
a 64-bit boundary once you've done that.
Stefan
signature.asc
Description: PGP signature
[RFC 3/4] qcow2: add zoned emulation capability, Sam Li, 2023/06/05
[RFC 4/4] iotests: test the zoned format feature for qcow2 file, Sam Li, 2023/06/05