[PATCH 0/3] qapi/hmp/cli: Remove the deprecated 'singlestep'
Deprecated for 8.1, good to go for 9.0. Based-on: <20240112100059.965041-5-thuth@redhat.com> "Remove deprecated command line options" Philippe Mathieu-Daudé (3): qapi: Remove deprecated 'singlestep' member of StatusInfo hmp: Remove deprecated 'singlestep' command cli: Remove deprecated '-singlestep' command line option docs/about/deprecated.rst | 40 --------------------------------- docs/about/removed-features.rst | 24 ++++++++++++++++++++ docs/user/main.rst | 6 ----- qapi/run-state.json | 11 --------- bsd-user/main.c | 3 +-- linux-user/main.c | 2 -- system/runstate.c | 8 ------- system/vl.c | 18 +-------------- tests/qtest/test-hmp.c | 1 - hmp-commands.hx | 13 ----------- qemu-options.hx | 8 ------- tests/qemu-iotests/183.out | 4 ++-- tests/qemu-iotests/234.out | 8 +++---- tests/qemu-iotests/262.out | 4 ++-- tests/qemu-iotests/280.out | 2 +- 15 files changed, 35 insertions(+), 117 deletions(-) -- 2.41.0
This member has been deprecated before the 8.1 release, in commit 34c18203d4 ("qmp: Deprecate 'singlestep' member of StatusInfo"). Time to drop it. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 14 -------------- qapi/run-state.json | 11 ----------- system/runstate.c | 8 -------- tests/qemu-iotests/183.out | 4 ++-- tests/qemu-iotests/234.out | 8 ++++---- tests/qemu-iotests/262.out | 4 ++-- tests/qemu-iotests/280.out | 2 +- 7 files changed, 9 insertions(+), 42 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 9a2c994617..25527da423 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -151,20 +151,6 @@ 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. -``StatusInfo`` member ``singlestep`` (since 8.1) -'''''''''''''''''''''''''''''''''''''''''''''''' - -The ``singlestep`` member of the ``StatusInfo`` returned from the -``query-status`` command is deprecated. This member has a confusing -name and it never did what the documentation claimed or what its name -suggests. We do not believe that anybody is actually using the -information provided in this member. - -The information it reports is whether the TCG JIT is in "one -instruction per translated block" mode (which can be set on the -command line or via the HMP, but not via QMP). The information remains -available via the HMP 'info jit' command. - QEMU Machine Protocol (QMP) events ---------------------------------- diff --git a/qapi/run-state.json b/qapi/run-state.json index ca05502e0a..08bc99cb85 100644 --- a/qapi/run-state.json +++ b/qapi/run-state.json @@ -106,25 +106,15 @@ # # @running: true if all VCPUs are runnable, false if not runnable # -# @singlestep: true if using TCG with one guest instruction per -# translation block -# # @status: the virtual machine @RunState # # Features: # -# @deprecated: Member 'singlestep' is deprecated (with no -# replacement). -# # Since: 0.14 # -# Notes: @singlestep is enabled on the command line with '-accel -# tcg,one-insn-per-tb=on', or with the HMP 'one-insn-per-tb' -# command. ## { 'struct': 'StatusInfo', 'data': {'running': 'bool', - 'singlestep': { 'type': 'bool', 'features': [ 'deprecated' ]}, 'status': 'RunState'} } ## @@ -140,7 +130,6 @@ # # -> { "execute": "query-status" } # <- { "return": { "running": true, -# "singlestep": false, # "status": "running" } } ## { 'command': 'query-status', 'returns': 'StatusInfo', diff --git a/system/runstate.c b/system/runstate.c index fb07b7b71a..d6ab860eca 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -242,15 +242,7 @@ bool runstate_needs_reset(void) StatusInfo *qmp_query_status(Error **errp) { StatusInfo *info = g_malloc0(sizeof(*info)); - AccelState *accel = current_accel(); - /* - * We ignore errors, which will happen if the accelerator - * is not TCG. "singlestep" is meaningless for other accelerators, - * so we will set the StatusInfo field to false for those. - */ - info->singlestep = object_property_get_bool(OBJECT(accel), - "one-insn-per-tb", NULL); info->running = runstate_is_running(); info->status = current_run_state; diff --git a/tests/qemu-iotests/183.out b/tests/qemu-iotests/183.out index fd9c2e52a5..9277643853 100644 --- a/tests/qemu-iotests/183.out +++ b/tests/qemu-iotests/183.out @@ -30,13 +30,13 @@ read 65536/65536 bytes at offset 0 'arguments': { 'uri': 'unix:SOCK_DIR/migrate', 'blk': true } } {"return": {}} { 'execute': 'query-status' } -{"return": {"status": "postmigrate", "singlestep": false, "running": false}} +{"return": {"status": "postmigrate", "running": false}} === Do some I/O on the destination === { 'execute': 'query-status' } {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESUME"} -{"return": {"status": "running", "singlestep": false, "running": true}} +{"return": {"status": "running", "running": true}} { 'execute': 'human-monitor-command', 'arguments': { 'command-line': 'qemu-io disk "read -P 0x55 0 64k"' } } diff --git a/tests/qemu-iotests/234.out b/tests/qemu-iotests/234.out index 692976d1c6..ac8b64350c 100644 --- a/tests/qemu-iotests/234.out +++ b/tests/qemu-iotests/234.out @@ -15,8 +15,8 @@ Starting migration to B... {"data": {"status": "completed"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} completed completed -{"return": {"running": false, "singlestep": false, "status": "postmigrate"}} -{"return": {"running": true, "singlestep": false, "status": "running"}} +{"return": {"running": false, "status": "postmigrate"}} +{"return": {"running": true, "status": "running"}} Add a second parent to drive0-file... {"return": {}} Restart A with -incoming and second parent... @@ -32,5 +32,5 @@ Starting migration back to A... {"data": {"status": "completed"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} completed completed -{"return": {"running": true, "singlestep": false, "status": "running"}} -{"return": {"running": false, "singlestep": false, "status": "postmigrate"}} +{"return": {"running": true, "status": "running"}} +{"return": {"running": false, "status": "postmigrate"}} diff --git a/tests/qemu-iotests/262.out b/tests/qemu-iotests/262.out index 8e04c496c4..b8a2d3598d 100644 --- a/tests/qemu-iotests/262.out +++ b/tests/qemu-iotests/262.out @@ -13,5 +13,5 @@ Starting migration to B... {"data": {"status": "completed"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} completed completed -{"return": {"running": false, "singlestep": false, "status": "postmigrate"}} -{"return": {"running": true, "singlestep": false, "status": "running"}} +{"return": {"running": false, "status": "postmigrate"}} +{"return": {"running": true, "status": "running"}} diff --git a/tests/qemu-iotests/280.out b/tests/qemu-iotests/280.out index c75f437c00..546dbb4a68 100644 --- a/tests/qemu-iotests/280.out +++ b/tests/qemu-iotests/280.out @@ -12,7 +12,7 @@ Enabling migration QMP events on VM... VM is now stopped: completed {"execute": "query-status", "arguments": {}} -{"return": {"running": false, "singlestep": false, "status": "postmigrate"}} +{"return": {"running": false, "status": "postmigrate"}} === Create a snapshot of the disk image === {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-top", "size": 0}}} -- 2.41.0
This command has been deprecated before the 8.1 release, in commit e9ccfdd91d ("hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep'"). Time to drop it. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 9 --------- docs/about/removed-features.rst | 6 ++++++ tests/qtest/test-hmp.c | 1 - hmp-commands.hx | 13 ------------- 4 files changed, 6 insertions(+), 23 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 25527da423..316a26a82c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -167,15 +167,6 @@ points was removed in 7.0. However QMP still exposed the vcpu parameter. This argument has now been deprecated and the remaining remaining trace points that used it are selected just by name. -Human Monitor Protocol (HMP) commands -------------------------------------- - -``singlestep`` (since 8.1) -'''''''''''''''''''''''''' - -The ``singlestep`` command has been replaced by the ``one-insn-per-tb`` -command, which has the same behaviour but a less misleading name. - Host Architectures ------------------ diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index a8546f4787..855d788259 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -635,6 +635,12 @@ Use ``migrate-set-parameters`` instead. This command didn't produce any output already. Removed with no replacement. +``singlestep`` (removed in 9.0) +''''''''''''''''''''''''''''''' + +The ``singlestep`` command has been replaced by the ``one-insn-per-tb`` +command, which has the same behaviour but a less misleading name. + Guest Emulator ISAs ------------------- diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c index fc9125f8bb..1b2e07522f 100644 --- a/tests/qtest/test-hmp.c +++ b/tests/qtest/test-hmp.c @@ -64,7 +64,6 @@ static const char *hmp_cmds[] = { "qom-get /machine initrd", "screendump /dev/null", "sendkey x", - "singlestep on", "wavcapture /dev/null", "stopcapture 0", "sum 0 512", diff --git a/hmp-commands.hx b/hmp-commands.hx index 765349ed14..fc5f5c57ad 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -398,19 +398,6 @@ SRST If called with option off, the emulation returns to normal mode. ERST - { - .name = "singlestep", - .args_type = "option:s?", - .params = "[on|off]", - .help = "deprecated synonym for one-insn-per-tb", - .cmd = hmp_one_insn_per_tb, - }, - -SRST -``singlestep [off]`` - This is a deprecated synonym for the one-insn-per-tb command. -ERST - { .name = "stop|s", .args_type = "", -- 2.41.0
* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
This command has been deprecated before the 8.1 release, in commit e9ccfdd91d ("hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep'"). Time to drop it.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
--- docs/about/deprecated.rst | 9 --------- docs/about/removed-features.rst | 6 ++++++ tests/qtest/test-hmp.c | 1 - hmp-commands.hx | 13 ------------- 4 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 25527da423..316a26a82c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -167,15 +167,6 @@ points was removed in 7.0. However QMP still exposed the vcpu parameter. This argument has now been deprecated and the remaining remaining trace points that used it are selected just by name.
-Human Monitor Protocol (HMP) commands -------------------------------------- - -``singlestep`` (since 8.1) -'''''''''''''''''''''''''' - -The ``singlestep`` command has been replaced by the ``one-insn-per-tb`` -command, which has the same behaviour but a less misleading name. - Host Architectures ------------------
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index a8546f4787..855d788259 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -635,6 +635,12 @@ Use ``migrate-set-parameters`` instead.
This command didn't produce any output already. Removed with no replacement.
+``singlestep`` (removed in 9.0) +''''''''''''''''''''''''''''''' + +The ``singlestep`` command has been replaced by the ``one-insn-per-tb`` +command, which has the same behaviour but a less misleading name. + Guest Emulator ISAs -------------------
diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c index fc9125f8bb..1b2e07522f 100644 --- a/tests/qtest/test-hmp.c +++ b/tests/qtest/test-hmp.c @@ -64,7 +64,6 @@ static const char *hmp_cmds[] = { "qom-get /machine initrd", "screendump /dev/null", "sendkey x", - "singlestep on", "wavcapture /dev/null", "stopcapture 0", "sum 0 512", diff --git a/hmp-commands.hx b/hmp-commands.hx index 765349ed14..fc5f5c57ad 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -398,19 +398,6 @@ SRST If called with option off, the emulation returns to normal mode. ERST
- { - .name = "singlestep", - .args_type = "option:s?", - .params = "[on|off]", - .help = "deprecated synonym for one-insn-per-tb", - .cmd = hmp_one_insn_per_tb, - }, - -SRST -``singlestep [off]`` - This is a deprecated synonym for the one-insn-per-tb command. -ERST - { .name = "stop|s", .args_type = "", -- 2.41.0
-- -----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 |_______/
This option has been deprecated before the 8.1 release, in commit 12fd0f41d0 ("Document that -singlestep command line option is deprecated"). Time to drop it. Inspired-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 17 ----------------- docs/about/removed-features.rst | 18 ++++++++++++++++++ docs/user/main.rst | 6 ------ bsd-user/main.c | 3 +-- linux-user/main.c | 2 -- system/vl.c | 18 +----------------- qemu-options.hx | 8 -------- 7 files changed, 20 insertions(+), 52 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 316a26a82c..f47446c079 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -63,23 +63,6 @@ as short-form boolean values, and passed to plugins as ``arg_name=on``. However, short-form booleans are deprecated and full explicit ``arg_name=on`` form is preferred. -``-singlestep`` (since 8.1) -''''''''''''''''''''''''''' - -The ``-singlestep`` option has been turned into an accelerator property, -and given a name that better reflects what it actually does. -Use ``-accel tcg,one-insn-per-tb=on`` instead. - -User-mode emulator command line arguments ------------------------------------------ - -``-singlestep`` (since 8.1) -''''''''''''''''''''''''''' - -The ``-singlestep`` option has been given a name that better reflects -what it actually does. For both linux-user and bsd-user, use the -new ``-one-insn-per-tb`` option instead. - QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 855d788259..54081a6c19 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -482,6 +482,24 @@ Use ``-run-with async-teardown=on`` instead. Use ``-run-with chroot=dir`` instead. +``-singlestep`` (removed in 9.0) +'''''''''''''''''''''''''''''''' + +The ``-singlestep`` option has been turned into an accelerator property, +and given a name that better reflects what it actually does. +Use ``-accel tcg,one-insn-per-tb=on`` instead. + + +User-mode emulator command line arguments +----------------------------------------- + +``-singlestep`` (removed in 9.0) +'''''''''''''''''''''''''''''''' + +The ``-singlestep`` option has been given a name that better reflects +what it actually does. For both linux-user and bsd-user, use the +``-one-insn-per-tb`` option instead. + QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/docs/user/main.rst b/docs/user/main.rst index f478635396..7e7ad07409 100644 --- a/docs/user/main.rst +++ b/docs/user/main.rst @@ -98,9 +98,6 @@ Debug options: This slows down emulation a lot, but can be useful in some situations, such as when trying to analyse the logs produced by the ``-d`` option. -``-singlestep`` - This is a deprecated synonym for the ``-one-insn-per-tb`` option. - Environment variables: QEMU_STRACE @@ -251,6 +248,3 @@ Debug options: Run the emulation with one guest instruction per translation block. This slows down emulation a lot, but can be useful in some situations, such as when trying to analyse the logs produced by the ``-d`` option. - -``-singlestep`` - This is a deprecated synonym for the ``-one-insn-per-tb`` option. diff --git a/bsd-user/main.c b/bsd-user/main.c index 4de226d211..e5efb7b845 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -163,7 +163,6 @@ static void usage(void) " (use '-d help' for a list of log items)\n" "-D logfile write logs to 'logfile' (default stderr)\n" "-one-insn-per-tb run with one guest instruction per emulated TB\n" - "-singlestep deprecated synonym for -one-insn-per-tb\n" "-strace log system calls\n" "-trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n" " specify tracing options\n" @@ -391,7 +390,7 @@ int main(int argc, char **argv) (void) envlist_unsetenv(envlist, "LD_PRELOAD"); } else if (!strcmp(r, "seed")) { seed_optarg = optarg; - } else if (!strcmp(r, "singlestep") || !strcmp(r, "one-insn-per-tb")) { + } else if (!strcmp(r, "one-insn-per-tb")) { opt_one_insn_per_tb = true; } else if (!strcmp(r, "strace")) { do_strace = 1; diff --git a/linux-user/main.c b/linux-user/main.c index 0cdaf30d34..c9470eeccf 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -500,8 +500,6 @@ static const struct qemu_argument arg_table[] = { {"one-insn-per-tb", "QEMU_ONE_INSN_PER_TB", false, handle_arg_one_insn_per_tb, "", "run with one guest instruction per emulated TB"}, - {"singlestep", "QEMU_SINGLESTEP", false, handle_arg_one_insn_per_tb, - "", "deprecated synonym for -one-insn-per-tb"}, {"strace", "QEMU_STRACE", false, handle_arg_strace, "", "log system calls"}, {"seed", "QEMU_RAND_SEED", true, handle_arg_seed, diff --git a/system/vl.c b/system/vl.c index c125fb9079..809f867bcc 100644 --- a/system/vl.c +++ b/system/vl.c @@ -181,7 +181,6 @@ static const char *log_file; static bool list_data_dirs; static const char *qtest_chrdev; static const char *qtest_log; -static bool opt_one_insn_per_tb; static int has_defaults = 1; static int default_audio = 1; @@ -2308,19 +2307,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp) qemu_opt_foreach(opts, accelerator_set_property, accel, &error_fatal); - /* - * If legacy -singlestep option is set, honour it for TCG and - * silently ignore for any other accelerator (which is how this - * option has always behaved). - */ - if (opt_one_insn_per_tb) { - /* - * This will always succeed for TCG, and we want to ignore - * the error from trying to set a nonexistent property - * on any other accelerator. - */ - object_property_set_bool(OBJECT(accel), "one-insn-per-tb", true, NULL); - } + ret = accel_init_machine(accel, current_machine); if (ret < 0) { if (!qtest_with_kvm || ret != -ENOENT) { @@ -3057,9 +3044,6 @@ void qemu_init(int argc, char **argv) case QEMU_OPTION_bios: qdict_put_str(machine_opts_dict, "firmware", optarg); break; - case QEMU_OPTION_singlestep: - opt_one_insn_per_tb = true; - break; case QEMU_OPTION_S: autostart = 0; break; diff --git a/qemu-options.hx b/qemu-options.hx index 9be6beb5a0..033fa873e4 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4357,14 +4357,6 @@ SRST from a script. ERST -DEF("singlestep", 0, QEMU_OPTION_singlestep, \ - "-singlestep deprecated synonym for -accel tcg,one-insn-per-tb=on\n", QEMU_ARCH_ALL) -SRST -``-singlestep`` - This is a deprecated synonym for the TCG accelerator property - ``one-insn-per-tb``. -ERST - DEF("preconfig", 0, QEMU_OPTION_preconfig, \ "--preconfig pause QEMU before machine is initialized (experimental)\n", QEMU_ARCH_ALL) -- 2.41.0
On 17/01/2024 16.14, Philippe Mathieu-Daudé wrote:
This option has been deprecated before the 8.1 release, in commit 12fd0f41d0 ("Document that -singlestep command line option is deprecated"). Time to drop it.
Inspired-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 17 ----------------- docs/about/removed-features.rst | 18 ++++++++++++++++++ docs/user/main.rst | 6 ------ bsd-user/main.c | 3 +-- linux-user/main.c | 2 -- system/vl.c | 18 +----------------- qemu-options.hx | 8 -------- 7 files changed, 20 insertions(+), 52 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
participants (4)
-
Dr. David Alan Gilbert -
Markus Armbruster -
Philippe Mathieu-Daudé -
Thomas Huth