| Message ID | 1369722844-24345-3-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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...
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
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
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 >
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 {
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(+)