March 2018 - Devel - libvirt List Archives

[libvirt] [GSoC] Design ideas for implementing cleanup attribute
by Sukrit Bhatnagar 01 Jun '18

01 Jun '18

Hi, I am interested in implementing the GCC cleanup attribute for automatic resource freeing as part of GSoC'18. I have shared a proposal for the same. This mail is to discuss the code design for implementing it. Here are some of my ideas: This attribute requires a cleanup function that is called automatically when the corresponding variable goes out of scope. There are some functions whose logic can be reused: - Functions such as virCommandFree, virConfFreeList and virCgroupFree can be directly used as cleanup functions. They have parameter and return type valid for a cleanup function. - Functions such as virFileClose and virFileFclose need some additional consideration as they return a value. I think we can set some global variable in a separate source file (just like errno variable from errno.h). Then the value to be returned can be accessed globally. - Functions such as virDomainEventGraphicsDispose need an entirely new design. They are used as callbacks in object classes and passed as an argument in virClassNew. This would require making changes to virObjectUnref's code too. *This is the part I am not sure how to implement cleanup logic for.* Also, since the __attribute__((__cleanup__(anyfunc))) looks ugly, a macro like autoclean (ideas for macro name welcome!) can be used instead. As Martin pointed out in my proposal, for some types, this can be done right after typedef declarations, so that the type itself contains this attribute. Basically, at most places where VIR_FREE is used to release memory explicitly, the corresponding variable can use the attribute. The existing virFree function also can be reused as it takes void pointer as an argument and returns nothing. One of the exceptions to this will be those variables which are struct members. The cleanup of member has to be done when the enclosing struct variable is cleaned. I can create new files vircleanup.{c,h} for defining cleanup functions for types which do not have an existing cleanup/free function. This can be done separately for each driver supported. For example, cleanups pertaining to lxc driver will be in src/lxc/lxc_cleanup.c. Your suggestions are welcome. Thanks, Sukrit Bhatnagar

7 25

[libvirt] [PATCH 0/3] qemu: use FD passing for monitor (and other unix sockets)
by Daniel P. Berrangé 16 Apr '18

16 Apr '18

This series makes use of the chardev fd passing arriving in QEMU 2.12 to get rid of the startup race wrt opening the QEMU monitor. The same thing benefits the agent too but I've not tackled that yet. Daniel P. Berrangé (3): qemu: probe for -chardev 'fd' parameter for FD passing qemu: support passing pre-opened UNIX socket listen FD qemu: don't retry connect() if doing FD passing src/qemu/qemu_capabilities.c | 4 ++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 58 ++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 54 ++++++++++++++++++++++++----------------- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 27 +++++++++++++++------ tests/qemumonitortestutils.c | 1 + 7 files changed, 114 insertions(+), 32 deletions(-) -- 2.14.3

3 9

[libvirt] [PATCH 0/9] Add Jansson support
by Ján Tomko 12 Apr '18

12 Apr '18

Prefer Jansson, but allow fallback/choice of yajl. Support for yajl can hopefully be dropped after we ditch CentOS 6 which has no Jansson. Ján Tomko (9): virmacmaptest: depend on yajl for 'empty' test virjsontest: Use a more stable floating point number for testing configure: rename LIBVIRT_*_YAJL to LIBVIRT_*_JSON Introduce WITH_JSON Introduce JSON_CFLAGS and JSON_LIBS virt-json.m4: generalize yajl dependency build: link setuid_rpc_client against JSON_LIBS virjson: add support for Jansson virt-json.m4: simplify QEMU check configure.ac | 10 +- m4/virt-json.m4 | 86 ++++++++++++++++ m4/virt-yajl.m4 | 55 ----------- src/Makefile.am | 9 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/util/Makefile.inc.am | 4 +- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 12 +-- tests/cputest.c | 16 +-- tests/libxlxml2domconfigtest.c | 4 +- tests/qemuagenttest.c | 2 +- tests/qemucapabilitiestest.c | 2 +- tests/qemucaps2xmltest.c | 2 +- tests/qemucommandutiltest.c | 2 +- tests/qemuhelptest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 2 +- tests/virjsontest.c | 2 +- tests/virmacmaptest.c | 3 + tests/virmocklibxl.c | 4 +- tests/virnetdaemontest.c | 2 +- tests/virstoragetest.c | 4 +- 23 files changed, 354 insertions(+), 96 deletions(-) create mode 100644 m4/virt-json.m4 delete mode 100644 m4/virt-yajl.m4 -- 2.16.1

3 22

[libvirt] virNetSocketNewListenTCP tries just one address
by Olaf Hering 12 Apr '18

12 Apr '18

To rescue this bug from the noise in a subthread: If a hostname resolves to more than one address and the host currently configured itself for just IPv4, doing a bind() to some IPv6 address will fail. As a result an error is returned instead of continuing with the next item in 'runp'. > Mär 20 09:35:52 macintyre-old libvirtd[4527]: 2018-03-20 08:35:52.521+0000: 4531: error : virNetSocketNewListenTCP:389 : Unable to bind to port: Cannot assign requested address After further debugging: 27672 16:04:46.774906 socket(PF_INET6, SOCK_STREAM, IPPROTO_TCP) = 35 27672 16:04:46.775041 setsockopt(35, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 27672 16:04:46.775172 setsockopt(35, SOL_IPV6, IPV6_V6ONLY, [1], 4) = 0 27672 16:04:46.775302 bind(35, {sa_family=AF_INET6, sin6_port=htons(49152), inet_pton(AF_INET6, "2620:113:80c0:8000:10:161:8:197", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EADDRNOTAVAIL (Cannot assign requested address) 27672 16:04:46.775455 gettid() = 27672 27672 16:04:46.775590 write(4, "2018-03-20 15:04:46.775+0000: 27672: error : virNetSocketNewListenTCP:389 : Unable to bind to port: Cannot assign requested address\n", 132) = 132 27672 16:04:46.775742 gettid() = 27672 27672 16:04:46.775875 write(4, "2018-03-20 15:04:46.775+0000: 27672: info : virObjectUnref:350 : OBJECT_UNREF: obj=0x7fa4bc003530\n", 98) = 98 27672 16:04:46.776026 gettid() = 27672 So for some reason libvirtd tries to bind to ipv6 even if the host does not have that ipv6 address at this point, only the link-local address. Not sure if there is a way to detect that within libvirt. Perhaps it should just move on with the runp list and try the next one? Olaf

3 6

[libvirt] [PATCH v6 0/9] Add setting CPU features (CPUID) with libxenlight driver.
by Marek Marczykowski-Górecki 11 Apr '18

11 Apr '18

Add support for CPUID setting based on <cpu> element. Since libxl format support only adjusting specific bits over host CPU, only mode='host-passthrough' is supported - other values are rejected (including default 'custom'). This will break some configurations working before (bare <cpu> element with for example NUMA configuration), but libxl driver never supported full 'custom' mode - it was silently ignored, which might lead to some unexpected effects. Since mode='host-passthrough' is now necessary to specify CPU options, do not enable nested HVM feature by mere presence of this element, require also enabling it in libxl.conf. Nested HVM is still in "preview" state, so better be explicit here. v2 of this patch series: https://www.redhat.com/archives/libvir-list/2017-July/msg00050.html v3 of this patch series: https://www.redhat.com/archives/libvir-list/2017-December/msg00314.html v4 of this patch series: https://www.redhat.com/archives/libvir-list/2018-February/msg00504.html v5 of this patch series: https://www.redhat.com/archives/libvir-list/2018-March/msg00796.html Marek Marczykowski-Górecki (9): libxl: fix libxlDriverConfigDispose for partially constructed object libxl: pass driver config to libxlMakeDomBuildInfo libxl: warn about ignored CPU mode=custom libxl: do not enable nested HVM unless global nested_hvm option enabled xenconfig: do not override def->cpu if already set elsewhere libxl: add support for CPUID features policy tests: check CPU features handling in libxl driver xenconfig: add CPUID handling to domXML <-> xl.cfg conversion tests: add test case for CPUID in xenconfig driver src/libxl/libvirtd_libxl.aug | 2 +- src/libxl/libxl.conf | 8 +- src/libxl/libxl_conf.c | 66 +++- src/libxl/libxl_conf.h | 6 +- src/libxl/libxl_domain.c | 2 +- src/libxl/test_libvirtd_libxl.aug.in | 1 +- src/xenconfig/xen_xl.c | 236 ++++++++++++++-- src/xenconfig/xen_xl.h | 2 +- tests/libxlxml2domconfigdata/fullvirt-cpuid.json | 64 ++++- tests/libxlxml2domconfigdata/fullvirt-cpuid.xml | 37 +++- tests/libxlxml2domconfigtest.c | 27 +- tests/virmocklibxl.c | 25 ++- tests/xlconfigdata/test-fullvirt-cpuid.cfg | 25 ++- tests/xlconfigdata/test-fullvirt-cpuid.xml | 35 ++- tests/xlconfigtest.c | 1 +- 15 files changed, 492 insertions(+), 45 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.json create mode 100644 tests/libxlxml2domconfigdata/fullvirt-cpuid.xml create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-cpuid.xml base-commit: 6ce3acc129bfdbe7fd02bcb8bbe8af6d13903684 -- git-series 0.9.1

3 35

[libvirt] [PATCH v2 0/9] Be consistent with vir*Obj*Remove* APIs
by John Ferlan 10 Apr '18

10 Apr '18

v1: https://www.redhat.com/archives/libvir-list/2018-March/msg01295.html NB: This can wait until 4.2.0 is release, but I figured I'd post this now just to put it on the radar and of course in hopes that someone will look during the idle moment or two before the release. Changes since v1: Short story: Rework the processing of the code Longer story: In his review Erik noted that there's a "fire dance" when processing the vir*Obj*Remove APIs of requiring a locked object upon entry, then adding a reference to that object, unlocking the object, locking the list to which it is contained, and then relocking the object. So it took some time to think about it and during one lengthy meeting today I had the aha moment that the *Remove API's could take the same key (e.g., uuid or name) used to Add or Find the object and use it for the Remove API. This allows the Remove API to not require a locked (and reffed) object upon entry and perform the lock dance, remove the object, and return just just a reffed object forcing the caller to know to Unref object. Instead, let's simplify things. The *Remove API can take the key, Find the object in the list, remove it from the hash tables, and dispose of the object. In essence the antecedent to the Add or AssignDef API's taking a def, creating an object, and adding it the object to the hash table(s). If there are two *Remove threads competing, one will win and perform the removal, while the other will call *Remove, but won't find the object in the hash table, and just return none the wiser. And yes, I think this can also work for the Domain code, but it's going to take a few patch series to get there as that code is not consistent between consumers. John Ferlan (9): secret: Rework LoadAllConfigs secret: Alter virSecretObjListRemove processing interface: Alter virInterfaceObjListRemove processing nodedev: Alter virNodeDeviceObjListRemove processing conf: Clean up virStoragePoolObjLoad error processing storage: Clean up storagePoolCreateXML error processing test: Clean up testStoragePoolCreateXML error processing conf: Move virStoragePoolObjRemove closer to AssignDef storagepool: Alter virStoragePoolObjRemove processing src/conf/virinterfaceobj.c | 26 +++++++---- src/conf/virinterfaceobj.h | 2 +- src/conf/virnodedeviceobj.c | 29 +++++++----- src/conf/virnodedeviceobj.h | 2 +- src/conf/virsecretobj.c | 58 +++++++++++------------- src/conf/virsecretobj.h | 2 +- src/conf/virstorageobj.c | 92 +++++++++++++++++++++++--------------- src/conf/virstorageobj.h | 2 +- src/node_device/node_device_hal.c | 16 ++++--- src/node_device/node_device_udev.c | 13 ++++-- src/secret/secret_driver.c | 15 ++++--- src/storage/storage_driver.c | 65 +++++++++++++++------------ src/test/test_driver.c | 78 +++++++++++++++++--------------- 13 files changed, 225 insertions(+), 175 deletions(-) -- 2.13.6

2 12

[libvirt] Race condition between qemuDomainCreate and qemuDomainDestroy
by Marc Hartmayer 10 Apr '18

10 Apr '18

Hi, there is a race condition between 'qemuDomainCreate' and 'qemuDomainDestroy' causing a NULL pointer segmentation fault when accessing priv->monConfig. The race condition can be easily reproduced using gdb. (gdb) set non-stop on # set breakpoint on line 'mon = qemuMonitorOpen(vm, …)' (gdb) b qemu_process.c:1799 # Actually, this second breakpoint is optional but it’s good to see where priv->monConfig is set to NULL # set breakpoint on line priv->monConfig = NULL; (gdb) b qemu_process.c:6589 (gdb) run # continue all threads - just for the case we hit a breakpoint already (gdb) c -a Now start a domain (that is using QEMU) $ virsh start domain The first breakpoint will be hit. Now run in a second shell $ virsh destroy domain The second breakpoint will be hit. Continue the thread where the second breakpoint was hit (for this example this is thread 4) (gdb) thread apply 4 continue Now continue the thread where the first breakpoint was hit. => Segmentation fault because of a NULL pointer dereference at config->value Since I'm not very familiar with that part of the code, I wanted to ask for your advice. Thanks in advance. Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

3 5

[libvirt] [RFC] Defining firmware (OVMF, et al) metadata format & file
by Kashyap Chamarthy 10 Apr '18

10 Apr '18

Problem background ------------------ The various OVMF binary file names and paths are slightly different[+] for each Linux distribution. And each high-level management tool (libguestfs, oVirt, `virt-manager` and OpenStack Nova) is inventing its own approach to detect and configure the said OVMF files. This email thread is about arriving at some common understanding to make this a bit more "unified" by addressing these needs in QEMU and libvirt. Suggested approach ------------------ Based on an upstream discussion on 'virt-tools'[1] mailing list and some Bugzillas, Gerd Hoffmann, Laszlo Ersek and Dan Berrangé had a suggestion to define a firmware metadata format and file (example in [1]): - For each firmware file we need a metadata file in a well defined location, e.g. /usr/share/qemu/bios/ that lists stuff like: - Path to the firmware binary - Path to the pre-built OVMF 'vars' file (if any) - Support architectures - associated QEMU feature flags (Secure Boot) - If the binary provides / requires SMM (System Management Mode) Essentially, QEMU would define[*] the file format, and provide metadata files from any ROMs it ships directly. If Linux distributions / vendors ship extra ROMs like OVMF, etc then they should provide suitable metadata files. - Libvirt can read these metadata files and then pick the correct firmware binary based on the settings for the guest. - Management tools can then wire up the libvirt-based OVMF SB (Secure Boot) configuration. [*] Open question: Who, between QEMU and libvirt, should define the said firmware metadata format and file? References ---------- [1] A past proposal from Gerd to create a sort of a "firmware registry" https://www.redhat.com/archives/virt-tools-list/2014-September/msg00145.html [2] A libvirt upstream RFE bug requesting to simplify handling UEFI https://bugzilla.redhat.com/show_bug.cgi?id=1295146 -- RFE: provide a bios=uefi XML convenience option [3] DanPB wrote a PoC in libvirt: https://www.redhat.com/archives/libvir-list/2016-October/msg00045.html -- [libvirt] [PATCH 0/3] Make UEFI firmware config simpler But later came to the conclusion that it is flawed, and said we go the route of "Suggested approach" mentioned earlier). [*] OVMF package path names for different distributions ------------------------------------------------------- - Debian (https://packages.debian.org/stretch/all/ovmf/filelist) - /usr/share/OVMF/OVMF_CODE.fd - /usr/share/OVMF/OVMF_VARS.fd - OpenSUSE (package: "qemu-ovmf-x86_64" -- and it has *35* files in that package): - /usr/share/qemu/ovmf-x86_64-opensuse-code.bin - /usr/share/qemu/ovmf-x86_64-opensuse-vars.bin [...] Their RPM spec file gives some hints (where all the files with 'ms' means signed with MS keys; the files with 'opensuse-4096' means signed with OpenSUSE 4096 bit CA keys -- I think that's one of the points of Secure Boot, to let people have control over system keys): https://build.opensuse.org/package/view_file/openSUSE:Factory/ovmf/ovmf.spe… - Fedora 27 (package: "edk2-ovmf", x86_64): - /usr/share/edk2/ovmf/OVMF_CODE.fd - /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd - /usr/share/edk2/ovmf/OVMF_VARS.fd - RHEL-7.4 (package: "OVMF", x86_64): - /usr/share/OVMF/OVMF_CODE.secboot.fd - /usr/share/OVMF/OVMF_VARS.fd - Gerd's firmware repo from Git: - /usr/share/edk2.git/ovmf-x64/OVMF_VARS-need-smm.fd - /usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd -- /kashyap

5 20

[libvirt] [PATCH 0/2] Change virCloseCallback typedef to return void
by John Ferlan 09 Apr '18

09 Apr '18

Slightly related to some current work to clean up the Add/Remove domain object list processing - as it turns out the close callbacks run code didn't even pay attention to what was returned and it really didn't need to - so let's just call it what it is a void function and let the virCloseCallbacksRun code handle the object manipulation (lock/ref and unlock/unref). John Ferlan (2): qemu: Fix qemuProcessAutoDestroy util: Alter virCloseCallback typedef to return void src/bhyve/bhyve_process.c | 8 ++------ src/lxc/lxc_process.c | 8 ++------ src/qemu/qemu_migration.c | 7 ++----- src/qemu/qemu_process.c | 10 ++-------- src/util/virclosecallbacks.c | 5 ++--- src/util/virclosecallbacks.h | 6 +++--- 6 files changed, 13 insertions(+), 31 deletions(-) -- 2.13.6

2 5

[libvirt] [PATCH 00/11] qemu: Implement pcie-to-pci-bridge controller
by Andrea Bolognani 09 Apr '18

09 Apr '18

Patches 1-4 are cleanups and preparation. For more information, take a peek at patch 8's commit message, which also includes a link to the relevant Bugzilla entry. Andrea Bolognani (11): docs: Tweak PCI controller model documentation tests: Add aarch64-traditional-pci test conf: Remove dubious code from virDomainPCIAddressSetGrow() conf: Rename virDomainPCIAddressSet.areMultipleRootsSupported qemu: Add QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE qemu: Implement pcie-to-pci-bridge controller conf: Add virDomainPCIAddressSet.isPCIeToPCIBridgeSupported conf: Prefer pcie-to-pci-bridge to dmi-to-pci-bridge tests: Use pcie-to-pci-bridge for aarch64-traditional-pci news: Add 4.3.0 release news: Update for pcie-to-pci-bridge support docs/formatdomain.html.in | 21 ++--- docs/news.xml | 17 ++++ docs/schemas/domaincommon.rng | 3 + src/conf/domain_addr.c | 95 ++++++++++++++++------ src/conf/domain_addr.h | 8 +- src/conf/domain_conf.c | 3 + src/conf/domain_conf.h | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 17 ++++ src/qemu/qemu_domain_address.c | 14 +++- tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + .../qemuxml2argvdata/aarch64-traditional-pci.args | 27 ++++++ tests/qemuxml2argvdata/aarch64-traditional-pci.xml | 19 +++++ tests/qemuxml2argvtest.c | 9 ++ .../qemuxml2xmloutdata/aarch64-traditional-pci.xml | 43 ++++++++++ tests/qemuxml2xmltest.c | 9 ++ 19 files changed, 251 insertions(+), 42 deletions(-) create mode 100644 tests/qemuxml2argvdata/aarch64-traditional-pci.args create mode 100644 tests/qemuxml2argvdata/aarch64-traditional-pci.xml create mode 100644 tests/qemuxml2xmloutdata/aarch64-traditional-pci.xml -- 2.14.3

3 34