Re: [Libguestfs] [nbdkit PATCH 1/3] connections: Don't advertise TRIM on readonly connection

Richard W.M. Jones
2018-01-24T14:19:40Z

On Tue, Jan 23, 2018 at 10:10:13PM -0600, Eric Blake
wrote:
> A client cannot attempt TRIM on a read-only connection (we already
> filter it in connections.c with EROFS); as such, it makes no sense
> to query the plugin's .can_trim nor to advertise TRIM support to
> the client on such a connection, similar to how we do not
> advertise WRITE_ZEROES.
>
> FUA only has defined meanings on write operations, but the NBD
> protocol allows us to advertise it on reads and to ignore clients
> that send it on read (older qemu did so), so we can continue to
> advertise it regardless of readonly state.  Likewise, while flush
> is unlikely to make a difference on a read-only connection, there
> is no reason why it shouldn't be advertised.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  src/connections.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/connections.c b/src/connections.c
> index 8e110a5..75c2c2d 100644
> --- a/src/connections.c
> +++ b/src/connections.c
> @@ -427,6 +427,14 @@ compute_eflags (struct connection *conn, uint16_t
*flags)
>    }
>    if (!conn->readonly) {
>      eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
> +
> +    fl = backend->can_trim (backend, conn);
> +    if (fl == -1)
> +      return -1;
> +    if (fl) {
> +      eflags |= NBD_FLAG_SEND_TRIM;
> +      conn->can_trim = 1;
> +    }
>    }
>
>    fl = backend->can_flush (backend, conn);
> @@ -445,14 +453,6 @@ compute_eflags (struct connection *conn, uint16_t
*flags)
>      conn->is_rotational = 1;
>    }
>
> -  fl = backend->can_trim (backend, conn);
> -  if (fl == -1)
> -    return -1;
> -  if (fl) {
> -    eflags |= NBD_FLAG_SEND_TRIM;
> -    conn->can_trim = 1;
> -  }
> -
>    *flags = eflags;
>    return 0;
>  }
> --

Simple & obvious, ACK.

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.