[PATCH 0/3] Threshold event improvements

Peter Krempa (3): storage_source: Add flag storing whether threshold event was registered with index qemu: Prevent two threshold events when it was registered with index virDomainSetBlockThreshold: Document that two events are fired when index isn't used src/conf/storage_source_conf.h | 5 +++++ src/libvirt-domain.c | 4 ++++ src/qemu/qemu_domain.c | 8 ++++++++ src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_process.c | 3 ++- tests/qemustatusxml2xmldata/modern-in.xml | 1 + 6 files changed, 24 insertions(+), 1 deletion(-) -- 2.31.1

When users register the threshold event for the top level image with an explicit index (e.g. vda[3]) they are clearly expecting the index in the event. This flag will help avoiding emission of the second event without the index when the client clearly requested one with the index. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_source_conf.h | 5 +++++ src/qemu/qemu_domain.c | 8 ++++++++ tests/qemustatusxml2xmldata/modern-in.xml | 1 + 3 files changed, 14 insertions(+) diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 389c7b56d1..40db29c418 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -387,6 +387,11 @@ struct _virStorageSource { char *nfs_group; uid_t nfs_uid; gid_t nfs_gid; + + /* We need a flag to remember that the threshold event for this source was + * registered with a full index (vda[3]) so that we can properly report just + * one event for it */ + bool thresholdEventWithIndex; }; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..efff6eff86 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1951,6 +1951,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, g_autofree char *encalias = NULL; g_autofree char *httpcookiealias = NULL; g_autofree char *tlskeyalias = NULL; + g_autofree char *thresholdEventWithIndex = NULL; src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); @@ -1990,6 +1991,10 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) return -1; + if ((thresholdEventWithIndex = virXPathString("string(./thresholdEvent/@indexUsed)", ctxt)) && + virTristateBoolTypeFromString(thresholdEventWithIndex) == VIR_TRISTATE_BOOL_YES) + src->thresholdEventWithIndex = true; + return 0; } @@ -2044,6 +2049,9 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src, virXMLFormatElement(buf, "objects", NULL, &tmp); + if (src->thresholdEventWithIndex) + virBufferAddLit(buf, "<thresholdEvent indexUsed='yes'/>\n"); + return 0; } diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index ba0858a4ff..cc5fd1cb74 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -340,6 +340,7 @@ <secret type='tlskey' alias='tls-certificate-key-alias'/> <TLSx509 alias='transport-alias'/> </objects> + <thresholdEvent indexUsed='yes'/> </privateData> </source> <backingStore/> -- 2.31.1

Remember whether the user passed an explicit index when registering the event so that we can avoid the top level event when it isn't needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_process.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 235f575901..eeca5074ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19604,6 +19604,10 @@ qemuDomainSetBlockThreshold(virDomainPtr dom, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) goto endjob; + /* we need to remember whether the threshold was registered with an explicit + * index to fire the correct event */ + src->thresholdEventWithIndex = !!strchr(dev, '['); + ret = 0; endjob: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2b03b0ab98..f8916282bf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1487,7 +1487,8 @@ qemuProcessHandleBlockThreshold(qemuMonitor *mon G_GNUC_UNUSED, if (virStorageSourceIsLocalStorage(src)) path = src->path; - if (src == disk->src) { + if (src == disk->src && + !src->thresholdEventWithIndex) { g_autofree char *dev = qemuDomainDiskBackingStoreGetName(disk, 0); eventDevice = virDomainEventBlockThresholdNewFromObj(vm, dev, path, -- 2.31.1

Libvirt started emitting two threshold events, once with index and once withouth when the index isn't registered. Document this caveat. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt-domain.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 750e32f0ca..0a7d2dd234 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12637,6 +12637,10 @@ int virDomainGetGuestInfo(virDomainPtr domain, * live VM XML for 'backingStore' or 'source' elements of a disk. If index is * given the threshold is set for the corresponding image. * + * In case when @dev does not contain index the + * VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD event may be emitted twice, once for the + * disk device target without index and once containing the index. + * * Note that the threshold event can be registered also for destinations of a * 'virDomainBlockCopy' destination by using the 'index' of the 'mirror' source. * -- 2.31.1

On 7/1/21 4:03 PM, Peter Krempa wrote:
Peter Krempa (3): storage_source: Add flag storing whether threshold event was registered with index qemu: Prevent two threshold events when it was registered with index virDomainSetBlockThreshold: Document that two events are fired when index isn't used
src/conf/storage_source_conf.h | 5 +++++ src/libvirt-domain.c | 4 ++++ src/qemu/qemu_domain.c | 8 ++++++++ src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_process.c | 3 ++- tests/qemustatusxml2xmldata/modern-in.xml | 1 + 6 files changed, 24 insertions(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník
-
Peter Krempa