On Tue 13 Oct 2015 05:42:33 PM CEST, Stefan Hajnoczi <[email protected]> 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
