- Devel - libvirt List Archives

[libvirt PATCH] remove HAL node device driver
by Pavel Hrdina 09 Sep '20

09 Sep '20

There was one attempt a year ago done by me to drop HAL [1] but it was never resolved. There was another time when Dan suggested to drop HAL driver [2] but it was decided to keep it around in case device assignment will be implemented for FreeBSD and the fact that virt-manager uses node device driver [3]. I checked git history and code and it doesn't look like bhyve supports device assignment so from that POV it should not block removing HAL. The argument about virt-manager is not strong as well because libvirt installed from FreeBSD packages doesn't have HAL support so it will not affect these users as well [4]. The only users affected by this change would be the ones compiling libvirt from GIT on FreeBSD. I looked into alternatives and there is libudev-devd package on FreeBSD but unfortunately it doesn't work as it doesn't list any devices when used with libvirt. It provides libudev APIs using devd. I also looked into devd directly and it provides some APIs but there are no APIs for device monitoring and events so that would have to be somehow done by libvirt. Main motivation for dropping HAL support is to replace libdbus with GLib dbus implementation and it cannot be done with HAL driver present in libvirt because HAL APIs heavily depends on symbols provided by libdbus. [1] <https://www.redhat.com/archives/libvir-list/2019-May/msg00203.html> [2] <https://www.redhat.com/archives/libvir-list/2016-April/msg00992.html> [3] <https://www.redhat.com/archives/libvir-list/2016-April/msg00994.html> [4] <https://svnweb.freebsd.org/ports/head/devel/libvirt/Makefile?view=markup> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- meson.build | 9 +- meson_options.txt | 1 - src/node_device/meson.build | 5 - src/node_device/node_device_driver.c | 10 +- src/node_device/node_device_driver.h | 5 - src/node_device/node_device_hal.c | 843 --------------------------- src/node_device/node_device_hal.h | 22 - 7 files changed, 3 insertions(+), 892 deletions(-) delete mode 100644 src/node_device/node_device_hal.c delete mode 100644 src/node_device/node_device_hal.h diff --git a/meson.build b/meson.build index 1aad385ad1..d9c91d88dd 100644 --- a/meson.build +++ b/meson.build @@ -1079,12 +1079,6 @@ glusterfs_dep = dependency('glusterfs-api', version: '>=' + glusterfs_version, r gnutls_version = '3.2.0' gnutls_dep = dependency('gnutls', version: '>=' + gnutls_version) -hal_version = '0.5.0' -hal_dep = dependency('hal', version: '>=' + hal_version, required: get_option('hal')) -if hal_dep.found() - conf.set('WITH_HAL', 1) -endif - # Check for BSD kvm (kernel memory interface) if host_machine.system() == 'freebsd' kvm_dep = cc.find_library('kvm') @@ -1728,7 +1722,7 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') and conf.set('WITH_NETWORK', 1) endif -if hal_dep.found() or udev_dep.found() +if udev_dep.found() conf.set('WITH_NODE_DEVICES', 1) endif @@ -2433,7 +2427,6 @@ libs_summary = { 'glib_dep': glib_dep.found(), 'glusterfs': glusterfs_dep.found(), 'gnutls': gnutls_dep.found(), - 'hal': hal_dep.found(), 'libiscsi': libiscsi_dep.found(), 'libnl': libnl_dep.found(), 'libpcap': libpcap_dep.found(), diff --git a/meson_options.txt b/meson_options.txt index 7838630c1e..c8886e1430 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -23,7 +23,6 @@ option('firewalld', type: 'feature', value: 'auto', description: 'firewalld supp option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') option('fuse', type: 'feature', value: 'auto', description: 'fuse support') option('glusterfs', type: 'feature', value: 'auto', description: 'glusterfs support') -option('hal', type: 'feature', value: 'auto', description: 'hal support') option('libiscsi', type: 'feature', value: 'auto', description: 'libiscsi support') option('libpcap', type: 'feature', value: 'auto', description: 'libpcap support') option('libssh', type: 'feature', value: 'auto', description: 'libssh support') diff --git a/src/node_device/meson.build b/src/node_device/meson.build index 5953c6b8ed..c4e4c3906b 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -4,10 +4,6 @@ node_device_driver_sources = [ stateful_driver_source_files += files(node_device_driver_sources) -if conf.has('WITH_HAL') - node_device_driver_sources += 'node_device_hal.c' -endif - if conf.has('WITH_UDEV') node_device_driver_sources += 'node_device_udev.c' endif @@ -22,7 +18,6 @@ if conf.has('WITH_NODE_DEVICES') ], dependencies: [ access_dep, - hal_dep, libnl_dep, pciaccess_dep, src_dep, diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index e89c8b0ee5..d5f5611361 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -35,7 +35,6 @@ #include "node_device_conf.h" #include "node_device_event.h" #include "node_device_driver.h" -#include "node_device_hal.h" #include "node_device_util.h" #include "virvhba.h" #include "viraccessapicheck.h" @@ -97,14 +96,13 @@ int nodeConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) return 1; } -#if defined (__linux__) && ( defined (WITH_HAL) || defined(WITH_UDEV)) +#if defined (__linux__) && defined(WITH_UDEV) /* NB: It was previously believed that changes in driver name were * relayed to libvirt as "change" events by udev, and the udev event * notification is setup to recognize such events and effectively * recreate the device entry in the cache. However, neither the kernel * nor udev sends such an event, so it is necessary to manually update - * the driver name for a device each time its entry is used, both for - * udev *and* HAL backends. + * the driver name for a device each time its entry is used. */ static int nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def) @@ -925,9 +923,5 @@ nodedevRegister(void) { #ifdef WITH_UDEV return udevNodeRegister(); -#else -# ifdef WITH_HAL - return halNodeRegister(); -# endif #endif } diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index be5d397828..2113d2b0a5 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -28,11 +28,6 @@ #define LINUX_NEW_DEVICE_WAIT_TIME 60 -#ifdef WITH_HAL -int -halNodeRegister(void); -#endif - #ifdef WITH_UDEV int udevNodeRegister(void); diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c deleted file mode 100644 index 53a49ba2aa..0000000000 --- a/src/node_device/node_device_hal.c +++ /dev/null @@ -1,843 +0,0 @@ -/* - * node_device_hal.c: node device enumeration - HAL-based implementation - * - * Copyright (C) 2011-2015 Red Hat, Inc. - * Copyright (C) 2008 Virtual Iron Software, Inc. - * Copyright (C) 2008 David F. Lively - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - */ - -#include <config.h> - -#include <libhal.h> - -#include "node_device_conf.h" -#include "node_device_driver.h" -#include "node_device_hal.h" -#include "virerror.h" -#include "driver.h" -#include "datatypes.h" -#include "viralloc.h" -#include "viruuid.h" -#include "virfile.h" -#include "virpci.h" -#include "virpidfile.h" -#include "virlog.h" -#include "virdbus.h" -#include "virstring.h" -#include "virutil.h" - -#include "configmake.h" - -#define VIR_FROM_THIS VIR_FROM_NODEDEV - -VIR_LOG_INIT("node_device.node_device_hal"); - -/* - * Host device enumeration (HAL implementation) - */ - -#define DRV_STATE_HAL_CTX(ds) ((LibHalContext *)((ds)->privateData)) - - -static const char * -hal_name(const char *udi) -{ - const char *name = strrchr(udi, '/'); - if (name) - return name + 1; - return udi; -} - - -static int -get_str_prop(LibHalContext *ctxt, const char *udi, - const char *prop, char **val_p) -{ - char *val = libhal_device_get_property_string(ctxt, udi, prop, NULL); - - if (val) { - if (*val) { - *val_p = val; - return 0; - } else { - /* Treat empty strings as NULL values */ - VIR_FREE(val); - } - } - - return -1; -} - -static int -get_int_prop(LibHalContext *ctxt, const char *udi, - const char *prop, int *val_p) -{ - DBusError err; - int val; - int rv; - - dbus_error_init(&err); - val = libhal_device_get_property_int(ctxt, udi, prop, &err); - rv = dbus_error_is_set(&err); - dbus_error_free(&err); - if (rv == 0) - *val_p = val; - - return rv; -} - -static int -get_bool_prop(LibHalContext *ctxt, const char *udi, - const char *prop, int *val_p) -{ - DBusError err; - int val; - int rv; - - dbus_error_init(&err); - val = libhal_device_get_property_bool(ctxt, udi, prop, &err); - rv = dbus_error_is_set(&err); - dbus_error_free(&err); - if (rv == 0) - *val_p = val; - - return rv; -} - -static int -get_uint64_prop(LibHalContext *ctxt, const char *udi, - const char *prop, unsigned long long *val_p) -{ - DBusError err; - unsigned long long val; - int rv; - - dbus_error_init(&err); - val = libhal_device_get_property_uint64(ctxt, udi, prop, &err); - rv = dbus_error_is_set(&err); - dbus_error_free(&err); - if (rv == 0) - *val_p = val; - - return rv; -} - -static int -gather_pci_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - char *sysfs_path; - - if (get_str_prop(ctx, udi, "pci.linux.sysfs_path", &sysfs_path) == 0) { - char *p = strrchr(sysfs_path, '/'); - if (p) { - ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.domain)); - ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.bus)); - ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.slot)); - ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.function)); - } - - if (virNodeDeviceGetPCIDynamicCaps(sysfs_path, &d->pci_dev) < 0) { - VIR_FREE(sysfs_path); - return -1; - } - VIR_FREE(sysfs_path); - } - - (void)get_int_prop(ctx, udi, "pci.vendor_id", (int *)&d->pci_dev.vendor); - if (get_str_prop(ctx, udi, "pci.vendor", &d->pci_dev.vendor_name) != 0) - (void)get_str_prop(ctx, udi, "info.vendor", &d->pci_dev.vendor_name); - (void)get_int_prop(ctx, udi, "pci.product_id", (int *)&d->pci_dev.product); - if (get_str_prop(ctx, udi, "pci.product", &d->pci_dev.product_name) != 0) - (void)get_str_prop(ctx, udi, "info.product", &d->pci_dev.product_name); - - return 0; -} - - -static int -gather_usb_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - (void)get_int_prop(ctx, udi, "usb.interface.number", - (int *)&d->usb_if.number); - (void)get_int_prop(ctx, udi, "usb.interface.class", - (int *)&d->usb_if.klass); - (void)get_int_prop(ctx, udi, "usb.interface.subclass", - (int *)&d->usb_if.subclass); - (void)get_int_prop(ctx, udi, "usb.interface.protocol", - (int *)&d->usb_if.protocol); - (void)get_str_prop(ctx, udi, "usb.interface.description", - &d->usb_if.description); - return 0; -} - - -static int -gather_usb_device_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - (void)get_int_prop(ctx, udi, "usb_device.bus_number", - (int *)&d->usb_dev.bus); - (void)get_int_prop(ctx, udi, "usb_device.linux.device_number", - (int *)&d->usb_dev.device); - (void)get_int_prop(ctx, udi, "usb_device.vendor_id", - (int *)&d->usb_dev.vendor); - if (get_str_prop(ctx, udi, "usb_device.vendor", - &d->usb_dev.vendor_name) != 0) - (void)get_str_prop(ctx, udi, "info.vendor", &d->usb_dev.vendor_name); - (void)get_int_prop(ctx, udi, "usb_device.product_id", - (int *)&d->usb_dev.product); - if (get_str_prop(ctx, udi, "usb_device.product", - &d->usb_dev.product_name) != 0) - (void)get_str_prop(ctx, udi, "info.product", &d->usb_dev.product_name); - return 0; -} - - -static int -gather_net_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - unsigned long long dummy; - (void)get_str_prop(ctx, udi, "net.interface", &d->net.ifname); - (void)get_str_prop(ctx, udi, "net.address", &d->net.address); - if (get_uint64_prop(ctx, udi, "net.80203.mac_address", - &dummy) == 0) - d->net.subtype = VIR_NODE_DEV_CAP_NET_80203; - else if (get_uint64_prop(ctx, udi, "net.80211.mac_address", - &dummy) == 0) - d->net.subtype = VIR_NODE_DEV_CAP_NET_80211; - else - d->net.subtype = VIR_NODE_DEV_CAP_NET_LAST; - - return 0; -} - - -static int -gather_scsi_host_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); - - return virNodeDeviceGetSCSIHostCaps(&d->scsi_host); -} - - -static int -gather_scsi_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - (void)get_int_prop(ctx, udi, "scsi.host", (int *)&d->scsi.host); - (void)get_int_prop(ctx, udi, "scsi.bus", (int *)&d->scsi.bus); - (void)get_int_prop(ctx, udi, "scsi.target", (int *)&d->scsi.target); - (void)get_int_prop(ctx, udi, "scsi.lun", (int *)&d->scsi.lun); - (void)get_str_prop(ctx, udi, "scsi.type", &d->scsi.type); - return 0; -} - - -static int -gather_storage_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - int val; - (void)get_str_prop(ctx, udi, "block.device", &d->storage.block); - (void)get_str_prop(ctx, udi, "storage.bus", &d->storage.bus); - (void)get_str_prop(ctx, udi, "storage.drive_type", &d->storage.drive_type); - (void)get_str_prop(ctx, udi, "storage.model", &d->storage.model); - (void)get_str_prop(ctx, udi, "storage.vendor", &d->storage.vendor); - (void)get_str_prop(ctx, udi, "storage.serial", &d->storage.serial); - if (get_bool_prop(ctx, udi, "storage.removable", &val) == 0 && val) { - d->storage.flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE; - if (get_bool_prop(ctx, udi, "storage.removable.media_available", - &val) == 0 && val) { - d->storage.flags |= - VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE; - (void)get_uint64_prop(ctx, udi, "storage.removable.media_size", - &d->storage.removable_media_size); - } - } else { - (void)get_uint64_prop(ctx, udi, "storage.size", &d->storage.size); - } - if (get_bool_prop(ctx, udi, "storage.hotpluggable", &val) == 0 && val) - d->storage.flags |= VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE; - return 0; -} - -static int -gather_scsi_generic_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - (void)get_str_prop(ctx, udi, "scsi_generic.device", &d->sg.path); - return 0; -} - - -static int -gather_system_cap(LibHalContext *ctx, const char *udi, - virNodeDevCapDataPtr d) -{ - char *uuidstr; - - (void)get_str_prop(ctx, udi, "system.product", &d->system.product_name); - (void)get_str_prop(ctx, udi, "system.hardware.vendor", - &d->system.hardware.vendor_name); - (void)get_str_prop(ctx, udi, "system.hardware.version", - &d->system.hardware.version); - (void)get_str_prop(ctx, udi, "system.hardware.serial", - &d->system.hardware.serial); - if (get_str_prop(ctx, udi, "system.hardware.uuid", &uuidstr) == 0) { - ignore_value(virUUIDParse(uuidstr, d->system.hardware.uuid)); - VIR_FREE(uuidstr); - } - (void)get_str_prop(ctx, udi, "system.firmware.vendor", - &d->system.firmware.vendor_name); - (void)get_str_prop(ctx, udi, "system.firmware.version", - &d->system.firmware.version); - (void)get_str_prop(ctx, udi, "system.firmware.release_date", - &d->system.firmware.release_date); - return 0; -} - - -struct _caps_tbl_entry { - const char *cap_name; - virNodeDevCapType type; - int (*gather_fn)(LibHalContext *ctx, - const char *udi, - virNodeDevCapDataPtr data); -}; - -typedef struct _caps_tbl_entry caps_tbl_entry; - -static caps_tbl_entry caps_tbl[] = { - { "system", VIR_NODE_DEV_CAP_SYSTEM, gather_system_cap }, - { "pci", VIR_NODE_DEV_CAP_PCI_DEV, gather_pci_cap }, - { "usb", VIR_NODE_DEV_CAP_USB_INTERFACE, gather_usb_cap }, - { "usb_device", VIR_NODE_DEV_CAP_USB_DEV, gather_usb_device_cap }, - { "net", VIR_NODE_DEV_CAP_NET, gather_net_cap }, - { "scsi_host", VIR_NODE_DEV_CAP_SCSI_HOST, gather_scsi_host_cap }, - { "scsi", VIR_NODE_DEV_CAP_SCSI, gather_scsi_cap }, - { "storage", VIR_NODE_DEV_CAP_STORAGE, gather_storage_cap }, - { "scsi_generic", VIR_NODE_DEV_CAP_SCSI_GENERIC, gather_scsi_generic_cap }, -}; - - -/* qsort/bsearch string comparator */ -static int -cmpstringp(const void *p1, const void *p2) -{ - /* from man 3 qsort */ - return strcmp(* (char * const *) p1, * (char * const *) p2); -} - - -static int -gather_capability(LibHalContext *ctx, const char *udi, - const char *cap_name, - virNodeDevCapsDefPtr *caps_p) -{ - caps_tbl_entry *entry; - - entry = bsearch(&cap_name, caps_tbl, G_N_ELEMENTS(caps_tbl), - sizeof(caps_tbl[0]), cmpstringp); - - if (entry) { - virNodeDevCapsDefPtr caps; - if (VIR_ALLOC(caps) < 0) - return ENOMEM; - caps->data.type = entry->type; - if (entry->gather_fn) { - int rv = (*entry->gather_fn)(ctx, udi, &caps->data); - if (rv != 0) { - virNodeDevCapsDefFree(caps); - return rv; - } - } - caps->next = *caps_p; - *caps_p = caps; - } - - return 0; -} - - -static int -gather_capabilities(LibHalContext *ctx, const char *udi, - virNodeDevCapsDefPtr *caps_p) -{ - char *bus_name = NULL; - virNodeDevCapsDefPtr caps = NULL; - char **hal_cap_names = NULL; - int rv; - size_t i; - - if (STREQ(udi, "/org/freedesktop/Hal/devices/computer")) { - rv = gather_capability(ctx, udi, "system", &caps); - if (rv != 0) - goto failure; - } - - if (get_str_prop(ctx, udi, "info.subsystem", &bus_name) == 0 || - get_str_prop(ctx, udi, "linux.subsystem", &bus_name) == 0) { - rv = gather_capability(ctx, udi, bus_name, &caps); - if (rv != 0) - goto failure; - } - - hal_cap_names = libhal_device_get_property_strlist(ctx, udi, - "info.capabilities", - NULL); - if (hal_cap_names) { - for (i = 0; hal_cap_names[i]; i++) { - if (! (bus_name && STREQ(hal_cap_names[i], bus_name))) { - rv = gather_capability(ctx, udi, hal_cap_names[i], &caps); - if (rv != 0) - goto failure; - } - } - for (i = 0; hal_cap_names[i]; i++) - VIR_FREE(hal_cap_names[i]); - VIR_FREE(hal_cap_names); - } - VIR_FREE(bus_name); - - *caps_p = caps; - return 0; - - failure: - VIR_FREE(bus_name); - if (hal_cap_names) { - for (i = 0; hal_cap_names[i]; i++) - VIR_FREE(hal_cap_names[i]); - VIR_FREE(hal_cap_names); - } - while (caps) { - virNodeDevCapsDefPtr next = caps->next; - virNodeDevCapsDefFree(caps); - caps = next; - } - return rv; -} - -static void -dev_create(const char *udi) -{ - LibHalContext *ctx; - char *parent_key = NULL; - virNodeDeviceObjPtr obj = NULL; - virNodeDeviceDefPtr def = NULL; - virNodeDeviceDefPtr objdef; - const char *name = hal_name(udi); - int rv; - char *devicePath = NULL; - - nodeDeviceLock(); - ctx = DRV_STATE_HAL_CTX(driver); - - if (VIR_ALLOC(def) < 0) - goto failure; - - def->name = g_strdup(name); - - if (get_str_prop(ctx, udi, "info.parent", &parent_key) == 0) { - def->parent = g_strdup(hal_name(parent_key)); - VIR_FREE(parent_key); - } - - rv = gather_capabilities(ctx, udi, &def->caps); - if (rv != 0) goto failure; - - if (def->caps == NULL) - goto cleanup; - - /* Some devices don't have a path in sysfs, so ignore failure */ - (void)get_str_prop(ctx, udi, "linux.sysfs_path", &devicePath); - - if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def))) { - VIR_FREE(devicePath); - goto failure; - } - objdef = virNodeDeviceObjGetDef(obj); - - objdef->sysfs_path = devicePath; - - virNodeDeviceObjEndAPI(&obj); - - nodeDeviceUnlock(); - return; - - failure: - VIR_DEBUG("FAILED TO ADD dev %s", name); - cleanup: - virNodeDeviceDefFree(def); - nodeDeviceUnlock(); -} - -static void -dev_refresh(const char *udi) -{ - const char *name = hal_name(udi); - virNodeDeviceObjPtr obj; - - if ((obj = virNodeDeviceObjListFindByName(driver->devs, name))) { - /* Simply "rediscover" device -- incrementally handling changes - * to sub-capabilities (like net.80203) is nasty ... so avoid it. - */ - virNodeDeviceObjListRemove(driver->devs, obj); - virObjectUnref(obj); - dev_create(udi); - } else { - VIR_DEBUG("no device named %s", name); - } -} - -static void -device_added(LibHalContext *ctx G_GNUC_UNUSED, - const char *udi) -{ - VIR_DEBUG("%s", hal_name(udi)); - dev_create(udi); -} - - -static void -device_removed(LibHalContext *ctx G_GNUC_UNUSED, - const char *udi) -{ - const char *name = hal_name(udi); - virNodeDeviceObjPtr obj; - - obj = virNodeDeviceObjListFindByName(driver->devs, name); - VIR_DEBUG("%s", name); - if (obj) - virNodeDeviceObjListRemove(driver->devs, obj); - else - VIR_DEBUG("no device named %s", name); - virObjectUnref(obj); -} - - -static void -device_cap_added(LibHalContext *ctx, - const char *udi, const char *cap) -{ - const char *name = hal_name(udi); - virNodeDeviceObjPtr obj; - virNodeDeviceDefPtr def; - - VIR_DEBUG("%s %s", cap, name); - if ((obj = virNodeDeviceObjListFindByName(driver->devs, name))) { - def = virNodeDeviceObjGetDef(obj); - (void)gather_capability(ctx, udi, cap, &def->caps); - virNodeDeviceObjEndAPI(&obj); - } else { - VIR_DEBUG("no device named %s", name); - } -} - - -static void -device_cap_lost(LibHalContext *ctx G_GNUC_UNUSED, - const char *udi, - const char *cap) -{ - const char *name = hal_name(udi); - VIR_DEBUG("%s %s", cap, name); - - dev_refresh(udi); -} - - -static void -device_prop_modified(LibHalContext *ctx G_GNUC_UNUSED, - const char *udi, - const char *key, - dbus_bool_t is_removed G_GNUC_UNUSED, - dbus_bool_t is_added G_GNUC_UNUSED) -{ - const char *name = hal_name(udi); - VIR_DEBUG("%s %s", name, key); - - dev_refresh(udi); -} - - -static int -nodeStateInitialize(bool privileged G_GNUC_UNUSED, - const char *root, - virStateInhibitCallback callback G_GNUC_UNUSED, - void *opaque G_GNUC_UNUSED) -{ - LibHalContext *hal_ctx = NULL; - char **udi = NULL; - int num_devs; - size_t i; - int ret = VIR_DRV_STATE_INIT_ERROR; - DBusConnection *sysbus; - DBusError err; - - if (root != NULL) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Driver does not support embedded mode")); - return -1; - } - - /* Ensure caps_tbl is sorted by capability name */ - qsort(caps_tbl, G_N_ELEMENTS(caps_tbl), sizeof(caps_tbl[0]), - cmpstringp); - - if (VIR_ALLOC(driver) < 0) - return VIR_DRV_STATE_INIT_ERROR; - - driver->lockFD = -1; - if (virMutexInit(&driver->lock) < 0) { - VIR_FREE(driver); - return VIR_DRV_STATE_INIT_ERROR; - } - - if (virCondInit(&driver->initCond) < 0) { - virReportSystemError(errno, "%s", - _("Unable to initialize condition variable")); - virMutexDestroy(&driver->lock); - VIR_FREE(driver); - return VIR_DRV_STATE_INIT_ERROR; - } - - nodeDeviceLock(); - - if (privileged) { - driver->stateDir = g_strdup_printf("%s/libvirt/nodedev", RUNSTATEDIR); - } else { - g_autofree char *rundir = NULL; - - rundir = virGetUserRuntimeDirectory(); - driver->stateDir = g_strdup_printf("%s/nodedev/run", rundir); - } - - if (virFileMakePathWithMode(driver->stateDir, S_IRWXU) < 0) { - virReportSystemError(errno, _("cannot create state directory '%s'"), - driver->stateDir); - goto failure; - } - - if ((driver->lockFD = - virPidFileAcquire(driver->stateDir, "driver", false, getpid())) < 0) - goto failure; - - if (!(driver->devs = virNodeDeviceObjListNew())) - goto failure; - - dbus_error_init(&err); - if (!(sysbus = virDBusGetSystemBus())) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("DBus not available, disabling HAL driver: %s"), - virGetLastErrorMessage()); - ret = VIR_DRV_STATE_INIT_SKIPPED; - goto failure; - } - - hal_ctx = libhal_ctx_new(); - if (hal_ctx == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libhal_ctx_new returned NULL")); - goto failure; - } - - if (!libhal_ctx_set_dbus_connection(hal_ctx, sysbus)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libhal_ctx_set_dbus_connection failed")); - goto failure; - } - if (!libhal_ctx_init(hal_ctx, &err)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libhal_ctx_init failed, haldaemon is probably " - "not running")); - /* We don't want to show a fatal error here, - otherwise entire libvirtd shuts down when - hald isn't running */ - ret = VIR_DRV_STATE_INIT_SKIPPED; - goto failure; - } - - /* Populate with known devices */ - driver->privateData = hal_ctx; - - /* We need to unlock state now, since setting these callbacks cause - * a dbus RPC call, and while this call is waiting for the reply, - * a signal may already arrive, triggering the callback and thus - * requiring the lock ! - */ - nodeDeviceUnlock(); - - /* Register HAL event callbacks */ - if (!libhal_ctx_set_device_added(hal_ctx, device_added) || - !libhal_ctx_set_device_removed(hal_ctx, device_removed) || - !libhal_ctx_set_device_new_capability(hal_ctx, device_cap_added) || - !libhal_ctx_set_device_lost_capability(hal_ctx, device_cap_lost) || - !libhal_ctx_set_device_property_modified(hal_ctx, device_prop_modified) || - !libhal_device_property_watch_all(hal_ctx, &err)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("setting up HAL callbacks failed")); - goto failure; - } - - udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); - if (udi == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libhal_get_all_devices failed")); - goto failure; - } - for (i = 0; i < num_devs; i++) { - dev_create(udi[i]); - VIR_FREE(udi[i]); - } - VIR_FREE(udi); - - nodeDeviceLock(); - driver->initialized = true; - nodeDeviceUnlock(); - virCondBroadcast(&driver->initCond); - - return VIR_DRV_STATE_INIT_COMPLETE; - - failure: - if (dbus_error_is_set(&err)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s: %s"), err.name, err.message); - dbus_error_free(&err); - } - virNodeDeviceObjListFree(driver->devs); - if (hal_ctx) - (void)libhal_ctx_free(hal_ctx); - nodeDeviceUnlock(); - VIR_FREE(driver); - - return ret; -} - - -static int -nodeStateCleanup(void) -{ - if (driver) { - nodeDeviceLock(); - LibHalContext *hal_ctx = DRV_STATE_HAL_CTX(driver); - virNodeDeviceObjListFree(driver->devs); - (void)libhal_ctx_shutdown(hal_ctx, NULL); - (void)libhal_ctx_free(hal_ctx); - if (driver->lockFD != -1) - virPidFileRelease(driver->stateDir, "driver", driver->lockFD); - - VIR_FREE(driver->stateDir); - nodeDeviceUnlock(); - virCondDestroy(&driver->initCond); - virMutexDestroy(&driver->lock); - VIR_FREE(driver); - return 0; - } - return -1; -} - - -static int -nodeStateReload(void) -{ - DBusError err; - char **udi = NULL; - int num_devs; - size_t i; - LibHalContext *hal_ctx; - - VIR_INFO("Reloading HAL device state"); - nodeDeviceLock(); - VIR_INFO("Removing existing objects"); - virNodeDeviceObjListFree(driver->devs); - nodeDeviceUnlock(); - - hal_ctx = DRV_STATE_HAL_CTX(driver); - VIR_INFO("Creating new objects"); - dbus_error_init(&err); - udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); - if (udi == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libhal_get_all_devices failed")); - return -1; - } - for (i = 0; i < num_devs; i++) { - dev_create(udi[i]); - VIR_FREE(udi[i]); - } - VIR_FREE(udi); - VIR_INFO("HAL device reload complete"); - - return 0; -} - - -static virNodeDeviceDriver halNodeDeviceDriver = { - .name = "HAL", - .nodeNumOfDevices = nodeNumOfDevices, /* 0.5.0 */ - .nodeListDevices = nodeListDevices, /* 0.5.0 */ - .connectListAllNodeDevices = nodeConnectListAllNodeDevices, /* 0.10.2 */ - .nodeDeviceLookupByName = nodeDeviceLookupByName, /* 0.5.0 */ - .nodeDeviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ - .nodeDeviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.5.0 */ - .nodeDeviceGetParent = nodeDeviceGetParent, /* 0.5.0 */ - .nodeDeviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.5.0 */ - .nodeDeviceListCaps = nodeDeviceListCaps, /* 0.5.0 */ - .nodeDeviceCreateXML = nodeDeviceCreateXML, /* 0.6.5 */ - .nodeDeviceDestroy = nodeDeviceDestroy, /* 0.6.5 */ -}; - - -static virHypervisorDriver halHypervisorDriver = { - .name = "nodedev", - .connectOpen = nodeConnectOpen, /* 4.1.0 */ - .connectClose = nodeConnectClose, /* 4.1.0 */ - .connectIsEncrypted = nodeConnectIsEncrypted, /* 4.1.0 */ - .connectIsSecure = nodeConnectIsSecure, /* 4.1.0 */ - .connectIsAlive = nodeConnectIsAlive, /* 4.1.0 */ -}; - - -static virConnectDriver halConnectDriver = { - .localOnly = true, - .uriSchemes = (const char *[]){ "nodedev", NULL }, - .hypervisorDriver = &halHypervisorDriver, - .nodeDeviceDriver = &halNodeDeviceDriver, -}; - - -static virStateDriver halStateDriver = { - .name = "HAL", - .stateInitialize = nodeStateInitialize, /* 0.5.0 */ - .stateCleanup = nodeStateCleanup, /* 0.5.0 */ - .stateReload = nodeStateReload, /* 0.5.0 */ -}; - -int -halNodeRegister(void) -{ - if (virRegisterConnectDriver(&halConnectDriver, false) < 0) - return -1; - if (virSetSharedNodeDeviceDriver(&halNodeDeviceDriver) < 0) - return -1; - return virRegisterStateDriver(&halStateDriver); -} diff --git a/src/node_device/node_device_hal.h b/src/node_device/node_device_hal.h deleted file mode 100644 index 5e9c25ae34..0000000000 --- a/src/node_device/node_device_hal.h +++ /dev/null @@ -1,22 +0,0 @@ -/* - * node_device_hal.h: node device enumeration - HAL-based implementation - * - * Copyright (C) 2009 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - */ - -#pragma once -- 2.26.2

4 7

[PATCH 00/12] assorted cleanups in cpu_ppc64.c
by Daniel Henrique Barboza 09 Sep '20

09 Sep '20

Hi, While reading and studying cpu_ppc64.c I noticed that it lacks the 'sophistication' that cpu_x86.c and cpu_arm.c enjoys. This is my attempt to tidy it up a bit, adding some typedefs and g_auto* cleanups to make it more up to par with cpu_x86.c Daniel Henrique Barboza (12): cpu_ppc64.c: use typedefs for 'struct ppc64_vendor' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64VendorPtr cpu_ppc64.c: modernize ppc64VendorParse() cpu_ppc64.c: use typedefs for 'struct ppc64_model' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64ModelPtr cpu_ppc64.c: use g_autopr() with virCPUppc64ModelPtr cpu_ppc64.c: use typedefs for 'struct ppc64_map' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64MapPtr cpu_ppc64.c: use g_autopr() with virCPUppc64MapPtr cpu_ppc64.c: use g_autoptr() in virCPUppc64GetHost() cpu_ppc64.c: use g_autofree() whenever possible cpu_ppc64.c: use g_autoptr() whenever possible src/cpu/cpu_ppc64.c | 245 ++++++++++++++++++-------------------------- 1 file changed, 98 insertions(+), 147 deletions(-) -- 2.26.2

2 24

[libvirt PATCH] src/storage/meson: fix vir_storage_file_gluster module dependencies
by Pavel Hrdina 08 Sep '20

08 Sep '20

The correct key for dependencies for virt_modules hash is `deps`. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/storage/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/meson.build b/src/storage/meson.build index 18ea0f3fb5..b4cefe9a89 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -187,7 +187,7 @@ if conf.has('WITH_STORAGE_GLUSTER') 'sources': [ files(storage_file_gluster_sources), ], - 'dependenciec': [ + 'deps': [ glusterfs_dep, ], 'install_dir': storage_file_install_dir, -- 2.26.2

4 4

[PATCH 0/1] Support Windows 10 Arm64 ISO installations
by Jan-Marek Glogowski 08 Sep '20

08 Sep '20

This patch was originally submitted via https://gitlab.com/libvirt/libvirt/-/merge_requests/19 That merge request contains additional information, if you find the commit message of the patch lacking in some way. It did pass CI and the test suite four weeks ago, seems small enough, and the rebase had no conflicts, so I didn't run the tests again on current master. And based on the comment from Peter Krempa in that merge request, it probably needs further discussion with QEMU developers. FWIW: while the installer generally works, I'm experiencing massive timeouts from the "Out of Box Experience" (OOBE) part of the Windows configuration after the install. And the latest ISOs don't even start in QEMU (5.1 and older) for me on Debian, so I used 1909 instead of the 2004 ISO "releases". I'm running the aarch64 system QEMU on x86_64 Debian Buster with my current QEMU build. Jan-Marek Glogowski (1): QEMU: support USB cdrom devices src/qemu/qemu_command.c | 21 ++++++++++++++++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 2 +- .../disk-cdrom-bus-other.x86_64-latest.args | 12 +++++------ 4 files changed, 28 insertions(+), 8 deletions(-) -- 2.20.1

4 10

[libvirt PATCH] conf: Add support for GlusterFS volumes as disk
by Vincent Vanlaer 08 Sep '20

08 Sep '20

While libvirt has support for GlusterFS pools and volumes, they cannot be added to a domain. This commit adds the necessary translation between the pool definition and the disk definition, allowing volumes in a domain to be backed by a gluster pool. Signed-off-by: Vincent Vanlaer <libvirt(a)volkihar.be> --- src/conf/domain_conf.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 72ac4f4191..c872d02200 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -32620,6 +32620,32 @@ virDomainDiskTranslateISCSIDirect(virStorageSourcePtr src, } +static int +virDomainDiskTranslateGluster(virStorageSourcePtr src, + virStoragePoolDefPtr pooldef) +{ + size_t i; + + src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER; + + src->nhosts = pooldef->source.nhost; + src->hosts = g_new0(virStorageNetHostDef, src->nhosts); + + for (i = 0; i < src->nhosts; i++) { + src->hosts[i].socket = NULL; + src->hosts[i].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + src->hosts[i].name = g_strdup(pooldef->source.hosts[i].name); + src->hosts[i].port = pooldef->source.hosts[i].port; + } + + src->volume = g_strdup(pooldef->source.name); + src->path = g_strdup_printf("%s/%s", pooldef->source.dir, src->srcpool->volume); + + return 0; +} + + static int virDomainStorageSourceTranslateSourcePool(virStorageSourcePtr src, virConnectPtr conn) @@ -32736,10 +32762,14 @@ virDomainStorageSourceTranslateSourcePool(virStorageSourcePtr src, } break; + case VIR_STORAGE_POOL_GLUSTER: + if (virDomainDiskTranslateGluster(src, pooldef) < 0) + return -1; + break; + case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: case VIR_STORAGE_POOL_SHEEPDOG: - case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("using '%s' pools for backing 'volume' disks " -- 2.28.0

2 1

[libvirt PATCH] check for NULL before calling g_regex_unref
by Ján Tomko 08 Sep '20

08 Sep '20

g_regex_unref reports an error if called with a NULL argument. We have two cases in the code where we (possibly) call it on a NULL argument. The interesting one is in virDomainQemuMonitorEventCleanup. Based on VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX, we unref data->regex, which has two problems: * On the client side, flags is -1 so the comparison is true even if no regex was used, reproducible by: $ virsh qemu-monitor-event --timeout 1 which results in an ugly error: (process:1289846): GLib-CRITICAL **: 14:58:42.631: g_regex_unref: assertion 'regex != NULL' failed * On the server side, we only create the regex if both the flag and the string are present, so it's possible to trigger this message by: $ virsh qemu-monitor-event --regex --timeout 1 Use a non-NULL comparison instead of the flag to decide whether we need to unref the regex. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Fixes: 71efb59a4de7c51b1bc889a316f1796ebf55738f https://bugzilla.redhat.com/show_bug.cgi?id=1861176 --- src/conf/domain_event.c | 2 +- tests/vboxsnapshotxmltest.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 33fbf10406..d3acde0236 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -2194,7 +2194,7 @@ virDomainQemuMonitorEventCleanup(void *opaque) virDomainQemuMonitorEventData *data = opaque; VIR_FREE(data->event); - if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) + if (data->regex) g_regex_unref(data->regex); if (data->freecb) (data->freecb)(data->opaque); diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c index 7e3f85cc58..d2beb7858d 100644 --- a/tests/vboxsnapshotxmltest.c +++ b/tests/vboxsnapshotxmltest.c @@ -135,7 +135,8 @@ mymain(void) DO_TEST("2disks-3snap-brother"); cleanup: - g_regex_unref(testSnapshotXMLVariableLineRegex); + if (testSnapshotXMLVariableLineRegex) + g_regex_unref(testSnapshotXMLVariableLineRegex); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.26.2

3 2

[PATCH v5 0/4] qemu: Support rbd namespace in rbd source name
by Han Han 08 Sep '20

08 Sep '20

v4: https://www.redhat.com/archives/libvir-list/2020-August/msg00351.html Difference from v4: - rebase to last master tree - parse namspace from name attribute and update the documents - add caps test xml for 5.2.0 x86_64 - adapt to DO_TEST_CAPS_ARCH_LATEST in qemuxml2xmltest in case of the test failure caused by missing caps test xml of 5.x at s390. git branch: https://gitlab.com/hhan2/libvirt/-/tree/rbd-namespace-v5 Han Han (4): qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE conf: Support to parse rbd namespace from source name qemu: Implement rbd namespace to the source name attribute news: qemu: Support rbd namespace NEWS.rst | 8 ++++ docs/formatdomain.rst | 12 ++++-- src/conf/domain_conf.c | 35 ++++++++++++++-- src/qemu/qemu_block.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_domain.c | 8 ++++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + ...k-network-rbd-namespace.x86_64-latest.args | 41 +++++++++++++++++++ .../disk-network-rbd-namespace.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...sk-network-rbd-namespace.x86_64-latest.xml | 41 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 20 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml -- 2.28.0

2 5

large number of hot plug disks
by Alexander Wels 08 Sep '20

08 Sep '20

Hi, I am working on a project, and one of the requirements is that I can hotplug a large number of disks. Think several dozens to potentially 100+. The guest OS will be some linux flavor so it should have no problem with a large number of disks. During my experimentation I quickly ran into the "No available PCI slots" limit. I found https://libvirt.org/pci-hotplug.html explaining to me why the limit is there. Now I could add a bunch of pcie-root-ports ahead of time, but there is likely a limit to that as well. So before I go down some crazy paths, I figured I would ask on here if there is a better plan than adding a bunch of root ports for adding a large number of hotplugged disks? Thanks, Alexander

3 4

[libvirt PATCH] docs: Rephrase the video 'none' type to clarify the reasoning
by Erik Skultety 08 Sep '20

08 Sep '20

Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- docs/formatdomain.rst | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) This is a tiny follow-up improvement resulting from [1] as I felt the paragraph wasn't clear enough + there were a couple of typographical issues. [1] https://www.redhat.com/archives/libvir-list/2020-September/msg00329.html diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1979dfb8d3..cf3ccbecc5 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5784,13 +5784,16 @@ A video device. value "vga", "cirrus", "vmvga", "xen", "vbox", "qxl" ( :since:`since 0.8.6` ), "virtio" ( :since:`since 1.3.0` ), "gop" ( :since:`since 3.2.0` ), "bochs" ( :since:`since 5.6.0` ), "ramfb" ( :since:`since 5.9.0` ), or "none" ( - :since:`since 4.6.0` , depending on the hypervisor features available. The - purpose of the type ``none`` is to instruct libvirt not to add a default - video device in the guest (see the paragraph above). This legacy behaviour - can be inconvenient in cases where GPU mediated devices are meant to be the - only rendering device within a guest and so specifying another ``video`` - device along with type ``none``. Refer to Host device assignment to see how - to add a mediated device into a guest. + :since:`since 4.6.0`) , depending on the hypervisor features available. + + Note: The purpose of the type ``none`` is to instruct libvirt not to add a + default video device in the guest (see the ``video`` element description + above), since such behaviour is inconvenient in cases where GPU mediated + devices are meant to be the only rendering device within a guest. If this + is your use case specify a ``none`` type ``video`` device in the XML to stop + the default behaviour. Refer to `Host device assignment + <#host-device-assignment>`__ to see how to add a mediated device into a + guest. You can provide the amount of video memory in kibibytes (blocks of 1024 bytes) using ``vram``. This is supported only for guest type of "vz", "qemu", -- 2.26.2

3 3

[PATCH] rpm: Remove pointless explicit dependency on 'readline'
by Neal Gompa 08 Sep '20

08 Sep '20

The 'readline' package only contains the library, which rpm is already generating the (stricter) correct dependency for. Signed-off-by: Neal Gompa <ngompa13(a)gmail.com> --- libvirt.spec.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index d124a7ce45..379cf0d9be 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -891,7 +891,6 @@ capabilities of VirtualBox %package client Summary: Client side utilities of the libvirt library Requires: %{name}-libs = %{version}-%{release} -Requires: readline # Needed by /usr/libexec/libvirt-guests.sh script. Requires: gettext # Needed by virt-pki-validate script. @@ -918,7 +917,6 @@ Shared libraries for accessing the libvirt daemon. %package admin Summary: Set of tools to control libvirt daemon Requires: %{name}-libs = %{version}-%{release} -Requires: readline %if %{with_bash_completion} Requires: %{name}-bash-completion = %{version}-%{release} %endif -- 2.26.2

2 1

[PATCH] wireshark: Prefer ws_version.h over config.h
by Michal Privoznik 08 Sep '20

08 Sep '20

A wireshark plugin must declare what major and minor version it was built with as these are checked when wireshark loads plugins. On the top of that, we use major + minor + micro to adapt to changed API between releases. So far, we were getting these version numbers from wireshark/config.h. And while most distributions install wireshark/config.h file some don't. On distros shipping it it's hack^Wsaved during built by packaging system and installed later. But some distros are not doing that. At least not for new enough wireshark because as of wireshark's commit v2.9.0~1273 the ws_version.h is installed which contains the version macros we need and is installed by wireshark itself. But of course, some distros which have new enough wireshark packaged do not ship ws_version.h and stick to the hack. That is why we can't simply bump the minimal version and switch to the new header file. We need a configure check and adopt our code to deal with both ways. At least for the time being. Based on Andrea's original patch: https://www.redhat.com/archives/libvir-list/2020-September/msg00156.html Closes: https://gitlab.com/libvirt/libvirt/-/issues/74 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- meson.build | 6 ++++++ tools/wireshark/src/packet-libvirt.c | 1 - tools/wireshark/src/plugin.c | 21 +++++++++++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index 1eadea33bf..33eaa9ff56 100644 --- a/meson.build +++ b/meson.build @@ -1472,6 +1472,12 @@ if wireshark_dep.found() # Since wireshark 2.5.0 plugins can't live in top level plugindir but have # to be under one of ["epan", "wiretap", "codecs"] subdir. The first one looks okay. wireshark_plugindir = wireshark_plugindir / 'epan' + + # Wireshark is installing ws_version.h since v2.9.0, but some distributions + # are not shipping it. + if cc.has_header('wireshark/ws_version.h') + conf.set('WITH_WS_VERSION', 1) + endif endif # On MinGW portablexdr provides XDR functions, on linux they are diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index 9f3c7f650d..965f1f5482 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -18,7 +18,6 @@ */ #include <config.h> -#include <wireshark/config.h> #include <wireshark/epan/proto.h> #include <wireshark/epan/packet.h> #include <wireshark/epan/dissectors/packet-tcp.h> diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c index 504e4383a7..13af1b6a73 100644 --- a/tools/wireshark/src/plugin.c +++ b/tools/wireshark/src/plugin.c @@ -14,7 +14,16 @@ #include <gmodule.h> -#include <wireshark/config.h> +#ifdef WITH_WS_VERSION +# include <wireshark/ws_version.h> +#else +# include <wireshark/config.h> +# define WIRESHARK_VERSION_MAJOR VERSION_MAJOR +# define WIRESHARK_VERSION_MINOR VERSION_MINOR +# define WIRESHARK_VERSION_MICRO VERSION_MICRO +#endif + +#define HAVE_PLUGINS 1 #include <wireshark/epan/proto.h> /* plugins are DLLs */ #define WS_BUILD_DLL @@ -26,9 +35,9 @@ #define PLUGIN_VERSION VERSION #define WIRESHARK_VERSION \ - ((VERSION_MAJOR * 1000 * 1000) + \ - (VERSION_MINOR * 1000) + \ - (VERSION_MICRO)) + ((WIRESHARK_VERSION_MAJOR * 1000 * 1000) + \ + (WIRESHARK_VERSION_MINOR * 1000) + \ + (WIRESHARK_VERSION_MICRO)) #if WIRESHARK_VERSION < 2005000 @@ -69,8 +78,8 @@ void proto_register_libvirt(void); void proto_reg_handoff_libvirt(void); WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; -WS_DLL_PUBLIC_DEF const int plugin_want_major = VERSION_MAJOR; -WS_DLL_PUBLIC_DEF const int plugin_want_minor = VERSION_MINOR; +WS_DLL_PUBLIC_DEF const int plugin_want_major = WIRESHARK_VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = WIRESHARK_VERSION_MINOR; WS_DLL_PUBLIC void plugin_register(void); -- 2.26.2

3 2

[PATCH] virnuma: Report error when NUMA -> CPUs translation fails
by Michal Privoznik 08 Sep '20

08 Sep '20

When starting a domain with <numatune/> set libvirt translates given NUMA nodes into a set of host CPUs which is then used to QEMU process affinity. But, if the numatune contains a non-existent NUMA node then the translation fails with no error reported. This is because virNumaNodesetToCPUset() calls virNumaGetNodeCPUs() and expects it to report an error on failure. Well, it does except for non-existent NUMA nodes. While this behaviour might look strange it is actually desired because of how we construct host capabilities. The virNumaGetNodeCPUs() is called from virCapabilitiesHostNUMAInitReal() where we do not want any error reported for non-existent NUMA nodes. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1724866 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virnuma.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index ba1e4363d6..2872ce3c5e 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -316,12 +316,21 @@ virNumaNodesetToCPUset(virBitmapPtr nodeset, for (i = 0; i < nodesetSize; i++) { g_autoptr(virBitmap) nodeCPUs = NULL; + int rc; if (!virBitmapIsBitSet(nodeset, i)) continue; - if (virNumaGetNodeCPUs(i, &nodeCPUs) < 0) + rc = virNumaGetNodeCPUs(i, &nodeCPUs); + if (rc < 0) { + /* Error is reported for cases other than non-existent NUMA node. */ + if (rc == -2) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("NUMA node %zu is not available"), + i); + } return -1; + } if (virBitmapUnion(allNodesCPUs, nodeCPUs) < 0) return -1; -- 2.26.2

2 1

[RFC PATCH 0/3] qemu: Propagate cluster size to new qcow2 images
by Peter Krempa 08 Sep '20

08 Sep '20

Cluster size may have performatnce implications. Propagate the cluster size used by the original image to any copy or overlay. Peter Krempa (3): qemu: monitor: Detect image cluster size from 'query-named-block-nodes' qemu: block: Allow specifying cluster size when using 'blockdev-create' qemuBlockStorageSourceCreateDetectSize: Propagate cluster size for 'qcow2' src/qemu/qemu_block.c | 6 ++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 3 +++ src/util/virstoragefile.h | 1 + 4 files changed, 13 insertions(+) -- 2.26.2

2 5

[PATCH] qemu_process: Separate VIR_PERF_EVENT_* setting into a function
by Michal Privoznik 08 Sep '20

08 Sep '20

When starting a domain, qemuProcessLaunch() iterates over all VIR_PERF_EVENT_* values and (possibly) enables them. While there is nothing wrong with the code, the for loop where its done makes it harder to jump onto next block of code. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Is this trivial enough to be pushed as such? src/qemu/qemu_process.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6b1644880a..dd60fb0ddf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6700,6 +6700,25 @@ qemuProcessEnableDomainNamespaces(virQEMUDriverPtr driver, } +static int +qemuProcessEnablePerf(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + + if (!(priv->perf = virPerfNew())) + return -1; + + for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { + if (vm->def->perf.events[i] == VIR_TRISTATE_BOOL_YES && + virPerfEventEnable(priv->perf, i, vm->pid) < 0) + return -1; + } + + return 0; +} + + /** * qemuProcessLaunch: * @@ -6733,7 +6752,6 @@ qemuProcessLaunch(virConnectPtr conn, g_autoptr(virQEMUDriverConfig) cfg = NULL; size_t nnicindexes = 0; g_autofree int *nicindexes = NULL; - size_t i; VIR_DEBUG("conn=%p driver=%p vm=%p name=%s if=%d asyncJob=%d " "incoming.launchURI=%s incoming.deferredURI=%s " @@ -6885,15 +6903,10 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuSetupCgroup(vm, nnicindexes, nicindexes) < 0) goto cleanup; - if (!(priv->perf = virPerfNew())) + VIR_DEBUG("Setting up domain perf (if required)"); + if (qemuProcessEnablePerf(vm) < 0) goto cleanup; - for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { - if (vm->def->perf.events[i] == VIR_TRISTATE_BOOL_YES && - virPerfEventEnable(priv->perf, i, vm->pid) < 0) - goto cleanup; - } - /* This must be done after cgroup placement to avoid resetting CPU * affinity */ if (qemuProcessInitCpuAffinity(vm) < 0) -- 2.26.2

2 1

[RFC PATCH] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation
by Alex Bennée 08 Sep '20

08 Sep '20

It's buggy and we are not sure anyone uses it. Cc: David Gibson <david(a)gibson.dropbear.id.au> Cc: Richard Henderson <rth(a)twiddle.net> Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org> --- docs/system/deprecated.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 851dbdeb8ab..11c763383d9 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -424,6 +424,15 @@ linux-user mode) is deprecated and will be removed in a future version of QEMU. Support for this CPU was removed from the upstream Linux kernel in 2018, and has also been dropped from glibc. +``ppc64abi32`` CPUs (since 5.2.0) +''''''''''''''''''''''''''''''''' + +The ``ppc64abi32`` architecture has a number of issues which regularly +trip up our CI testing and is suspected to be quite broken. +Furthermore the maintainers are unsure what the correct behaviour +should be and strongly suspect no one actually uses it. + + Related binaries ---------------- -- 2.20.1

4 6

[PATCH] news: add 1 improvement for v6.6.0
by Yanqiu Zhang 07 Sep '20

07 Sep '20

Support TLS for NBD connections for pull mode backup. Signed-off-by: Yanqiu Zhang <yanqzhan(a)redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e4b71e6563..d0b16e28fc 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -161,6 +161,12 @@ v6.6.0 (2020-08-02) The secret key used for disks can now be encrypted similarly to TLS keys used for migration, chardev and others. + * qemu: Support TLS for NBD connections for pull mode backup + + Use the configured TLS env to setup encryption of the TLS transport. Add 'tls' + attribute for 'server' element to enable tls. Possible values are ``yes`` + or ``no``. + * qemu: ``VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD`` can now be registered for ``<mirror>`` The event can now be used also for block copy destinations by using the -- 2.25.4

4 7

[PATCH] Revert "news: qemu: Support TLS for NBD connections for pull mode backup"
by Peter Krempa 07 Sep '20

07 Sep '20

Incremental backups are not enabled. There's no point documenting individual patches implementing an incomplete feature. This reverts commit e6285f84faeacccdef12b69a7bc38975230d6662. --- NEWS.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index 008a6358c6..14f098b1f6 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -167,12 +167,6 @@ v6.6.0 (2020-08-02) The secret key used for disks can now be encrypted similarly to TLS keys used for migration, chardev and others. - * qemu: Support TLS for NBD connections for pull mode backup - - Use the configured TLS env to setup encryption of the TLS transport. Add - 'tls' attribute for 'server' element to enable TLS. Possible values are - ``yes`` or ``no``. - * qemu: ``VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD`` can now be registered for ``<mirror>`` The event can now be used also for block copy destinations by using the -- 2.26.2

3 2

[libvirt PATCH] wireshark: Don't include config.h
by Andrea Bolognani 07 Sep '20

07 Sep '20

While both Debian and Fedora include the header in their development packages for Wireshark, that's not something that the upstream developers intended and arguably quite wrong, as config.h is obviously intended to only be used to drive the compilation of Wireshark itself. The Arch Linux package behaves like the upstream Wireshark package, and thus libvirt fails to build there. It seems that there are multiple bugs to be addressed: * libvirt shouldn't include config.h; * Debian and Fedora shouldn't be shipping config.h in their Wireshark packages; * Wireshark should not use config.h defines such as HAVE_PLUGINS in its public headers, and define a public variant of them instead. This patch takes care of the first one. https://gitlab.com/libvirt/libvirt/-/issues/74 Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- tools/wireshark/src/packet-libvirt.c | 1 - tools/wireshark/src/plugin.c | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c index 9f3c7f650d..965f1f5482 100644 --- a/tools/wireshark/src/packet-libvirt.c +++ b/tools/wireshark/src/packet-libvirt.c @@ -18,7 +18,6 @@ */ #include <config.h> -#include <wireshark/config.h> #include <wireshark/epan/proto.h> #include <wireshark/epan/packet.h> #include <wireshark/epan/dissectors/packet-tcp.h> diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c index 504e4383a7..3c6fae9ef5 100644 --- a/tools/wireshark/src/plugin.c +++ b/tools/wireshark/src/plugin.c @@ -14,11 +14,12 @@ #include <gmodule.h> -#include <wireshark/config.h> +#define HAVE_PLUGINS 1 #include <wireshark/epan/proto.h> /* plugins are DLLs */ #define WS_BUILD_DLL #include <wireshark/ws_symbol_export.h> +#include <wireshark/ws_version.h> #include "packet-libvirt.h" @@ -26,9 +27,9 @@ #define PLUGIN_VERSION VERSION #define WIRESHARK_VERSION \ - ((VERSION_MAJOR * 1000 * 1000) + \ - (VERSION_MINOR * 1000) + \ - (VERSION_MICRO)) + ((WIRESHARK_VERSION_MAJOR * 1000 * 1000) + \ + (WIRESHARK_VERSION_MINOR * 1000) + \ + (WIRESHARK_VERSION_MICRO)) #if WIRESHARK_VERSION < 2005000 @@ -69,8 +70,8 @@ void proto_register_libvirt(void); void proto_reg_handoff_libvirt(void); WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; -WS_DLL_PUBLIC_DEF const int plugin_want_major = VERSION_MAJOR; -WS_DLL_PUBLIC_DEF const int plugin_want_minor = VERSION_MINOR; +WS_DLL_PUBLIC_DEF const int plugin_want_major = WIRESHARK_VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = WIRESHARK_VERSION_MINOR; WS_DLL_PUBLIC void plugin_register(void); -- 2.26.2

3 8

[libvirt PATCH 0/2] Fix IPv6 network startup after removal of "dummy" tap device
by Laine Stump 07 Sep '20

07 Sep '20

After the patch that removed the dummy tap device was added (commit ee6c936fbb), IPv6 networks would no longer start due to a "failure to complete DAD". Since this wait was only necessary because of the existence of the dummy tap device (see detailed git log spelunking in Patch 1 comment), we can just remove the wait. Laine Stump (2): network: don't wait for IPv6 DAD completion when starting a network util: remove unused virNetDevIPWaitDadFinish() src/libvirt_private.syms | 1 - src/network/bridge_driver.c | 32 ---------- src/util/virnetdevip.c | 119 ------------------------------------ src/util/virnetdevip.h | 2 - 4 files changed, 154 deletions(-) -- 2.26.2

3 4

watchdog fed successfully event of 6300esb
by ligang (P) 07 Sep '20

07 Sep '20

Hi folks, I have an question to discuss about the 6300esb watchdog. I think is it possible that qemu can send an event while the watchdog successfully fed by the vm at the first time. Here is the situation: Qemu will send an VIR_DOMAIN_EVENT_ID_WATCHDOG event while watch dog timeout, and if the action of the watchdog in xml of the vm was set to "reset", the vm will be rebooted while timeout. I have an monitor process that register callback function of the VIR_DOMAIN_EVENT_ID_WATCHDOG event, the callback function will send an alarm to my upper layer monitor platform indicate that the vm is fault, and the cluster deployed business on the vm will isolate the vm by the alarm. And after the vm rebooted , the monitor process will receive an reboot event and send it to the platform, the upper layer monitor platform will clear the alarm, and business continue to run on the vm. In most cases ,the watch dog process in vm will feed the watchdog after vm rebooted and all things go back on track. In some other cases,the guestos may failed to start (in my environment vm start failed by io error), but the reboot event will still be received and the alarm will be cleared and the vm is still fault. So the this may not a good idea to clear the alarm by the reboot event. So, I think it will be helpful that the qemu can send an event while the watchdog successfully fed by the vm at the first time. So I can exactly know that the guest os go back on running and the watch dog initialized successfully. Or any other opintion about this situation. Thanks.

1 0

[PATCH v2] qemu_namespace: Don't leak mknod items that are being skipped over
by Michal Privoznik 07 Sep '20

07 Sep '20

When building and populating domain NS a couple of functions are called that append paths to a string list. This string list is then inspected, one item at the time by qemuNamespacePrepareOneItem() which gathers all the info for given path (stat buffer, possible link target, ACLs, SELinux label) using qemuNamespaceMknodItemInit(). If the path needs to be created in the domain's private /dev then it's added onto this qemuNamespaceMknodData list which is freed later in the process. But, if the path does not need to be created in the domain's private /dev, then the memory allocated by qemuNamespaceMknodItemInit() is not freed anywhere leading to a leak. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://www.redhat.com/archives/libvir-list/2020-September/msg00248.html diff to v2: - autocleanup approach is implemented in qemuNamespacePrepareOneItem() src/qemu/qemu_namespace.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 87f4fd8d58..b0d1d0d083 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -871,7 +871,7 @@ qemuDomainNamespaceAvailable(qemuDomainNamespace ns G_GNUC_UNUSED) typedef struct _qemuNamespaceMknodItem qemuNamespaceMknodItem; typedef qemuNamespaceMknodItem *qemuNamespaceMknodItemPtr; struct _qemuNamespaceMknodItem { - const char *file; + char *file; char *target; bool bindmounted; GStatBuf sb; @@ -892,6 +892,7 @@ struct _qemuNamespaceMknodData { static void qemuNamespaceMknodItemClear(qemuNamespaceMknodItemPtr item) { + VIR_FREE(item->file); VIR_FREE(item->target); virFileFreeACLs(&item->acl); #ifdef WITH_SELINUX @@ -900,6 +901,8 @@ qemuNamespaceMknodItemClear(qemuNamespaceMknodItemPtr item) } +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuNamespaceMknodItem, qemuNamespaceMknodItemClear); + static void qemuNamespaceMknodDataClear(qemuNamespaceMknodDataPtr data) { @@ -1091,7 +1094,7 @@ qemuNamespaceMknodItemInit(qemuNamespaceMknodItemPtr item, bool isLink; bool needsBindMount; - item->file = file; + item->file = g_strdup(file); if (g_lstat(file, &item->sb) < 0) { if (errno == ENOENT) @@ -1166,11 +1169,13 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodDataPtr data, size_t ndevMountsPath) { long ttl = sysconf(_SC_SYMLOOP_MAX); - const char *next = file; + g_autofree char *next = g_strdup(file); size_t i; while (1) { - qemuNamespaceMknodItem item = { 0 }; + g_auto(qemuNamespaceMknodItem) item = { 0 }; + bool isLink; + bool addToData = false; int rc; rc = qemuNamespaceMknodItemInit(&item, cfg, vm, next); @@ -1182,6 +1187,8 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodDataPtr data, return -1; } + isLink = S_ISLNK(item.sb.st_mode); + if (STRPREFIX(next, QEMU_DEVPREFIX)) { for (i = 0; i < ndevMountsPath; i++) { if (STREQ(devMountsPath[i], "/dev")) @@ -1190,12 +1197,18 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodDataPtr data, break; } - if (i == ndevMountsPath && - VIR_APPEND_ELEMENT_COPY(data->items, data->nitems, item) < 0) - return -1; + if (i == ndevMountsPath) + addToData = true; } - if (!S_ISLNK(item.sb.st_mode)) + g_free(next); + next = g_strdup(item.target); + + if (addToData && + VIR_APPEND_ELEMENT(data->items, data->nitems, item) < 0) + return -1; + + if (!isLink) break; if (ttl-- == 0) { @@ -1204,8 +1217,6 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodDataPtr data, next); return -1; } - - next = item.target; } return 0; -- 2.26.2

2 1

[PATCH] qemu: Allow setting affinity to fail and don't report error
by Martin Kletzander 07 Sep '20

07 Sep '20

This is just a clean-up of commit 3791f29b085c using the new parameter of virProcessSetAffinity() introduced in commit 9514e24984ee so that there is no error reported in the logs. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_process.c | 65 +++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 04e5cbb65969..e29f35e10576 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2569,24 +2569,21 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) return -1; } + /* + * We only want to error out if we failed to set the affinity to + * user-requested mapping. If we are just trying to reset the affinity + * to all CPUs and this fails it can only be an issue if: + * 1) libvirtd does not have CAP_SYS_NICE + * 2) libvirtd does not run on all CPUs + * + * This scenario can easily occurr when libvirtd is run inside a + * container with restrictive permissions and CPU pinning. + * + * See also: https://bugzilla.redhat.com/1819801#c2 + */ if (cpumapToSet && - virProcessSetAffinity(vm->pid, cpumapToSet, false) < 0) { - /* - * We only want to error out if we failed to set the affinity to - * user-requested mapping. If we are just trying to reset the affinity - * to all CPUs and this fails it can only be an issue if: - * 1) libvirtd does not have CAP_SYS_NICE - * 2) libvirtd does not run on all CPUs - * - * This scenario can easily occurr when libvirtd is run inside a - * container with restrictive permissions and CPU pinning. - * - * See also: https://bugzilla.redhat.com/1819801#c2 - */ - if (settingAll) - virResetLastError(); - else - return -1; + virProcessSetAffinity(vm->pid, cpumapToSet, settingAll) < 0) { + return -1; } return 0; @@ -2739,25 +2736,23 @@ qemuProcessSetupPid(virDomainObjPtr vm, if (!affinity_cpumask) affinity_cpumask = use_cpumask; - /* Setup legacy affinity. */ + /* Setup legacy affinity. + * + * We only want to error out if we failed to set the affinity to + * user-requested mapping. If we are just trying to reset the affinity + * to all CPUs and this fails it can only be an issue if: + * 1) libvirtd does not have CAP_SYS_NICE + * 2) libvirtd does not run on all CPUs + * + * This scenario can easily occurr when libvirtd is run inside a + * container with restrictive permissions and CPU pinning. + * + * See also: https://bugzilla.redhat.com/1819801#c2 + */ if (affinity_cpumask && - virProcessSetAffinity(pid, affinity_cpumask, false) < 0) { - /* - * We only want to error out if we failed to set the affinity to - * user-requested mapping. If we are just trying to reset the affinity - * to all CPUs and this fails it can only be an issue if: - * 1) libvirtd does not have CAP_SYS_NICE - * 2) libvirtd does not run on all CPUs - * - * This scenario can easily occurr when libvirtd is run inside a - * container with restrictive permissions and CPU pinning. - * - * See also: https://bugzilla.redhat.com/1819801#c2 - */ - if (affinity_cpumask == hostcpumap) - virResetLastError(); - else - goto cleanup; + virProcessSetAffinity(pid, affinity_cpumask, + affinity_cpumask == hostcpumap) < 0) { + goto cleanup; } /* Set scheduler type and priority, but not for the main thread. */ -- 2.28.0

2 1

[PATCH] Do not report error when setting affinity is allowed to fail
by Martin Kletzander 07 Sep '20

07 Sep '20

Suggested-by: Ján Tomko <jtomko(a)redhat.com> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/lxc/lxc_controller.c | 2 +- src/qemu/qemu_driver.c | 7 ++++--- src/qemu/qemu_process.c | 10 +++++----- src/util/virprocess.c | 32 +++++++++++++++++++++++--------- src/util/virprocess.h | 2 +- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 0a496fb7886c..37a28ac2f3c1 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -775,7 +775,7 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessSetAffinity(0 /* Self */, cpumapToSet) < 0) { + if (virProcessSetAffinity(0 /* Self */, cpumapToSet, false) < 0) { virBitmapFree(cpumap); return -1; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ce72e1021d16..a792a1283253 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4529,7 +4529,8 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, goto cleanup; } - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), cpumap) < 0) + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), + cpumap, false) < 0) goto cleanup; } @@ -4747,7 +4748,7 @@ qemuDomainPinEmulator(virDomainPtr dom, } } - if (virProcessSetAffinity(vm->pid, pcpumap) < 0) + if (virProcessSetAffinity(vm->pid, pcpumap, false) < 0) goto endjob; virBitmapFree(def->cputune.emulatorpin); @@ -5222,7 +5223,7 @@ qemuDomainPinIOThread(virDomainPtr dom, } } - if (virProcessSetAffinity(iothrid->thread_id, pcpumap) < 0) + if (virProcessSetAffinity(iothrid->thread_id, pcpumap, false) < 0) goto endjob; if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 17d083d192a5..423c60c7c85b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2573,7 +2573,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) } if (cpumapToSet && - virProcessSetAffinity(vm->pid, cpumapToSet) < 0) { + virProcessSetAffinity(vm->pid, cpumapToSet, false) < 0) { /* * We only want to error out if we failed to set the affinity to * user-requested mapping. If we are just trying to reset the affinity @@ -2743,7 +2743,8 @@ qemuProcessSetupPid(virDomainObjPtr vm, affinity_cpumask = use_cpumask; /* Setup legacy affinity. */ - if (affinity_cpumask && virProcessSetAffinity(pid, affinity_cpumask) < 0) { + if (affinity_cpumask && + virProcessSetAffinity(pid, affinity_cpumask, false) < 0) { /* * We only want to error out if we failed to set the affinity to * user-requested mapping. If we are just trying to reset the affinity @@ -2751,9 +2752,8 @@ qemuProcessSetupPid(virDomainObjPtr vm, * 1) libvirtd does not have CAP_SYS_NICE * 2) libvirtd does not run on all CPUs * - * However since this scenario is very improbable, we rather skip - * reporting the error because it helps running libvirtd in a a scenario - * where pinning is handled by someone else. + * This scenario can easily occurr when libvirtd is run inside a + * container with restrictive permissions and CPU pinning. * * See also: https://bugzilla.redhat.com/1819801#c2 */ diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 9de356505104..e9df56389621 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -441,7 +441,7 @@ int virProcessKillPainfully(pid_t pid, bool force) #if WITH_SCHED_GETAFFINITY -int virProcessSetAffinity(pid_t pid, virBitmapPtr map) +int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool quiet) { size_t i; int numcpus = 1024; @@ -479,9 +479,14 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map) numcpus = numcpus << 2; goto realloc; } - virReportSystemError(errno, - _("cannot set CPU affinity on process %d"), pid); - return -1; + if (quiet) { + VIR_DEBUG("cannot set CPU affinity on process %d: %s", + pid, g_strerror(errno)); + } else { + virReportSystemError(errno, + _("cannot set CPU affinity on process %d"), pid); + return -1; + } } CPU_FREE(mask); @@ -533,7 +538,8 @@ virProcessGetAffinity(pid_t pid) #elif defined(WITH_BSD_CPU_AFFINITY) int virProcessSetAffinity(pid_t pid, - virBitmapPtr map) + virBitmapPtr map, + bool quiet) { size_t i; cpuset_t mask; @@ -546,9 +552,14 @@ int virProcessSetAffinity(pid_t pid, if (cpuset_setaffinity(CPU_LEVEL_WHICH, CPU_WHICH_PID, pid, sizeof(mask), &mask) != 0) { - virReportSystemError(errno, - _("cannot set CPU affinity on process %d"), pid); - return -1; + if (quiet) { + VIR_DEBUG("cannot set CPU affinity on process %d: %s", + pid, g_strerror(errno)); + } else { + virReportSystemError(errno, + _("cannot set CPU affinity on process %d"), pid); + return -1; + } } return 0; @@ -582,8 +593,11 @@ virProcessGetAffinity(pid_t pid) #else /* WITH_SCHED_GETAFFINITY */ int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, - virBitmapPtr map G_GNUC_UNUSED) + virBitmapPtr map G_GNUC_UNUSED, + bool quiet G_GNUC_UNUSED) { + /* The @quiet parameter is ignored here, it is used only for silencing + * actual failures. */ virReportSystemError(ENOSYS, "%s", _("Process CPU affinity is not supported on this platform")); return -1; diff --git a/src/util/virprocess.h b/src/util/virprocess.h index 437deb18305b..34210d6c9d62 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -58,7 +58,7 @@ int virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay); -int virProcessSetAffinity(pid_t pid, virBitmapPtr map); +int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool quiet); virBitmapPtr virProcessGetAffinity(pid_t pid); -- 2.28.0

2 3

Various issues when using multiple graphic outputs
by Christian Ehrhardt 07 Sep '20

07 Sep '20

Hi, I've had continuous issues with this and wanted to reach out if that is a common issue everyone has or just me lacking a little detail on my setup. Setup: - tried qemu up to 4.2 - tried libvirt up to 6.0 - virt-viewer up to 7.0-2build1 - virt-manager up to 2.2.1 - I plan to retry with qemu 5.0, libvirt 6.6 and virt-viewer 9.0 but I don't have all pieces ready yet. - get a Desktop Guest (I've seen it with Ubuntu and Windows, therefore I'm rather sure it will affect all desktops) - configure your guest-xml to use multiple graphic adapters and start it - connect with virt-viewer to your guest which will open both screens The exact issue depends on the kind of graphic device I configure. The following list is ordered in terms of increasing confusion :-): - QXL with multiple heads - works fine (so multi display in general seems fine for the involved components) - VGA + QXL, QXL + Virtio, Virtio + QXL: - Guest only detects primary display - Can't convince X in the guest to use the second device as well - lspci lists both devices just fine - dmesg shows the kernel is initializing them, e.g. drm for virtio - 2*QXL - X has issues to initialize on user login - no error in the log, just hung - QXL + Mdev - mouse handling seems broken - in gnome I see no mouse pointer at all - in windows it is just strange - has offsets relative to mouse pointer entering - and can't move in one random direction (e.g. not up) - keyboard works just fine - I have played with adding/removing different input devices without much success. - I disabled the qxl display via xorg conf without the case getting any better The last case is what I initially tried and totally confused me at first. And to be clear, each of those graphics adapter types works fine if being attached "alone" on the guest (Except mdev alone, as I'm unable to convince libvirt to only attach the mdev without adding back a qxl graphics adapter). The Guest XML is the default that virt-manager gives me plus adding the second graphics adapter (example 2*QXL: https://paste.ubuntu.com/p/cVw8GVZ9dD/) If this is known in some way or even "yeah try version XY of component Foo" I'm happy about any hint/pointer you can give me. But if there is a chance that I'm the only one seeing it I'd like to understand why. Thanks in advance for thinking into this with me, Christian P.S. to avoid cross-posting I'll start with libvirt as it is in the middle of all involved components.

3 5

[PATCH] Fix linkage to libutil and libkvm on FreeBSD 11
by Daniel P. Berrangé 07 Sep '20

07 Sep '20

We are currently adding -lutil and -lkvm to the linker using the add_project_link_arguments method. On FreeBSD 11.4, this results in build errors because the args appear too early in the command line. We need to pass the libraries as dependencies so that they get placed at the same point in the linker args as other dependencies. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- meson.build | 16 +++++++--------- src/bhyve/meson.build | 15 +++++++++++---- src/util/meson.build | 26 +++++++++++++++++--------- 3 files changed, 35 insertions(+), 22 deletions(-) Using the CI patch I posted earlier to add FreeBSD 11: https://gitlab.com/berrange/libvirt/-/pipelines/185834799 diff --git a/meson.build b/meson.build index 1eadea33bf..c30ff187aa 100644 --- a/meson.build +++ b/meson.build @@ -1086,7 +1086,8 @@ endif # Check for BSD kvm (kernel memory interface) if host_machine.system() == 'freebsd' kvm_dep = cc.find_library('kvm') - add_project_link_arguments('-lkvm', language: 'c') +else + kvm_dep = disabler() endif libiscsi_version = '1.18.0' @@ -1203,11 +1204,9 @@ have_gnu_gettext_tools = false if not get_option('nls').disabled() have_gettext = cc.has_function('gettext') if not have_gettext - intl_lib = cc.find_library('intl', required: false) - have_gettext = intl_lib.found() - if have_gettext - add_project_link_arguments('-lintl', language: 'c') - endif + intl_dep = cc.find_library('intl', required: false) + else + intl_dep = disabler() endif if not have_gettext and get_option('nls').enabled() error('gettext() is required to build libvirt') @@ -1235,6 +1234,8 @@ if not get_option('nls').disabled() have_gnu_gettext_tools = true endif endif +else + intl_dep = disabler() endif numactl_dep = cc.find_library('numa', required: get_option('numactl')) @@ -1402,9 +1403,6 @@ if udev_dep.found() endif util_dep = cc.find_library('util', required: false) -if util_dep.found() - add_project_link_arguments('-lutil', language: 'c') -endif if not get_option('virtualport').disabled() if cc.has_header_symbol('linux/if_link.h', 'IFLA_PORT_MAX') diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build index 7d54718820..c382f64aee 100644 --- a/src/bhyve/meson.build +++ b/src/bhyve/meson.build @@ -14,15 +14,22 @@ driver_source_files += bhyve_sources stateful_driver_source_files += bhyve_sources if conf.has('WITH_BHYVE') + bhyve_driver_deps = [ + access_dep, + src_dep, + ] + if kvm_dep.found() + bhyve_driver_deps += kvm_dep + endif + if util_dep.found() + bhyve_driver_deps += util_dep + endif bhyve_driver_impl = static_library( 'virt_driver_bhyve_impl', [ bhyve_sources, ], - dependencies: [ - access_dep, - src_dep, - ], + dependencies: bhyve_driver_deps, include_directories: [ conf_inc_dir, hypervisor_inc_dir, diff --git a/src/util/meson.build b/src/util/meson.build index f7092cc3f1..c899f232e6 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -172,15 +172,7 @@ io_helper_sources = [ 'iohelper.c', ] -virt_util_lib = static_library( - 'virt_util', - [ - util_sources, - util_public_sources, - keycode_gen_sources, - dtrace_gen_headers, - ], - dependencies: [ +virt_util_deps = [ acl_dep, audit_dep, capng_dep, @@ -195,7 +187,23 @@ virt_util_lib = static_library( thread_dep, win32_dep, yajl_dep, + ] +if util_dep.found() + virt_util_deps += util_dep +endif +if intl_dep.found() + virt_util_deps += intl_dep +endif + +virt_util_lib = static_library( + 'virt_util', + [ + util_sources, + util_public_sources, + keycode_gen_sources, + dtrace_gen_headers, ], + dependencies: virt_util_deps, ) libvirt_libs += virt_util_lib -- 2.26.2

4 3