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