[Libguestfs] [nbdkit PATCH 1/4] server: Normalize plugin can_* values

[Richard W.M. Jones]

On Mon, Mar 16, 2020 at 10:36:14PM -0500, Eric Blake wrote:
> The documentation for .can_write and friends mentioned merely that the
> callback must return a boolean value, or -1 on failure.  Historically,
> we have treated any non-zero value other than -1 as true, and some
> plugins have sloppily relied on this behavior, including the
> standalone nbd plugin prior to the use of libnbd in v1.14.  But in
> 1.15.1, we added an assert in our backend code if an enabled feature
> bit is not exactly 1.  To avoid tripping the assertion, we have to
> normalize the plugin values before storing our internal value.
> 
> Fixes: c306fa93ab
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  server/plugins.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/server/plugins.c b/server/plugins.c
> index 78ed6723..4a4fcdee 100644
> --- a/server/plugins.c
> +++ b/server/plugins.c
> @@ -1,5 +1,5 @@
>  /* nbdkit
> - * Copyright (C) 2013-2019 Red Hat Inc.
> + * Copyright (C) 2013-2020 Red Hat Inc.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions are
> @@ -308,13 +308,21 @@ plugin_get_size (struct backend *b, void *handle)
>    return p->plugin.get_size (handle);
>  }
> 
> +static int normalize_bool (int value)
> +{
> +  if (value == -1 || value == 0)
> +    return value;
> +  /* Normalize all other non-zero values to true */
> +  return 1;
> +}
> +
>  static int
>  plugin_can_write (struct backend *b, void *handle)
>  {
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.can_write)
> -    return p->plugin.can_write (handle);
> +    return normalize_bool (p->plugin.can_write (handle));
>    else
>      return p->plugin.pwrite || p->plugin._pwrite_v1;
>  }
> @@ -325,7 +333,7 @@ plugin_can_flush (struct backend *b, void *handle)
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.can_flush)
> -    return p->plugin.can_flush (handle);
> +    return normalize_bool (p->plugin.can_flush (handle));
>    else
>      return p->plugin.flush || p->plugin._flush_v1;
>  }
> @@ -336,7 +344,7 @@ plugin_is_rotational (struct backend *b, void *handle)
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.is_rotational)
> -    return p->plugin.is_rotational (handle);
> +    return normalize_bool (p->plugin.is_rotational (handle));
>    else
>      return 0; /* assume false */
>  }
> @@ -347,7 +355,7 @@ plugin_can_trim (struct backend *b, void *handle)
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.can_trim)
> -    return p->plugin.can_trim (handle);
> +    return normalize_bool (p->plugin.can_trim (handle));
>    else
>      return p->plugin.trim || p->plugin._trim_v1;
>  }
> @@ -380,7 +388,7 @@ plugin_can_fast_zero (struct backend *b, void *handle)
>    int r;
> 
>    if (p->plugin.can_fast_zero)
> -    return p->plugin.can_fast_zero (handle);
> +    return normalize_bool (p->plugin.can_fast_zero (handle));
>    /* Advertise support for fast zeroes if no .zero or .can_zero is
>     * false: in those cases, we fail fast instead of using .pwrite.
>     * This also works when v1 plugin has only ._zero_v1.
> @@ -399,7 +407,7 @@ plugin_can_extents (struct backend *b, void *handle)
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.can_extents)
> -    return p->plugin.can_extents (handle);
> +    return normalize_bool (p->plugin.can_extents (handle));
>    else
>      return p->plugin.extents != NULL;
>  }
> @@ -430,7 +438,7 @@ plugin_can_multi_conn (struct backend *b, void *handle)
>    struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
> 
>    if (p->plugin.can_multi_conn)
> -    return p->plugin.can_multi_conn (handle);
> +    return normalize_bool (p->plugin.can_multi_conn (handle));
>    else
>      return 0; /* assume false */
>  }
> -- 

Nice one, ACK.

I guess this one will need to be backported to (only?) 1.16 and 1.18?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v