Re: [libvirt] [RFC PATCH] threshold: new API virDomainBlockSetWriteThreshold

On Sat, May 23, 2015 at 21:45:56 -0600, Eric Blake wrote: > qemu 2.3 added a new QMP command block-set-write-threshold, > which allows callers to get an interrupt when a file hits a > write threshold, rather than the current approach of repeatedly > polling for file allocation. This patch prepares the API for > callers to register to receive the event, as well as a way > to query the threshold. > > The event is one-shot in qemu - a guest must re-register a new > threshold after each time it triggers. However, the > virConnectDomainEventRegisterAny() call does not allow > parameterization, so callers must use a pair of APIs - one > to register the callback (one-time call), and another to > repeatedly set the desired threshold (repeated each time a > threshold changes). > > Note that the threshold is registered as a double, but reported > as an unsigned long long. This is intentional, as it allows > the use of a flag for registering a threshold via a percentage, > but once registered, the threshold is normalized according to > the current size of the disk. > > To make the patch series more digestible, this patch > intentionally omits remote support, by using a couple of > placeholders at a point where the compiler forces the addition > of a case within a switch statement. > > Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > --- > > Posting now to get feedback on the proposed API, before the actual > implementation is complete. > > daemon/remote.c | 2 + > include/libvirt/libvirt-domain.h | 48 +++++++++++++++++++ > src/conf/domain_event.c | 4 +- > src/driver-hypervisor.h | 7 +++ > src/libvirt-domain.c | 101 +++++++++++++++++++++++++++++++++++++++ > src/libvirt_public.syms | 1 + > tools/virsh-domain.c | 23 +++++++++ > tools/virsh.pod | 1 + > 8 files changed, 186 insertions(+), 1 deletion(-) > > diff --git a/daemon/remote.c b/daemon/remote.c > index e259a76..51d7de5 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -1102,6 +1102,8 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { > VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), > VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), > VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), > + /* TODO: Implement RPC support for this */ > + VIR_DOMAIN_EVENT_CALLBACK(NULL), > }; > > verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h > index d851225..0c4fd62 100644 > --- a/include/libvirt/libvirt-domain.h > +++ b/include/libvirt/libvirt-domain.h > @@ -515,6 +515,15 @@ typedef virDomainBlockStatsStruct *virDomainBlockStatsPtr; > # define VIR_DOMAIN_BLOCK_STATS_ERRS "errs" > > /** > + * VIR_DOMAIN_BLOCK_STATS_WRITE_THRESHOLD: > + * > + * Macro represents the typed parameter, as an llong, that reports Signed? The rest of the block stats fields is signed for a reason, is there a reason why it's here? > + * the threshold in bytes at which the block device will trigger a > + * VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD, or 0 if no threshold is registered. In case it remains signed, the negative values should be somehow described. > + */ > +# define VIR_DOMAIN_BLOCK_STATS_WRITE_THRESHOLD "write-threshold" > + > +/** > * virDomainInterfaceStats: > * > * Network interface stats for virDomainInterfaceStats. > @@ -1297,6 +1306,16 @@ int virDomainBlockStatsFlags (virDomainPtr dom, > virTypedParameterPtr params, > int *nparams, > unsigned int flags); > + > +typedef enum { > + /* threshold is a percentage rather than byte limit */ > + VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE = (1 << 0), > +} virDomainBlockSetWriteThresholdFlags; > +int virDomainBlockSetWriteThreshold(virDomainPtr dom, > + const char *disk, > + double threshold, Double? Your last proposal used unsigned long long. Using double with byte sizes seems a bit odd to me. The file size won't exceed unsigned long long for a while so I don't see a reason to use double here.