[libvirt] [PATCH] qemuBuildHostNetStr: Don't leak @addr
by Michal Privoznik
The virSocketAddrFormat() allocates the string and it's caller
responsibility to free it afterwards.
==28857== 11 bytes in 1 blocks are definitely lost in loss record 37 of 168
==28857== at 0x4C2BEDF: malloc (vg_replace_malloc.c:299)
==28857== by 0x9A81D79: strdup (in /lib64/libc-2.23.so)
==28857== by 0x5DA3BF0: virStrdup (virstring.c:902)
==28857== by 0x5D96182: virSocketAddrFormatFull (virsocketaddr.c:427)
==28857== by 0x5D95E13: virSocketAddrFormat (virsocketaddr.c:352)
==28857== by 0x5706890: qemuBuildHostNetStr (qemu_command.c:3891)
==28857== by 0x57138D3: qemuBuildInterfaceCommandLine (qemu_command.c:8597)
==28857== by 0x5713D6A: qemuBuildNetCommandLine (qemu_command.c:8699)
==28857== by 0x57176F6: qemuBuildCommandLine (qemu_command.c:10027)
==28857== by 0x5769D61: qemuProcessCreatePretendCmd (qemu_process.c:6004)
==28857== by 0x4056EC: testCompareXMLToArgv (qemuxml2argvtest.c:502)
==28857== by 0x41DF40: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9b3e3fc04..abeb24846 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3900,6 +3900,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
if (ip->prefix)
virBufferAsprintf(&buf, "/%u", ip->prefix);
virBufferAddChar(&buf, ',');
+ VIR_FREE(addr);
}
break;
--
2.13.5
7 years, 2 months
[libvirt] [PATCH v2] qemu: Introduce a wrapper over virFileWrapperFdClose
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1448268
When migrating to a file (e.g. when doing 'virsh save file'),
couple of things are happening in the thread that is executing
the API:
1) the domain obj is locked
2) iohelper is spawned as a separate process to handle all I/O
3) the thread waits for iohelper to finish
4) the domain obj is unlocked
Now, the problem is that while the thread waits in step 3 for
iohelper to finish this may take ages because iohelper calls
fdatasync(). And unfortunately, we are waiting the whole time
with the domain locked. So if another thread wants to jump in and
say copy the domain name ('virsh list' for instance), they are
stuck.
The solution is to unlock the domain whenever waiting for I/O and
lock it back again when it finished.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
diff to v1:
- check after locking the domain obj back if the domain is still alive,
since it was unlocked it might have changed its state.
src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e1a0dd553..6095a5ec5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3216,6 +3216,31 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
goto cleanup;
}
+
+static int
+qemuFileWrapperFDClose(virDomainObjPtr vm,
+ virFileWrapperFdPtr fd)
+{
+ int ret;
+
+ /* virFileWrapperFd uses iohelper to write data onto disk.
+ * However, iohelper calls fdatasync() which may take ages to
+ * finish. Therefore, we shouldn't be waiting with the domain
+ * object locked. */
+
+ virObjectUnlock(vm);
+ ret = virFileWrapperFdClose(fd);
+ virObjectLock(vm);
+ if (!virDomainObjIsActive(vm)) {
+ if (!virGetLastError())
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("domain is no longer running"));
+ ret = -1;
+ }
+ return ret;
+}
+
+
/* Helper function to execute a migration to file with a correct save header
* the caller needs to make sure that the processors are stopped and do all other
* actions besides saving memory */
@@ -3276,7 +3301,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
goto cleanup;
if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0 ||
@@ -3827,7 +3852,7 @@ doCoreDump(virQEMUDriverPtr driver,
path);
goto cleanup;
}
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
goto cleanup;
ret = 0;
@@ -6687,7 +6712,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path,
false, QEMU_ASYNC_JOB_START);
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
VIR_WARN("Failed to close %s", path);
qemuProcessEndJob(driver, vm);
@@ -6962,7 +6987,7 @@ qemuDomainObjRestore(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path,
start_paused, asyncJob);
- if (virFileWrapperFdClose(wrapperFd) < 0)
+ if (qemuFileWrapperFDClose(vm, wrapperFd) < 0)
VIR_WARN("Failed to close %s", path);
cleanup:
--
2.13.5
7 years, 2 months
[libvirt] [PATCH 0/2] qemu: Order PCI controllers on the command line
by Andrea Bolognani
All the details are in the commit message for patch 2/2.
Andrea Bolognani (2):
qemu: Clean up qemuBuildControllerDevCommandLine()
qemu: Order PCI controllers on the command line
src/qemu/qemu_command.c | 91 +++++++++++++++-------
.../qemuxml2argv-pci-autoadd-idx.args | 2 +-
.../qemuxml2argv-pseries-many-buses-2.args | 4 +-
3 files changed, 68 insertions(+), 29 deletions(-)
--
2.13.5
7 years, 2 months
[libvirt] [PATCH 0/6] Fix guest CPU updates for inactive domains
by Jiri Denemark
This series originally started as a cleanup work, but it turned out two
real bugs got fixed with this cleanup :-)
Since commit v2.2.0-199-g7ce711a30e libvirt stores an updated guest CPU
in domain's live definition and there's no need to update it every time
we want to format the definition. Thus libvirt should never internally
request a guest CPU update when formatting domain XML. But if it is
asked by a user to do so, it should use the same host CPU model which is
used when starting a domain to provide meaningful results.
https://bugzilla.redhat.com/show_bug.cgi?id=1481309
https://bugzilla.redhat.com/show_bug.cgi?id=1485022
Jiri Denemark (6):
qemuxml2xmltest: Add tests for Power CPUs
cpu_conf: Drop updateCPU from virCPUDefFormat
cpu_conf: Simplify formatting of guest CPU attributes
conf: Drop unused VIR_DOMAIN_DEF_FORMAT_UPDATE_CPU
qemu: Use correct host model for updating guest cpu
qemu: Don't update CPU when formatting live def
src/bhyve/bhyve_driver.c | 2 +-
src/conf/capabilities.c | 2 +-
src/conf/cpu_conf.c | 38 +++++++++------------
src/conf/cpu_conf.h | 9 ++---
src/conf/domain_capabilities.c | 2 +-
src/conf/domain_conf.c | 6 +---
src/conf/domain_conf.h | 1 -
src/conf/snapshot_conf.c | 3 +-
src/libxl/libxl_driver.c | 2 +-
src/qemu/qemu_domain.c | 17 +++++++---
src/qemu/qemu_domain.h | 3 +-
src/qemu/qemu_driver.c | 9 ++++-
src/qemu/qemu_migration_cookie.c | 2 +-
src/test/test_driver.c | 2 +-
src/vz/vz_driver.c | 2 +-
tests/cputest.c | 15 ++++-----
.../ppc64-host+guest-compat-incompatible.xml | 2 +-
.../ppc64-host+guest-compat-invalid.xml | 2 +-
.../cputestdata/ppc64-host+guest-compat-valid.xml | 2 +-
.../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 38 +++++++++++++++++++++
.../qemuxml2xmlout-pseries-cpu-compat.xml | 38 +++++++++++++++++++++
.../qemuxml2xmlout-pseries-cpu-exact.xml | 39 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 4 +++
23 files changed, 178 insertions(+), 62 deletions(-)
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat-power9.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml
--
2.14.1
7 years, 2 months
[libvirt] [PATCH 0/5] Reject parallel ports on s390, and pseries
by Pino Toscano
Hi,
this is a simple series to reject parallel ports on s390 architectures,
and pseries machines -- both simply do not support them. This fixes
https://bugzilla.redhat.com/show_bug.cgi?id=1487499
Pino Toscano (5):
tests: qemuxml2argv: fix expected type for usb-bus-missing
tests: qemuxml2argv: fail also on unexpected pass
qemu: pass the virDomainDef to qemuDomainChrDefValidate
qemu: reject parallel ports for s390 archs
qemu: reject parallel ports for pseries machines
src/qemu/qemu_domain.c | 16 ++++++++++++----
.../qemuxml2argv-pseries-no-parallel.xml | 18 ++++++++++++++++++
.../qemuxml2argv-s390-no-parallel.xml | 22 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 20 +++++++++++++++++---
4 files changed, 69 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-no-parallel.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-no-parallel.xml
--
2.13.5
7 years, 2 months
[libvirt] [PATCH] qemu: capabilities: Remove support for downstream-only QMP monitor backport
by Peter Krempa
Some distros (see diff) chose to backport QMP support rather than rebase
to newer version of qemu. As a hack they added the string 'libvirt' to
the qemu -help output. Remove this as downstream-only hacks should be
carried by downstream and not litter upstream.
This effectively reverts commit ff88cd590572277f10ecee4ebb1174d9b70fc0d7
---
src/qemu/qemu_capabilities.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7bfb71420..085910dd4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1335,8 +1335,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
if ((netdev = strstr(help, "-netdev"))) {
/* Disable -netdev on 0.12 since although it exists,
* the corresponding netdev_add/remove monitor commands
- * do not, and we need them to be able to do hotplug.
- * But see below about RHEL build. */
+ * do not, and we need them to be able to do hotplug. */
if (version >= 13000) {
if (strstr(netdev, "bridge"))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE);
@@ -1369,20 +1368,10 @@ virQEMUCapsComputeCmdFlags(const char *help,
/* While JSON mode was available in 0.12.0, it was too
* incomplete to contemplate using. The 0.13.0 release
* is good enough to use, even though it lacks one or
- * two features. This is also true of versions of qemu
- * built for RHEL, labeled 0.12.1, but with extra text
- * in the help output that mentions that features were
- * backported for libvirt. The benefits of JSON mode now
- * outweigh the downside.
- */
+ * two features. */
#if WITH_YAJL
- if (version >= 13000) {
- virQEMUCapsSet(qemuCaps, QEMU_CAPS_MONITOR_JSON);
- } else if (version >= 12000 &&
- strstr(help, "libvirt")) {
+ if (version >= 13000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_MONITOR_JSON);
- virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV);
- }
#else
/* Starting with qemu 0.15 and newer, upstream qemu no longer
* promises to keep the human interface stable, but requests that
@@ -1392,8 +1381,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
* telling them to recompile (the spec file includes the
* dependency, so distros won't hit this). This check is
* also in m4/virt-yajl.m4 (see $with_yajl). */
- if (version >= 15000 ||
- (version >= 12000 && strstr(help, "libvirt"))) {
+ if (version >= 15000) {
if (check_yajl) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("this qemu binary requires libvirt to be "
--
2.13.5
7 years, 2 months
[libvirt] [PATCH] Fix commandhelper build on win32
by Daniel P. Berrange
For win32 we need EXIT_AM_SKIP which is in testutils.h. We must
define NO_LIBVIRT to prevent replacement of fprintf with
virFilePrintf as we can't link to libvirt_util.la
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
Pushed as a build fix
tests/commandhelper.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index e9e6353f3..1da2834aa 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -28,6 +28,8 @@
#include <sys/stat.h>
#include "internal.h"
+#define NO_LIBVIRT
+#include "testutils.h"
#ifndef WIN32
--
2.13.5
7 years, 2 months
[libvirt] [PATCH libvirt-glib 0/5] Portability fixes
by Andrea Bolognani
Currently, trying to build libvirt-glib on FreeBSD results in all
sorts of interesting failures.
After applying this series, it's possible to build libvirt-glib
and run the test suite successfully.
There's even a couple of clean-ups thrown in for good measure :)
Andrea Bolognani (5):
maint: Drop autobuild.sh
configure: Bump required libvirt version to 1.2.5
configure: Look for Perl interpreter path
tests: Don't rely on non-portable paths
scripts: Fix sha-bang lines
autobuild.sh | 91 -------------------------------------------
build-aux/check-symfile.pl | 2 +-
build-aux/check-symsorting.pl | 2 +-
configure.ac | 4 +-
examples/config-demo.py | 2 +-
examples/conn-test.js | 2 +-
examples/conn-test.py | 2 +-
examples/event-test.py | 1 +
tap-driver.sh | 2 +-
tap-test | 2 +-
tests/test-events.c | 2 +-
11 files changed, 12 insertions(+), 100 deletions(-)
delete mode 100755 autobuild.sh
--
2.13.5
7 years, 2 months
[libvirt] Questions about function virPCIDeviceIsBehindSwitchLackingACS in virpci.c
by Wuzongyong (Euler Dept)
Hi,
In function virPCIDeviceIsBehindSwitchLackingACS, I noticed that(line 8):
1 if (virPCIDeviceGetParent(dev, &parent) < 0)
2 return -1;
3 if (!parent) {
4 /* if we have no parent, and this is the root bus, ACS doesn't come
5 * into play since devices on the root bus can't P2P without going
6 * through the root IOMMU.
7 */
8 if (dev->address.bus == 0) {
9 return 0;
10 } else {
11 virReportError(VIR_ERR_INTERNAL_ERROR,
12 _("Failed to find parent device for %s"),
13 dev->name);
14 return -1;
15 }
16 }
Why we just return 0 only if device's bus is 0?
In my server, I can see a root bus which bus number is greater than 0, see the
results(just a part) after I run lspci -t:
+-[0000:80]-+-02.0-[81-83]--+-00.0
| | \-00.1
| +-05.0
| +-05.1
| +-05.2
| \-05.4
+-[0000:7f]-+-08.0
| +-08.2
| +-08.3
| + . . .
| \-1f.2
\-[0000:00]-+-00.0
+-01.0-[01]----00.0
+-02.0-[02]--+-00.0
| +-00.1
| +-00.2
| \-00.3
+-02.2-[03]--
+-03.0-[04-0b]----00.0-[05-0b]--+-08.0-[06-08]----00.0
| \-10.0-[09-0b]----00.0
+-05.0
+-05.1
+-05.2
+-05.4
+-11.0
+-11.4
+-16.0
+-16.1
+-1a.0
If I assign the device 0000:81:00.0 to a VM, I get "Failed to find parent device".
I think I should get no error with return value 0 just like bus number is 0, because
bus 80 is the root bus as well in my case.
In the <<Intel C610 Series Chipset and Intel X99 Chipset Platform Controller Hub(PCH)>>
Datasheet, I found that(Chapter 9.1):
For some server platforms, it may be desirable to have multiple PCHs in the system
Which means some PCH's may reside on a bus greater than 0.
So, is this a bug?
Thanks,
Zongyong Wu
7 years, 2 months
[libvirt] [PATCH] iohelper: avoid calling read() with misaligned buffers for O_DIRECT
by Daniel P. Berrange
The iohelper currently calls saferead() to get data from the
underlying file. This has a problem with O_DIRECT when hitting
end-of-file. saferead() is asked to read 1MB, but the first
read() it does may return only a few KB, so it'll try another
read() to fill the remaining buffer. Unfortunately the buffer
pointer passed into this 2nd read() is likely not aligned
to the extent that O_DIRECT requires, so rather than seeing
'0' for end-of-file, we'll get -1 + EINVAL due to misaligned
buffer.
The way the iohelper is currently written, it already handles
getting short reads, so there is actually no need to use
saferead() at all. We can simply call read() directly. The
benefit of this is that we can now write() the data immediately
so when we go into the subsequent reads() we'll always have a
correctly aligned buffer.
Technically the file position ought to be aligned for O_DIRECT
too, but this does not appear to matter when at end-of-file.
Tested-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/iohelper.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index fe15a92e6..5416d4506 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -109,7 +109,9 @@ runIO(const char *path, int fd, int oflags)
while (1) {
ssize_t got;
- if ((got = saferead(fdin, buf, buflen)) < 0) {
+ if ((got = read(fdin, buf, buflen)) < 0) {
+ if (errno == EINTR)
+ continue;
virReportSystemError(errno, _("Unable to read %s"), fdinname);
goto cleanup;
}
--
2.13.5
7 years, 2 months
