Re: [PATCH 2/2] nbd/server: Allow users to adjust handshake limit in
QMP
by Eric Blake
On Thu, Feb 06, 2025 at 10:20:09AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > ---
> > qapi/block-export.json | 10 ++++++++++
> > include/block/nbd.h | 6 +++---
>
> [..]
>
> > @@ -52,6 +57,10 @@
> > #
> > # @addr: Address on which to listen.
> > #
> > +# @handshake-max-secs: Time limit, in seconds, at which a client that
> > +# has not completed the negotiation handshake will be disconnected,
> > +# or 0 for no limit (since 10.0; default: 10).
> > +#
>
> Hmm. [not about the series], shouldn't we finally deprecate older interface?
By older interface, you are asking about the QMP command
'nbd-server-start' as compared to struct NbdServerOptions. But the
struct is not directly present in any QMP commands; rather, it only
appears to be used by qemu-storage-daemon as one of its command line
options that needs to set up an NBD server with a JSON-like syntax
that has less nesting than QMP nbd-server-start. blockdev-nbd.c has
two functions [nbd_server_start_options(NbdServerOPtions *arg...) and
qmp_nbd_server_start(args...)] that both unpack their slightly
different forms and pass them as parameters to nbd_server_start() that
is then agnostic to whether the older QMP command or newer q-s-d CLI
option was specified.
It looks like libvirt is still using QMP nbd-server-start. If we were
to start the deprecation process for qemu 10.0, what would the new
command look like? What would everyone be required to use by qemu
10.2?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
2 hours, 9 minutes
[PATCH] tests: Record negative tests as pass instead of expected-fail
by Tim Wiederhake
meson's "test()" function provides a "should_fail: bool" argument that
checks for a command to exit with a non-zero exit code instead of the usual
zero exit code to signal success. If the program under test does so, it is
recorded as "EXPECTEDFAIL" instead of "OK". While there is an argument to be
made that the program under test failed as expected, the test in itself is
successful and should be recorded as such.
Before:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config EXPECTEDFAIL 0.03s exit status 1
...
Ok: 299
Expected Fail: 1
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
After:
$ meson test
...
151/300 libvirt:bin / libvirtd fail with missing config OK 0.03s
...
Ok: 300
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
tests/expected-fail | 13 +++++++++++++
tests/meson.build | 5 ++---
2 files changed, 15 insertions(+), 3 deletions(-)
create mode 100755 tests/expected-fail
diff --git a/tests/expected-fail b/tests/expected-fail
new file mode 100755
index 0000000000..85738c95dc
--- /dev/null
+++ b/tests/expected-fail
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# meson's "test()" function provides a "should_fail: bool" argument that
+# checks for a command to exit with a non-zero exit code instead of the usual
+# zero exit code to signal success. If the program under test does so, it is
+# recorded as "EXPECTEDFAIL" instead of "OK". While there is an argument to be
+# made that the program under test failed as expected, the test in itself is
+# successful and should be recorded as such.
+
+if "$@"
+then
+ exit 1
+fi
diff --git a/tests/meson.build b/tests/meson.build
index 0d76d37959..0ebcdc1496 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -725,9 +725,8 @@ endif
if conf.has('WITH_LIBVIRTD')
test('libvirtd fail with missing config',
- libvirtd_prog,
- args: [ '--config=no-such-conf', '--timeout=5' ],
- should_fail: true,
+ find_program('expected-fail'),
+ args: [ libvirtd_prog, '--config=no-such-conf', '--timeout=5' ],
suite: 'bin',
)
--
2.48.1
10 hours, 51 minutes
[PATCH v5 00/18] *** qemu: block: Support block disk along with throttle filters ***
by Harikumar R
*** BLURB HERE ***
Chun Feng Wu (17):
schema: Add new domain elements to support multiple throttle groups
schema: Add new domain elements to support multiple throttle filters
config: Introduce ThrottleGroup and corresponding XML parsing
config: Introduce ThrottleFilter and corresponding XML parsing
qemu: monitor: Add support for ThrottleGroup operations
tests: Test qemuMonitorJSONGetThrottleGroup and
qemuMonitorJSONUpdateThrottleGroup
remote: New APIs for ThrottleGroup lifecycle management
qemu: Refactor qemuDomainSetBlockIoTune to extract common methods
qemu: Implement qemu driver for throttle API
qemu: helper: throttle filter nodename and preparation processing
qemu: block: Support block disk along with throttle filters
config: validate: Verify iotune, throttle group and filter
qemuxmlconftest: Add 'throttlefilter' tests
test_driver: Test throttle group lifecycle APIs
virsh: Refactor iotune options for re-use
virsh: Add support for throttle group operations
virsh: Add option "throttle-groups" to "attach_disk"
Harikumar Rajkumar (1):
tests: Test qemuxmlactivetestThrottleGroup
docs/formatdomain.rst | 47 ++
docs/manpages/virsh.rst | 135 +++-
include/libvirt/libvirt-domain.h | 21 +
src/conf/domain_conf.c | 398 ++++++++++
src/conf/domain_conf.h | 45 ++
src/conf/domain_validate.c | 119 ++-
src/conf/schemas/domaincommon.rng | 293 ++++----
src/conf/virconftypes.h | 4 +
src/driver-hypervisor.h | 22 +
src/libvirt-domain.c | 174 +++++
src/libvirt_private.syms | 8 +
src/libvirt_public.syms | 7 +
src/qemu/qemu_block.c | 136 ++++
src/qemu/qemu_block.h | 49 ++
src/qemu/qemu_command.c | 180 +++++
src/qemu/qemu_command.h | 6 +
src/qemu/qemu_domain.c | 73 +-
src/qemu/qemu_driver.c | 619 +++++++++++++---
src/qemu/qemu_hotplug.c | 33 +
src/qemu/qemu_monitor.c | 34 +
src/qemu/qemu_monitor.h | 14 +
src/qemu/qemu_monitor_json.c | 134 ++++
src/qemu/qemu_monitor_json.h | 14 +
src/remote/remote_daemon_dispatch.c | 44 ++
src/remote/remote_driver.c | 40 ++
src/remote/remote_protocol.x | 48 +-
src/remote_protocol-structs | 28 +
src/test/test_driver.c | 452 ++++++++----
tests/qemumonitorjsontest.c | 86 +++
.../throttlefilter-in.xml | 392 ++++++++++
.../throttlefilter-out.xml | 393 ++++++++++
tests/qemuxmlactivetest.c | 1 +
.../throttlefilter-invalid.x86_64-latest.err | 1 +
.../throttlefilter-invalid.xml | 89 +++
.../throttlefilter.x86_64-latest.args | 55 ++
.../throttlefilter.x86_64-latest.xml | 105 +++
tests/qemuxmlconfdata/throttlefilter.xml | 95 +++
tests/qemuxmlconftest.c | 2 +
tools/virsh-completer-domain.c | 82 +++
tools/virsh-completer-domain.h | 16 +
tools/virsh-domain.c | 680 ++++++++++++++----
41 files changed, 4649 insertions(+), 525 deletions(-)
create mode 100644 tests/qemustatusxml2xmldata/throttlefilter-in.xml
create mode 100644 tests/qemustatusxml2xmldata/throttlefilter-out.xml
create mode 100644 tests/qemuxmlconfdata/throttlefilter-invalid.x86_64-latest.err
create mode 100644 tests/qemuxmlconfdata/throttlefilter-invalid.xml
create mode 100644 tests/qemuxmlconfdata/throttlefilter.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/throttlefilter.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/throttlefilter.xml
--
2.39.5 (Apple Git-154)
12 hours, 38 minutes
[PATCH V3 00/19] qemu: support mapped-ram+directio+mulitfd
by Jim Fehlig
V3 series adding support for QEMU's mapped-ram stream format [1] and
migration capability. The use of mapped-ram is controlled by extending
save_image_format setting in qemu.conf with a new 'sparse' option. The
'raw' format continues to be the default.
Also included are patches that leverage mapped-ram to add support for
parallel save/restore.
In one of the previous threads on this topic we discussed per-VM control
of save image format by adding a 'format' parameter to virDomainSaveParams.
I can do that as a followup series if folks agree its useful.
Changes in V3:
* Drop patch adding qemuFDPassGetId
* Drop now upstream patch decomposing qemuSaveImageOpen
* Add patch to get an FDPass from fdset
* If qemu is still running after save, remove fdset
V1:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/M...
V2:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/G...
[1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/m...
Claudio Fontana (3):
include: Define constants for parallel save/restore
tools: add parallel parameter to virsh save command
tools: add parallel parameter to virsh restore command
Jim Fehlig (16):
lib: virDomain{Save,Restore}Params: Ensure absolute path
qemu: Add function to get FDPass object from monitor
qemu: Add function to check capability in migration params
qemu: Add function to get bool value from migration params
qemu: Add mapped-ram migration capability
qemu: Add function to get migration params for save
qemu_saveimage: add "sparse" to supported save image formats
qemu: Add helper function for creating save image fd
qemu: Move declaration of virQEMUSaveFormat to header file
qemu: Add support for mapped-ram on save
qemu: Move creation of qemuProcessIncomingDef struct
qemu: Apply migration parameters in qemuMigrationDstRun
qemu: Add support for mapped-ram on restore
qemu: Support O_DIRECT with mapped-ram on save
qemu: Support O_DIRECT with mapped-ram on restore
qemu: Add support for parallel save and restore
docs/manpages/virsh.rst | 21 +++-
include/libvirt/libvirt-domain.h | 11 ++
src/libvirt-domain.c | 95 +++++++++++---
src/qemu/qemu.conf.in | 9 +-
src/qemu/qemu_driver.c | 78 +++++++++---
src/qemu/qemu_fd.c | 46 +++++++
src/qemu/qemu_fd.h | 4 +
src/qemu/qemu_migration.c | 204 ++++++++++++++++++++++---------
src/qemu/qemu_migration.h | 10 +-
src/qemu/qemu_migration_params.c | 92 ++++++++++++++
src/qemu/qemu_migration_params.h | 17 +++
src/qemu/qemu_monitor.c | 37 ++++++
src/qemu/qemu_monitor.h | 5 +
src/qemu/qemu_process.c | 100 ++++++++++-----
src/qemu/qemu_process.h | 19 ++-
src/qemu/qemu_saveimage.c | 141 ++++++++++++---------
src/qemu/qemu_saveimage.h | 23 ++++
src/qemu/qemu_snapshot.c | 22 +++-
tools/virsh-domain.c | 81 ++++++++++--
19 files changed, 805 insertions(+), 210 deletions(-)
--
2.43.0
3 days, 5 hours
[PATCH] qemu: snapshot: Limit scope of checkpoint-snapshot interlock
by Peter Krempa
'qemuDomainSupportsCheckpointsBlockjobs()' should really be used only
with active VMs based on the scope of interlocking it does.
This means that the inactive snapshot code path needs to do the
interlocking based on what's supported:
- external snapshot support was not implemented yet
(bitmaps need to be propagated to the new overlay image)
- internal snapshot support can be deferred to qemu
Move the check inside qemuSnapshotPrepare() which has knowledge about
the snapshot type and implement an explicit check for the inactive case.
See: https://gitlab.com/libvirt/libvirt/-/issues/739
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 7ce018b026..ecf8eb3cca 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -39,6 +39,7 @@
#include "domain_audit.h"
#include "locking/domain_lock.h"
#include "virdomainsnapshotobjlist.h"
+#include "virdomaincheckpointobjlist.h"
#include "virqemu.h"
#include "storage_source.h"
@@ -1067,6 +1068,23 @@ qemuSnapshotPrepare(virDomainObj *vm,
}
+ /* Handle interlocking with 'checkpoints':
+ * - if the VM is online use qemuDomainSupportsCheckpointsBlockjobs
+ * - if the VM is offline disallow external snapshots as the support for
+ * propagating bitmaps into the would-be-created overlay is not yet implemented
+ */
+ if (!active) {
+ if (external &&
+ virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("support for offline external snapshots while checkpoint exists was not yet implemented"));
+ return -1;
+ }
+ } else {
+ if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
+ return -1;
+ }
+
/* Alter flags to let later users know what we learned. */
if (external && !active)
*flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
@@ -2134,9 +2152,6 @@ qemuSnapshotCreateXML(virDomainPtr domain,
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
NULL);
- if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
- return NULL;
-
if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot halt after transient domain snapshot"));
--
2.48.1
3 days, 6 hours
[PATCH] node_device: Do not lock the driver state needlessly
by Martin Kletzander
When processing the PCI devices we can only read the configs for each of
them if running as privileged. That information is saved in the driver
state as a boolean introduced in commit 643c74abff01. However since
that version it is only written to once during nodeStateInitialize() and
only read from that point (apart from some commits around v3.9.0 release
when it was not even set, but that was fixed before v3.10.0). And it is
only read once, just to store that boolean in a temporary variable which
is also used in only one condition.
Rewrite this without locking and save few lines of code.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/node_device/node_device_udev.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 7b4ff80f8fb4..be5ee0108c85 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -430,7 +430,6 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state,
g_autofree char *linkpath = NULL;
int ret = -1;
char *p;
- bool privileged = false;
linkpath = g_strdup_printf("%s/config", udev_device_get_syspath(device));
if (virFileWaitForExists(linkpath, 10, 100) < 0) {
@@ -440,10 +439,6 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state,
goto cleanup;
}
- VIR_WITH_MUTEX_LOCK_GUARD(&driver_state->lock) {
- privileged = driver_state->privileged;
- }
-
pci_dev->klass = -1;
if (udevGetIntProperty(device, "PCI_CLASS", &pci_dev->klass, 16) < 0)
goto cleanup;
@@ -491,7 +486,7 @@ udevProcessPCI(virNodeDeviceDriverState *driver_state,
goto cleanup;
/* We need to be root to read PCI device configs */
- if (privileged) {
+ if (driver_state->privileged) {
if (virPCIGetHeaderType(pciDev, &pci_dev->hdrType) < 0)
goto cleanup;
--
2.48.1
3 days, 8 hours
[PATCH] cpu_map: Add missing feature "bhi-no"
by Tim Wiederhake
Introduced in qemu commit b611931d4f70b9a3e49e39c405c63b3b5e9c0df1.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/cpu_map/x86_features.xml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
index d06d60e230..08519a37bf 100644
--- a/src/cpu_map/x86_features.xml
+++ b/src/cpu_map/x86_features.xml
@@ -873,6 +873,9 @@
<feature name='fb-clear'>
<msr index='0x0000010a' edx='0x00000000' eax='0x00020000'/>
</feature>
+ <feature name='bhi-no'>
+ <msr index='0x0000010a' edx='0x00000000' eax='0x00100000'/>
+ </feature>
<feature name='pbrsb-no'>
<msr index='0x0000010a' edx='0x00000000' eax='0x01000000'/>
</feature>
--
2.48.1
3 days, 9 hours
Support for: VIR_DOMAIN_EVENT_ID_DBGTOOLS.
by Martin Harvey
Probably a FAQ.
I have implemented a prototype:
*
QEMU event for an in-guest issue (tools installation / crash etc), via fwcfg / vmcore (similar). (QEMU_PROCESS_EVENT_DBGTOOLS_CHANGED).
*
Qemu Process / QemuMonitor handling for same event.
*
Mapped this through to VIR_DOMAIN_EVENT_ID_DBG_TOOLS,
*
Corresponding changes to top level event handling.
We would find this *very useful* in cases where vm migration, debugging and the like require co-operation with guest user-mode.
Are you folks happy to countenance some simple, but top-level, event handling changes triggered by guest software, or is this considered a no no? Should I be burying the details somewhere in EVENT_TUNABLE instead?
Patchsets also available ...
MH.
3 days, 9 hours
[PATCH] node_device_udev: add error reporting to udevProcessCCWGroup
by Boris Fiuczynski
Add reporting an internal error when the string to type conversion of
devtype fails as this indicates a serious problem since devtype was used
to get into this method during the udev event handling.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/node_device/node_device_udev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 7b4ff80f8f..344b39e97a 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1415,8 +1415,12 @@ udevProcessCCWGroup(struct udev_device *device,
udevGenerateDeviceName(device, def, NULL);
- if ((tmp = virNodeDevCCWGroupCapTypeFromString(devtype)) < 0)
+ if ((tmp = virNodeDevCCWGroupCapTypeFromString(devtype)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to find a supported CCW group capability type '%1$s'"),
+ devtype);
return -1;
+ }
data->ccwgroup_dev.type = tmp;
--
2.47.0
3 days, 9 hours
[PATCH 0/3] qemu: fix setup of FD passed disks
by Peter Krempa
See 3/3
Peter Krempa (3):
qemuxmlconftest: Allow testing of the 'writable' flag for passed FDs
for disks
qemuxmlconftest: Add testing of FDs with 'writable' flag in
'disk-source-fd'
qemu: domain: Initialize FD passthrough for a virStorageSource before
using it
src/qemu/qemu_domain.c | 6 ++--
.../disk-source-fd.x86_64-latest.args | 36 +++++++++++--------
.../disk-source-fd.x86_64-latest.xml | 21 +++++++++--
tests/qemuxmlconfdata/disk-source-fd.xml | 16 +++++++--
tests/qemuxmlconftest.c | 9 +++--
tests/testutilsqemu.c | 2 ++
tests/testutilsqemu.h | 2 +-
7 files changed, 67 insertions(+), 25 deletions(-)
--
2.48.1
3 days, 11 hours