Re: [Qemu-devel] [PATCH v8 3/9] block: add empty account cookie type

[Anton Nefedov]

On 16/5/2019 6:34 PM, Vladimir Sementsov-Ogievskiy wrote:
> 16.05.2019 17:33, Anton Nefedov wrote:
>> This adds some protection from accounting uninitialized cookie.
>> That is, block_acct_failed/done without previous block_acct_start;
>> in that case, cookie probably holds values from previous operation.
>>
>> (Note: it might also be uninitialized holding garbage value and there is
>>    still "< BLOCK_MAX_IOTYPE" assertion for that.
>>    So block_acct_failed/done without previous block_acct_start should be used
>>    with caution.)
>>
>> Currently this is particularly useful in ide code where it's hard to
>> keep track whether the request started accounting or not. For example,
>> trim requests do the accounting separately.
>>
>> Signed-off-by: Anton Nefedov <address@hidden>
>> ---
>>    include/block/accounting.h | 1 +
>>    block/accounting.c         | 6 ++++++
>>    2 files changed, 7 insertions(+)
>>
>> diff --git a/include/block/accounting.h b/include/block/accounting.h
>> index ba8b04d572..878b4c3581 100644
>> --- a/include/block/accounting.h
>> +++ b/include/block/accounting.h
>> @@ -33,6 +33,7 @@ typedef struct BlockAcctTimedStats BlockAcctTimedStats;
>>    typedef struct BlockAcctStats BlockAcctStats;
>>    
>>    enum BlockAcctType {
>> +    BLOCK_ACCT_NONE = 0,
>>        BLOCK_ACCT_READ,
>>        BLOCK_ACCT_WRITE,
>>        BLOCK_ACCT_FLUSH,
>> diff --git a/block/accounting.c b/block/accounting.c
>> index 70a3d9a426..8d41c8a83a 100644
>> --- a/block/accounting.c
>> +++ b/block/accounting.c
>> @@ -195,6 +195,10 @@ static void block_account_one_io(BlockAcctStats *stats, 
>> BlockAcctCookie *cookie,
>>    
>>        assert(cookie->type < BLOCK_MAX_IOTYPE);
>>    
>> +    if (cookie->type == BLOCK_ACCT_NONE) {
> 
> worth error_report() ?
> 

I don't think we should necessarily consider it an error;
as mentioned in the commit message this might be useful in some places
like IDE trim handling.