src/util/virdevmapper.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
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 <christian.ehrhardt@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/util/virdevmapper.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 3cc399f382..7c89c2a952 100644
--- a/src/util/virdevmapper.c
+++ b/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)) {
@@ -309,9 +316,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;
}
--
2.26.2
On Wed, Aug 19, 2020 at 4:13 PM Michal Privoznik <mprivozn@redhat.com> wrote: > > 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. While I'd love a warn_once such a thing doesn't exist right now. It isn't the most common case and as discussed on IRC users are kind of used to trim logs, so it should be ok. Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > src/util/virdevmapper.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c > index 3cc399f382..7c89c2a952 100644 > --- a/src/util/virdevmapper.c > +++ b/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)) { > @@ -309,9 +316,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; > } > > -- > 2.26.2 > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
© 2016 - 2024 Red Hat, Inc.