Re: [Qemu-block] [PATCH v2 07/22] block: Add statistics for failed and invalid I/O operations

[Alberto Garcia]

On Tue 13 Oct 2015 05:42:33 PM CEST, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, Oct 02, 2015 at 05:26:17PM +0300, Alberto Garcia wrote:
>> +void block_acct_failed(BlockAcctStats *stats, BlockAcctCookie *cookie)
>> +{
>> +    int64_t time_ns = qemu_clock_get_ns(clock_type);
>> +
>> +    assert(cookie->type < BLOCK_MAX_IOTYPE);
>> +
>> +    stats->failed_ops[cookie->type]++;
>> +    stats->total_time_ns[cookie->type] += time_ns - cookie->start_time_ns;
>> +    stats->last_access_time_ns = time_ns;
>> +}
>> +
>> +void block_acct_invalid(BlockAcctStats *stats, enum BlockAcctType type)
>> +{
>> +    assert(type < BLOCK_MAX_IOTYPE);
>> +
>> +    stats->invalid_ops[type]++;
>> +    stats->last_access_time_ns = qemu_clock_get_ns(clock_type);
>> +}
>
> block_acct_failed() updates total_time_ns[] but block_acct_invalid()
> does not.  I guess that's because block_acct_invalid() is expected to
> happen during request submission and has effectively 0 duration?

That's right, I put it in the commit message but I guess it's a good
idea to mention it here as well.

I was actually updating total_time_ns[] in block_acct_invalid() as well,
but then when I was adding the block_acct_invalid() calls to the device
models I realized that most/all of them happen before we even call
block_acct_start(). So I decided to leave it like it is now, first
because it makes the code simpler and second because as you say there
would be no I/O to measure.

Berto