April 2023 - Devel - libvirt List Archives

[libvirt PATCH] network: do not assume dnsmasq in networkUpdateState
by Ján Tomko 17 Apr '23

17 Apr '23

If we don't have dnsmasq, it's pointless to try to find its pidfile. Also, dnsmasqCapsGetBinaryPath would access a NULL pointer. Fixes: 4b68c982e283471575bacbf87302495864da46fe Foxes: https://gitlab.com/libvirt/libvirt/-/issues/456 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/network/bridge_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3fa56bfc09..ee4bbd4a93 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -492,7 +492,7 @@ networkUpdateState(virNetworkObj *obj, virNetworkObjPortForEach(obj, networkUpdatePort, obj); /* Try and read dnsmasq pids of active networks */ - if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { + if (dnsmasq_caps && virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { pid_t dnsmasqPid; if (networkSetMacMap(cfg, obj) < 0) -- 2.39.2

3 5

[libvirt] [RFC] support vhost-user-scsi configuration
by wangjian (AN) 17 Apr '23

17 Apr '23

Hi Guys, Currently qemu and spdk already support vhost-user-scsi, but there is no vhost-user-scsi configuration in libvirt. We hope that libvirt supports the following configurations to facilitate docking with qemu. <controller type='scsi' index='0' model='vhost-user-scsi'> <driver queues='4'/> <source type='unix' path='/var/tmp/scsi'> <reconnect enabled='yes' timeout='3'/> </source> <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> </controller> The usage in qemu like this: -chardev socket,id=chr-vu-virtio-disk10,path=/var/tmp/scsi,reconnect=3 -device vhost-user-blk-pci,num-queues=4,bus=pci.2,addr=0x0,chardev=chr-vu-virtio-disk10,id=scsi0 Could anyone give some suggestions?

2 1

[PATCH v2] qemu: Fix domxml-to-native command failure
by Zhenzhong Duan 17 Apr '23

17 Apr '23

virsh command domxml-to-native failed with below error but start command succeed for same domain xml. "internal error: invalid PCI passthrough type 'default'" If host PCI device's driver attribute isn't defined in domain xml, qemu driver will choose a proper value based on host environment before starting qemu process. But this is missed in domxml-to-native command, add the same logic so domxml-to-native could pass and replace the relevant code in qemuHostdevPreparePCIDevicesCheckSupport() with an error report. Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com> --- v2: replace setting with an error report, suggested by Michal src/qemu/qemu_domain.c | 11 +++++++++++ src/qemu/qemu_hostdev.c | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63b13b6875c9..517c90f93f0a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11274,6 +11274,17 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, } } } + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; + + if (*backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT && + supportsPassthroughVFIO && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } + } return 0; } diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 45cd1066f0a5..305a323a0769 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -175,15 +175,15 @@ qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (supportsPassthroughVFIO && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemuDomainPrepareHostdev() should have set backend " + "to VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO")); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("host doesn't support passthrough of " "host PCI devices")); - return false; } - - break; + return false; case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: if (!supportsPassthroughVFIO) { -- 2.25.1

3 2

Improve default machine type selection
by Jim Fehlig 15 Apr '23

15 Apr '23

If an explicit machine type is not specified in the VM config, the qemu driver will select the first machine type in the list of machine types for the specified accelerator. See virQEMUCapsGetPreferredMachine https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_capabilities… On my test machines, this works reasonably well for x86_64 where the first machine type is pc-i440fx-7.1. But for aarch64, the first machine is integratorcp, which is not very useful with maxCpus=1 and other limitations. Would it be possible to select a "better" default machine type? E.g. 'pc' for x86_64, 'virt' for aarch64, etc. I'm happy to work on this if folks deem it's a reasonable improvement. It would benefit tools like the libvirt terraform provider, which prefer to use something more generic like <type>hvm</type>. Regards, Jim

4 18

[PATCH] qemu: Fix incorrect command name in error messages
by Andrea Bolognani 14 Apr '23

14 Apr '23

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Pushed as trivial. src/qemu/qemu_monitor_json.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3b42b28fa1..b2c0b20a11 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4813,7 +4813,7 @@ int qemuMonitorJSONGetMachines(qemuMonitor *mon, if (virJSONValueObjectHasKey(child, "numa-mem-supported")) { if (virJSONValueObjectGetBoolean(child, "numa-mem-supported", &info->numaMemSupported) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu-machines reply has malformed " + _("query-machines reply has malformed " "'numa-mem-supported' data")); goto cleanup; } @@ -4841,7 +4841,7 @@ int qemuMonitorJSONGetMachines(qemuMonitor *mon, if (virJSONValueObjectGetBoolean(child, "acpi", &acpi) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu-machines reply has malformed 'acpi data")); + _("query-machines reply has malformed 'acpi data")); goto cleanup; } -- 2.39.2

1 0

[PATCH v2 00/10] Deprecate/rename singlestep command line option, monitor interfaces
by Peter Maydell 14 Apr '23

14 Apr '23

The command line option '-singlestep' and its HMP equivalent the 'singlestep' command are very confusingly named, because they have nothing to do with single-stepping the guest (either via the gdb stub or by emulation of guest CPU architectural debug facilities). What they actually do is put TCG into a mode where it puts only one guest instruction into each translation block. This is useful for some circumstances such as when you want the -d debug logging to be easier to interpret, or if you have a finicky guest binary that wants to see interrupts delivered at something other than the end of a basic block. The confusing name is made worse by the fact that our documentation for these is so minimal as to be useless for telling users what they really do. This series: * changes the command line interface: for user-mode emulators, the new option is '-one-insn-per-tb', and for system mode emulators it is a TCG accel property '-accel tcg,one-insn-per-tb=on' * updates all the places which currently directly touch the 'singlestep' global variable to instead get the current accelerator and query/set the QOM property * documents that the old -singlestep option is deprecated * adds a new HMP command 'one-insn-per-tb', and deprecates the old 'singlestep' command. (Strictly we don't need to deprecate HMP commands, but I'd already written the code for v1 of this series and it's a minor user convenience.) * makes the HMP 'info status' output report "one insn per TB" instead of "single step mode" * adds a new 'one-insn-per-tb' member to the QMP StatusInfo type, and deprecates the old 'singlestep' field Notes: * I hope I have got the QMP changes and deprecation right, but that's probably the bit in most need of review from an expert * There's an argument for just dropping the reporting of one-insn-per-tb in QMP StatusInfo at all, except that (a) it's hard to know if anybody's really using it (b) then the info isn't reported to HMP 'info status', which wouldn't line up with HMP having a mechanism to get/set the value * I have written patch 3 on the assumption that curr_cflags() is not such a hot codepath that we can't afford to have a QOM cast macro in it; the alternative would be to keep it using a global variable but make the global be restricted to accel/tcg/internals.h. RTH: opinions welcome... * Still haven't tested on bsd-user, but the patch is identical to the linux-user change thanks -- PMM Peter Maydell (10): make one-insn-per-tb an accel option softmmu: Don't use 'singlestep' global in QMP and HMP commands tcg: Use one-insn-per-tb accelerator property in curr_cflags() linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep' bsd-user: Add '-one-insn-per-tb' option equivalent to '-singlestep' Document that -singlestep command line option is deprecated hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep' hmp: Report 'one-insn-per-tb', not 'single step mode', in 'info status' output qapi/run-state.json: Fix missing newline at end of file hmp: Deprecate 'singlestep' member of StatusInfo docs/about/deprecated.rst | 35 +++++++++++++++++++++++++++++++++++ docs/interop/qmp-intro.txt | 1 + docs/user/main.rst | 14 ++++++++++++-- qapi/run-state.json | 19 +++++++++++++++---- accel/tcg/internal.h | 16 ++++++++++++++++ include/exec/cpu-common.h | 3 --- include/monitor/hmp.h | 2 +- accel/tcg/cpu-exec.c | 5 +++-- accel/tcg/tcg-all.c | 32 ++++++++++++++++++-------------- bsd-user/main.c | 14 +++++++++----- linux-user/main.c | 18 ++++++++++++------ softmmu/globals.c | 1 - softmmu/runstate-hmp-cmds.c | 22 ++++++++++++++++++---- softmmu/runstate.c | 12 +++++++++++- softmmu/vl.c | 17 +++++++++++++++-- tests/qtest/test-hmp.c | 1 + hmp-commands.hx | 25 +++++++++++++++++++++---- qemu-options.hx | 12 ++++++++++-- tcg/tci/README | 2 +- 19 files changed, 199 insertions(+), 52 deletions(-) -- 2.34.1

6 34

[PATCH] security: do not remember/recall labels for VFIO MDEVs
by Eric Farman 13 Apr '23

13 Apr '23

Commit dbf1f68410 ("security: do not remember/recall labels for VFIO") rightly changed the DAC and SELinux labeling parameters to fix a problem with "VFIO hostdevs" but really only addressed the PCI codepaths. As a result, we can still encounter this with VFIO MDEVs such as vfio-ccw and vfio-ap, which can fail on a hotplug: [test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-eff72715c4f0 [test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-eff72715c4f0 [test@host ~]# cat disk.xml <hostdev mode='subsystem' type='mdev' model='vfio-ccw'> <source> <address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/> </source> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/> </hostdev> [test@host ~]# virsh attach-device guest ~/disk.xml error: Failed to attach device from /home/test/disk.xml error: Requested operation is not valid: Setting different SELinux label on /dev/vfio/3 which is already in use Make the same changes as reported in commit dbf1f68410, for the mdev paths. Reported-by: Matthew Rosato <mjrosato(a)linux.ibm.com> Signed-off-by: Eric Farman <farman(a)linux.ibm.com> --- src/security/security_dac.c | 4 ++-- src/security/security_selinux.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9be8f458d1..bceb6a5c24 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1310,7 +1310,7 @@ virSecurityDACSetHostdevLabel(virSecurityManager *mgr, if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) return -1; - ret = virSecurityDACSetHostdevLabelHelper(vfiodev, true, &cbdata); + ret = virSecurityDACSetHostdevLabelHelper(vfiodev, false, &cbdata); break; } @@ -1466,7 +1466,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManager *mgr, if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) return -1; - ret = virSecurityDACRestoreFileLabel(mgr, vfiodev); + ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, vfiodev, false); break; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e43962435f..9c23735aa3 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2217,7 +2217,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManager *mgr, if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) return ret; - ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, true, &data); + ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, false, &data); break; } @@ -2445,7 +2445,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManager *mgr, if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) return -1; - ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, true); + ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, false); break; } -- 2.37.2

2 3

[RFC PATCH 0/8] Translatable string handling improvements
by Peter Krempa 13 Apr '23

13 Apr '23

Many things in libvirt attempt to construct error messages from various sub-strings which is not translation friednly. The beginning of the series refactors the cgroup code to avoid this practice. The end of this series is an example how the worst offender, virsh event handling code, can be improved to avoid the bad practice. I've found most cases using: git grep ' _([^"]' Unfortunateluy the better practice is a bit more verbose, thus RFC. Peter Krempa (8): virCgroupV1GetBlkioIo(Device)Serviced: Refactor extraction of cgroup data Don't translate strings used with VIR_DEBUG virshGraphicsAddressToString: Remove pointless translation vshPrint: Add version using 'va_list' virsh: event: Introduce virshEventPrintf virsh-domain-event: Make 'virshEventTrayChangePrint' translation friendly virsh-domain-event: Make 'virshEventWatchdogPrint' translation friendly virsh-domain-event: Make 'virshEventIOError(Reason)Print' translation friendly src/util/vircgroupv1.c | 264 ++++++++++++++++--------------------- src/util/virnetlink.c | 6 +- src/vz/vz_driver.c | 4 +- tools/virsh-domain-event.c | 223 +++++++++++++++++-------------- tools/vsh.c | 32 +++-- tools/vsh.h | 4 + 6 files changed, 274 insertions(+), 259 deletions(-) -- 2.39.2

3 12

[PATCH] qemu: Fix domxml-to-native command failure
by Zhenzhong Duan 13 Apr '23

13 Apr '23

virsh command domxml-to-native failed with below error but start command suceed for same domain xml. "internal error: invalid PCI passthrough type 'default'" If host pci device's driver attribute isn't defined in domain xml, qemu driver will choose a proper value based on host environment before starting qemu process. But this is missed in domxml-to-native command, add the same logic so domxml-to-native could pass. Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com> --- src/qemu/qemu_domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63b13b6875c9..517c90f93f0a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11274,6 +11274,17 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, } } } + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; + + if (*backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT && + supportsPassthroughVFIO && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } + } return 0; } -- 2.25.1

3 2

[PATCH] conf: Introduce igb model for <interface>
by Akihiko Odaki 13 Apr '23

13 Apr '23

igb is a new network device which will be introduced with QEMU 8.0.0. It is a successor of e1000e so it has PCIe interface and is understands virtio-net headers as e1000e does. Signed-off-by: Akihiko Odaki <akihiko.odaki(a)daynix.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_domain_address.c | 3 ++- src/qemu/qemu_interface.c | 1 + 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d16a247a45..25401f742f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -587,6 +587,7 @@ VIR_ENUM_IMPL(virDomainNetModel, "virtio", "e1000", "e1000e", + "igb", "virtio-transitional", "virtio-non-transitional", "usb-net", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9e281692ff..44b070ba0c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -954,6 +954,7 @@ typedef enum { VIR_DOMAIN_NET_MODEL_VIRTIO, VIR_DOMAIN_NET_MODEL_E1000, VIR_DOMAIN_NET_MODEL_E1000E, + VIR_DOMAIN_NET_MODEL_IGB, VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL, VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL, VIR_DOMAIN_NET_MODEL_USB_NET, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b8d1969fbe..9f13f5f195 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -750,7 +750,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL) return pciFlags; - if (net->model == VIR_DOMAIN_NET_MODEL_E1000E) + if (net->model == VIR_DOMAIN_NET_MODEL_E1000E || + net->model == VIR_DOMAIN_NET_MODEL_IGB) return pcieFlags; /* the only time model can be "unknown" is for type='hostdev' diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 264d5e060c..2aa4d53215 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -241,6 +241,7 @@ qemuInterfaceIsVnetCompatModel(const virDomainNetDef *net) { return (virDomainNetIsVirtioModel(net) || net->model == VIR_DOMAIN_NET_MODEL_E1000E || + net->model == VIR_DOMAIN_NET_MODEL_IGB || net->model == VIR_DOMAIN_NET_MODEL_VMXNET3); } -- 2.40.0

3 5