February 2021 - Devel - libvirt List Archives

[PATCH 00/14] util: Remove VIR_DISPOSE(_N)
by Peter Krempa 01 Feb '21

01 Feb '21

Most callers are way better off using memset directly additionally few places didn't even use it to clear sensitive data in the first place since the name probably sounded as the right thing to use. Peter Krempa (14): hypervFreeInvokeParams: Don't use VIR_DISPOSE_N for freeing 'params' libxlMakeDomBuildInfo: Don't use VIR_DISPOSE_N for USB device list storage_backend_iscsi(_direct): Properly clear secrets libxlMakeNetworkDiskSrc: Avoid use of VIR_DISPOSE_N qemuDomainMasterKeyCreate: Don't use VIR_DISPOSE_N on failure qemu: domain: Use memset for clearing secrets instead of VIR_DISPOSE_N cmdSecretSetValue: Make it obvious that --file, --base64 and --interactive are exlcusive virsh: cmdSecretSetValue: Rework handling of the secret value virsh: cmdSecretGetValue: Use memset instead of VIR_DISPOSE_N virStorageBackendRBDOpenRADOSConn: Use memset instead of VIR_DISPOSE_N virCryptoEncryptDataAESgnutls: Use memset instead of VIR_DISPOSE_N storageBackendCreateQemuImgSecretPath: Use memset instead of VIR_DISPOSE_N tests: viralloc: Remove testDispose case util: viralloc: Remove VIR_DISPOSE(_N) src/hyperv/hyperv_wmi.c | 4 +- src/libvirt_private.syms | 1 - src/libxl/libxl_conf.c | 8 +-- src/qemu/qemu_domain.c | 24 ++++++--- src/storage/storage_backend_iscsi.c | 16 +++--- src/storage/storage_backend_iscsi_direct.c | 17 ++++--- src/storage/storage_backend_rbd.c | 5 +- src/storage/storage_util.c | 5 +- src/util/viralloc.c | 39 +------------- src/util/viralloc.h | 27 ---------- src/util/vircrypto.c | 3 +- tests/viralloctest.c | 34 ------------- tools/virsh-secret.c | 59 ++++++++++------------ 13 files changed, 76 insertions(+), 166 deletions(-) -- 2.29.2

2 16

[PATCH] util: virstring: Remove unused prototypes for virStr(n)dup
by Peter Krempa 01 Feb '21

01 Feb '21

The headers weren't removed after use of VIR_STRDUP was removed. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/virstring.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/util/virstring.h b/src/util/virstring.h index 55547b040a..cfd24f0b74 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -123,13 +123,6 @@ int virStrcpy(char *dest, const char *src, size_t destbytes) G_GNUC_WARN_UNUSED_RESULT; #define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest)) -/* Don't call these directly - use the macros below */ -int virStrdup(char **dest, const char *src) - G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1); - -int virStrndup(char **dest, const char *src, ssize_t n) - G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1); - size_t virStringListLength(const char * const *strings); int virStringSortCompare(const void *a, const void *b); -- 2.29.2

2 1

[libvirt PATCH v2 00/10] Random bits found by clang-tidy
by Tim Wiederhake 01 Feb '21

01 Feb '21

clang-tidy is a static code analysis tool under the llvm umbrella. It is primarily meant to be used on C++ code bases, but some of the checks it provides also apply to C. The findings vary in severity and contain pseudo-false-positives, i.e. clang-tidy is flagging potential execution flows that could happen in theory but are virtually impossible in real life: In function `virGetUnprivSGIOSysfsPath`, variables `maj` and `min` would be read unintialized if `stat()` failed and set `errno` to a negative value, to name just one example. The main source of false positive findings is the lack of support for `__attribute__((cleanup))` in clang-tidy, which is heavily used in libvirt through glib's `g_autofree` and `g_auto()` macros: #include <stdlib.h> void freeptr(int** p) { if (*p) free(*p); } int main() { __attribute__((cleanup(freeptr))) int *ptr = NULL; ptr = calloc(sizeof(int), 1); return 0; /* flagged as memory leak of `ptr` */ } This sadly renders clang-tidy's analysis of dynamic memory useless, hiding all real issues that it could otherwise find. Meson provides excellent integration for clang-tidy (a "clang-tidy" target is automatically generated if a ".clang-tidy" configuration file is present in the project's root directory). The amount of false-positives (many of which present in a group of checks that cannot be disabled), random segfaults in memory constraint environments such as the containers in the CI, and the slow analysis (triggering time-outs in the CI), make this tool unfit for inclusion in libvirt's GitLab CI though. The patches in this series are the result of fixing some of the issues reported by running CC=clang meson build ninja -C build # generate sources and header files ninja -C build clang-tidy with the following `.clang-tidy` configuration file: --- Checks: > *, -abseil-*, -android-*, -boost-*, -cppcoreguidelines-*, -fuchsia-*, -google-*, -hicpp-*, -llvm-*, -modernize-*, -mpi-, -objc-, -openmp-, -zircon-*, -readability-braces-around-statements, -readability-magic-numbers WarningsAsErrors: '*' HeaderFilterRegex: '' FormatStyle: none ... V1: https://www.redhat.com/archives/libvir-list/2021-January/msg01152.html Changes since V1: * Expanded the justification for the "Replace bzero() with memset()" patch. * Rewrote "udevProcessCCW: Initialize variable". * Rewrote "tests: Prevent mallo with size 0". * Dropped "vircommand: Remove NULL check in virCommandAddArg". Note that this series now depends on the rewrite of tests/commandhelper.c, which can be found here: https://www.redhat.com/archives/libvir-list/2021-February/msg00034.html Regards, Tim Tim Wiederhake (10): virfile: Remove redundant #ifndef xen: Fix indentation in xenParseXLSpice qemu_tpm: Fix indentation in qemuTPMEmulatorBuildCommand virsh-domain: Fix error handling of pthread_sigmask Replace bzero() with memset() udevProcessCCW: Initialize variable virhostuptime: Fix rounding in uptime calculation tests: Prevent malloc with size 0 vircryptotest: Directly assign string to avoid memcpy vircommand: Simplify virCommandAddArg src/libxl/xen_xl.c | 5 ++--- src/node_device/node_device_udev.c | 2 +- src/qemu/qemu_tpm.c | 8 +++++--- src/util/virarptable.c | 2 +- src/util/vircommand.c | 7 +------ src/util/virfile.c | 4 ++-- src/util/virhostuptime.c | 4 +++- tests/commandhelper.c | 3 +++ tests/vircryptotest.c | 5 +---- tests/virpcimock.c | 2 +- tools/virsh-domain.c | 8 ++++---- 11 files changed, 24 insertions(+), 26 deletions(-) -- 2.26.2

2 13

[PATCH] virfile: workaround for when posix_fallocate() is not supported by FS
by Roman Bogorodskiy 01 Feb '21

01 Feb '21

posix_fallocate() might be not supported by a filesystem, for example, it's not supported by ZFS. In that case it fails with return code 22 (EINVAL), and thus safezero_posix_fallocate() returns -1. As safezero_posix_fallocate() is the first function tried by safezero() and it tries other functions only when it returns -2, it fails immediately without falling back to other methods, such as safezero_slow(). Fix that by returning -2 if posix_fallocate() returns EINVAL, to give safezero() a chance to try other functions. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- src/util/virfile.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util/virfile.c b/src/util/virfile.c index 609cafdd34..67a350deb3 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1119,6 +1119,16 @@ safezero_posix_fallocate(int fd, off_t offset, off_t len) int ret = posix_fallocate(fd, offset, len); if (ret == 0) return 0; + else if (ret == EINVAL) + /* EINVAL is returned when either: + - Operation is not supported by the underlying filesystem, + - offset or len argument values are invalid. + Assuming that offset and len are valid, this error means + the operation is not supported, and we need to fall back + to other methods. + */ + return -2; + errno = ret; return -1; } -- 2.30.0

2 1