[PATCH] virdevmapper: Ignore all errors when opening /dev/mapper/control

[Christian Ehrhardt]

On Wed, Aug 19, 2020 at 4:13 PM Michal Privoznik <mprivozn at 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 at canonical.com>

> Reported-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> Signed-off-by: Michal Privoznik <mprivozn at 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