On 11/22/14 07:03, Eric Blake wrote: > Based on Dan's recommendation [1], I'm looking at enhancing > virDomainListBlockStats to report the allocation numbers of a backing > file during a virDomainBlockCommit operation. Getting the information > from qemu is not difficult, but the question is how should it be > represented to the end user. See below for my ideas, and I'm open to > feedback. > > [1] https://www.redhat.com/archives/libvir-list/2014-November/msg00604.html > > Some background - highest allocation is mainly applicable to using a > qcow2 format on top of a raw block device (that's the only time that > libvirt reports a different number based on querying qemu, other file > formats just go off of stat information), so even setting this up to > test can be interesting. Also, while qemu reports wr_highest_offset > during 'query-blockstats', a disk has to actually have write activity > performed during that given qemu process before the offset will be > accurate. It took me a while to figure this out; when I set up a dummy > guest with no OS (and therefore no writes), the offset being reported > was 0 even when I had used qemu-io to poke data into the file prior to > starting qemu. I finally figured out that metadata writes also count as > part of the highest offset visited; so using > 'blockdev-snapshot-internal-sync' followed by > 'blockdev-snapshot-delete-internal-sync' is sufficient to cause qemu to > write metadata and therefore reveal the highest offset. > ... > Next, I can play with blockcommit, where a qemu-monitor-command on > 'query-blockstats' will show me the growing allocation when the backing > file is being written during the commit. The other useful output is the > new virDomainListBlockStats: > # virsh domstats --block testvm2Domain: 'testvm2' > block.count=2 > block.0.name=vda > block.0.rd.reqs=1 > block.0.rd.bytes=512 > block.0.rd.times=31635 > 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=458752 > block.0.capacity=786432000 > block.1.name=vdb > block.1.rd.reqs=0 > block.1.rd.bytes=0 > block.1.rd.times=0 > block.1.wr.reqs=0 > block.1.wr.bytes=0 > block.1.wr.times=0 > block.1.fl.reqs=0 > block.1.fl.times=0 > block.1.allocation=0 > block.1.capacity=1310720000 > > The problem is that once we have a domain with more than one <disk>, and > where one or all disks have more than one <backingStore>, then how > should virDomainListBlockStats represent that? > > One idea I have is to just expose a block.count equal to the total > number of devices I'm about to report on, where the array can be larger > than the number of disks, and using the name field to correlate back to > dumpxml layout: I'm voting for this option as it's more scalable. When we combine it with the introduction of the new flag users won't be confused. > > block.count=3 > block.0.name=vda # information on wrapper.qcow2 > ... > block.1.name=vda[1] # information on backingStore index 1 of vda > block.1.rd.reqs=0 #+ that is, on /dev/sda6 > ... > block.2.name=vdb # information on /dev/sda7 > ... > > It may make things easier if I also add a block.n.path that lists the > file name of the block being described (might get tricky with > NBD/gluster/sheepdog network disks). Well, yes, that might be a helping pointer although we need to make sure that users won't rely on that. Also a ultimate fix for this is to start using node names in qemu for backing chain elements. This will allow us to make the indexes stick to the image they were assigned to and thus remove the possible ambiguity when a block operation that modifies the backing chain finished. > > Also, I'm thinking of adding block.n.physical to match the older > virDomainGetBlockInfo() information. > > Another possible layout is to mirror the nesting of XML backingChain. > Something like: > > block.count=2 > block.0.name=vda > block.0.backing=1 > block.0.allocation=... # information on /tmp/wrapper.qcow2 > ... > block.0.0.allocation=... # information on /dev/sda6 > ... > block.1.name=vdb > block.1.backing=0 > block.1.allocation=... # information on /dev/sda7 > > But there, we run into a possible problem: virTypedParameter has a > finite length for field names, so we can only cover a finite depth of > backing chain before we run out of space and can't report on the full > chain. Any other ideas for the best way to lay this out, and for how to > make it as easy as possible for client applications to correlate > information on allocation back to the appropriate block device in the chain?\ Yep, this will make the entry name lengths non-deterministic and most certainly will end up in problems > > I also wonder if adding more information to the existing --block flag > for stats is okay, or whether it is better to add yet another statistics > flag grouping that must be requested to turn on information about > backing files. Technically, it's still block-related statistics, but as > we have already got released libvirt that still has a 1:1 block.n > mapping to <disk> elements, using a new flag would make it easier to > learn if libvirt is new enough to support information on backing chains, > all without confusing existing clients that aren't expecting backing > chain stats. This question needs answering regardless of which layout > we choose above for representing backing chain stats. I think we should add a new flag. The new flag may automatically imply the use of the old one. If we'd add more block entries (option 1 as I'm voting for it) it may confuse users of the old output. Having them explicitly enable the block info is a way to override that problem. Also most of the time (at time no block operation is running) the information won't be relevant and may possibly be expensive to collect. > > Thoughts welcome. Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list