3:45 p.m.
We are trying to unify all qemu-system-FOO to a single binary.
In order to do that we need to remove QAPI target specific code.
@dump-skeys is only available on qemu-system-s390x. This series
rename it as @dump-s390-skey, making it available on other
binaries. We take care of backward compatibility via deprecation.
Philippe Mathieu-Daudé (4):
hw/s390x: Introduce the @dump-s390-skeys QMP command
hw/s390x: Introduce the 'dump_s390_skeys' HMP command
hw/s390x: Deprecate the HMP 'dump_skeys' command
hw/s390x: Deprecate the QMP @dump-skeys command
docs/about/deprecated.rst | 5 +++++
qapi/misc-target.json | 5 +++++
qapi/misc.json | 18 ++++++++++++++++++
include/monitor/hmp.h | 1 +
hw/s390x/s390-skeys-stub.c | 24 ++++++++++++++++++++++++
hw/s390x/s390-skeys.c | 19 +++++++++++++++++--
hmp-commands.hx | 17 +++++++++++++++--
hw/s390x/meson.build | 5 +++++
8 files changed, 90 insertions(+), 4 deletions(-)
create mode 100644 hw/s390x/s390-skeys-stub.c
--
2.41.0
3:45 p.m.
New subject: [PATCH 1/4] hw/s390x: Introduce the @dump-s390-skeys QMP command
@dump-skeys is specific to the qemu-system-s390x binary.
In order to provide it in an unified single binary, add
a equivalent new command named @dump-s390-skeys, which
works identically on s390x and reports an error on other
targets.
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
qapi/misc.json | 18 ++++++++++++++++++
hw/s390x/s390-skeys-stub.c | 16 ++++++++++++++++
hw/s390x/s390-skeys.c | 8 +++++++-
hw/s390x/meson.build | 5 +++++
4 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 hw/s390x/s390-skeys-stub.c
diff --git a/qapi/misc.json b/qapi/misc.json
index ec30e5c570..d192dd1bef 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -605,3 +605,21 @@
{ 'event': 'VFU_CLIENT_HANGUP',
'data': { 'vfu-id': 'str', 'vfu-qom-path':
'str',
'dev-id': 'str', 'dev-qom-path': 'str' } }
+
+##
+# @dump-s390-skeys:
+#
+# Dump guest's storage keys
+#
+# @filename: the path to the file to dump to
+#
+# Since: 9.1
+#
+# Example:
+#
+# -> { "execute": "dump-skeys",
+# "arguments": { "filename": "/tmp/skeys" } }
+# <- { "return": {} }
+##
+{ 'command': 'dump-s390-skeys',
+ 'data': { 'filename': 'str' } }
diff --git a/hw/s390x/s390-skeys-stub.c b/hw/s390x/s390-skeys-stub.c
new file mode 100644
index 0000000000..50b5f83437
--- /dev/null
+++ b/hw/s390x/s390-skeys-stub.c
@@ -0,0 +1,16 @@
+/*
+ * s390 storage key device stubs
+ *
+ * SPDX-FileContributor: Philippe Mathieu-Daudé <philmd(a)linaro.org>
+ * SPDX-FileCopyrightText: 2023 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc.h"
+
+void qmp_dump_s390_skeys(const char *filename, Error **errp)
+{
+ error_setg(errp, "This guest is not using s390 storage keys - "
+ "nothing to dump");
+}
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 5c535d483e..f9bb08eb92 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -15,6 +15,7 @@
#include "hw/qdev-properties.h"
#include "hw/s390x/storage-keys.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-misc-target.h"
#include "qapi/qmp/qdict.h"
#include "qemu/error-report.h"
@@ -115,7 +116,7 @@ void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
}
}
-void qmp_dump_skeys(const char *filename, Error **errp)
+void qmp_dump_s390_skeys(const char *filename, Error **errp)
{
S390SKeysState *ss = s390_get_skeys_device();
S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
@@ -192,6 +193,11 @@ out:
fclose(f);
}
+void qmp_dump_skeys(const char *filename, Error **errp)
+{
+ qmp_dump_s390_skeys(filename, errp);
+}
+
static bool qemu_s390_skeys_are_enabled(S390SKeysState *ss)
{
QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(ss);
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 482fd13420..c94b8d819d 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -50,6 +50,11 @@ virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true:
files('vhost-vsock-ccw.c'))
virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true:
files('vhost-user-fs-ccw.c'))
s390x_ss.add_all(when: 'CONFIG_VIRTIO_CCW', if_true: virtio_ss)
+s390x_stub_ss = ss.source_set()
+s390x_stub_ss.add(when: 'TARGET_S390X', if_false:
files('s390-skeys-stub.c'))
+
+specific_ss.add_all(s390x_stub_ss)
+
hw_arch += {'s390x': s390x_ss}
hw_s390x_modules = {}
--
2.41.0
3:45 p.m.
New subject: [PATCH 2/4] hw/s390x: Introduce the 'dump_s390_skeys' HMP command
'dump_skeys' is specific to the qemu-system-s390x binary.
In order to provide it in an unified single binary, add
a equivalent new command named 'dump_s390_skeys', which
works identically on s390x and reports an error on other
targets.
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
include/monitor/hmp.h | 1 +
hw/s390x/s390-skeys-stub.c | 8 ++++++++
hw/s390x/s390-skeys.c | 9 ++++++++-
hmp-commands.hx | 13 +++++++++++++
4 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 954f3c83ad..647a1e4a3f 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -181,5 +181,6 @@ void hmp_boot_set(Monitor *mon, const QDict *qdict);
void hmp_info_mtree(Monitor *mon, const QDict *qdict);
void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
+void hmp_dump_s390_skeys(Monitor *mon, const QDict *qdict);
#endif
diff --git a/hw/s390x/s390-skeys-stub.c b/hw/s390x/s390-skeys-stub.c
index 50b5f83437..94b491425b 100644
--- a/hw/s390x/s390-skeys-stub.c
+++ b/hw/s390x/s390-skeys-stub.c
@@ -8,6 +8,14 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-misc.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
+
+void hmp_dump_s390_skeys(Monitor *mon, const QDict *qdict)
+{
+ monitor_printf(mon, "This guest is not using s390 storage keys - "
+ "nothing to dump\n");
+}
void qmp_dump_s390_skeys(const char *filename, Error **errp)
{
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index f9bb08eb92..e7dab52a6e 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -24,6 +24,8 @@
#include "sysemu/kvm.h"
#include "migration/qemu-file-types.h"
#include "migration/register.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */
#define S390_SKEYS_SAVE_FLAG_EOS 0x01
@@ -105,7 +107,7 @@ void hmp_info_skeys(Monitor *mon, const QDict *qdict)
monitor_printf(mon, " key: 0x%X\n", key);
}
-void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
+void hmp_dump_s390_skeys(Monitor *mon, const QDict *qdict)
{
const char *filename = qdict_get_str(qdict, "filename");
Error *err = NULL;
@@ -116,6 +118,11 @@ void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
}
}
+void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
+{
+ hmp_dump_s390_skeys(mon, qdict);
+}
+
void qmp_dump_s390_skeys(const char *filename, Error **errp)
{
S390SKeysState *ss = s390_get_skeys_device();
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 06746f0afc..c12e2c2bd9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1120,6 +1120,19 @@ SRST
ERST
+ {
+ .name = "dump-s390-skeys",
+ .args_type = "filename:F",
+ .params = "",
+ .help = "Save guest storage keys into file 'filename'.",
+ .cmd = hmp_dump_s390_skeys,
+ },
+
+SRST
+``dump-s390-skeys`` *filename*
+ Save guest storage keys to a file.
+ERST
+
#if defined(TARGET_S390X)
{
.name = "dump-skeys",
--
2.41.0
3:45 p.m.
New subject: [PATCH 3/4] hw/s390x: Deprecate the HMP 'dump_skeys' command
Prefer 'dump_s390_skeys' which is target agnostic.
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
hw/s390x/s390-skeys.c | 2 ++
hmp-commands.hx | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index e7dab52a6e..12ec8a808e 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -120,6 +120,8 @@ void hmp_dump_s390_skeys(Monitor *mon, const QDict *qdict)
void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
{
+ monitor_printf(mon, "This command is deprecated"
+ " (replaced by 'dump-s390-skeys')\n");
hmp_dump_s390_skeys(mon, qdict);
}
diff --git a/hmp-commands.hx b/hmp-commands.hx
index c12e2c2bd9..04ae897134 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1138,14 +1138,14 @@ ERST
.name = "dump-skeys",
.args_type = "filename:F",
.params = "",
- .help = "Save guest storage keys into file
'filename'.\n",
+ .help = "deprecated synonym for dump-s390-skeys.",
.cmd = hmp_dump_skeys,
},
#endif
SRST
``dump-skeys`` *filename*
- Save guest storage keys to a file.
+ This is a deprecated synonym for the dump-s390-skeys command.
ERST
#if defined(TARGET_S390X)
--
2.41.0
3:45 p.m.
New subject: [PATCH 4/4] hw/s390x: Deprecate the QMP @dump-skeys command
Prefer @dump-s390-skeys which is target agnostic.
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
docs/about/deprecated.rst | 5 +++++
qapi/misc-target.json | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 40585ca7d5..3cb43085ba 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -148,6 +148,11 @@ accepted incorrect commands will return an error. Users should make
sure that
all arguments passed to ``device_add`` are consistent with the documented
property types.
+``dump-skeys`` (since 9.1)
+''''''''''''''''''''''''''
+
+Use the more generic ``dump-s390-skeys`` command.
+
QEMU Machine Protocol (QMP) events
----------------------------------
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4e0a6492a9..e5109b1265 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -237,6 +237,10 @@
#
# @filename: the path to the file to dump to
#
+# Features:
+#
+# @deprecated: This command is deprecated. Use @dump-s390-skeys instead.
+#
# Since: 2.5
#
# Example:
@@ -247,6 +251,7 @@
##
{ 'command': 'dump-skeys',
'data': { 'filename': 'str' },
+ 'features': ['deprecated'],
'if': 'TARGET_S390X' }
##
--
2.41.0
12:49 p.m.
New subject: [PATCH 4/4] hw/s390x: Deprecate the QMP @dump-skeys command
On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
Prefer @dump-s390-skeys which is target agnostic.
Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org>
---
docs/about/deprecated.rst | 5 +++++
qapi/misc-target.json | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 40585ca7d5..3cb43085ba 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -148,6 +148,11 @@ accepted incorrect commands will return an error. Users should make
sure that
all arguments passed to ``device_add`` are consistent with the documented
property types.
+``dump-skeys`` (since 9.1)
+''''''''''''''''''''''''''
+
+Use the more generic ``dump-s390-skeys`` command.
FWIW, that's "more specific", not "more generic".
But as I said in my reply to the cover letter, we should maybe consider a
more generic command instead, indeed.
Thomas
5:13 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
On 30/05/24, Philippe Mathieu-Daudé wrote:
We are trying to unify all qemu-system-FOO to a single binary.
In order to do that we need to remove QAPI target specific code.
@dump-skeys is only available on qemu-system-s390x. This series
rename it as @dump-s390-skey, making it available on other
binaries. We take care of backward compatibility via deprecation.
Philippe Mathieu-Daudé (4):
hw/s390x: Introduce the @dump-s390-skeys QMP command
hw/s390x: Introduce the 'dump_s390_skeys' HMP command
hw/s390x: Deprecate the HMP 'dump_skeys' command
hw/s390x: Deprecate the QMP @dump-skeys command
Series:
Reviewed-by: Anton Johansson <anjo(a)rev.ng>
12:47 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
We are trying to unify all qemu-system-FOO to a single binary.
In order to do that we need to remove QAPI target specific code.
@dump-skeys is only available on qemu-system-s390x. This series
rename it as @dump-s390-skey, making it available on other
binaries. We take care of backward compatibility via deprecation.
Philippe Mathieu-Daudé (4):
hw/s390x: Introduce the @dump-s390-skeys QMP command
hw/s390x: Introduce the 'dump_s390_skeys' HMP command
hw/s390x: Deprecate the HMP 'dump_skeys' command
hw/s390x: Deprecate the QMP @dump-skeys command
Why do we have to rename the command? Just for the sake of it? I think
renaming HMP commands is maybe ok, but breaking the API in QMP is something
you should consider twice.
And even if we decide to rename ... maybe we should discuss whether it makes
sense to come up with a generic command instead: As far as I know, ARM also
has something similar, called MTE. Maybe we also want to dump MTE keys one
day? So the new command should maybe be called "dump-memory-keys" instead?
Or should it maybe rather be an option to the existing "dump-guest-memory"
command instead?
Thomas
10:02 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
* Thomas Huth (thuth(a)redhat.com) wrote:
On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> We are trying to unify all qemu-system-FOO to a single binary.
> In order to do that we need to remove QAPI target specific code.
>
> @dump-skeys is only available on qemu-system-s390x. This series
> rename it as @dump-s390-skey, making it available on other
> binaries. We take care of backward compatibility via deprecation.
>
> Philippe Mathieu-Daudé (4):
> hw/s390x: Introduce the @dump-s390-skeys QMP command
> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> hw/s390x: Deprecate the HMP 'dump_skeys' command
> hw/s390x: Deprecate the QMP @dump-skeys command
Why do we have to rename the command? Just for the sake of it? I think
renaming HMP commands is maybe ok, but breaking the API in QMP is something
you should consider twice.
And even if we decide to rename ... maybe we should discuss whether it makes
sense to come up with a generic command instead: As far as I know, ARM also
has something similar, called MTE. Maybe we also want to dump MTE keys one
day? So the new command should maybe be called "dump-memory-keys" instead?
I think there are at least two different concepts; but I agree it would be
nice to keep a single command for matching concepts across different architectures;
I can't say I know the details of any, but:
a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing
where pages marked a special way are associated with keys.
That sounds similar to what the skeys are???
b) Upper bit things - where you steal a few bits from the virtual address
and then use that to associate some security; I think that's closer
to what MTE is isn't it?
I'm not sure the two fit in the same command.
Dave
Or should it maybe rather be an option to the existing
"dump-guest-memory"
command instead?
Thomas
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
12:23 a.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
On 31/05/2024 16.02, Dr. David Alan Gilbert wrote:
* Thomas Huth (thuth(a)redhat.com) wrote:
> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>> We are trying to unify all qemu-system-FOO to a single binary.
>> In order to do that we need to remove QAPI target specific code.
>>
>> @dump-skeys is only available on qemu-system-s390x. This series
>> rename it as @dump-s390-skey, making it available on other
>> binaries. We take care of backward compatibility via deprecation.
>>
>> Philippe Mathieu-Daudé (4):
>> hw/s390x: Introduce the @dump-s390-skeys QMP command
>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
>> hw/s390x: Deprecate the HMP 'dump_skeys' command
>> hw/s390x: Deprecate the QMP @dump-skeys command
>
> Why do we have to rename the command? Just for the sake of it? I think
> renaming HMP commands is maybe ok, but breaking the API in QMP is something
> you should consider twice.
>
> And even if we decide to rename ... maybe we should discuss whether it makes
> sense to come up with a generic command instead: As far as I know, ARM also
> has something similar, called MTE. Maybe we also want to dump MTE keys one
> day? So the new command should maybe be called "dump-memory-keys" instead?
I think there are at least two different concepts; but I agree it would be
nice to keep a single command for matching concepts across different architectures;
I can't say I know the details of any, but:
a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing
where pages marked a special way are associated with keys.
That sounds similar to what the skeys are???
Sounds a little bit similar, but s390 storage keys are independent from page
tables. It's rather that each page (4096 bytes) of RAM has an additional
7-bit value that contains the storage key and some additional bits. It's
also usable when page tables are still disabled.
I'm not sure the two fit in the same command.
Does it make sense to dump all the MTE or x86 keys all at once? If so, we
could maybe come up with an unified command. Otherwise it might not make
sense, indeed.
Thomas
1:06 a.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
* Thomas Huth (thuth(a)redhat.com) wrote:
On 31/05/2024 16.02, Dr. David Alan Gilbert wrote:
> * Thomas Huth (thuth(a)redhat.com) wrote:
> > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > > We are trying to unify all qemu-system-FOO to a single binary.
> > > In order to do that we need to remove QAPI target specific code.
> > >
> > > @dump-skeys is only available on qemu-system-s390x. This series
> > > rename it as @dump-s390-skey, making it available on other
> > > binaries. We take care of backward compatibility via deprecation.
> > >
> > > Philippe Mathieu-Daudé (4):
> > > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > > hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> > > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > > hw/s390x: Deprecate the QMP @dump-skeys command
> >
> > Why do we have to rename the command? Just for the sake of it? I think
> > renaming HMP commands is maybe ok, but breaking the API in QMP is something
> > you should consider twice.
> >
> > And even if we decide to rename ... maybe we should discuss whether it makes
> > sense to come up with a generic command instead: As far as I know, ARM also
> > has something similar, called MTE. Maybe we also want to dump MTE keys one
> > day? So the new command should maybe be called "dump-memory-keys"
instead?
>
> I think there are at least two different concepts; but I agree it would be
> nice to keep a single command for matching concepts across different architectures;
> I can't say I know the details of any, but:
>
> a) Page table things - I think x86 PKRU/PKEY (???) is a page table thing
> where pages marked a special way are associated with keys.
> That sounds similar to what the skeys are???
Sounds a little bit similar, but s390 storage keys are independent from page
tables. It's rather that each page (4096 bytes) of RAM has an additional
7-bit value that contains the storage key and some additional bits. It's
also usable when page tables are still disabled.
> I'm not sure the two fit in the same command.
Does it make sense to dump all the MTE or x86 keys all at once? If so, we
could maybe come up with an unified command. Otherwise it might not make
sense, indeed.
So they're all really different granularities:
s390 - one key/physical page
ARM MTE - one tag/16 bytes physical
x86 - per virtual page; then a current register with 16 permission
sets
For x86 I guess it would be easy to dump the register, and then the
values for a range of virtual addresses.
But then if you dumped a range of physical addresses on ARM MTE you'd
get hundreds times more output than for the equivalent s390.
Dave
Thomas
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
8:18 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> We are trying to unify all qemu-system-FOO to a single binary.
> In order to do that we need to remove QAPI target specific code.
>
> @dump-skeys is only available on qemu-system-s390x. This series
> rename it as @dump-s390-skey, making it available on other
> binaries. We take care of backward compatibility via deprecation.
>
> Philippe Mathieu-Daudé (4):
> hw/s390x: Introduce the @dump-s390-skeys QMP command
> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> hw/s390x: Deprecate the HMP 'dump_skeys' command
> hw/s390x: Deprecate the QMP @dump-skeys command
Why do we have to rename the command? Just for the sake of it? I think
renaming HMP commands is maybe ok, but breaking the API in QMP is something
you should consider twice.
That was going to be my question too. Seems like its possible to simply
stub out the existing command for other targets.
The renaming is just window dressing.
And even if we decide to rename ... maybe we should discuss whether it makes
sense to come up with a generic command instead: As far as I know, ARM also
has something similar, called MTE. Maybe we also want to dump MTE keys one
day? So the new command should maybe be called "dump-memory-keys" instead?
Or should it maybe rather be an option to the existing "dump-guest-memory"
command instead?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:54 a.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
* Daniel P. Berrangé (berrange(a)redhat.com) wrote:
On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > We are trying to unify all qemu-system-FOO to a single binary.
> > In order to do that we need to remove QAPI target specific code.
> >
> > @dump-skeys is only available on qemu-system-s390x. This series
> > rename it as @dump-s390-skey, making it available on other
> > binaries. We take care of backward compatibility via deprecation.
> >
> > Philippe Mathieu-Daudé (4):
> > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > hw/s390x: Deprecate the QMP @dump-skeys command
>
> Why do we have to rename the command? Just for the sake of it? I think
> renaming HMP commands is maybe ok, but breaking the API in QMP is something
> you should consider twice.
That was going to be my question too. Seems like its possible to simply
stub out the existing command for other targets.
Are these commands really supposed to be stable, or are they just debug
commands? If they are debug, then add the x- and don't worry too much.
Dave
The renaming is just window dressing.
>
> And even if we decide to rename ... maybe we should discuss whether it makes
> sense to come up with a generic command instead: As far as I know, ARM also
> has something similar, called MTE. Maybe we also want to dump MTE keys one
> day? So the new command should maybe be called "dump-memory-keys"
instead?
> Or should it maybe rather be an option to the existing
"dump-guest-memory"
> command instead?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
12:58 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
"Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
* Daniel P. Berrangé (berrange(a)redhat.com) wrote:
> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
> > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > > We are trying to unify all qemu-system-FOO to a single binary.
> > > In order to do that we need to remove QAPI target specific code.
> > >
> > > @dump-skeys is only available on qemu-system-s390x. This series
> > > rename it as @dump-s390-skey, making it available on other
> > > binaries. We take care of backward compatibility via deprecation.
> > >
> > > Philippe Mathieu-Daudé (4):
> > > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > > hw/s390x: Introduce the 'dump_s390_skeys' HMP command
> > > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > > hw/s390x: Deprecate the QMP @dump-skeys command
> >
> > Why do we have to rename the command? Just for the sake of it? I think
> > renaming HMP commands is maybe ok, but breaking the API in QMP is something
> > you should consider twice.
PRO rename: the command's tie to S390 is them immediately obvious, which
may be useful when the command becomes available in qemu-systems capable
of running other targets.
CON rename: users need to adapt.
What are the users? Not libvirt, as far as I can tell.
> That was going to be my question too. Seems like its possible to
simply
> stub out the existing command for other targets.
That's going to happen whether we rename the commands or not.
Are these commands really supposed to be stable, or are they just
debug
commands? If they are debug, then add the x- and don't worry too much.
docs/devel/qapi-code-gen.rst:
Names beginning with ``x-`` used to signify "experimental". This
convention has been replaced by special feature "unstable".
Feature "unstable" is what makes something unstable, and is what
machines should check.
An "x-" prefix may still be useful for humans. Machines should *not*
key on the prefix. It's unreliable anyway: InputBarrierProperties
member @x-origin is stable despite it's name. Renames to gain or lose
the prefix may or may not be worth the bother.
Making an existing part of the interface unstable should be treated
similar to deprecating it: we keep it stable for at least the
deprecation period. Not written policy, because we didn't consider such
changes when we documented our deprecation policy in
docs/about/deprecated.rst.
Alternative path to a renamed command (*if* we want that):
1. Make it unstable.
2. But keep it stable for two releases.
3. Rename.
[...]
5:45 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
Hi Daniel, Dave, Markus & Thomas.
On 4/6/24 06:58, Markus Armbruster wrote:
"Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>>>> We are trying to unify all qemu-system-FOO to a single binary.
>>>> In order to do that we need to remove QAPI target specific code.
>>>>
>>>> @dump-skeys is only available on qemu-system-s390x. This series
>>>> rename it as @dump-s390-skey, making it available on other
>>>> binaries. We take care of backward compatibility via deprecation.
>>>>
>>>> Philippe Mathieu-Daudé (4):
>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command
>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command
>>>> hw/s390x: Deprecate the QMP @dump-skeys command
>>>
>>> Why do we have to rename the command? Just for the sake of it? I think
>>> renaming HMP commands is maybe ok, but breaking the API in QMP is something
>>> you should consider twice.
I'm looking at how to include this command in the new "single binary".
Markus explained in an earlier series, just expanding this command as
stub to targets that don't implement it is not backward compatible and
breaks QMP introspection. Currently on s390x we get a result, on other
targets the command doesn't exist. If we add a stubs, then other targets
return something (even if it is an empty list), confusing management
interface.
So this approach use to deprecate process to include a new command
which behaves differently on non-s390x targets.
If we don't care for this particular case, better. However I'd still
like to discuss this approach for other target-specific commands.
PRO rename: the command's tie to S390 is them immediately
obvious, which
may be useful when the command becomes available in qemu-systems capable
of running other targets.
CON rename: users need to adapt.
What are the users? Not libvirt, as far as I can tell.
Years ago we said, "all HMP must be based on QMP". Now we realize HMP
became stable because QMP-exposed, although not consumed externally...
Does the concept of "internal QMP commands" makes sense for HMP debug
ones? (Looking at a way to not expose them). We could use the "x-"
prefix to not care about stable / backward compat, but what is the point
of exposing to QMP commands that will never be accessed there?
>> That was going to be my question too. Seems like its possible
to simply
>> stub out the existing command for other targets.
That's going to happen whether we rename the commands or not.
> Are these commands really supposed to be stable, or are they just debug
> commands? If they are debug, then add the x- and don't worry too much.
OK.
docs/devel/qapi-code-gen.rst:
Names beginning with ``x-`` used to signify "experimental". This
convention has been replaced by special feature "unstable".
Feature "unstable" is what makes something unstable, and is what
machines should check.
What I mentioned earlier could be 'Feature "internal" or
"debug"'.
An "x-" prefix may still be useful for humans. Machines
should *not*
key on the prefix. It's unreliable anyway: InputBarrierProperties
member @x-origin is stable despite it's name. Renames to gain or lose
the prefix may or may not be worth the bother.
Could follow the rules and be renamed as "origin-coordinate-x".
Making an existing part of the interface unstable should be treated
similar to deprecating it: we keep it stable for at least the
deprecation period. Not written policy, because we didn't consider such
changes when we documented our deprecation policy in
docs/about/deprecated.rst.
Alternative path to a renamed command (*if* we want that):
1. Make it unstable.
2. But keep it stable for two releases.
3. Rename.
[...]
5:59 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
On Tue, Jun 04, 2024 at 11:45:11AM +0200, Philippe Mathieu-Daudé wrote:
Hi Daniel, Dave, Markus & Thomas.
On 4/6/24 06:58, Markus Armbruster wrote:
> "Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
> > * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
> > > On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
> > > > On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> > > > > We are trying to unify all qemu-system-FOO to a single binary.
> > > > > In order to do that we need to remove QAPI target specific
code.
> > > > >
> > > > > @dump-skeys is only available on qemu-system-s390x. This series
> > > > > rename it as @dump-s390-skey, making it available on other
> > > > > binaries. We take care of backward compatibility via
deprecation.
> > > > >
> > > > > Philippe Mathieu-Daudé (4):
> > > > > hw/s390x: Introduce the @dump-s390-skeys QMP command
> > > > > hw/s390x: Introduce the 'dump_s390_skeys' HMP
command
> > > > > hw/s390x: Deprecate the HMP 'dump_skeys' command
> > > > > hw/s390x: Deprecate the QMP @dump-skeys command
> > > >
> > > > Why do we have to rename the command? Just for the sake of it? I
think
> > > > renaming HMP commands is maybe ok, but breaking the API in QMP is
something
> > > > you should consider twice.
I'm looking at how to include this command in the new "single binary".
Markus explained in an earlier series, just expanding this command as
stub to targets that don't implement it is not backward compatible and
breaks QMP introspection. Currently on s390x we get a result, on other
targets the command doesn't exist. If we add a stubs, then other targets
return something (even if it is an empty list), confusing management
interface.
I'm not convinced about calling this "not backward compatible".
We're always free to add new commands, and there's never any
guarantee that you can execute a given command under every
possible QEMU configuration.
IOW, adding 'dump-skeys' and having it always return an error
on non-s390x targets is valid IMHO, and I wouldn't call that
a backwards compatibilit problem.
Apps shouldn't assume that just because a command is reported
in introspection, it can be used in any scenario. An app is
expected to itself understand the behaviour of any given command
sufficiently well, to know when they can execute it, or be prpared
to accept errors.
> PRO rename: the command's tie to S390 is them immediately
obvious, which
> may be useful when the command becomes available in qemu-systems capable
> of running other targets.
>
> CON rename: users need to adapt.
>
> What are the users? Not libvirt, as far as I can tell.
Years ago we said, "all HMP must be based on QMP". Now we realize HMP
became stable because QMP-exposed, although not consumed externally...
That's not the case though. We've added plenty of conversions of
HMP to QMP, while declaring them unstable. We might have missed
that in some cases, but that's just a bug, and we can fix that
any time by adding the 'unstable' feature tag to the schema.
Does the concept of "internal QMP commands" makes sense for
HMP debug
ones? (Looking at a way to not expose them). We could use the "x-"
prefix to not care about stable / backward compat, but what is the point
of exposing to QMP commands that will never be accessed there?
Debug only, unstable commands are totally valid for QMP. There's nothing
that requires them to be inherantly HMP only.
QAPI modelling can benefit many debug only, unstable commands, but we
can also still just return printf formatted strings as documented here:
https://www.qemu.org/docs/master/devel/writing-monitor-commands.html#writ...
I remain convinced that we should eventually delete the HMP impl as it
exists today, and just provide a QMP client whose interactive interface
matches what HMP provides today. There's no compelling need for HMP to
run inside QEMU, if QMP exposes all the required functionality.
> docs/devel/qapi-code-gen.rst:
>
> Names beginning with ``x-`` used to signify "experimental". This
> convention has been replaced by special feature "unstable".
>
> Feature "unstable" is what makes something unstable, and is what
> machines should check.
What I mentioned earlier could be 'Feature "internal" or
"debug"'.
IMHO we don't need to invent new terms for this.
"unstable" is sufficient as it expresses that the command's
inputs and outputs are liable to change their format, which
is the case for most of the historical debug only commands.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
Philippe Mathieu-Daudé <philmd(a)linaro.org> writes:
Hi Daniel, Dave, Markus & Thomas.
On 4/6/24 06:58, Markus Armbruster wrote:
> "Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
>> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>>>>> We are trying to unify all qemu-system-FOO to a single binary.
>>>>> In order to do that we need to remove QAPI target specific code.
>>>>>
>>>>> @dump-skeys is only available on qemu-system-s390x. This series
>>>>> rename it as @dump-s390-skey, making it available on other
>>>>> binaries. We take care of backward compatibility via deprecation.
>>>>>
>>>>> Philippe Mathieu-Daudé (4):
>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command
>>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
>>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command
>>>>> hw/s390x: Deprecate the QMP @dump-skeys command
>>>>
>>>> Why do we have to rename the command? Just for the sake of it? I think
>>>> renaming HMP commands is maybe ok, but breaking the API in QMP is
something
>>>> you should consider twice.
I'm looking at how to include this command in the new "single binary".
Markus explained in an earlier series, just expanding this command as
stub to targets that don't implement it is not backward compatible and
breaks QMP introspection. Currently on s390x we get a result, on other
targets the command doesn't exist. If we add a stubs, then other targets
return something (even if it is an empty list), confusing management
interface.
Loss of introspection precision is a concern, not a hard "no".
We weigh all the concerns, and pick a solution we hate the least :)
So this approach use to deprecate process to include a new command
which behaves differently on non-s390x targets.
If we don't care for this particular case, better. However I'd still
like to discuss this approach for other target-specific commands.
> PRO rename: the command's tie to S390 is them immediately obvious, which
> may be useful when the command becomes available in qemu-systems capable
> of running other targets.
>
> CON rename: users need to adapt.
>
> What are the users? Not libvirt, as far as I can tell.
Years ago we said, "all HMP must be based on QMP".
In practice, it's closer to "HMP must be base on QMP when the
functionality does or should exist in QMP."
Now we realize
HMP
became stable because QMP-exposed, although not consumed externally...
I'm afraid I didn't get this part.
Does the concept of "internal QMP commands" makes sense for
HMP debug
ones? (Looking at a way to not expose them). We could use the "x-"
prefix to not care about stable / backward compat, but what is the point
of exposing to QMP commands that will never be accessed there?
>>> That was going to be my question too. Seems like its possible to simply
>>> stub out the existing command for other targets.
>
> That's going to happen whether we rename the commands or not.
>
>> Are these commands really supposed to be stable, or are they just debug
>> commands? If they are debug, then add the x- and don't worry too much.
OK.
> docs/devel/qapi-code-gen.rst:
>
> Names beginning with ``x-`` used to signify "experimental". This
> convention has been replaced by special feature "unstable".
>
> Feature "unstable" is what makes something unstable, and is what
> machines should check.
What I mentioned earlier could be 'Feature "internal" or
"debug"'.
What's the difference to "unstable"?
> An "x-" prefix may still be useful for humans.
Machines should *not*
> key on the prefix. It's unreliable anyway: InputBarrierProperties
> member @x-origin is stable despite it's name. Renames to gain or lose
> the prefix may or may not be worth the bother.
Could follow the rules and be renamed as "origin-coordinate-x".
I don't think it's worth the trouble. The "x-" prefix is now strictly
for humans, and humans can figure out what the x- in @x-origin,
@y-origin means.
[...]
7:44 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
* Markus Armbruster (armbru(a)redhat.com) wrote:
Philippe Mathieu-Daudé <philmd(a)linaro.org> writes:
> Hi Daniel, Dave, Markus & Thomas.
>
> On 4/6/24 06:58, Markus Armbruster wrote:
>> "Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
>>> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
>>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
>>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>>>>>> We are trying to unify all qemu-system-FOO to a single binary.
>>>>>> In order to do that we need to remove QAPI target specific
code.
>>>>>>
>>>>>> @dump-skeys is only available on qemu-system-s390x. This series
>>>>>> rename it as @dump-s390-skey, making it available on other
>>>>>> binaries. We take care of backward compatibility via
deprecation.
>>>>>>
>>>>>> Philippe Mathieu-Daudé (4):
>>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command
>>>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP
command
>>>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command
>>>>>> hw/s390x: Deprecate the QMP @dump-skeys command
>>>>>
>>>>> Why do we have to rename the command? Just for the sake of it? I
think
>>>>> renaming HMP commands is maybe ok, but breaking the API in QMP is
something
>>>>> you should consider twice.
>
> I'm looking at how to include this command in the new "single
binary".
>
> Markus explained in an earlier series, just expanding this command as
> stub to targets that don't implement it is not backward compatible and
> breaks QMP introspection. Currently on s390x we get a result, on other
> targets the command doesn't exist. If we add a stubs, then other targets
> return something (even if it is an empty list), confusing management
> interface.
Loss of introspection precision is a concern, not a hard "no".
We weigh all the concerns, and pick a solution we hate the least :)
> So this approach use to deprecate process to include a new command
> which behaves differently on non-s390x targets.
>
> If we don't care for this particular case, better. However I'd still
> like to discuss this approach for other target-specific commands.
>
>> PRO rename: the command's tie to S390 is them immediately obvious, which
>> may be useful when the command becomes available in qemu-systems capable
>> of running other targets.
>>
>> CON rename: users need to adapt.
>>
>> What are the users? Not libvirt, as far as I can tell.
>
> Years ago we said, "all HMP must be based on QMP".
In practice, it's closer to "HMP must be base on QMP when the
functionality does or should exist in QMP."
> Now we realize HMP
> became stable because QMP-exposed, although not consumed externally...
I'm afraid I didn't get this part.
> Does the concept of "internal QMP commands" makes sense for HMP debug
> ones? (Looking at a way to not expose them). We could use the "x-"
> prefix to not care about stable / backward compat, but what is the point
> of exposing to QMP commands that will never be accessed there?
>
>>>> That was going to be my question too. Seems like its possible to simply
>>>> stub out the existing command for other targets.
>>
>> That's going to happen whether we rename the commands or not.
>>
>>> Are these commands really supposed to be stable, or are they just debug
>>> commands? If they are debug, then add the x- and don't worry too much.
>
> OK.
>
>> docs/devel/qapi-code-gen.rst:
>>
>> Names beginning with ``x-`` used to signify "experimental".
This
>> convention has been replaced by special feature "unstable".
>>
>> Feature "unstable" is what makes something unstable, and is what
>> machines should check.
>
> What I mentioned earlier could be 'Feature "internal" or
"debug"'.
What's the difference to "unstable"?
It should be clear *why* something is marked x- - something that's
marked 'x-' because the feature is still in development is expected to shake
out at some point, and the interface designed so it can.
(and at some point the developer should get a prod to be asked whethere the
x- can be removed).
That's different from it permenantly being x- because it's expected to
change as the needs of the people debugging change.
Dave
>> An "x-" prefix may still be useful for humans.
Machines should *not*
>> key on the prefix. It's unreliable anyway: InputBarrierProperties
>> member @x-origin is stable despite it's name. Renames to gain or lose
>> the prefix may or may not be worth the bother.
>
> Could follow the rules and be renamed as "origin-coordinate-x".
I don't think it's worth the trouble. The "x-" prefix is now strictly
for humans, and humans can figure out what the x- in @x-origin,
@y-origin means.
[...]
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
1:20 p.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
"Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
* Markus Armbruster (armbru(a)redhat.com) wrote:
> Philippe Mathieu-Daudé <philmd(a)linaro.org> writes:
>
> > Hi Daniel, Dave, Markus & Thomas.
> >
> > On 4/6/24 06:58, Markus Armbruster wrote:
> >> "Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
> >>> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
> >>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
> >>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
> >>>>>> We are trying to unify all qemu-system-FOO to a single
binary.
> >>>>>> In order to do that we need to remove QAPI target specific
code.
> >>>>>>
> >>>>>> @dump-skeys is only available on qemu-system-s390x. This
series
> >>>>>> rename it as @dump-s390-skey, making it available on other
> >>>>>> binaries. We take care of backward compatibility via
deprecation.
> >>>>>>
> >>>>>> Philippe Mathieu-Daudé (4):
> >>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command
> >>>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP
command
> >>>>>> hw/s390x: Deprecate the HMP 'dump_skeys'
command
> >>>>>> hw/s390x: Deprecate the QMP @dump-skeys command
> >>>>>
> >>>>> Why do we have to rename the command? Just for the sake of it? I
think
> >>>>> renaming HMP commands is maybe ok, but breaking the API in QMP
is something
> >>>>> you should consider twice.
> >
> > I'm looking at how to include this command in the new "single
binary".
> >
> > Markus explained in an earlier series, just expanding this command as
> > stub to targets that don't implement it is not backward compatible and
> > breaks QMP introspection. Currently on s390x we get a result, on other
> > targets the command doesn't exist. If we add a stubs, then other targets
> > return something (even if it is an empty list), confusing management
> > interface.
>
> Loss of introspection precision is a concern, not a hard "no".
>
> We weigh all the concerns, and pick a solution we hate the least :)
>
> > So this approach use to deprecate process to include a new command
> > which behaves differently on non-s390x targets.
> >
> > If we don't care for this particular case, better. However I'd still
> > like to discuss this approach for other target-specific commands.
> >
> >> PRO rename: the command's tie to S390 is them immediately obvious,
which
> >> may be useful when the command becomes available in qemu-systems capable
> >> of running other targets.
> >>
> >> CON rename: users need to adapt.
> >>
> >> What are the users? Not libvirt, as far as I can tell.
> >
> > Years ago we said, "all HMP must be based on QMP".
>
> In practice, it's closer to "HMP must be base on QMP when the
> functionality does or should exist in QMP."
>
> > Now we realize HMP
> > became stable because QMP-exposed, although not consumed externally...
>
> I'm afraid I didn't get this part.
>
> > Does the concept of "internal QMP commands" makes sense for HMP debug
> > ones? (Looking at a way to not expose them). We could use the "x-"
> > prefix to not care about stable / backward compat, but what is the point
> > of exposing to QMP commands that will never be accessed there?
> >
> >>>> That was going to be my question too. Seems like its possible to
simply
> >>>> stub out the existing command for other targets.
> >>
> >> That's going to happen whether we rename the commands or not.
> >>
> >>> Are these commands really supposed to be stable, or are they just debug
> >>> commands? If they are debug, then add the x- and don't worry too
much.
> >
> > OK.
> >
> >> docs/devel/qapi-code-gen.rst:
> >>
> >> Names beginning with ``x-`` used to signify "experimental".
This
> >> convention has been replaced by special feature "unstable".
> >>
> >> Feature "unstable" is what makes something unstable, and is what
> >> machines should check.
> >
> > What I mentioned earlier could be 'Feature "internal" or
"debug"'.
>
> What's the difference to "unstable"?
It should be clear *why* something is marked x- - something that's
marked 'x-' because the feature is still in development is expected to shake
out at some point, and the interface designed so it can.
(and at some point the developer should get a prod to be asked whethere the
x- can be removed).
That's different from it permenantly being x- because it's expected to
change as the needs of the people debugging change.
When you add special feature 'unstable', the tooling insists you cover
it in the doc comment. Review should then ensure the doc comment
explains why it is unstable. Examples:
# @unstable: Member @x-perf is experimental.
# @unstable: This command is meant for debugging.
Dave
> >> An "x-" prefix may still be useful for humans. Machines should
*not*
> >> key on the prefix. It's unreliable anyway: InputBarrierProperties
> >> member @x-origin is stable despite it's name. Renames to gain or lose
> >> the prefix may or may not be worth the bother.
> >
> > Could follow the rules and be renamed as "origin-coordinate-x".
>
> I don't think it's worth the trouble. The "x-" prefix is now
strictly
> for humans, and humans can figure out what the x- in @x-origin,
> @y-origin means.
>
> [...]
>
12:46 a.m.
New subject: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and
deprecate
* Markus Armbruster (armbru(a)redhat.com) wrote:
"Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
> * Markus Armbruster (armbru(a)redhat.com) wrote:
>> Philippe Mathieu-Daudé <philmd(a)linaro.org> writes:
>>
>> > Hi Daniel, Dave, Markus & Thomas.
>> >
>> > On 4/6/24 06:58, Markus Armbruster wrote:
>> >> "Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
>> >>> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
>> >>>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
>> >>>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>> >>>>>> We are trying to unify all qemu-system-FOO to a single
binary.
>> >>>>>> In order to do that we need to remove QAPI target
specific code.
>> >>>>>>
>> >>>>>> @dump-skeys is only available on qemu-system-s390x.
This series
>> >>>>>> rename it as @dump-s390-skey, making it available on
other
>> >>>>>> binaries. We take care of backward compatibility via
deprecation.
>> >>>>>>
>> >>>>>> Philippe Mathieu-Daudé (4):
>> >>>>>> hw/s390x: Introduce the @dump-s390-skeys QMP
command
>> >>>>>> hw/s390x: Introduce the 'dump_s390_skeys'
HMP command
>> >>>>>> hw/s390x: Deprecate the HMP 'dump_skeys'
command
>> >>>>>> hw/s390x: Deprecate the QMP @dump-skeys command
>> >>>>>
>> >>>>> Why do we have to rename the command? Just for the sake of
it? I think
>> >>>>> renaming HMP commands is maybe ok, but breaking the API in
QMP is something
>> >>>>> you should consider twice.
>> >
>> > I'm looking at how to include this command in the new "single
binary".
>> >
>> > Markus explained in an earlier series, just expanding this command as
>> > stub to targets that don't implement it is not backward compatible and
>> > breaks QMP introspection. Currently on s390x we get a result, on other
>> > targets the command doesn't exist. If we add a stubs, then other
targets
>> > return something (even if it is an empty list), confusing management
>> > interface.
>>
>> Loss of introspection precision is a concern, not a hard "no".
>>
>> We weigh all the concerns, and pick a solution we hate the least :)
>>
>> > So this approach use to deprecate process to include a new command
>> > which behaves differently on non-s390x targets.
>> >
>> > If we don't care for this particular case, better. However I'd
still
>> > like to discuss this approach for other target-specific commands.
>> >
>> >> PRO rename: the command's tie to S390 is them immediately obvious,
which
>> >> may be useful when the command becomes available in qemu-systems
capable
>> >> of running other targets.
>> >>
>> >> CON rename: users need to adapt.
>> >>
>> >> What are the users? Not libvirt, as far as I can tell.
>> >
>> > Years ago we said, "all HMP must be based on QMP".
>>
>> In practice, it's closer to "HMP must be base on QMP when the
>> functionality does or should exist in QMP."
>>
>> > Now we realize HMP
>> > became stable because QMP-exposed, although not consumed externally...
>>
>> I'm afraid I didn't get this part.
>>
>> > Does the concept of "internal QMP commands" makes sense for HMP
debug
>> > ones? (Looking at a way to not expose them). We could use the
"x-"
>> > prefix to not care about stable / backward compat, but what is the point
>> > of exposing to QMP commands that will never be accessed there?
>> >
>> >>>> That was going to be my question too. Seems like its possible
to simply
>> >>>> stub out the existing command for other targets.
>> >>
>> >> That's going to happen whether we rename the commands or not.
>> >>
>> >>> Are these commands really supposed to be stable, or are they just
debug
>> >>> commands? If they are debug, then add the x- and don't worry
too much.
>> >
>> > OK.
>> >
>> >> docs/devel/qapi-code-gen.rst:
>> >>
>> >> Names beginning with ``x-`` used to signify
"experimental". This
>> >> convention has been replaced by special feature
"unstable".
>> >>
>> >> Feature "unstable" is what makes something unstable, and is
what
>> >> machines should check.
>> >
>> > What I mentioned earlier could be 'Feature "internal" or
"debug"'.
>>
>> What's the difference to "unstable"?
>
> It should be clear *why* something is marked x- - something that's
> marked 'x-' because the feature is still in development is expected to
shake
> out at some point, and the interface designed so it can.
> (and at some point the developer should get a prod to be asked whethere the
> x- can be removed).
> That's different from it permenantly being x- because it's expected to
> change as the needs of the people debugging change.
When you add special feature 'unstable', the tooling insists you cover
it in the doc comment. Review should then ensure the doc comment
explains why it is unstable. Examples:
# @unstable: Member @x-perf is experimental.
# @unstable: This command is meant for debugging.
OK, that makes some sense.
Dave
> Dave
>
>> >> An "x-" prefix may still be useful for humans. Machines
should *not*
>> >> key on the prefix. It's unreliable anyway: InputBarrierProperties
>> >> member @x-origin is stable despite it's name. Renames to gain or
lose
>> >> the prefix may or may not be worth the bother.
>> >
>> > Could follow the rules and be renamed as "origin-coordinate-x".
>>
>> I don't think it's worth the trouble. The "x-" prefix is now
strictly
>> for humans, and humans can figure out what the x- in @x-origin,
>> @y-origin means.
>>
>> [...]
>>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
72
days inactive
100
days old
19 comments
6 participants
participants (6)
-
Anton Johansson -
Daniel P. Berrangé -
Dr. David Alan Gilbert -
Markus Armbruster -
Philippe Mathieu-Daudé -
Thomas Huth