- Devel - libvirt List Archives

[libvirt] [PATCH] util: fix build on non-Linux
by Roman Bogorodskiy 23 Mar '17

23 Mar '17

Decorate unused arguments of the virNetDevGetMaster() stub with ATTRIBUTE_UNUSED to fix build on systems where this stub is used. --- Pushed as a build-breaker. src/util/virnetdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 6805d32..455239a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1017,7 +1017,8 @@ virNetDevGetMaster(const char *ifname, char **master) int -virNetDevGetMaster(const char *ifname, char **master) +virNetDevGetMaster(const char *ifname ATTRIBUTE_UNUSED, + char **master ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to get device master from netlink on this platform")); -- 2.7.4

1 0

[libvirt] [PATCH 1/1] qemu: snapshot: Forbid internal snapshots with OVMF firmware
by Peter Krempa 23 Mar '17

23 Mar '17

QEMU does not snapshot the pflash drive when doing a 'savevm' thus internal snapshots with OVMF would be incomplete. Forbid such snapshot so that we can avoid problems. --- CC: lersek(a)redhat.com There might be slight regression potential. While this did not work as expected I did not encounter any error while doing a savevm, thus users might actually think that this worked before. src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e065081d..344917451 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13873,6 +13873,16 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup; } + /* Internal snapshots don't work with VMs with OVMF loader since qemu does + * not snapshot the variable store */ + if (found_internal && + vm->def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("internal snapshots of a VM with pflash based " + "firmware are not supported by QEMU")); + goto cleanup; + } + /* Alter flags to let later users know what we learned. */ if (external && !active) *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; -- 2.12.1

2 2

[libvirt] [PATCH 0/3] allow fixing tap-bridge connections when network is restarted
by Laine Stump 23 Mar '17

23 Mar '17

It's been a long-standing problem that when you stop and restart a libvirt network, the guest tap devices are no longer connected to the network. Until now the only way to recover from this was to either shutdown and restart all the affected guests, or to detach and then re-attach all affected guest network devices. These patches permit the situation to be remedied by just restarting libvirtd - when the hypervisor drivers are notifying the network drivers of each connection, the network can retrieve the current "master" for the tap device and compare it to the network's bridge device. If they don't match, then it disconnects the tap from any incorrect bridge and connects to the correct bridge. *** For now, that's as far as we go - *semi*-automated (since you need to restart libvirtd for it to happen). My intent is for this to be completely automated, but the logic to do that is a bit "convoluted" so I've deferred trying to implement it until I've given it more thought - a few different trains of thought have led to dead-ends, and so far only one seems reasonably doable: * add a networkStartCallback list to the network driver state object. * as each hypervisor driver that uses the network driver is initialized, it will call an internal-only function in the network driver to register a callback. * whenever a network is started, it will call all the registered callback functions (sending the name of the network being started, and some HV-specific context pointer). * Each hypervisor's callback function will look through all of its active domains for interfaces that are using the given network, and for each such interface, will call networkNotifyActualDevice() (the function that has been updated in Patch 3/3 to reconnect taps to bridges). The "convoluted" part here is that we need to make sure there is enough (but not too much!) locking/refcounting both when adding items to the callback list (which should only be done single threaded, since it happens during the driver initializations) and when the networkStart() function (in some state of lockedness/refcountedness) calls over to some e.g. qemu function that will then need to do some amount of locking (at least on each domain as it is processed) and then calls back into the network driver (which will need "some amount" of locking/refcounting). Since we're calling from network driver into qemu into network driver, while the normal nesting is just calling from qemu into network, I want to be sure there is no possibility for a deadlock. (Also, I want to avoid making the list of callbacks any more complicated than absolutely necessary - the "in" thing to do these days seems to be to allocate a hash table, but there's a lot of extra code required for that (see util/virclosecallbacks.[hc]) which seems like overkill for a list of maximum 4-5 items that will *never* change after the driver initialization - if anyone has ideas/opinions about that, please speak up. Laine Stump (2): util: new function virNetDevGetMaster() util: new function virNetDevTapAttachBridge() root (1): network: reconnect tap devices during networkNotifyActualDevice src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 30 +++++++++++- src/util/virnetdev.c | 49 +++++++++++++++++++ src/util/virnetdev.h | 3 ++ src/util/virnetdevtap.c | 111 +++++++++++++++++++++++++++++--------------- src/util/virnetdevtap.h | 12 +++++ 6 files changed, 169 insertions(+), 38 deletions(-) -- 2.9.3

2 4

[libvirt] [PATCH 00/22] Resolve Coverity and build issues
by John Ferlan 23 Mar '17

23 Mar '17

Hopefully the quantity doesn't scare anyone off... These are mostly innocuous, but separated each out to appease the masses. The first patch fixes a recently uncovered Coverity warning regarding FORWARD_NULL of the 'formatStr'... According to an IRC response from pkrempa the format should be set up properly in qemuDomainSnapshotPrepare Patches 2-22 resolve various different build issues in the Coverity environment that sets "lv_cv_static_analysis=yes" during the build. This is 'reproducible' if one sets that as part of their autogen.sh processing - something that we should probably do in the CI build environment so we can get all those ATTRIBUTE_NONNULL checks to be run on a daily basis. John Ferlan (22): qemu: Always format formatStr for blockdev-snapshot-sync util: Remove NONNULL(1) for virNetDevGetName conf: Remove NONNULL(2) for virNetDevBandwidthParse conf: Remove NONNULL(1,2) for virNetDevBandwidthFormat conf: Remove NONNULL(1) for virDomainNumaGetNodeCount network: Remove null newBandwidth check from networkBandwidthUpdate qemu: Remove non null 'vm' check from qemuMonitorOpen qemu: Remove NONNULL(1) for qemu_monitor prototypes util: Change return argument for virBitmapParseUnlimited util: Remove NONNULL(1) for virBitmapParseUnlimited util: Remove NONNULL(2,3) for virHostdevReAttachPCIDevices util: Remove NONNULL(2,3) for virHostdevReAttachUSBDevices util: Remove NONNULL(2,3) for virHostdevReAttachSCSIVHostDevices util: Remove NONNULL(1) for virHostdevPrepareDomainDevices util: Remove NONNULL(1) for virHostdevReAttachDomainDevices util: Remove NONNULL(1) for virNetDevOpenvswitchSetMigrateData util: Remove NONNULL(1,3,4) from virTypedParamsFilter util: Remove NONNULL(2) for virNetDevBandwidthPlug util: Remove NONNULL(1) for virNetDevOpenvswitchGetVhostuserIfname util: Remove NONNULL's for virNetDevVPortProfile[Associate|Disassociate] util: Remove NONNULL(1) for virNetDevMacVLanDeleteWithVPortProfile cpu: Remove NONNULL(1) for cpuBaseline src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 5 ++--- src/conf/numa_conf.h | 4 ++-- src/cpu/cpu.h | 3 +-- src/network/bridge_driver.c | 2 +- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_monitor.c | 3 +-- src/qemu/qemu_monitor.h | 8 ++++---- src/util/virbitmap.c | 28 +++++++++++++--------------- src/util/virbitmap.h | 6 ++---- src/util/virhostdev.c | 9 ++++++--- src/util/virhostdev.h | 10 +++++----- src/util/virnetdev.h | 2 +- src/util/virnetdevbandwidth.h | 3 +-- src/util/virnetdevmacvlan.h | 2 +- src/util/virnetdevopenvswitch.h | 4 ++-- src/util/virnetdevvportprofile.h | 10 ++++------ src/util/virtypedparam.h | 3 +-- tests/testutils.c | 2 +- tests/virbitmaptest.c | 2 +- 20 files changed, 51 insertions(+), 60 deletions(-) -- 2.9.3

3 25

[libvirt] [PATCH 0/3] Three random small patches
by Laine Stump 23 Mar '17

23 Mar '17

I found these while trying out some other code I decided not to use (I was going to add more attributes to the <link> element, virNetDevIfLink, and virNetDevGetLinkInfo(), but then decided not to for now (in case Michal has a different idea of the meaning of "link" than, e.g., iproute's "ip link" command). The first two taken together actually have a very positive impact - they make the output of "virsh nodedev-dumpxml" much more informative when run as non-root. Laine Stump (3): util: use AF_UNIX family (not AF_PACKET) for ioctl sockets util: allow retrieving ethtool features when unprivileged network: remove unused typedef for networkDnsmasqLeaseFileNameFunc src/network/bridge_driver.h | 2 -- src/util/virnetdev.c | 8 +------- src/util/virnetdevip.c | 2 +- 3 files changed, 2 insertions(+), 10 deletions(-) -- 2.9.3

2 5

[libvirt] [PATCH v3 00/15] Introduce vGPU mdev framework to libvirt
by Erik Skultety 23 Mar '17

23 Mar '17

since v1: - new <hostdev> attribute model introduced which tells libvirt which device API should be considered when auto-assigning guest address - device_api is properly checked, thus taking the 'model' attribute only as a hint to assign "some" address - new address type 'mdev' is introduced rather than using plain <uuid> element, since the address element is more conveniently extendable. - the emulated mtty driver now works as well out of the box, so no HW needed to review this series --> let's try it :) - fixed all the nits from v1 since v2: - dropped the patch introducing new address type 'mdev' since I added by mistake and only after that realized that the device address type enum is used for guest addresses only --> the mdevs are still identified by address element containing an 'uuid' attribute, I just dropped the enum - resolved the driver hostdev list race condition raised by Pavel in his review --> the device API is now checked every time our internal mdev object is created as opposed to the previous version where because of the model being checked separately, the locking issues arose. - rewrote the docs, reflecting the mdev address type drop change - squashed all security related stuff into 1 patch, also added app-armor bits - as Pavel suggested, moved most of the mdev-related functions out of virhostdev.c to virmdev.c - added a few more test cases I also created a new branch 'mdev-next' (more suitable name than a strict version number) on https://github.com/eskultety/libvirt/commits/mdev-next Erik Erik Skultety (15): conf: hostdev: Enforce enum-in-switch compile-time checks conf: hostdev: Introduce virDomainHostdevSubsysSCSIClear conf: Introduce virDomainHostdevDefPostParse conf: Introduce new hostdev device type mdev conf: Add post parse code for mdevs to virDomainHostdevDefPostParse security: Enable labeling of vfio mediated devices conf: Enable cold-plug of a mediated device qemu: Assign PCI addresses for mediated devices as well hostdev: Maintain a driver list of active mediated devices qemu: cgroup: Adjust cgroups' logic to allow mediated devices qemu: Bump the memory locking limit for mdevs as well qemu: Format mdevs on qemu command line test: Add some test cases for our test suite regarding the mdevs docs: Document the new hostdev and address type 'mdev' news: Update the NEWS.xml about the new mdev feature docs/formatdomain.html.in | 46 +- docs/news.xml | 10 + docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/domain_conf.c | 225 ++++++++-- src/conf/domain_conf.h | 9 + src/libvirt_private.syms | 25 ++ src/qemu/qemu_command.c | 45 ++ src/qemu/qemu_command.h | 5 + src/qemu/qemu_domain.c | 24 +- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 14 +- src/qemu/qemu_hostdev.c | 56 +++ src/qemu/qemu_hostdev.h | 10 + src/qemu/qemu_hotplug.c | 2 + src/security/security_apparmor.c | 22 + src/security/security_dac.c | 43 ++ src/security/security_selinux.c | 45 ++ src/util/virhostdev.c | 165 ++++++- src/util/virhostdev.h | 23 + src/util/virmdev.c | 487 +++++++++++++++++++++ src/util/virmdev.h | 123 ++++++ tests/domaincapsschemadata/full.xml | 1 + ...ml2argv-hostdev-mdev-invalid-target-address.xml | 33 ++ ...muxml2argv-hostdev-mdev-src-address-invalid.xml | 35 ++ .../qemuxml2argv-hostdev-mdev-unmanaged.args | 25 ++ .../qemuxml2argv-hostdev-mdev-unmanaged.xml | 35 ++ tests/qemuxml2argvtest.c | 9 + .../qemuxml2xmlout-hostdev-mdev-unmanaged.xml | 40 ++ tests/qemuxml2xmltest.c | 1 + 31 files changed, 1528 insertions(+), 55 deletions(-) create mode 100644 src/util/virmdev.c create mode 100644 src/util/virmdev.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-mdev-invalid-target-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-mdev-src-address-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-mdev-unmanaged.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-mdev-unmanaged.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-mdev-unmanaged.xml -- 2.12.0

3 20

[libvirt] [BUG] mlock support breakage
by Luiz Capitulino 22 Mar '17

22 Mar '17

Libvirt commit c2e60ad0e51 added a new check to the XML validation logic where XMLs containing <memoryBacking><mlocked/> must also contain <memtune><hard_limit>. This causes two breakages where working guests won't start anymore: 1. Systems where mlock limit was set in /etc/security/limits.conf 2. Guests using hugeTLB pages. In this case, guests were relying on the fact that libvirt automagically increases mlock limit to 1GB While it's true that <memoryBacking><mlocked/> documentation says that <memtune><hard_limit> is required, this is actually an extremely bad request because: A. <memtune><hard_limit> own documention strongly recommends NOT using it B. <memtune><hard_limit> does more than setting memory locking limit C. <memtune><hard_limit> does not support infinity, so you have to guess a limit D. If <memtune><hard_limit> is less than 1GB, it will lower VFIO's automatic limit of "guest memory + 1GB" Here's two possible solutions to fix this all: 1. Drop change c2e60ad0e51 and drop automatic increases. Let users configure limits in /etc/security/limits.conf pros: this is the most correct way to do it, and how it should be done originally IMHO cons: will break working VFIO setups, so probably undoable 2. Drop change c2e60ad0e51 and automtically increase memory locking limit to infinity when seeing <memoryBacking><locked/> pros: make all cases work, no more <hard_limit> requirement cons: allows guests with <locked/> to lock all memory assigned to them plus QEMU allocations. While this seems undesirable or even a security issue, using <hard_limit> will have the same effect Lastly, <locked/> doesn't belong to <memoryBacking>, it should be in <memtune>. I recommend deprecating it from <memoryBacking> and adding it where it belongs.

5 18

[libvirt] [PATCH 0/3] Debug: Improve log messages.
by Prerna Saxena 22 Mar '17

22 Mar '17

Libvirt logs include many snippets for debugging daemon state, but some of those messages are either missing vital information or end up logging "errors" for normal operating conditions as well. This series improves log messages, also adding additional WARN'ings for connection instantiation and closure. Prerna Saxena (3): Debug: Add WARN'ing messages for when a client has opened/closed connection. Debug: Remove unnecessary errors reported while parsing non-existent sysfs files. Debug: Report VM errors more concisely. src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 18 ++++++++++++++---- src/util/virnetdev.c | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) -- 1.8.1.2

3 10

Re: [libvirt] [PATCH 3/3] Debug: Report VM errors more concisely.
by Prerna 22 Mar '17

22 Mar '17

Oops, sorry. Dropping the list was not intentional. I didnt realise I had sent a "reply" in place of "reply-all" . Adding back :) I still would argue that having the VM's name narrows down the problem space. If a client knows what operations have been fired for a VM over the last time window, it is not too difficult to deduce which of them could have caused this message to occur. As I said before, use of libvirt Debug logs adds way too much logspew. If we have more pointed error/warning messages, it can largely help debugging on its own. Even if we turn debugging on, libvirt honestly has way too many messages that look like an en-masse CTRL-C/V operation. Sample: All qemuMonitorEmit* except one have this message: VIR_DEBUG("mon=%p", mon); Even if this message shows up in logs, it is near impossible to trace which event was seen by the VM. And it is quite another task to see which VM this monitor belonged to. Not that more vebosity wont help here. However, I am only trying to make logs easier to work with. Every message should ideally contribute to something meaningful, else it has little value to add while debugging. On Wed, Mar 22, 2017 at 1:59 PM, Peter Krempa <pkrempa(a)redhat.com> wrote: > [Please don't drop the list on the responses.] > > On Wed, Mar 22, 2017 at 13:52:14 +0530, Prerna wrote: > > Always enabling debug logs adds a *lot* of logspew. > > It would be good to have self-sufficient error messages, dont you think ? > > What is wrong with adding a VM name field here ? > > It's mostly useless. You get the VM name, but you don't have the > operation that failed. So it's useless for debugging. >

1 0

[libvirt] [PATCH] cleanup: qemu_capabilities: remove redundant error messages
by Chen Hanxiao 22 Mar '17

22 Mar '17

From: Chen Hanxiao <chenhanxiao(a)gmail.com> We reported error in caller qemuConnectGetDomainCapabilities. So the same error messages in virQEMUCapsCacheLookupByArch are useless. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- src/qemu/qemu_capabilities.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5a3b4ac..eca46e7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5192,10 +5192,6 @@ virQEMUCapsCacheLookupByArch(virCapsPtr caps, virQEMUCapsCacheValidate(cache, binary, caps, &ret); VIR_FREE(binary); } - } else { - virReportError(VIR_ERR_INVALID_ARG, - _("unable to find any emulator to serve '%s' " - "architecture"), virArchToString(arch)); } virObjectRef(ret); -- 2.7.4

2 3