From: Akihiko Odaki <akihiko.odaki@daynix.com>

vhost_dev_cleanup() clears vhost_dev so back up its vqs member to free

the memory pointed by the member.

Fixes: 98fc1ada4c ("virtio: add vhost-user-fs base device")

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-Id: <20230130140225.77964-1-akihiko.odaki@daynix.com>

---

hw/virtio/vhost-user-fs.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/virtio/vhost-user-fs.c

+++ b/hw/virtio/vhost-user-fs.c

@@ -XXX,XX +XXX,XX @@ static void vuf_device_unrealize(DeviceState *dev)

{

VirtIODevice *vdev = VIRTIO_DEVICE(dev);

VHostUserFS *fs = VHOST_USER_FS(dev);

+ struct vhost_virtqueue *vhost_vqs = fs->vhost_dev.vqs;

int i;

/* This will stop vhost backend if appropriate. */

@@ -XXX,XX +XXX,XX @@ static void vuf_device_unrealize(DeviceState *dev)

}

g_free(fs->req_vqs);

virtio_cleanup(vdev);

- g_free(fs->vhost_dev.vqs);

- fs->vhost_dev.vqs = NULL;

+ g_free(vhost_vqs);

}

static struct vhost_dev *vuf_get_vhost(VirtIODevice *vdev)

--

2.39.1

Steps to reproduce the issue: simply boot a VM with

-object '{"qom-type":"iothread","id":"iothread1"}' \

-blockdev '{"driver":"file","filename":"$QCOW2","aio":"native","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}' \

-blockdev '{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"qcow2","file":"libvirt-1-storage"}' \

-device virtio-blk-pci,iothread=iothread1,drive=libvirt-1-format,id=virtio-disk0,bootindex=1,write-cache=on

and observe that it will fail not manage to boot with "qemu_mutex_unlock_impl: Operation not permitted"

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Tested-by: Lukáš Doktor <ldoktor@redhat.com>

When a write request is converted into a write zeroes request by the

detect-zeroes= feature, it is no longer associated with an I/O buffer.

The BDRV_REQ_REGISTERED_BUF flag doesn't make sense without an I/O

buffer and must be cleared because bdrv_co_do_pwrite_zeroes() fails with

-EINVAL when it's set.

Fiona Ebner <f.ebner@proxmox.com> bisected and diagnosed this QEMU 7.2

regression where writes containing zeroes to a blockdev with

discard=unmap,detect-zeroes=unmap fail.

Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1404

Fixes: e8b6535533be ("block: add BDRV_REQ_REGISTERED_BUF request flag")

Tested-by: Fiona Ebner <f.ebner@proxmox.com>

Cc: qemu-stable@nongnu.org

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-Id: <20230207203719.242926-2-stefanha@redhat.com>

---

block/io.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/block/io.c b/block/io.c

index XXXXXXX..XXXXXXX 100644

--- a/block/io.c

+++ b/block/io.c

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,

if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {

flags |= BDRV_REQ_MAY_UNMAP;

}

+

+ /* Can't use optimization hint with bufferless zero write */

+ flags &= ~BDRV_REQ_REGISTERED_BUF;

}

if (ret < 0) {

--

2.39.1

The block layer APIs use BdrvRequestFlags while qemu-io code uses int.

Although the code compiles and runs fine, BdrvRequestFlags is clearer

because it differentiates between other types of flags like bdrv_open()

flags.

This is purely refactoring.

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-Id: <20230207203719.242926-3-stefanha@redhat.com>

---

qemu-io-cmds.c | 13 +++++++------

1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-io-cmds.c

+++ b/qemu-io-cmds.c

@@ -XXX,XX +XXX,XX @@ static int do_pread(BlockBackend *blk, char *buf, int64_t offset,

}

static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,

- int64_t bytes, int flags, int64_t *total)

+ int64_t bytes, BdrvRequestFlags flags, int64_t *total)

{

int ret;

@@ -XXX,XX +XXX,XX @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,

}

static int do_pwrite_zeroes(BlockBackend *blk, int64_t offset,

- int64_t bytes, int flags, int64_t *total)

+ int64_t bytes, BdrvRequestFlags flags,

+ int64_t *total)

{

int ret = blk_pwrite_zeroes(blk, offset, bytes,

flags | BDRV_REQ_ZERO_WRITE);

@@ -XXX,XX +XXX,XX @@ static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov,

}

static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov,

- int64_t offset, int flags, int *total)

+ int64_t offset, BdrvRequestFlags flags, int *total)

{

int async_ret = NOT_DONE;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

struct timespec t1, t2;

bool Cflag = false, qflag = false, bflag = false;

bool Pflag = false, zflag = false, cflag = false, sflag = false;

- int flags = 0;

+ BdrvRequestFlags flags = 0;

int c, cnt, ret;

char *buf = NULL;

int64_t offset;

@@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv)

{

struct timespec t1, t2;

bool Cflag = false, qflag = false;

- int flags = 0;

+ BdrvRequestFlags flags = 0;

int c, cnt, ret;

char *buf;

int64_t offset;

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

int nr_iov, c;

int pattern = 0xcd;

struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);

- int flags = 0;

+ BdrvRequestFlags flags = 0;

ctx->blk = blk;

while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) {

--

2.39.1

The blk_register_buf() API is an optimization hint that allows some

block drivers to avoid I/O buffer housekeeping or bounce buffers.

Add an -r option to register the I/O buffer so that qemu-io can be used

to test the blk_register_buf() API. The next commit will add a test that

uses the new option.

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-Id: <20230207203719.242926-4-stefanha@redhat.com>

---

qemu-io-cmds.c | 204 +++++++++++++++++++++++++++++++------------------

1 file changed, 129 insertions(+), 75 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-io-cmds.c

+++ b/qemu-io-cmds.c

@@ -XXX,XX +XXX,XX @@ static int parse_pattern(const char *arg)

*/

#define MISALIGN_OFFSET 16

-static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern)

+static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern,

+ bool register_buf)

{

void *buf;

@@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern)

}

buf = blk_blockalign(blk, len);

memset(buf, pattern, len);

+ if (register_buf) {

+ blk_register_buf(blk, buf, len, &error_abort);

+ }

if (qemuio_misalign) {

buf += MISALIGN_OFFSET;

}

return buf;

}

-static void qemu_io_free(void *p)

+static void qemu_io_free(BlockBackend *blk, void *p, size_t len,

+ bool unregister_buf)

{

if (qemuio_misalign) {

p -= MISALIGN_OFFSET;

+ len += MISALIGN_OFFSET;

+ }

+ if (unregister_buf) {

+ blk_unregister_buf(blk, p, len);

}

qemu_vfree(p);

}

@@ -XXX,XX +XXX,XX @@ static void qemu_io_free(void *p)

* @blk - the block backend where the buffer content is going to be written to

* @len - the buffer length

* @file_name - the file to read the content from

+ * @register_buf - call blk_register_buf()

*

* Returns: the buffer pointer on success

* NULL on error

*/

static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,

- const char *file_name)

+ const char *file_name, bool register_buf)

{

- char *buf, *buf_origin;

+ size_t alloc_len = len + (qemuio_misalign ? MISALIGN_OFFSET : 0);

+ char *alloc_buf, *buf, *end;

FILE *f = fopen(file_name, "r");

int pattern_len;

@@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,

return NULL;

}

- if (qemuio_misalign) {

- len += MISALIGN_OFFSET;

- }

-

- buf_origin = buf = blk_blockalign(blk, len);

+ alloc_buf = buf = blk_blockalign(blk, alloc_len);

if (qemuio_misalign) {

- buf_origin += MISALIGN_OFFSET;

buf += MISALIGN_OFFSET;

- len -= MISALIGN_OFFSET;

}

- pattern_len = fread(buf_origin, 1, len, f);

+ pattern_len = fread(buf, 1, len, f);

if (ferror(f)) {

perror(file_name);

@@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,

fclose(f);

f = NULL;

- if (len > pattern_len) {

- len -= pattern_len;

- buf += pattern_len;

-

- while (len > 0) {

- size_t len_to_copy = MIN(pattern_len, len);

-

- memcpy(buf, buf_origin, len_to_copy);

+ if (register_buf) {

+ blk_register_buf(blk, alloc_buf, alloc_len, &error_abort);

+ }

- len -= len_to_copy;

- buf += len_to_copy;

- }

+ end = buf + len;

+ for (char *p = buf + pattern_len; p < end; p += pattern_len) {

+ memcpy(p, buf, MIN(pattern_len, end - p));

}

- return buf_origin;

+ return buf;

error:

- qemu_io_free(buf_origin);

+ /*

+ * This code path is only taken before blk_register_buf() is called, so

+ * hardcode the qemu_io_free() unregister_buf argument to false.

+ */

+ qemu_io_free(blk, alloc_buf, alloc_len, false);

if (f) {

fclose(f);

}

@@ -XXX,XX +XXX,XX @@ static void print_report(const char *op, struct timespec *t, int64_t offset,

*/

static void *

create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov,

- int pattern)

+ int pattern, bool register_buf)

{

size_t *sizes = g_new0(size_t, nr_iov);

size_t count = 0;

@@ -XXX,XX +XXX,XX @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov,

qemu_iovec_init(qiov, nr_iov);

- buf = p = qemu_io_alloc(blk, count, pattern);

+ buf = p = qemu_io_alloc(blk, count, pattern, register_buf);

for (i = 0; i < nr_iov; i++) {

qemu_iovec_add(qiov, p, sizes[i]);

@@ -XXX,XX +XXX,XX @@ fail:

}

static int do_pread(BlockBackend *blk, char *buf, int64_t offset,

- int64_t bytes, int64_t *total)

+ int64_t bytes, BdrvRequestFlags flags, int64_t *total)

{

int ret;

@@ -XXX,XX +XXX,XX @@ static int do_pread(BlockBackend *blk, char *buf, int64_t offset,

return -ERANGE;

}

- ret = blk_pread(blk, offset, bytes, (uint8_t *)buf, 0);

+ ret = blk_pread(blk, offset, bytes, (uint8_t *)buf, flags);

if (ret < 0) {

return ret;

}

@@ -XXX,XX +XXX,XX @@ static void aio_rw_done(void *opaque, int ret)

}

static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov,

- int64_t offset, int *total)

+ int64_t offset, BdrvRequestFlags flags, int *total)

{

int async_ret = NOT_DONE;

- blk_aio_preadv(blk, offset, qiov, 0, aio_rw_done, &async_ret);

+ blk_aio_preadv(blk, offset, qiov, flags, aio_rw_done, &async_ret);

while (async_ret == NOT_DONE) {

main_loop_wait(false);

}

@@ -XXX,XX +XXX,XX @@ static void read_help(void)

" -p, -- ignored for backwards compatibility\n"

" -P, -- use a pattern to verify read data\n"

" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

" -s, -- start offset for pattern verification (only with -P)\n"

" -v, -- dump buffer to standard output\n"

"\n");

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t read_cmd = {

.cfunc = read_f,

.argmin = 2,

.argmax = -1,

- .args = "[-abCqv] [-P pattern [-s off] [-l len]] off len",

+ .args = "[-abCqrv] [-P pattern [-s off] [-l len]] off len",

.oneline = "reads a number of bytes at a specified offset",

.help = read_help,

};

@@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv)

int64_t total = 0;

int pattern = 0;

int64_t pattern_offset = 0, pattern_count = 0;

+ BdrvRequestFlags flags = 0;

- while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) {

+ while ((c = getopt(argc, argv, "bCl:pP:qrs:v")) != -1) {

switch (c) {

case 'b':

bflag = true;

@@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv)

case 'q':

qflag = true;

break;

+ case 'r':

+ flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 's':

sflag = true;

pattern_offset = cvtnum(optarg);

@@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv)

count);

return -EINVAL;

}

+ if (flags & BDRV_REQ_REGISTERED_BUF) {

+ printf("I/O buffer registration is not supported when reading "

+ "from vmstate\n");

+ return -EINVAL;

+ }

}

- buf = qemu_io_alloc(blk, count, 0xab);

+ buf = qemu_io_alloc(blk, count, 0xab, flags & BDRV_REQ_REGISTERED_BUF);

clock_gettime(CLOCK_MONOTONIC, &t1);

if (bflag) {

ret = do_load_vmstate(blk, buf, offset, count, &total);

} else {

- ret = do_pread(blk, buf, offset, count, &total);

+ ret = do_pread(blk, buf, offset, count, flags, &total);

}

clock_gettime(CLOCK_MONOTONIC, &t2);

@@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv)

print_report("read", &t2, offset, count, total, cnt, Cflag);

out:

- qemu_io_free(buf);

+ qemu_io_free(blk, buf, count, flags & BDRV_REQ_REGISTERED_BUF);

return ret;

}

@@ -XXX,XX +XXX,XX @@ static void readv_help(void)

" Uses multiple iovec buffers if more than one byte range is specified.\n"

" -C, -- report statistics in a machine parsable format\n"

" -P, -- use a pattern to verify read data\n"

-" -v, -- dump buffer to standard output\n"

" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

+" -v, -- dump buffer to standard output\n"

"\n");

}

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t readv_cmd = {

.cfunc = readv_f,

.argmin = 2,

.argmax = -1,

- .args = "[-Cqv] [-P pattern] off len [len..]",

+ .args = "[-Cqrv] [-P pattern] off len [len..]",

.oneline = "reads a number of bytes at a specified offset",

.help = readv_help,

};

@@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv)

QEMUIOVector qiov;

int pattern = 0;

bool Pflag = false;

+ BdrvRequestFlags flags = 0;

- while ((c = getopt(argc, argv, "CP:qv")) != -1) {

+ while ((c = getopt(argc, argv, "CP:qrv")) != -1) {

switch (c) {

case 'C':

Cflag = true;

@@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv)

case 'q':

qflag = true;

break;

+ case 'r':

+ flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 'v':

vflag = true;

break;

@@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv)

optind++;

nr_iov = argc - optind;

- buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab);

+ buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab,

+ flags & BDRV_REQ_REGISTERED_BUF);

if (buf == NULL) {

return -EINVAL;

}

clock_gettime(CLOCK_MONOTONIC, &t1);

- ret = do_aio_readv(blk, &qiov, offset, &total);

+ ret = do_aio_readv(blk, &qiov, offset, flags, &total);

clock_gettime(CLOCK_MONOTONIC, &t2);

if (ret < 0) {

@@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv)

print_report("read", &t2, offset, qiov.size, total, cnt, Cflag);

out:

+ qemu_io_free(blk, buf, qiov.size, flags & BDRV_REQ_REGISTERED_BUF);

qemu_iovec_destroy(&qiov);

- qemu_io_free(buf);

return ret;

}

@@ -XXX,XX +XXX,XX @@ static void write_help(void)

" filled with a set pattern (0xcdcdcdcd).\n"

" -b, -- write to the VM state rather than the virtual disk\n"

" -c, -- write compressed data with blk_write_compressed\n"

+" -C, -- report statistics in a machine parsable format\n"

" -f, -- use Force Unit Access semantics\n"

" -n, -- with -z, don't allow slow fallback\n"

" -p, -- ignored for backwards compatibility\n"

" -P, -- use different pattern to fill file\n"

+" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

" -s, -- use a pattern file to fill the write buffer\n"

-" -C, -- report statistics in a machine parsable format\n"

-" -q, -- quiet mode, do not show I/O statistics\n"

" -u, -- with -z, allow unmapping\n"

" -z, -- write zeroes using blk_pwrite_zeroes\n"

"\n");

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t write_cmd = {

.perm = BLK_PERM_WRITE,

.argmin = 2,

.argmax = -1,

- .args = "[-bcCfnquz] [-P pattern | -s source_file] off len",

+ .args = "[-bcCfnqruz] [-P pattern | -s source_file] off len",

.oneline = "writes a number of bytes at a specified offset",

.help = write_help,

};

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

int pattern = 0xcd;

const char *file_name = NULL;

- while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) {

+ while ((c = getopt(argc, argv, "bcCfnpP:qrs:uz")) != -1) {

switch (c) {

case 'b':

bflag = true;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

case 'q':

qflag = true;

break;

+ case 'r':

+ flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 's':

sflag = true;

file_name = optarg;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

}

}

- if (!zflag) {

+ if (zflag) {

+ if (flags & BDRV_REQ_REGISTERED_BUF) {

+ printf("cannot combine zero write with registered I/O buffer\n");

+ return -EINVAL;

+ }

+ } else {

if (sflag) {

- buf = qemu_io_alloc_from_file(blk, count, file_name);

+ buf = qemu_io_alloc_from_file(blk, count, file_name,

+ flags & BDRV_REQ_REGISTERED_BUF);

if (!buf) {

return -EINVAL;

}

} else {

- buf = qemu_io_alloc(blk, count, pattern);

+ buf = qemu_io_alloc(blk, count, pattern,

+ flags & BDRV_REQ_REGISTERED_BUF);

}

}

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

out:

if (!zflag) {

- qemu_io_free(buf);

+ qemu_io_free(blk, buf, count, flags & BDRV_REQ_REGISTERED_BUF);

}

return ret;

}

@@ -XXX,XX +XXX,XX @@ writev_help(void)

"\n"

" Writes into a segment of the currently open file, using a buffer\n"

" filled with a set pattern (0xcdcdcdcd).\n"

-" -P, -- use different pattern to fill file\n"

" -C, -- report statistics in a machine parsable format\n"

" -f, -- use Force Unit Access semantics\n"

+" -P, -- use different pattern to fill file\n"

" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

"\n");

}

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t writev_cmd = {

.perm = BLK_PERM_WRITE,

.argmin = 2,

.argmax = -1,

- .args = "[-Cfq] [-P pattern] off len [len..]",

+ .args = "[-Cfqr] [-P pattern] off len [len..]",

.oneline = "writes a number of bytes at a specified offset",

.help = writev_help,

};

@@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv)

int pattern = 0xcd;

QEMUIOVector qiov;

- while ((c = getopt(argc, argv, "CfqP:")) != -1) {

+ while ((c = getopt(argc, argv, "CfP:qr")) != -1) {

switch (c) {

case 'C':

Cflag = true;

@@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv)

case 'q':

qflag = true;

break;

+ case 'r':

+ flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 'P':

pattern = parse_pattern(optarg);

if (pattern < 0) {

@@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv)

optind++;

nr_iov = argc - optind;

- buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern);

+ buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern,

+ flags & BDRV_REQ_REGISTERED_BUF);

if (buf == NULL) {

return -EINVAL;

}

@@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv)

t2 = tsub(t2, t1);

print_report("wrote", &t2, offset, qiov.size, total, cnt, Cflag);

out:

+ qemu_io_free(blk, buf, qiov.size, flags & BDRV_REQ_REGISTERED_BUF);

qemu_iovec_destroy(&qiov);

- qemu_io_free(buf);

return ret;

}

@@ -XXX,XX +XXX,XX @@ struct aio_ctx {

bool zflag;

BlockAcctCookie acct;

int pattern;

+ BdrvRequestFlags flags;

struct timespec t1;

};

@@ -XXX,XX +XXX,XX @@ static void aio_write_done(void *opaque, int ret)

ctx->qiov.size, 1, ctx->Cflag);

out:

if (!ctx->zflag) {

- qemu_io_free(ctx->buf);

+ qemu_io_free(ctx->blk, ctx->buf, ctx->qiov.size,

+ ctx->flags & BDRV_REQ_REGISTERED_BUF);

qemu_iovec_destroy(&ctx->qiov);

}

g_free(ctx);

@@ -XXX,XX +XXX,XX @@ static void aio_read_done(void *opaque, int ret)

print_report("read", &t2, ctx->offset, ctx->qiov.size,

ctx->qiov.size, 1, ctx->Cflag);

out:

- qemu_io_free(ctx->buf);

+ qemu_io_free(ctx->blk, ctx->buf, ctx->qiov.size,

+ ctx->flags & BDRV_REQ_REGISTERED_BUF);

qemu_iovec_destroy(&ctx->qiov);

g_free(ctx);

}

@@ -XXX,XX +XXX,XX @@ static void aio_read_help(void)

" considered successful once the request is submitted, independently\n"

" of potential I/O errors or pattern mismatches.\n"

" -C, -- report statistics in a machine parsable format\n"

-" -P, -- use a pattern to verify read data\n"

" -i, -- treat request as invalid, for exercising stats\n"

-" -v, -- dump buffer to standard output\n"

+" -P, -- use a pattern to verify read data\n"

" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

+" -v, -- dump buffer to standard output\n"

"\n");

}

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t aio_read_cmd = {

.cfunc = aio_read_f,

.argmin = 2,

.argmax = -1,

- .args = "[-Ciqv] [-P pattern] off len [len..]",

+ .args = "[-Ciqrv] [-P pattern] off len [len..]",

.oneline = "asynchronously reads a number of bytes",

.help = aio_read_help,

};

@@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)

struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);

ctx->blk = blk;

- while ((c = getopt(argc, argv, "CP:iqv")) != -1) {

+ while ((c = getopt(argc, argv, "CiP:qrv")) != -1) {

switch (c) {

case 'C':

ctx->Cflag = true;

@@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)

case 'q':

ctx->qflag = true;

break;

+ case 'r':

+ ctx->flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 'v':

ctx->vflag = true;

break;

@@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)

optind++;

nr_iov = argc - optind;

- ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, 0xab);

+ ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, 0xab,

+ ctx->flags & BDRV_REQ_REGISTERED_BUF);

if (ctx->buf == NULL) {

block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);

g_free(ctx);

@@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)

clock_gettime(CLOCK_MONOTONIC, &ctx->t1);

block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,

BLOCK_ACCT_READ);

- blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx);

+ blk_aio_preadv(blk, ctx->offset, &ctx->qiov, ctx->flags, aio_read_done,

+ ctx);

return 0;

}

@@ -XXX,XX +XXX,XX @@ static void aio_write_help(void)

" Note that due to its asynchronous nature, this command will be\n"

" considered successful once the request is submitted, independently\n"

" of potential I/O errors or pattern mismatches.\n"

-" -P, -- use different pattern to fill file\n"

" -C, -- report statistics in a machine parsable format\n"

" -f, -- use Force Unit Access semantics\n"

" -i, -- treat request as invalid, for exercising stats\n"

+" -P, -- use different pattern to fill file\n"

" -q, -- quiet mode, do not show I/O statistics\n"

+" -r, -- register I/O buffer\n"

" -u, -- with -z, allow unmapping\n"

" -z, -- write zeroes using blk_aio_pwrite_zeroes\n"

"\n");

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t aio_write_cmd = {

.perm = BLK_PERM_WRITE,

.argmin = 2,

.argmax = -1,

- .args = "[-Cfiquz] [-P pattern] off len [len..]",

+ .args = "[-Cfiqruz] [-P pattern] off len [len..]",

.oneline = "asynchronously writes a number of bytes",

.help = aio_write_help,

};

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

int nr_iov, c;

int pattern = 0xcd;

struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);

- BdrvRequestFlags flags = 0;

ctx->blk = blk;

- while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) {

+ while ((c = getopt(argc, argv, "CfiP:qruz")) != -1) {

switch (c) {

case 'C':

ctx->Cflag = true;

break;

case 'f':

- flags |= BDRV_REQ_FUA;

+ ctx->flags |= BDRV_REQ_FUA;

break;

case 'q':

ctx->qflag = true;

break;

+ case 'r':

+ ctx->flags |= BDRV_REQ_REGISTERED_BUF;

+ break;

case 'u':

- flags |= BDRV_REQ_MAY_UNMAP;

+ ctx->flags |= BDRV_REQ_MAY_UNMAP;

break;

case 'P':

pattern = parse_pattern(optarg);

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

return -EINVAL;

}

- if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {

+ if ((ctx->flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {

printf("-u requires -z to be specified\n");

g_free(ctx);

return -EINVAL;

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

return -EINVAL;

}

+ if (ctx->zflag && (ctx->flags & BDRV_REQ_REGISTERED_BUF)) {

+ printf("cannot combine zero write with registered I/O buffer\n");

+ g_free(ctx);

+ return -EINVAL;

+ }

+

ctx->offset = cvtnum(argv[optind]);

if (ctx->offset < 0) {

int ret = ctx->offset;

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

}

ctx->qiov.size = count;

- blk_aio_pwrite_zeroes(blk, ctx->offset, count, flags, aio_write_done,

- ctx);

+ blk_aio_pwrite_zeroes(blk, ctx->offset, count, ctx->flags,

+ aio_write_done, ctx);

} else {

nr_iov = argc - optind;

ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov,

- pattern);

+ pattern, ctx->flags & BDRV_REQ_REGISTERED_BUF);

if (ctx->buf == NULL) {

block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);

g_free(ctx);

@@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,

BLOCK_ACCT_WRITE);

- blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, flags, aio_write_done,

- ctx);

+ blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, ctx->flags,

+ aio_write_done, ctx);

}

return 0;

--

2.39.1

This regression test demonstrates that detect-zeroes works with

registered buffers. Bug details:

https://gitlab.com/qemu-project/qemu/-/issues/1404

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-Id: <20230207203719.242926-5-stefanha@redhat.com>

---

.../tests/detect-zeroes-registered-buf | 58 +++++++++++++++++++

.../tests/detect-zeroes-registered-buf.out | 7 +++

2 files changed, 65 insertions(+)

create mode 100755 tests/qemu-iotests/tests/detect-zeroes-registered-buf

create mode 100644 tests/qemu-iotests/tests/detect-zeroes-registered-buf.out

diff --git a/tests/qemu-iotests/tests/detect-zeroes-registered-buf b/tests/qemu-iotests/tests/detect-zeroes-registered-buf

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf

@@ -XXX,XX +XXX,XX @@

+#!/usr/bin/env bash

+# group: rw auto quick

+#

+# Check that detect-zeroes=unmap works on writes with registered I/O buffers.

+# This is a regression test for

+# https://gitlab.com/qemu-project/qemu/-/issues/1404 where I/O requests failed

+# unexpectedly.

+#

+# Copyright Red Hat

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+# creator

+owner=stefanha@redhat.com

+

+seq=`basename $0`

+echo "QA output created by $seq"

+

+status=1    # failure is the default!

+

+_cleanup()

+{

+    _cleanup_test_img

+}

+trap "_cleanup; exit \$status" 0 1 2 3 15

+

+# get standard environment, filters and checks

+cd ..

+. ./common.rc

+. ./common.filter

+

+_supported_fmt qcow2

+_supported_proto generic

+

+size=128M

+_make_test_img $size

+IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,discard=unmap,detect-zeroes=unmap"

+

+echo

+echo "== writing zero buffer to image =="

+QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS_NO_FMT" $QEMU_IO -c "write -r -P 0 0 4k" --image-opts "$IMGSPEC" | _filter_qemu_io

+

+# success, all done

+echo "*** done"

+rm -f $seq.full

+status=0

diff --git a/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out b/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out

@@ -XXX,XX +XXX,XX @@

+QA output created by detect-zeroes-registered-buf

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728

+

+== writing zero buffer to image ==

+wrote 4096/4096 bytes at offset 0

+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

+*** done

--

2.39.1