[06/11] chardev: add file chardev support to chardev-add (qmp)

Submitted by Gerd Hoffmann on Jan. 7, 2013, 1:55 p.m.

Details

Message ID 1357566928-25361-7-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Jan. 7, 2013, 1:55 p.m.
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(-)

Comments

Eric Blake Jan. 9, 2013, 6:12 p.m.
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?
Gerd Hoffmann Jan. 10, 2013, 9:10 a.m.
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
Paolo Bonzini Jan. 10, 2013, 10:35 a.m.
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

Patch hide | download patch | download mbox

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) {