[PATCH v2 0/3] char: Deprecate backend aliases

[PATCH v2 0/3] char: Deprecate backend aliases

[Kevin Wolf]

A small series from November that has fallen through the cracks...

These aliases only work the command line, but not in QMP. Command line
QAPIfication involves writing some compatibility glue for them, which
I'm doing, but I think it's desirable to unify accepted values of both
paths. So deprecate the aliases so that we can drop the compatibility
glue later.

v2:
- Don't mention deprecated options in the help [Markus]

- Added third patch to simplify the code again. Markus had suggested
  not complicating it in the first place, but then I would have to merge
  patches 1 and 2 even though they address different points, so this
  didn't feel very desirable.

Kevin Wolf (3):
  char: Skip CLI aliases in query-chardev-backends
  char: Deprecate backend aliases 'tty' and 'parport'
  char: Simplify chardev_name_foreach()

 docs/system/deprecated.rst |  6 ++++++
 chardev/char.c             | 19 +++++++++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.29.2

[PATCH v2 1/3] char: Skip CLI aliases in query-chardev-backends

[Kevin Wolf]

The aliases "tty" and "parport" are only valid on the command line, QMP
commands like chardev-add don't know them. query-chardev-backends should
describe QMP and therefore not include them in the list of available
backends.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/char.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 97cafd6849..dd925cf9a4 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -547,7 +547,7 @@ static const struct ChardevAlias {
 };
 
 typedef struct ChadevClassFE {
-    void (*fn)(const char *name, void *opaque);
+    void (*fn)(const char *name, bool is_cli_alias, void *opaque);
     void *opaque;
 } ChadevClassFE;
 
@@ -561,11 +561,13 @@ chardev_class_foreach(ObjectClass *klass, void *opaque)
         return;
     }
 
-    fe->fn(object_class_get_name(klass) + 8, fe->opaque);
+    fe->fn(object_class_get_name(klass) + 8, false, fe->opaque);
 }
 
 static void
-chardev_name_foreach(void (*fn)(const char *name, void *opaque), void *opaque)
+chardev_name_foreach(void (*fn)(const char *name, bool is_cli_alias,
+                                void *opaque),
+                     void *opaque)
 {
     ChadevClassFE fe = { .fn = fn, .opaque = opaque };
     int i;
@@ -573,12 +575,12 @@ chardev_name_foreach(void (*fn)(const char *name, void *opaque), void *opaque)
     object_class_foreach(chardev_class_foreach, TYPE_CHARDEV, false, &fe);
 
     for (i = 0; i < (int)ARRAY_SIZE(chardev_alias_table); i++) {
-        fn(chardev_alias_table[i].alias, opaque);
+        fn(chardev_alias_table[i].alias, true, opaque);
     }
 }
 
 static void
-help_string_append(const char *name, void *opaque)
+help_string_append(const char *name, bool is_cli_alias, void *opaque)
 {
     GString *str = opaque;
 
@@ -798,11 +800,16 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
 }
 
 static void
-qmp_prepend_backend(const char *name, void *opaque)
+qmp_prepend_backend(const char *name, bool is_cli_alias, void *opaque)
 {
     ChardevBackendInfoList **list = opaque;
-    ChardevBackendInfo *value = g_new0(ChardevBackendInfo, 1);
+    ChardevBackendInfo *value;
 
+    if (is_cli_alias) {
+        return;
+    }
+
+    value = g_new0(ChardevBackendInfo, 1);
     value->name = g_strdup(name);
     QAPI_LIST_PREPEND(*list, value);
 }
-- 
2.29.2

[PATCH v2 2/3] char: Deprecate backend aliases 'tty' and 'parport'

[Kevin Wolf]

QAPI doesn't know the aliases 'tty' and 'parport' and there is no
reason to prefer them to the real names of the backends 'serial' and
'parallel'.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/system/deprecated.rst |  6 ++++++
 chardev/char.c             | 12 +++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 3e9e3a26f6..11da79ea5b 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -75,6 +75,12 @@ The ``pretty=on|off`` switch has no effect for HMP monitors, but is
 silently ignored. Using the switch with HMP monitors will become an
 error in the future.
 
+``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+``tty`` and ``parport`` are aliases that will be removed. Instead, the
+actual backend names ``serial`` and ``parallel`` should be used.
+
 RISC-V ``-bios`` (since 5.1)
 ''''''''''''''''''''''''''''
 
diff --git a/chardev/char.c b/chardev/char.c
index dd925cf9a4..7be9579dd8 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -534,9 +534,10 @@ static const ChardevClass *char_get_class(const char *driver, Error **errp)
     return cc;
 }
 
-static const struct ChardevAlias {
+static struct ChardevAlias {
     const char *typename;
     const char *alias;
+    bool deprecation_warning_printed;
 } chardev_alias_table[] = {
 #ifdef HAVE_CHARDEV_PARPORT
     { "parallel", "parport" },
@@ -584,6 +585,10 @@ help_string_append(const char *name, bool is_cli_alias, void *opaque)
 {
     GString *str = opaque;
 
+    if (is_cli_alias) {
+        return;
+    }
+
     g_string_append_printf(str, "\n  %s", name);
 }
 
@@ -592,6 +597,11 @@ static const char *chardev_alias_translate(const char *name)
     int i;
     for (i = 0; i < (int)ARRAY_SIZE(chardev_alias_table); i++) {
         if (g_strcmp0(chardev_alias_table[i].alias, name) == 0) {
+            if (!chardev_alias_table[i].deprecation_warning_printed) {
+                warn_report("The alias '%s' is deprecated, use '%s' instead",
+                            name, chardev_alias_table[i].typename);
+                chardev_alias_table[i].deprecation_warning_printed = true;
+            }
             return chardev_alias_table[i].typename;
         }
     }
-- 
2.29.2

[PATCH v2 3/3] char: Simplify chardev_name_foreach()

[Kevin Wolf]

Both callers use callbacks that don't do anything when they are called
for CLI aliases. Instead of passing the cli_alias parameter, just don't
call the callbacks for aliases in the first place.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 chardev/char.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 7be9579dd8..140d6d9d36 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -548,7 +548,7 @@ static struct ChardevAlias {
 };
 
 typedef struct ChadevClassFE {
-    void (*fn)(const char *name, bool is_cli_alias, void *opaque);
+    void (*fn)(const char *name, void *opaque);
     void *opaque;
 } ChadevClassFE;
 
@@ -562,33 +562,23 @@ chardev_class_foreach(ObjectClass *klass, void *opaque)
         return;
     }
 
-    fe->fn(object_class_get_name(klass) + 8, false, fe->opaque);
+    fe->fn(object_class_get_name(klass) + 8, fe->opaque);
 }
 
 static void
-chardev_name_foreach(void (*fn)(const char *name, bool is_cli_alias,
-                                void *opaque),
+chardev_name_foreach(void (*fn)(const char *name, void *opaque),
                      void *opaque)
 {
     ChadevClassFE fe = { .fn = fn, .opaque = opaque };
-    int i;
 
     object_class_foreach(chardev_class_foreach, TYPE_CHARDEV, false, &fe);
-
-    for (i = 0; i < (int)ARRAY_SIZE(chardev_alias_table); i++) {
-        fn(chardev_alias_table[i].alias, true, opaque);
-    }
 }
 
 static void
-help_string_append(const char *name, bool is_cli_alias, void *opaque)
+help_string_append(const char *name, void *opaque)
 {
     GString *str = opaque;
 
-    if (is_cli_alias) {
-        return;
-    }
-
     g_string_append_printf(str, "\n  %s", name);
 }
 
@@ -810,15 +800,11 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
 }
 
 static void
-qmp_prepend_backend(const char *name, bool is_cli_alias, void *opaque)
+qmp_prepend_backend(const char *name, void *opaque)
 {
     ChardevBackendInfoList **list = opaque;
     ChardevBackendInfo *value;
 
-    if (is_cli_alias) {
-        return;
-    }
-
     value = g_new0(ChardevBackendInfo, 1);
     value->name = g_strdup(name);
     QAPI_LIST_PREPEND(*list, value);
-- 
2.29.2

Re: [PATCH v2 0/3] char: Deprecate backend aliases

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> A small series from November that has fallen through the cracks...

Glad you remembered!

> These aliases only work the command line, but not in QMP. Command line
> QAPIfication involves writing some compatibility glue for them, which
> I'm doing, but I think it's desirable to unify accepted values of both
> paths. So deprecate the aliases so that we can drop the compatibility
> glue later.
>
> v2:
> - Don't mention deprecated options in the help [Markus]
>
> - Added third patch to simplify the code again. Markus had suggested
>   not complicating it in the first place, but then I would have to merge
>   patches 1 and 2 even though they address different points, so this
>   didn't feel very desirable.

Complicated review a bit, but I'm not sure the alternative would've been
easier.

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks!