Our 'file' chardev backend supports both "output from this chardev
is written to a file" and "input from this chardev should be read
from a file" (except on Windows). However, you can only set up
the input file if you're using the QMP interface -- there is no
command line syntax to do it.

Add command line syntax to allow specifying an input file
as well as an output file, using a new 'input-path' suboption.

The specific use case I have is that I'd like to be able to
feed fuzzer reproducer input into qtest without having to use
'-qtest stdio' and put the input onto stdin. Being able to
use a file chardev like this:
 -chardev file,id=repro,path=/dev/null,input-path=repro.txt -qtest chardev:repro
means that stdio is free for use by gdb.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The "not on Windows" ifdeffery is because qmp_chardev_open_file()
does something similar; it seems likely to produce a nicer
error message to catch it at parse time rather than open time.
---
 chardev/char-file.c |  8 ++++++++
 chardev/char.c      |  3 +++
 qemu-options.hx     | 10 ++++++++--
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/chardev/char-file.c b/chardev/char-file.c
index 3a7b9caf6f0..263e6da5636 100644
--- a/chardev/char-file.c
+++ b/chardev/char-file.c
@@ -100,6 +100,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
                                     Error **errp)
 {
     const char *path = qemu_opt_get(opts, "path");
+    const char *inpath = qemu_opt_get(opts, "input-path");
     ChardevFile *file;
 
     backend->type = CHARDEV_BACKEND_KIND_FILE;
@@ -107,9 +108,16 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
         error_setg(errp, "chardev: file: no filename given");
         return;
     }
+#ifdef _WIN32
+    if (inpath) {
+        error_setg(errp, "chardev: file: input-path not supported on Windows");
+        return;
+    }
+#endif
     file = backend->u.file.data = g_new0(ChardevFile, 1);
     qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
     file->out = g_strdup(path);
+    file->in = g_strdup(inpath);
 
     file->has_append = true;
     file->append = qemu_opt_get_bool(opts, "append", false);
diff --git a/chardev/char.c b/chardev/char.c
index e69390601fc..661ad8176a9 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -805,6 +805,9 @@ QemuOptsList qemu_chardev_opts = {
         },{
             .name = "path",
             .type = QEMU_OPT_STRING,
+        },{
+            .name = "input-path",
+            .type = QEMU_OPT_STRING,
         },{
             .name = "host",
             .type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c5..31d08c60264 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3360,7 +3360,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
     "         [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
     "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
-    "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+    "-chardev file,id=id,path=path[,input-file=input-file][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
     "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
 #ifdef _WIN32
     "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
@@ -3563,13 +3563,19 @@ The available backends are:
     Create a ring buffer with fixed size ``size``. size must be a power
     of two and defaults to ``64K``.
 
-``-chardev file,id=id,path=path``
+``-chardev file,id=id,path=path[,input-path=input-path]``
     Log all traffic received from the guest to a file.
 
     ``path`` specifies the path of the file to be opened. This file will
     be created if it does not already exist, and overwritten if it does.
     ``path`` is required.
 
+    If ``input-path`` is specified, this is the path of a second file
+    which will be used for input. If ``input-path`` is not specified,
+    no input will be available from the chardev.
+
+    Note that ``input-path`` is not supported on Windows hosts.
+
 ``-chardev pipe,id=id,path=path``
     Create a two-way connection to the guest. The behaviour differs
     slightly between Windows hosts and other hosts:
-- 
2.34.1

On Thu, Apr 13, 2023 at 7:07 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> Our 'file' chardev backend supports both "output from this chardev
> is written to a file" and "input from this chardev should be read
> from a file" (except on Windows). However, you can only set up
> the input file if you're using the QMP interface -- there is no
> command line syntax to do it.
>
> Add command line syntax to allow specifying an input file
> as well as an output file, using a new 'input-path' suboption.
>
> The specific use case I have is that I'd like to be able to
> feed fuzzer reproducer input into qtest without having to use
> '-qtest stdio' and put the input onto stdin. Being able to
> use a file chardev like this:
>  -chardev file,id=repro,path=/dev/null,input-path=repro.txt -qtest
> chardev:repro
> means that stdio is free for use by gdb.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



> ---
> The "not on Windows" ifdeffery is because qmp_chardev_open_file()
> does something similar; it seems likely to produce a nicer
> error message to catch it at parse time rather than open time.
>

(I wonder if we could actually reduce the win32-specific code, I'll have a
look eventually)

---
>  chardev/char-file.c |  8 ++++++++
>  chardev/char.c      |  3 +++
>  qemu-options.hx     | 10 ++++++++--
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-file.c b/chardev/char-file.c
> index 3a7b9caf6f0..263e6da5636 100644
> --- a/chardev/char-file.c
> +++ b/chardev/char-file.c
> @@ -100,6 +100,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts,
> ChardevBackend *backend,
>                                      Error **errp)
>  {
>      const char *path = qemu_opt_get(opts, "path");
> +    const char *inpath = qemu_opt_get(opts, "input-path");
>      ChardevFile *file;
>
>      backend->type = CHARDEV_BACKEND_KIND_FILE;
> @@ -107,9 +108,16 @@ static void qemu_chr_parse_file_out(QemuOpts *opts,
> ChardevBackend *backend,
>          error_setg(errp, "chardev: file: no filename given");
>          return;
>      }
> +#ifdef _WIN32
> +    if (inpath) {
> +        error_setg(errp, "chardev: file: input-path not supported on
> Windows");
> +        return;
> +    }
> +#endif
>      file = backend->u.file.data = g_new0(ChardevFile, 1);
>      qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
>      file->out = g_strdup(path);
> +    file->in = g_strdup(inpath);
>
>      file->has_append = true;
>      file->append = qemu_opt_get_bool(opts, "append", false);
> diff --git a/chardev/char.c b/chardev/char.c
> index e69390601fc..661ad8176a9 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -805,6 +805,9 @@ QemuOptsList qemu_chardev_opts = {
>          },{
>              .name = "path",
>              .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "input-path",
> +            .type = QEMU_OPT_STRING,
>          },{
>              .name = "host",
>              .type = QEMU_OPT_STRING,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c5..31d08c60264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3360,7 +3360,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>      "-chardev
> vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
>      "         [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>      "-chardev
> ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
> -    "-chardev
> file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> +    "-chardev
> file,id=id,path=path[,input-file=input-file][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>      "-chardev
> pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>  #ifdef _WIN32
>      "-chardev
> console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> @@ -3563,13 +3563,19 @@ The available backends are:
>      Create a ring buffer with fixed size ``size``. size must be a power
>      of two and defaults to ``64K``.
>
> -``-chardev file,id=id,path=path``
> +``-chardev file,id=id,path=path[,input-path=input-path]``
>      Log all traffic received from the guest to a file.
>
>      ``path`` specifies the path of the file to be opened. This file will
>      be created if it does not already exist, and overwritten if it does.
>      ``path`` is required.
>
> +    If ``input-path`` is specified, this is the path of a second file
> +    which will be used for input. If ``input-path`` is not specified,
> +    no input will be available from the chardev.
> +
> +    Note that ``input-path`` is not supported on Windows hosts.
> +
>  ``-chardev pipe,id=id,path=path``
>      Create a two-way connection to the guest. The behaviour differs
>      slightly between Windows hosts and other hosts:
> --
> 2.34.1
>
>

On 13/04/2023 17.07, Peter Maydell wrote:
> Our 'file' chardev backend supports both "output from this chardev
> is written to a file" and "input from this chardev should be read
> from a file" (except on Windows). However, you can only set up
> the input file if you're using the QMP interface -- there is no
> command line syntax to do it.
> 
> Add command line syntax to allow specifying an input file
> as well as an output file, using a new 'input-path' suboption.
> 
> The specific use case I have is that I'd like to be able to
> feed fuzzer reproducer input into qtest without having to use
> '-qtest stdio' and put the input onto stdin. Being able to
> use a file chardev like this:
>   -chardev file,id=repro,path=/dev/null,input-path=repro.txt -qtest chardev:repro
> means that stdio is free for use by gdb.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
...
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c5..31d08c60264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3360,7 +3360,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>       "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
>       "         [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>       "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
> -    "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> +    "-chardev file,id=id,path=path[,input-file=input-file][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"

s/input-file=/input-path=/

  Thomas

On 13/4/23 17:07, Peter Maydell wrote:
> Our 'file' chardev backend supports both "output from this chardev
> is written to a file" and "input from this chardev should be read
> from a file" (except on Windows). However, you can only set up
> the input file if you're using the QMP interface -- there is no
> command line syntax to do it.
> 
> Add command line syntax to allow specifying an input file
> as well as an output file, using a new 'input-path' suboption.
> 
> The specific use case I have is that I'd like to be able to
> feed fuzzer reproducer input into qtest without having to use
> '-qtest stdio' and put the input onto stdin. Being able to
> use a file chardev like this:
>   -chardev file,id=repro,path=/dev/null,input-path=repro.txt -qtest chardev:repro
> means that stdio is free for use by gdb.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The "not on Windows" ifdeffery is because qmp_chardev_open_file()
> does something similar; it seems likely to produce a nicer
> error message to catch it at parse time rather than open time.
> ---
>   chardev/char-file.c |  8 ++++++++
>   chardev/char.c      |  3 +++
>   qemu-options.hx     | 10 ++++++++--
>   3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/chardev/char-file.c b/chardev/char-file.c
> index 3a7b9caf6f0..263e6da5636 100644
> --- a/chardev/char-file.c
> +++ b/chardev/char-file.c
> @@ -100,6 +100,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
>                                       Error **errp)
>   {
>       const char *path = qemu_opt_get(opts, "path");
> +    const char *inpath = qemu_opt_get(opts, "input-path");


>       file->out = g_strdup(path);
> +    file->in = g_strdup(inpath);


> diff --git a/chardev/char.c b/chardev/char.c
> index e69390601fc..661ad8176a9 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -805,6 +805,9 @@ QemuOptsList qemu_chardev_opts = {
>           },{
>               .name = "path",
>               .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "input-path",
> +            .type = QEMU_OPT_STRING,
>           },{


> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c5..31d08c60264 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx


> -``-chardev file,id=id,path=path``
> +``-chardev file,id=id,path=path[,input-path=input-path]``
>       Log all traffic received from the guest to a file.
>   
>       ``path`` specifies the path of the file to be opened. This file will
>       be created if it does not already exist, and overwritten if it does.
>       ``path`` is required.

I find "path" vs. "input-path" confusing and would rather rename it as
"output-path" for consistency; or at least add an alias.
Possibly deprecating the "path" alias. Maybe matter of taste...

Can be done as follow-up, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    If ``input-path`` is specified, this is the path of a second file
> +    which will be used for input. If ``input-path`` is not specified,
> +    no input will be available from the chardev.
> +
> +    Note that ``input-path`` is not supported on Windows hosts.

On Fri, 14 Apr 2023 at 15:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 13/4/23 17:07, Peter Maydell wrote:
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
>
>
> > -``-chardev file,id=id,path=path``
> > +``-chardev file,id=id,path=path[,input-path=input-path]``
> >       Log all traffic received from the guest to a file.
> >
> >       ``path`` specifies the path of the file to be opened. This file will
> >       be created if it does not already exist, and overwritten if it does.
> >       ``path`` is required.
>
> I find "path" vs. "input-path" confusing and would rather rename it as
> "output-path" for consistency; or at least add an alias.
> Possibly deprecating the "path" alias. Maybe matter of taste...

The much more common use is the preexisting one of "write the
output to the file". I don't particularly want to break all
the uses of that just because we added this option.

thanks
-- PMM