On 12/16/2014 08:45 AM, Peter Krempa wrote:
> On 12/16/14 09:04, Eric Blake wrote:
>> Wire up backing chain recursion. Note that for now, we just use
>> the same allocation numbers for read-only backing files as what
>> offline domains would report. It is not the correct allocation
>> number for qcow2 over block devices during block-commit, and it
Stale comments; I'll update them.
>> misses out on the fact that qemu also reports read statistics on
>> backing files that are worth knowing about (seriously - for a
>> thin-provisioned setup, it would be nice to easily get at a count
>> of how many reads were serviced from the backing file in relation
>> to reads serviced by the active layer). But it is at least
>> sufficient to prove that the algorithm is working, and to let
>> other people start coding to the interface while waiting for
>> later patches that get the correct information.
>>
>> $ virsh domstats --block --backing testvm2
>> Domain: 'testvm2'
>> block.count=3
>> block.0.name=vda
>> block.0.path=/tmp/wrapper.qcow2
>> block.0.rd.reqs=1
>> block.0.rd.bytes=512
>> block.0.rd.times=28858
>> block.0.wr.reqs=0
>> block.0.wr.bytes=0
>> block.0.wr.times=0
>> block.0.fl.reqs=0
>> block.0.fl.times=0
>> block.0.allocation=0
>> block.0.capacity=1310720000
>> block.0.physical=200704
>> block.1.name=vda
>> block.1.path=/dev/sda6
>> block.1.backingIndex=1
>> block.1.allocation=1073741824
>> block.1.capacity=1310720000
>> block.1.physical=1073741824
actually even longer - I get block.1.rd.reqs and so forth as well.
>> - if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
>> - disk, disk->src, i, abbreviated,
>> - stats) < 0)
>> - goto cleanup;
>> + while (src && (!backing_idx || backing)) {
>
> I'd make the special case of the first element a bit more obvious by
> using "backing_idx == 0" rather than the negation sign used usually with
> pointers.
Will fix.
>
>> + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
>> + disk, src, visited, backing_idx,
>> + abbreviated, stats) < 0)
>> + goto cleanup;
>> + visited++;
>> + backing_idx++;
>> + src = src->backingStore;
>> + }
>> }
>>
>> ret = 0;
>
> As an additional thought. The stats are now extermely long for VMs with
> a long backing chain and few of the stats tend to be 0 always. We could
> start skipping them perhaps (at least for the non-top, devices)
That can be a followup patch (I agree with it, but it's too late for me
to fix tonight...)
>
> ACK.
I've pushed 11 and 12 with fixes. Thanks for the reviews.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list