From a309351b09c7e5ad87050ee0c1bb2ec6cde809579cceb72125be7ad708409568 Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Tue, 18 Aug 2020 21:42:51 +0000 Subject: [PATCH 1/3] - util: Fix logic in virFileSetCOW 2edd63a0-fix-virFileSetCOW-logic.patch boo#1175463 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=836 --- 2edd63a0-fix-virFileSetCOW-logic.patch | 29 ++++++++++++++++++++++++++ libvirt.changes | 7 +++++++ libvirt.spec | 2 ++ 3 files changed, 38 insertions(+) create mode 100644 2edd63a0-fix-virFileSetCOW-logic.patch diff --git a/2edd63a0-fix-virFileSetCOW-logic.patch b/2edd63a0-fix-virFileSetCOW-logic.patch new file mode 100644 index 0000000..5093a75 --- /dev/null +++ b/2edd63a0-fix-virFileSetCOW-logic.patch @@ -0,0 +1,29 @@ +commit 2edd63a0dbd445112db23596ee0128521e8f1ff5 +Author: Jiri Denemark +Date: Wed Aug 5 10:01:45 2020 +0200 + + util: Fix logic in virFileSetCOW + + When COW is not explicitly requested to be disabled or enabled, the + function is supposed to do nothing on non-BTRFS file systems. + + Fixes commit 7230bc95aa78379c9ee20cf59394c5fc4305b75b. + + https://bugzilla.redhat.com/show_bug.cgi?id=1866157 + + Signed-off-by: Jiri Denemark + Reviewed-by: Daniel P. Berrangé + +Index: libvirt-6.6.0/src/util/virfile.c +=================================================================== +--- libvirt-6.6.0.orig/src/util/virfile.c ++++ libvirt-6.6.0/src/util/virfile.c +@@ -4550,7 +4550,7 @@ virFileSetCOW(const char *path, + } + + if (buf.f_type != BTRFS_SUPER_MAGIC) { +- if (state == VIR_TRISTATE_BOOL_ABSENT) { ++ if (state != VIR_TRISTATE_BOOL_ABSENT) { + virReportSystemError(ENOSYS, + _("unable to control COW flag on '%s', not btrfs"), + path); diff --git a/libvirt.changes b/libvirt.changes index 0d9b32b..2685827 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Tue Aug 18 21:40:48 UTC 2020 - James Fehlig + +- util: Fix logic in virFileSetCOW + 2edd63a0-fix-virFileSetCOW-logic.patch + boo#1175463 + ------------------------------------------------------------------- Tue Aug 4 22:46:13 UTC 2020 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index 841a14a..357d417 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -336,6 +336,7 @@ Source6: libvirtd-relocation-server.xml Source99: baselibs.conf Source100: %{name}-rpmlintrc # Upstream patches +Patch0: 2edd63a0-fix-virFileSetCOW-logic.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch @@ -875,6 +876,7 @@ libvirt plugin for NSS for translating domain names into IP addresses. %prep %setup -q +%patch0 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1 From 101cb20b189a8b7a4f31e42c13e5f36d05c69d6730fc34c04cdcbf2bb1a2f05d Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Wed, 19 Aug 2020 19:39:18 +0000 Subject: [PATCH 2/3] - virdevmapper: Handle kernel without device-mapper support 82bb167f-dont-cache-devmapper-major.patch, feb8564a-handle-no-devmapper.patch boo#1175465 OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=837 --- 82bb167f-dont-cache-devmapper-major.patch | 86 +++++++++++++++++++++++ feb8564a-handle-no-devmapper.patch | 74 +++++++++++++++++++ libvirt.changes | 8 +++ libvirt.spec | 4 ++ 4 files changed, 172 insertions(+) create mode 100644 82bb167f-dont-cache-devmapper-major.patch create mode 100644 feb8564a-handle-no-devmapper.patch diff --git a/82bb167f-dont-cache-devmapper-major.patch b/82bb167f-dont-cache-devmapper-major.patch new file mode 100644 index 0000000..e4b238f --- /dev/null +++ b/82bb167f-dont-cache-devmapper-major.patch @@ -0,0 +1,86 @@ +commit 82bb167f0d15b733b23931205be3488b83cb9ec6 +Author: Michal Prívozník +Date: Tue Aug 18 11:08:15 2020 +0200 + + virdevmapper: Don't cache device-mapper major + + The device mapper major is needed in virIsDevMapperDevice() which + determines whether given device is managed by device-mapper. This + number is obtained by parsing /proc/devices and then stored in a + global variable so that the file doesn't have to be parsed again. + However, as it turns out this logic is flawed - the major number + is not static and can change as it can be specified as a + parameter when loading the dm-mod module. + + Unfortunately, I was not able to come up with a good solution and + thus the /proc/devices file is being parsed every time we need + the device mapper major. + + Signed-off-by: Michal Privoznik + Reviewed-by: Peter Krempa + Reviewed-by: Christian Ehrhardt + Tested-by: Christian Ehrhardt + +Index: libvirt-6.6.0/src/util/virdevmapper.c +=================================================================== +--- libvirt-6.6.0.orig/src/util/virdevmapper.c ++++ libvirt-6.6.0/src/util/virdevmapper.c +@@ -46,11 +46,9 @@ + + G_STATIC_ASSERT(BUF_SIZE > sizeof(struct dm_ioctl)); + +-static unsigned int virDMMajor; +- + + static int +-virDevMapperOnceInit(void) ++virDevMapperGetMajor(unsigned int *major) + { + g_autofree char *buf = NULL; + VIR_AUTOSTRINGLIST lines = NULL; +@@ -69,7 +67,7 @@ virDevMapperOnceInit(void) + + if (sscanf(lines[i], "%u %ms\n", &maj, &dev) == 2 && + STREQ(dev, DM_NAME)) { +- virDMMajor = maj; ++ *major = maj; + break; + } + } +@@ -85,9 +83,6 @@ virDevMapperOnceInit(void) + } + + +-VIR_ONCE_GLOBAL_INIT(virDevMapper); +- +- + static void * + virDMIoctl(int controlFD, int cmd, struct dm_ioctl *dm, char **buf) + { +@@ -305,9 +300,6 @@ virDevMapperGetTargets(const char *path, + * consist of devices or yet another targets. If that's the + * case, we have to stop recursion somewhere. */ + +- if (virDevMapperInitialize() < 0) +- return -1; +- + if ((controlFD = virDMOpen()) < 0) + return -1; + +@@ -319,13 +311,14 @@ bool + virIsDevMapperDevice(const char *dev_name) + { + struct stat buf; ++ unsigned int major; + +- if (virDevMapperInitialize() < 0) ++ if (virDevMapperGetMajor(&major) < 0) + return false; + + if (!stat(dev_name, &buf) && + S_ISBLK(buf.st_mode) && +- major(buf.st_rdev) == virDMMajor) ++ major(buf.st_rdev) == major) + return true; + + return false; diff --git a/feb8564a-handle-no-devmapper.patch b/feb8564a-handle-no-devmapper.patch new file mode 100644 index 0000000..f68ff68 --- /dev/null +++ b/feb8564a-handle-no-devmapper.patch @@ -0,0 +1,74 @@ +commit feb8564a3cc63bc8f68284063d53ec0d2d81a1cc +Author: Michal Prívozník +Date: Tue Aug 18 11:04:24 2020 +0200 + + virdevmapper: Handle kernel without device-mapper support + + In one of my latest patch (v6.6.0~30) I was trying to remove + libdevmapper use in favor of our own implementation. However, the + code did not take into account that device mapper can be not + compiled into the kernel (e.g. be a separate module that's not + loaded) in which case /proc/devices won't have the device-mapper + major number and thus virDevMapperGetTargets() and/or + virIsDevMapperDevice() fails. + + However, such failure is safe to ignore, because if device mapper + is missing then there can't be any multipath devices and thus we + don't need to allow the deps in CGroups, nor create them in the + domain private namespace, etc. + + Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e + Reported-by: Andrea Bolognani + Reported-by: Christian Ehrhardt + Signed-off-by: Michal Privoznik + Reviewed-by: Peter Krempa + Reviewed-by: Christian Ehrhardt + Tested-by: Christian Ehrhardt + +Index: libvirt-6.6.0/src/util/virdevmapper.c +=================================================================== +--- libvirt-6.6.0.orig/src/util/virdevmapper.c ++++ libvirt-6.6.0/src/util/virdevmapper.c +@@ -54,6 +54,9 @@ virDevMapperGetMajor(unsigned int *major + VIR_AUTOSTRINGLIST lines = NULL; + size_t i; + ++ if (!virFileExists(CONTROL_PATH)) ++ return -2; ++ + if (virFileReadAll(PROC_DEVICES, BUF_SIZE, &buf) < 0) + return -1; + +@@ -126,8 +129,13 @@ virDMOpen(void) + + memset(&dm, 0, sizeof(dm)); + +- if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) ++ if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) { ++ if (errno == ENOENT) ++ return -2; ++ ++ virReportSystemError(errno, _("Unable to open %s"), CONTROL_PATH); + return -1; ++ } + + if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { + virReportSystemError(errno, "%s", +@@ -300,8 +308,16 @@ virDevMapperGetTargets(const char *path, + * consist of devices or yet another targets. If that's the + * case, we have to stop recursion somewhere. */ + +- if ((controlFD = virDMOpen()) < 0) ++ if ((controlFD = virDMOpen()) < 0) { ++ if (controlFD == -2) { ++ /* The CONTROL_PATH doesn't exist. Probably the ++ * module isn't loaded, yet. Don't error out, just ++ * exit. */ ++ return 0; ++ } ++ + return -1; ++ } + + return virDevMapperGetTargetsImpl(controlFD, path, devPaths, ttl); + } diff --git a/libvirt.changes b/libvirt.changes index 2685827..e57170e 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Wed Aug 19 19:36:52 UTC 2020 - James Fehlig + +- virdevmapper: Handle kernel without device-mapper support + 82bb167f-dont-cache-devmapper-major.patch, + feb8564a-handle-no-devmapper.patch + boo#1175465 + ------------------------------------------------------------------- Tue Aug 18 21:40:48 UTC 2020 - James Fehlig diff --git a/libvirt.spec b/libvirt.spec index 357d417..8e2f6c2 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -337,6 +337,8 @@ Source99: baselibs.conf Source100: %{name}-rpmlintrc # Upstream patches Patch0: 2edd63a0-fix-virFileSetCOW-logic.patch +Patch1: 82bb167f-dont-cache-devmapper-major.patch +Patch2: feb8564a-handle-no-devmapper.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch @@ -877,6 +879,8 @@ libvirt plugin for NSS for translating domain names into IP addresses. %prep %setup -q %patch0 -p1 +%patch1 -p1 +%patch2 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1 From 92aa53bd73780647e269b9d207c04aefafa4b9a15b6d014ebbc1eaca529c337e Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Wed, 19 Aug 2020 21:53:01 +0000 Subject: [PATCH 3/3] add another device mapper fix OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=838 --- 53d9af1e-ignore-devmapper-open-errors.patch | 74 +++++++++++++++++++++ libvirt.changes | 3 +- libvirt.spec | 2 + 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 53d9af1e-ignore-devmapper-open-errors.patch diff --git a/53d9af1e-ignore-devmapper-open-errors.patch b/53d9af1e-ignore-devmapper-open-errors.patch new file mode 100644 index 0000000..965f9f3 --- /dev/null +++ b/53d9af1e-ignore-devmapper-open-errors.patch @@ -0,0 +1,74 @@ +commit 53d9af1e7924757e3b5f661131dd707d7110d094 +Author: Michal Prívozník +Date: Wed Aug 19 13:35:55 2020 +0200 + + virdevmapper: Ignore all errors when opening /dev/mapper/control + + So far, only ENOENT is ignored (to deal with kernels without + devmapper). However, as reported on the list, under certain + scenarios a different error can occur. For instance, when libvirt + is running inside a container which doesn't have permissions to + talk to the devmapper. If this is the case, then open() returns + -1 and sets errno=EPERM. + + Assuming that multipath devices are fairly narrow use case and + using them in a restricted container is even more narrow the best + fix seems to be to ignore all open errors BUT produce a warning + on failure. To avoid flooding logs with warnings on kernels + without devmapper the level is reduced to a plain debug message. + + Reported-by: Christian Ehrhardt + Reviewed-by: Christian Ehrhardt + Signed-off-by: Michal Privoznik + +Index: libvirt-6.6.0/src/util/virdevmapper.c +=================================================================== +--- libvirt-6.6.0.orig/src/util/virdevmapper.c ++++ libvirt-6.6.0/src/util/virdevmapper.c +@@ -35,9 +35,12 @@ + # include "viralloc.h" + # include "virstring.h" + # include "virfile.h" ++# include "virlog.h" + + # define VIR_FROM_THIS VIR_FROM_STORAGE + ++VIR_LOG_INIT("util.virdevmapper"); ++ + # define PROC_DEVICES "/proc/devices" + # define DM_NAME "device-mapper" + # define DEV_DM_DIR "/dev/" DM_DIR +@@ -130,11 +133,15 @@ virDMOpen(void) + memset(&dm, 0, sizeof(dm)); + + if ((controlFD = open(CONTROL_PATH, O_RDWR)) < 0) { +- if (errno == ENOENT) +- return -2; +- +- virReportSystemError(errno, _("Unable to open %s"), CONTROL_PATH); +- return -1; ++ /* We can't talk to devmapper. Produce a warning and let ++ * the caller decide what to do next. */ ++ if (errno == ENOENT) { ++ VIR_DEBUG("device mapper not available"); ++ } else { ++ VIR_WARN("unable to open %s: %s", ++ CONTROL_PATH, g_strerror(errno)); ++ } ++ return -2; + } + + if (!virDMIoctl(controlFD, DM_VERSION, &dm, &tmp)) { +@@ -310,9 +317,9 @@ virDevMapperGetTargets(const char *path, + + if ((controlFD = virDMOpen()) < 0) { + if (controlFD == -2) { +- /* The CONTROL_PATH doesn't exist. Probably the +- * module isn't loaded, yet. Don't error out, just +- * exit. */ ++ /* The CONTROL_PATH doesn't exist or is unusable. ++ * Probably the module isn't loaded, yet. Don't error ++ * out, just exit. */ + return 0; + } + diff --git a/libvirt.changes b/libvirt.changes index e57170e..e18642d 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -3,7 +3,8 @@ Wed Aug 19 19:36:52 UTC 2020 - James Fehlig - virdevmapper: Handle kernel without device-mapper support 82bb167f-dont-cache-devmapper-major.patch, - feb8564a-handle-no-devmapper.patch + feb8564a-handle-no-devmapper.patch, + 53d9af1e-ignore-devmapper-open-errors.patch boo#1175465 ------------------------------------------------------------------- diff --git a/libvirt.spec b/libvirt.spec index 8e2f6c2..9267ba4 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -339,6 +339,7 @@ Source100: %{name}-rpmlintrc Patch0: 2edd63a0-fix-virFileSetCOW-logic.patch Patch1: 82bb167f-dont-cache-devmapper-major.patch Patch2: feb8564a-handle-no-devmapper.patch +Patch3: 53d9af1e-ignore-devmapper-open-errors.patch # Patches pending upstream review Patch100: libxl-dom-reset.patch Patch101: network-don-t-use-dhcp-authoritative-on-static-netwo.patch @@ -881,6 +882,7 @@ libvirt plugin for NSS for translating domain names into IP addresses. %patch0 -p1 %patch1 -p1 %patch2 -p1 +%patch3 -p1 %patch100 -p1 %patch101 -p1 %patch150 -p1