| Message ID | 20180512012537.22478-8-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu-img: add bitmap queries | expand |
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 <jsnow@redhat.com> > --- > 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. > + } > + ftmp->next = flist; > + flist = ftmp; > + flags &= ~(1 << i); > + } > + > + return flist; > +} > + > +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs) > +{ > + Qcow2BitmapList *bm_list; > + Qcow2Bitmap *bm; > + BitmapInfoList *info_list = NULL; > + BitmapInfoList *tmp; > + BitmapInfo *bitmap_info; > + > + bm_list = get_bitmap_list(bs, NULL); > + if (!bm_list) { > + return info_list; > + } > + > + QSIMPLEQ_FOREACH(bm, bm_list, entry) { > + bitmap_info = g_new0(BitmapInfo, 1); > + bitmap_info->name = g_strdup(bm->name); > + if (bm->type == BT_DIRTY_TRACKING_BITMAP) { > + bitmap_info->type = BITMAP_TYPE_ENUM_DIRTY_TRACKING; > + } else { > + bitmap_info->type = BITMAP_TYPE_ENUM_UNKNOWN; > + } > + bitmap_info->granularity = 1 << bm->granularity_bits; > + bitmap_info->count = bdrv_get_dirty_count(bm->dirty_bitmap); > + bitmap_info->flags = get_bitmap_flags(bm->flags); > + bitmap_info->has_flags = !!bitmap_info->flags; > + > + tmp = g_new0(BitmapInfoList, 1); > + tmp->value = bitmap_info; > + tmp->next = info_list; > + info_list = tmp; > + } > + > + return info_list; > +} > + > int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > void **refcount_table, > int64_t *refcount_table_size) > diff --git a/block/qcow2.c b/block/qcow2.c > index 7ae9000656..0b24ecb6b2 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3960,6 +3960,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) > BDRVQcow2State *s = bs->opaque; > ImageInfoSpecific *spec_info; > QCryptoBlockInfo *encrypt_info = NULL; > + BitmapInfoList *bitmap_list = NULL; > > if (s->crypto != NULL) { > encrypt_info = qcrypto_block_get_info(s->crypto, &error_abort); > @@ -4016,6 +4017,12 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) > spec_info->u.qcow2.data->encrypt = qencrypt; > } > > + bitmap_list = qcow2_get_bitmap_info(bs); > + if (bitmap_list) { > + spec_info->u.qcow2.data->has_bitmaps = true; > + spec_info->u.qcow2.data->bitmaps = bitmap_list; > + } > + > return spec_info; > } > > diff --git a/block/qcow2.h b/block/qcow2.h > index 796a8c914b..76bf5fa55d 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -686,5 +686,6 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, > void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > const char *name, > Error **errp); > +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs); > > #endif
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 <jsnow@redhat.com> >> --- >> 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 > It sure is! I'll remove it. It's an artifact from an earlier revision. >> + 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.". > Based on your feedback from the previous patch I'll probably just call this @reserved for any unrecognized flag. > Finally, can we export values for unknown flags? It should be more > informative. > I'll see what I can do -- I hate losing this information too.
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 <jsnow@redhat.com> >> --- >> 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
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 <jsnow@redhat.com> >>> --- >>> 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.
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; + 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; + } + ftmp->next = flist; + flist = ftmp; + flags &= ~(1 << i); + } + + return flist; +} + +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs) +{ + Qcow2BitmapList *bm_list; + Qcow2Bitmap *bm; + BitmapInfoList *info_list = NULL; + BitmapInfoList *tmp; + BitmapInfo *bitmap_info; + + bm_list = get_bitmap_list(bs, NULL); + if (!bm_list) { + return info_list; + } + + QSIMPLEQ_FOREACH(bm, bm_list, entry) { + bitmap_info = g_new0(BitmapInfo, 1); + bitmap_info->name = g_strdup(bm->name); + if (bm->type == BT_DIRTY_TRACKING_BITMAP) { + bitmap_info->type = BITMAP_TYPE_ENUM_DIRTY_TRACKING; + } else { + bitmap_info->type = BITMAP_TYPE_ENUM_UNKNOWN; + } + bitmap_info->granularity = 1 << bm->granularity_bits; + bitmap_info->count = bdrv_get_dirty_count(bm->dirty_bitmap); + bitmap_info->flags = get_bitmap_flags(bm->flags); + bitmap_info->has_flags = !!bitmap_info->flags; + + tmp = g_new0(BitmapInfoList, 1); + tmp->value = bitmap_info; + tmp->next = info_list; + info_list = tmp; + } + + return info_list; +} + int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void **refcount_table, int64_t *refcount_table_size) diff --git a/block/qcow2.c b/block/qcow2.c index 7ae9000656..0b24ecb6b2 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3960,6 +3960,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) BDRVQcow2State *s = bs->opaque; ImageInfoSpecific *spec_info; QCryptoBlockInfo *encrypt_info = NULL; + BitmapInfoList *bitmap_list = NULL; if (s->crypto != NULL) { encrypt_info = qcrypto_block_get_info(s->crypto, &error_abort); @@ -4016,6 +4017,12 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) spec_info->u.qcow2.data->encrypt = qencrypt; } + bitmap_list = qcow2_get_bitmap_info(bs); + if (bitmap_list) { + spec_info->u.qcow2.data->has_bitmaps = true; + spec_info->u.qcow2.data->bitmaps = bitmap_list; + } + return spec_info; } diff --git a/block/qcow2.h b/block/qcow2.h index 796a8c914b..76bf5fa55d 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -686,5 +686,6 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name, Error **errp); +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs); #endif
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 <jsnow@redhat.com> --- block/qcow2-bitmap.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++-- block/qcow2.c | 7 +++++ block/qcow2.h | 1 + 3 files changed, 87 insertions(+), 2 deletions(-)