Re: [Qemu-block] [Qemu-devel] [RFC PATCH 07/12] qcow2-bitmap: add basic bitmaps info

[Vladimir Sementsov-Ogievskiy]

17.05.2018 00:17, John Snow wrote:
> On 05/14/2018 11:12 AM, Vladimir Sementsov-Ogievskiy wrote:
> > 12.05.2018 04:25, John Snow wrote:
> > > Add functions for querying the basic information inside of bitmaps.
> > > Restructure the bitmaps flags masks to facilitate providing a list of
> > > flags belonging to the bitmap(s) being queried.
> > >
> > > Signed-off-by: John Snow <address@hidden>
> > > ---
> > >    block/qcow2-bitmap.c | 81
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > >    block/qcow2.c        |  7 +++++
> > >    block/qcow2.h        |  1 +
> > >    3 files changed, 87 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> > > index 60e01abfd7..811b82743a 100644
> > > --- a/block/qcow2-bitmap.c
> > > +++ b/block/qcow2-bitmap.c
> > > @@ -49,8 +49,28 @@
> > >      /* Bitmap directory entry flags */
> > >    #define BME_RESERVED_FLAGS 0xfffffffcU
> > > -#define BME_FLAG_IN_USE (1U << 0)
> > > -#define BME_FLAG_AUTO   (1U << 1)
> > > +
> > > +enum BME_FLAG_BITS {
> > > +    BME_FLAG_BIT__BEGIN = 0,
> > > +    BME_FLAG_BIT_IN_USE = BME_FLAG_BIT__BEGIN,
> > > +    BME_FLAG_BIT_AUTO   = 1,
> > > +    BME_FLAG_BIT_EXTRA  = 2,
> > > +    BME_FLAG_BIT__MAX,
> > > +};
> > > +
> > > +#define BME_FLAG_IN_USE (1U << BME_FLAG_BIT_IN_USE)
> > > +#define BME_FLAG_AUTO   (1U << BME_FLAG_BIT_AUTO)
> > > +#define BME_FLAG_EXTRA  (1U << BME_FLAG_BIT_EXTRA)
> > > +
> > > +/* Correlate canonical spec values to autogenerated QAPI values */
> > > +struct {
> > > +    uint32_t mask;
> > mask is unused in this patch
> >
> > > +    int qapi_value;
> > > +} BMEFlagMap[BME_FLAG_BIT__MAX] = {
> > > +    [BME_FLAG_BIT_IN_USE] = { BME_FLAG_IN_USE,
> > > BITMAP_FLAG_ENUM_IN_USE },
> > > +    [BME_FLAG_BIT_AUTO]   = { BME_FLAG_AUTO,   BITMAP_FLAG_ENUM_AUTO },
> > > +    [BME_FLAG_BIT_EXTRA]  = { BME_FLAG_EXTRA,
> > > BITMAP_FLAG_ENUM_EXTRA_DATA_COMPATIBLE },
> > > +};
> > >      /* bits [1, 8] U [56, 63] are reserved */
> > >    #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001feULL
> > > @@ -663,6 +683,63 @@ static void del_bitmap_list(BlockDriverState *bs)
> > >        }
> > >    }
> > >    +static BitmapFlagEnumList *get_bitmap_flags(uint32_t flags)
> > > +{
> > > +    int i;
> > > +    BitmapFlagEnumList *flist = NULL;
> > > +    BitmapFlagEnumList *ftmp;
> > > +
> > > +    while (flags) {
> > > +        i = ctz32(flags);
> > > +        ftmp = g_new0(BitmapFlagEnumList, 1);
> > > +        if (i >= BME_FLAG_BIT__BEGIN && i < BME_FLAG_BIT__MAX) {
> > > +            ftmp->value = BMEFlagMap[i].qapi_value;
> > > +        } else {
> > > +            ftmp->value = BITMAP_FLAG_ENUM_UNKNOWN;
> > so, there may be several "unknown" entries. It's inconsistent with
> > "@unknown: This bitmap has unknown or reserved properties.".
> >
> > Finally, can we export values for unknown flags? It should be more
> > informative.
> I changed my mind -- qemu-img is not a debugging tool and shouldn't get
> lost in the weeds trying to enumerate the different kinds of reserved
> values that are set.
>
> It's an error to have them set, or not -- which ones specifically is not
> information that an end-user need know or care about, and I don't want
> to create an extra structure to contain the value.
>
> I'm going to rewrite this loop to just only ever have one unknown value
> at a maximum.
>
> --js

Ok, I agree.

--
Best regards,
Vladimir