[Qemu-devel] [PATCH v2 0/3] char: fix chardev aliases regression

[Qemu-devel] [PATCH v2 0/3] char: fix chardev aliases regression

[Marc-André Lureau]

Hi,

The patch "char: move CharBackend handling in char-fe unit" broke
chardev aliases. Here is a small series to fix it, and add a simple
unit test to check the alias keep working.

v2:
- move HAVE_CHARDEV_* in osdep.h (Markus Armbruster)
- add r-b tags from Eric for the patches that didn't change

Marc-André Lureau (3):
  char: fix alias devices regression
  chardev: don't use alias names in parse_compat()
  test-char: start a /char/serial test

 include/chardev/char-parallel.h |  5 -----
 include/chardev/char-serial.h   |  8 --------
 include/qemu/osdep.h            | 13 +++++++++++++
 chardev/char.c                  |  4 ++--
 tests/test-char.c               | 30 ++++++++++++++++++++++++++++++
 5 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.13.0.91.g00982b8dd

[Qemu-devel] [PATCH v2 1/3] char: fix alias devices regression

[Marc-André Lureau]

Fix regression from commit 4d43a603c71, where the serial and parallel
headers got removed from char.c, which broke the alias table.

Move the HAVE_CHARDEV_SERIAL/HAVE_CHARDEV_PARPORT to osdep.h instead
of being in seperate headers.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/chardev/char-parallel.h |  5 -----
 include/chardev/char-serial.h   |  8 --------
 include/qemu/osdep.h            | 13 +++++++++++++
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/chardev/char-parallel.h b/include/chardev/char-parallel.h
index 3284a1b96b..c09751fd6c 100644
--- a/include/chardev/char-parallel.h
+++ b/include/chardev/char-parallel.h
@@ -26,11 +26,6 @@
 
 #include "chardev/char.h"
 
-#if defined(__linux__) || defined(__FreeBSD__) ||               \
-    defined(__FreeBSD_kernel__) || defined(__DragonFly__)
-#define HAVE_CHARDEV_PARPORT 1
-#endif
-
 #define CHR_IOCTL_PP_READ_DATA        3
 #define CHR_IOCTL_PP_WRITE_DATA       4
 #define CHR_IOCTL_PP_READ_CONTROL     5
diff --git a/include/chardev/char-serial.h b/include/chardev/char-serial.h
index cb2e59e82a..ad6891b26d 100644
--- a/include/chardev/char-serial.h
+++ b/include/chardev/char-serial.h
@@ -26,14 +26,6 @@
 
 #include "chardev/char.h"
 
-#ifdef _WIN32
-#define HAVE_CHARDEV_SERIAL 1
-#elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)    \
-    || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
-    || defined(__GLIBC__)
-#define HAVE_CHARDEV_SERIAL 1
-#endif
-
 #define CHR_IOCTL_SERIAL_SET_PARAMS   1
 typedef struct {
     int speed;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1c9f5e260c..fb008a2e65 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -284,6 +284,19 @@ void qemu_anon_ram_free(void *ptr, size_t size);
 
 #endif
 
+#ifdef _WIN32
+#define HAVE_CHARDEV_SERIAL 1
+#elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)    \
+    || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
+    || defined(__GLIBC__)
+#define HAVE_CHARDEV_SERIAL 1
+#endif
+
+#if defined(__linux__) || defined(__FreeBSD__) ||               \
+    defined(__FreeBSD_kernel__) || defined(__DragonFly__)
+#define HAVE_CHARDEV_PARPORT 1
+#endif
+
 #if defined(CONFIG_LINUX)
 #ifndef BUS_MCEERR_AR
 #define BUS_MCEERR_AR 4
-- 
2.13.0.91.g00982b8dd

[Qemu-devel] [PATCH v2 2/3] chardev: don't use alias names in parse_compat()

[Marc-André Lureau]

"parport" is considered "old" since commit 88a946d32d, when "parallel"
was added. Similarly for "tty" in commit d59044ef74d.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 chardev/char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 7aa0210765..bcfc065d16 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -450,12 +450,12 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
     }
     if (strstart(filename, "/dev/parport", NULL) ||
         strstart(filename, "/dev/ppi", NULL)) {
-        qemu_opt_set(opts, "backend", "parport", &error_abort);
+        qemu_opt_set(opts, "backend", "parallel", &error_abort);
         qemu_opt_set(opts, "path", filename, &error_abort);
         return opts;
     }
     if (strstart(filename, "/dev/", NULL)) {
-        qemu_opt_set(opts, "backend", "tty", &error_abort);
+        qemu_opt_set(opts, "backend", "serial", &error_abort);
         qemu_opt_set(opts, "path", filename, &error_abort);
         return opts;
     }
-- 
2.13.0.91.g00982b8dd

[Qemu-devel] [PATCH v2 3/3] test-char: start a /char/serial test

[Marc-André Lureau]

Quite limited test, to check that the chardev can be created with a
path and with the tty alias.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-char.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index dfe856cb85..ecc3fec194 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -5,6 +5,7 @@
 #include "qemu/config-file.h"
 #include "qemu/sockets.h"
 #include "chardev/char-fe.h"
+#include "chardev/char-serial.h" /* for HAVE_CHARDEV_SERIAL */
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
 #include "qom/qom-qobject.h"
@@ -450,6 +451,32 @@ static void char_udp_test(void)
     g_free(tmp);
 }
 
+#ifdef HAVE_CHARDEV_SERIAL
+static void char_serial_test(void)
+{
+    QemuOpts *opts;
+    Chardev *chr;
+
+    opts = qemu_opts_create(qemu_find_opts("chardev"), "serial-id",
+                            1, &error_abort);
+    qemu_opt_set(opts, "backend", "serial", &error_abort);
+    qemu_opt_set(opts, "path", "/dev/null", &error_abort);
+
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    g_assert_nonnull(chr);
+    /* TODO: add more tests with a pty */
+    object_unparent(OBJECT(chr));
+
+    /* test tty alias */
+    qemu_opt_set(opts, "backend", "tty", &error_abort);
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    g_assert_nonnull(chr);
+    object_unparent(OBJECT(chr));
+
+    qemu_opts_del(opts);
+}
+#endif
+
 static void char_file_test(void)
 {
     char *tmp_path = g_dir_make_tmp("qemu-test-char.XXXXXX", NULL);
@@ -597,6 +624,9 @@ int main(int argc, char **argv)
     g_test_add_func("/char/file", char_file_test);
     g_test_add_func("/char/socket", char_socket_test);
     g_test_add_func("/char/udp", char_udp_test);
+#ifdef HAVE_CHARDEV_SERIAL
+    g_test_add_func("/char/serial", char_serial_test);
+#endif
 
     return g_test_run();
 }
-- 
2.13.0.91.g00982b8dd

Re: [Qemu-devel] [PATCH v2 1/3] char: fix alias devices regression

[Eric Blake]

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

On 06/08/2017 06:59 AM, Marc-André Lureau wrote:
> Fix regression from commit 4d43a603c71, where the serial and parallel
> headers got removed from char.c, which broke the alias table.
> 
> Move the HAVE_CHARDEV_SERIAL/HAVE_CHARDEV_PARPORT to osdep.h instead
> of being in seperate headers.

s/seperate/separate/

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/chardev/char-parallel.h |  5 -----
>  include/chardev/char-serial.h   |  8 --------
>  include/qemu/osdep.h            | 13 +++++++++++++
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v2 3/3] test-char: start a /char/serial test

[Eric Blake]

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

On 06/08/2017 06:59 AM, Marc-André Lureau wrote:
> Quite limited test, to check that the chardev can be created with a
> path and with the tty alias.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/test-char.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/tests/test-char.c b/tests/test-char.c
> index dfe856cb85..ecc3fec194 100644
> --- a/tests/test-char.c
> +++ b/tests/test-char.c
> @@ -5,6 +5,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/sockets.h"
>  #include "chardev/char-fe.h"
> +#include "chardev/char-serial.h" /* for HAVE_CHARDEV_SERIAL */

Either the comment or the entire #include is now spurious, given the new
1/3.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v2 3/3] test-char: start a /char/serial test

[Marc-André Lureau]

Hi

----- Original Message -----
> On 06/08/2017 06:59 AM, Marc-André Lureau wrote:
> > Quite limited test, to check that the chardev can be created with a
> > path and with the tty alias.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > ---
> >  tests/test-char.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/tests/test-char.c b/tests/test-char.c
> > index dfe856cb85..ecc3fec194 100644
> > --- a/tests/test-char.c
> > +++ b/tests/test-char.c
> > @@ -5,6 +5,7 @@
> >  #include "qemu/config-file.h"
> >  #include "qemu/sockets.h"
> >  #include "chardev/char-fe.h"
> > +#include "chardev/char-serial.h" /* for HAVE_CHARDEV_SERIAL */
> 
> Either the comment or the entire #include is now spurious, given the new
> 1/3.

indeed, removed

> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 
> 

Re: [Qemu-devel] [PATCH v2 1/3] char: fix alias devices regression

[Anthony PERARD]

On Thu, Jun 08, 2017 at 03:59:37PM +0400, Marc-André Lureau wrote:
> Fix regression from commit 4d43a603c71, where the serial and parallel
> headers got removed from char.c, which broke the alias table.
> 
> Move the HAVE_CHARDEV_SERIAL/HAVE_CHARDEV_PARPORT to osdep.h instead
> of being in seperate headers.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks, that works for me.

Tested-by: Anthony PERARD <anthony.perard@citrix.com>

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH v2 0/3] char: fix chardev aliases regression

[Paolo Bonzini]

On 08/06/2017 13:59, Marc-André Lureau wrote:
> Hi,
> 
> The patch "char: move CharBackend handling in char-fe unit" broke
> chardev aliases. Here is a small series to fix it, and add a simple
> unit test to check the alias keep working.
> 
> v2:
> - move HAVE_CHARDEV_* in osdep.h (Markus Armbruster)
> - add r-b tags from Eric for the patches that didn't change
> 
> Marc-André Lureau (3):
>   char: fix alias devices regression
>   chardev: don't use alias names in parse_compat()
>   test-char: start a /char/serial test
> 
>  include/chardev/char-parallel.h |  5 -----
>  include/chardev/char-serial.h   |  8 --------
>  include/qemu/osdep.h            | 13 +++++++++++++
>  chardev/char.c                  |  4 ++--
>  tests/test-char.c               | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 45 insertions(+), 15 deletions(-)
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH v2 0/3] char: fix chardev aliases regression

[Marc-André Lureau]

Hi Paolo

On Mon, Jun 12, 2017 at 8:06 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

>
>
> On 08/06/2017 13:59, Marc-André Lureau wrote:
> > Hi,
> >
> > The patch "char: move CharBackend handling in char-fe unit" broke
> > chardev aliases. Here is a small series to fix it, and add a simple
> > unit test to check the alias keep working.
> >
> > v2:
> > - move HAVE_CHARDEV_* in osdep.h (Markus Armbruster)
> > - add r-b tags from Eric for the patches that didn't change
> >
> > Marc-André Lureau (3):
> >   char: fix alias devices regression
> >   chardev: don't use alias names in parse_compat()
> >   test-char: start a /char/serial test
> >
> >  include/chardev/char-parallel.h |  5 -----
> >  include/chardev/char-serial.h   |  8 --------
> >  include/qemu/osdep.h            | 13 +++++++++++++
> >  chardev/char.c                  |  4 ++--
> >  tests/test-char.c               | 30 ++++++++++++++++++++++++++++++
> >  5 files changed, 45 insertions(+), 15 deletions(-)
> >
>
> Queued, thanks.
>

I also sent a pull request with those patches "[PULL 0/3] Char patches" a
few days ago. I should take the habit to notice it on the PATCH series.

Thanks
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 0/3] char: fix chardev aliases regression

[Paolo Bonzini]

On 12/06/2017 21:01, Marc-André Lureau wrote:
> Hi Paolo
> 
> On Mon, Jun 12, 2017 at 8:06 PM Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
> 
> 
> 
>     On 08/06/2017 13:59, Marc-André Lureau wrote:
>     > Hi,
>     >
>     > The patch "char: move CharBackend handling in char-fe unit" broke
>     > chardev aliases. Here is a small series to fix it, and add a simple
>     > unit test to check the alias keep working.
>     >
>     > v2:
>     > - move HAVE_CHARDEV_* in osdep.h (Markus Armbruster)
>     > - add r-b tags from Eric for the patches that didn't change
>     >
>     > Marc-André Lureau (3):
>     >   char: fix alias devices regression
>     >   chardev: don't use alias names in parse_compat()
>     >   test-char: start a /char/serial test
>     >
>     >  include/chardev/char-parallel.h |  5 -----
>     >  include/chardev/char-serial.h   |  8 --------
>     >  include/qemu/osdep.h            | 13 +++++++++++++
>     >  chardev/char.c                  |  4 ++--
>     >  tests/test-char.c               | 30 ++++++++++++++++++++++++++++++
>     >  5 files changed, 45 insertions(+), 15 deletions(-)
>     >
> 
>     Queued, thanks.
> 
> 
> I also sent a pull request with those patches "[PULL0/3] Charpatches" a
> few days ago. I should take the habit to notice it on the PATCH series.

No problem, I would have noticed sooner or later. :)

Paolo