Re: [PATCH 1/3] block/block-backend: add converter from BlockAcctStats to BlockBackend

2020-09-17 Thread Max Reitz
On 10.08.20 12:14, Denis V. Lunev wrote:
> Right now BlockAcctStats is always reside on BlockBackend. This structure
> is not used in any other place. Thus we are able to create a converter
> from one pointer to another.
> 
> Signed-off-by: Denis V. Lunev 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> CC: Stefan Hajnoczi 
> CC: Kevin Wolf 
> CC: Max Reitz 
> ---
>  block/block-backend.c  | 5 +
>  include/sysemu/block-backend.h | 1 +
>  2 files changed, 6 insertions(+)

(Note: I’m just writing this mail because I already responded to patch
2.  I wouldn’t have if I didn’t have anything to say regarding the other
patches in this series, so nothing I say here is backed by a strong
opinion from me.)

> diff --git a/block/block-backend.c b/block/block-backend.c
> index 3a13cb5f0b..88e531df98 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -2153,6 +2153,11 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk)
>  return >stats;
>  }
>  
> +BlockBackend *blk_stats2blk(BlockAcctStats *s)

(Extreme bikeshedding: I’d prefer something like blk_from_stats().)

> +{
> +return container_of(s, BlockBackend, stats);
> +}

Works, but I get all itchy, especially given the reasoning in the commit
message, which is basically “Right now this works”.

That sounds to me like maybe in the future someone could get the idea of
moving BlockAcctStats somewhere else and then this is something that we
absolutely must not forget about, lest horrible accidents occur.

Would a back pointer from BlockAcctStats to the BlockBackend work or do
you find that just too ugly and unnecessary?  (I think it would help at
least so that we do not forget this place here.)

Or maybe just a comment above BlockAcctStats would help quench my itch,
too, like “Note: blk_stats2blk() expects objects of this type only to
occur as part of a BlockBackend”.

Max



signature.asc
Description: OpenPGP digital signature


[PATCH 1/3] block/block-backend: add converter from BlockAcctStats to BlockBackend

2020-08-10 Thread Denis V. Lunev
Right now BlockAcctStats is always reside on BlockBackend. This structure
is not used in any other place. Thus we are able to create a converter
from one pointer to another.

Signed-off-by: Denis V. Lunev 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
CC: Stefan Hajnoczi 
CC: Kevin Wolf 
CC: Max Reitz 
---
 block/block-backend.c  | 5 +
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 3a13cb5f0b..88e531df98 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2153,6 +2153,11 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk)
 return >stats;
 }
 
+BlockBackend *blk_stats2blk(BlockAcctStats *s)
+{
+return container_of(s, BlockBackend, stats);
+}
+
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque)
 {
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8203d7f6f9..bd4694e7bc 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -227,6 +227,7 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier 
*notify);
 void blk_io_plug(BlockBackend *blk);
 void blk_io_unplug(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
+BlockBackend *blk_stats2blk(BlockAcctStats *stats);
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
 bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk);
-- 
2.17.1




[PATCH 1/3] block/block-backend: add converter from BlockAcctStats to BlockBackend

2020-08-05 Thread Denis V. Lunev
Right now BlockAcctStats is always reside on BlockBackend. This structure
is not used in any other place. Thus we are able to create a converter
from one pointer to another.

Signed-off-by: Denis V. Lunev 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
CC: Stefan Hajnoczi 
CC: Kevin Wolf 
CC: Max Reitz 
---
 block/block-backend.c  | 5 +
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 3a13cb5f0b..88e531df98 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2153,6 +2153,11 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk)
 return >stats;
 }
 
+BlockBackend *blk_stats2blk(BlockAcctStats *s)
+{
+return container_of(s, BlockBackend, stats);
+}
+
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque)
 {
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8203d7f6f9..bd4694e7bc 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -227,6 +227,7 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier 
*notify);
 void blk_io_plug(BlockBackend *blk);
 void blk_io_unplug(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
+BlockBackend *blk_stats2blk(BlockAcctStats *stats);
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
 bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk);
-- 
2.17.1