On 03/24/2017 12:45 PM, Eric Blake wrote: >>> The event is useful for mgmt apps using thin-provisioned storage so that they >>> don't have to poll for the disk filling all the time. >> >> I've pushed the updated version after incorporating most of the review >> feedback to: >> >> git fetch git://pipo.sk/pipo/libvirt.git block-threshold-2 >> > > Here's the interdiff; still a couple of things to tweak: > > Once you make the right tweaks in the corresponding patches, I think > that means you've earned an ACK to the series. My next email should be > a summary of my testing... First round of testing: new virsh, old libvirtd: # tools/virsh domblkthreshold mydom error: command 'domblkthreshold' requires <dev> option error: command 'domblkthreshold' requires --threshold option Weird that we aren't consistent in reporting the missing option name (<dev> vs. --threshold); but it appears to be based on the type the option is expecting, and not the fault of your series, so a fix (if any) would be for a followup, and doesn't block your series. # tools/virsh domblkthreshold mydom vdb 1m error: unknown procedure: 386 It might be nicer if we could teach the remote driver (on the client side) to augment unknown procedure errors to additionally include the procedure name it thought it was calling, but again not the fault of your series. # tools/virsh event --list ... metadata-change block-threshold Good - virsh displays the new event to register for. # tools/virsh event --domain mydom --event block-threshold error: internal error: unsupported event ID 24 Urggh - what's that "internal error:" doing in there? We should fix that. But again, I don't think it's the fault of your series, and can be done as a followup. Next round of testing: new virsh, new libvirtd, Fedora 25 fedora-virt-preview's qemu-kvm-2.9.0-0.1.rc1.fc25.x86_64: First, I validated that my domain is set up to spot watermark growth (it requires a qcow2 image), merely booting the guest is enough to see that writes start happening to the guest's view of the image, all as part of mounting the disk prior to even showing the login screen (I specifically am testing with vdb, with the guest OS installed in vda, so that the file system on vdb is less heavily used by the OS and therefore a bit more predictable in behavior as I reboot the guest): # tools/virsh start mydom --paused Domain mydom started # tools/virsh domstats mydom --block Domain: 'mydom' block.count=2 ... block.1.allocation=0 block.1.capacity=21474836480 block.1.physical=21480095744 # tools/virsh resume mydom Domain mydom resumed # sleep 10 # tools/virsh domstats mydom --block Domain: 'mydom' block.count=2 ... block.1.allocation=10878128640 block.1.capacity=21474836480 block.1.physical=21480095744 # tools/virsh shutdown mydom Domain mydom is being shutdown block.1.allocation does indeed grow (and I repeated this test a couple of times to validate that my particular guest OS has the same allocation upon reaching the login screen), so the next step is to see if I can register a threshold. I'm going to pick something near where the OS probes (note that I'm starting the domain paused so that I'm guaranteed my threshold won't fire until the guest is resumed): # tools/virsh domblkthreshold mydom vdb 10GB error: Operation not supported: this qemu does not support setting device threshold That's a bit misleading. In this case, we don't support setting device threshold because the domain is not running (there is no qemu process at all when it is shutdown). But at least you correctly failed to set a threshold in this state. # tools/virsh start mydom --paused Domain mydom started # tools/virsh domblkthreshold mydom vdb 9GB # tools/virsh domblkthreshold mydom vdb 10GB Nothing at all printed. But that's okay (I don't think we have to always be chatty on success); and at least we can register a new value even if an old value is already in place and has not yet fired. # tools/virsh domstats mydom --block Domain: 'mydom' block.count=2 ... block.1.allocation=0 block.1.capacity=21474836480 block.1.physical=21480095744 block.1.threshold.storage=10000000000 Yay - the new block.1.threshold.storage stat matches my most-recent registration! (Remember, this is just testing the current state of your series; if someone repeats these steps for validation later on, and you choose to tweak it to a shorter name for the final commit, then be prepared for a slight difference in output) # tools/virsh event --domain mydom --event block-threshold & [1] 11351 # tools/virsh resume mydom Domain mydom resumed event 'block-threshold' for domain mydom: dev: vdb(/path/to/vdb.qcow2) 10000000000 880225792 events received: 1 [1]+ Done tools/virsh event --domain mydom --event block-threshold # tools/virsh domstats mydom --block Domain: 'mydom' block.count=2 ... block.1.allocation=10880225792 block.1.capacity=21474836480 block.1.physical=21480095744 Would you look at that: block.1.threshold.storage disappeared now that the event fired; and the new block.1.allocation matches the sum of the threshold and the excess as reported in the event (meaning my OS did a single far-flung write beyond the 10GB mark to cause the event). I think we can declare success that the watermark event worked! So overall, it looks like you wired things up nicely, and I wasn't able to cause any major problems in my testing, even if we can improve some of the error message. Looks good to go, and you should be able to get it in before DV freezes in preparation for the release. -- 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
