Re: [PATCH 2/2] chardev: Allow setting file chardev input file on the command line

[Philippe Mathieu-Daudé]

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.