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