| Message ID | 20200625152215.941773-26-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Deal with filters | expand |
On 25.06.2020 18:21, Max Reitz wrote: > If every BlockDriver were to implement bdrv_get_allocated_file_size(), > there are basically three ways it would be handled: > (1) For protocol drivers: Figure out the actual allocated file size in > some protocol-specific way > (2) For protocol drivers: If that is not possible (or we just have not > bothered to implement it yet), return -ENOTSUP > (3) For drivers with children: Return the sum of some or all their > children's sizes > > For the drivers we have, case (3) boils down to either: > (a) The sum of all children's sizes > (b) The size of the primary child > > (2), (3a) and (3b) can be implemented generically, so this patch adds > such generic implementations for drivers to use. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > include/block/block_int.h | 5 ++++ > block.c | 51 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 5da793bfc3..c963ee9f28 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -1318,6 +1318,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, > int64_t *pnum, > int64_t *map, > BlockDriverState **file); > + > +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs); > +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs); > +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs); > + > const char *bdrv_get_parent_name(const BlockDriverState *bs); > void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); > bool blk_dev_has_removable_media(BlockBackend *blk); > diff --git a/block.c b/block.c > index 1c71ecab7c..fc01ce90b3 100644 > --- a/block.c > +++ b/block.c > @@ -5003,6 +5003,57 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) > return -ENOTSUP; > } > > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * block drivers that want it to sum all children they store data on. > + * (This excludes backing children.) > + */ > +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) > +{ > + BdrvChild *child; > + int64_t child_size, sum = 0; > + > + QLIST_FOREACH(child, &bs->children, next) { > + if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA | > + BDRV_CHILD_FILTERED)) > + { > + child_size = bdrv_get_allocated_file_size(child->bs); > + if (child_size < 0) { > + return child_size; > + } > + sum += child_size; > + } > + } > + > + return sum; > +} > + > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * block drivers that want it to return only the size of a node's > + * primary child. > + */ > +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs) > +{ > + BlockDriverState *primary_bs; > + > + primary_bs = bdrv_primary_bs(bs); > + if (!primary_bs) { > + return -ENOTSUP; > + } > + > + return bdrv_get_allocated_file_size(primary_bs); > +} > + > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * protocol block drivers that just do not support it. > + */ > +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs) > +{ > + return -ENOTSUP; > +} > + > /* > * bdrv_measure: > * @drv: Format driver Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Am 25.06.2020 um 17:21 hat Max Reitz geschrieben: > If every BlockDriver were to implement bdrv_get_allocated_file_size(), > there are basically three ways it would be handled: > (1) For protocol drivers: Figure out the actual allocated file size in > some protocol-specific way > (2) For protocol drivers: If that is not possible (or we just have not > bothered to implement it yet), return -ENOTSUP > (3) For drivers with children: Return the sum of some or all their > children's sizes > > For the drivers we have, case (3) boils down to either: > (a) The sum of all children's sizes > (b) The size of the primary child > > (2), (3a) and (3b) can be implemented generically, so this patch adds > such generic implementations for drivers to use. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > include/block/block_int.h | 5 ++++ > block.c | 51 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 5da793bfc3..c963ee9f28 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -1318,6 +1318,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, > int64_t *pnum, > int64_t *map, > BlockDriverState **file); > + > +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs); > +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs); > +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs); > + > const char *bdrv_get_parent_name(const BlockDriverState *bs); > void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); > bool blk_dev_has_removable_media(BlockBackend *blk); > diff --git a/block.c b/block.c > index 1c71ecab7c..fc01ce90b3 100644 > --- a/block.c > +++ b/block.c > @@ -5003,6 +5003,57 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) > return -ENOTSUP; > } > > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * block drivers that want it to sum all children they store data on. > + * (This excludes backing children.) > + */ > +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) > +{ > + BdrvChild *child; > + int64_t child_size, sum = 0; > + > + QLIST_FOREACH(child, &bs->children, next) { > + if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA | > + BDRV_CHILD_FILTERED)) > + { > + child_size = bdrv_get_allocated_file_size(child->bs); > + if (child_size < 0) { > + return child_size; > + } > + sum += child_size; > + } > + } > + > + return sum; > +} The only user apart from bdrv_get_allocated_file_size() is blkverify. As I argued that blkverify shouldn't use it, this can become static. > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * block drivers that want it to return only the size of a node's > + * primary child. > + */ > +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs) > +{ > + BlockDriverState *primary_bs; > + > + primary_bs = bdrv_primary_bs(bs); > + if (!primary_bs) { > + return -ENOTSUP; > + } > + > + return bdrv_get_allocated_file_size(primary_bs); > +} This can become static, too (never used as a callback), and possibly even be inlined. > +/** > + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for > + * protocol block drivers that just do not support it. > + */ > +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs) > +{ > + return -ENOTSUP; > +} Also never used as a callback. I think inlining it would almost certainly make more sense. Kevin
On 19.08.20 12:57, Kevin Wolf wrote: > Am 25.06.2020 um 17:21 hat Max Reitz geschrieben: >> If every BlockDriver were to implement bdrv_get_allocated_file_size(), >> there are basically three ways it would be handled: >> (1) For protocol drivers: Figure out the actual allocated file size in >> some protocol-specific way >> (2) For protocol drivers: If that is not possible (or we just have not >> bothered to implement it yet), return -ENOTSUP >> (3) For drivers with children: Return the sum of some or all their >> children's sizes >> >> For the drivers we have, case (3) boils down to either: >> (a) The sum of all children's sizes >> (b) The size of the primary child >> >> (2), (3a) and (3b) can be implemented generically, so this patch adds >> such generic implementations for drivers to use. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> include/block/block_int.h | 5 ++++ >> block.c | 51 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 56 insertions(+) >> >> diff --git a/include/block/block_int.h b/include/block/block_int.h >> index 5da793bfc3..c963ee9f28 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -1318,6 +1318,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, >> int64_t *pnum, >> int64_t *map, >> BlockDriverState **file); >> + >> +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs); >> +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs); >> +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs); >> + >> const char *bdrv_get_parent_name(const BlockDriverState *bs); >> void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); >> bool blk_dev_has_removable_media(BlockBackend *blk); >> diff --git a/block.c b/block.c >> index 1c71ecab7c..fc01ce90b3 100644 >> --- a/block.c >> +++ b/block.c >> @@ -5003,6 +5003,57 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) >> return -ENOTSUP; >> } >> >> +/** >> + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for >> + * block drivers that want it to sum all children they store data on. >> + * (This excludes backing children.) >> + */ >> +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) >> +{ >> + BdrvChild *child; >> + int64_t child_size, sum = 0; >> + >> + QLIST_FOREACH(child, &bs->children, next) { >> + if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA | >> + BDRV_CHILD_FILTERED)) >> + { >> + child_size = bdrv_get_allocated_file_size(child->bs); >> + if (child_size < 0) { >> + return child_size; >> + } >> + sum += child_size; >> + } >> + } >> + >> + return sum; >> +} > > The only user apart from bdrv_get_allocated_file_size() is blkverify. As > I argued that blkverify shouldn't use it, this can become static. > >> +/** >> + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for >> + * block drivers that want it to return only the size of a node's >> + * primary child. >> + */ >> +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs) >> +{ >> + BlockDriverState *primary_bs; >> + >> + primary_bs = bdrv_primary_bs(bs); >> + if (!primary_bs) { >> + return -ENOTSUP; >> + } >> + >> + return bdrv_get_allocated_file_size(primary_bs); >> +} > > This can become static, too (never used as a callback), and possibly > even be inlined. > >> +/** >> + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for >> + * protocol block drivers that just do not support it. >> + */ >> +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs) >> +{ >> + return -ENOTSUP; >> +} > > Also never used as a callback. I think inlining it would almost > certainly make more sense. I think they’re all artifacts from the development process, yeah. Originally, I wanted to make .bdrv_get_allocated_file_size() mandatory, but then I saw that led nowhere and could be done well generically. Max
diff --git a/include/block/block_int.h b/include/block/block_int.h index 5da793bfc3..c963ee9f28 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1318,6 +1318,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, int64_t *pnum, int64_t *map, BlockDriverState **file); + +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs); +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs); +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs); + const char *bdrv_get_parent_name(const BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); bool blk_dev_has_removable_media(BlockBackend *blk); diff --git a/block.c b/block.c index 1c71ecab7c..fc01ce90b3 100644 --- a/block.c +++ b/block.c @@ -5003,6 +5003,57 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) return -ENOTSUP; } +/** + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for + * block drivers that want it to sum all children they store data on. + * (This excludes backing children.) + */ +int64_t bdrv_sum_allocated_file_size(BlockDriverState *bs) +{ + BdrvChild *child; + int64_t child_size, sum = 0; + + QLIST_FOREACH(child, &bs->children, next) { + if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA | + BDRV_CHILD_FILTERED)) + { + child_size = bdrv_get_allocated_file_size(child->bs); + if (child_size < 0) { + return child_size; + } + sum += child_size; + } + } + + return sum; +} + +/** + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for + * block drivers that want it to return only the size of a node's + * primary child. + */ +int64_t bdrv_primary_allocated_file_size(BlockDriverState *bs) +{ + BlockDriverState *primary_bs; + + primary_bs = bdrv_primary_bs(bs); + if (!primary_bs) { + return -ENOTSUP; + } + + return bdrv_get_allocated_file_size(primary_bs); +} + +/** + * Implementation of BlockDriver.bdrv_get_allocated_file_size() for + * protocol block drivers that just do not support it. + */ +int64_t bdrv_notsup_allocated_file_size(BlockDriverState *bs) +{ + return -ENOTSUP; +} + /* * bdrv_measure: * @drv: Format driver
If every BlockDriver were to implement bdrv_get_allocated_file_size(), there are basically three ways it would be handled: (1) For protocol drivers: Figure out the actual allocated file size in some protocol-specific way (2) For protocol drivers: If that is not possible (or we just have not bothered to implement it yet), return -ENOTSUP (3) For drivers with children: Return the sum of some or all their children's sizes For the drivers we have, case (3) boils down to either: (a) The sum of all children's sizes (b) The size of the primary child (2), (3a) and (3b) can be implemented generically, so this patch adds such generic implementations for drivers to use. Signed-off-by: Max Reitz <mreitz@redhat.com> --- include/block/block_int.h | 5 ++++ block.c | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)