* Re: [PATCH] chardev: enable distinct input for -chardev file
2020-05-01 17:25 [PATCH] chardev: enable distinct input for -chardev file Alexander Bulekov
@ 2020-05-01 17:34 ` Marc-André Lureau
2020-05-01 17:46 ` no-reply
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2020-05-01 17:34 UTC (permalink / raw)
To: Alexander Bulekov; +Cc: Paolo Bonzini, QEMU, Stefan Hajnoczi
Hi
On Fri, May 1, 2020 at 7:26 PM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> char-file already supports distinct paths for input/output but it was
> only possible to specify a distinct input through QMP. With this change,
> we can also specify a distinct input with the -chardev file argument:
> qemu -chardev file,id=char1,path=/out/file,in=/in/file
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Looks ok. Please consider writing a test.
> ---
> chardev/char-file.c | 5 +++++
> chardev/char.c | 3 +++
> qemu-options.hx | 7 +++++--
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> The naming here is awkward, with path= really turning into "out" when
> in= is specified, though i'm not sure about what is a good solution.
>
> diff --git a/chardev/char-file.c b/chardev/char-file.c
> index 2fd80707e5..cc742cc234 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 *in = qemu_opt_get(opts, "in");
> ChardevFile *file;
>
> backend->type = CHARDEV_BACKEND_KIND_FILE;
> @@ -110,6 +111,10 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
> file = backend->u.file.data = g_new0(ChardevFile, 1);
> qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
> file->out = g_strdup(path);
> + if (in) {
> + file->has_in = true;
> + file->in = g_strdup(qemu_opt_get(opts, "in"));
> + }
>
> file->has_append = true;
> file->append = qemu_opt_get_bool(opts, "append", false);
> diff --git a/chardev/char.c b/chardev/char.c
> index e77564060d..797574f205 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -849,6 +849,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "path",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "in",
> + .type = QEMU_OPT_STRING,
> },{
> .name = "host",
> .type = QEMU_OPT_STRING,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 292d4e7c0c..bbb091872f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2938,7 +2938,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[,in=PATH][,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"
> @@ -3137,13 +3137,16 @@ 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,in=in``
> 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.
>
> + ``in`` specifies a separate file as the input to the chardev. If
> + ``in`` is omitted, ``path`` is used for both input and output
> +
> ``-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.26.2
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chardev: enable distinct input for -chardev file
2020-05-01 17:25 [PATCH] chardev: enable distinct input for -chardev file Alexander Bulekov
2020-05-01 17:34 ` Marc-André Lureau
@ 2020-05-01 17:46 ` no-reply
2020-05-04 9:34 ` Daniel P. Berrangé
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2020-05-01 17:46 UTC (permalink / raw)
To: alxndr; +Cc: alxndr, marcandre.lureau, qemu-devel, stefanha, pbonzini
Patchew URL: https://patchew.org/QEMU/20200501172559.6470-1-alxndr@bu.edu/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
qemu-system-aarch64: falling back to tcg
socket_accept failed: Resource temporarily unavailable
**
ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
/tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
TEST iotest-qcow2: 060
qemu-system-aarch64: -accel kvm: invalid accelerator kvm
---
TEST iotest-qcow2: 158
socket_accept failed: Resource temporarily unavailable
**
ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
/tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/libqtest.c:301:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0)
make: *** [check-qtest-aarch64] Error 1
TEST iotest-qcow2: 159
TEST iotest-qcow2: 161
TEST iotest-qcow2: 170
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=89b646b710164554a4f95c0874a96e73', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-45s6eccb/src/docker-src.2020-05-01-13.31.49.28895:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=89b646b710164554a4f95c0874a96e73
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-45s6eccb/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 14m18.443s
user 0m9.135s
The full log is available at
http://patchew.org/logs/20200501172559.6470-1-alxndr@bu.edu/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chardev: enable distinct input for -chardev file
2020-05-01 17:25 [PATCH] chardev: enable distinct input for -chardev file Alexander Bulekov
2020-05-01 17:34 ` Marc-André Lureau
2020-05-01 17:46 ` no-reply
@ 2020-05-04 9:34 ` Daniel P. Berrangé
2020-05-04 13:40 ` Stefan Hajnoczi
2020-05-04 14:00 ` Alexander Bulekov
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-05-04 9:34 UTC (permalink / raw)
To: Alexander Bulekov
Cc: Paolo Bonzini, qemu-devel, stefanha, Marc-André Lureau
On Fri, May 01, 2020 at 01:25:59PM -0400, Alexander Bulekov wrote:
> char-file already supports distinct paths for input/output but it was
> only possible to specify a distinct input through QMP. With this change,
> we can also specify a distinct input with the -chardev file argument:
> qemu -chardev file,id=char1,path=/out/file,in=/in/file
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> chardev/char-file.c | 5 +++++
> chardev/char.c | 3 +++
> qemu-options.hx | 7 +++++--
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> The naming here is awkward, with path= really turning into "out" when
> in= is specified, though i'm not sure about what is a good solution.
>
> diff --git a/chardev/char-file.c b/chardev/char-file.c
> index 2fd80707e5..cc742cc234 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 *in = qemu_opt_get(opts, "in");
> ChardevFile *file;
>
> backend->type = CHARDEV_BACKEND_KIND_FILE;
> @@ -110,6 +111,10 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
> file = backend->u.file.data = g_new0(ChardevFile, 1);
> qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
> file->out = g_strdup(path);
> + if (in) {
> + file->has_in = true;
> + file->in = g_strdup(qemu_opt_get(opts, "in"));
> + }
>
> file->has_append = true;
> file->append = qemu_opt_get_bool(opts, "append", false);
> diff --git a/chardev/char.c b/chardev/char.c
> index e77564060d..797574f205 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -849,6 +849,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "path",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "in",
> + .type = QEMU_OPT_STRING,
> },{
> .name = "host",
> .type = QEMU_OPT_STRING,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 292d4e7c0c..bbb091872f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2938,7 +2938,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[,in=PATH][,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"
> @@ -3137,13 +3137,16 @@ 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,in=in``
> 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.
>
> + ``in`` specifies a separate file as the input to the chardev. If
> + ``in`` is omitted, ``path`` is used for both input and output
I'd suggest "pathin" rather than just "in"
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chardev: enable distinct input for -chardev file
2020-05-01 17:25 [PATCH] chardev: enable distinct input for -chardev file Alexander Bulekov
` (2 preceding siblings ...)
2020-05-04 9:34 ` Daniel P. Berrangé
@ 2020-05-04 13:40 ` Stefan Hajnoczi
2020-05-04 14:00 ` Alexander Bulekov
4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-05-04 13:40 UTC (permalink / raw)
To: Alexander Bulekov
Cc: Paolo Bonzini, qemu-devel, stefanha, Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 774 bytes --]
On Fri, May 01, 2020 at 01:25:59PM -0400, Alexander Bulekov wrote:
> char-file already supports distinct paths for input/output but it was
> only possible to specify a distinct input through QMP. With this change,
> we can also specify a distinct input with the -chardev file argument:
> qemu -chardev file,id=char1,path=/out/file,in=/in/file
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> chardev/char-file.c | 5 +++++
> chardev/char.c | 3 +++
> qemu-options.hx | 7 +++++--
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> The naming here is awkward, with path= really turning into "out" when
> in= is specified, though i'm not sure about what is a good solution.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chardev: enable distinct input for -chardev file
2020-05-01 17:25 [PATCH] chardev: enable distinct input for -chardev file Alexander Bulekov
` (3 preceding siblings ...)
2020-05-04 13:40 ` Stefan Hajnoczi
@ 2020-05-04 14:00 ` Alexander Bulekov
4 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2020-05-04 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, stefanha, Paolo Bonzini
On 200501 1325, Alexander Bulekov wrote:
...
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 292d4e7c0c..bbb091872f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2938,7 +2938,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[,in=PATH][,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"
> @@ -3137,13 +3137,16 @@ 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,in=in``
Just noticed that this should be
-chardev file,id=id,path=path,[in=in]
I'll fix it in v2.
-Alex
> 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.
>
> + ``in`` specifies a separate file as the input to the chardev. If
> + ``in`` is omitted, ``path`` is used for both input and output
> +
> ``-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.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread