Re: [Qemu-devel] [RFC V8 02/13] quorum: Create BDRVQuorumState and BlkDriver and do init.

[Kevin Wolf]

Am 28.01.2013 18:07, schrieb Benoît Canet:
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  block/quorum.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index 8dc6e4c..d8fffbe 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -15,6 +15,13 @@
>  
>  #include "block/block_int.h"

This needs more documentation as well:

> +typedef struct {
> +    BlockDriverState **bs;
> +    unsigned long long threshold;

What kind of threshold is this?

> +    unsigned long long total;

Is this the array length for .bs and .filenames? Looking at the type,
probably not, so what is it - and where is the array size stored?

Why are both long long? It's not a type that appears a lot in the qemu
code. Should it be replaced by uint64_t?

> +    char **filenames;

What do you need the filenames for? If at all possible we should avoid
dealing with file names outside of the initialisation. Once we got all
the -blockdev infrastructure we're dreaming of, you wouldn't even be
able to get file names any more, just BlockDriverStates.

> +} BDRVQuorumState;
> +
>  typedef struct QuorumAIOCB QuorumAIOCB;
>  
>  typedef struct QuorumSingleAIOCB {

Kevin