[libvirt] [PATCHv3 05/10] threshold: add qemu capability bits

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