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 via virDomainListGetStats(). 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) that will be used each time a threshold triggers for any guest disk, and another to repeatedly set the desired threshold (must be called each time a threshold should be changed). Note that the threshold can either be registered by a byte offset, or by a parts-per-million proportion (a value between 0 and 1000000, scaled to the disk size). But the value is always reported as a byte offset, even when registered as a proportion. I also considered having the setup parameter be a double, to allow a finer resolution rather than fixed-point proportion; but that much resolution is probably not necessary (for a 100G disk, the resulting 100k granularity is pretty much in the noise). 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 label within a switch statement. * include/libvirt/libvirt-domain.h (virDomainBlockSetWriteThreshold): New API. (virConnectDomainEventWriteThresholdCallback): New event. * src/libvirt_public.syms (LIBVIRT_1.3.0): Export it. * src/libvirt-domain.c (virDomainBlockSetWriteThreshold): New API. (virConnectGetAllDomainStats): New stat. * src/driver-hypervisor.h (virDrvDomainBlockSetWriteThreshold): New hypervisor entry point. * tools/virsh-domain.c (vshEventWriteThresholdPrint): Print new event. * tools/virsh.pod (domstats): Document new stat. * daemon/remote.c (domainEventCallbacks): Add stub. * src/conf/domain_event.c (virDomainEventDispatchDefaultFunc): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- daemon/remote.c | 2 + include/libvirt/libvirt-domain.h | 53 ++++++++++++++++++++++ src/conf/domain_event.c | 4 +- src/driver-hypervisor.h | 7 +++ src/libvirt-domain.c | 98 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + tools/virsh-domain.c | 24 ++++++++++ tools/virsh.pod | 1 + 8 files changed, 189 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index e9e2dca..283ece2 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 7564c20..ca2f929 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1306,6 +1306,18 @@ int virDomainBlockStatsFlags (virDomainPtr dom, virTypedParameterPtr params, int *nparams, unsigned int flags); + +typedef enum { + /* threshold is a parts-per-million proportion of the image size + * rather than byte limit */ + VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION = (1 << 0), +} virDomainBlockSetWriteThresholdFlags; + +int virDomainBlockSetWriteThreshold(virDomainPtr dom, + const char *disk, + unsigned long long threshold, + unsigned int flags); + int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, @@ -3255,6 +3267,46 @@ typedef void (*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn, void *opaque); /** + * virConnectDomainEventWriteThresholdCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @devAlias: device alias + * @path: a local path name of the host resource, or NULL if not available + * @threshold: threshold that was exceeded, in bytes + * @length: length beyond @threshold that was involved in the triggering + * write, or 0 if not known + * @opaque: application specified data + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD with virConnectDomainEventRegisterAny() + * + * This callback occurs when a block device detects a write event that + * exceeds a non-zero threshold set by + * virDomainBlockSetWriteThreshold(). When this event occurs, the + * threshold is reset to 0, and a new limit must be installed to see + * the event again on the same device. The intent of this event is to + * allow time for the underlying storage to be resized dynamically + * prior to the point where the guest would be paused due to running + * out of space, without having to poll for allocation values. + * + * The contents of @devAlias will be "vda" when the threshold is + * triggered on the active layer of guest disk vda. Some hypervisors + * also support threshold reporting on backing images, such as during + * a block commit; when that happens, @devAlias will be "vda[1]" for + * the backingStore at index 1 within the chain of host resources for + * guest disk vda. For convenience, if the host resource has a local + * file name, that will be listed in @path (note that @path will be + * NULL for network resources). + */ +typedef void (*virConnectDomainEventWriteThresholdCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + const char *path, + unsigned long long threshold, + unsigned long long length, + void *opaque); + +/** * VIR_DOMAIN_TUNABLE_CPU_VCPUPIN: * * Macro represents formatted pinning for one vcpu specified by id which is @@ -3537,6 +3589,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_TUNABLE = 17, /* virConnectDomainEventTunableCallback */ VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE = 18,/* virConnectDomainEventAgentLifecycleCallback */ VIR_DOMAIN_EVENT_ID_DEVICE_ADDED = 19, /* virConnectDomainEventDeviceAddedCallback */ + VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD = 20, /* virConnectDomainEventWriteThreshold */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 20d66e1..c43799f 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,7 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * - * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2010-2015 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. * @@ -1614,6 +1614,8 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; } + case VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD: + /* TODO: Implement RPC support for this */ case VIR_DOMAIN_EVENT_ID_LAST: break; } diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 3275343..b5b51f1 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -484,6 +484,12 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainBlockSetWriteThreshold)(virDomainPtr domain, + const char *disk, + unsigned long long threshold, + unsigned int flags); + +typedef int (*virDrvDomainInterfaceStats)(virDomainPtr domain, const char *path, virDomainInterfaceStatsPtr stats); @@ -1324,6 +1330,7 @@ struct _virHypervisorDriver { virDrvDomainBlockResize domainBlockResize; virDrvDomainBlockStats domainBlockStats; virDrvDomainBlockStatsFlags domainBlockStatsFlags; + virDrvDomainBlockSetWriteThreshold domainBlockSetWriteThreshold; virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainSetInterfaceParameters domainSetInterfaceParameters; virDrvDomainGetInterfaceParameters domainGetInterfaceParameters; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 4d7b88a..4ac9325 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -5743,6 +5743,102 @@ virDomainBlockStatsFlags(virDomainPtr dom, /** + * virDomainBlockSetWriteThreshold: + * @dom: pointer to domain object + * @disk: path to the block device, or device shorthand + * @threshold: limit at which a write threshold event can trigger + * @flags: bitwise-OR of virDomainBlockSetWriteThresholdFlags + * + * This function is used to set a one-shot write threshold. It should + * be used in tandem with virConnectDomainEventRegisterAny() + * installing a handler for VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD. If + * the hypervisor detects that a write request (whether guest data, or + * host metadata) would exceed the host byte offset specified in + * @threshold, then an event is raised, and the threshold is reset to + * 0 at that time. The event registration is only needed once, but + * this function must be called each time a new threshold is desired; + * the event will only fire if a non-zero threshold is + * exceeded. + * + * By default, @threshold is specified in bytes, and must not exceed + * the size of the block device. However, when @flags includes + * VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION, @threshold is + * instead a value between 0 an 1,000,000, as a parts-per-million + * proportion to the current size of the disk, and the driver will + * compute the corresponding byte value. For example, 500000 + * represents a threshold when half the disk has been allocated. A + * driver may round the requested threshold to a granularity that can + * actually be supported. + * + * Setting a threshold allows event-based resizing of host resources + * that back a guest disk without having to poll the current disk + * allocation, while still having enough time to complete the resize + * before the guest would end up halting due to insufficient space. + * Calling this function to set the threshold back to zero will stop + * further firing of the event. virConnectGetAllDomainStats() can be + * used to track the current threshold value, always in the form + * normalized to bytes. + * + * The @disk parameter is either the device target shorthand (the + * <target dev='...'/> sub-element, such as "vda"), or an unambiguous + * source name of the block device (the <source file='...'/> + * sub-element, such as "/path/to/image"). Valid names can be found + * by calling virDomainGetXMLDesc() and inspecting elements within + * //domain/devices/disk. Some drivers might also accept strings such + * as "vda[1]" for setting the threshold of a backing image, useful + * when doing a block commit into the backing image. Hypervisors may + * restrict threshold reporting to certain types of host resources, + * such as a qcow2 format on top of a block device (as allocation + * tracking differs according to the type of host resource). + * + * Domains may have more than one block device. To set thresholds for + * each you should make multiple calls to this function. If write + * thresholds are not supported, an application will have to instead + * poll virDomainGetBlockInfo() or similar to track allocation. + * + * Returns -1 in case of error, 0 in case of success. + */ +int +virDomainBlockSetWriteThreshold(virDomainPtr dom, + const char *disk, + unsigned long long threshold, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%s, threshold=%llu, flags=%x", + disk, threshold, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckNonNullArgGoto(disk, error); + if (flags & VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PROPORTION && + threshold > 1000000) { + virReportError(VIR_ERR_INVALID_ARG, + _("threshold in %s is larger than 100%%"), + __FUNCTION__); + goto error; + } + conn = dom->conn; + + if (conn->driver->domainBlockSetWriteThreshold) { + int ret; + ret = conn->driver->domainBlockSetWriteThreshold(dom, disk, threshold, + flags); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + + +/** * virDomainInterfaceStats: * @dom: pointer to the domain object * @path: path to the interface @@ -11176,6 +11272,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * "block.<num>.errors" - Xen only: the 'oo_req' value as * unsigned long long. + * "block.<num>.write-threshold" - byte at which a write threshold event + * will fire, as unsigned long long. * "block.<num>.allocation" - offset of the highest written sector * as unsigned long long. * "block.<num>.capacity" - logical size in bytes of the block device backing diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 59d8c12..14b2373 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -717,6 +717,7 @@ LIBVIRT_1.2.16 { LIBVIRT_1.3.0 { global: + virDomainBlockSetWriteThreshold; virTypedParamsAddStringList; } LIBVIRT_1.2.16; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index baf4fa3..141be3a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12082,6 +12082,28 @@ vshEventDeviceAddedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, } static void +vshEventWriteThresholdPrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *alias, + const char *path, + unsigned long long threshold, + unsigned long long length, + void *opaque) +{ + vshDomEventData *data = opaque; + + if (!data->loop && *data->count) + return; + vshPrint(data->ctl, + _("event 'write-threshold' for domain %s disk %s (%s): " + "threshold %llu exceeded by %llu bytes\n"), + virDomainGetName(dom), alias, NULLSTR(path), threshold, length); + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + +static void vshEventTunablePrint(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, virTypedParameterPtr params, @@ -12188,6 +12210,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventAgentLifecyclePrint), }, { "device-added", VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceAddedPrint), }, + { "write-threshold", + VIR_DOMAIN_EVENT_CALLBACK(vshEventWriteThresholdPrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks)); diff --git a/tools/virsh.pod b/tools/virsh.pod index bcfa165..600ea42 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -919,6 +919,7 @@ local file or block device, "block.<num>.fl.reqs" - total flush requests, "block.<num>.fl.times" - total time (ns) spent on cache flushing, "block.<num>.errors" - Xen only: the 'oo_req' value, +"block.<num>.write-threshold" - write threshold event trigger, in bytes, "block.<num>.allocation" - offset of highest written sector in bytes, "block.<num>.capacity" - logical size of source file in bytes, "block.<num>.physical" - physical size of source file in bytes -- 2.4.3

Common code for tracking a write threshold event from creation to delivery through a callback. * src/conf/domain_event.c (_virDomainEventWriteThreshold): New struct. (virDomainEventsOnceInit, virDomainEventWriteThresholdDispose) (virDomainEventWriteThresholdNew) (virDomainEventWriteThresholdNewFromObj) (virDomainEventWriteThresholdNewFromDom) (virDomainEventDispatchDefaultFunc): Wire it up. * src/conf/domain_event.h: Expose new functions. * src/libvirt-private.syms (domain_event.h): Export them. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/domain_event.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_event.h | 16 +++++++- src/libvirt_private.syms | 2 + 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index c43799f..400cfa0 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -56,6 +56,7 @@ static virClassPtr virDomainQemuMonitorEventClass; static virClassPtr virDomainEventTunableClass; static virClassPtr virDomainEventAgentLifecycleClass; static virClassPtr virDomainEventDeviceAddedClass; +static virClassPtr virDomainEventWriteThresholdClass; static void virDomainEventDispose(void *obj); @@ -74,6 +75,7 @@ static void virDomainQemuMonitorEventDispose(void *obj); static void virDomainEventTunableDispose(void *obj); static void virDomainEventAgentLifecycleDispose(void *obj); static void virDomainEventDeviceAddedDispose(void *obj); +static void virDomainEventWriteThresholdDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -236,6 +238,17 @@ struct _virDomainEventAgentLifecycle { typedef struct _virDomainEventAgentLifecycle virDomainEventAgentLifecycle; typedef virDomainEventAgentLifecycle *virDomainEventAgentLifecyclePtr; +struct _virDomainEventWriteThreshold { + virDomainEvent parent; + + char *disk; + char *path; + unsigned long long threshold; + unsigned long long length; +}; +typedef struct _virDomainEventWriteThreshold virDomainEventWriteThreshold; +typedef virDomainEventWriteThreshold *virDomainEventWriteThresholdPtr; + static int virDomainEventsOnceInit(void) @@ -336,6 +349,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventAgentLifecycle), virDomainEventAgentLifecycleDispose))) return -1; + if (!(virDomainEventWriteThresholdClass = + virClassNew(virDomainEventClass, + "virDomainEventWriteThreshold", + sizeof(virDomainEventWriteThreshold), + virDomainEventWriteThresholdDispose))) + return -1; return 0; } @@ -496,6 +515,16 @@ virDomainEventAgentLifecycleDispose(void *obj) VIR_DEBUG("obj=%p", event); }; +static void +virDomainEventWriteThresholdDispose(void *obj) +{ + virDomainEventWriteThresholdPtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->disk); + VIR_FREE(event->path); +}; + static void * virDomainEventNew(virClassPtr klass, @@ -1390,6 +1419,62 @@ virDomainEventTunableNewFromDom(virDomainPtr dom, nparams); } +static virObjectEventPtr +virDomainEventWriteThresholdNew(int id, + const char *name, + unsigned char *uuid, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length) +{ + virDomainEventWriteThresholdPtr ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventWriteThresholdClass, + VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD, + id, name, uuid))) + return NULL; + + if (VIR_STRDUP(ev->disk, disk) < 0) + goto error; + if (VIR_STRDUP(ev->path, path) < 0) + goto error; + ev->threshold = threshold; + ev->length = length; + + return (virObjectEventPtr)ev; + + error: + virObjectUnref(ev); + return NULL; +} + +virObjectEventPtr +virDomainEventWriteThresholdNewFromObj(virDomainObjPtr obj, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length) +{ + return virDomainEventWriteThresholdNew(obj->def->id, obj->def->name, + obj->def->uuid, disk, path, + threshold, length); +} + +virObjectEventPtr +virDomainEventWriteThresholdNewFromDom(virDomainPtr dom, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length) +{ + return virDomainEventWriteThresholdNew(dom->id, dom->name, dom->uuid, + disk, path, threshold, length); +} + static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -1615,7 +1700,19 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, } case VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD: - /* TODO: Implement RPC support for this */ + { + virDomainEventWriteThresholdPtr ev; + + ev = (virDomainEventWriteThresholdPtr)event; + ((virConnectDomainEventWriteThresholdCallback)cb)(conn, dom, + ev->disk, + ev->path, + ev->threshold, + ev->length, + cbopaque); + goto cleanup; + } + case VIR_DOMAIN_EVENT_ID_LAST: break; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index afbed89..6e31e3e 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -1,7 +1,7 @@ /* * domain_event.h: domain event queue processing helpers * - * Copyright (C) 2012-2014 Red Hat, Inc. + * Copyright (C) 2012-2015 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. * @@ -209,6 +209,20 @@ virDomainEventAgentLifecycleNewFromDom(virDomainPtr dom, int state, int reason); +virObjectEventPtr +virDomainEventWriteThresholdNewFromObj(virDomainObjPtr obj, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length); +virObjectEventPtr +virDomainEventWriteThresholdNewFromDom(virDomainPtr dom, + const char *disk, + const char *path, + unsigned long long threshold, + unsigned long long length); + + int virDomainEventStateRegister(virConnectPtr conn, virObjectEventStatePtr state, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1566d11..bd16860 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -541,6 +541,8 @@ virDomainEventTunableNewFromDom; virDomainEventTunableNewFromObj; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; +virDomainEventWriteThresholdNewFromDom; +virDomainEventWriteThresholdNewFromObj; virDomainQemuMonitorEventNew; virDomainQemuMonitorEventStateRegisterID; -- 2.4.3

Track whether qemu is new enough to do block thresholds on the active layer (feature added in qemu 2.3). The plan is that even if qemu is too old, attempts to register a threshold value will return failure; but the event handler callback can still be registered successfuly (and will just never fire). Thresholds only work if you use named nodes. But overhauling libvirt to track named nodes is a pain; particularly since a single qcow2 libvirt <disk> element has multiple qemu nodes (one for the qcow2 protocol, that controls what the guest sees, and one for the underlying file, that controls what is done on the host); the threshold information we are interested in is associated with the host view, but that is NOT the node at the active layer of a block device. So while we may eventually want to track node names in libvirt XML and have control over the names of (some) nodes used by qemu, there is no guarantee that we want to do it for all nodes. For now, it is easier to just assume that qemu will supply a generated node name when we don't (a feature added in qemu 2.4), and the rest of this series will merely cache generated node names (re-querying it from qemu as needed after a libvirtd restart) rather than trying to control a fixed name. The ability to set thresholds and the ability to auto name nodes are indepenent enough to be backported separately in downstream qemu, so we have to use two capability bits, and will require both to be set before allowing a write threshold registration. If my assumptions during probing are ever broken (for example, if fdset 0 is no longer wired to read-only /dev/null, or if a named node already exists prior to our point of probing), the code could be made more robust by using the 'null-co' driver instead of 'file', and then by iterating through the array to ensure one of the nodes is visiting the just-registered fdset file. But this patch works for now. I _hate_ the way qemucapabilitiestests works - it is NOT easy to debug how to fix failures in that test. But I don't have time to revamp it today (ideally, the .replies file should include as a comment the command that was associated with a given reply, so that injecting new commands in the code can more easily match that to the .replies file. Furthermore, the replies should not be so sensitive to changing "id" fields). * src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_WRITE_THRESHOLD) (QEMU_CAPS_AUTO_NODE_NAMES): New bits * src/qemu/qemu_capabilities.c (virQEMUCapsCommands): Enable them... (virQEMUCapsProbeQMPCommands): ...but only if they actually work. * src/qemu/qemu_monitor.c (qemuMonitorSupportsNodeNames): New function. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONSupportsNodeNames): Likewise. * src/qemu/qemu_monitor.h (qemuMonitorSupportsNodeNames): New prototype. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONSupportsNodeNames): Likewise. * tests/qemucapabilitiesdata/caps_2.1.1-1.replies: Update test. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 12 +- src/qemu/qemu_capabilities.h | 4 +- src/qemu/qemu_monitor.c | 12 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 65 ++++ src/qemu/qemu_monitor_json.h | 3 + tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 470 ++++++++++++------------ 7 files changed, 336 insertions(+), 231 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e7002a3..af597e3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1,7 +1,7 @@ /* * qemu_capabilities.c: QEMU capabilities generation * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -288,6 +288,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vhost-user-multiqueue", /* 190 */ "arm-virt-pci", + "block-write-threshold", + "auto-node-names", ); @@ -1499,6 +1501,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "nbd-server-start", QEMU_CAPS_NBD_SERVER }, { "change-backing-file", QEMU_CAPS_CHANGE_BACKING_FILE }, { "rtc-reset-reinjection", QEMU_CAPS_RTC_RESET_REINJECTION }, + { "block-set-write-threshold", QEMU_CAPS_BLOCK_WRITE_THRESHOLD }, + { "query-named-block-nodes", QEMU_CAPS_AUTO_NODE_NAMES }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { @@ -2359,6 +2363,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, qemuMonitorSupportsActiveCommit(mon)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + /* Qemu 2.3 did not automatically set node names, so querying node + * information is useless if we don't supply them. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_AUTO_NODE_NAMES) && + !qemuMonitorSupportsNodeNames(mon)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_AUTO_NODE_NAMES); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f4180a8..32877c8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -1,7 +1,7 @@ /* * qemu_capabilities.h: QEMU capabilities generation * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -231,6 +231,8 @@ typedef enum { QEMU_CAPS_CPU_AARCH64_OFF = 189, /* -cpu ...,aarch64=off */ QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ QEMU_CAPS_ARM_VIRT_PCI = 191, /* ARM 'virt' machine has PCI bus */ + QEMU_CAPS_BLOCK_WRITE_THRESHOLD = 192, /* block-set-write-threshold */ + QEMU_CAPS_AUTO_NODE_NAMES = 193, /* query-named-block-nodes is useful */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 904d682..2e9e2de 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3082,6 +3082,18 @@ qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon) } +/* Probe whether node names are automatically assigned by a given qemu + * binary; assumes that fd set 0 is available for device hotplug. */ +bool +qemuMonitorSupportsNodeNames(qemuMonitorPtr mon) +{ + if (!mon || !mon->json) + return false; + + return qemuMonitorJSONSupportsNodeNames(mon); +} + + /* Determine the name that qemu is using for tracking the backing * element TARGET within the chain starting at TOP. */ char * diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 1afc344..e198c06 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -739,6 +739,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon, unsigned long long bandwidth) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon); +bool qemuMonitorSupportsNodeNames(qemuMonitorPtr mon); char *qemuMonitorDiskNameLookup(qemuMonitorPtr mon, const char *device, virStorageSourcePtr top, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 71e8f4b..e4701aa 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3900,6 +3900,71 @@ qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, } +/* Used only for capability probing. Assumes that fd set 0 is already + * connected to /dev/null and that we have no existing nodes; we then + * try to add the fd as a block device, and see if the new device has + * a node name assigned by qemu. */ +bool +qemuMonitorJSONSupportsNodeNames(qemuMonitorPtr mon) +{ + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr options = virJSONValueNewObject(); + bool ret = false; + virJSONValuePtr nodes; + + /* Create a node. */ + /* TODO: We could try using 'null-co' instead of 'file' as the + * driver, if we don't want to depend on /dev/fdset/0 already + * being present. */ + if (!options || + virJSONValueObjectAdd(options, + "s:driver", "file", + "s:id", "drive-fdset0", + "s:filename", "/dev/fdset/0", + "b:read-only", true, + NULL) < 0) + goto cleanup; + cmd = qemuMonitorJSONMakeCommand("blockdev-add", + "a:options", options, + NULL); + if (!cmd) + goto cleanup; + options = NULL; + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + if (virJSONValueObjectGet(reply, "error")) + goto cleanup; + + /* Then query for all nodes. */ + virJSONValueFree(cmd); + virJSONValueFree(reply); + cmd = qemuMonitorJSONMakeCommand("query-named-block-nodes", NULL); + if (!cmd) + goto cleanup; + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + if (virJSONValueObjectGet(reply, "error")) + goto cleanup; + + /* The array will be empty unless an automatic node name was assigned. */ + /* TODO: If we are paranoid that other named nodes might already + * exist before this probe, we could iterate the returned array + * and look for the fdset file name we just added. */ + nodes = virJSONValueObjectGet(reply, "return"); + if (nodes && virJSONValueArraySize(nodes) > 0) + ret = true; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(options); + + return ret; + +} + + int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, char **reply_str, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c0ee4ce..a8ae411 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -282,6 +282,9 @@ char *qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +bool qemuMonitorJSONSupportsNodeNames(qemuMonitorPtr mon) + ATTRIBUTE_NONNULL(1); + int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, char **reply_str, diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.replies b/tests/qemucapabilitiesdata/caps_2.1.1-1.replies index 511461a..04d4431 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.replies +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.replies @@ -391,6 +391,18 @@ } { + "return": { + }, + "id": "libvirt-7" +} + +{ + "return": [ + ], + "id": "libvirt-8" +} + +{ "return": [ { "name": "VSERPORT_CHANGE" @@ -489,7 +501,7 @@ "name": "BLOCK_IMAGE_CORRUPTED" } ], - "id": "libvirt-7" + "id": "libvirt-9" } { @@ -1290,7 +1302,7 @@ "name": "fusbh200-ehci-usb" } ], - "id": "libvirt-8" + "id": "libvirt-10" } { @@ -1400,7 +1412,7 @@ "type": "uint32" } ], - "id": "libvirt-9" + "id": "libvirt-11" } { @@ -1562,11 +1574,11 @@ "type": "on/off" } ], - "id": "libvirt-10" + "id": "libvirt-12" } { - "id": "libvirt-11", + "id": "libvirt-13", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-ccw' not found" @@ -1574,7 +1586,7 @@ } { - "id": "libvirt-12", + "id": "libvirt-14", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-ccw' not found" @@ -1582,7 +1594,7 @@ } { - "id": "libvirt-13", + "id": "libvirt-15", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-blk-s390' not found" @@ -1590,7 +1602,7 @@ } { - "id": "libvirt-14", + "id": "libvirt-16", "error": { "class": "DeviceNotFound", "desc": "Device 'virtio-net-s390' not found" @@ -1598,7 +1610,7 @@ } { - "id": "libvirt-15", + "id": "libvirt-17", "error": { "class": "DeviceNotFound", "desc": "Device 'pci-assign' not found" @@ -1648,7 +1660,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-16" + "id": "libvirt-18" } { @@ -1690,7 +1702,7 @@ "type": "pci-host-devaddr" } ], - "id": "libvirt-17" + "id": "libvirt-19" } { @@ -1776,7 +1788,7 @@ "type": "drive" } ], - "id": "libvirt-18" + "id": "libvirt-20" } { @@ -1830,7 +1842,7 @@ "type": "drive" } ], - "id": "libvirt-19" + "id": "libvirt-21" } { @@ -1880,7 +1892,7 @@ "type": "uint32" } ], - "id": "libvirt-20" + "id": "libvirt-22" } { @@ -1918,7 +1930,7 @@ "type": "chr" } ], - "id": "libvirt-21" + "id": "libvirt-23" } { @@ -1980,76 +1992,76 @@ "type": "uint32" } ], - "id": "libvirt-22" -} - -{ - "return": [ - { - "name": "lun", - "type": "uint32" - }, - { - "name": "scsi-id", - "type": "uint32" - }, - { - "name": "channel", - "type": "uint32" - }, - { - "name": "bootindex", - "type": "int32" - }, - { - "name": "drive", - "type": "drive" - } - ], - "id": "libvirt-23" -} - -{ - "return": [ - { - "name": "pci-hole64-end", - "type": "int" - }, - { - "name": "pci-hole64-start", - "type": "int" - }, - { - "name": "pci-hole-end", - "type": "int" - }, - { - "name": "pci-hole-start", - "type": "int" - }, - { - "name": "pci-conf-data[0]", - "type": "child<qemu:memory-region>" - }, - { - "name": "pci-conf-idx[0]", - "type": "child<qemu:memory-region>" - }, - { - "name": "short_root_bus", - "type": "uint32" - }, - { - "name": "pci-hole64-size", - "type": "size" - } - ], "id": "libvirt-24" } { "return": [ { + "name": "lun", + "type": "uint32" + }, + { + "name": "scsi-id", + "type": "uint32" + }, + { + "name": "channel", + "type": "uint32" + }, + { + "name": "bootindex", + "type": "int32" + }, + { + "name": "drive", + "type": "drive" + } + ], + "id": "libvirt-25" +} + +{ + "return": [ + { + "name": "pci-hole64-end", + "type": "int" + }, + { + "name": "pci-hole64-start", + "type": "int" + }, + { + "name": "pci-hole-end", + "type": "int" + }, + { + "name": "pci-hole-start", + "type": "int" + }, + { + "name": "pci-conf-data[0]", + "type": "child<qemu:memory-region>" + }, + { + "name": "pci-conf-idx[0]", + "type": "child<qemu:memory-region>" + }, + { + "name": "short_root_bus", + "type": "uint32" + }, + { + "name": "pci-hole64-size", + "type": "size" + } + ], + "id": "libvirt-26" +} + +{ + "return": [ + { "name": "mcfg_size", "type": "int" }, @@ -2094,7 +2106,7 @@ "type": "uint64" } ], - "id": "libvirt-25" + "id": "libvirt-27" } { @@ -2148,7 +2160,7 @@ "type": "drive" } ], - "id": "libvirt-26" + "id": "libvirt-28" } { @@ -2162,7 +2174,7 @@ "type": "uint32" } ], - "id": "libvirt-27" + "id": "libvirt-29" } { @@ -2196,106 +2208,6 @@ "type": "uint32" } ], - "id": "libvirt-28" -} - -{ - "return": [ - { - "name": "command_serr_enable", - "type": "on/off" - }, - { - "name": "multifunction", - "type": "on/off" - }, - { - "name": "rombar", - "type": "uint32" - }, - { - "name": "romfile", - "type": "string" - }, - { - "name": "addr", - "type": "pci-devfn" - }, - { - "name": "vgamem_mb", - "type": "uint32" - } - ], - "id": "libvirt-29" -} - -{ - "return": [ - { - "name": "command_serr_enable", - "type": "on/off" - }, - { - "name": "multifunction", - "type": "on/off" - }, - { - "name": "rombar", - "type": "uint32" - }, - { - "name": "romfile", - "type": "string" - }, - { - "name": "addr", - "type": "pci-devfn" - }, - { - "name": "surfaces", - "type": "int32" - }, - { - "name": "vgamem_mb", - "type": "uint32" - }, - { - "name": "vram64_size_mb", - "type": "uint32" - }, - { - "name": "vram_size_mb", - "type": "uint32" - }, - { - "name": "ram_size_mb", - "type": "uint32" - }, - { - "name": "cmdlog", - "type": "uint32" - }, - { - "name": "guestdebug", - "type": "uint32" - }, - { - "name": "debug", - "type": "uint32" - }, - { - "name": "revision", - "type": "uint32" - }, - { - "name": "vram_size", - "type": "uint32" - }, - { - "name": "ram_size", - "type": "uint32" - } - ], "id": "libvirt-30" } @@ -2322,54 +2234,154 @@ "type": "pci-devfn" }, { - "name": "surfaces", - "type": "int32" - }, - { "name": "vgamem_mb", "type": "uint32" - }, - { - "name": "vram64_size_mb", - "type": "uint32" - }, - { - "name": "vram_size_mb", - "type": "uint32" - }, - { - "name": "ram_size_mb", - "type": "uint32" - }, - { - "name": "cmdlog", - "type": "uint32" - }, - { - "name": "guestdebug", - "type": "uint32" - }, - { - "name": "debug", - "type": "uint32" - }, - { - "name": "revision", - "type": "uint32" - }, - { - "name": "vram_size", - "type": "uint32" - }, - { - "name": "ram_size", - "type": "uint32" } ], "id": "libvirt-31" } { + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + }, + { + "name": "surfaces", + "type": "int32" + }, + { + "name": "vgamem_mb", + "type": "uint32" + }, + { + "name": "vram64_size_mb", + "type": "uint32" + }, + { + "name": "vram_size_mb", + "type": "uint32" + }, + { + "name": "ram_size_mb", + "type": "uint32" + }, + { + "name": "cmdlog", + "type": "uint32" + }, + { + "name": "guestdebug", + "type": "uint32" + }, + { + "name": "debug", + "type": "uint32" + }, + { + "name": "revision", + "type": "uint32" + }, + { + "name": "vram_size", + "type": "uint32" + }, + { + "name": "ram_size", + "type": "uint32" + } + ], + "id": "libvirt-32" +} + +{ + "return": [ + { + "name": "command_serr_enable", + "type": "on/off" + }, + { + "name": "multifunction", + "type": "on/off" + }, + { + "name": "rombar", + "type": "uint32" + }, + { + "name": "romfile", + "type": "string" + }, + { + "name": "addr", + "type": "pci-devfn" + }, + { + "name": "surfaces", + "type": "int32" + }, + { + "name": "vgamem_mb", + "type": "uint32" + }, + { + "name": "vram64_size_mb", + "type": "uint32" + }, + { + "name": "vram_size_mb", + "type": "uint32" + }, + { + "name": "ram_size_mb", + "type": "uint32" + }, + { + "name": "cmdlog", + "type": "uint32" + }, + { + "name": "guestdebug", + "type": "uint32" + }, + { + "name": "debug", + "type": "uint32" + }, + { + "name": "revision", + "type": "uint32" + }, + { + "name": "vram_size", + "type": "uint32" + }, + { + "name": "ram_size", + "type": "uint32" + } + ], + "id": "libvirt-33" +} + +{ "return": [ { "name": "pc-1.3", @@ -2479,7 +2491,7 @@ "cpu-max": 255 } ], - "id": "libvirt-32" + "id": "libvirt-34" } { @@ -2560,7 +2572,7 @@ "name": "qemu64" } ], - "id": "libvirt-33" + "id": "libvirt-35" } { @@ -2568,21 +2580,21 @@ "enabled": false, "present": true }, - "id": "libvirt-34" + "id": "libvirt-36" } { "return": [ "tpm-tis" ], - "id": "libvirt-35" + "id": "libvirt-37" } { "return": [ "passthrough" ], - "id": "libvirt-36" + "id": "libvirt-38" } { @@ -3442,7 +3454,7 @@ "option": "drive" } ], - "id": "libvirt-37" + "id": "libvirt-39" } { @@ -3464,5 +3476,5 @@ "capability": "zero-blocks" } ], - "id": "libvirt-38" + "id": "libvirt-40" } -- 2.4.3

On Mon, Jun 22, 2015 at 17:06:42 -0600, Eric Blake wrote: ... Since this is yet another function that would call query-blockstats ... ... and works only on one device and thus needs to be re-called for every single disk I'd rather see us fold this code into qemuMonitorJSONGetOneBlockStatsInfo so that all the info can be returned via qemuMonitorJSONGetAllBlockStatsInfo. I'll post a series that kills qemuMonitorJSONGetBlockExtent since it's basically the same instance. Peter

On Mon, Jun 22, 2015 at 17:06:43 -0600, Eric Blake wrote: Rather than doing the ad-hoc back and forth mapping I think we should cache all node names for all disks when reconnecting to the monitor and when we complete a snapshot operation. This will simplfiy the event code quite a bit and additionally it will be more future proof. The callers should use qemuMonitorCleanBlockStats rather than deleting it here. ... We use either Clear or Free. I'd go with qemuMonitorBlockStatsFree. ... See below ... The node name will be useful for other operations too so I'd choose a more universal name. Peter

On Mon, Jun 22, 2015 at 17:06:44 -0600, Eric Blake wrote: While this is okay in this intermediate part I am not okay with merging this series without the full support for non-top layers. If we add a partial impl we will force the users to check whether given libvirt supports it. Since I know of only one consumer of this feature which is oVirt and they are interrested in both data we would increase the complexity of usefullnes of this feature. I'll send a series today that will remove this function. qemuMonitorJSONGetOneBlockStatsInfo will need to do this operation so that we don't duplicate calls to query-blockstats. Spurious whitespace change. I think in the future it will be necessary to have the data from query-named-blocks available for use so I think that this helper should similarly to qemuMonitorJSONGetAllBlockStatsInfo return a hash of the useful data, in this case keyed by the node name, and the caller would do the disk->node matching when it requires the data if it requires it. Peter

On Mon, Jun 22, 2015 at 17:06:45 -0600, Eric Blake wrote: ... This duality of operations is why we should cache the node names upfront. Peter

On Mon, Jun 22, 2015 at 17:06:46 -0600, Eric Blake wrote: This function begins with a rather long comment explaining that it's not entirely a good idea to use it on online VMs and especially on write-enabled disk sources. Shouldn't we query the monitor rather than use that? This function needs a node name passed so it should also be called that way to avoid confusion since the function lacks docs. Peter