| Message ID | 20200501172559.6470-1-alxndr@bu.edu |
|---|---|
| State | New |
| Headers | show |
| Series | chardev: enable distinct input for -chardev file | expand |
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 > >
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
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
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>
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 >
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:
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.