[PATCH] qemu_hotplug: Don't dereference NULL pointer @newb in qemuDomainChangeNet()
by Michal Privoznik
In one of my previous commits I've made an attempt to restore the
noqueue qdisc on a TAP corresponding to domain's <interface/> if
QoS is cleared out. The commit consisted of two almost identical
hunks. In both the pointer is dereferenced. But in one of them,
the pointer to new bandwidth can't be NULL while in the other it
can leading to a crash.
Fixes: d53b09235398c1320ed2f1b45b640823171467ed
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1919619
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e7863328db..a66354426d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3900,10 +3900,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* If the old bandwidth was cleared out, restore qdisc. */
if (virDomainNetTypeSharesHostView(newdev)) {
- if (!newb->out || newb->out->average == 0)
+ if (!newb || !newb->out || newb->out->average == 0)
qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
} else {
- if (!newb->in || newb->in->average == 0)
+ if (!newb || !newb->in || newb->in->average == 0)
qemuDomainInterfaceSetDefaultQDisc(driver, newdev);
}
needReplaceDevDef = true;
--
2.26.2
4 years, 2 months
[PATCH] qemu: add delay time cpu stats
by Aleksei Zakharov
This commit adds delay time (steal time inside guest) to libvirt
domain CPU stats. It's more convenient to work with this statistic
in a context of libvirt domain. Any monitoring software may use
this information.
As an example: the next step is to add support to
libvirt-go and expose metrics with libvirt-exporter.
Signed-off-by: Aleksei Zakharov <zaharov(a)selectel.ru>
---
include/libvirt/libvirt-domain.h | 6 ++++
src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++
tools/virsh-domain.c | 3 +-
3 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index de2456812c..b3f9f375a5 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1350,6 +1350,12 @@ int virDomainGetState (virDomainPtr domain,
*/
# define VIR_DOMAIN_CPU_STATS_SYSTEMTIME "system_time"
+/**
+ * VIR_DOMAIN_CPU_STATS_DELAYTIME:
+ * cpu time waiting on runqueue in nanoseconds, as a ullong
+ */
+# define VIR_DOMAIN_CPU_STATS_DELAYTIME "delay_time"
+
/**
* VIR_DOMAIN_CPU_STATS_VCPUTIME:
* vcpu usage in nanoseconds (cpu_time excluding hypervisor time),
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6193376544..41839a0239 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16674,6 +16674,36 @@ qemuDomainGetMetadata(virDomainPtr dom,
return ret;
}
+static int
+virSchedstatGetDelay(virDomainObjPtr dom, unsigned long long *delay)
+{
+ char *proc = NULL;
+ FILE* schedstat;
+ unsigned long long curr_delay, oncpu = 0;
+ pid_t pid = dom->pid;
+ for (size_t i = 0; i < virDomainDefGetVcpusMax(dom->def); i++) {
+ pid_t vcpupid = qemuDomainGetVcpuPid(dom, i);
+ if (vcpupid) {
+ if (asprintf(&proc, "/proc/%d/task/%d/schedstat",
+ pid, vcpupid) < 0)
+ return -1;
+ } else {
+ if (asprintf(&proc, "/proc/%d/schedstat", pid) < 0)
+ return -1;
+ }
+ schedstat = fopen(proc, "r");
+ VIR_FREE(proc);
+ if (!schedstat ||
+ fscanf(schedstat, "%llu %llu",
+ &oncpu, &curr_delay) < 2) {
+ return -1;
+ }
+
+ *delay += curr_delay;
+ }
+ return 0;
+}
+
static int
qemuDomainGetCPUStats(virDomainPtr domain,
@@ -16687,6 +16717,7 @@ qemuDomainGetCPUStats(virDomainPtr domain,
int ret = -1;
qemuDomainObjPrivatePtr priv;
virBitmapPtr guestvcpus = NULL;
+ unsigned long long delay = 0;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -16712,8 +16743,19 @@ qemuDomainGetCPUStats(virDomainPtr domain,
goto cleanup;
if (start_cpu == -1)
+ {
ret = virCgroupGetDomainTotalCpuStats(priv->cgroup,
params, nparams);
+ if (nparams > 3) {
+ if (virSchedstatGetDelay(vm,&delay) < 0)
+ return -1;
+ if (virTypedParameterAssign(¶ms[3],
+ VIR_DOMAIN_CPU_STATS_DELAYTIME,
+ VIR_TYPED_PARAM_ULLONG, delay) < 0)
+ return -1;
+ }
+ ret++;
+ }
else
ret = virCgroupGetPercpuStats(priv->cgroup, params, nparams,
start_cpu, ncpus, guestvcpus);
@@ -17845,6 +17887,17 @@ qemuDomainGetStatsMemoryBandwidth(virQEMUDriverPtr driver,
return ret;
}
+static int
+qemuDomainGetStatsCpuDelay(virDomainObjPtr dom,
+ virTypedParamListPtr params)
+{
+ unsigned long long delay_time = 0;
+ int err = 0;
+ err = virSchedstatGetDelay(dom, &delay_time);
+ if (!err && virTypedParamListAddULLong(params, delay_time, "cpu.delay") < 0)
+ return -1;
+ return 0;
+}
static int
qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver,
@@ -17950,6 +18003,9 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver,
if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0)
return -1;
+ if (qemuDomainGetStatsCpuDelay(dom, params) < 0)
+ return -1;
+
return 0;
}
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2bb136333f..a1780da1a5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8085,7 +8085,8 @@ vshCPUStatsPrintField(vshControl *ctl,
if ((STREQ(param->field, VIR_DOMAIN_CPU_STATS_CPUTIME) ||
STREQ(param->field, VIR_DOMAIN_CPU_STATS_VCPUTIME) ||
STREQ(param->field, VIR_DOMAIN_CPU_STATS_USERTIME) ||
- STREQ(param->field, VIR_DOMAIN_CPU_STATS_SYSTEMTIME)) &&
+ STREQ(param->field, VIR_DOMAIN_CPU_STATS_SYSTEMTIME) ||
+ STREQ(param->field, VIR_DOMAIN_CPU_STATS_DELAYTIME)) &&
param->type == VIR_TYPED_PARAM_ULLONG) {
vshPrint(ctl, "%9lld.%09lld seconds\n",
param->value.ul / 1000000000,
--
2.17.1
4 years, 2 months
[libvirt PATCH v2 0/3] ci: Paint the pipeline green
by Andrea Bolognani
Various workarounds that are necessary due to breakages in external
services and distribution archives, plus fixes for a couple of issues
that were discovered in the process.
Changes from [v1]:
* the first three patches have been dropped from the series as
they've been pushed already;
* Dockerfiles have been refreshed using a more recent version of
lcitool.
[v1] https://listman.redhat.com/archives/libvir-list/2021-February/msg00664.html
Andrea Bolognani (3):
ci: Refresh Dockerfiles
ci: Add temporary workaround for Fedora Rawhide
ci: Build on FreeBSD 12.2
.gitlab-ci.yml | 2 +-
ci/containers/ci-centos-7.Dockerfile | 3 ++-
ci/containers/ci-centos-8.Dockerfile | 3 ++-
ci/containers/ci-centos-stream.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-i686.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-mips.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-mips64el.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-ppc64le.Dockerfile | 3 ++-
ci/containers/ci-debian-10-cross-s390x.Dockerfile | 3 ++-
ci/containers/ci-debian-10.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-aarch64.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-armv6l.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-armv7l.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-i686.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-mips64el.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-mipsel.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-ppc64le.Dockerfile | 3 ++-
ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 3 ++-
ci/containers/ci-debian-sid.Dockerfile | 3 ++-
ci/containers/ci-fedora-32.Dockerfile | 3 ++-
ci/containers/ci-fedora-33.Dockerfile | 3 ++-
ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 8 +++++---
ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 8 +++++---
ci/containers/ci-fedora-rawhide.Dockerfile | 8 +++++---
ci/containers/ci-opensuse-152.Dockerfile | 3 ++-
ci/containers/ci-ubuntu-1804.Dockerfile | 3 ++-
ci/containers/ci-ubuntu-2004.Dockerfile | 3 ++-
31 files changed, 70 insertions(+), 37 deletions(-)
--
2.26.2
4 years, 2 months
[libvirt PATCH] news: Mention Apple Silicon support
by Andrea Bolognani
After the recent fixes, it's now confirmed to work.
https://gitlab.com/libvirt/libvirt/-/issues/121
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
NEWS.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst
index a9a1a61119..440dbf2601 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -11,6 +11,12 @@ For a more fine-grained view, use the `git log`_.
v7.1.0 (unreleased)
===================
+* **Portability**
+
+ * Implement Apple Silicon support
+
+ libvirt now runs on the ARM-based Apple Silicon Macs.
+
* **New features**
* Introduce virtio-pmem ``<memory/>`` model
--
2.26.2
4 years, 2 months
[libvirt PATCH v2 0/6] APIs for reporting tainting and deprecation messages
by Daniel P. Berrangé
This is a follow up to
https://listman.redhat.com/archives/libvir-list/2021-January/msg00988.html
I pushed the first non-API parts of that series already.
This posting takes a different approach to the APIs. Instead of separte
APIs for tainting and deprecations, there is now one API for reporting
general informational messages. This is explicitly only targetted at
humans.
v2:
- Change formatting for better translation
- Fix leak of strings
Daniel P. Berrangé (6):
conf: record deprecation messages against the domain
qemu: record deprecation messages against the domain
src: define virDomainGetMessages API
remote: add RPC support for the virDomainGetMessages API
qemu: implement virDomainGetMessages API
tools: report messages for 'dominfo' command
include/libvirt/libvirt-domain.h | 9 +++++
src/conf/domain_conf.c | 45 ++++++++++++++++++++--
src/conf/domain_conf.h | 5 +++
src/driver-hypervisor.h | 6 +++
src/libvirt-domain.c | 54 +++++++++++++++++++++++++++
src/libvirt_private.syms | 3 ++
src/libvirt_public.syms | 5 +++
src/qemu/qemu_domain.c | 5 +++
src/qemu/qemu_domain.h | 3 ++
src/qemu/qemu_driver.c | 58 +++++++++++++++++++++++++++++
src/qemu/qemu_process.c | 5 +++
src/remote/remote_daemon_dispatch.c | 45 ++++++++++++++++++++++
src/remote/remote_driver.c | 44 ++++++++++++++++++++++
src/remote/remote_protocol.x | 21 ++++++++++-
src/remote_protocol-structs | 11 ++++++
tools/virsh-domain-monitor.c | 10 +++++
16 files changed, 325 insertions(+), 4 deletions(-)
--
2.29.2
4 years, 2 months
[libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions
by Laine Stump
I've looked at a few of these, and one thing I've found is that very
often we have a function called somethingSomethingClear(), and:
1) The only places it is ever called will immediately free the memory
of the object as soon as they clear it.
and very possibly
2) It doesn't actually *clear* everything. Some items are cleared via VIR_FREE(), but then some of the other pointers call
bobLoblawFree(def->bobloblaw)
and then don't actually set def->bobloblaw to NULL - so the functions
aren't actually "Clearing", they're "Freeing and then clearing a few
things, but not everything".
So I'm wondering if it is worthwhile to
A) audit all the *Clear() functions and rename the functions that
don't actually need to clear the contents to be, e.g.
bobLobLawFreeContents(), while also replacing VIR_FREE with g_free().
(this is what I've done in these 5 patches)
or if we should
B) just do the wholesale approach and (as danpb suggested last week)
change all VIR_FREE in *Clear() functions to g_free(), and put a
"memset(obj, 0, sizeof(*obj))" at the end of each function, ignoring
whether or not we actually need that.
(B) would obviously be much faster to get done, and simpler for
everyone to keep track of what's happening, but of course it is less
efficient. Very likely this efficiency is completely meaningless in
the large scheme (even in the medium or small scheme).
(I actually like the idea of 0'ing out *everything*[*] when we're done
with it, extra cycles be damned! I think of the two choices above,
after going through this exercise, I'd say (B) is a more reasonable
choice, but since I tried this, I thought I'd send it and see if
anyone else has an opinion (or different idea)
[*](including all those places I've replaced VIR_FREE with g_free in
the name of "progress?")
Laine Stump (5):
conf: rename and narrow scope of virDomainHostdevDefClear()
conf: rename virDomainHostdevSubsysSCSIClear
conf: replace pointless VIR_FREEs with g_free in
virDomainVideoDefClear()
conf: don't call virDomainDeviceInfoClear from
virDomainDeviceInfoParseXML
conf: replace virDomainDeviceInfoClear with
virDomainDeviceInfoFreeContents
src/conf/device_conf.c | 15 +++-----
src/conf/device_conf.h | 2 +-
src/conf/domain_conf.c | 81 ++++++++++++++++++++--------------------
src/conf/domain_conf.h | 1 -
src/libvirt_private.syms | 1 -
5 files changed, 47 insertions(+), 53 deletions(-)
--
2.29.2
4 years, 2 months
[PATCH] qemu_shim: URI escape root directory
by Michal Privoznik
The root directory can be provided by user (or a temporary one is
generated) and is always formatted into connection URI for both
secret driver and QEMU driver, like this:
qemu:///embed?root=$root
But if it so happens that there is an URI unfriendly character in
root directory or path to it (say a space) then invalid URI is
formatted which results in unexpected results. We can trust
g_dir_make_tmp() to generate valid URI but we can't trust user.
Escape user provided root directory. Always.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920400
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_shim.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c
index 18bdc99256..c10598df4b 100644
--- a/src/qemu/qemu_shim.c
+++ b/src/qemu/qemu_shim.c
@@ -140,7 +140,8 @@ int main(int argc, char **argv)
g_autofree char *xml = NULL;
g_autofree char *uri = NULL;
g_autofree char *suri = NULL;
- char *root = NULL;
+ const char *root = NULL;
+ g_autofree char *escaped = NULL;
bool tmproot = false;
int ret = 1;
g_autoptr(GError) error = NULL;
@@ -216,6 +217,8 @@ int main(int argc, char **argv)
}
}
+ escaped = g_uri_escape_string(root, NULL, true);
+
virFileActivateDirOverrideForProg(argv[0]);
if (verbose)
@@ -242,7 +245,7 @@ int main(int argc, char **argv)
eventLoopThread = g_thread_new("event-loop", qemuShimEventLoop, NULL);
if (secrets && *secrets) {
- suri = g_strdup_printf("secret:///embed?root=%s", root);
+ suri = g_strdup_printf("secret:///embed?root=%s", escaped);
if (verbose)
g_printerr("%s: %lld: opening %s\n",
@@ -303,7 +306,7 @@ int main(int argc, char **argv)
}
}
- uri = g_strdup_printf("qemu:///embed?root=%s", root);
+ uri = g_strdup_printf("qemu:///embed?root=%s", escaped);
if (verbose)
g_printerr("%s: %lld: opening %s\n",
--
2.26.2
4 years, 2 months
[libvirt PATCH 00/10] [RFC] clang-tidy CI integration
by Tim Wiederhake
"clang-tidy" is a static code analysis tool for c and c++ code. See
https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html
for some issues found by clang-tidy and more background information.
Meson has support for clang-tidy and generates target named "clang-tidy" if
it finds a ".clang-tidy" configuration file in the project's root directory.
There are some problems with this approach though, with regards to inclusion
in GitLab's CI:
* Single-threaded runtime of a complete scan takes between 95 and 110 minutes,
depending on the enabled checks, which is significantly longer than GitLab's
maximum run time of 60 minutes, after which jobs are aborted.
* Even without this limit on runtime, this new check would double to triple
the run time of the libVirt pipeline in GitLab.
* clang-tidy finds a lot of false positives (see link above for explanation)
and has checks that contradict the libVirt code style (e.g. braces around
blocks). This makes a quite complex configuration in ".clang-tidy" neccessary.
* I was unable to make clang-tidy recognize the settings from the
configuration file for generated files, leading clang-tidy to always add some
checks. These checks were among those that produced false positives.
* The list of enabled / disabled checks in the yaml configuration file is a
quite long string, making it hard to weave in some comments / documentation
on which checks are enabled / disabled for what reason.
This series introduces a new script, "run-clang-tidy.py". This is a
replacement for the script of the same name from clang-tools-extra. It offers
parallel execution, caching of results and a configurable soft-timeout.
Please see the individual commits for more details. Comments welcome.
https://gitlab.com/twiederh/libvirt/-/pipelines/255321968 → "clang-tidy".
Tim
Tim Wiederhake (10):
clang-tidy: Add a simple runner
clang-tidy: Run in parallel
clang-tidy: Filter output
clang-tidy: Add cache
clang-tidy: Add timeout
clang-tidy: Allow timeouts
clang-tidy: Add shuffle
clang-tidy: Make list of checks explicit
clang-tidy: Disable irrelevant and failing checks
clang-tidy: Add CI integration
.gitlab-ci.yml | 88 ++++++
scripts/run-clang-tidy.py | 557 ++++++++++++++++++++++++++++++++++++++
2 files changed, 645 insertions(+)
create mode 100755 scripts/run-clang-tidy.py
--
2.26.2
4 years, 2 months
[libvirt PATCH v2 0/4] ci: Adjustments to remove our dependency on sudo
by Erik Skultety
Our Debian containers don't have sudo pre-installed and the only reason we
actually needed it was to run a custom prepare script which as it turns out
does nothing really.
Since v1:
- applied quoting to the variables as asked during the review process
- simplified setting of CI_PODMAN_ARGS
- moved 4/4 to 3/4
Erik Skultety (4):
ci: Specify the shebang sequence for build.sh
ci: Run podman command directly without wrapping it with prepare.sh
ci: Drop the prepare.sh script
ci: Makefile: Expose the CI_USER_LOGIN variable for users to use
ci/Makefile | 62 +++++++++++++++++++++++++--------------------------
ci/build.sh | 2 ++
ci/prepare.sh | 13 -----------
3 files changed, 33 insertions(+), 44 deletions(-)
delete mode 100644 ci/prepare.sh
--
2.29.2
4 years, 2 months
[libvirt PATCH 0/4] ci: Adjustments to remove our dependency on sudo
by Erik Skultety
Our Debian containers don't have sudo pre-installed and the only reason we
actually needed it was to run a custom prepare script which as it turns out d=
oes
nothing really.
Erik Skultety (4):
ci: Specify the shebang sequence for build.sh
ci: Run podman command directly without wrapping it with prepare.sh
ci: Expose CI_USER_LOGIN as a Makefile variable for users to use
ci: Drop the prepare.sh script
ci/Makefile | 43 ++++++++++++++++++++++++-------------------
ci/build.sh | 2 ++
ci/prepare.sh | 13 -------------
3 files changed, 26 insertions(+), 32 deletions(-)
delete mode 100644 ci/prepare.sh
--=20
2.29.2
4 years, 2 months
