[libvirt PATCH 0/3] Fix null string specifiers detected by GCC
by Scott Davis
Each of the following tripped the -Wformat-overflow warning in GCC 11. The
fixes all seemed pretty obvious based on the context.
Apologies for any formatting or other mistakes, this is my first time
submitting libvirt patches and using git publish.
Scott Davis (3):
qemu: fix null string specifier argument in qemuDomainBlockJobAbort
remote: fix null string specifier argument in
remoteProbeSessionDriverFromBinary
vircgroupv1: fix null string specifier argument in
virCgroupV1ValidatePlacement
src/qemu/qemu_driver.c | 8 ++++----
src/remote/remote_sockets.c | 2 +-
src/util/vircgroupv1.c | 5 ++---
3 files changed, 7 insertions(+), 8 deletions(-)
--
2.25.1
2 years, 6 months
Logging code, async signal safety, and syslog CEE
by Mike Pontillo
I was recently looking at an old (rejected) patch to add syslog CEE
support[1], and a related ticket asking to rewrite the syslog logger to be
async signal safe[2]. I'm hoping someone can help me understand the
potential issues here, because I would like to work on adding syslog CCE
support, and it seems like I should proceed with caution.
According to issue #5, the syslog() function is unsafe due to the
underlying use of sprintf(). However, in the logging code itself,
g_strdup_vprintf() is called prior to the callouts to the defined log
outputs. Is this call considered safe? What about other calls to GLib that
occur within the output routines, such as the call to g_mkstemp_full()?
Is there a test case I can execute to gain confidence that any new logging
code I add will be async signal safe? Are there existing, known calls to
the logger that can occur within a signal handler (or other circumstance
that requires async signal safety)?
The approach I was thinking of could build upon what was done for journald
support, in that it could make use of the "meta" struct to define
additional keys for the JSON message. I think the only complexity is the
need to build up a new string. I don't think there is any need to support a
"hierarchy"; building up a flat JSON dictionary should be all that is
needed. So, if it is undesirable to call out to a JSON library (this was
considered problematic in the original patch), the strings to be
logged could be escaped and concatenated into a dictionary, and then sent
up in the same way as existing syslog messages.
NB: I also noticed that the journald structured logging fields tend to be
all uppercase. I feel like it might also be good to convert the keys to
lowercase, for consistency with the other CEE logs I have seen.
Would a contribution like this be welcome in libvirt? I would appreciate
any thoughts.
Thanks,
Mike
[1]:
https://listman.redhat.com/archives/libvir-list/2012-September/064560.html
[2]: https://gitlab.com/libvirt/libvirt/-/issues/5
2 years, 6 months
[PATCH 0/2] Avoid memcpy with NULL argument
by Peter Krempa
Behaviour is undefined per C standard.
Peter Krempa (2):
virNetMessageEncodePayloadRaw: Tolerate empty 'data'
virNetMessageEncodePayloadEmpty: Replace by
virNetMessageEncodePayloadRaw(msg, NULL, 0)
src/rpc/virkeepalive.c | 2 +-
src/rpc/virnetmessage.c | 76 ++++++++++++++---------------------
src/rpc/virnetmessage.h | 2 -
src/rpc/virnetserverprogram.c | 9 +----
4 files changed, 34 insertions(+), 55 deletions(-)
--
2.35.3
2 years, 6 months
[PATCH 0/3] ppc64: POWER10 support
by Daniel Henrique Barboza
Changes from v2:
- rebased with current master
- patch 2:
- added qemuxml2xmltest
- all tests are now using DO_TEST_CAPS_ARCH_LATEST()
- new patch 3: added NEWS.rst update
- v2 link: https://listman.redhat.com/archives/libvir-list/2022-May/230746.html
Daniel Henrique Barboza (3):
cpu_map: add POWER10 cpu model
cpu_ppc64: add support for host-model on POWER10
NEWS.rst: document Power10 support
NEWS.rst | 4 ++
src/cpu/cpu_ppc64.c | 8 ++--
src/cpu_map/index.xml | 1 +
src/cpu_map/meson.build | 1 +
src/cpu_map/ppc64_POWER10.xml | 6 +++
tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 1 +
tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 4 +-
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 4 +-
.../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 4 +-
.../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 4 +-
...eries-cpu-compat-power10.ppc64-latest.args | 34 +++++++++++++++
...series-cpu-compat-power10.ppc64-latest.err | 1 +
.../pseries-cpu-compat-power10.xml | 21 ++++++++++
tests/qemuxml2argvtest.c | 4 ++
...series-cpu-compat-power10.ppc64-latest.xml | 41 +++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
tests/testutilshostcpus.h | 11 +++++
tests/testutilsqemu.c | 6 ++-
tests/testutilsqemu.h | 2 +
22 files changed, 148 insertions(+), 13 deletions(-)
create mode 100644 src/cpu_map/ppc64_POWER10.xml
create mode 100644 tests/qemuxml2argvdata/pseries-cpu-compat-power10.ppc64-latest.args
create mode 100644 tests/qemuxml2argvdata/pseries-cpu-compat-power10.ppc64-latest.err
create mode 100644 tests/qemuxml2argvdata/pseries-cpu-compat-power10.xml
create mode 100644 tests/qemuxml2xmloutdata/pseries-cpu-compat-power10.ppc64-latest.xml
--
2.32.0
2 years, 6 months
[libvirt PATCH 0/6] Fix and refactor QMP event processing
by Jiri Denemark
Jiri Denemark (6):
qemu: Avoid unlocked access to vm object in monitor callbacks
qemu: Pass arguments to qemuProcessEventSubmit directly
qemu: Make vm parameter of qemuProcessEventSubmit mandatory
qemu: Drop driver parameter from qemuProcessEventSubmit
qemu: Do not use opaque pointer in QEMU monitor callbacks
qemu: Do not pass unused opaque pointer to monitor callbacks
src/qemu/qemu_monitor.c | 16 +-
src/qemu/qemu_monitor.h | 108 ++++------
src/qemu/qemu_monitor_priv.h | 1 -
src/qemu/qemu_process.c | 384 +++++++++++++----------------------
src/qemu/qemu_processpriv.h | 3 +-
tests/qemuhotplugtest.c | 3 +-
tests/qemumonitortestutils.c | 15 +-
tests/qemumonitortestutils.h | 5 +-
8 files changed, 191 insertions(+), 344 deletions(-)
--
2.35.1
2 years, 6 months
[PATCH] build-aux: remove syntax checks for ATTRIBUTE_* and ARRAY_CARDINALITY
by Daniel P. Berrangé
These checks made sense when we were in process of converting code.
Since the definition of the macros has been entirely removed now,
the compiler will already thrown an error. There aren't likely to
be any in-flight patches that would hit this anyone either.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
build-aux/syntax-check.mk | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 09e7a014a3..e4c142dfd0 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -338,18 +338,6 @@ sc_avoid_g_gnuc_unused_in_header:
halt='use G_GNUC_UNUSED in .c rather than .h files' \
$(_sc_search_regexp)
-sc_prohibit_attribute_macros:
- @prohibit='ATTRIBUTE_(UNUSED|NORETURN|SENTINEL|RETURN_CHECK|NOINLINE|FMT_PRINTF|FALLTHROUGH)' \
- in_vc_files='\.[ch]$$' \
- halt='use GLib macros: G_GNUC_<ATTRIBUTE_SUFFIX> for most' \
- $(_sc_search_regexp)
-
-sc_prohibit_non_glib_macros:
- @prohibit='ARRAY_CARDINALITY' \
- in_vc_files='\.[ch]$$' \
- halt='use GLib macros: G_N_ELEMENTS' \
- $(_sc_search_regexp)
-
sc_prohibit_int_index:
@prohibit='\<(int|unsigned)\s*\*?index\>(\s|,|;)' \
halt='use different name than 'index' for declaration' \
--
2.36.1
2 years, 6 months
[libvirt PATCH] qemu: Lock vm object in qemuProcessHandleMemoryFailure
by Jiri Denemark
This event handler was accessing a vm object without locking it first.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_process.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 033d3d5bc6..6f70d5d065 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1773,6 +1773,8 @@ qemuProcessHandleMemoryFailure(qemuMonitor *mon G_GNUC_UNUSED,
virDomainMemoryFailureActionType action;
unsigned int flags = 0;
+ virObjectLock(vm);
+
switch (mfp->recipient) {
case QEMU_MONITOR_MEMORY_FAILURE_RECIPIENT_HYPERVISOR:
recipient = VIR_DOMAIN_EVENT_MEMORY_FAILURE_RECIPIENT_HYPERVISOR;
@@ -1809,6 +1811,9 @@ qemuProcessHandleMemoryFailure(qemuMonitor *mon G_GNUC_UNUSED,
flags |= VIR_DOMAIN_MEMORY_FAILURE_RECURSIVE;
event = virDomainEventMemoryFailureNewFromObj(vm, recipient, action, flags);
+
+ virObjectUnlock(vm);
+
virObjectEventStateQueue(driver->domainEventState, event);
}
--
2.35.1
2 years, 6 months
[PATCH v2] virprocess: Drop workaround for setns() wrt old glibc
by Michal Privoznik
We have our own implementation of setns() which was introduced in
v1.2.9-rc1~190 and extended afterwards. The reason was that back
in 2014 we were dealing with glibc that in some of its older
versions did not provide the function. Mostly for non-intel
arches. Nevertheless, glibc now offers the function for all
architectures we care about (aarch64 being the freshest
architecture where the function was introduced, in glibc-2.17).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
v2 of:
https://listman.redhat.com/archives/libvir-list/2022-May/231773.html
diff to v1:
- drop setns() detection in meson
- make virProcessSetNamespaces() impl depend on __linux__
meson.build | 1 -
src/util/virprocess.c | 55 +++++++++----------------------------------
2 files changed, 11 insertions(+), 45 deletions(-)
diff --git a/meson.build b/meson.build
index a1c802b00d..c4585bd92c 100644
--- a/meson.build
+++ b/meson.build
@@ -557,7 +557,6 @@ functions = [
'prlimit',
'sched_setscheduler',
'setgroups',
- 'setns',
'setrlimit',
'symlink',
'sysctlbyname',
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 36d7df050a..5ed0b5d0db 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -27,7 +27,6 @@
#ifndef WIN32
# include <sys/wait.h>
#endif
-#include <unistd.h>
#if WITH_SYS_MOUNT_H
# include <sys/mount.h>
#endif
@@ -70,49 +69,6 @@
VIR_LOG_INIT("util.process");
-#ifdef __linux__
-/*
- * Workaround older glibc. While kernel may support the setns
- * syscall, the glibc wrapper might not exist. If that's the
- * case, use our own.
- */
-# ifndef __NR_setns
-# if defined(__x86_64__)
-# define __NR_setns 308
-# elif defined(__i386__)
-# define __NR_setns 346
-# elif defined(__arm__)
-# define __NR_setns 375
-# elif defined(__aarch64__)
-# define __NR_setns 375
-# elif defined(__powerpc__)
-# define __NR_setns 350
-# elif defined(__s390__)
-# define __NR_setns 339
-# endif
-# endif
-
-# ifndef WITH_SETNS
-# if defined(__NR_setns)
-# include <sys/syscall.h>
-
-static inline int setns(int fd, int nstype)
-{
- return syscall(__NR_setns, fd, nstype);
-}
-# else /* !__NR_setns */
-# error Please determine the syscall number for setns on your architecture
-# endif
-# endif
-#else /* !__linux__ */
-static inline int setns(int fd G_GNUC_UNUSED, int nstype G_GNUC_UNUSED)
-{
- virReportSystemError(ENOSYS, "%s",
- _("Namespaces are not supported on this platform."));
- return -1;
-}
-#endif
-
VIR_ENUM_IMPL(virProcessSchedPolicy,
VIR_PROC_POLICY_LAST,
"none",
@@ -714,6 +670,7 @@ int virProcessGetNamespaces(pid_t pid,
}
+#ifdef __linux__
int virProcessSetNamespaces(size_t nfdlist,
int *fdlist)
{
@@ -742,6 +699,16 @@ int virProcessSetNamespaces(size_t nfdlist,
}
return 0;
}
+#else
+int virProcessSetNamespaces(size_t nfdlist G_GNUC_UNUSED,
+ int *fdlist G_GNUC_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Namespaces are not supported on this platform."));
+ return -1;
+}
+#endif
+
#if WITH_PRLIMIT
static int
--
2.35.1
2 years, 6 months
[PATCH] virprocess: Drop workaround for setns() wrt old glibc
by Michal Privoznik
We have our own implementation of setns() which was introduced in
v1.2.9-rc1~190 and extended afterwards. The reason was that back
in 2014 we were dealing with glibc that in some of its older
versions did not provide the function. Mostly for non-intel
arches. Nevertheless, glibc now offers the function for all
architectures we care about (aarch64 being the freshest
architecture where the function was introduced, in glibc-2.17).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virprocess.c | 55 +++++++++----------------------------------
1 file changed, 11 insertions(+), 44 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 36d7df050a..f0a79396e7 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -27,7 +27,6 @@
#ifndef WIN32
# include <sys/wait.h>
#endif
-#include <unistd.h>
#if WITH_SYS_MOUNT_H
# include <sys/mount.h>
#endif
@@ -70,49 +69,6 @@
VIR_LOG_INIT("util.process");
-#ifdef __linux__
-/*
- * Workaround older glibc. While kernel may support the setns
- * syscall, the glibc wrapper might not exist. If that's the
- * case, use our own.
- */
-# ifndef __NR_setns
-# if defined(__x86_64__)
-# define __NR_setns 308
-# elif defined(__i386__)
-# define __NR_setns 346
-# elif defined(__arm__)
-# define __NR_setns 375
-# elif defined(__aarch64__)
-# define __NR_setns 375
-# elif defined(__powerpc__)
-# define __NR_setns 350
-# elif defined(__s390__)
-# define __NR_setns 339
-# endif
-# endif
-
-# ifndef WITH_SETNS
-# if defined(__NR_setns)
-# include <sys/syscall.h>
-
-static inline int setns(int fd, int nstype)
-{
- return syscall(__NR_setns, fd, nstype);
-}
-# else /* !__NR_setns */
-# error Please determine the syscall number for setns on your architecture
-# endif
-# endif
-#else /* !__linux__ */
-static inline int setns(int fd G_GNUC_UNUSED, int nstype G_GNUC_UNUSED)
-{
- virReportSystemError(ENOSYS, "%s",
- _("Namespaces are not supported on this platform."));
- return -1;
-}
-#endif
-
VIR_ENUM_IMPL(virProcessSchedPolicy,
VIR_PROC_POLICY_LAST,
"none",
@@ -714,6 +670,7 @@ int virProcessGetNamespaces(pid_t pid,
}
+#if WITH_SETNS
int virProcessSetNamespaces(size_t nfdlist,
int *fdlist)
{
@@ -742,6 +699,16 @@ int virProcessSetNamespaces(size_t nfdlist,
}
return 0;
}
+#else
+int virProcessSetNamespaces(size_t nfdlist G_GNUC_UNUSED,
+ int *fdlist G_GNUC_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Namespaces are not supported on this platform."));
+ return -1;
+}
+#endif
+
#if WITH_PRLIMIT
static int
--
2.35.1
2 years, 6 months
