[08/11] chardev: add serial chardev support to chardev-add (qmp)

Add support for serial chardevs.  Windup qemu_chr_open_win() on
Windows, alias to 'tty' on Linux.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    3 ++-
 qemu-char.c      |   17 +++++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

Il 07/01/2013 14:55, Gerd Hoffmann ha scritto:
> Add support for serial chardevs.  Windup qemu_chr_open_win() on
> Windows, alias to 'tty' on Linux.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |    3 ++-
>  qemu-char.c      |   17 +++++++++++++++--
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7e5c8c2..d833385 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3036,7 +3036,8 @@
>  { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
>                                     'out' : 'ChardevFileSource' } }
>  
> -{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
> +{ 'enum': 'ChardevPortKind', 'data': [ 'tty',
> +                                       'serial' ] }

Can we just use one name, either tty or serial?  And for the QemuOpts
version, alias them too.

Paolo

>  
>  { 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
>                                     'type'   : 'ChardevPortKind'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index 764321b..4232fea 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1672,9 +1672,8 @@ static int win_chr_poll(void *opaque)
>      return 0;
>  }
>  
> -static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
> +static CharDriverState *qemu_chr_open_win_path(const char *filename)
>  {
> -    const char *filename = qemu_opt_get(opts, "path");
>      CharDriverState *chr;
>      WinCharState *s;
>  
> @@ -1693,6 +1692,11 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
>      return chr;
>  }
>  
> +static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
> +{
> +    return qemu_chr_open_win_path(qemu_opt_get(opts, "path"));
> +}
> +
>  static int win_chr_pipe_poll(void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -2771,6 +2775,7 @@ static const struct {
>  #endif
>  #ifdef HAVE_CHARDEV_TTY
>      { .name = "tty",       .open = qemu_chr_open_tty },
> +    { .name = "serial",    .open = qemu_chr_open_tty },
>      { .name = "pty",       .open = qemu_chr_open_pty },
>  #endif
>  #ifdef HAVE_CHARDEV_PARPORT
> @@ -2970,7 +2975,14 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
>  
>  static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
>  {
> +    if (port->device->kind != CHARDEV_FILE_SOURCE_KIND_PATH) {
> +        error_setg(errp, "only file paths supported");
> +        return NULL;
> +    }
> +
>      switch (port->type) {
> +    case CHARDEV_PORT_KIND_SERIAL:
> +        return qemu_chr_open_win_path(port->device->path);
>      default:
>          error_setg(errp, "unknown chardev port (%d)", port->type);
>          return NULL;
> @@ -3033,6 +3045,7 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
>      switch (port->type) {
>  #ifdef HAVE_CHARDEV_TTY
>      case CHARDEV_PORT_KIND_TTY:
> +    case CHARDEV_PORT_KIND_SERIAL:
>          flags = O_RDWR | O_NONBLOCK;
>          fd = qmp_chardev_open_file_source(port->device, flags, errp);
>          if (error_is_set(errp)) {
>

Hi,

>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 7e5c8c2..d833385 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3036,7 +3036,8 @@
>>  { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
>>                                     'out' : 'ChardevFileSource' } }
>>  
>> -{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
>> +{ 'enum': 'ChardevPortKind', 'data': [ 'tty',
>> +                                       'serial' ] }
> 
> Can we just use one name, either tty or serial?

Both are in use today, 'serial' on windows and 'tty' on linux.

'tty' on windows doesn't make sense.

'serial' on linux does, but it actually is a subset of 'tty' and needs
no special care, thats why I simply aliased it for convenience.

We could switch to use 'serial' only for qmp, but I think that will be
confusing as the old 'tty' name will have to stay for -chardev.

We could just not do the alias thing and continue to use the 'serial' on
windows + 'tty' on linux (posix) scheme.

> And for the QemuOpts
> version, alias them too.

Did that ...

>>  #ifdef HAVE_CHARDEV_TTY
>>      { .name = "tty",       .open = qemu_chr_open_tty },
>> +    { .name = "serial",    .open = qemu_chr_open_tty },
>>      { .name = "pty",       .open = qemu_chr_open_pty },
>>  #endif

... here.

cheers,
  Gerd

Hi,

>> Can we just use one name, either tty or serial?

Ah, after reading the parport patch comment I think I understand what
you suggest:

 (1) Standardize on 'serial' + 'parallel' on the new qmp interface.
 (2) Support these names on -chardev too.
 (3) Alias the old names to the new ones for -chardev for backward
     compatibility.

correct?

cheers,
  Gerd

Il 10/01/2013 13:37, Gerd Hoffmann ha scritto:
>>> >> Can we just use one name, either tty or serial?
> Ah, after reading the parport patch comment I think I understand what
> you suggest:
> 
>  (1) Standardize on 'serial' + 'parallel' on the new qmp interface.

Or tty + parallel, as you see more fit.

>  (2) Support these names on -chardev too.
>  (3) Alias the old names to the new ones for -chardev for backward
>      compatibility.

Yes, exactly.

Paolo