All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] Clean up accelerator options
@ 2018-06-13  5:05 Thomas Huth
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
                   ` (4 more replies)
  0 siblings, 5 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13  5:05 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

The current state of the accelerator options is quite messy: There are
three ways of enabling an accelerator (-machine accel=xyz, -accel xyz
and -enable-xyz) and this can be very confusing for the users. This
patch series now tries to standardize our documentation and examples
on the preferred -accel option.

Thomas Huth (4):
  Replace '-machine accel=xyz' with '-accel xyz'
  Replace '-enable-kvm' with '-accel kvm' in docs and help texts
  qemu-options: Improve the documentation of '-accel' and '-machine
    accel=...'
  qemu-options: Do not show -enable-kvm and -enable-hax in the docs
    anymore

 docs/COLO-FT.txt                    |  8 +++----
 docs/can.txt                        |  4 ++--
 docs/multi-thread-compression.txt   |  2 +-
 docs/multiseat.txt                  |  2 +-
 docs/specs/tpm.txt                  |  8 +++----
 hw/block/dataplane/virtio-blk.c     |  4 ++--
 hw/scsi/virtio-scsi-dataplane.c     |  4 ++--
 qemu-doc.texi                       |  3 +--
 qemu-options.hx                     | 44 ++++++++++++-------------------------
 scripts/qtest.py                    |  2 +-
 tests/bios-tables-test.c            |  2 +-
 tests/boot-serial-test.c            |  2 +-
 tests/libqtest.c                    |  2 +-
 tests/migration-test.c              |  8 +++----
 tests/migration/guestperf/engine.py |  2 +-
 tests/pnv-xscom-test.c              |  2 +-
 tests/prom-env-test.c               |  2 +-
 tests/pxe-test.c                    |  2 +-
 tests/qemu-iotests/172              |  2 +-
 tests/qemu-iotests/check            |  2 +-
 tests/vmgenid-test.c                |  2 +-
 21 files changed, 46 insertions(+), 63 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
@ 2018-06-13  5:05 ` Thomas Huth
  2018-06-13 12:20   ` Eric Blake
                     ` (2 more replies)
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts Thomas Huth
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13  5:05 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

We've got a separate option to configure the accelerator nowadays.
Use it in the source and examples to demonstrate that this is the
preferred way of setting this option now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qemu-doc.texi                       | 3 +--
 qemu-options.hx                     | 2 +-
 scripts/qtest.py                    | 2 +-
 tests/bios-tables-test.c            | 2 +-
 tests/boot-serial-test.c            | 2 +-
 tests/libqtest.c                    | 2 +-
 tests/migration-test.c              | 8 ++++----
 tests/migration/guestperf/engine.py | 2 +-
 tests/pnv-xscom-test.c              | 2 +-
 tests/prom-env-test.c               | 2 +-
 tests/pxe-test.c                    | 2 +-
 tests/qemu-iotests/172              | 2 +-
 tests/qemu-iotests/check            | 2 +-
 tests/vmgenid-test.c                | 2 +-
 14 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index cd05760..1c47e7c 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2793,8 +2793,7 @@ which is the default.
 
 @subsection -no-kvm (since 1.3.0)
 
-The ``-no-kvm'' argument is now a synonym for setting
-``-machine accel=tcg''.
+The ``-no-kvm'' argument is now a synonym for setting ``-accel tcg''.
 
 @subsection -vnc tls (since 2.5.0)
 
diff --git a/qemu-options.hx b/qemu-options.hx
index c0d3951..451f7a6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3922,7 +3922,7 @@ STEXI
 Enable FIPS 140-2 compliance mode.
 ETEXI
 
-HXCOMM Deprecated by -machine accel=tcg property
+HXCOMM Deprecated by -accel tcg
 DEF("no-kvm", 0, QEMU_OPTION_no_kvm, "", QEMU_ARCH_I386)
 
 DEF("msg", HAS_ARG, QEMU_OPTION_msg,
diff --git a/scripts/qtest.py b/scripts/qtest.py
index df0daf2..8c074a6 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -89,7 +89,7 @@ class QEMUQtestMachine(qemu.QEMUMachine):
     def _base_args(self):
         args = super(QEMUQtestMachine, self)._base_args()
         args.extend(['-qtest', 'unix:path=' + self._qtest_path,
-                     '-machine', 'accel=qtest'])
+                     '-accel', 'qtest'])
         return args
 
     def _pre_launch(self):
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 4e24930..f41ec39 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -613,7 +613,7 @@ static void test_acpi_one(const char *params, test_data *data)
     char *args;
 
     /* Disable kernel irqchip to be able to override apic irq0. */
-    args = g_strdup_printf("-machine %s,accel=%s,kernel-irqchip=off "
+    args = g_strdup_printf("-machine %s,kernel-irqchip=off -accel %s "
                            "-net none -display none %s "
                            "-drive id=hd0,if=none,file=%s,format=raw "
                            "-device ide-hd,drive=hd0 ",
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 4d6815c..84a2aa9 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -175,7 +175,7 @@ static void test_machine(const void *data)
      * Make sure that this test uses tcg if available: It is used as a
      * fast-enough smoketest for that.
      */
-    global_qtest = qtest_startf("%s %s -M %s,accel=tcg:kvm "
+    global_qtest = qtest_startf("%s %s -M %s -accel tcg:kvm "
                                 "-chardev file,id=serial0,path=%s "
                                 "-no-shutdown -serial chardev:serial0 %s",
                                 codeparam, code ? codetmp : "",
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 098af6a..51481b0 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -209,7 +209,7 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob,
                                   "-qtest-log %s "
                                   "-chardev socket,path=%s,nowait,id=char0 "
                                   "-mon chardev=char0,mode=control%s "
-                                  "-machine accel=qtest "
+                                  "-accel qtest "
                                   "-display none "
                                   "%s", qemu_binary, socket_path,
                                   getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
diff --git a/tests/migration-test.c b/tests/migration-test.c
index 3a85446..2b96ff0 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -372,12 +372,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         init_bootfile_x86(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
+        cmd_src = g_strdup_printf("-accel %s -m 150M"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,format=raw",
                                   accel, tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
+        cmd_dst = g_strdup_printf("-accel %s -m 150M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,format=raw"
@@ -389,7 +389,7 @@ static void test_migrate_start(QTestState **from, QTestState **to,
         if (access("/sys/module/kvm_hv", F_OK)) {
             accel = "tcg";
         }
-        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
+        cmd_src = g_strdup_printf("-accel %s -m 256M"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -prom-env '"
@@ -397,7 +397,7 @@ static void test_migrate_start(QTestState **from, QTestState **to,
                                   "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
                                   "until'",  accel, tmpfs, end_address,
                                   start_address);
-        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
+        cmd_dst = g_strdup_printf("-accel %s -m 256M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s",
diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
index 398e3f2..cf3d7c8 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -286,7 +286,7 @@ class Engine(object):
             cmdline = "'" + cmdline + "'"
 
         argv = [
-            "-machine", "accel=kvm",
+            "-accel", "kvm",
             "-cpu", "host",
             "-kernel", self._kernel,
             "-initrd", self._initrd,
diff --git a/tests/pnv-xscom-test.c b/tests/pnv-xscom-test.c
index efb7c83..4f6686c 100644
--- a/tests/pnv-xscom-test.c
+++ b/tests/pnv-xscom-test.c
@@ -79,7 +79,7 @@ static void test_cfam_id(const void *data)
 {
     const PnvChip *chip = data;
 
-    global_qtest = qtest_startf("-M powernv,accel=tcg -cpu %s",
+    global_qtest = qtest_startf("-M powernv -accel tcg -cpu %s",
                                 chip->cpu_model);
     test_xscom_cfam_id(chip);
     qtest_quit(global_qtest);
diff --git a/tests/prom-env-test.c b/tests/prom-env-test.c
index 8c867e6..e066269 100644
--- a/tests/prom-env-test.c
+++ b/tests/prom-env-test.c
@@ -49,7 +49,7 @@ static void test_machine(const void *machine)
     /* The pseries firmware boots much faster without the default devices */
     extra_args = strcmp(machine, "pseries") == 0 ? "-nodefaults" : "";
 
-    global_qtest = qtest_startf("-M %s,accel=tcg %s "
+    global_qtest = qtest_startf("-M %s -accel tcg %s "
                                 "-prom-env 'use-nvramrc?=true' "
                                 "-prom-env 'nvramrc=%x %x l!' ",
                                 (const char *)machine, extra_args,
diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 6e36796..a8c4daa 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -64,7 +64,7 @@ static void test_pxe_one(const testdef_t *test, bool ipv6)
     char *args;
 
     args = g_strdup_printf(
-        "-machine %s,accel=kvm:tcg -nodefaults -boot order=n "
+        "-machine %s -accel kvm:tcg -nodefaults -boot order=n "
         "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,ipv4=%s,ipv6=%s "
         "-device %s,bootindex=1,netdev=" NETNAME,
         test->machine, disk, ipv6 ? "off" : "on", ipv6 ? "on" : "off",
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 02c5f79..93742a7 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -56,7 +56,7 @@ function do_run_qemu()
             done
         fi
         echo quit
-    ) | $QEMU -machine accel=qtest -nographic -monitor stdio -serial none "$@"
+    ) | $QEMU -accel qtest -nographic -monitor stdio -serial none "$@"
     echo
 }
 
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index aa94c6c..5ffbf5d 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -129,7 +129,7 @@ export CACHEMODE="writeback"
 export QEMU_IO_OPTIONS=""
 export QEMU_IO_OPTIONS_NO_FMT=""
 export CACHEMODE_IS_DEFAULT=true
-export QEMU_OPTIONS="-nodefaults -machine accel=qtest"
+export QEMU_OPTIONS="-nodefaults -accel qtest"
 export VALGRIND_QEMU=
 export IMGKEYSECRET=
 export IMGOPTSSYNTAX=false
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
index 8d915c6..4324034 100644
--- a/tests/vmgenid-test.c
+++ b/tests/vmgenid-test.c
@@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
 static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
 
 #define GUID_CMD(guid)                          \
-    "-machine accel=kvm:tcg "                   \
+    "-accel kvm:tcg "                           \
     "-device vmgenid,id=testvgid,guid=%s "      \
     "-drive id=hd0,if=none,file=%s,format=raw " \
     "-device ide-hd,drive=hd0 ", guid, disk
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts
  2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
@ 2018-06-13  5:05 ` Thomas Huth
  2018-06-13 12:51   ` Paolo Bonzini
  2018-06-13 13:38   ` Stefan Hajnoczi
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...' Thomas Huth
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13  5:05 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

The preferred way to select the KVM accelerator is to use "-accel kvm"
these days, so let's be consistent in our documentation and help texts.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/COLO-FT.txt                  | 8 ++++----
 docs/can.txt                      | 4 ++--
 docs/multi-thread-compression.txt | 2 +-
 docs/multiseat.txt                | 2 +-
 docs/specs/tpm.txt                | 8 ++++----
 hw/block/dataplane/virtio-blk.c   | 4 ++--
 hw/scsi/virtio-scsi-dataplane.c   | 4 ++--
 7 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index e289be2..d7c7dcd 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -113,16 +113,16 @@ by using 'x-colo-lost-heartbeat' command.
 == Test procedure ==
 1. Startup qemu
 Primary:
-# qemu-kvm -enable-kvm -m 2048 -smp 2 -qmp stdio -vnc :7 -name primary \
-  -device piix3-usb-uhci \
+# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name primary \
+  -device piix3-usb-uhci -vnc :7 \
   -device usb-tablet -netdev tap,id=hn0,vhost=off \
   -device virtio-net-pci,id=net-pci0,netdev=hn0 \
   -drive if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
          children.0.file.filename=1.raw,\
          children.0.driver=raw -S
 Secondary:
-# qemu-kvm -enable-kvm -m 2048 -smp 2 -qmp stdio -vnc :7 -name secondary \
-  -device piix3-usb-uhci \
+# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name secondary \
+  -device piix3-usb-uhci -vnc :7 \
   -device usb-tablet -netdev tap,id=hn0,vhost=off \
   -device virtio-net-pci,id=net-pci0,netdev=hn0 \
   -drive if=none,id=secondary-disk0,file.filename=1.raw,driver=raw,node-name=node0 \
diff --git a/docs/can.txt b/docs/can.txt
index a357105..7ba23b2 100644
--- a/docs/can.txt
+++ b/docs/can.txt
@@ -52,7 +52,7 @@ The ''kvaser_pci'' board/device model is compatible with and has been tested wit
 The tested setup was Linux 4.9 kernel on the host and guest side.
 Example for qemu-system-x86_64:
 
-    qemu-system-x86_64 -enable-kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \
+    qemu-system-x86_64 -accel kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \
       -initrd ramdisk.cpio \
       -virtfs local,path=shareddir,security_model=none,mount_tag=shareddir \
       -object can-bus,id=canbus0 \
@@ -104,4 +104,4 @@ Links to other resources
      Slides
      http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can-slides.pdf
  (5) Linux SocketCAN utilities
-     https://github.com/linux-can/can-utils/
\ No newline at end of file
+     https://github.com/linux-can/can-utils/
diff --git a/docs/multi-thread-compression.txt b/docs/multi-thread-compression.txt
index d0caaf7..bb88c6b 100644
--- a/docs/multi-thread-compression.txt
+++ b/docs/multi-thread-compression.txt
@@ -62,7 +62,7 @@ RAM: 128G
 NIC: Intel I350 (10/100/1000Mbps)
 Host OS: CentOS 7 64-bit
 Guest OS: RHEL 6.5 64-bit
-Parameter: qemu-system-x86_64 -enable-kvm -smp 4 -m 4096
+Parameter: qemu-system-x86_64 -accel kvm -smp 4 -m 4096
  /share/ia32e_rhel6u5.qcow -monitor stdio
 
 There is no additional application is running on the guest when doing
diff --git a/docs/multiseat.txt b/docs/multiseat.txt
index 807518c..dc28cdb 100644
--- a/docs/multiseat.txt
+++ b/docs/multiseat.txt
@@ -18,7 +18,7 @@ or
 
 Next put together the qemu command line (sdk/gtk):
 
-qemu	-enable-kvm -usb $memory $disk $whatever \
+qemu	-accel kvm -usb $memory $disk $whatever \
 	-display [ sdl | gtk ] \
 	-vga std \
 	-device usb-tablet
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c..70ad4a0 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -98,7 +98,7 @@ QEMU files related to the TPM passthrough device:
 Command line to start QEMU with the TPM passthrough device using the host's
 hardware TPM /dev/tpm0:
 
-qemu-system-x86_64 -display sdl -enable-kvm \
+qemu-system-x86_64 -display sdl -accel kvm \
   -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
   -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \
   -device tpm-tis,tpmdev=tpm0 test.img
@@ -164,7 +164,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
 Command line to start QEMU with the TPM emulator device communicating with
 the swtpm:
 
-qemu-system-x86_64 -display sdl -enable-kvm \
+qemu-system-x86_64 -display sdl -accel kvm \
   -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
   -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
   -tpmdev emulator,id=tpm0,chardev=chrtpm \
@@ -222,7 +222,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
 
 In a 2nd terminal start the VM:
 
-qemu-system-x86_64 -display sdl -enable-kvm \
+qemu-system-x86_64 -display sdl -accel kvm \
   -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
   -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
   -tpmdev emulator,id=tpm0,chardev=chrtpm \
@@ -255,7 +255,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
 In the 2nd terminal restore the state of the VM using the additonal
 '-incoming' option.
 
-qemu-system-x86_64 -display sdl -enable-kvm \
+qemu-system-x86_64 -display sdl -accel kvm \
   -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
   -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
   -tpmdev emulator,id=tpm0,chardev=chrtpm \
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index d648aeb..8c37bd3 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -190,8 +190,8 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
     /* Set up guest notifier (irq) */
     r = k->set_guest_notifiers(qbus->parent, nvqs, true);
     if (r != 0) {
-        fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
-                "ensure -enable-kvm is set\n", r);
+        error_report("virtio-blk failed to set guest notifier (%d), "
+                     "ensure -accel kvm is set.", r);
         goto fail_guest_notifiers;
     }
 
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 912e500..b995bab 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -142,8 +142,8 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
     /* Set up guest notifier (irq) */
     rc = k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, true);
     if (rc != 0) {
-        fprintf(stderr, "virtio-scsi: Failed to set guest notifiers (%d), "
-                "ensure -enable-kvm is set\n", rc);
+        error_report("virtio-scsi: Failed to set guest notifiers (%d), "
+                     "ensure -accel kvm is set.", rc);
         goto fail_guest_notifiers;
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...'
  2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts Thomas Huth
@ 2018-06-13  5:05 ` Thomas Huth
  2018-06-13 13:38   ` Stefan Hajnoczi
  2018-06-13  5:05 ` [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore Thomas Huth
  2018-06-13 13:39 ` [Qemu-devel] [PATCH 0/4] Clean up accelerator options Stefan Hajnoczi
  4 siblings, 1 reply; 46+ messages in thread
From: Thomas Huth @ 2018-06-13  5:05 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

Instead of repeating the same text for both options, let's rather change
the '-machine accel' documentation to point to the preferred '-accel' option
instead.
And in the documenation of the -accel option, make it clear that you can
use colons to specify multiple accelerators.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qemu-options.hx | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 451f7a6..0ec9cac 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -30,8 +30,7 @@ ETEXI
 DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "-machine [type=]name[,prop[=value][,...]]\n"
     "                selects emulated machine ('-machine help' for list)\n"
-    "                property accel=accel1[:accel2[:...]] selects accelerator\n"
-    "                supported accelerators are kvm, xen, hax, hvf, whpx or tcg (default: tcg)\n"
+    "                property accel=accel1[:accel2[:...]] selects accelerator (see -accel)\n"
     "                kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n"
     "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
     "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
@@ -66,10 +65,8 @@ of QEMU will support machine types from many previous versions.
 Supported machine properties are:
 @table @option
 @item accel=@var{accels1}[:@var{accels2}[:...]]
-This is used to enable an accelerator. Depending on the target architecture,
-kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If there is
-more than one accelerator specified, the next one is used if the previous one
-fails to initialize.
+This can be used to enable an accelerator. See the preferred @code{-accel}
+option for a list of available accelerators.
 @item kernel_irqchip=on|off
 Controls in-kernel irqchip support for the chosen accelerator when available.
 @item gfx_passthru=on|off
@@ -128,16 +125,17 @@ Select CPU model (@code{-cpu help} for list and additional feature selection)
 ETEXI
 
 DEF("accel", HAS_ARG, QEMU_OPTION_accel,
-    "-accel [accel=]accelerator[,thread=single|multi]\n"
+    "-accel [accel=]accelerator[:accelerator[:...]][,thread=single|multi]\n"
     "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
-    "                thread=single|multi (enable multi-threaded TCG)", QEMU_ARCH_ALL)
+    "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
 STEXI
 @item -accel @var{name}[,prop=@var{value}[,...]]
 @findex -accel
 This is used to enable an accelerator. Depending on the target architecture,
-kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If there is
-more than one accelerator specified, the next one is used if the previous one
-fails to initialize.
+kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used.
+Multiple accelerators can be specified by separating them with a colon. If
+there is more than one accelerator specified, the next one is used if the
+previous one fails to initialize.
 @table @option
 @item thread=single|multi
 Controls number of TCG threads. When the TCG is multi-threaded there will be one
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
                   ` (2 preceding siblings ...)
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...' Thomas Huth
@ 2018-06-13  5:05 ` Thomas Huth
  2018-06-13 12:51   ` Paolo Bonzini
  2018-06-13 13:38   ` Stefan Hajnoczi
  2018-06-13 13:39 ` [Qemu-devel] [PATCH 0/4] Clean up accelerator options Stefan Hajnoczi
  4 siblings, 2 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13  5:05 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

We've got three ways of enabling an accelerator: -machine accel=xyz,
-accel xyz and -enable-xyz. For new QEMU users, this must be very
confusing ("Which one do I have to use? Is there a difference between
the options?"). While -enable-kvm was useful in the past, there is no
real good reason for using it anymore today ("-accel kvm" is even less
to type than "-enable-kvm"), so let's decrease the confusing amount of
options in our documenation a little bit by removing the -enable-xyz
here. Note that the option itself is neither removed nor marked as
deprecated - since -enable-kvm is likely used in a lot of scripts and
since its code is easy to maintain, we should keep it around to avoid
to break old setups.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 PS: I guess Paolo won't like this patch ... let's try it anyway ;-)

 qemu-options.hx | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 0ec9cac..f33a81e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3414,25 +3414,11 @@ STEXI
 Set the filename for the BIOS.
 ETEXI
 
-DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
-    "-enable-kvm     enable KVM full virtualization support\n", QEMU_ARCH_ALL)
-STEXI
-@item -enable-kvm
-@findex -enable-kvm
-Enable KVM full virtualization support. This option is only available
-if KVM support is enabled when compiling.
-ETEXI
+HXCOMM -accel kvm should be used instead, thus this is not documented anymore
+DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "", QEMU_ARCH_ALL)
 
-DEF("enable-hax", 0, QEMU_OPTION_enable_hax, \
-    "-enable-hax     enable HAX virtualization support\n", QEMU_ARCH_I386)
-STEXI
-@item -enable-hax
-@findex -enable-hax
-Enable HAX (Hardware-based Acceleration eXecution) support. This option
-is only available if HAX support is enabled when compiling. HAX is only
-applicable to MAC and Windows platform, and thus does not conflict with
-KVM.
-ETEXI
+HXCOMM -accel hax should be used instead, thus this is not documented anymore
+DEF("enable-hax", 0, QEMU_OPTION_enable_hax, "", QEMU_ARCH_I386)
 
 DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
     "-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
@ 2018-06-13 12:20   ` Eric Blake
  2018-06-13 12:48   ` Paolo Bonzini
  2018-06-13 13:38   ` Stefan Hajnoczi
  2 siblings, 0 replies; 46+ messages in thread
From: Eric Blake @ 2018-06-13 12:20 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Paolo Bonzini
  Cc: Eduardo Habkost, Ben Warren, qemu-trivial, Markus Armbruster,
	Stefan Hajnoczi, zhang.zhanghailiang

On 06/13/2018 12:05 AM, Thomas Huth wrote:
> We've got a separate option to configure the accelerator nowadays.
> Use it in the source and examples to demonstrate that this is the
> preferred way of setting this option now.

Is it worth bike-shedding the preferred spelling as '--accel xyz', to 
match 
https://wiki.qemu.org/BiteSizedTasks#Consistent_option_usage_in_documentation?

It's more important to use double-dash for options shared between 
binaries, but qemu-nbd and qemu-img don't use --accel, so that makes the 
argument slightly weaker for the doubled form here.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
  2018-06-13 12:20   ` Eric Blake
@ 2018-06-13 12:48   ` Paolo Bonzini
  2018-06-13 12:53     ` Thomas Huth
  2018-06-13 13:38   ` Stefan Hajnoczi
  2 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-13 12:48 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On 13/06/2018 07:05, Thomas Huth wrote:
> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> index 8d915c6..4324034 100644
> --- a/tests/vmgenid-test.c
> +++ b/tests/vmgenid-test.c
> @@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
>  static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
>  
>  #define GUID_CMD(guid)                          \
> -    "-machine accel=kvm:tcg "                   \
> +    "-accel kvm:tcg "                           \
>      "-device vmgenid,id=testvgid,guid=%s "      \

"-accel kvm:tcg" works, but it really shouldn't (and I think we can
change it without a deprecation period).   The right syntax would be
"-accel kvm -accel tcg", so that you can specify options that are valid
only for KVM, or onlty for TCG.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13  5:05 ` [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore Thomas Huth
@ 2018-06-13 12:51   ` Paolo Bonzini
  2018-06-13 13:38   ` Stefan Hajnoczi
  1 sibling, 0 replies; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-13 12:51 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On 13/06/2018 07:05, Thomas Huth wrote:
> We've got three ways of enabling an accelerator: -machine accel=xyz,
> -accel xyz and -enable-xyz. For new QEMU users, this must be very
> confusing ("Which one do I have to use? Is there a difference between
> the options?"). While -enable-kvm was useful in the past, there is no
> real good reason for using it anymore today ("-accel kvm" is even less
> to type than "-enable-kvm"), so let's decrease the confusing amount of
> options in our documenation a little bit by removing the -enable-xyz
> here. Note that the option itself is neither removed nor marked as
> deprecated - since -enable-kvm is likely used in a lot of scripts and
> since its code is easy to maintain, we should keep it around to avoid
> to break old setups.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)

Heh.  I'm not sure actually.  I'm more in favor of introducing an
"Obsolete options" section and putting it there.

Paolo


>  qemu-options.hx | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0ec9cac..f33a81e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3414,25 +3414,11 @@ STEXI
>  Set the filename for the BIOS.
>  ETEXI
>  
> -DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
> -    "-enable-kvm     enable KVM full virtualization support\n", QEMU_ARCH_ALL)
> -STEXI
> -@item -enable-kvm
> -@findex -enable-kvm
> -Enable KVM full virtualization support. This option is only available
> -if KVM support is enabled when compiling.
> -ETEXI
> +HXCOMM -accel kvm should be used instead, thus this is not documented anymore
> +DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, "", QEMU_ARCH_ALL)
>  
> -DEF("enable-hax", 0, QEMU_OPTION_enable_hax, \
> -    "-enable-hax     enable HAX virtualization support\n", QEMU_ARCH_I386)
> -STEXI
> -@item -enable-hax
> -@findex -enable-hax
> -Enable HAX (Hardware-based Acceleration eXecution) support. This option
> -is only available if HAX support is enabled when compiling. HAX is only
> -applicable to MAC and Windows platform, and thus does not conflict with
> -KVM.
> -ETEXI
> +HXCOMM -accel hax should be used instead, thus this is not documented anymore
> +DEF("enable-hax", 0, QEMU_OPTION_enable_hax, "", QEMU_ARCH_I386)
>  
>  DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
>      "-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts Thomas Huth
@ 2018-06-13 12:51   ` Paolo Bonzini
  2018-06-13 13:38   ` Stefan Hajnoczi
  1 sibling, 0 replies; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-13 12:51 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On 13/06/2018 07:05, Thomas Huth wrote:
> The preferred way to select the KVM accelerator is to use "-accel kvm"
> these days, so let's be consistent in our documentation and help texts.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/COLO-FT.txt                  | 8 ++++----
>  docs/can.txt                      | 4 ++--
>  docs/multi-thread-compression.txt | 2 +-
>  docs/multiseat.txt                | 2 +-
>  docs/specs/tpm.txt                | 8 ++++----
>  hw/block/dataplane/virtio-blk.c   | 4 ++--
>  hw/scsi/virtio-scsi-dataplane.c   | 4 ++--
>  7 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
> index e289be2..d7c7dcd 100644
> --- a/docs/COLO-FT.txt
> +++ b/docs/COLO-FT.txt
> @@ -113,16 +113,16 @@ by using 'x-colo-lost-heartbeat' command.
>  == Test procedure ==
>  1. Startup qemu
>  Primary:
> -# qemu-kvm -enable-kvm -m 2048 -smp 2 -qmp stdio -vnc :7 -name primary \
> -  -device piix3-usb-uhci \
> +# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name primary \
> +  -device piix3-usb-uhci -vnc :7 \
>    -device usb-tablet -netdev tap,id=hn0,vhost=off \
>    -device virtio-net-pci,id=net-pci0,netdev=hn0 \
>    -drive if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
>           children.0.file.filename=1.raw,\
>           children.0.driver=raw -S
>  Secondary:
> -# qemu-kvm -enable-kvm -m 2048 -smp 2 -qmp stdio -vnc :7 -name secondary \
> -  -device piix3-usb-uhci \
> +# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name secondary \
> +  -device piix3-usb-uhci -vnc :7 \
>    -device usb-tablet -netdev tap,id=hn0,vhost=off \
>    -device virtio-net-pci,id=net-pci0,netdev=hn0 \
>    -drive if=none,id=secondary-disk0,file.filename=1.raw,driver=raw,node-name=node0 \
> diff --git a/docs/can.txt b/docs/can.txt
> index a357105..7ba23b2 100644
> --- a/docs/can.txt
> +++ b/docs/can.txt
> @@ -52,7 +52,7 @@ The ''kvaser_pci'' board/device model is compatible with and has been tested wit
>  The tested setup was Linux 4.9 kernel on the host and guest side.
>  Example for qemu-system-x86_64:
>  
> -    qemu-system-x86_64 -enable-kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \
> +    qemu-system-x86_64 -accel kvm -kernel /boot/vmlinuz-4.9.0-4-amd64 \
>        -initrd ramdisk.cpio \
>        -virtfs local,path=shareddir,security_model=none,mount_tag=shareddir \
>        -object can-bus,id=canbus0 \
> @@ -104,4 +104,4 @@ Links to other resources
>       Slides
>       http://rtime.felk.cvut.cz/publications/public/rtlws2015-qemu-can-slides.pdf
>   (5) Linux SocketCAN utilities
> -     https://github.com/linux-can/can-utils/
> \ No newline at end of file
> +     https://github.com/linux-can/can-utils/
> diff --git a/docs/multi-thread-compression.txt b/docs/multi-thread-compression.txt
> index d0caaf7..bb88c6b 100644
> --- a/docs/multi-thread-compression.txt
> +++ b/docs/multi-thread-compression.txt
> @@ -62,7 +62,7 @@ RAM: 128G
>  NIC: Intel I350 (10/100/1000Mbps)
>  Host OS: CentOS 7 64-bit
>  Guest OS: RHEL 6.5 64-bit
> -Parameter: qemu-system-x86_64 -enable-kvm -smp 4 -m 4096
> +Parameter: qemu-system-x86_64 -accel kvm -smp 4 -m 4096
>   /share/ia32e_rhel6u5.qcow -monitor stdio
>  
>  There is no additional application is running on the guest when doing
> diff --git a/docs/multiseat.txt b/docs/multiseat.txt
> index 807518c..dc28cdb 100644
> --- a/docs/multiseat.txt
> +++ b/docs/multiseat.txt
> @@ -18,7 +18,7 @@ or
>  
>  Next put together the qemu command line (sdk/gtk):
>  
> -qemu	-enable-kvm -usb $memory $disk $whatever \
> +qemu	-accel kvm -usb $memory $disk $whatever \
>  	-display [ sdl | gtk ] \
>  	-vga std \
>  	-device usb-tablet
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index c230c4c..70ad4a0 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -98,7 +98,7 @@ QEMU files related to the TPM passthrough device:
>  Command line to start QEMU with the TPM passthrough device using the host's
>  hardware TPM /dev/tpm0:
>  
> -qemu-system-x86_64 -display sdl -enable-kvm \
> +qemu-system-x86_64 -display sdl -accel kvm \
>    -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
>    -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \
>    -device tpm-tis,tpmdev=tpm0 test.img
> @@ -164,7 +164,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
>  Command line to start QEMU with the TPM emulator device communicating with
>  the swtpm:
>  
> -qemu-system-x86_64 -display sdl -enable-kvm \
> +qemu-system-x86_64 -display sdl -accel kvm \
>    -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
>    -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
>    -tpmdev emulator,id=tpm0,chardev=chrtpm \
> @@ -222,7 +222,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
>  
>  In a 2nd terminal start the VM:
>  
> -qemu-system-x86_64 -display sdl -enable-kvm \
> +qemu-system-x86_64 -display sdl -accel kvm \
>    -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
>    -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
>    -tpmdev emulator,id=tpm0,chardev=chrtpm \
> @@ -255,7 +255,7 @@ swtpm socket --tpmstate dir=/tmp/mytpm1 \
>  In the 2nd terminal restore the state of the VM using the additonal
>  '-incoming' option.
>  
> -qemu-system-x86_64 -display sdl -enable-kvm \
> +qemu-system-x86_64 -display sdl -accel kvm \
>    -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
>    -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
>    -tpmdev emulator,id=tpm0,chardev=chrtpm \
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index d648aeb..8c37bd3 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -190,8 +190,8 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
>      /* Set up guest notifier (irq) */
>      r = k->set_guest_notifiers(qbus->parent, nvqs, true);
>      if (r != 0) {
> -        fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
> -                "ensure -enable-kvm is set\n", r);
> +        error_report("virtio-blk failed to set guest notifier (%d), "
> +                     "ensure -accel kvm is set.", r);
>          goto fail_guest_notifiers;
>      }
>  
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index 912e500..b995bab 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -142,8 +142,8 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
>      /* Set up guest notifier (irq) */
>      rc = k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, true);
>      if (rc != 0) {
> -        fprintf(stderr, "virtio-scsi: Failed to set guest notifiers (%d), "
> -                "ensure -enable-kvm is set\n", rc);
> +        error_report("virtio-scsi: Failed to set guest notifiers (%d), "
> +                     "ensure -accel kvm is set.", rc);
>          goto fail_guest_notifiers;
>      }
>  
> 

Queued this one.  Thanks,

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13 12:48   ` Paolo Bonzini
@ 2018-06-13 12:53     ` Thomas Huth
  2018-06-13 13:54       ` Paolo Bonzini
  2018-06-13 16:10       ` Eduardo Habkost
  0 siblings, 2 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13 12:53 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On 13.06.2018 14:48, Paolo Bonzini wrote:
> On 13/06/2018 07:05, Thomas Huth wrote:
>> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
>> index 8d915c6..4324034 100644
>> --- a/tests/vmgenid-test.c
>> +++ b/tests/vmgenid-test.c
>> @@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
>>  static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
>>  
>>  #define GUID_CMD(guid)                          \
>> -    "-machine accel=kvm:tcg "                   \
>> +    "-accel kvm:tcg "                           \
>>      "-device vmgenid,id=testvgid,guid=%s "      \
> 
> "-accel kvm:tcg" works, but it really shouldn't (and I think we can
> change it without a deprecation period).   The right syntax would be
> "-accel kvm -accel tcg", so that you can specify options that are valid
> only for KVM, or onlty for TCG.

I see your point, but this would break these qtests that are trying to
override the "-machine accel=qtest" from libqtest.c this way... and if
any other tool out there in the wild is already depending on this
behavior, too, we can not change it so easily anymore.

 Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13  5:05 ` [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore Thomas Huth
  2018-06-13 12:51   ` Paolo Bonzini
@ 2018-06-13 13:38   ` Stefan Hajnoczi
  2018-06-13 13:44     ` Daniel P. Berrangé
  1 sibling, 1 reply; 46+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 13:38 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
> We've got three ways of enabling an accelerator: -machine accel=xyz,
> -accel xyz and -enable-xyz. For new QEMU users, this must be very
> confusing ("Which one do I have to use? Is there a difference between
> the options?"). While -enable-kvm was useful in the past, there is no
> real good reason for using it anymore today ("-accel kvm" is even less
> to type than "-enable-kvm"), so let's decrease the confusing amount of
> options in our documenation a little bit by removing the -enable-xyz
> here. Note that the option itself is neither removed nor marked as
> deprecated - since -enable-kvm is likely used in a lot of scripts and
> since its code is easy to maintain, we should keep it around to avoid
> to break old setups.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)

It's widely used and we're removing the documentation for it?!  That is
likely to cause issues for new users who refer to the man page to
understand the QEMU command-lines they see online, in scripts, etc.

IMO -enable-kvm is not worth inconveniencing users about.  We gain very
little from abolishing it from docs and/or code, but its absence will
annoy our users.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
  2018-06-13 12:20   ` Eric Blake
  2018-06-13 12:48   ` Paolo Bonzini
@ 2018-06-13 13:38   ` Stefan Hajnoczi
  2 siblings, 0 replies; 46+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 13:38 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On Wed, Jun 13, 2018 at 07:05:18AM +0200, Thomas Huth wrote:
> We've got a separate option to configure the accelerator nowadays.
> Use it in the source and examples to demonstrate that this is the
> preferred way of setting this option now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qemu-doc.texi                       | 3 +--
>  qemu-options.hx                     | 2 +-
>  scripts/qtest.py                    | 2 +-
>  tests/bios-tables-test.c            | 2 +-
>  tests/boot-serial-test.c            | 2 +-
>  tests/libqtest.c                    | 2 +-
>  tests/migration-test.c              | 8 ++++----
>  tests/migration/guestperf/engine.py | 2 +-
>  tests/pnv-xscom-test.c              | 2 +-
>  tests/prom-env-test.c               | 2 +-
>  tests/pxe-test.c                    | 2 +-
>  tests/qemu-iotests/172              | 2 +-
>  tests/qemu-iotests/check            | 2 +-
>  tests/vmgenid-test.c                | 2 +-
>  14 files changed, 17 insertions(+), 18 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts Thomas Huth
  2018-06-13 12:51   ` Paolo Bonzini
@ 2018-06-13 13:38   ` Stefan Hajnoczi
  1 sibling, 0 replies; 46+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 13:38 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Wed, Jun 13, 2018 at 07:05:19AM +0200, Thomas Huth wrote:
> The preferred way to select the KVM accelerator is to use "-accel kvm"
> these days, so let's be consistent in our documentation and help texts.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/COLO-FT.txt                  | 8 ++++----
>  docs/can.txt                      | 4 ++--
>  docs/multi-thread-compression.txt | 2 +-
>  docs/multiseat.txt                | 2 +-
>  docs/specs/tpm.txt                | 8 ++++----
>  hw/block/dataplane/virtio-blk.c   | 4 ++--
>  hw/scsi/virtio-scsi-dataplane.c   | 4 ++--
>  7 files changed, 16 insertions(+), 16 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...'
  2018-06-13  5:05 ` [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...' Thomas Huth
@ 2018-06-13 13:38   ` Stefan Hajnoczi
  0 siblings, 0 replies; 46+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 13:38 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Wed, Jun 13, 2018 at 07:05:20AM +0200, Thomas Huth wrote:
> Instead of repeating the same text for both options, let's rather change
> the '-machine accel' documentation to point to the preferred '-accel' option
> instead.
> And in the documenation of the -accel option, make it clear that you can
> use colons to specify multiple accelerators.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qemu-options.hx | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] Clean up accelerator options
  2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
                   ` (3 preceding siblings ...)
  2018-06-13  5:05 ` [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore Thomas Huth
@ 2018-06-13 13:39 ` Stefan Hajnoczi
  4 siblings, 0 replies; 46+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 13:39 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Wed, Jun 13, 2018 at 07:05:17AM +0200, Thomas Huth wrote:
> The current state of the accelerator options is quite messy: There are
> three ways of enabling an accelerator (-machine accel=xyz, -accel xyz
> and -enable-xyz) and this can be very confusing for the users. This
> patch series now tries to standardize our documentation and examples
> on the preferred -accel option.

Nice, thank you for keeping things tidy.

Stefan

> Thomas Huth (4):
>   Replace '-machine accel=xyz' with '-accel xyz'
>   Replace '-enable-kvm' with '-accel kvm' in docs and help texts
>   qemu-options: Improve the documentation of '-accel' and '-machine
>     accel=...'
>   qemu-options: Do not show -enable-kvm and -enable-hax in the docs
>     anymore
> 
>  docs/COLO-FT.txt                    |  8 +++----
>  docs/can.txt                        |  4 ++--
>  docs/multi-thread-compression.txt   |  2 +-
>  docs/multiseat.txt                  |  2 +-
>  docs/specs/tpm.txt                  |  8 +++----
>  hw/block/dataplane/virtio-blk.c     |  4 ++--
>  hw/scsi/virtio-scsi-dataplane.c     |  4 ++--
>  qemu-doc.texi                       |  3 +--
>  qemu-options.hx                     | 44 ++++++++++++-------------------------
>  scripts/qtest.py                    |  2 +-
>  tests/bios-tables-test.c            |  2 +-
>  tests/boot-serial-test.c            |  2 +-
>  tests/libqtest.c                    |  2 +-
>  tests/migration-test.c              |  8 +++----
>  tests/migration/guestperf/engine.py |  2 +-
>  tests/pnv-xscom-test.c              |  2 +-
>  tests/prom-env-test.c               |  2 +-
>  tests/pxe-test.c                    |  2 +-
>  tests/qemu-iotests/172              |  2 +-
>  tests/qemu-iotests/check            |  2 +-
>  tests/vmgenid-test.c                |  2 +-
>  21 files changed, 46 insertions(+), 63 deletions(-)
> 
> -- 
> 1.8.3.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 13:38   ` Stefan Hajnoczi
@ 2018-06-13 13:44     ` Daniel P. Berrangé
  2018-06-13 15:11       ` Thomas Huth
  0 siblings, 1 reply; 46+ messages in thread
From: Daniel P. Berrangé @ 2018-06-13 13:44 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Thomas Huth, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	Markus Armbruster, qemu-devel, Paolo Bonzini, Eduardo Habkost

On Wed, Jun 13, 2018 at 02:38:40PM +0100, Stefan Hajnoczi wrote:
> On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
> > We've got three ways of enabling an accelerator: -machine accel=xyz,
> > -accel xyz and -enable-xyz. For new QEMU users, this must be very
> > confusing ("Which one do I have to use? Is there a difference between
> > the options?"). While -enable-kvm was useful in the past, there is no
> > real good reason for using it anymore today ("-accel kvm" is even less
> > to type than "-enable-kvm"), so let's decrease the confusing amount of
> > options in our documenation a little bit by removing the -enable-xyz
> > here. Note that the option itself is neither removed nor marked as
> > deprecated - since -enable-kvm is likely used in a lot of scripts and
> > since its code is easy to maintain, we should keep it around to avoid
> > to break old setups.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)
> 
> It's widely used and we're removing the documentation for it?!  That is
> likely to cause issues for new users who refer to the man page to
> understand the QEMU command-lines they see online, in scripts, etc.

Agreed, this is a very bad idea. Any option that is accepted by QEMU,
but not documented is a bug that must be fixed. IOW removing docs
is creating bugs.

If we want to help users understand why we have -enable-kvm, just
make the docs say that it is syntactic for '-machine accel=kvm'.
Users can decide for themselves whether they want to switch to
the more verbose way or not

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13 12:53     ` Thomas Huth
@ 2018-06-13 13:54       ` Paolo Bonzini
  2018-06-13 14:12         ` Thomas Huth
  2018-06-13 16:10       ` Eduardo Habkost
  1 sibling, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-13 13:54 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Stefan Hajnoczi, Eduardo Habkost, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On 13/06/2018 14:53, Thomas Huth wrote:
> On 13.06.2018 14:48, Paolo Bonzini wrote:
>> On 13/06/2018 07:05, Thomas Huth wrote:
>>> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
>>> index 8d915c6..4324034 100644
>>> --- a/tests/vmgenid-test.c
>>> +++ b/tests/vmgenid-test.c
>>> @@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
>>>  static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
>>>  
>>>  #define GUID_CMD(guid)                          \
>>> -    "-machine accel=kvm:tcg "                   \
>>> +    "-accel kvm:tcg "                           \
>>>      "-device vmgenid,id=testvgid,guid=%s "      \
>>
>> "-accel kvm:tcg" works, but it really shouldn't (and I think we can
>> change it without a deprecation period).   The right syntax would be
>> "-accel kvm -accel tcg", so that you can specify options that are valid
>> only for KVM, or onlty for TCG.
> 
> I see your point, but this would break these qtests that are trying to
> override the "-machine accel=qtest" from libqtest.c this way...

The solution would be to put "-accel kvm -accel tcg" before "-accel qtest".

> and if
> any other tool out there in the wild is already depending on this
> behavior, too, we can not change it so easily anymore.

I think -accel is new and obscure enough that it's unlikely to be used
with a colon.  In my opinion, the risk is worth the benefit of finally
getting a proper interface for accelerators.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13 13:54       ` Paolo Bonzini
@ 2018-06-13 14:12         ` Thomas Huth
  0 siblings, 0 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13 14:12 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: Eduardo Habkost, Ben Warren, qemu-trivial, Markus Armbruster,
	Stefan Hajnoczi, zhang.zhanghailiang

On 13.06.2018 15:54, Paolo Bonzini wrote:
> On 13/06/2018 14:53, Thomas Huth wrote:
>> On 13.06.2018 14:48, Paolo Bonzini wrote:
>>> On 13/06/2018 07:05, Thomas Huth wrote:
>>>> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
>>>> index 8d915c6..4324034 100644
>>>> --- a/tests/vmgenid-test.c
>>>> +++ b/tests/vmgenid-test.c
>>>> @@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
>>>>  static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
>>>>  
>>>>  #define GUID_CMD(guid)                          \
>>>> -    "-machine accel=kvm:tcg "                   \
>>>> +    "-accel kvm:tcg "                           \
>>>>      "-device vmgenid,id=testvgid,guid=%s "      \
>>>
>>> "-accel kvm:tcg" works, but it really shouldn't (and I think we can
>>> change it without a deprecation period).   The right syntax would be
>>> "-accel kvm -accel tcg", so that you can specify options that are valid
>>> only for KVM, or onlty for TCG.
>>
>> I see your point, but this would break these qtests that are trying to
>> override the "-machine accel=qtest" from libqtest.c this way...
> 
> The solution would be to put "-accel kvm -accel tcg" before "-accel qtest".
> 
>> and if
>> any other tool out there in the wild is already depending on this
>> behavior, too, we can not change it so easily anymore.
> 
> I think -accel is new and obscure enough that it's unlikely to be used
> with a colon.  In my opinion, the risk is worth the benefit of finally
> getting a proper interface for accelerators.

I agree with you. Let's try to get this right before it's too late!

 Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 13:44     ` Daniel P. Berrangé
@ 2018-06-13 15:11       ` Thomas Huth
  2018-06-13 15:19         ` Daniel P. Berrangé
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Huth @ 2018-06-13 15:11 UTC (permalink / raw)
  To: Daniel P. Berrangé, Stefan Hajnoczi
  Cc: zhang.zhanghailiang, Ben Warren, qemu-trivial, Markus Armbruster,
	qemu-devel, Paolo Bonzini, Eduardo Habkost

On 13.06.2018 15:44, Daniel P. Berrangé wrote:
> On Wed, Jun 13, 2018 at 02:38:40PM +0100, Stefan Hajnoczi wrote:
>> On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
>>> We've got three ways of enabling an accelerator: -machine accel=xyz,
>>> -accel xyz and -enable-xyz. For new QEMU users, this must be very
>>> confusing ("Which one do I have to use? Is there a difference between
>>> the options?"). While -enable-kvm was useful in the past, there is no
>>> real good reason for using it anymore today ("-accel kvm" is even less
>>> to type than "-enable-kvm"), so let's decrease the confusing amount of
>>> options in our documenation a little bit by removing the -enable-xyz
>>> here. Note that the option itself is neither removed nor marked as
>>> deprecated - since -enable-kvm is likely used in a lot of scripts and
>>> since its code is easy to maintain, we should keep it around to avoid
>>> to break old setups.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)
>>
>> It's widely used and we're removing the documentation for it?!  That is
>> likely to cause issues for new users who refer to the man page to
>> understand the QEMU command-lines they see online, in scripts, etc.
> 
> Agreed, this is a very bad idea. Any option that is accepted by QEMU,
> but not documented is a bug that must be fixed. IOW removing docs
> is creating bugs.

Not documenting unliked options that are still kept for compatibility
was at least a common practice in the past (see -no-kvm for example, or
many of those deprecated options like -net channel that have been
removed in the past year).

> If we want to help users understand why we have -enable-kvm, just
> make the docs say that it is syntactic for '-machine accel=kvm'.
> Users can decide for themselves whether they want to switch to
> the more verbose way or not

Uh, well, in this case "-enable-kvm" is already the more verbose way:
"-accel kvm" is shorter :-)

It's just a big mess: We've got -enable-kvm, -enable-hax, but there is
no -enable-hvf, -enable-whpx or -enable-xen option. And to force TCG
mode, you've got to use -no-kvm ... honestly, if I were a new user, I'd
simply say: WTF!?!

But ok, since -enable-kvm has such a big tradition and is used in a lot
of examples out there, it's likely really better if we keep it in the
documentation. But we should either move it to a "obsolete option"
chapter, or update the current documentation with some words like
"obsolete" or "legacy" (to make it clear that nobody gets the idea of
introducing -enable-hvf or other similar options in the future).

And what about -enable-hax? That hardly has any tradtion. Should we
maybe even deprecate it?

 Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 15:11       ` Thomas Huth
@ 2018-06-13 15:19         ` Daniel P. Berrangé
  2018-06-13 15:44           ` Thomas Huth
  2018-06-13 16:02           ` Markus Armbruster
  0 siblings, 2 replies; 46+ messages in thread
From: Daniel P. Berrangé @ 2018-06-13 15:19 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Stefan Hajnoczi, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	Markus Armbruster, qemu-devel, Paolo Bonzini, Eduardo Habkost

On Wed, Jun 13, 2018 at 05:11:51PM +0200, Thomas Huth wrote:
> On 13.06.2018 15:44, Daniel P. Berrangé wrote:
> > On Wed, Jun 13, 2018 at 02:38:40PM +0100, Stefan Hajnoczi wrote:
> >> On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
> >>> We've got three ways of enabling an accelerator: -machine accel=xyz,
> >>> -accel xyz and -enable-xyz. For new QEMU users, this must be very
> >>> confusing ("Which one do I have to use? Is there a difference between
> >>> the options?"). While -enable-kvm was useful in the past, there is no
> >>> real good reason for using it anymore today ("-accel kvm" is even less
> >>> to type than "-enable-kvm"), so let's decrease the confusing amount of
> >>> options in our documenation a little bit by removing the -enable-xyz
> >>> here. Note that the option itself is neither removed nor marked as
> >>> deprecated - since -enable-kvm is likely used in a lot of scripts and
> >>> since its code is easy to maintain, we should keep it around to avoid
> >>> to break old setups.
> >>>
> >>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >>> ---
> >>>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)
> >>
> >> It's widely used and we're removing the documentation for it?!  That is
> >> likely to cause issues for new users who refer to the man page to
> >> understand the QEMU command-lines they see online, in scripts, etc.
> > 
> > Agreed, this is a very bad idea. Any option that is accepted by QEMU,
> > but not documented is a bug that must be fixed. IOW removing docs
> > is creating bugs.
> 
> Not documenting unliked options that are still kept for compatibility
> was at least a common practice in the past (see -no-kvm for example, or
> many of those deprecated options like -net channel that have been
> removed in the past year).

If we're planning to deprecate & then delete an option, then I
don't mind if docs are dropped, but IIUC, in this case we're
not doing that - the option will essentially exist forever.

> > If we want to help users understand why we have -enable-kvm, just
> > make the docs say that it is syntactic for '-machine accel=kvm'.
> > Users can decide for themselves whether they want to switch to
> > the more verbose way or not
> 
> Uh, well, in this case "-enable-kvm" is already the more verbose way:
> "-accel kvm" is shorter :-)

If I'm a user looking for how to enable KVM, then -enable-kvm is the
one I'll pick because of the obvious name.

> It's just a big mess: We've got -enable-kvm, -enable-hax, but there is
> no -enable-hvf, -enable-whpx or -enable-xen option. And to force TCG
> mode, you've got to use -no-kvm ... honestly, if I were a new user, I'd
> simply say: WTF!?!

Personally I'd just clean that up by just adding the missing
-enable-xxx options for consistency :-)

> But ok, since -enable-kvm has such a big tradition and is used in a lot
> of examples out there, it's likely really better if we keep it in the
> documentation. But we should either move it to a "obsolete option"
> chapter, or update the current documentation with some words like
> "obsolete" or "legacy" (to make it clear that nobody gets the idea of
> introducing -enable-hvf or other similar options in the future).
> 
> And what about -enable-hax? That hardly has any tradtion. Should we
> maybe even deprecate it?

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 15:19         ` Daniel P. Berrangé
@ 2018-06-13 15:44           ` Thomas Huth
  2018-06-13 16:02           ` Markus Armbruster
  1 sibling, 0 replies; 46+ messages in thread
From: Thomas Huth @ 2018-06-13 15:44 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Stefan Hajnoczi, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	Markus Armbruster, qemu-devel, Paolo Bonzini, Eduardo Habkost

On 13.06.2018 17:19, Daniel P. Berrangé wrote:
> On Wed, Jun 13, 2018 at 05:11:51PM +0200, Thomas Huth wrote:
>> On 13.06.2018 15:44, Daniel P. Berrangé wrote:
>>> On Wed, Jun 13, 2018 at 02:38:40PM +0100, Stefan Hajnoczi wrote:
>>>> On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
>>>>> We've got three ways of enabling an accelerator: -machine accel=xyz,
>>>>> -accel xyz and -enable-xyz. For new QEMU users, this must be very
>>>>> confusing ("Which one do I have to use? Is there a difference between
>>>>> the options?"). While -enable-kvm was useful in the past, there is no
>>>>> real good reason for using it anymore today ("-accel kvm" is even less
>>>>> to type than "-enable-kvm"), so let's decrease the confusing amount of
>>>>> options in our documenation a little bit by removing the -enable-xyz
>>>>> here. Note that the option itself is neither removed nor marked as
>>>>> deprecated - since -enable-kvm is likely used in a lot of scripts and
>>>>> since its code is easy to maintain, we should keep it around to avoid
>>>>> to break old setups.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)
>>>>
>>>> It's widely used and we're removing the documentation for it?!  That is
>>>> likely to cause issues for new users who refer to the man page to
>>>> understand the QEMU command-lines they see online, in scripts, etc.
>>>
>>> Agreed, this is a very bad idea. Any option that is accepted by QEMU,
>>> but not documented is a bug that must be fixed. IOW removing docs
>>> is creating bugs.
>>
>> Not documenting unliked options that are still kept for compatibility
>> was at least a common practice in the past (see -no-kvm for example, or
>> many of those deprecated options like -net channel that have been
>> removed in the past year).
> 
> If we're planning to deprecate & then delete an option, then I
> don't mind if docs are dropped, but IIUC, in this case we're
> not doing that - the option will essentially exist forever.
> 
>>> If we want to help users understand why we have -enable-kvm, just
>>> make the docs say that it is syntactic for '-machine accel=kvm'.
>>> Users can decide for themselves whether they want to switch to
>>> the more verbose way or not
>>
>> Uh, well, in this case "-enable-kvm" is already the more verbose way:
>> "-accel kvm" is shorter :-)
> 
> If I'm a user looking for how to enable KVM, then -enable-kvm is the
> one I'll pick because of the obvious name.

Hmm, maybe we should also add -configure-network-backend,
--configure-character-device-backend and -setup-block-backend as
synonyms for -netdev, -chardev and -blockdev, just because they have a
more obvious name? ;-)

 Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 15:19         ` Daniel P. Berrangé
  2018-06-13 15:44           ` Thomas Huth
@ 2018-06-13 16:02           ` Markus Armbruster
  2018-06-19 15:15             ` Cornelia Huck
  1 sibling, 1 reply; 46+ messages in thread
From: Markus Armbruster @ 2018-06-13 16:02 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Thomas Huth, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	qemu-devel, Markus Armbruster, Stefan Hajnoczi, Paolo Bonzini,
	Eduardo Habkost

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Jun 13, 2018 at 05:11:51PM +0200, Thomas Huth wrote:
>> On 13.06.2018 15:44, Daniel P. Berrangé wrote:
>> > On Wed, Jun 13, 2018 at 02:38:40PM +0100, Stefan Hajnoczi wrote:
>> >> On Wed, Jun 13, 2018 at 07:05:21AM +0200, Thomas Huth wrote:
>> >>> We've got three ways of enabling an accelerator: -machine accel=xyz,
>> >>> -accel xyz and -enable-xyz. For new QEMU users, this must be very
>> >>> confusing ("Which one do I have to use? Is there a difference between
>> >>> the options?"). While -enable-kvm was useful in the past, there is no
>> >>> real good reason for using it anymore today ("-accel kvm" is even less
>> >>> to type than "-enable-kvm"), so let's decrease the confusing amount of
>> >>> options in our documenation a little bit by removing the -enable-xyz
>> >>> here. Note that the option itself is neither removed nor marked as
>> >>> deprecated - since -enable-kvm is likely used in a lot of scripts and
>> >>> since its code is easy to maintain, we should keep it around to avoid
>> >>> to break old setups.
>> >>>
>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> >>> ---
>> >>>  PS: I guess Paolo won't like this patch ... let's try it anyway ;-)
>> >>
>> >> It's widely used and we're removing the documentation for it?!  That is
>> >> likely to cause issues for new users who refer to the man page to
>> >> understand the QEMU command-lines they see online, in scripts, etc.
>> > 
>> > Agreed, this is a very bad idea. Any option that is accepted by QEMU,
>> > but not documented is a bug that must be fixed. IOW removing docs
>> > is creating bugs.
>> 
>> Not documenting unliked options that are still kept for compatibility
>> was at least a common practice in the past (see -no-kvm for example, or
>> many of those deprecated options like -net channel that have been
>> removed in the past year).

Not least because both --help output and the user manual are hard enough
to read without them droning about umpteen deprecated things you could
also use, but shouldn't.

> If we're planning to deprecate & then delete an option, then I
> don't mind if docs are dropped,

De-documenting deprecated options that warn "use this instead" feels
like a no-brainer to me.

>                                 but IIUC, in this case we're
> not doing that - the option will essentially exist forever.

Deprecated option: something we don't want users to use, and intend to
remove.  It should warn on use, pointing to the replacement, and
documentation should no longer cover it.

Convenience option: something we consider perfectly fine to use, say
because it's much less typing.  Document normally.

Legacy option: something in between, i.e. we don't intend to remove it,
but we don't want to advertise it, either.  The less of those we have,
the happier I am.  Their documentation to be shunted out of the way, so
users can find it if they need it, but won't find it *first* when they
look for how to do something.

>> > If we want to help users understand why we have -enable-kvm, just
>> > make the docs say that it is syntactic for '-machine accel=kvm'.
>> > Users can decide for themselves whether they want to switch to
>> > the more verbose way or not
>> 
>> Uh, well, in this case "-enable-kvm" is already the more verbose way:
>> "-accel kvm" is shorter :-)
>
> If I'm a user looking for how to enable KVM, then -enable-kvm is the
> one I'll pick because of the obvious name.

Why does a user have to know how to enable KVM?  Oh, because our default
is "run this guest much slower than necessary".  Great!

By "pick", I guess you mean "pick out of output of --help".  If the only
occurence of KVM there was --accel kvm, I trust the user would pick that
without any trouble.  Less confusing than what we have now, I'd say.

>> It's just a big mess: We've got -enable-kvm, -enable-hax, but there is
>> no -enable-hvf, -enable-whpx or -enable-xen option. And to force TCG
>> mode, you've got to use -no-kvm ... honestly, if I were a new user, I'd
>> simply say: WTF!?!
>
> Personally I'd just clean that up by just adding the missing
> -enable-xxx options for consistency :-)

I disagree.  The way to a saner QEMU CLI is reducing crap, not adding
crap for consistency.

>> But ok, since -enable-kvm has such a big tradition and is used in a lot
>> of examples out there, it's likely really better if we keep it in the
>> documentation. But we should either move it to a "obsolete option"
>> chapter, or update the current documentation with some words like
>> "obsolete" or "legacy" (to make it clear that nobody gets the idea of
>> introducing -enable-hvf or other similar options in the future).
>> 
>> And what about -enable-hax? That hardly has any tradtion. Should we
>> maybe even deprecate it?

I would, but it's not a hill I'm prepared to die on.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz'
  2018-06-13 12:53     ` Thomas Huth
  2018-06-13 13:54       ` Paolo Bonzini
@ 2018-06-13 16:10       ` Eduardo Habkost
  1 sibling, 0 replies; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-13 16:10 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Ben Warren,
	zhang.zhanghailiang, Markus Armbruster, qemu-trivial

On Wed, Jun 13, 2018 at 02:53:04PM +0200, Thomas Huth wrote:
> On 13.06.2018 14:48, Paolo Bonzini wrote:
> > On 13/06/2018 07:05, Thomas Huth wrote:
> >> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> >> index 8d915c6..4324034 100644
> >> --- a/tests/vmgenid-test.c
> >> +++ b/tests/vmgenid-test.c
> >> @@ -131,7 +131,7 @@ static void read_guid_from_monitor(QemuUUID *guid)
> >>  static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
> >>  
> >>  #define GUID_CMD(guid)                          \
> >> -    "-machine accel=kvm:tcg "                   \
> >> +    "-accel kvm:tcg "                           \
> >>      "-device vmgenid,id=testvgid,guid=%s "      \
> > 
> > "-accel kvm:tcg" works, but it really shouldn't (and I think we can
> > change it without a deprecation period).   The right syntax would be
> > "-accel kvm -accel tcg", so that you can specify options that are valid
> > only for KVM, or onlty for TCG.
> 
> I see your point, but this would break these qtests that are trying to
> override the "-machine accel=qtest" from libqtest.c this way... and if
> any other tool out there in the wild is already depending on this
> behavior, too, we can not change it so easily anymore.

Tools might rely in "-machine accel=kvm:tcg", but I don't think
anybody should be relying on "-accel kvm:tcg" to work.

I agree with Paolo.  If "-accel kvm:tcg" works today, I think we
should either remove that behavior immediately, or deprecate it
on QEMU 3.0.

We should also make sure "-accel kvm,FOO=BAR -accel tcg" won't
set FOO=BAR in the TCG accelerator if KVM is unavailable.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-13 16:02           ` Markus Armbruster
@ 2018-06-19 15:15             ` Cornelia Huck
  2018-06-19 16:16               ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Cornelia Huck @ 2018-06-19 15:15 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Daniel P. Berrangé,
	Thomas Huth, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	qemu-devel, Stefan Hajnoczi, Paolo Bonzini, Eduardo Habkost

On Wed, 13 Jun 2018 18:02:27 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Daniel P. Berrangé <berrange@redhat.com> writes:

> > If I'm a user looking for how to enable KVM, then -enable-kvm is the
> > one I'll pick because of the obvious name.  
> 
> Why does a user have to know how to enable KVM?  Oh, because our default
> is "run this guest much slower than necessary".  Great!

Should we try again to default to a better accelerator, if possible? I
don't quite recall why we didn't do so the last time that came up...
was it tests?

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-19 15:15             ` Cornelia Huck
@ 2018-06-19 16:16               ` Paolo Bonzini
  2018-06-22 18:11                 ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-19 16:16 UTC (permalink / raw)
  To: Cornelia Huck, Markus Armbruster
  Cc: Daniel P. Berrangé,
	Thomas Huth, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	qemu-devel, Stefan Hajnoczi, Eduardo Habkost

On 19/06/2018 17:15, Cornelia Huck wrote:
>> Why does a user have to know how to enable KVM?  Oh, because our default
>> is "run this guest much slower than necessary".  Great!
> Should we try again to default to a better accelerator, if possible? I
> don't quite recall why we didn't do so the last time that came up...
> was it tests?

My plan was to create qemu-{kvm,hax,hvf,whpx} binaries that default to a
better accelerator, and leave qemu-system-* as defaulting to TCG.  This
matches what distributions already do.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-19 16:16               ` Paolo Bonzini
@ 2018-06-22 18:11                 ` Eduardo Habkost
  2018-06-22 19:19                   ` Thomas Huth
  0 siblings, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-22 18:11 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Cornelia Huck, Markus Armbruster, Daniel P. Berrangé,
	Thomas Huth, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	qemu-devel, Stefan Hajnoczi

On Tue, Jun 19, 2018 at 06:16:33PM +0200, Paolo Bonzini wrote:
> On 19/06/2018 17:15, Cornelia Huck wrote:
> >> Why does a user have to know how to enable KVM?  Oh, because our default
> >> is "run this guest much slower than necessary".  Great!
> > Should we try again to default to a better accelerator, if possible? I
> > don't quite recall why we didn't do so the last time that came up...
> > was it tests?
> 
> My plan was to create qemu-{kvm,hax,hvf,whpx} binaries that default to a
> better accelerator, and leave qemu-system-* as defaulting to TCG.  This
> matches what distributions already do.

Why is this better than using KVM by default if it's available?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-22 18:11                 ` Eduardo Habkost
@ 2018-06-22 19:19                   ` Thomas Huth
  2018-06-22 19:35                     ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Huth @ 2018-06-22 19:19 UTC (permalink / raw)
  To: Eduardo Habkost, Paolo Bonzini
  Cc: Cornelia Huck, Markus Armbruster, Daniel P. Berrangé,
	zhang.zhanghailiang, Ben Warren, qemu-trivial, qemu-devel,
	Stefan Hajnoczi

On 22.06.2018 20:11, Eduardo Habkost wrote:
> On Tue, Jun 19, 2018 at 06:16:33PM +0200, Paolo Bonzini wrote:
>> On 19/06/2018 17:15, Cornelia Huck wrote:
>>>> Why does a user have to know how to enable KVM?  Oh, because our default
>>>> is "run this guest much slower than necessary".  Great!
>>> Should we try again to default to a better accelerator, if possible? I
>>> don't quite recall why we didn't do so the last time that came up...
>>> was it tests?
>>
>> My plan was to create qemu-{kvm,hax,hvf,whpx} binaries that default to a
>> better accelerator, and leave qemu-system-* as defaulting to TCG.  This
>> matches what distributions already do.
> 
> Why is this better than using KVM by default if it's available?

The answer is (as almost always): Compatibility with migration. Nobody
dares to sacrifice that chicken :-(

 Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-22 19:19                   ` Thomas Huth
@ 2018-06-22 19:35                     ` Eduardo Habkost
  2018-06-22 21:22                       ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-22 19:35 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, Cornelia Huck, Markus Armbruster,
	Daniel P. Berrangé,
	zhang.zhanghailiang, Ben Warren, qemu-trivial, qemu-devel,
	Stefan Hajnoczi

On Fri, Jun 22, 2018 at 09:19:56PM +0200, Thomas Huth wrote:
> On 22.06.2018 20:11, Eduardo Habkost wrote:
> > On Tue, Jun 19, 2018 at 06:16:33PM +0200, Paolo Bonzini wrote:
> >> On 19/06/2018 17:15, Cornelia Huck wrote:
> >>>> Why does a user have to know how to enable KVM?  Oh, because our default
> >>>> is "run this guest much slower than necessary".  Great!
> >>> Should we try again to default to a better accelerator, if possible? I
> >>> don't quite recall why we didn't do so the last time that came up...
> >>> was it tests?
> >>
> >> My plan was to create qemu-{kvm,hax,hvf,whpx} binaries that default to a
> >> better accelerator, and leave qemu-system-* as defaulting to TCG.  This
> >> matches what distributions already do.
> > 
> > Why is this better than using KVM by default if it's available?
> 
> The answer is (as almost always): Compatibility with migration. Nobody
> dares to sacrifice that chicken :-(

We can now kill it if we announce the feature as deprecated a
couple of releases in advance.

If we declare that compatibility when the accelerator is omitted
is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
different default accelerator.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-22 19:35                     ` Eduardo Habkost
@ 2018-06-22 21:22                       ` Paolo Bonzini
  2018-06-25  6:50                         ` Markus Armbruster
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-22 21:22 UTC (permalink / raw)
  To: Eduardo Habkost, Thomas Huth
  Cc: Cornelia Huck, Markus Armbruster, Daniel P. Berrangé,
	zhang.zhanghailiang, Ben Warren, qemu-trivial, qemu-devel,
	Stefan Hajnoczi

On 22/06/2018 21:35, Eduardo Habkost wrote:
>>> Why is this better than using KVM by default if it's available?
>> The answer is (as almost always): Compatibility with migration. Nobody
>> dares to sacrifice that chicken :-(
> We can now kill it if we announce the feature as deprecated a
> couple of releases in advance.
> 
> If we declare that compatibility when the accelerator is omitted
> is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
> different default accelerator.

We can, we don't necessarily want it.

The status quo is that people using KVM are invoking qemu as qemu-kvm,
people using TCG are invoking qemu as qemu-system-*.  All distros are
shipping a qemu-kvm or more rarely kvm binary, which is invariably a
wrapper script except for RHEL because RHEL doesn't have a qemu-system-*
binary at all.

By the way, changing qemu-system-*'s default to e.g. RHEL's "kvm or tcg"
would not help distros that have "-accel kvm" in their /usr/bin/qemu-kvm
script.

All in all, it seems simpler for me to take the status quo, which is
what non-RHEL distros do, and make it part of upstream.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-22 21:22                       ` Paolo Bonzini
@ 2018-06-25  6:50                         ` Markus Armbruster
  2018-06-25 10:28                           ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Markus Armbruster @ 2018-06-25  6:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Eduardo Habkost, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, Markus Armbruster, qemu-devel,
	Stefan Hajnoczi

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 22/06/2018 21:35, Eduardo Habkost wrote:
>>>> Why is this better than using KVM by default if it's available?
>>> The answer is (as almost always): Compatibility with migration. Nobody
>>> dares to sacrifice that chicken :-(
>> We can now kill it if we announce the feature as deprecated a
>> couple of releases in advance.
>> 
>> If we declare that compatibility when the accelerator is omitted
>> is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
>> different default accelerator.
>
> We can, we don't necessarily want it.
>
> The status quo is that people using KVM are invoking qemu as qemu-kvm,
> people using TCG are invoking qemu as qemu-system-*.  All distros are
> shipping a qemu-kvm or more rarely kvm binary, which is invariably a
> wrapper script except for RHEL because RHEL doesn't have a qemu-system-*
> binary at all.
>
> By the way, changing qemu-system-*'s default to e.g. RHEL's "kvm or tcg"
> would not help distros that have "-accel kvm" in their /usr/bin/qemu-kvm
> script.

It wouldn't hurt them, either.

Attentive distros could even replace the wrapper script by a link.

> All in all, it seems simpler for me to take the status quo, which is
> what non-RHEL distros do, and make it part of upstream.
>
> Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25  6:50                         ` Markus Armbruster
@ 2018-06-25 10:28                           ` Paolo Bonzini
  2018-06-25 17:30                             ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-25 10:28 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Eduardo Habkost, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 25/06/2018 08:50, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 22/06/2018 21:35, Eduardo Habkost wrote:
>>>>> Why is this better than using KVM by default if it's available?
>>>> The answer is (as almost always): Compatibility with migration. Nobody
>>>> dares to sacrifice that chicken :-(
>>> We can now kill it if we announce the feature as deprecated a
>>> couple of releases in advance.
>>>
>>> If we declare that compatibility when the accelerator is omitted
>>> is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
>>> different default accelerator.
>>
>> We can, we don't necessarily want it.
>>
>> The status quo is that people using KVM are invoking qemu as qemu-kvm,
>> people using TCG are invoking qemu as qemu-system-*.  All distros are
>> shipping a qemu-kvm or more rarely kvm binary, which is invariably a
>> wrapper script except for RHEL because RHEL doesn't have a qemu-system-*
>> binary at all.
>>
>> By the way, changing qemu-system-*'s default to e.g. RHEL's "kvm or tcg"
>> would not help distros that have "-accel kvm" in their /usr/bin/qemu-kvm
>> script.
> 
> It wouldn't hurt them, either.

Right; to sum up, it does make things a little less consistent for their
users in two ways:

- qemu-system-<native> behaves differently from qemu-system-<others>.
For example, for ARM the default CPU model might not work for KVM, so
you would have to add a "-cpu xxx" option.

- qemu-system-<native> would still need an accelerator option on OS X or
Windows, where there is not quite parity between TCG and the native
accelerator, in terms of either features or stability.  Because of this
we wouldn't be able to change the default to "whatever virtualizing
accelerators are available followed by TCG".

> Attentive distros could even replace the wrapper script by a link.

If they are okay with replacing the "KVM only" semantics with "KVM or
TCG", which I think is generally worse.

Paolo

>> All in all, it seems simpler for me to take the status quo, which is
>> what non-RHEL distros do, and make it part of upstream.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25 10:28                           ` Paolo Bonzini
@ 2018-06-25 17:30                             ` Eduardo Habkost
  2018-06-25 18:26                               ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-25 17:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On Mon, Jun 25, 2018 at 12:28:34PM +0200, Paolo Bonzini wrote:
> On 25/06/2018 08:50, Markus Armbruster wrote:
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> > 
> >> On 22/06/2018 21:35, Eduardo Habkost wrote:
> >>>>> Why is this better than using KVM by default if it's available?
> >>>> The answer is (as almost always): Compatibility with migration. Nobody
> >>>> dares to sacrifice that chicken :-(
> >>> We can now kill it if we announce the feature as deprecated a
> >>> couple of releases in advance.
> >>>
> >>> If we declare that compatibility when the accelerator is omitted
> >>> is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
> >>> different default accelerator.
> >>
> >> We can, we don't necessarily want it.
> >>
> >> The status quo is that people using KVM are invoking qemu as qemu-kvm,
> >> people using TCG are invoking qemu as qemu-system-*.  All distros are
> >> shipping a qemu-kvm or more rarely kvm binary, which is invariably a
> >> wrapper script except for RHEL because RHEL doesn't have a qemu-system-*
> >> binary at all.
> >>
> >> By the way, changing qemu-system-*'s default to e.g. RHEL's "kvm or tcg"
> >> would not help distros that have "-accel kvm" in their /usr/bin/qemu-kvm
> >> script.
> > 
> > It wouldn't hurt them, either.
> 
> Right; to sum up, it does make things a little less consistent for their
> users in two ways:
> 
> - qemu-system-<native> behaves differently from qemu-system-<others>.
> For example, for ARM the default CPU model might not work for KVM, so
> you would have to add a "-cpu xxx" option.
> 
> - qemu-system-<native> would still need an accelerator option on OS X or
> Windows, where there is not quite parity between TCG and the native
> accelerator, in terms of either features or stability.  Because of this
> we wouldn't be able to change the default to "whatever virtualizing
> accelerators are available followed by TCG".
> 
> > Attentive distros could even replace the wrapper script by a link.
> 
> If they are okay with replacing the "KVM only" semantics with "KVM or
> TCG", which I think is generally worse.

If we can't get agreement on what's the right default for each
QEMU binary, I think that's yet another reason to document that
upstream QEMU won't guarantee ABI compatibility if -accel is
omitted.

If downstream distributions want to keep promising ABI
compatibility, it will be up to them.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25 17:30                             ` Eduardo Habkost
@ 2018-06-25 18:26                               ` Paolo Bonzini
  2018-06-25 19:51                                 ` Eduardo Habkost
  2018-06-26  4:40                                 ` Thomas Huth
  0 siblings, 2 replies; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-25 18:26 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 25/06/2018 19:30, Eduardo Habkost wrote:
>>> Attentive distros could even replace the wrapper script by a link.
>> If they are okay with replacing the "KVM only" semantics with "KVM or
>> TCG", which I think is generally worse.
>
> If we can't get agreement on what's the right default for each
> QEMU binary, I think that's yet another reason to document that
> upstream QEMU won't guarantee ABI compatibility if -accel is
> omitted.

Before that we should ask what the benefit is in changing the default
for qemu-system-*.  Nobody is using it in practice to start QEMU with
KVM enabled...

Paolo

> If downstream distributions want to keep promising ABI
> compatibility, it will be up to them.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25 18:26                               ` Paolo Bonzini
@ 2018-06-25 19:51                                 ` Eduardo Habkost
  2018-06-26  5:57                                   ` Paolo Bonzini
  2018-06-26  4:40                                 ` Thomas Huth
  1 sibling, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-25 19:51 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On Mon, Jun 25, 2018 at 08:26:28PM +0200, Paolo Bonzini wrote:
> On 25/06/2018 19:30, Eduardo Habkost wrote:
> >>> Attentive distros could even replace the wrapper script by a link.
> >> If they are okay with replacing the "KVM only" semantics with "KVM or
> >> TCG", which I think is generally worse.
> >
> > If we can't get agreement on what's the right default for each
> > QEMU binary, I think that's yet another reason to document that
> > upstream QEMU won't guarantee ABI compatibility if -accel is
> > omitted.
> 
> Before that we should ask what the benefit is in changing the default
> for qemu-system-*.  Nobody is using it in practice to start QEMU with
> KVM enabled...

How can you be sure?  If qemu-system-* is installed with KVM
compiled in, libvirt will probe it using
"-machine none,accel=kvm:tcg" and run the VM using
"-machine $MACHINE,accel=kvm".

In either case, I'm not arguing (yet) for changing the default
upstream.  I'm just arguing for upstream QEMU to not make any
promises about the default.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25 18:26                               ` Paolo Bonzini
  2018-06-25 19:51                                 ` Eduardo Habkost
@ 2018-06-26  4:40                                 ` Thomas Huth
  2018-06-26  7:50                                   ` Cornelia Huck
  1 sibling, 1 reply; 46+ messages in thread
From: Thomas Huth @ 2018-06-26  4:40 UTC (permalink / raw)
  To: Paolo Bonzini, Eduardo Habkost
  Cc: Markus Armbruster, zhang.zhanghailiang, Ben Warren, qemu-trivial,
	Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 25.06.2018 20:26, Paolo Bonzini wrote:
> On 25/06/2018 19:30, Eduardo Habkost wrote:
>>>> Attentive distros could even replace the wrapper script by a link.
>>> If they are okay with replacing the "KVM only" semantics with "KVM or
>>> TCG", which I think is generally worse.
>>
>> If we can't get agreement on what's the right default for each
>> QEMU binary, I think that's yet another reason to document that
>> upstream QEMU won't guarantee ABI compatibility if -accel is
>> omitted.
> 
> Before that we should ask what the benefit is in changing the default
> for qemu-system-*.  Nobody is using it in practice to start QEMU with
> KVM enabled...

That's certainly not true. I've seen a couple of times already that
people ask on IRC why their guests are running so slow, and if you ask
them about their command line, it's obvious that they simply were not
aware of "-accel" / "-enable-kvm" yet.

<semi-sarcastic>
Maybe we simply should add a "--verbose" command line option that people
can use to diagnose their problems:

$ qemu-system-x86_64 --verbose
QEMU emulator version 2.12.50
Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
....
</semi-sarcastic>

 Cheers,
  Thomas

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-25 19:51                                 ` Eduardo Habkost
@ 2018-06-26  5:57                                   ` Paolo Bonzini
  2018-06-26 12:29                                     ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-26  5:57 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 25/06/2018 21:51, Eduardo Habkost wrote:
>> Before that we should ask what the benefit is in changing the default
>> for qemu-system-*.  Nobody is using it in practice to start QEMU with
>> KVM enabled...
>
> How can you be sure?  If qemu-system-* is installed with KVM
> compiled in, libvirt will probe it using
> "-machine none,accel=kvm:tcg" and run the VM using
> "-machine $MACHINE,accel=kvm".

Right, that needs to be qualified with "without libvirt".

> In either case, I'm not arguing (yet) for changing the default
> upstream.  I'm just arguing for upstream QEMU to not make any
> promises about the default.

It would be a guest ABI breakage for TCG guests, so it would only apply
to new machine types.  I don't think it's worth the complication.

BTW, another thing that needs documenting is ABI promises for HAX and
WHPX.  Both do not support -cpu in any meaningful way, at least WHPX
installs a migration blocker.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26  4:40                                 ` Thomas Huth
@ 2018-06-26  7:50                                   ` Cornelia Huck
  2018-06-26 12:56                                     ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Cornelia Huck @ 2018-06-26  7:50 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, Eduardo Habkost, Markus Armbruster,
	zhang.zhanghailiang, Ben Warren, qemu-trivial, qemu-devel,
	Stefan Hajnoczi

On Tue, 26 Jun 2018 06:40:56 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 25.06.2018 20:26, Paolo Bonzini wrote:
> > On 25/06/2018 19:30, Eduardo Habkost wrote:  
> >>>> Attentive distros could even replace the wrapper script by a link.  
> >>> If they are okay with replacing the "KVM only" semantics with "KVM or
> >>> TCG", which I think is generally worse.  
> >>
> >> If we can't get agreement on what's the right default for each
> >> QEMU binary, I think that's yet another reason to document that
> >> upstream QEMU won't guarantee ABI compatibility if -accel is
> >> omitted.  
> > 
> > Before that we should ask what the benefit is in changing the default
> > for qemu-system-*.  Nobody is using it in practice to start QEMU with
> > KVM enabled...  
> 
> That's certainly not true. I've seen a couple of times already that
> people ask on IRC why their guests are running so slow, and if you ask
> them about their command line, it's obvious that they simply were not
> aware of "-accel" / "-enable-kvm" yet.
> 
> <semi-sarcastic>
> Maybe we simply should add a "--verbose" command line option that people
> can use to diagnose their problems:
> 
> $ qemu-system-x86_64 --verbose
> QEMU emulator version 2.12.50
> Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
> Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
> ....
> </semi-sarcastic>

Not sure how serious you meant that, but I actually quite like the
idea :)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26  5:57                                   ` Paolo Bonzini
@ 2018-06-26 12:29                                     ` Eduardo Habkost
  2018-06-26 13:05                                       ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-26 12:29 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
> On 25/06/2018 21:51, Eduardo Habkost wrote:
> > In either case, I'm not arguing (yet) for changing the default
> > upstream.  I'm just arguing for upstream QEMU to not make any
> > promises about the default.
> 
> It would be a guest ABI breakage for TCG guests, so it would only apply
> to new machine types.  I don't think it's worth the complication.

That's exactly the point: I want to stop promising a stable guest
ABI when the accelerator is omitted, because I see no benefit in
wasting energy on this.

(I don't think we ever kept the guest ABI correctly with TCG, by
the way.)


> 
> BTW, another thing that needs documenting is ABI promises for HAX and
> WHPX.  [...]

Absolutely.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26  7:50                                   ` Cornelia Huck
@ 2018-06-26 12:56                                     ` Eduardo Habkost
  2018-06-26 16:43                                       ` Markus Armbruster
  0 siblings, 1 reply; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-26 12:56 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Thomas Huth, Paolo Bonzini, Markus Armbruster,
	zhang.zhanghailiang, Ben Warren, qemu-trivial, qemu-devel,
	Stefan Hajnoczi

On Tue, Jun 26, 2018 at 09:50:04AM +0200, Cornelia Huck wrote:
> On Tue, 26 Jun 2018 06:40:56 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 25.06.2018 20:26, Paolo Bonzini wrote:
> > > On 25/06/2018 19:30, Eduardo Habkost wrote:  
> > >>>> Attentive distros could even replace the wrapper script by a link.  
> > >>> If they are okay with replacing the "KVM only" semantics with "KVM or
> > >>> TCG", which I think is generally worse.  
> > >>
> > >> If we can't get agreement on what's the right default for each
> > >> QEMU binary, I think that's yet another reason to document that
> > >> upstream QEMU won't guarantee ABI compatibility if -accel is
> > >> omitted.  
> > > 
> > > Before that we should ask what the benefit is in changing the default
> > > for qemu-system-*.  Nobody is using it in practice to start QEMU with
> > > KVM enabled...  
> > 
> > That's certainly not true. I've seen a couple of times already that
> > people ask on IRC why their guests are running so slow, and if you ask
> > them about their command line, it's obvious that they simply were not
> > aware of "-accel" / "-enable-kvm" yet.
> > 
> > <semi-sarcastic>
> > Maybe we simply should add a "--verbose" command line option that people
> > can use to diagnose their problems:
> > 
> > $ qemu-system-x86_64 --verbose
> > QEMU emulator version 2.12.50
> > Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
> > Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
> > ....
> > </semi-sarcastic>
> 
> Not sure how serious you meant that, but I actually quite like the
> idea :)

Also, this mode could be enabled by default if stderr is a tty.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 12:29                                     ` Eduardo Habkost
@ 2018-06-26 13:05                                       ` Paolo Bonzini
  2018-06-26 16:06                                         ` Eduardo Habkost
  0 siblings, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-26 13:05 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 26/06/2018 14:29, Eduardo Habkost wrote:
> On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
>> On 25/06/2018 21:51, Eduardo Habkost wrote:
>>> In either case, I'm not arguing (yet) for changing the default
>>> upstream.  I'm just arguing for upstream QEMU to not make any
>>> promises about the default.
>>
>> It would be a guest ABI breakage for TCG guests, so it would only apply
>> to new machine types.  I don't think it's worth the complication.
> 
> That's exactly the point: I want to stop promising a stable guest
> ABI when the accelerator is omitted, because I see no benefit in
> wasting energy on this.

On the other hand I see no benefit in changing a default that people are
obviously not using (since most people use KVM, not TCG).  Distros will
keep shipping, and people will keep using qemu-kvm even if we change the
default.

> (I don't think we ever kept the guest ABI correctly with TCG, by
> the way.)

It would not be any different from KVM.  Less tested and likely to be
more buggy, yes, but not particularly harder.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 13:05                                       ` Paolo Bonzini
@ 2018-06-26 16:06                                         ` Eduardo Habkost
  2018-06-26 16:10                                           ` Daniel P. Berrangé
  2018-06-27  8:39                                           ` Paolo Bonzini
  0 siblings, 2 replies; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-26 16:06 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On Tue, Jun 26, 2018 at 03:05:33PM +0200, Paolo Bonzini wrote:
> On 26/06/2018 14:29, Eduardo Habkost wrote:
> > On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
> >> On 25/06/2018 21:51, Eduardo Habkost wrote:
> >>> In either case, I'm not arguing (yet) for changing the default
> >>> upstream.  I'm just arguing for upstream QEMU to not make any
> >>> promises about the default.
> >>
> >> It would be a guest ABI breakage for TCG guests, so it would only apply
> >> to new machine types.  I don't think it's worth the complication.
> > 
> > That's exactly the point: I want to stop promising a stable guest
> > ABI when the accelerator is omitted, because I see no benefit in
> > wasting energy on this.
> 
> On the other hand I see no benefit in changing a default that people are
> obviously not using (since most people use KVM, not TCG).  Distros will
> keep shipping, and people will keep using qemu-kvm even if we change the
> default.

Not changing the default is different from promising we will keep
ABI compatibility if the accelerator is omitted.  I just want to
get rid of the latter.


> > (I don't think we ever kept the guest ABI correctly with TCG, by
> > the way.)
> 
> It would not be any different from KVM.  Less tested and likely to be
> more buggy, yes, but not particularly harder.

We can keep trying to not break it when "-accel tcg" is
explicitly provided.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 16:06                                         ` Eduardo Habkost
@ 2018-06-26 16:10                                           ` Daniel P. Berrangé
  2018-06-27  8:39                                           ` Paolo Bonzini
  1 sibling, 0 replies; 46+ messages in thread
From: Daniel P. Berrangé @ 2018-06-26 16:10 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Paolo Bonzini, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, Markus Armbruster, qemu-devel,
	Stefan Hajnoczi

On Tue, Jun 26, 2018 at 01:06:23PM -0300, Eduardo Habkost wrote:
> On Tue, Jun 26, 2018 at 03:05:33PM +0200, Paolo Bonzini wrote:
> > On 26/06/2018 14:29, Eduardo Habkost wrote:
> > > On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
> > >> On 25/06/2018 21:51, Eduardo Habkost wrote:
> > >>> In either case, I'm not arguing (yet) for changing the default
> > >>> upstream.  I'm just arguing for upstream QEMU to not make any
> > >>> promises about the default.
> > >>
> > >> It would be a guest ABI breakage for TCG guests, so it would only apply
> > >> to new machine types.  I don't think it's worth the complication.
> > > 
> > > That's exactly the point: I want to stop promising a stable guest
> > > ABI when the accelerator is omitted, because I see no benefit in
> > > wasting energy on this.
> > 
> > On the other hand I see no benefit in changing a default that people are
> > obviously not using (since most people use KVM, not TCG).  Distros will
> > keep shipping, and people will keep using qemu-kvm even if we change the
> > default.
> 
> Not changing the default is different from promising we will keep
> ABI compatibility if the accelerator is omitted.  I just want to
> get rid of the latter.

I guess the key question is what is the risk of causing problems if we
switch from tcg to kvm:tcg when accelerator is omitted ?

Based on what I've seen the likely troublespot would be people who are
using QEMU inside a guest with nested-virt enabled. Some nested-virt
impls are buggy and will cause L2 guest hangs, or worse L1 host crashes.
Then again we're not causing that brokenness - just revealing what
already exists.


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 12:56                                     ` Eduardo Habkost
@ 2018-06-26 16:43                                       ` Markus Armbruster
  2018-06-27  8:49                                         ` Paolo Bonzini
  0 siblings, 1 reply; 46+ messages in thread
From: Markus Armbruster @ 2018-06-26 16:43 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Cornelia Huck, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, qemu-devel, Stefan Hajnoczi, Paolo Bonzini

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Jun 26, 2018 at 09:50:04AM +0200, Cornelia Huck wrote:
>> On Tue, 26 Jun 2018 06:40:56 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>> 
>> > On 25.06.2018 20:26, Paolo Bonzini wrote:
>> > > On 25/06/2018 19:30, Eduardo Habkost wrote:  
>> > >>>> Attentive distros could even replace the wrapper script by a link.  
>> > >>> If they are okay with replacing the "KVM only" semantics with "KVM or
>> > >>> TCG", which I think is generally worse.  
>> > >>
>> > >> If we can't get agreement on what's the right default for each
>> > >> QEMU binary, I think that's yet another reason to document that
>> > >> upstream QEMU won't guarantee ABI compatibility if -accel is
>> > >> omitted.  
>> > > 
>> > > Before that we should ask what the benefit is in changing the default
>> > > for qemu-system-*.  Nobody is using it in practice to start QEMU with
>> > > KVM enabled...  
>> > 
>> > That's certainly not true. I've seen a couple of times already that
>> > people ask on IRC why their guests are running so slow, and if you ask
>> > them about their command line, it's obvious that they simply were not
>> > aware of "-accel" / "-enable-kvm" yet.
>> > 
>> > <semi-sarcastic>
>> > Maybe we simply should add a "--verbose" command line option that people
>> > can use to diagnose their problems:
>> > 
>> > $ qemu-system-x86_64 --verbose
>> > QEMU emulator version 2.12.50
>> > Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
>> > Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
>> > ....
>> > </semi-sarcastic>
>> 
>> Not sure how serious you meant that, but I actually quite like the
>> idea :)
>
> Also, this mode could be enabled by default if stderr is a tty.

It could be enabled by default, period.

Our inability to change defaults that have become sub-optimal for most
users is depressing.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 16:06                                         ` Eduardo Habkost
  2018-06-26 16:10                                           ` Daniel P. Berrangé
@ 2018-06-27  8:39                                           ` Paolo Bonzini
  2018-06-27 14:05                                             ` Eduardo Habkost
  1 sibling, 1 reply; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-27  8:39 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On 26/06/2018 18:06, Eduardo Habkost wrote:
>> On the other hand I see no benefit in changing a default that people are
>> obviously not using (since most people use KVM, not TCG).  Distros will
>> keep shipping, and people will keep using qemu-kvm even if we change the
>> default.
>
> Not changing the default is different from promising we will keep
> ABI compatibility if the accelerator is omitted.  I just want to
> get rid of the latter.

I understand; what I don't understand is why do the latter unless you
plan to change the default.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-26 16:43                                       ` Markus Armbruster
@ 2018-06-27  8:49                                         ` Paolo Bonzini
  0 siblings, 0 replies; 46+ messages in thread
From: Paolo Bonzini @ 2018-06-27  8:49 UTC (permalink / raw)
  To: Markus Armbruster, Eduardo Habkost
  Cc: Cornelia Huck, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, qemu-devel, Stefan Hajnoczi

On 26/06/2018 18:43, Markus Armbruster wrote:
>>> Not sure how serious you meant that, but I actually quite like the
>>> idea :)
>> Also, this mode could be enabled by default if stderr is a tty.
> It could be enabled by default, period.

I agree, summarizing the configuration to stderr on startup is a good
idea and it shouldn't be a problem for backwards compatibility.  Doing
it only if it is a tty could be a good idea though, at least initially.

Paolo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore
  2018-06-27  8:39                                           ` Paolo Bonzini
@ 2018-06-27 14:05                                             ` Eduardo Habkost
  0 siblings, 0 replies; 46+ messages in thread
From: Eduardo Habkost @ 2018-06-27 14:05 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Markus Armbruster, Thomas Huth, zhang.zhanghailiang, Ben Warren,
	qemu-trivial, Cornelia Huck, qemu-devel, Stefan Hajnoczi

On Wed, Jun 27, 2018 at 10:39:29AM +0200, Paolo Bonzini wrote:
> On 26/06/2018 18:06, Eduardo Habkost wrote:
> >> On the other hand I see no benefit in changing a default that people are
> >> obviously not using (since most people use KVM, not TCG).  Distros will
> >> keep shipping, and people will keep using qemu-kvm even if we change the
> >> default.
> >
> > Not changing the default is different from promising we will keep
> > ABI compatibility if the accelerator is omitted.  I just want to
> > get rid of the latter.
> 
> I understand; what I don't understand is why do the latter unless you
> plan to change the default.

Why should we make a compatibility promise if we don't have to?
Making and keeping promises have a cost, and I'd like us to spend
our energy elsewhere.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2018-06-27 14:06 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-13  5:05 [Qemu-devel] [PATCH 0/4] Clean up accelerator options Thomas Huth
2018-06-13  5:05 ` [Qemu-devel] [PATCH 1/4] Replace '-machine accel=xyz' with '-accel xyz' Thomas Huth
2018-06-13 12:20   ` Eric Blake
2018-06-13 12:48   ` Paolo Bonzini
2018-06-13 12:53     ` Thomas Huth
2018-06-13 13:54       ` Paolo Bonzini
2018-06-13 14:12         ` Thomas Huth
2018-06-13 16:10       ` Eduardo Habkost
2018-06-13 13:38   ` Stefan Hajnoczi
2018-06-13  5:05 ` [Qemu-devel] [PATCH 2/4] Replace '-enable-kvm' with '-accel kvm' in docs and help texts Thomas Huth
2018-06-13 12:51   ` Paolo Bonzini
2018-06-13 13:38   ` Stefan Hajnoczi
2018-06-13  5:05 ` [Qemu-devel] [PATCH 3/4] qemu-options: Improve the documentation of '-accel' and '-machine accel=...' Thomas Huth
2018-06-13 13:38   ` Stefan Hajnoczi
2018-06-13  5:05 ` [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore Thomas Huth
2018-06-13 12:51   ` Paolo Bonzini
2018-06-13 13:38   ` Stefan Hajnoczi
2018-06-13 13:44     ` Daniel P. Berrangé
2018-06-13 15:11       ` Thomas Huth
2018-06-13 15:19         ` Daniel P. Berrangé
2018-06-13 15:44           ` Thomas Huth
2018-06-13 16:02           ` Markus Armbruster
2018-06-19 15:15             ` Cornelia Huck
2018-06-19 16:16               ` Paolo Bonzini
2018-06-22 18:11                 ` Eduardo Habkost
2018-06-22 19:19                   ` Thomas Huth
2018-06-22 19:35                     ` Eduardo Habkost
2018-06-22 21:22                       ` Paolo Bonzini
2018-06-25  6:50                         ` Markus Armbruster
2018-06-25 10:28                           ` Paolo Bonzini
2018-06-25 17:30                             ` Eduardo Habkost
2018-06-25 18:26                               ` Paolo Bonzini
2018-06-25 19:51                                 ` Eduardo Habkost
2018-06-26  5:57                                   ` Paolo Bonzini
2018-06-26 12:29                                     ` Eduardo Habkost
2018-06-26 13:05                                       ` Paolo Bonzini
2018-06-26 16:06                                         ` Eduardo Habkost
2018-06-26 16:10                                           ` Daniel P. Berrangé
2018-06-27  8:39                                           ` Paolo Bonzini
2018-06-27 14:05                                             ` Eduardo Habkost
2018-06-26  4:40                                 ` Thomas Huth
2018-06-26  7:50                                   ` Cornelia Huck
2018-06-26 12:56                                     ` Eduardo Habkost
2018-06-26 16:43                                       ` Markus Armbruster
2018-06-27  8:49                                         ` Paolo Bonzini
2018-06-13 13:39 ` [Qemu-devel] [PATCH 0/4] Clean up accelerator options Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.