[Qemu-devel] [PULL 0/2] chardev: fix "info chardev" output

[Qemu-devel] [PULL 0/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

  Hi,

Tiny series, first reverting the broken patch from Lei Li,
then fixing it for real.

please pull,
  Gerd

The following changes since commit 6a4e17711442849bf2cc731ccddef5a2a2d92d29:

  Remove Sun4c, Sun4d and a few CPUs (2013-05-26 11:37:58 +0000)

are available in the git repository at:

  git://git.kraxel.org/qemu chardev.6

for you to fetch changes up to 60d95386abf1f5f37ded6a812d12ac458df2491b:

  chardev: fix "info chardev" output (2013-05-27 12:47:28 +0200)

----------------------------------------------------------------
Gerd Hoffmann (2):
      Revert "chardev: Get filename for new qapi backend"
      chardev: fix "info chardev" output

 qemu-char.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

[Qemu-devel] [PATCH 1/2] Revert "chardev: Get filename for new qapi backend"

[Gerd Hoffmann]

Does not handle chardevs created via chardev-add monitor command.

This reverts commit 2b220025993e76d4116781ca91a4fabc5ad9c722.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 4f8382e..f825294 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3276,7 +3276,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         ChardevReturn *ret = NULL;
         const char *id = qemu_opts_id(opts);
         const char *bid = NULL;
-        char *filename = g_strdup(qemu_opt_get(opts, "backend"));
 
         if (qemu_opt_get_bool(opts, "mux", 0)) {
             bid = g_strdup_printf("%s-base", id);
@@ -3309,7 +3308,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         }
 
         chr = qemu_chr_find(id);
-        chr->filename = filename;
 
     qapi_out:
         qapi_free_ChardevBackend(backend);
-- 
1.7.9.7

[Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

Fill unset CharDriverState->filename with the backend name, so
'info chardev' will return at least the chardev type.  Don't
touch it in case the chardev init function filled it already,
like the socket+pty chardevs do for example.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index f825294..d04b429 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3801,6 +3801,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         chr->label = g_strdup(id);
         chr->avail_connections =
             (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
+        if (!chr->filename) {
+            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
+        }
         QTAILQ_INSERT_TAIL(&chardevs, chr, next);
         return ret;
     } else {
-- 
1.7.9.7

Re: [Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

On 05/28/13 08:34, Gerd Hoffmann wrote:
> Fill unset CharDriverState->filename with the backend name, so
> 'info chardev' will return at least the chardev type.  Don't
> touch it in case the chardev init function filled it already,
> like the socket+pty chardevs do for example.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-char.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index f825294..d04b429 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3801,6 +3801,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          chr->label = g_strdup(id);
>          chr->avail_connections =
>              (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
> +        if (!chr->filename) {
> +            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
> +        }
>          QTAILQ_INSERT_TAIL(&chardevs, chr, next);
>          return ret;
>      } else {
> 

this one is a stable candidate, cc'ing qemu-stable

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On 05/28/2013 12:34 AM, Gerd Hoffmann wrote:
> Fill unset CharDriverState->filename with the backend name, so
> 'info chardev' will return at least the chardev type.  Don't
> touch it in case the chardev init function filled it already,
> like the socket+pty chardevs do for example.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-char.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index f825294..d04b429 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3801,6 +3801,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          chr->label = g_strdup(id);
>          chr->avail_connections =
>              (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
> +        if (!chr->filename) {
> +            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
> +        }
>          QTAILQ_INSERT_TAIL(&chardevs, chr, next);
>          return ret;
>      } else {
> 

Peter was telling me on IRC that this patch is still broken with regards
to libvirt; I've cc'd him to provide more details...

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Peter Krempa]

On 05/31/13 14:36, Eric Blake wrote:
> On 05/28/2013 12:34 AM, Gerd Hoffmann wrote:
>> Fill unset CharDriverState->filename with the backend name, so
>> 'info chardev' will return at least the chardev type.  Don't
>> touch it in case the chardev init function filled it already,
>> like the socket+pty chardevs do for example.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   qemu-char.c |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index f825294..d04b429 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -3801,6 +3801,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>>           chr->label = g_strdup(id);
>>           chr->avail_connections =
>>               (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
>> +        if (!chr->filename) {
>> +            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
>> +        }
>>           QTAILQ_INSERT_TAIL(&chardevs, chr, next);
>>           return ret;
>>       } else {
>>
> 
> Peter was telling me on IRC that this patch is still broken with regards
> to libvirt; I've cc'd him to provide more details...
> 

Without this patch the returned message for "query-chardev" is:

{
    "return": [
        {
            "filename": "pty:/dev/pts/8",
            "label": "charserial0"
        },
        {
            "filename": "unix:/var/lib/libvirt/qemu/qemu-git.monitor,server",
            "label": "charmonitor"
        }
    ],
    "id": "libvirt-2"
}


this patch changes it to:

{
    "return": [
        {
            "filename": "pty",
            "label": "charserial0"
        },
        {
            "filename": "unix:/var/lib/libvirt/qemu/qemu-git.monitor,server",
            "label": "charmonitor"
        }
    ],
    "id": "libvirt-2"
}

It's apparent that some code being executed after the code in this patch
fills the actual pty path that was allocated. With it pre-allocated the
code ignores it. Libvirt is using the output to gather names of the pty
so that they can be used to connect to the console.

Peter

Re: [Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

  Hi,

> Without this patch the returned message for "query-chardev" is:
> 
> {
>     "return": [
>         {
>             "filename": "pty:/dev/pts/8",
>             "label": "charserial0"
>         },
>         {
>             "filename": "unix:/var/lib/libvirt/qemu/qemu-git.monitor,server",
>             "label": "charmonitor"
>         }
>     ],
>     "id": "libvirt-2"
> }
> 
> 
> this patch changes it to:
> 
> {
>     "return": [
>         {
>             "filename": "pty",
>             "label": "charserial0"
>         },
>         {
>             "filename": "unix:/var/lib/libvirt/qemu/qemu-git.monitor,server",
>             "label": "charmonitor"
>         }
>     ],
>     "id": "libvirt-2"
> }

Please double-check.  Current master
(87d23f78aa79b72da022afda358bbc8a8509ca70 to be exact) works just fine
for me.  libvirt works, including a serial line redirected to pty, and
'info chardev' looks sane too.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Peter Krempa]

On 05/31/13 15:21, Gerd Hoffmann wrote:
>    Hi,
>

Hi,

>
> Please double-check.  Current master
> (87d23f78aa79b72da022afda358bbc8a8509ca70 to be exact) works just fine
> for me.  libvirt works, including a serial line redirected to pty, and
> 'info chardev' looks sane too.

sorry for the fuzz. :/ Upstream is really working, One of the tested 
binaries was actually predating the pull of your fix and I forgot to 
verify that as It was built yesterday.


Again sorry for the noise, it's working now.

Peter

>
> cheers,
>    Gerd
>

Re: [Qemu-devel] [PULL 0/2] chardev: fix "info chardev" output

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On 05/28/2013 12:34 AM, Gerd Hoffmann wrote:
>   Hi,
> 
> Tiny series, first reverting the broken patch from Lei Li,
> then fixing it for real.
> 
> please pull,
>   Gerd
> 
> The following changes since commit 6a4e17711442849bf2cc731ccddef5a2a2d92d29:
> 
>   Remove Sun4c, Sun4d and a few CPUs (2013-05-26 11:37:58 +0000)
> 
> are available in the git repository at:
> 
>   git://git.kraxel.org/qemu chardev.6
> 
> for you to fetch changes up to 60d95386abf1f5f37ded6a812d12ac458df2491b:
> 
>   chardev: fix "info chardev" output (2013-05-27 12:47:28 +0200)
> 
> ----------------------------------------------------------------
> Gerd Hoffmann (2):
>       Revert "chardev: Get filename for new qapi backend"
>       chardev: fix "info chardev" output
> 

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

>  qemu-char.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PULL 0/2] chardev: fix "info chardev" output

[Anthony Liguori]

Pulled.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [PULL 0/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

  Hi,

Tiny series, first reverting the broken patch from Lei Li,
then fixing it for real.

please pull,
  Gerd

The following changes since commit 6a4e17711442849bf2cc731ccddef5a2a2d92d29:

  Remove Sun4c, Sun4d and a few CPUs (2013-05-26 11:37:58 +0000)

are available in the git repository at:

  git://git.kraxel.org/qemu chardev.6

for you to fetch changes up to 60d95386abf1f5f37ded6a812d12ac458df2491b:

  chardev: fix "info chardev" output (2013-05-27 12:47:28 +0200)

----------------------------------------------------------------
Gerd Hoffmann (2):
      Revert "chardev: Get filename for new qapi backend"
      chardev: fix "info chardev" output

 qemu-char.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

[Qemu-devel] [PATCH 2/2] chardev: fix "info chardev" output

[Gerd Hoffmann]

Fill unset CharDriverState->filename with the backend name, so
'info chardev' will return at least the chardev type.  Don't
touch it in case the chardev init function filled it already,
like the socket+pty chardevs do for example.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index f825294..d04b429 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3801,6 +3801,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         chr->label = g_strdup(id);
         chr->avail_connections =
             (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
+        if (!chr->filename) {
+            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
+        }
         QTAILQ_INSERT_TAIL(&chardevs, chr, next);
         return ret;
     } else {
-- 
1.7.9.7