2:08 p.m.
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.
In the deprecation documentation I assumed that this is for 6.0, but if
we want to include it in 5.2 still, this can be changed, of course.
Kevin Wolf (2):
char: Skip CLI aliases in query-chardev-backends
char: Deprecate backend aliases 'tty' and 'parport'
docs/system/deprecated.rst | 6 ++++++
chardev/char.c | 32 ++++++++++++++++++++++++--------
2 files changed, 30 insertions(+), 8 deletions(-)
--
2.28.0
2:08 p.m.
New subject: [PATCH 1/2] char: Skip CLI aliases in query-chardev-backends
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(a)redhat.com>
---
chardev/char.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index aa4282164a..c406e61db6 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;
@@ -800,11 +802,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;
- ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
+ ChardevBackendInfoList *info;
+ if (is_cli_alias) {
+ return;
+ }
+
+ info = g_malloc0(sizeof(*info));
info->value = g_malloc0(sizeof(*info->value));
info->value->name = g_strdup(name);
info->next = *list;
--
2.28.0
9:22 a.m.
New subject: [PATCH 1/2] char: Skip CLI aliases in query-chardev-backends
Kevin Wolf <kwolf(a)redhat.com> writes:
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(a)redhat.com>
I'd call that a bug.
In the light of PATCH 2, I propose to put that one first (with the
help_string_append() hunk dropped), then remove the aliases from CLI
help, too, like this:
diff --git a/chardev/char.c b/chardev/char.c
index aa4282164a..8b6e78a122 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -568,13 +568,8 @@ static void
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, opaque);
- }
}
static void
11:58 a.m.
New subject: [PATCH 1/2] char: Skip CLI aliases in query-chardev-backends
Am 12.11.2020 um 09:22 hat Markus Armbruster geschrieben:
Kevin Wolf <kwolf(a)redhat.com> writes:
> 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(a)redhat.com>
I'd call that a bug.
Which is why I'm fixing it, separate from the deprecation.
In the light of PATCH 2, I propose to put that one first (with the
help_string_append() hunk dropped), then remove the aliases from CLI
help, too, like this: [...]
Going one step back without thinking in solutions immediately, what
you're suggesting is that deprecated options should become undocumented
instead of just annotated as deprecated?
Kevin
9:01 a.m.
New subject: [PATCH 1/2] char: Skip CLI aliases in query-chardev-backends
Kevin Wolf <kwolf(a)redhat.com> writes:
Am 12.11.2020 um 09:22 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf(a)redhat.com> writes:
>
> > 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(a)redhat.com>
>
> I'd call that a bug.
Which is why I'm fixing it, separate from the deprecation.
Thank you :)
> In the light of PATCH 2, I propose to put that one first (with
the
> help_string_append() hunk dropped), then remove the aliases from CLI
> help, too, like this: [...]
Going one step back without thinking in solutions immediately, what
you're suggesting is that deprecated options should become undocumented
instead of just annotated as deprecated?
"Should become" is open to debate. "Have become" is a fact:
3478eae990 "qemu-options: Deprecate -nodefconfig", 2017-10-09
80f52a6694 "Deprecate -M command line options", 2011-07-23
and more.
I don't see the why *help* output should mention deprecated options.
It's distracting. Tell me how to use this thing, not how I could use
it, but really shouldn't.
Mentioning them in the reference manual may have value.
Covering them in deprecated.rst is of course mandatory.
2:08 p.m.
New subject: [PATCH 2/2] char: Deprecate backend aliases 'tty' and 'parport'
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(a)redhat.com>
---
docs/system/deprecated.rst | 6 ++++++
chardev/char.c | 11 ++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index bbaae0d97c..7e313eae4f 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -81,6 +81,12 @@ error in the future.
The ``-realtime mlock=on|off`` argument has been replaced by the
``-overcommit mem-lock=on|off`` argument.
+``-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 c406e61db6..f9e297185d 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" },
@@ -585,6 +586,9 @@ help_string_append(const char *name, bool is_cli_alias, void *opaque)
GString *str = opaque;
g_string_append_printf(str, "\n %s", name);
+ if (is_cli_alias) {
+ g_string_append(str, " (deprecated)");
+ }
}
static const char *chardev_alias_translate(const char *name)
@@ -592,6 +596,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.28.0
2:22 p.m.
On 11/11/20 14:08, Kevin Wolf wrote:
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.
In the deprecation documentation I assumed that this is for 6.0, but if
we want to include it in 5.2 still, this can be changed, of course.
Kevin Wolf (2):
char: Skip CLI aliases in query-chardev-backends
char: Deprecate backend aliases 'tty' and 'parport'
docs/system/deprecated.rst | 6 ++++++
chardev/char.c | 32 ++++++++++++++++++++++++--------
2 files changed, 30 insertions(+), 8 deletions(-)
Even though I disagree with QAPIfying -chardev, this one is obviously a
good thing.
Acked-by: Paolo Bonzini <pbonzini(a)redhat.com>
Paolo
1406
days inactive
1408
days old
6 comments
3 participants
participants (3)
-
Kevin Wolf -
Markus Armbruster -
Paolo Bonzini