search for: nbd_cmd_write_zero

...quest (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 NBD_CMD_WRITE_ZEROES: - r...

...t 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 is not used at all, so it's not relevant to this problem. However t...

While it may be counterintuitive at first, the introduction of NBD_CMD_WRITE_ZEROES and NBD_CMD_BLOCK_STATUS has caused a performance regression in qemu [1], when copying a sparse file. When the destination file must contain the same contents as the source, but it is not known in advance whether the destination started life with all zero content, then there are cases where it is...

While it may be counterintuitive at first, the introduction of NBD_CMD_WRITE_ZEROES and NBD_CMD_BLOCK_STATUS has caused a performance regression in qemu [1], when copying a sparse file. When the destination file must contain the same contents as the source, but it is not known in advance whether the destination started life with all zero content, then there are cases where it is...

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 clie...

...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

On Fri, Mar 22, 2019 at 6:43 PM Eric Blake <eblake@redhat.com> wrote: > While it may be counterintuitive at first, the introduction of > NBD_CMD_WRITE_ZEROES and NBD_CMD_BLOCK_STATUS has caused a performance > regression in qemu [1], when copying a sparse file. When the > destination file must contain the same contents as the source, but it > is not known in advance whether the destination started life with all > zero content, then there a...

...gt;>>> 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 is not used at all, > so it's not relevant...

...uests, 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 is not used at all, > so it's not rel...

I'm planning on tweaking the language callbacks to support fua; first up is the python bindings. I want to move from: def zero(h, count, offset, may_trim): to a nicer: def zero(h, count, offset, may_trim=False, fua=False): where the C code passes keywords for the flags (we can add new flags as needed), perhaps by using introspection to learn whether the plugin has a mandatory may_trim

...tsize; -} - 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)); - *error = EROFS; - retu...

...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

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.

The existing test of language bindings was not covering any use of NBD_CMD_WRITE_ZEROES, making it harder to test changes to the plugin's zero callback. Recent Linux kernels are now smart enough to turn fallocate(2) with FALLOC_FL_ZERO_RANGE into a SCSI WRITE SAME request, which qemu in turn converts into an NBD_CMD_WRITE_ZEROES. Since libguestfs does not directly have an API...

No need to calloc/free a buffer every time NBD_CMD_WRITE_ZEROES has to fall back to a .pread call. Just reserve the maximum buffer up front in our bss. Signed-off-by: Eric Blake <eblake@redhat.com> --- I noticed that buf was a candidate for CLEANUP_FREE, but then further noticed that we can avoid the calloc/free altogether if we don't mind the bss...

...uest (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_ZEROES) { + r = backend_can_write (...

...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: - case NBD_CMD_WRITE: - case NBD_...

...sal where I'm also going to submit a qemu patch: https://lists.debian.org/nbd/2018/03/msg00017.html Right now, the NBD protocol has no clean distinction between maximum data request (hard limit of 32M for NBD_CMD_WRITE in qemu-img) and for maximum length on a request with no accompanying data (NBD_CMD_WRITE_ZEROES). Once we add NBD_INFO_ZERO_SIZE, then it becomes obvious that sending a 2G NBD_CMD_WRITE_ZEROES request makes sense, even when 32M is the maximum for a normal write; but until that point, qemu is being conservative and capping EVERYTHING to the 32M limit. There's also talk about enhancing...

On 8/23/19 1:48 PM, Wouter Verhelst wrote: > On Fri, Aug 23, 2019 at 09:34:26AM -0500, Eric Blake wrote: >> +- bit 4, `NBD_CMD_FLAG_FAST_ZERO`; valid during >> + `NBD_CMD_WRITE_ZEROES`. If set, but the server cannot perform the >> + write zeroes any faster than it would for an equivalent >> + `NBD_CMD_WRITE`, > > One way of fulfilling the letter of this requirement but not its spirit > could be to have background writes; that is, the server makes a note...

...plement the > poor-man's version that always reports that the entire image is data, > for all requests, without any feedback from the plugins). The former > feels odd, but the latter seems oddly appealing as progress towards our > end goal (kind of how our initial implementation of NBD_CMD_WRITE_ZEROES > was always advertised even before we finished wiring up plug support). A good point here is what happens with the block-status branch[1]. Let's see: $ ./nbdkit memory size=64M --run '/home/rjones/d/qemu/qemu-img convert $nbd /var/tmp/out' qemu-img: Payload too large nbdkit: mem...