diff mbox

[v2,06/10] chardev: add file chardev support to chardev-add (qmp)

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

Commit Message

Gerd Hoffmann Jan. 10, 2013, 2:23 p.m. UTC 
Add support for file chardevs.  Output file is mandatory,
input file is optional.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |   13 ++++++++++-
 qemu-char.c      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |    8 ++++++-
 3 files changed, 80 insertions(+), 2 deletions(-)

Comments

Eric Blake Jan. 10, 2013, 7:33 p.m. UTC | #1 
On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Add support for file chardevs.  Output file is mandatory,
> input file is optional.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |   13 ++++++++++-
>  qemu-char.c      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |    8 ++++++-
>  3 files changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 53d4b9e..7930139 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3019,6 +3019,16 @@
>  { 'command': 'nbd-server-stop' }
>  
>  ##
> +# @ChardevFile:

Should you mention '@in: #optional' and '@out:' in any further detail?

> +#
> +# Configuration info for file chardevs.
> +#
> +# Since: 1.4
> +##
> +{ 'type': 'ChardevFile', 'data': { '*in' : 'str',
> +                                   'out' : 'str' } }

Hmm; here you document ChardevFile as a separate type, but you didn't
document ChardevDummy in patch 4/10.


> +#ifdef _WIN32

> +
> +#else /* WIN32 */

Wouldn't this be /* !_WIN32 */?
Gerd Hoffmann Jan. 11, 2013, 10:37 a.m. UTC | #2 
>> ## +# @ChardevFile:
> 
> Should you mention '@in: #optional' and '@out:' in any further
> detail?

Done.

> Hmm; here you document ChardevFile as a separate type, but you
> didn't document ChardevDummy in patch 4/10.

As the name says it is just a dummy passed when the backend in
question doesn't need any parameters.  Therefore it is empty.  Does it
really need its own documentation stanza?

cheers,
  Gerd
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 53d4b9e..7930139 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3019,6 +3019,16 @@ 
 { 'command': 'nbd-server-stop' }
 
 ##
+# @ChardevFile:
+#
+# Configuration info for file chardevs.
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevFile', 'data': { '*in' : 'str',
+                                   'out' : 'str' } }
+
+##
 # @ChardevBackend:
 #
 # Configuration info for the new chardev backend.
@@ -3027,7 +3037,8 @@ 
 ##
 { 'type': 'ChardevDummy', 'data': { } }
 
-{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+                                       'null' : 'ChardevDummy' } }
 
 ##
 # @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index 62d35b6..fa0a884 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2938,6 +2938,64 @@  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;
+    }
+
+    out = CreateFile(file->out, 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);
+        return NULL;
+    }
+    return qemu_chr_open_win_file(out);
+}
+
+#else /* WIN32 */
+
+static int qmp_chardev_open_file_source(char *src, int flags,
+                                        Error **errp)
+{
+    int fd = -1;
+
+    TFR(fd = qemu_open(src, flags, 0666));
+    if (fd == -1) {
+        error_setg(errp, "open %s: %s", src, strerror(errno));
+    }
+    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 */
+
 ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
                                Error **errp)
 {
@@ -2945,6 +3003,9 @@  ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     CharDriverState *chr = NULL;
 
     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;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c9ab37c..4d382f4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2672,13 +2672,19 @@  Arguments:
 - "id": the chardev's ID, must be unique (json-string)
 - "backend": chardev backend type + parameters
 
-Example:
+Examples:
 
 -> { "execute" : "chardev-add",
      "arguments" : { "id" : "foo",
                      "backend" : { "type" : "null", "data" : {} } } }
 <- { "return": {} }
 
+-> { "execute" : "chardev-add",
+     "arguments" : { "id" : "bar",
+                     "backend" : { "type" : "file",
+                                   "data" : { "out" : "/tmp/bar.log" } } } }
+<- { "return": {} }
+
 EQMP
 
     {