[libvirt] [PATCH 00/23] Add block write threshold event
Eric Blake
eblake at redhat.com
Fri Mar 24 17:45:27 UTC 2017
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170324/c22913f7/attachment-0001.sig>
More information about the libvir-list
mailing list