| Message ID | 1357566928-25361-7-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/07/2013 06:55 AM, Gerd Hoffmann wrote: > Add support for file chardevs. Output file is mandatory, > input file is optional. Both file names and file descriptor > passing is supported. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > qapi-schema.json | 9 +++++- > qemu-char.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 84 insertions(+), 2 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index e3f0d44..8904d36 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3030,9 +3030,16 @@ > # > # Since: 1.4 > ## > +{ 'union': 'ChardevFileSource', 'data': { 'path' : 'str', > + 'fd' : 'str' } } > + > +{ 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource', > + 'out' : 'ChardevFileSource' } } > + > { 'type': 'ChardevDummy', 'data': { } } > > -{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } } > +{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', > + 'null' : 'ChardevDummy' } } > > { 'command': 'chardev-add', 'data': {'id' : 'str', > 'backend' : 'ChardevBackend' } } An example in qmp-commands.hx would be helpful; am I correct that an example would be: -> { "execute" : "chardev-add", "arguments" : { "id" : "foo", "backend" : { "type" : "file", "data" : { "in" : { "type" : "fd", "data" : "namedfd" }, "out" : { "type" : "path", "data" : "/path/to/file" } } } } } <- { "return": {} } where namedfd was previously given via 'getfd'? Do we need the complexity of supporting fd passing explicitly? 'getfd' is less than ideal compared to 'add-fd', and for 'add-fd', we would pass via "path":"/dev/fdset/nnn". That is, why do we need to bend over backwards to support an alternate syntax for fd passing in a new command, when we can already use existing commands to get fd passing for free?
On 01/09/13 19:12, Eric Blake wrote: > On 01/07/2013 06:55 AM, Gerd Hoffmann wrote: >> Add support for file chardevs. Output file is mandatory, input >> file is optional. Both file names and file descriptor passing is >> supported. >> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- >> qapi-schema.json | 9 +++++- qemu-char.c | 77 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files >> changed, 84 insertions(+), 2 deletions(-) >> >> diff --git a/qapi-schema.json b/qapi-schema.json index >> e3f0d44..8904d36 100644 --- a/qapi-schema.json +++ >> b/qapi-schema.json @@ -3030,9 +3030,16 @@ # # Since: 1.4 ## +{ >> 'union': 'ChardevFileSource', 'data': { 'path' : 'str', + >> 'fd' : 'str' } } + +{ 'type': 'ChardevFile', 'data': { '*in' : >> 'ChardevFileSource', + 'out' : >> 'ChardevFileSource' } } + { 'type': 'ChardevDummy', 'data': { } >> } >> >> -{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } >> } +{ 'union': 'ChardevBackend', 'data': { 'file' : >> 'ChardevFile', + 'null' : >> 'ChardevDummy' } } >> >> { 'command': 'chardev-add', 'data': {'id' : 'str', 'backend' >> : 'ChardevBackend' } } > > An example in qmp-commands.hx would be helpful; am I correct that > an example would be: > > -> { "execute" : "chardev-add", "arguments" : { "id" : "foo", > "backend" : { "type" : "file", "data" : { "in" : { "type" : "fd", > "data" : "namedfd" }, "out" : { "type" : "path", "data" : > "/path/to/file" } } } } } <- { "return": {} } > > where namedfd was previously given via 'getfd'? Yes. > Do we need the complexity of supporting fd passing explicitly? > 'getfd' is less than ideal compared to 'add-fd', and for 'add-fd', > we would pass via "path":"/dev/fdset/nnn". That is, why do we need > to bend over backwards to support an alternate syntax for fd > passing in a new command, when we can already use existing commands > to get fd passing for free? Oh, didn't know that. Was just following what SocketAddress does and what Paolo suggested. It isn't needed indeed. cheers, Gerd
Il 10/01/2013 10:10, Gerd Hoffmann ha scritto: > >> > Do we need the complexity of supporting fd passing explicitly? >> > 'getfd' is less than ideal compared to 'add-fd', and for 'add-fd', >> > we would pass via "path":"/dev/fdset/nnn". That is, why do we need >> > to bend over backwards to support an alternate syntax for fd >> > passing in a new command, when we can already use existing commands >> > to get fd passing for free? > Oh, didn't know that. Was just following what SocketAddress does and > what Paolo suggested. It isn't needed indeed. Yeah, I hadn't followed fdset either. We really need more documentation. Paolo
diff --git a/qapi-schema.json b/qapi-schema.json index e3f0d44..8904d36 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3030,9 +3030,16 @@ # # Since: 1.4 ## +{ 'union': 'ChardevFileSource', 'data': { 'path' : 'str', + 'fd' : 'str' } } + +{ 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource', + 'out' : 'ChardevFileSource' } } + { 'type': 'ChardevDummy', 'data': { } } -{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } } +{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', + 'null' : 'ChardevDummy' } } { 'command': 'chardev-add', 'data': {'id' : 'str', 'backend' : 'ChardevBackend' } } diff --git a/qemu-char.c b/qemu-char.c index c9ae8f3..91b0a57 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2938,11 +2938,86 @@ CharDriverState *qemu_char_get_next_serial(void) return serial_hds[next_serial++]; } +#ifdef _WIN32 + +static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp) +{ + HANDLE out; + + if (file->in) { + error_setg(errp, "input file not supported"); + return NULL; + } + if (file->out->kind != CHARDEV_FILE_SOURCE_KIND_PATH) { + error_setg(errp, "only file paths supported"); + return NULL; + } + + out = CreateFile(file->out->path, GENERIC_WRITE, FILE_SHARE_READ, NULL, + OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + if (out == INVALID_HANDLE_VALUE) { + error_setg(errp, "open %s failed", file->out->path); + return NULL; + } + return qemu_chr_open_win_file(out); +} + +#else /* WIN32 */ + +static int qmp_chardev_open_file_source(ChardevFileSource *src, int flags, + Error **errp) +{ + int fd = -1; + + switch (src->kind) { + case CHARDEV_FILE_SOURCE_KIND_PATH: + TFR(fd = qemu_open(src->path, flags, 0666)); + if (fd == -1) { + error_setg(errp, "open %s: %s", src->path, strerror(errno)); + } + break; + case CHARDEV_FILE_SOURCE_KIND_FD: + fd = monitor_get_fd(cur_mon, src->fd, errp); + break; + default: + error_setg(errp, "unknown chardev file source (%d)", src->kind); + break; + } + return fd; +} + +static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp) +{ + int flags, in = -1, out = -1; + + flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY; + out = qmp_chardev_open_file_source(file->out, flags, errp); + if (error_is_set(errp)) { + return NULL; + } + + if (file->in) { + flags = O_RDONLY; + in = qmp_chardev_open_file_source(file->in, flags, errp); + if (error_is_set(errp)) { + qemu_close(out); + return NULL; + } + } + + return qemu_chr_open_fd(in, out); +} + +#endif /* WIN32 */ + void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp) { CharDriverState *chr; switch (backend->kind) { + case CHARDEV_BACKEND_KIND_FILE: + chr = qmp_chardev_open_file(backend->file, errp); + break; case CHARDEV_BACKEND_KIND_NULL: chr = qemu_chr_open_null(NULL); break; @@ -2951,7 +3026,7 @@ void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp) return; } - if (chr == NULL) { + if (chr == NULL && !error_is_set(errp)) { error_setg(errp, "Failed to create chardev"); } if (chr) {
Add support for file chardevs. Output file is mandatory, input file is optional. Both file names and file descriptor passing is supported. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- qapi-schema.json | 9 +++++- qemu-char.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-)