qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/5] qemu-options: Remove the deprecated -singlestep option


From: Thomas Huth
Subject: Re: [PATCH 5/5] qemu-options: Remove the deprecated -singlestep option
Date: Mon, 15 Jan 2024 14:54:25 +0100
User-agent: Mozilla Thunderbird

On 12/01/2024 16.39, Philippe Mathieu-Daudé wrote:
Hi Thomas

+Laurent & Peter

On 12/1/24 11:00, Thomas Huth wrote:
It's been marked as deprecated since QEMU 8.1, so it should be fine
to remove this now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
  docs/about/deprecated.rst       |  6 ------
  docs/about/removed-features.rst |  7 +++++++
  system/vl.c                     | 18 +-----------------
  qemu-options.hx                 |  8 --------
  4 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index b50cfe596b..81a5149f1e 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,12 +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
  -----------------------------------------
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index a8546f4787..d5c60ff47f 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -482,6 +482,13 @@ 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.
+
  QEMU Machine Protocol (QMP) commands
  ------------------------------------
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)

This is the system part, what about the user part?

StatusInfo::singlestep was deprecated at the same time,
can we remove it?

IOW could we complete your patch with this?

-- >8 --
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index b47763330c..a1ae93664a 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,17 +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.

-
-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
  ------------------------------------

@@ -145,20 +134,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/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/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'} }

Uh, oh, that's a bigger change already ... can we safely remove the field here without upsetting 3rd party apps that rely on this interface? Anyway, I withdraw my patch for -singlestep ... this should be handled by someone who's more familiar with these pieces of the code.

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]