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

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(a)redhat.com&gt; --- 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; } --