On 03/24/2017 09:55 AM, Peter Krempa wrote: > On Wed, Mar 15, 2017 at 17:37:12 +0100, Peter Krempa wrote: >> This is another version of the stuff that I've posted here: >> https://www.redhat.com/archives/libvir-list/2017-February/msg01391.html >> which was partially based on the very old discussion at >> https://www.redhat.com/archives/libvir-list/2015-May/msg00580.html >> >> This version fixes some of the review feedback that I've got and also fixes all >> the issues pointed out in the original cover letter since I managed to implement >> the node name detection in a way suitable for this. >> >> 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: > diff --git w/src/libvirt-domain.c c/src/libvirt-domain.c > index 815cbb1..ca63138 100644 > --- w/src/libvirt-domain.c > +++ c/src/libvirt-domain.c > @@ -11838,8 +11838,12 @@ virDomainSetVcpu(virDomainPtr domain, > * Set the threshold level for delivering the > * VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD if the device or backing chain element > * described by @dev is written beyond the set threshold level. The threshold > - * level is unset once the event fired. The event may not be delivered at all if > - * libvirtd was not running at the moment when the threshold was reached. > + * level is unset once the event fired. The event might not be delivered at all maybe s/fired/fires/ > + * if libvirtd was not running at the moment when the threshold was reached. > + * > + * Hypervisors report the last written sector of an image in the bulk stats API extra space > + * (virConnectGetAllDomainStats/virDomainListGetStats) as > + * "block.<num>.allocation" in the VIR_DOMAIN_STATS_BLOCK group. Maybe also mention (thanks to the followup patch) that block.<num>.threshold can be inspected in bulk stats to learn the current threshold. > * > * This event allows to use thin-provisioned storage which needs management > * tools to grow it without the need for polling of the data. > diff --git w/src/qemu/qemu_domain.c c/src/qemu/qemu_domain.c > index 0469a1f..6985643 100644 > --- w/src/qemu/qemu_domain.c > +++ c/src/qemu/qemu_domain.c > @@ -8521,7 +8521,7 @@ qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, > * @src: filled with the specific backing store element if provided > * @idx: index of @src in the backing chain, if provided > * > - * Looks up the disk in the domain via @nodename and returns it's definition. > + * Looks up the disk in the domain via @nodename and returns its definition. > * Optionally fills @src and @idx if provided with the specific backing chain > * element which corresponds to the node name. > */ > diff --git w/src/qemu/qemu_process.c c/src/qemu/qemu_process.c > index 8477710..53b4c5f 100644 > --- w/src/qemu/qemu_process.c > +++ c/src/qemu/qemu_process.c > @@ -1470,6 +1470,7 @@ qemuProcessHandleBlockThreshold(qemuMonitorPtr mon ATTRIBUTE_UNUSED, > if ((dev = qemuDomainDiskBackingStoreGetName(disk, src, idx))) { > event = virDomainEventBlockThresholdNewFromObj(vm, dev, path, > threshold, excess); > + VIR_FREE(dev); > } Oh nice - you caught something I missed. > } > > diff --git w/src/remote/remote_driver.c c/src/remote/remote_driver.c > index baa5cba..27bcc9b 100644 > --- w/src/remote/remote_driver.c > +++ c/src/remote/remote_driver.c > @@ -8436,7 +8436,7 @@ static virHypervisorDriver hypervisor_driver = { > .domainGetGuestVcpus = remoteDomainGetGuestVcpus, /* 2.0.0 */ > .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */ > .domainSetVcpu = remoteDomainSetVcpu, /* 3.1.0 */ > - .domainSetBlockThreshold = remoteDomainSetBlockThreshold, /* 3.1.0 */ > + .domainSetBlockThreshold = remoteDomainSetBlockThreshold, /* 3.2.0 */ > }; > > static virNetworkDriver network_driver = { > diff --git w/src/util/virbuffer.c c/src/util/virbuffer.c > index 8f0f49d..80c8e28 100644 > --- w/src/util/virbuffer.c > +++ c/src/util/virbuffer.c > @@ -90,7 +90,7 @@ virBufferAdjustIndent(virBufferPtr buf, int indent) > > > /** > - * virBufferAdjustIndent: > + * virBufferSetIndent: > * @buf: the buffer > * @indent: new indentation size. > * > diff --git w/tests/virbuftest.c c/tests/virbuftest.c > index 34160e6..8ec6ce5 100644 > --- w/tests/virbuftest.c > +++ c/tests/virbuftest.c > @@ -405,6 +405,34 @@ testBufEscapeN(const void *opaque) > > > static int > +testBufSetIndent(const void *opaque ATTRIBUTE_UNUSED) > +{ I might have just squashed in a test with the existing tests of AdjustIndent, but what you have is fine. > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + char *actual; > + int ret = -1; > + > + virBufferSetIndent(&buf, 11); > + virBufferAddLit(&buf, "test\n"); > + virBufferSetIndent(&buf, 2); > + virBufferAddLit(&buf, "test2\n"); > + > + if (!(actual = virBufferContentAndReset(&buf))) > + goto cleanup; > + > + if (STRNEQ(actual, " test\n test2\n")) { > + VIR_TEST_DEBUG("testBufSetIndent: expected indent not set\n"); > + goto cleanup; > + } > + > + ret = 0; > + > + cleanup: > + VIR_FREE(actual); > + return ret; > +} > + > + > +static int > mymain(void) > { > int ret = 0; > @@ -422,6 +450,7 @@ mymain(void) > DO_TEST("Auto-indentation", testBufAutoIndent, 0); > DO_TEST("Trim", testBufTrim, 0); > DO_TEST("AddBuffer", testBufAddBuffer, 0); > + DO_TEST("set indent", testBufSetIndent, 0); > > #define DO_TEST_ADD_STR(DATA, EXPECT) \ > do { \ > diff --git w/tools/virsh-domain.c c/tools/virsh-domain.c > index 3662952..74d0a0e 100644 > --- w/tools/virsh-domain.c > +++ c/tools/virsh-domain.c > @@ -7105,7 +7105,7 @@ static const vshCmdInfo info_domblkthreshold[] = { > "device or it's backing chain element") > }, > {.name = "desc", > - .data = N_("set threshold for ") > + .data = N_("set threshold for block-threshold even for a block device") s/even/event/ > }, > {.name = NULL} > }; 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... -- 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
