- Devel - libvirt List Archives

[libvirt] [PATCH 0/9] virsh: don't overwrite errors in virDomain(Snapshot)Free functions and other refactors
by Peter Krempa 12 Apr '17

12 Apr '17

Avoid the annoying problem that virDomainFree and virDomainSnapshotFree reset libvirt errors and thus errors from helper functions don't get reported. This seriesl also contains other refactors which I noticed along. Peter Krempa (9): virsh: Add new file for utility functions and move a few virsh-util: Move domain lookup helpers into virsh-util virsh-util: Add wrapper for virDomainFree virsh: Add wrapper for virDomainSnapshotFree vsh: Add helper for safe remembering of libvirt errors virsh: add helpers for getting domain XML for XPath purposes virsh-domain-monitor: Use the virsh wrappers for getting XML to simplify code virsh-domain: Use the virsh wrappers for getting XML to simplify code virsh-domain: Refactor cmdTTYConsole cfg.mk | 8 + po/POTFILES.in | 1 + tools/Makefile.am | 1 + tools/virsh-domain-monitor.c | 89 +++-------- tools/virsh-domain.c | 370 ++++++++++++++----------------------------- tools/virsh-domain.h | 12 -- tools/virsh-host.c | 1 - tools/virsh-snapshot.c | 74 ++++----- tools/virsh-util.c | 222 ++++++++++++++++++++++++++ tools/virsh-util.h | 78 +++++++++ tools/virsh-volume.c | 1 + tools/virsh.c | 41 ----- tools/virsh.h | 4 - tools/vsh.c | 15 ++ tools/vsh.h | 1 + 15 files changed, 493 insertions(+), 425 deletions(-) create mode 100644 tools/virsh-util.c create mode 100644 tools/virsh-util.h -- 2.12.2

2 18

[libvirt] [PATCH] qemu: Fix mdev checking for VFIO support
by Erik Skultety 12 Apr '17

12 Apr '17

Commit a4a39d90 added a check that checks for VFIO support with mediated devices. The problem is that the hostdev preparing functions behave like a fallthrough if device of that specific type doesn't exist. However, the check for VFIO support was independent of the existence of a mdev device which caused the guest to fail to start with any device to be directly assigned if VFIO was disabled/unavailable in the kernel. The proposed change first ensures that it makes sense to check for VFIO support in the first place, and only then performs the VFIO support check itself. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441291 Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- src/qemu/qemu_hostdev.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 685bf5b59..9b5504832 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -330,11 +330,19 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver, int nhostdevs) { virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + size_t i; - if (!qemuHostdevHostSupportsPassthroughVFIO()) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("host doesn't support VFIO PCI interface")); - return -1; + for (i = 0; i < nhostdevs; i++) { + if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) { + if (!supportsVFIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support VFIO PCI interface")); + return -1; + } + break; + } } return virHostdevPrepareMediatedDevices(hostdev_mgr, QEMU_DRIVER_NAME, -- 2.12.2

2 2

[libvirt] [PATCH] util: Resolve coverity issues
by Wang King 12 Apr '17

12 Apr '17

Coverity complains about virBufferCurrentContent might be return null when calling strlen, so check virBufferError first before calling strlen. --- src/util/virsystemd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index ceb1c1a..83737a2 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -135,6 +135,8 @@ virSystemdAppendValidMachineName(virBufferPtr buf, bool skip_dot = false; for (; *name; name++) { + if (virBufferError(buf)) + break; if (strlen(virBufferCurrentContent(buf)) >= 64) break; -- 2.8.3

2 2

[libvirt] [PATCH] Align vol-resize arguments with the output of help
by Martin Kletzander 12 Apr '17

12 Apr '17

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- tools/virsh.pod | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index d1030808a3ed..34679e363a8e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -4025,12 +4025,12 @@ I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-path> is the name or path of the volume to return the volume key for. -=item B<vol-resize> [I<--pool> I<pool-or-uuid>] I<vol-name-or-path> -I<pool-or-uuid> I<capacity> [I<--allocate>] [I<--delta>] [I<--shrink>] +=item B<vol-resize> I<vol-name-or-path> I<capacity> [I<--pool> I<pool-or-uuid>] +[I<--allocate>] [I<--delta>] [I<--shrink>] Resize the capacity of the given volume, in bytes. I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume -is in. I<vol-name-or-key-or-path> is the name or key or path of the volume +is in. I<vol-name-or-path> is the name or key or path of the volume to resize. The new capacity might be sparse unless I<--allocate> is specified. Normally, I<capacity> is the new size, but if I<--delta> is present, then it is added to the existing size. Attempts to shrink -- 2.12.2

2 3

[libvirt] [PATCH] interface: Fix resource leak in netcfConnectListAllInterfaces error path
by Wang King 12 Apr '17

12 Apr '17

On virGetInterface failure, call virInterfaceDefFree for the @def. --- src/interface/interface_backend_netcf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index 700a8a0..c7cc071 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -622,8 +622,10 @@ netcfConnectListAllInterfaces(virConnectPtr conn, } if (ifaces) { - if (!(iface_obj = virGetInterface(conn, def->name, def->mac))) + if (!(iface_obj = virGetInterface(conn, def->name, def->mac))) { + virInterfaceDefFree(def); goto cleanup; + } tmp_iface_objs[niface_objs] = iface_obj; } niface_objs++; -- 2.8.3

2 1

[libvirt] [PATCH] tools: don't leak @cpumap
by Wang King 12 Apr '17

12 Apr '17

==18591== 16 bytes in 1 blocks are definitely lost in loss record 41 of 183 ==18591== at 0x4C2B934: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==18591== by 0x54EBB1C: virAllocN (viralloc.c:191) ==18591== by 0x1628CA: _vshMalloc (vsh.c:136) ==18591== by 0x1344C4: virshVcpuPinQuery (virsh-domain.c:6603) ==18591== by 0x1344C4: cmdVcpuPin (virsh-domain.c:6707) ==18591== by 0x1631BF: vshCommandRun (vsh.c:1312) ==18591== by 0x12DBB1: main (virsh.c:961) --- tools/virsh-domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4b6c13c..e5fb11a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6618,6 +6618,7 @@ virshVcpuPinQuery(vshControl *ctl, } } + VIR_FREE(cpumap); return ret; } -- 2.8.3

2 1

Re: [libvirt] Determining domain job kind from job stats?
by Milan Zamazal 12 Apr '17

12 Apr '17

[Starting to move to the development list.] Milan Zamazal <mzamazal(a)redhat.com> writes: > Jiri Denemark <jdenemar(a)redhat.com> writes: > >> On Fri, Feb 17, 2017 at 12:38:24 +0100, Milan Zamazal wrote: >>> >>> There are basically two problems: >>> >>> - When the job completion callback is called, I need to distinguish what >>> kind of job was it to perform the appropriate actions. It would be >>> easier if I knew the job type directly in the callback (no need to >>> coordinate anything), but "external" job tracking is also possible. >> >> An immediate answer would be: "don't rely on the completion callback and >> just check the return value of the API which started the job", but I >> guess you want it because checking the return value is not possible when >> the process which started the job is not running anymore as described >> below. > > Well, avoiding using the completion callback is probably OK for me. Thinking about it more, it's not very nice: I have to use the callback to get the completed job stats (I'm not guaranteed the domain still exists on the source host when I ask it for the stats explicitly) *and* to track the jobs outside the callback to know whether the callback is related to the type of domain jobs I'm going to handle. Although not absolutely necessary, it would be much nicer if the job type was identified in the callback. > (In case of the process restart, I don't expect having everything > perfectly working, just some basic sanity.) > >>> - If I lost track of my jobs (e.g. because of a crash and restart), I'd >>> like to find out whether a given VM is migrating. Examining the job >>> looked like a good candidate to get the information, but apparently >>> it's not. Again, I can probably arrange things to handle that, but to >>> get the information directly from libvirt (not necessarily via job >>> info) would be easier and more reliable. >> >> Apparently you are talking about peer-to-peer migration, > > Yes. > >> otherwise the migration would be automatically canceled when the >> process which started it disappears. I'm afraid this is not currently >> possible in general. You might be able to get something by checking >> the domain's status, but it won't work in all cases. > > Too bad. Could some future libvirt version provide that information? If libvirt provided information about the job type, it would help with several things, for instance: With the callback problem above, with using libvirt as the ultimate single source of information about the VMs, or with handling VMs not running under complete control of a particular piece of software. If some piece of information about a VM is missing from libvirt I need to store it somewhere. Domain metadata is a natural place for that, since it's closely bound to the corresponding VM and keeps libvirt as the ultimate single source of information. But putting there information about migration is weird at best. Based on those thoughts I think libvirt should really provide a simple way to find out whether the VM is migrating to another host and to identify the domain job type in the job completed callback. Is there anything preventing to add that information? Thanks, Milan

3 7

[libvirt] virsh manpage bug: wrong argument order for virsh vol-resize
by Feiko Nanninga 12 Apr '17

12 Apr '17

According to the manpage the arguments for virsh vol-resize are: vol-resize [--pool pool-or-uuid] vol-name-or-path pool-or-uuid capacity […] But actually only vol-resize --pool pool-or-uuid vol-name-or-path capacity or vol-resize vol-name-or-path capacity pool-or-uuid are accepted. So <capacity> and <pool-or-uuid> are swapped. Tested on Debian 9 (stretch): $ virsh --version 3.0.0 Best regards, Feiko Nanninga

2 1

[libvirt] [PATCH v4 00/14] Introduce vGPU mdev framework to libvirt
by Erik Skultety 11 Apr '17

11 Apr '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 - created a new branch 'mdev-next' on my github (more suitable name than a strict version number) on https://github.com/eskultety/libvirt/commits/mdev-next since v3: - 'undo' an accidental squash of virmdev.{c,h} module introduction into patch 4/15 and made it a separate patch again - squash 5/15 into 4/15 as Pavel suggested - dropped the NEWS patch, as I've so far got at least 4 merge conflicts because of it when rebasing...I'll add it before the series is ready to be merged...or I'll forget about it like I usually do and add it later :/ Erik Erik Skultety (14): conf: hostdev: Enforce enum-in-switch compile-time checks conf: hostdev: Introduce virDomainHostdevSubsysSCSIClear conf: Introduce virDomainHostdevDefPostParse util: Introduce new module virmdev conf: Introduce new hostdev device type mdev 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' docs/formatdomain.html.in | 46 +- 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 + 30 files changed, 1518 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.1

5 41

[libvirt] Question about virsh domifaddr
by Vinod Chegu 11 Apr '17

11 Apr '17

Hi, In one of our test env. the virsh domifaddr <domain-name> --full doesn't provide any info about the interfaces of a live VM when the interfaces were configured on linux bridges (DHCP is being used to IPs). This command does return information when interfaces were using NAT. Is this expected behavior ? Thanks Vinod

2 1