[PATCH 0/8] qemu: Report proper stats when a backup job is running
by Peter Krempa
Patch 1 fixes the bug this series is about, patch 2 fixes same thing in
the bulk stats. Patches 3-6 remove some pointless cleanup sections and
patches 7-8 remove the 'backingChain' argument as we always want to
fetch all stats.
Peter Krempa (8):
qemuDomainBlocksStatsGather: Always fetch stats for the full backing
chain
qemuDomainGetStatsBlock: Always fetch stats for the full backing chain
qemuMonitorJSONBlockStatsUpdateCapacity: Refactor cleanup
qemuMonitorJSONBlockStatsUpdateCapacityOne: Refactor cleanup
qemuDomainGetStatsBlock: Refactor cleanup
qemuMonitorJSONQueryBlockstats: Refactor cleanup
qemuMonitorGetAllBlockStatsInfo: Remove 'backingChain' argument
qemuMonitorBlockStatsUpdateCapacity: Remove 'backingChain' argument
src/qemu/qemu_driver.c | 31 ++++++-------
src/qemu/qemu_migration_cookie.c | 2 +-
src/qemu/qemu_monitor.c | 20 +++------
src/qemu/qemu_monitor.h | 6 +--
src/qemu/qemu_monitor_json.c | 74 +++++++++++---------------------
src/qemu/qemu_monitor_json.h | 6 +--
tests/qemumonitorjsontest.c | 2 +-
7 files changed, 48 insertions(+), 93 deletions(-)
--
2.31.1
3 years, 1 month
[libvirt PATCH v6 0/5] Add a PCI/PCIe device VPD Capability
by Dmitrii Shcherbakov
Add support for deserializing the binary PCI/PCIe VPD format and
exposing VPD resources as XML elements in a new nested capability
of PCI/PCIe devices called 'vpd'.
The series contains the following incremental changes:
* The PCI VPD parser module, in-memory resource representation
and tests;
* VPD-related helpers added to the virpci module;
* VPD capability support: XML serialization/deserialization from/into
VPD resource data structures;
* Documentation.
The VPD format is specified in "I.3. VPD Definitions" in PCI specs
(2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0
notes, the PCI Local Bus and PCIe VPD formats are binary compatible
and PCIe 4.0 merely started incorporating what was already present in
PCI specs.
Linux kernel exposes a binary blob in the VPD format via sysfs since
v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires
a parser to interpret.
There are usage scenarios where information such as the board serial
number needs to be retrieved from PCI(e) VPD. Projects like Nova can
utilize this information for cases which involve virtual interface
plugging on SmartNIC DPUs but there may be other scenarios and types of
information useful to retrieve from VPD. The fact that the format is
binary requires proper parsing instead of substring searching hence the
full parser is proposed. Likewise, checksum validation requires proper
parsing as well.
A usage example is present here:
https://review.opendev.org/c/openstack/nova/+/808199
The patch follows a prior discussion on the mailing list which has
additional context about the use-case but a narrower proposal:
https://listman.redhat.com/archives/libvir-list/2021-May/msg00873.html
https://www.mail-archive.com/libvir-list@redhat.com/msg218165.html
The new functionality is mostly contained in virpcivpd with a
couple of new functions added to virpci. Additionally, the necessary XML
serialization/deserialization and glue code is added to expose the VPD
capability to external clients as XML.
A new capability flag is added along with a new capability in order to
allow for filtering of PCI devices with the VPD capability using virsh:
virsh nodedev-list --cap vpd
sudo virsh nodedev-dumpxml --device pci_dddd_bb_ss_f
In this example having the root uid is required in order to access the
vpd sysfs entry, therefore, the nodedev XML output will only contain
the VPD capability if virsh is run as root.
The capability is treated as dynamic due to the presence of read-write
sections in the VPD format per PCI/PCIe specs (the idea being that
read-write resource fields may potentially be altered by the DPU OS
over time independently from the host OS).
Unit tests cover the parser functionality (including many possible
invalid cases), in-memory representation as well as XML serialization
and deserialization.
Manual functional testing was performed with 2 DPUs and several other
NIC models which expose PCI(e) VPD. Testing have also been performed
for devices that do not have VPD or those that expose a VPD capability
but exhibit invalid behavior (I/O errors while reading a sysfs entry).
v6 changes:
* Reworked in-memory data structures according to the feedback from
Daniel: virPCIVPDResource is now a struct with nested structures for
read-only and read-write parts of the VPD. Custom fields (vendor,
system are now stored in GPtrArray instances;
* Reworked XML element naming: well-known fields from the spec now have
human-readable names. Legacy PICMIG fields and binary fields are
omitted as before;
* The fieldValueFormats hash table is now initialized in a thread-safe
manner using VIR_ONCE_GLOBAL_INIT. The values are still duplicated
since the GHashTable requires non-const pointers;
* VPD field value validation now uses a simpler character set validation
instead of Glib-provided regular expressions as was suggested in
comments to v5;
* virPCIVPDParse no longer attempts to close the fd passed to it and
leaves that for the caller to do;
* Extended XML schema testing with RW section validation and reworked
the RNG schema to account for the new XML formatting structure;
* Improved test coverage in multiple places;
* Refactoring: replaced gchar and gboolean usage with char and bool per
the mailing list discussion.
Build & test results for targets in ci/manifest.yaml:
ci/helper test --meson-args='-Dexpensive_tests=enabled' <target>
https://gist.github.com/dshcherb/867d87a7419b91bb68388518ca5a9c32
Dmitrii Shcherbakov (5):
Add a PCI/PCIe device VPD Parser
Add PCI VPD-related helper functions to virpci
Add PCI VPD Capability Support
Add PCI VPD Capability Documentation
news: Add PCI VPD parser & capability notes
NEWS.rst | 22 +
build-aux/syntax-check.mk | 4 +-
docs/drvnodedev.html.in | 69 ++
docs/formatnode.html.in | 63 +-
docs/schemas/nodedev.rng | 96 +++
include/libvirt/libvirt-nodedev.h | 1 +
po/POTFILES.in | 1 +
src/conf/node_device_conf.c | 293 +++++++
src/conf/node_device_conf.h | 7 +-
src/conf/virnodedeviceobj.c | 7 +-
src/libvirt_private.syms | 20 +
src/node_device/node_device_driver.c | 2 +
src/node_device/node_device_udev.c | 2 +
src/util/meson.build | 1 +
src/util/virpci.c | 70 ++
src/util/virpci.h | 4 +
src/util/virpcivpd.c | 743 +++++++++++++++++
src/util/virpcivpd.h | 111 +++
src/util/virpcivpdpriv.h | 48 ++
tests/meson.build | 1 +
.../pci_0000_42_00_0_vpd.xml | 42 +
.../pci_0000_42_00_0_vpd.xml | 1 +
tests/nodedevxml2xmltest.c | 1 +
tests/testutils.c | 35 +
tests/testutils.h | 4 +
tests/virpcimock.c | 30 +
tests/virpcitest.c | 39 +
tests/virpcivpdtest.c | 768 ++++++++++++++++++
tools/virsh-nodedev.c | 3 +
29 files changed, 2483 insertions(+), 5 deletions(-)
create mode 100644 src/util/virpcivpd.c
create mode 100644 src/util/virpcivpd.h
create mode 100644 src/util/virpcivpdpriv.h
create mode 100644 tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml
create mode 120000 tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml
create mode 100644 tests/virpcivpdtest.c
--
2.30.2
3 years, 1 month
[PATCH] rpc: Temporarily stop accept()-ing new clients on EMFILE
by Michal Privoznik
This commit is related to 5de203f879 which I pushed a few days
ago. While that commit prioritized closing clients socket over
the rest of I/O process, this one goes one step further and
temporarily suspends processing new connection requests.
A brief recapitulation of the problem:
1) assume that libvirt is at the top of RLIMIT_NOFILE (that is no
new FDs can be opened).
2) we have a client trying to connect to a UNIX/TCP socket
Because of 2) our event loop sees POLLIN on the socket and thus
calls virNetServerServiceAccept(). But since no new FDs can be
opened (because of 1)) the request is not handled and we will get
the same event on next iteration. The poll() will exit
immediately because there is an event on the socket. Thus we end
up in an endless loop.
To break the loop and stop burning CPU cycles we can stop
listening for events on the socket and set up a timer tho enable
listening again after some time (I chose 5 seconds because of no
obvious reason).
There's another area where we play with temporarily suspending
accept() of new clients - when a client disconnects and we check
max_clients against number of current clients. Problem here is
that max_clients can be orders of magnitude larger than
RLIMIT_NOFILE but more importantly, what this code considers
client disconnect is not equal to closing client's FD.
A client disconnecting means that the corresponding client
structure is removed from the internal list of clients. Closing
of the client's FD is done from event loop - asynchronously.
To avoid this part stepping on the toes of my fix, let's make the
code NOP if socket timer (as described above) is active.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetserver.c | 9 ++++++
src/rpc/virnetserverservice.c | 53 ++++++++++++++++++++++++++++++++++-
src/rpc/virnetserverservice.h | 2 ++
src/rpc/virnetsocket.c | 15 ++++++++++
5 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index b4265adf2e..942e1013a6 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -221,6 +221,7 @@ virNetServerServiceNewTCP;
virNetServerServiceNewUNIX;
virNetServerServicePreExecRestart;
virNetServerServiceSetDispatcher;
+virNetServerServiceTimerActive;
virNetServerServiceToggle;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index b3214883ee..dc8f32b095 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -252,6 +252,15 @@ virNetServerDispatchNewMessage(virNetServerClient *client,
static void
virNetServerCheckLimits(virNetServer *srv)
{
+ size_t i;
+
+ for (i = 0; i < srv->nservices; i++) {
+ if (virNetServerServiceTimerActive(srv->services[i])) {
+ VIR_DEBUG("Skipping client-related limits evaluation");
+ return;
+ }
+ }
+
VIR_DEBUG("Checking client-related limits to re-enable or temporarily "
"suspend services: nclients=%zu nclients_max=%zu "
"nclients_unauth=%zu nclients_unauth_max=%zu",
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 0c4c437a49..214eae1acb 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -43,6 +43,8 @@ struct _virNetServerService {
int auth;
bool readonly;
size_t nrequests_client_max;
+ int timer;
+ bool timerActive;
virNetTLSContext *tls;
@@ -71,9 +73,25 @@ static void virNetServerServiceAccept(virNetSocket *sock,
{
virNetServerService *svc = opaque;
virNetSocket *clientsock = NULL;
+ int rc;
- if (virNetSocketAccept(sock, &clientsock) < 0)
+ rc = virNetSocketAccept(sock, &clientsock);
+ if (rc < 0) {
+ if (rc == -2) {
+ /* Could not accept new client due to EMFILE. Suspend listening on
+ * the socket and set up a timer to enable it later. Hopefully,
+ * some FDs will be closed meanwhile. */
+ VIR_DEBUG("Temporarily suspending listening on svc=%p because accept() on sock=%p failed (errno=%d)",
+ svc, sock, errno);
+
+ virNetServerServiceToggle(svc, false);
+
+ svc->timerActive = true;
+ /* Retry in 5 seconds. */
+ virEventUpdateTimeout(svc->timer, 5 * 1000);
+ }
goto cleanup;
+ }
if (!clientsock) /* Connection already went away */
goto cleanup;
@@ -88,6 +106,21 @@ static void virNetServerServiceAccept(virNetSocket *sock,
}
+static void
+virNetServerServiceTimerFunc(int timer,
+ void *opaque)
+{
+ virNetServerService *svc = opaque;
+
+ VIR_DEBUG("Resuming listening on service svc=%p after previous suspend", svc);
+
+ virNetServerServiceToggle(svc, true);
+
+ virEventUpdateTimeout(timer, -1);
+ svc->timerActive = false;
+}
+
+
static virNetServerService *
virNetServerServiceNewSocket(virNetSocket **socks,
size_t nsocks,
@@ -117,6 +150,14 @@ virNetServerServiceNewSocket(virNetSocket **socks,
svc->nrequests_client_max = nrequests_client_max;
svc->tls = virObjectRef(tls);
+ virObjectRef(svc);
+ svc->timer = virEventAddTimeout(-1, virNetServerServiceTimerFunc,
+ svc, virObjectFreeCallback);
+ if (svc->timer < 0) {
+ virObjectUnref(svc);
+ goto error;
+ }
+
for (i = 0; i < svc->nsocks; i++) {
if (virNetSocketListen(svc->socks[i], max_queued_clients) < 0)
goto error;
@@ -407,6 +448,9 @@ void virNetServerServiceDispose(void *obj)
virNetServerService *svc = obj;
size_t i;
+ if (svc->timer >= 0)
+ virEventRemoveTimeout(svc->timer);
+
for (i = 0; i < svc->nsocks; i++)
virObjectUnref(svc->socks[i]);
g_free(svc->socks);
@@ -438,3 +482,10 @@ void virNetServerServiceClose(virNetServerService *svc)
virNetSocketClose(svc->socks[i]);
}
}
+
+
+bool
+virNetServerServiceTimerActive(virNetServerService *svc)
+{
+ return svc->timerActive;
+}
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index ab5798938e..f3d55a9cc0 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -78,3 +78,5 @@ void virNetServerServiceToggle(virNetServerService *svc,
bool enabled);
void virNetServerServiceClose(virNetServerService *svc);
+
+bool virNetServerServiceTimerActive(virNetServerService *svc);
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 212089520d..60dff71718 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -2067,6 +2067,19 @@ int virNetSocketListen(virNetSocket *sock, int backlog)
return 0;
}
+
+/**
+ * virNetSocketAccept:
+ * @sock: socket to accept connection on
+ * @clientsock: returned client socket
+ *
+ * For given socket @sock accept incoming connection and create
+ * @clientsock representation of the new accepted connection.
+ *
+ * Returns: 0 on success,
+ * -2 if accepting failed due to EMFILE error,
+ * -1 otherwise.
+ */
int virNetSocketAccept(virNetSocket *sock, virNetSocket **clientsock)
{
int fd = -1;
@@ -2087,6 +2100,8 @@ int virNetSocketAccept(virNetSocket *sock, virNetSocket **clientsock)
errno == EAGAIN) {
ret = 0;
goto cleanup;
+ } else if (errno == EMFILE) {
+ ret = -2;
}
virReportSystemError(errno, "%s",
--
2.32.0
3 years, 1 month
Plans for the next release
by Jiri Denemark
We are getting close to the next release of libvirt. To aim for the
release on Nov 01 I suggest entering the freeze on Tuesday Oct 26 and
tagging RC2 on Friday Oct 29.
I hope this works for everyone.
Jirka
3 years, 1 month
[PATCH v4] qemu: tpm: Run swtpm_setup --create-config-files in session mode
by Stefan Berger
Using swtpm v0.7.0 we can run swtpm_setup to create default config files
for swtpm_setup and swtpm-localca in session mode. Now a user can start
a VM with an attached TPM without having to run this program on the
command line before. This program needs to run once.
This patch addresses the issue raised in
https://bugzilla.redhat.com/show_bug.cgi?id=2010649
Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com>
---
v4:
- Append stderr output to virReportError if swtpm_setup fails
v3:
- Removed logfile parameter
v2:
- fixed return code if swtpm_setup doesn't support the option
---
src/qemu/qemu_tpm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/util/virtpm.c | 1 +
src/util/virtpm.h | 1 +
3 files changed, 44 insertions(+)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 100481503c..f797a87fdc 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -385,6 +385,45 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid,
return virCommandSetSendBuffer(cmd, g_steal_pointer(&secret), secret_len);
}
+
+/*
+ * qemuTPMCreateConfigFiles: run swtpm_setup --create-config-files skip-if-exist
+ */
+static int
+qemuTPMCreateConfigFiles(void)
+{
+ g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ g_autoptr(virCommand) cmd = NULL;
+ g_autofree char *errbuf = NULL;
+ int exitstatus;
+
+ if (!swtpm_setup)
+ return -1;
+
+ if (!virTPMSwtpmSetupCapsGet(
+ VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES))
+ return 0;
+
+ cmd = virCommandNew(swtpm_setup);
+ if (!cmd)
+ return -1;
+
+ virCommandAddArgList(cmd, "--create-config-files", "skip-if-exist", NULL);
+ virCommandClearCaps(cmd);
+ virCommandSetErrorBuffer(cmd, &errbuf);
+
+ if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not run '%s' to create config files. "
+ "exitstatus: %d;\nError: %s"),
+ swtpm_setup, exitstatus, errbuf);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/*
* qemuTPMEmulatorRunSetup
*
@@ -432,6 +471,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
"this requires privileged mode for a "
"TPM 1.2\n"), 0600);
+ if (!privileged && qemuTPMCreateConfigFiles())
+ return -1;
+
cmd = virCommandNew(swtpm_setup);
if (!cmd)
return -1;
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 1a567139b4..0f50de866c 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature,
VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
VIR_TPM_SWTPM_SETUP_FEATURE_LAST,
"cmdarg-pwdfile-fd",
+ "cmdarg-create-config-files",
);
/**
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index d021a083b4..3bb03b3b33 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -38,6 +38,7 @@ typedef enum {
typedef enum {
VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
+ VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
VIR_TPM_SWTPM_SETUP_FEATURE_LAST
} virTPMSwtpmSetupFeature;
--
2.31.1
3 years, 1 month
[PATCH v2 00/15] qdev: Add JSON -device
by Kevin Wolf
It's still a long way until we'll have QAPIfied devices, but there are
some improvements that we can already make now to make the future switch
easier.
One important part of this is having code paths without QemuOpts, which
we want to get rid of and replace with the keyval parser in the long
run. This series adds support for JSON syntax to -device, which bypasses
QemuOpts.
While we're not using QAPI yet, devices are based on QOM, so we already
do have type checks and an implied schema. JSON syntax supported now can
be supported by QAPI later and regarding command line compatibility,
actually switching to it becomes an implementation detail this way (of
course, it will still add valuable user-visible features like
introspection and documentation).
Apart from making things more future proof, this also immediately adds
a way to do non-scalar properties on the command line. nvme could have
used list support recently, and the lack of it in -device led to some
rather unnatural solution in the first version (doing the relationship
between a device and objects backwards) and loss of features in the
following. With this series, using a list as a device property should be
possible without any weird tricks.
Unfortunately, even QMP device_add goes through QemuOpts before this
series, which destroys any type safety QOM provides and also can't
support non-scalar properties. This is a bug, but it turns out that
libvirt actually relies on it and passes only strings for everything.
So this series still leaves device_add alone until libvirt is fixed.
v2:
- Drop type safe QMP device_add because libvirt gets it wrong, too
- More network patches to eliminate dependencies on the legacy QemuOpts
data structures which would not contain all devices any more after
this series. Fix some bugs there as a side effect.
- Remove an unnecessary use of ERRP_GUARD()
- Replaced error handling patch for qdev_set_id() with Damien's
- Drop the deprecation patch until libvirt uses the new JSON syntax
Damien Hedde (1):
softmmu/qdev-monitor: add error handling in qdev_set_id
Kevin Wolf (14):
net: Introduce NetClientInfo.check_peer_type()
net/vhost-user: Fix device compatibility check
net/vhost-vdpa: Fix device compatibility check
qom: Reduce use of error_propagate()
iotests/245: Fix type for iothread property
iotests/051: Fix typo
qdev: Avoid using string visitor for properties
qdev: Make DeviceState.id independent of QemuOpts
qemu-option: Allow deleting opts during qemu_opts_foreach()
qdev: Add Error parameter to hide_device() callbacks
virtio-net: Store failover primary opts pointer locally
virtio-net: Avoid QemuOpts in failover_find_primary_device()
qdev: Base object creation on QDict rather than QemuOpts
vl: Enable JSON syntax for -device
qapi/qdev.json | 15 +++--
include/hw/qdev-core.h | 15 +++--
include/hw/virtio/virtio-net.h | 2 +
include/monitor/qdev.h | 27 +++++++-
include/net/net.h | 2 +
hw/arm/virt.c | 2 +-
hw/core/qdev-properties-system.c | 6 ++
hw/core/qdev.c | 11 +++-
hw/net/virtio-net.c | 85 ++++++++++++-------------
hw/pci-bridge/pci_expander_bridge.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/vfio/pci.c | 4 +-
hw/xen/xen-legacy-backend.c | 3 +-
net/vhost-user.c | 41 ++++--------
net/vhost-vdpa.c | 37 ++++-------
qom/object.c | 7 +-
qom/object_interfaces.c | 19 ++----
softmmu/qdev-monitor.c | 99 +++++++++++++++++++----------
softmmu/vl.c | 63 ++++++++++++++++--
util/qemu-option.c | 4 +-
tests/qemu-iotests/051 | 2 +-
tests/qemu-iotests/051.pc.out | 4 +-
tests/qemu-iotests/245 | 4 +-
23 files changed, 278 insertions(+), 178 deletions(-)
--
2.31.1
3 years, 1 month
[PATCH v2] virNodeDeviceDefParse: Don't call post-parse callbacks with NULL def
by Peter Krempa
When parsing of the node device XML fails we'd still call the post-parse
and validation callbacks which makes no sense. Additionally the
callbacks were expecting a non-NULL pointer which leads to a crash.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2014139
Fixes: d5ae634ba28
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
v2:
- Also handle failure of virXMLParse
- add 'fails' into first sentence of commit message
src/conf/node_device_conf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 9bbff97ffd..1f39e2cbfd 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2177,10 +2177,10 @@ virNodeDeviceDefParse(const char *str,
g_autoptr(xmlDoc) xml = NULL;
g_autoptr(virNodeDeviceDef) def = NULL;
- if ((xml = virXMLParse(filename, str, _("(node_device_definition)"), NULL, false))) {
- def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml),
- create, virt_type);
- }
+ if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"), NULL, false)) ||
+ !(def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml),
+ create, virt_type)))
+ return NULL;
if (parserCallbacks) {
int ret = 0;
--
2.31.1
3 years, 1 month
[PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'
by Peter Krempa
Issuing simple QMP commands is pain as they need to be wrapped by the
JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh
which allows simple usage of QMP and additionally prepares also for
passing through of the 'arguments' section:
virsh qemu-monitor-command $VM query-status
is equivalent to
virsh qemu-monitor-command $VM '{"execute":"query-status"}'
and
virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
or
virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'
is equivalent to
virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", "arguments":{"flat":true}}'
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
v2:
- dropped the '--qmpwrap' option and do wrapping if we don't get a
JSON object instead. Similarly for arguments.
docs/manpages/virsh.rst | 16 ++++++-
tools/virsh-domain.c | 98 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 103 insertions(+), 11 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 350ded2026..b4d31bf884 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -7772,7 +7772,21 @@ If more than one argument is provided for *command*, they are concatenated with
a space in between before passing the single command to the monitor.
Note that libvirt uses the QMP to talk to qemu so *command* must be valid JSON
-in QMP format to work properly.
+in QMP format to work properly. If *command* is not a JSON object libvirt tries
+to wrap it as a JSON object to provide convenient interface such as the groups
+of commands with identical handling:
+
+::
+
+ # simple command
+ $ virsh qemu-monitor-command VM commandname
+ $ virsh qemu-monitor-command VM '{"execute":"commandname"}'
+
+ # with arguments
+ $ virsh qemu-monitor-command VM commandname '"arg1":123' '"arg2":"test"'
+ $ virsh qemu-monitor-command VM commandname '{"arg1":123,"arg2":"test"}'
+ $ virsh qemu-monitor-command VM '{"execute":"commandname", "arguments":{"arg1":123,"arg2":"test"}}'
+
If *--pretty* is given the QMP reply is pretty-printed.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 70aa4167c2..659aa1a242 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9445,6 +9445,84 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
{.name = NULL}
};
+
+static char *
+cmdQemuMonitorCommandConcatCmd(vshControl *ctl,
+ const vshCmd *cmd,
+ const vshCmdOpt *opt)
+{
+ g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+ while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+ virBufferAsprintf(&buf, "%s ", opt->data);
+
+ virBufferTrim(&buf, " ");
+
+ return virBufferContentAndReset(&buf);
+}
+
+
+static char *
+cmdQemuMonitorCommandQMPWrap(vshControl *ctl,
+ const vshCmd *cmd)
+{
+ g_autofree char *fullcmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
+ g_autoptr(virJSONValue) fullcmdjson = virJSONValueFromString(fullcmd);
+ g_autofree char *fullargs = NULL;
+ g_autoptr(virJSONValue) fullargsjson = NULL;
+ const vshCmdOpt *opt = NULL;
+ const char *commandname = NULL;
+ g_autoptr(virJSONValue) command = NULL;
+ g_autoptr(virJSONValue) arguments = NULL;
+
+ /* if we've got a JSON object, pass it through */
+ if (virJSONValueIsObject(fullcmdjson))
+ return g_steal_pointer(&fullcmd);
+
+ /* we try to wrap the command and possible arguments into a JSON object, if
+ * we as fall back we pass through what we've got from the user */
+
+ if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+ commandname = opt->data;
+
+ /* now we process arguments similarly to how we've dealt with the full command */
+ if ((fullargs = cmdQemuMonitorCommandConcatCmd(ctl, cmd, opt)))
+ fullargsjson = virJSONValueFromString(fullargs);
+
+ /* for empty args or a valid JSON object we just use that */
+ if (!fullargs || virJSONValueIsObject(fullargsjson)) {
+ arguments = g_steal_pointer(&fullargsjson);
+ } else {
+ /* for a non-object we try to concatenate individual _ARGV bits into a
+ * JSON object wrapper and try using that */
+ g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAddLit(&buf, "{");
+ /* opt points to the _ARGV option bit containing the command so we'll
+ * iterate through the arguments now */
+ while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+ virBufferAsprintf(&buf, "%s,", opt->data);
+
+ virBufferTrim(&buf, ",");
+ virBufferAddLit(&buf, "}");
+
+ if (!(arguments = virJSONValueFromString(virBufferCurrentContent(&buf)))) {
+ vshError(ctl, _("failed to wrap arguments '%s' into a QMP command wrapper"),
+ fullargs);
+ return NULL;
+ }
+ }
+
+ if (virJSONValueObjectCreate(&command,
+ "s:execute", commandname,
+ "A:arguments", &arguments,
+ NULL) < 0)
+ return NULL;
+
+ return virJSONValueToString(command, false);
+}
+
+
static bool
cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
{
@@ -9453,8 +9531,6 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
g_autofree char *result = NULL;
g_autoptr(virJSONValue) resultjson = NULL;
unsigned int flags = 0;
- const vshCmdOpt *opt = NULL;
- g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
bool pretty = vshCommandOptBool(cmd, "pretty");
bool returnval = vshCommandOptBool(cmd, "return-value");
virJSONValue *formatjson;
@@ -9466,15 +9542,17 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
- while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
- virBufferAsprintf(&buf, "%s ", opt->data);
-
- virBufferTrim(&buf, " ");
-
- monitor_cmd = virBufferContentAndReset(&buf);
-
- if (vshCommandOptBool(cmd, "hmp"))
+ if (vshCommandOptBool(cmd, "hmp")) {
flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP;
+ monitor_cmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
+ } else {
+ monitor_cmd = cmdQemuMonitorCommandQMPWrap(ctl, cmd);
+ }
+
+ if (!monitor_cmd) {
+ vshSaveLibvirtError();
+ return NULL;
+ }
if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0)
return false;
--
2.31.1
3 years, 1 month
