On Thu, Jan 26, 2023 at 03:14:00PM -0500, Stefan Hajnoczi wrote:
> 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.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> qemu-io-cmds.c | 149 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 103 insertions(+), 46 deletions(-)
>
> 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;
> FILE *f = fopen(file_name, "r");
> @@ -414,6 +423,10 @@ static void *qemu_io_alloc_from_file(BlockBackend *blk,
> size_t len,
> fclose(f);
> f = NULL;
>
> + if (register_buf) {
> + blk_register_buf(blk, buf_origin, len, &error_abort);
> + }
Why is this conditionally true...
> +
> if (len > pattern_len) {
> len -= pattern_len;
> buf += pattern_len;
> @@ -431,7 +444,7 @@ static void *qemu_io_alloc_from_file(BlockBackend *blk,
> size_t len,
> return buf_origin;
>
> error:
> - qemu_io_free(buf_origin);
> + qemu_io_free(blk, buf_origin, len, false);
...but cleanup unconditionally false?
> @@ -719,6 +732,7 @@ 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"
Here we have kept more or less alphabetical...
> @@ -909,6 +932,7 @@ static void readv_help(void)
> " -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"
...here we have not. While it's pre-existing, would it be worth
touching up while you're here?
> @@ -1455,6 +1503,7 @@ static void aio_read_help(void)
> " -i, -- treat request as invalid, for exercising stats\n"
> " -v, -- dump buffer to standard output\n"
> " -q, -- quiet mode, do not show I/O statistics\n"
> +" -r, -- register I/O buffer\n"
Another pre-existing spot.
Overall makes sense to me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
