[Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Gerd Hoffmann 
This patch adds chardev_add_file, chardev_add_tty and chardev_remove
monitor commands.

chardev_add_file and chardev_add_tty expect an id and a path, they
create a file/tty chardev.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hmp-commands.hx  |   44 +++
 hmp.c|   29 
 hmp.h|3 ++
 qapi-schema.json |   43 ++
 qemu-char.c  |   41 +
 qemu-char.h  |2 +
 qmp-commands.hx  |   76 ++
 7 files changed, 238 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..82a855a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1485,6 +1485,50 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
 {
+.name   = chardev_add_file,
+.args_type  = id:s,path:s,
+.params = id path ,
+.help   = add file chardev,
+.mhandler.cmd = hmp_chardev_add_file,
+},
+
+STEXI
+@item chardev_add_file id path
+@findex chardev_add_file
+
+ETEXI
+
+{
+.name   = chardev_add_tty,
+.args_type  = id:s,path:s,
+.params = id path ,
+.help   = add tty chardev,
+.mhandler.cmd = hmp_chardev_add_tty,
+},
+
+STEXI
+@item chardev_add_tty id path
+@findex chardev_add_tty
+
+ETEXI
+
+{
+.name   = chardev_remove,
+.args_type  = id:s,
+.params = id,
+.help   = remove chardev,
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
 .name   = info,
 .args_type  = item:s?,
 .params = [subcommand],
diff --git a/hmp.c b/hmp.c
index 180ba2b..8780e7c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1335,3 +1335,32 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
*qdict)
 qmp_nbd_server_stop(errp);
 hmp_handle_error(mon, errp);
 }
+
+static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict,
+ const char *backend)
+{
+const char *id   = qdict_get_str(qdict, args);
+const char *path = qdict_get_str(qdict, path);
+Error *local_err = NULL;
+
+qmp_chardev_add_path(id, path, backend, local_err);
+hmp_handle_error(mon, local_err);
+}
+
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict)
+{
+hmp_chardev_add_path(mon, qdict, file);
+}
+
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict)
+{
+hmp_chardev_add_path(mon, qdict, tty);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_chardev_remove(qdict_get_str(qdict, id), local_err);
+hmp_handle_error(mon, local_err);
+}
diff --git a/hmp.h b/hmp.h
index 0ab03be..8cd50d1 100644
--- a/hmp.h
+++ b/hmp.h
@@ -80,5 +80,8 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..34c0e58 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3017,3 +3017,46 @@
 # Since: 1.3.0
 ##
 { 'command': 'nbd-server-stop' }
+
+##
+# @chardev-add-file:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @path: file path
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add-file', 'data': {'id'   : 'str',
+  'path' : 'str' } }
+
+##
+# @chardev-add-tty:
+#
+# Add a terminal chardev
+#
+# @id: the chardev's ID, must be unique
+# @path: device path
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add-tty', 'data': {'id'   : 'str',
+ 'path' : 'str' } }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index 876714f..169743b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2922,3 +2922,44 @@ CharDriverState *qemu_char_get_next_serial(void)
 return serial_hds[next_serial++];
 }
 
+void qmp_chardev_add_path(const char *id, const char *path,
+  const char *backend, Error **errp)
+{
+QemuOpts *opts;
+
+opts = qemu_opts_create(qemu_find_opts(chardev), id, 1, errp);
+if (error_is_set(errp)) {
+return;
+}
+
+qemu_opt_set(opts, path, path);
+qemu_opt_set(opts,

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Paolo Bonzini 
Il 14/12/2012 10:38, Gerd Hoffmann ha scritto:
 This patch adds chardev_add_file, chardev_add_tty and chardev_remove
 monitor commands.
 
 chardev_add_file and chardev_add_tty expect an id and a path, they
 create a file/tty chardev.

I'd rather avoid introducing this interface.  Using multiple commands is
different from all previous examples, both HMP and QMP (including recent
ones such as the NBD server).  It is also hard to extend, for example
file descriptor passing is hard to retrofit.

Perhaps you can define a QAPI union and slowly build it up?  Something
that ultimately can become this:

{ 'enum': 'ChardevFileMode', 'data':
  # pty = console under Windows
  # serial = tty under POSIX
  [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }

{ 'enum: 'ChardevFileSource', 'data':
  [ 'path', 'fd' ] }

{ 'type': 'ChardevFile',
  'data': {'source': 'string', 'source-type': 'ChardevFileSource',
   'mode': 'ChardevFileMode'}}

{ 'type': 'ChardevVC',
  'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}}

{ 'type': 'ChardevSocket',
  'data': {'addr': 'SocketAddress', '*server': 'bool',
   '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} }

# For future extensibility...
{ 'ChardevDummy', 'data': {} }

{ 'union': 'ChardevBackend', 'data': {
  'socket': 'ChardevSocket',
  'udp': 'UDPSocketAddress',
  'file': 'ChardevFile',
  'null': 'ChardevDummy',
  'msmouse': 'ChardevDummy',
  'braille': 'ChardevDummy',
  'stdio': 'ChardevDummy',
  'vc': 'ChardevVC',

  # Solely for HMP usage.
  'legacy': 'str'
}

{ 'command': 'chardev-add', 'data': {
  'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } }

A simple conversion from this to QemuOpts should be easy, later the
backends can move to taking a ChardevBackend *.

Paolo

 chardev_del just takes an id argument and zaps the chardev specified.
 
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  hmp-commands.hx  |   44 +++
  hmp.c|   29 
  hmp.h|3 ++
  qapi-schema.json |   43 ++
  qemu-char.c  |   41 +
  qemu-char.h  |2 +
  qmp-commands.hx  |   76 
 ++
  7 files changed, 238 insertions(+), 0 deletions(-)
 
 diff --git a/hmp-commands.hx b/hmp-commands.hx
 index 010b8c9..82a855a 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -1485,6 +1485,50 @@ passed since 1970, i.e. unix epoch.
  ETEXI
  
  {
 +.name   = chardev_add_file,
 +.args_type  = id:s,path:s,
 +.params = id path ,
 +.help   = add file chardev,
 +.mhandler.cmd = hmp_chardev_add_file,
 +},
 +
 +STEXI
 +@item chardev_add_file id path
 +@findex chardev_add_file
 +
 +ETEXI
 +
 +{
 +.name   = chardev_add_tty,
 +.args_type  = id:s,path:s,
 +.params = id path ,
 +.help   = add tty chardev,
 +.mhandler.cmd = hmp_chardev_add_tty,
 +},
 +
 +STEXI
 +@item chardev_add_tty id path
 +@findex chardev_add_tty
 +
 +ETEXI
 +
 +{
 +.name   = chardev_remove,
 +.args_type  = id:s,
 +.params = id,
 +.help   = remove chardev,
 +.mhandler.cmd = hmp_chardev_remove,
 +},
 +
 +STEXI
 +@item chardev_remove id
 +@findex chardev_remove
 +
 +Removes the chardev @var{id}.
 +
 +ETEXI
 +
 +{
  .name   = info,
  .args_type  = item:s?,
  .params = [subcommand],
 diff --git a/hmp.c b/hmp.c
 index 180ba2b..8780e7c 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -1335,3 +1335,32 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
 *qdict)
  qmp_nbd_server_stop(errp);
  hmp_handle_error(mon, errp);
  }
 +
 +static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict,
 + const char *backend)
 +{
 +const char *id   = qdict_get_str(qdict, args);
 +const char *path = qdict_get_str(qdict, path);
 +Error *local_err = NULL;
 +
 +qmp_chardev_add_path(id, path, backend, local_err);
 +hmp_handle_error(mon, local_err);
 +}
 +
 +void hmp_chardev_add_file(Monitor *mon, const QDict *qdict)
 +{
 +hmp_chardev_add_path(mon, qdict, file);
 +}
 +
 +void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict)
 +{
 +hmp_chardev_add_path(mon, qdict, tty);
 +}
 +
 +void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
 +{
 +Error *local_err = NULL;
 +
 +qmp_chardev_remove(qdict_get_str(qdict, id), local_err);
 +hmp_handle_error(mon, local_err);
 +}
 diff --git a/hmp.h b/hmp.h
 index 0ab03be..8cd50d1 100644
 --- a/hmp.h
 +++ b/hmp.h
 @@ -80,5 +80,8 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict);
  void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
  void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
  void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 +void hmp_chardev_add_file(Monitor *mon, const QDict

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Eric Blake 
On 12/14/2012 05:17 AM, Paolo Bonzini wrote:
 Il 14/12/2012 10:38, Gerd Hoffmann ha scritto:
 This patch adds chardev_add_file, chardev_add_tty and chardev_remove
 monitor commands.

 chardev_add_file and chardev_add_tty expect an id and a path, they
 create a file/tty chardev.
 
 I'd rather avoid introducing this interface.  Using multiple commands is
 different from all previous examples, both HMP and QMP (including recent
 ones such as the NBD server).  It is also hard to extend, for example
 file descriptor passing is hard to retrofit.

File descriptor passing via magic /dev/fdset/nnn should probably already
work.  That said, a single command that uses a QAPI union, rather than
one command per source type, would be nicer from the UI perspective, and
it is the QMP UI perspective that libvirt is concerned about.

 
 Perhaps you can define a QAPI union and slowly build it up?  Something
 that ultimately can become this:
 
 { 'enum': 'ChardevFileMode', 'data':
   # pty = console under Windows
   # serial = tty under POSIX
   [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }
 
 { 'enum: 'ChardevFileSource', 'data':
   [ 'path', 'fd' ] }
 
 { 'type': 'ChardevFile',
   'data': {'source': 'string', 'source-type': 'ChardevFileSource',
'mode': 'ChardevFileMode'}}
 
 { 'type': 'ChardevVC',
   'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}}
 
 { 'type': 'ChardevSocket',
   'data': {'addr': 'SocketAddress', '*server': 'bool',
'*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} }
 
 # For future extensibility...
 { 'ChardevDummy', 'data': {} }
 
 { 'union': 'ChardevBackend', 'data': {
   'socket': 'ChardevSocket',
   'udp': 'UDPSocketAddress',
   'file': 'ChardevFile',
   'null': 'ChardevDummy',
   'msmouse': 'ChardevDummy',
   'braille': 'ChardevDummy',
   'stdio': 'ChardevDummy',
   'vc': 'ChardevVC',
 
   # Solely for HMP usage.
   'legacy': 'str'
 }
 
 { 'command': 'chardev-add', 'data': {
   'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } }

Yes, this looks nicer.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Gerd Hoffmann 
  Hi,

 { 'enum': 'ChardevFileMode', 'data':
   # pty = console under Windows
   # serial = tty under POSIX
   [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }

Hmm, why this enum?  I'd stay close to -chardev, i.e. specify the type
by backend name.

 { 'enum: 'ChardevFileSource', 'data':
   [ 'path', 'fd' ] }

I guess I'd just create a new backend type for file descriptor passing
instead of fitting that into all the existing ones.

 { 'union': 'ChardevBackend', 'data': {

This union thing is new, isn't it?
Makes sense to use that indeed.

   'socket': 'ChardevSocket',
   'udp': 'UDPSocketAddress',
   'file': 'ChardevFile',
   'null': 'ChardevDummy',
   'msmouse': 'ChardevDummy',
   'braille': 'ChardevDummy',
   'stdio': 'ChardevDummy',
   'vc': 'ChardevVC',

I doubt we need them all hotpluggable.

cheers,
  Gerd

From 6ea61630245d8ff9f87ed56b825dcc5f8d1f6e6d Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Thu, 11 Oct 2012 14:53:00 +0200
Subject: [PATCH] chardev: add hotplug support.

This patch adds chardev_add and chardev_remove monitor commands.

chardev_add expects a backend struct filled in and creates the chardev
from that.  For now only file and tty backends are supported.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hmp-commands.hx  |   30 ++
 hmp.c|   19 +++
 hmp.h|2 ++
 qapi-schema.json |   31 +++
 qemu-char.c  |   46 ++
 qemu-char.h  |2 ++
 qmp-commands.hx  |   50 ++
 7 files changed, 180 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..9a0b2eb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1485,6 +1485,36 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
 {
+.name   = chardev_add,
+.args_type  = id:s,backend:s,arg1:s,
+.params = id backend arg,
+.help   = add chardev,
+.mhandler.cmd = hmp_chardev_add,
+},
+
+STEXI
+@item chardev_add id backend arg
+@findex chardev_add
+
+ETEXI
+
+{
+.name   = chardev_remove,
+.args_type  = id:s,
+.params = id,
+.help   = remove chardev,
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
 .name   = info,
 .args_type  = item:s?,
 .params = [subcommand],
diff --git a/hmp.c b/hmp.c
index 180ba2b..c65f4f7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1335,3 +1335,22 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
*qdict)
 qmp_nbd_server_stop(errp);
 hmp_handle_error(mon, errp);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+const char *id  = qdict_get_str(qdict, id);
+const char *backend = qdict_get_str(qdict, backend);
+const char *path= qdict_get_str(qdict, arg1);
+Error *local_err = NULL;
+
+qmp_chardev_add_path(id, path, backend, local_err);
+hmp_handle_error(mon, local_err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_chardev_remove(qdict_get_str(qdict, id), local_err);
+hmp_handle_error(mon, local_err);
+}
diff --git a/hmp.h b/hmp.h
index 0ab03be..e67e482 100644
--- a/hmp.h
+++ b/hmp.h
@@ -80,5 +80,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..7349757 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3017,3 +3017,34 @@
 # Since: 1.3.0
 ##
 { 'command': 'nbd-server-stop' }
+
+##
+# @chardev-add:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: backend type and parameters
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'type': 'ChardevFile', 'data': { 'path': 'str' } }
+{ 'union': 'ChardevBackend', 'data': { 'file': 'ChardevFile',
+   'tty': 'ChardevFile' } }
+{ 'command': 'chardev-add', 'data': {'id'  : 'str',
+ 'backend' : 'ChardevBackend' } }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index 876714f..bf7fdb6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2922,3 +2922,49 @@ CharDriverState *qemu_char_get_next_serial(void)

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Paolo Bonzini 
Il 14/12/2012 14:18, Gerd Hoffmann ha scritto:
   Hi,
 
 { 'enum': 'ChardevFileMode', 'data':
   # pty = console under Windows
   # serial = tty under POSIX
   [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }
 
 Hmm, why this enum?  I'd stay close to -chardev, i.e. specify the type
 by backend name.

Because...

 { 'enum: 'ChardevFileSource', 'data':
   [ 'path', 'fd' ] }
 
 I guess I'd just create a new backend type for file descriptor passing
 instead of fitting that into all the existing ones.

... are you passing a file descriptor for a pipe, a file or a
parallel/serial port?

(pty and console have no arguments, I misremembered).

 { 'union': 'ChardevBackend', 'data': {
 
 This union thing is new, isn't it?

Yeah, a few months old.

 Makes sense to use that indeed.
 
   'socket': 'ChardevSocket',
   'udp': 'UDPSocketAddress',
   'file': 'ChardevFile',
   'null': 'ChardevDummy',
   'msmouse': 'ChardevDummy',
   'braille': 'ChardevDummy',
   'stdio': 'ChardevDummy',
   'vc': 'ChardevVC',
 
 I doubt we need them all hotpluggable.

True, but:

1) I believe long term it's good to move away from QemuOpts; it's good
to provide a complete interface even if all we're doing for now is
building QemuOpts out of the struct.

2) most of them have no options and trivial anyway;

3) the complicated ones (socket, file, perhaps udp) are also the useful
ones.

Paolo

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Gerd Hoffmann 
  Hi,

 ... are you passing a file descriptor for a pipe, a file or a
 parallel/serial port?
 
 The open function of the file-based backends basically do (1) create
 file handles and (2) call qemu_chr_open_fd().  So of you already have an
 fd the differences are gone.  Well, almost.  tty has an special ioctl
 callback to configure line speed.

Also you might want to pass in a socket fd ...

So I really think a -chardev
fd,type={listening-stream-socket,connected-stream-socket,datagram-socket,tty,fd-readwrite,fd-writeonly}
(+ QMP for that) will be more useful.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Gerd Hoffmann 
On 12/14/12 14:45, Paolo Bonzini wrote:
 Il 14/12/2012 14:18, Gerd Hoffmann ha scritto:
   Hi,

 { 'enum': 'ChardevFileMode', 'data':
   # pty = console under Windows
   # serial = tty under POSIX
   [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }

 Hmm, why this enum?  I'd stay close to -chardev, i.e. specify the type
 by backend name.
 
 Because...
 
 { 'enum: 'ChardevFileSource', 'data':
   [ 'path', 'fd' ] }

 I guess I'd just create a new backend type for file descriptor passing
 instead of fitting that into all the existing ones.
 
 ... are you passing a file descriptor for a pipe, a file or a
 parallel/serial port?

The open function of the file-based backends basically do (1) create
file handles and (2) call qemu_chr_open_fd().  So of you already have an
fd the differences are gone.  Well, almost.  tty has an special ioctl
callback to configure line speed.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-12-14 Thread Paolo Bonzini 
Il 14/12/2012 15:19, Gerd Hoffmann ha scritto:
   Hi,
 
 ... are you passing a file descriptor for a pipe, a file or a
 parallel/serial port?

 The open function of the file-based backends basically do (1) create
 file handles and (2) call qemu_chr_open_fd().

Right, and there's the ugliness where you can only specify an output
file, not an input.

 So of you already have an
 fd the differences are gone.  Well, almost.  tty has an special ioctl
 callback to configure line speed.

What about this then:

{ 'union': 'ChardevFileSource',
  'data': {'path': 'str', 'fd': 'str'} }

{ 'type': 'ChardevFile',
  'data': {'*in': 'ChardevFileSource', '*out': 'ChardevFileSource'} }

{ 'enum': 'ChardevPortKind',
  'data': ['serial', 'parallel'] }

{ 'type': 'ChardevPort',
  'data': {'device': 'ChardevFileSource', 'type': 'ChardevPortKind'} }

{ 'type': 'ChardevSpice',
  'data': {'name': 'str', '*debug': 'bool'}}

{ 'type': 'ChardevVC',
  'data': {'*width': 'int', '*height': 'int',
   '*cols': 'int', '*rows': 'int'}}

{ 'type': 'ChardevSocket',
  'data': {'addr': 'SocketAddress', '*server': 'bool',
   '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} }

{ 'type': 'ChardevUDP',
  'data': {'peer': 'SocketAddress',
   '*localhost': 'str', '*localport': 'str' } }

# For future extensibility...
{ 'ChardevDummy', 'data': {} }

{ 'union': 'ChardevBackend', 'data': {
  'socket': 'ChardevSocket',
  'winpipe': 'String',# pipe for _WIN32 case
  'udp': 'ChardevUDP',
  'file': 'ChardevFile',
  'port': 'ChardevPort',
  'pty': 'ChardevDummy',
  'null': 'ChardevDummy',
  'msmouse': 'ChardevDummy',
  'braille': 'ChardevDummy',
  'stdio': 'ChardevDummy',
  'vc': 'ChardevVC',

  # Solely for HMP usage.
  'legacy': 'str'
}

 Also you might want to pass in a socket fd ...

This is already supported by socket_connect/socket_listen in
qemu-sockets.c (thus by the SocketAddress QAPI union).

Paolo

 So I really think a -chardev
 fd,type={listening-stream-socket,connected-stream-socket,datagram-socket,tty,fd-readwrite,fd-writeonly}
 (+ QMP for that) will be more useful.


 cheers,
   Gerd

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-18 Thread Gerd Hoffmann 
On 10/17/12 12:09, Gerd Hoffmann wrote:
 This patch adds chardev_add and chardev_remove monitor commands.
 
 They work similar to the netdev_{add,del} commands.  The hmp version of
 chardev_add accepts like the -chardev command line option does.  The qmp
 version expects the arguments being passed as named parameters.

Trying another approach, see attached patch.  This adds backend-specific
qemu commands to add chardevs, with just the parameters needed for the
specific backend.  Starting with file and tty, both accepting a path.
'file' is nice for testing, 'tty' very useful when hotplugging serial
devices on the host.  Others can be added as needed.  We probably don't
need all of them.  For example hotplugging the 'stdio' chardev doesn't
make much sense.

Advantage #1: Cleaner API.
Advantage #2: No legacy syntax headache when qomifying chardevs.

Comments?

cheers,
  Gerd
From 7f80af55530c9a448e4aed5a41c76a8e9514a27c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Thu, 11 Oct 2012 14:53:00 +0200
Subject: [PATCH] chardev: add hotplug support.

This patch adds chardev_add_file, chardev_add_tty and chardev_remove
monitor commands.

chardev_add_file and chardev_add_tty expect an id and a path, they
create a file/tty chardev.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hmp-commands.hx  |   44 +++
 hmp.c|   29 
 hmp.h|3 ++
 qapi-schema.json |   43 ++
 qemu-char.c  |   41 +
 qemu-char.h  |2 +
 qmp-commands.hx  |   76 ++
 7 files changed, 238 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..ecfc497 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,50 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
 {
+.name   = chardev_add_file,
+.args_type  = id:s,path:s,
+.params = id path ,
+.help   = add file chardev,
+.mhandler.cmd = hmp_chardev_add_file,
+},
+
+STEXI
+@item chardev_add_file id path
+@findex chardev_add_file
+
+ETEXI
+
+{
+.name   = chardev_add_tty,
+.args_type  = id:s,path:s,
+.params = id path ,
+.help   = add tty chardev,
+.mhandler.cmd = hmp_chardev_add_tty,
+},
+
+STEXI
+@item chardev_add_tty id path
+@findex chardev_add_tty
+
+ETEXI
+
+{
+.name   = chardev_remove,
+.args_type  = id:s,
+.params = id,
+.help   = remove chardev,
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
 .name   = info,
 .args_type  = item:s?,
 .params = [subcommand],
diff --git a/hmp.c b/hmp.c
index 70bdec2..346dd29 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,32 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
 qmp_screendump(filename, err);
 hmp_handle_error(mon, err);
 }
+
+static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict,
+ const char *backend)
+{
+const char *id   = qdict_get_str(qdict, args);
+const char *path = qdict_get_str(qdict, path);
+Error *local_err = NULL;
+
+qmp_chardev_add_path(id, path, backend, local_err);
+hmp_handle_error(mon, local_err);
+}
+
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict)
+{
+hmp_chardev_add_path(mon, qdict, file);
+}
+
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict)
+{
+hmp_chardev_add_path(mon, qdict, tty);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_chardev_remove(qdict_get_str(qdict, id), local_err);
+hmp_handle_error(mon, local_err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..107941d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,8 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..76e765b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,46 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev-add-file:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @path: file path
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add-file', 'data': {'id'   : 'str',
+  'path' : 'str'

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-18 Thread Paolo Bonzini 
Il 18/10/2012 12:03, Gerd Hoffmann ha scritto:
 Trying another approach, see attached patch.  This adds backend-specific
 qemu commands to add chardevs, with just the parameters needed for the
 specific backend.  Starting with file and tty, both accepting a path.
 'file' is nice for testing, 'tty' very useful when hotplugging serial
 devices on the host.  Others can be added as needed.  We probably don't
 need all of them.  For example hotplugging the 'stdio' chardev doesn't
 make much sense.
 
 Advantage #1: Cleaner API.
 Advantage #2: No legacy syntax headache when qomifying chardevs.

Disadvantage #1: Harder to extend, more code
Disadvantage #2: Different from all other kinds of host devices.

I still prefer the other one.

Paolo

[Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-17 Thread Gerd Hoffmann 
This patch adds chardev_add and chardev_remove monitor commands.

They work similar to the netdev_{add,del} commands.  The hmp version of
chardev_add accepts like the -chardev command line option does.  The qmp
version expects the arguments being passed as named parameters.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hmp-commands.hx  |   32 
 hmp.c|   23 
 hmp.h|2 +
 qapi-schema.json |   47 +
 qemu-char.c  |   44 ++
 qemu-char.h  |1 +
 qmp-commands.hx  |   61 ++
 7 files changed, 210 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..5207383 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
 {
+.name   = chardev_add,
+.args_type  = args:s,
+.params = args,
+.help   = add chardev,
+.mhandler.cmd = hmp_chardev_add,
+},
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+{
+.name   = chardev_remove,
+.args_type  = id:s,
+.params = id,
+.help   = remove chardev,
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
 .name   = info,
 .args_type  = item:s?,
 .params = [subcommand],
diff --git a/hmp.c b/hmp.c
index 70bdec2..fc9d7bc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
 qmp_screendump(filename, err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+const char *args = qdict_get_str(qdict, args);
+Error *err = NULL;
+QemuOpts *opts;
+
+opts = qemu_opts_parse(qemu_find_opts(chardev), args, 1);
+if (opts == NULL) {
+error_setg(err, Parsing chardev args failed\n);
+} else {
+qemu_chr_new_from_opts(opts, NULL, err);
+}
+hmp_handle_error(mon, err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+qmp_chardev_remove(qdict_get_str(qdict, id),
+   err);
+hmp_handle_error(mon, err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..cabe944 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..fab6bbc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,50 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev-add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: file, socket, ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add', 'data': {'id'  : 'str',
+ 'backend' : 'str',
+ '*path'   : 'str',
+ '*name'   : 'str',
+ '*host'   : 'str',
+ '*port'   : 'str',
+ '*server' : 'bool',
+ '*wait'   : 'bool',
+ '*ipv4'   : 'bool',
+ '*ipv6'   : 'bool',
+ '*telnet' : 'bool' },
+  'gen': 'no' }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index be4ec61..cbfa24e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
 return serial_hds[next_serial++];
 }
 
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+Error *err = NULL;
+QemuOptsList *opts_list;
+QemuOpts *opts;
+
+opts_list =

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-17 Thread Eric Blake 
On 10/17/2012 04:09 AM, Gerd Hoffmann wrote:
 This patch adds chardev_add and chardev_remove monitor commands.
 
 They work similar to the netdev_{add,del} commands.  The hmp version of
 chardev_add accepts like the -chardev command line option does.  The qmp
 version expects the arguments being passed as named parameters.
 
 chardev_del just takes an id argument and zaps the chardev specified.
 
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---

 +##
 +# @chardev-add:
 +#
 +# Add a chardev
 +#
 +# @id: the chardev's ID, must be unique
 +# @backend: the chardev backend: file, socket, ...

This still needs to be an enum.

 +# @path: file / device / unix socket path
 +# @name: spice channel name
 +# @host: host name
 +# @port: port number
 +# @server: create socket in server mode
 +# @wait: wait for connect
 +# @ipv4: force ipv4-only
 +# @ipv6: force ipv6-only
 +# @telnet: telnet negotiation

But this part looks nicer.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-17 Thread Paolo Bonzini 
Il 17/10/2012 17:26, Eric Blake ha scritto:
 On 10/17/2012 04:09 AM, Gerd Hoffmann wrote:
 This patch adds chardev_add and chardev_remove monitor commands.

 They work similar to the netdev_{add,del} commands.  The hmp version of
 chardev_add accepts like the -chardev command line option does.  The qmp
 version expects the arguments being passed as named parameters.

 chardev_del just takes an id argument and zaps the chardev specified.

 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
 
 +##
 +# @chardev-add:
 +#
 +# Add a chardev
 +#
 +# @id: the chardev's ID, must be unique
 +# @backend: the chardev backend: file, socket, ...
 
 This still needs to be an enum.
 
 +# @path: file / device / unix socket path
 +# @name: spice channel name
 +# @host: host name
 +# @port: port number
 +# @server: create socket in server mode
 +# @wait: wait for connect
 +# @ipv4: force ipv4-only
 +# @ipv6: force ipv6-only
 +# @telnet: telnet negotiation
 
 But this part looks nicer.

Note that this schema is just for documentation.  It is not used at QEMU
build time.

Paolo