search for: nbd_cmd_trim

...tatic bool validate_request (struct connection *conn, uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count, uint32_t *error) { int r; + /* Readonly connection? */ + if (conn->readonly && + (cmd == NBD_CMD_WRITE || + cmd == NBD_CMD_TRIM || cmd == NBD_CMD_WRITE_ZEROES)) { + nbdkit_error ("invalid request: write request on readonly connection"); + *error = EROFS; + return false; + } + /* Validate cmd, offset, count. */ switch (cmd) { case NBD_CMD_READ: case NBD_CMD_WRITE: case NBD_CMD_TRIM: case...

...favor of the double check: a completely non-sparse file is just wasting time on further lseek() calls that won't tell us anything different than our default of all data (and which may be helpful for things like tmpfs that have abysmal lseek() performance). Perhaps a reason against: if we expect NBD_CMD_TRIM/NBD_CMD_WRITE_ZEROES or even a third-party to be punching holes in the meantime, then opting out of future lseek() won't see those later additions of holes. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...@ -64,14 +64,19 @@ validate_request (struct connection *conn, uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, uint32_t *error) { + int r; + /* Readonly connection? */ - if (conn->readonly && - (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || - cmd == NBD_CMD_WRITE_ZEROES)) { - nbdkit_error ("invalid request: %s: write request on readonly connection", - name_of_nbd_cmd (cmd)); - *error = EROFS; - return false; + if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || + cmd == NBD_CMD_WRITE_Z...

...mp; offset + count <= exportsize; -} - static bool validate_request (struct connection *conn, uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, uint32_t *error) { - int r; - - /* Readonly connection? */ - if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || - cmd == NBD_CMD_WRITE_ZEROES) { - r = backend_can_write (backend, conn); - assert (r >= 0); /* Guaranteed by eflags computation */ - if (!r) { - nbdkit_error ("invalid request: %s: write request on readonly connection", - name_of_nbd_cmd (cmd))...

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

...r this, but Eric should know. > However, since you suggest that we could use "trim" request for these > requests, it means that these requests are advisory (since trim is), and > we can just ignore them if the server does not support trim. What qemu-img sends shouldn't be a NBD_CMD_TRIM request (which is indeed advisory), but a NBD_CMD_WRITE_ZEROES request. qemu-img relies on the image actually being zeroed after this. Kevin

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.

...ever, since you suggest that we could use "trim" request for these > > > requests, it means that these requests are advisory (since trim is), and > > > we can just ignore them if the server does not support trim. > > > > What qemu-img sends shouldn't be a NBD_CMD_TRIM request (which is indeed > > advisory), but a NBD_CMD_WRITE_ZEROES request. qemu-img relies on the > > image actually being zeroed after this. > > > > So it seems that may_trim=1 is wrong, since trim cannot replace zero. Note that the current plugin ignores may_trim. It i...

...d_in_flight *cmd; - if (nbd_unlocked_read_only (conn->h) == 1) { set_error (EINVAL, "server does not support write operations"); return -1; @@ -397,21 +378,14 @@ nbd_unlocked_aio_trim (struct nbd_connection *conn, return -1; } - cmd = command_common (conn, flags, NBD_CMD_TRIM, offset, count, NULL); - if (!cmd) - return -1; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; - - return cmd->handle; + return nbd_internal_command_common (conn, flags, NBD_CMD_TRIM, offset, count, + NULL, 0, NULL); } i...

Tested via: term1$ qemu-nbd -k $PWD/sock -t -f raw -x foo junk --trace=nbd_\* term2$ ./nbdkit -f -v -e bar nbd socket=$PWD/sock export=foo term3$ qemu-io -t none -f raw nbd://localhost:10809/bar --trace=nbd_\* and checking the traces to see that 'w 0 1' vs. 'w -f 0 1' was able to influence whether the FUA flag showed up at the server in term1. Still to go: figure out how to

...exportsize; -} - -static bool -validate_request (struct connection *conn, - uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, - uint32_t *error) -{ - /* Readonly connection? */ - if (conn->readonly && - (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || - cmd == NBD_CMD_WRITE_ZEROES)) { - nbdkit_error ("invalid request: %s: write request on readonly connection", - name_of_nbd_cmd (cmd)); - *error = EROFS; - return false; - } - - /* Validate cmd, offset, count. */ - switch (cmd) { - case NBD_CMD_READ...

.../* block type (hole etc) */ +} __attribute__((packed)); + /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. */ @@ -199,10 +212,12 @@ extern const char *name_of_nbd_cmd (int); #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 +#define NBD_CMD_BLOCK_STATUS 7 extern const char *name_of_nbd_cmd_flag (int); #define NBD_CMD_FLAG_FUA (1<<0) #define NBD_CMD_FLAG_NO_HOLE (1<<1) +#define NBD_CMD_FLAG_REQ_ONE (1<<3) /* Error codes (previousl...

So I had a think about this. Isn't this easier/better solved by lifting the 32 bit restriction on the size of certain non-data requests (ie. NBD_CMD_BLOCK_STATUS, NBD_CMD_TRIM, NBD_CMD_WRITE_ZEROES). The client can then query the disk efficiently to see if it starts as zeroes, and can decide on the basis of that whether it needs to "infill" zeroes as it goes along, or can ignore zeroes because they are already zero. While at the same time lifting this restric...

...check: a completely non-sparse file is just > wasting time on further lseek() calls that won't tell us anything > different than our default of all data (and which may be helpful for > things like tmpfs that have abysmal lseek() performance). Perhaps a > reason against: if we expect NBD_CMD_TRIM/NBD_CMD_WRITE_ZEROES or even a > third-party to be punching holes in the meantime, then opting out of > future lseek() won't see those later additions of holes. This last point seems crucial - the file could become sparse, even if it starts off as non-sparse. The test is designed really...

...ne NBD_FLAG_SEND_CACHE (1 << 10) /* NBD options (new style handshake only). */ extern const char *name_of_nbd_opt (int); @@ -217,6 +218,7 @@ extern const char *name_of_nbd_cmd (int); #define NBD_CMD_DISC 2 /* Disconnect. */ #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 +#define NBD_CMD_CACHE 5 #define NBD_CMD_WRITE_ZEROES 6 #define NBD_CMD_BLOCK_STATUS 7 diff --git a/server/internal.h b/server/internal.h index 67fccfc..6414a78 100644 --- a/server/internal.h +++ b/server/internal.h @@ -170,6 +170,7 @@ struct connection {...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

...e you suggest that we could use "trim" request for these >>>> requests, it means that these requests are advisory (since trim is), and >>>> we can just ignore them if the server does not support trim. >>> >>> What qemu-img sends shouldn't be a NBD_CMD_TRIM request (which is indeed >>> advisory), but a NBD_CMD_WRITE_ZEROES request. qemu-img relies on the >>> image actually being zeroed after this. >>> >> >> So it seems that may_trim=1 is wrong, since trim cannot replace zero. > > Note that the current plug...

...> > > However, since you suggest that we could use "trim" request for these > > requests, it means that these requests are advisory (since trim is), and > > we can just ignore them if the server does not support trim. > > What qemu-img sends shouldn't be a NBD_CMD_TRIM request (which is indeed > advisory), but a NBD_CMD_WRITE_ZEROES request. qemu-img relies on the > image actually being zeroed after this. > So it seems that may_trim=1 is wrong, since trim cannot replace zero. Nir

...command_common (conn, 0, NBD_CMD_FLUSH, 0, 0, NULL); if (!cmd) return -1; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; return cmd->handle; } @@ -400,8 +396,6 @@ nbd_unlocked_aio_trim (struct nbd_connection *conn, cmd = command_common (conn, flags, NBD_CMD_TRIM, offset, count, NULL); if (!cmd) return -1; - if (nbd_internal_run (conn->h, conn, cmd_issue) == -1) - return -1; return cmd->handle; } @@ -424,8 +418,6 @@ nbd_unlocked_aio_cache (struct nbd_connection *conn, cmd = command_common (conn, 0, NBD_CMD_CACHE, offset, count, NUL...

...fine NBD_REPLY_TYPE_BLOCK_STATUS 3 +#define NBD_REPLY_TYPE_BLOCK_STATUS 5 #define NBD_REPLY_TYPE_ERROR ((1<<15) + 1) #define NBD_REPLY_TYPE_ERROR_OFFSET ((1<<15) + 2) @@ -199,10 +212,12 @@ extern const char *name_of_nbd_cmd (int); #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 +#define NBD_CMD_BLOCK_STATUS 7 extern const char *name_of_nbd_cmd_flag (int); #define NBD_CMD_FLAG_FUA (1<<0) #define NBD_CMD_FLAG_NO_HOLE (1<<1) +#define NBD_CMD_FLAG_REQ_ONE (1<<3) /* Error codes (previousl...