[PATCH 0/3] x86: Switch over to q35 as the default machine type

[PATCH 0/3] x86: Switch over to q35 as the default machine type

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

We've been using q35 heavily for a while now and it generally works
quite nicely; downstream in RH we prefer it as our default, and I wanted
to see what people think of making it the default.

The only pain really is that it requires some more setup for hotplug;
so for now I've forced hotplug tests to stay on i440fx aka pc.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Dr. David Alan Gilbert (3):
  tests/x86: Use 'pc' machine type for old hardware tests
  tests/x86: Use 'pc' machine type for hotplug tests
  x86: Switch to q35 as the default machine type

 hw/i386/pc_piix.c              | 12 +-----------
 hw/i386/pc_q35.c               |  2 ++
 tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
 tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
 tests/qtest/fdc-test.c         |  2 +-
 tests/qtest/hd-geo-test.c      | 16 +++++++++++-----
 tests/qtest/i440fx-test.c      |  2 +-
 tests/qtest/ide-test.c         |  3 ++-
 tests/qtest/ivshmem-test.c     |  7 ++++++-
 9 files changed, 72 insertions(+), 27 deletions(-)

-- 
2.35.1

[PATCH 1/3] tests/x86: Use 'pc' machine type for old hardware tests

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

For tests that rely on old hardware, e.g. floppies or IDE drives,
explicitly select the 'pc' machine type.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/fdc-test.c    |  2 +-
 tests/qtest/hd-geo-test.c | 12 +++++++++---
 tests/qtest/i440fx-test.c |  2 +-
 tests/qtest/ide-test.c    |  3 ++-
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 8f6eee84a4..b0d40012e6 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -598,7 +598,7 @@ int main(int argc, char **argv)
     /* Run the tests */
     g_test_init(&argc, &argv, NULL);
 
-    qtest_start("-device floppy,id=floppy0");
+    qtest_start("-machine pc -device floppy,id=floppy0");
     qtest_irq_intercept_in(global_qtest, "ioapic");
     qtest_add_func("/fdc/cmos", test_cmos);
     qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 771eaa741b..3554b5d500 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -178,9 +178,15 @@ static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
 
 static int setup_common(char *argv[], int argv_sz)
 {
+    int new_argc;
     memset(cur_ide, 0, sizeof(cur_ide));
-    return append_arg(0, argv, argv_sz,
-                      g_strdup("-nodefaults"));
+    new_argc = append_arg(0, argv, argv_sz,
+                          g_strdup("-nodefaults"));
+    new_argc = append_arg(new_argc, argv, argv_sz,
+                          g_strdup("-machine"));
+    new_argc = append_arg(new_argc, argv, argv_sz,
+                          g_strdup("pc"));
+    return new_argc;
 }
 
 static void setup_mbr(int img_idx, MBRcontents mbr)
@@ -697,7 +703,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
index 1f57d9684b..6d7d4d8d8f 100644
--- a/tests/qtest/i440fx-test.c
+++ b/tests/qtest/i440fx-test.c
@@ -35,7 +35,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
 {
     char *cmdline;
 
-    cmdline = g_strdup_printf("-smp %d", s->num_cpus);
+    cmdline = g_strdup_printf("-machine pc -smp %d", s->num_cpus);
     qtest_start(cmdline);
     g_free(cmdline);
     return qpci_new_pc(global_qtest, NULL);
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 3f8081e77d..84935578fb 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -128,10 +128,11 @@ static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
 static QTestState *ide_test_start(const char *cmdline_fmt, ...)
 {
     QTestState *qts;
+    g_autofree char *full_fmt = g_strdup_printf("-machine pc %s", cmdline_fmt);
     va_list ap;
 
     va_start(ap, cmdline_fmt);
-    qts = qtest_vinitf(cmdline_fmt, ap);
+    qts = qtest_vinitf(full_fmt, ap);
     va_end(ap);
 
     pc_alloc_init(&guest_malloc, qts, 0);
-- 
2.35.1

Re: [PATCH 1/3] tests/x86: Use 'pc' machine type for old hardware tests

[Thomas Huth]

On 15/02/2022 17.25, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> For tests that rely on old hardware, e.g. floppies or IDE drives,
> explicitly select the 'pc' machine type.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   tests/qtest/fdc-test.c    |  2 +-
>   tests/qtest/hd-geo-test.c | 12 +++++++++---
>   tests/qtest/i440fx-test.c |  2 +-
>   tests/qtest/ide-test.c    |  3 ++-
>   4 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
> index 8f6eee84a4..b0d40012e6 100644
> --- a/tests/qtest/fdc-test.c
> +++ b/tests/qtest/fdc-test.c
> @@ -598,7 +598,7 @@ int main(int argc, char **argv)
>       /* Run the tests */
>       g_test_init(&argc, &argv, NULL);
>   
> -    qtest_start("-device floppy,id=floppy0");
> +    qtest_start("-machine pc -device floppy,id=floppy0");
>       qtest_irq_intercept_in(global_qtest, "ioapic");
>       qtest_add_func("/fdc/cmos", test_cmos);
>       qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 771eaa741b..3554b5d500 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -178,9 +178,15 @@ static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
>   
>   static int setup_common(char *argv[], int argv_sz)
>   {
> +    int new_argc;
>       memset(cur_ide, 0, sizeof(cur_ide));
> -    return append_arg(0, argv, argv_sz,
> -                      g_strdup("-nodefaults"));
> +    new_argc = append_arg(0, argv, argv_sz,
> +                          g_strdup("-nodefaults"));
> +    new_argc = append_arg(new_argc, argv, argv_sz,
> +                          g_strdup("-machine"));
> +    new_argc = append_arg(new_argc, argv, argv_sz,
> +                          g_strdup("pc"));
> +    return new_argc;
>   }
>   
>   static void setup_mbr(int img_idx, MBRcontents mbr)
> @@ -697,7 +703,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
>   
>       joined_args = g_strjoinv(" ", args->argv);
>   
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>       fw_cfg = pc_fw_cfg_init(qts);
>   
>       read_bootdevices(fw_cfg, expected);
> diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
> index 1f57d9684b..6d7d4d8d8f 100644
> --- a/tests/qtest/i440fx-test.c
> +++ b/tests/qtest/i440fx-test.c
> @@ -35,7 +35,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
>   {
>       char *cmdline;
>   
> -    cmdline = g_strdup_printf("-smp %d", s->num_cpus);
> +    cmdline = g_strdup_printf("-machine pc -smp %d", s->num_cpus);
>       qtest_start(cmdline);
>       g_free(cmdline);
>       return qpci_new_pc(global_qtest, NULL);
> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
> index 3f8081e77d..84935578fb 100644
> --- a/tests/qtest/ide-test.c
> +++ b/tests/qtest/ide-test.c
> @@ -128,10 +128,11 @@ static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
>   static QTestState *ide_test_start(const char *cmdline_fmt, ...)
>   {
>       QTestState *qts;
> +    g_autofree char *full_fmt = g_strdup_printf("-machine pc %s", cmdline_fmt);
>       va_list ap;
>   
>       va_start(ap, cmdline_fmt);
> -    qts = qtest_vinitf(cmdline_fmt, ap);
> +    qts = qtest_vinitf(full_fmt, ap);
>       va_end(ap);
>   
>       pc_alloc_init(&guest_malloc, qts, 0);

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

Re: [PATCH 1/3] tests/x86: Use 'pc' machine type for old hardware tests

[Michael S. Tsirkin]

On Tue, Feb 15, 2022 at 04:25:31PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> For tests that rely on old hardware, e.g. floppies or IDE drives,
> explicitly select the 'pc' machine type.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

This makes sense regardless of the rest of the series.

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  tests/qtest/fdc-test.c    |  2 +-
>  tests/qtest/hd-geo-test.c | 12 +++++++++---
>  tests/qtest/i440fx-test.c |  2 +-
>  tests/qtest/ide-test.c    |  3 ++-
>  4 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
> index 8f6eee84a4..b0d40012e6 100644
> --- a/tests/qtest/fdc-test.c
> +++ b/tests/qtest/fdc-test.c
> @@ -598,7 +598,7 @@ int main(int argc, char **argv)
>      /* Run the tests */
>      g_test_init(&argc, &argv, NULL);
>  
> -    qtest_start("-device floppy,id=floppy0");
> +    qtest_start("-machine pc -device floppy,id=floppy0");
>      qtest_irq_intercept_in(global_qtest, "ioapic");
>      qtest_add_func("/fdc/cmos", test_cmos);
>      qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 771eaa741b..3554b5d500 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -178,9 +178,15 @@ static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
>  
>  static int setup_common(char *argv[], int argv_sz)
>  {
> +    int new_argc;
>      memset(cur_ide, 0, sizeof(cur_ide));
> -    return append_arg(0, argv, argv_sz,
> -                      g_strdup("-nodefaults"));
> +    new_argc = append_arg(0, argv, argv_sz,
> +                          g_strdup("-nodefaults"));
> +    new_argc = append_arg(new_argc, argv, argv_sz,
> +                          g_strdup("-machine"));
> +    new_argc = append_arg(new_argc, argv, argv_sz,
> +                          g_strdup("pc"));
> +    return new_argc;
>  }
>  
>  static void setup_mbr(int img_idx, MBRcontents mbr)
> @@ -697,7 +703,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
>  
>      joined_args = g_strjoinv(" ", args->argv);
>  
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>      fw_cfg = pc_fw_cfg_init(qts);
>  
>      read_bootdevices(fw_cfg, expected);
> diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
> index 1f57d9684b..6d7d4d8d8f 100644
> --- a/tests/qtest/i440fx-test.c
> +++ b/tests/qtest/i440fx-test.c
> @@ -35,7 +35,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
>  {
>      char *cmdline;
>  
> -    cmdline = g_strdup_printf("-smp %d", s->num_cpus);
> +    cmdline = g_strdup_printf("-machine pc -smp %d", s->num_cpus);
>      qtest_start(cmdline);
>      g_free(cmdline);
>      return qpci_new_pc(global_qtest, NULL);
> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
> index 3f8081e77d..84935578fb 100644
> --- a/tests/qtest/ide-test.c
> +++ b/tests/qtest/ide-test.c
> @@ -128,10 +128,11 @@ static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
>  static QTestState *ide_test_start(const char *cmdline_fmt, ...)
>  {
>      QTestState *qts;
> +    g_autofree char *full_fmt = g_strdup_printf("-machine pc %s", cmdline_fmt);
>      va_list ap;
>  
>      va_start(ap, cmdline_fmt);
> -    qts = qtest_vinitf(cmdline_fmt, ap);
> +    qts = qtest_vinitf(full_fmt, ap);
>      va_end(ap);
>  
>      pc_alloc_init(&guest_malloc, qts, 0);
> -- 
> 2.35.1

[PATCH 2/3] tests/x86: Use 'pc' machine type for hotplug tests

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Hotplug tests need a bridge setting up on q35, for now
keep them on 'pc'.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
 tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
 tests/qtest/hd-geo-test.c      |  4 ++--
 tests/qtest/ivshmem-test.c     |  7 ++++++-
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index ad79bd4c14..404a92e132 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -63,7 +63,15 @@ static void wait_device_deleted_event(QTestState *qtest, const char *id)
 
 static void test_pci_unplug_request(void)
 {
-    QTestState *qtest = qtest_initf("-device virtio-mouse-pci,id=dev0");
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
+
+    QTestState *qtest = qtest_initf("%s -device virtio-mouse-pci,id=dev0",
+                                    machine_addition);
 
     /*
      * Request device removal. As the guest is not running, the request won't
@@ -79,8 +87,16 @@ static void test_pci_unplug_request(void)
 
 static void test_pci_unplug_json_request(void)
 {
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
+
     QTestState *qtest = qtest_initf(
-        "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'");
+        "%s -device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'",
+        machine_addition);
 
     /*
      * Request device removal. As the guest is not running, the request won't
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 8d08ee9995..0cc18dfa4a 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -235,14 +235,21 @@ static void test_drive_del_device_del(void)
 static void test_cli_device_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
     /*
      * -drive/-device and device_del.  Start with a drive used by a
      * device that unplugs after reset.
      */
-    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
                       "file.read-zeroes=on,format=raw"
                       " -device virtio-blk-%s,drive=drive0,id=dev0",
+                      machine_addition,
                       qvirtio_get_dev_type());
 
     device_del(qts, true);
@@ -266,13 +273,19 @@ static void test_empty_device_del(void)
 static void test_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
     /*
      * -drive/device_add and device_del.  Start with a drive used by a
      * device that unplugs after reset.
      */
-    qts = qtest_init("-drive if=none,id=drive0,file=null-co://,"
-                     "file.read-zeroes=on,format=raw");
+    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
+                     "file.read-zeroes=on,format=raw", machine_addition);
 
     device_add(qts);
     device_del(qts, true);
@@ -284,8 +297,14 @@ static void test_device_add_and_del(void)
 static void test_drive_add_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
-    qts = qtest_init("");
+    qts = qtest_init(machine_addition);
 
     /*
      * drive_add/device_add and device_del.  The drive is used by a
@@ -302,8 +321,14 @@ static void test_drive_add_device_add_and_del(void)
 static void test_blockdev_add_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
-    qts = qtest_init("");
+    qts = qtest_init(machine_addition);
 
     /*
      * blockdev_add/device_add and device_del.  The it drive is used by a
diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 3554b5d500..64023c0574 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -839,7 +839,7 @@ static void test_override_scsi_hot_unplug(void)
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
@@ -899,7 +899,7 @@ static void test_override_virtio_hot_unplug(void)
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index fe94dd3b96..4e8af42a9d 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -385,7 +385,12 @@ static void test_ivshmem_hotplug(void)
     QTestState *qts;
     const char *arch = qtest_get_arch();
 
-    qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
+                         " -machine pc");
+    } else {
+        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
+    }
 
     qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
                          "{'addr': %s, 'memdev': 'mb1'}",
-- 
2.35.1

Re: [PATCH 2/3] tests/x86: Use 'pc' machine type for hotplug tests

[Thomas Huth]

On 15/02/2022 17.25, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Hotplug tests need a bridge setting up on q35, for now
> keep them on 'pc'.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
>   tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
>   tests/qtest/hd-geo-test.c      |  4 ++--
>   tests/qtest/ivshmem-test.c     |  7 ++++++-
>   4 files changed, 56 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
> index ad79bd4c14..404a92e132 100644
> --- a/tests/qtest/device-plug-test.c
> +++ b/tests/qtest/device-plug-test.c
> @@ -63,7 +63,15 @@ static void wait_device_deleted_event(QTestState *qtest, const char *id)
>   
>   static void test_pci_unplug_request(void)
>   {
> -    QTestState *qtest = qtest_initf("-device virtio-mouse-pci,id=dev0");
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
> +
> +    QTestState *qtest = qtest_initf("%s -device virtio-mouse-pci,id=dev0",
> +                                    machine_addition);
>   
>       /*
>        * Request device removal. As the guest is not running, the request won't
> @@ -79,8 +87,16 @@ static void test_pci_unplug_request(void)
>   
>   static void test_pci_unplug_json_request(void)
>   {
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
> +
>       QTestState *qtest = qtest_initf(
> -        "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'");
> +        "%s -device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'",
> +        machine_addition);
>   
>       /*
>        * Request device removal. As the guest is not running, the request won't
> diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
> index 8d08ee9995..0cc18dfa4a 100644
> --- a/tests/qtest/drive_del-test.c
> +++ b/tests/qtest/drive_del-test.c
> @@ -235,14 +235,21 @@ static void test_drive_del_device_del(void)
>   static void test_cli_device_del(void)
>   {
>       QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>   
>       /*
>        * -drive/-device and device_del.  Start with a drive used by a
>        * device that unplugs after reset.
>        */
> -    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
> +    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
>                         "file.read-zeroes=on,format=raw"
>                         " -device virtio-blk-%s,drive=drive0,id=dev0",
> +                      machine_addition,
>                         qvirtio_get_dev_type());
>   
>       device_del(qts, true);
> @@ -266,13 +273,19 @@ static void test_empty_device_del(void)
>   static void test_device_add_and_del(void)
>   {
>       QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>   
>       /*
>        * -drive/device_add and device_del.  Start with a drive used by a
>        * device that unplugs after reset.
>        */
> -    qts = qtest_init("-drive if=none,id=drive0,file=null-co://,"
> -                     "file.read-zeroes=on,format=raw");
> +    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
> +                     "file.read-zeroes=on,format=raw", machine_addition);
>   
>       device_add(qts);
>       device_del(qts, true);
> @@ -284,8 +297,14 @@ static void test_device_add_and_del(void)
>   static void test_drive_add_device_add_and_del(void)
>   {
>       QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>   
> -    qts = qtest_init("");
> +    qts = qtest_init(machine_addition);
>   
>       /*
>        * drive_add/device_add and device_del.  The drive is used by a
> @@ -302,8 +321,14 @@ static void test_drive_add_device_add_and_del(void)
>   static void test_blockdev_add_device_add_and_del(void)
>   {
>       QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>   
> -    qts = qtest_init("");
> +    qts = qtest_init(machine_addition);
>   
>       /*
>        * blockdev_add/device_add and device_del.  The it drive is used by a
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 3554b5d500..64023c0574 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -839,7 +839,7 @@ static void test_override_scsi_hot_unplug(void)
>   
>       joined_args = g_strjoinv(" ", args->argv);
>   
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>       fw_cfg = pc_fw_cfg_init(qts);
>   
>       read_bootdevices(fw_cfg, expected);
> @@ -899,7 +899,7 @@ static void test_override_virtio_hot_unplug(void)
>   
>       joined_args = g_strjoinv(" ", args->argv);
>   
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>       fw_cfg = pc_fw_cfg_init(qts);
>   
>       read_bootdevices(fw_cfg, expected);
> diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
> index fe94dd3b96..4e8af42a9d 100644
> --- a/tests/qtest/ivshmem-test.c
> +++ b/tests/qtest/ivshmem-test.c
> @@ -385,7 +385,12 @@ static void test_ivshmem_hotplug(void)
>       QTestState *qts;
>       const char *arch = qtest_get_arch();
>   
> -    qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
> +                         " -machine pc");
> +    } else {
> +        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
> +    }
>   
>       qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
>                            "{'addr': %s, 'memdev': 'mb1'}",

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

Re: [PATCH 2/3] tests/x86: Use 'pc' machine type for hotplug tests

[Michael S. Tsirkin]

On Tue, Feb 15, 2022 at 04:25:32PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Hotplug tests need a bridge setting up on q35, for now
> keep them on 'pc'.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I'd really much rather we actually tested q35.

> ---
>  tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
>  tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
>  tests/qtest/hd-geo-test.c      |  4 ++--
>  tests/qtest/ivshmem-test.c     |  7 ++++++-
>  4 files changed, 56 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
> index ad79bd4c14..404a92e132 100644
> --- a/tests/qtest/device-plug-test.c
> +++ b/tests/qtest/device-plug-test.c
> @@ -63,7 +63,15 @@ static void wait_device_deleted_event(QTestState *qtest, const char *id)
>  
>  static void test_pci_unplug_request(void)
>  {
> -    QTestState *qtest = qtest_initf("-device virtio-mouse-pci,id=dev0");
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
> +
> +    QTestState *qtest = qtest_initf("%s -device virtio-mouse-pci,id=dev0",
> +                                    machine_addition);
>  
>      /*
>       * Request device removal. As the guest is not running, the request won't
> @@ -79,8 +87,16 @@ static void test_pci_unplug_request(void)
>  
>  static void test_pci_unplug_json_request(void)
>  {
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
> +
>      QTestState *qtest = qtest_initf(
> -        "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'");
> +        "%s -device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'",
> +        machine_addition);
>  
>      /*
>       * Request device removal. As the guest is not running, the request won't
> diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
> index 8d08ee9995..0cc18dfa4a 100644
> --- a/tests/qtest/drive_del-test.c
> +++ b/tests/qtest/drive_del-test.c
> @@ -235,14 +235,21 @@ static void test_drive_del_device_del(void)
>  static void test_cli_device_del(void)
>  {
>      QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>  
>      /*
>       * -drive/-device and device_del.  Start with a drive used by a
>       * device that unplugs after reset.
>       */
> -    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
> +    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
>                        "file.read-zeroes=on,format=raw"
>                        " -device virtio-blk-%s,drive=drive0,id=dev0",
> +                      machine_addition,
>                        qvirtio_get_dev_type());
>  
>      device_del(qts, true);
> @@ -266,13 +273,19 @@ static void test_empty_device_del(void)
>  static void test_device_add_and_del(void)
>  {
>      QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>  
>      /*
>       * -drive/device_add and device_del.  Start with a drive used by a
>       * device that unplugs after reset.
>       */
> -    qts = qtest_init("-drive if=none,id=drive0,file=null-co://,"
> -                     "file.read-zeroes=on,format=raw");
> +    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
> +                     "file.read-zeroes=on,format=raw", machine_addition);
>  
>      device_add(qts);
>      device_del(qts, true);
> @@ -284,8 +297,14 @@ static void test_device_add_and_del(void)
>  static void test_drive_add_device_add_and_del(void)
>  {
>      QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>  
> -    qts = qtest_init("");
> +    qts = qtest_init(machine_addition);
>  
>      /*
>       * drive_add/device_add and device_del.  The drive is used by a
> @@ -302,8 +321,14 @@ static void test_drive_add_device_add_and_del(void)
>  static void test_blockdev_add_device_add_and_del(void)
>  {
>      QTestState *qts;
> +    const char *arch = qtest_get_arch();
> +    const char *machine_addition = "";
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        machine_addition = "-machine pc";
> +    }
>  
> -    qts = qtest_init("");
> +    qts = qtest_init(machine_addition);
>  
>      /*
>       * blockdev_add/device_add and device_del.  The it drive is used by a
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 3554b5d500..64023c0574 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -839,7 +839,7 @@ static void test_override_scsi_hot_unplug(void)
>  
>      joined_args = g_strjoinv(" ", args->argv);
>  
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>      fw_cfg = pc_fw_cfg_init(qts);
>  
>      read_bootdevices(fw_cfg, expected);
> @@ -899,7 +899,7 @@ static void test_override_virtio_hot_unplug(void)
>  
>      joined_args = g_strjoinv(" ", args->argv);
>  
> -    qts = qtest_init(joined_args);
> +    qts = qtest_initf("-machine pc %s", joined_args);
>      fw_cfg = pc_fw_cfg_init(qts);
>  
>      read_bootdevices(fw_cfg, expected);
> diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
> index fe94dd3b96..4e8af42a9d 100644
> --- a/tests/qtest/ivshmem-test.c
> +++ b/tests/qtest/ivshmem-test.c
> @@ -385,7 +385,12 @@ static void test_ivshmem_hotplug(void)
>      QTestState *qts;
>      const char *arch = qtest_get_arch();
>  
> -    qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
> +                         " -machine pc");
> +    } else {
> +        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
> +    }
>  
>      qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
>                           "{'addr': %s, 'memdev': 'mb1'}",
> -- 
> 2.35.1

[PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The 'q35' machine type series has been around for a few years now, and
is getting heavily used downstream without many problems;  lets flip
to using it as the default.

While it is of course newer and shinier than it's old i440fx cousin,
the main reasons are:
  * PCIe support
  * No default floppy or IDE
  * More modern defaults for NIC
  * Better OVMF support

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/i386/pc_piix.c | 12 +-----------
 hw/i386/pc_q35.c  |  2 ++
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d9b344248d..50c7e6c2c0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -420,7 +420,7 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_machine_options(m);
     m->alias = "pc";
-    m->is_default = true;
+    m->is_default = false;
     pcmc->default_cpu_version = 1;
 }
 
@@ -431,7 +431,6 @@ static void pc_i440fx_6_2_machine_options(MachineClass *m)
 {
     pc_i440fx_7_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
     compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
 }
@@ -443,7 +442,6 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
 {
     pc_i440fx_6_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
     m->smp_props.prefer_sockets = true;
@@ -456,7 +454,6 @@ static void pc_i440fx_6_0_machine_options(MachineClass *m)
 {
     pc_i440fx_6_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_0, hw_compat_6_0_len);
     compat_props_add(m->compat_props, pc_compat_6_0, pc_compat_6_0_len);
 }
@@ -468,7 +465,6 @@ static void pc_i440fx_5_2_machine_options(MachineClass *m)
 {
     pc_i440fx_6_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
     compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
 }
@@ -482,7 +478,6 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m)
 
     pc_i440fx_5_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
@@ -496,7 +491,6 @@ static void pc_i440fx_5_0_machine_options(MachineClass *m)
 {
     pc_i440fx_5_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     m->numa_mem_supported = true;
     compat_props_add(m->compat_props, hw_compat_5_0, hw_compat_5_0_len);
     compat_props_add(m->compat_props, pc_compat_5_0, pc_compat_5_0_len);
@@ -510,7 +504,6 @@ static void pc_i440fx_4_2_machine_options(MachineClass *m)
 {
     pc_i440fx_5_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
     compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
 }
@@ -522,7 +515,6 @@ static void pc_i440fx_4_1_machine_options(MachineClass *m)
 {
     pc_i440fx_4_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_4_1, hw_compat_4_1_len);
     compat_props_add(m->compat_props, pc_compat_4_1, pc_compat_4_1_len);
 }
@@ -535,7 +527,6 @@ static void pc_i440fx_4_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_4_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     pcmc->default_cpu_version = CPU_VERSION_LEGACY;
     compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
     compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
@@ -549,7 +540,6 @@ static void pc_i440fx_3_1_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 
     pc_i440fx_4_0_machine_options(m);
-    m->is_default = false;
     pcmc->do_not_add_smb_acpi = true;
     m->smbus_no_migration_support = true;
     m->alias = NULL;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1780f79bc1..aec4e09c9f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -365,6 +365,7 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_machine_options(m);
     m->alias = "q35";
+    m->is_default = true;
     pcmc->default_cpu_version = 1;
 }
 
@@ -375,6 +376,7 @@ static void pc_q35_6_2_machine_options(MachineClass *m)
 {
     pc_q35_7_0_machine_options(m);
     m->alias = NULL;
+    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
     compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
 }
-- 
2.35.1

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Daniel P. Berrangé]

On Tue, Feb 15, 2022 at 04:25:33PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The 'q35' machine type series has been around for a few years now, and
> is getting heavily used downstream without many problems;  lets flip
> to using it as the default.

I don't think I'd claim 'without many problems'.  In RHEL while 'q35'
is recommended, it explicitly isn't the default because it would havbe
created compatibility problems for applications. Every non-trivial
application needs to make code changes to cope with 'q35' if they're
using 'i440fx' today.  Apps that Red Hat ships, either in RHEL or as
add-on products, have been updated, but it was certainly not painfree
and took considerable time for OpenStack at least. I worry about how
ready the broader QEMU consumers are to work with 'q35' as an opt-in,
let alone as a default.


PCI is the really big ticket item here. If keeping the same command
line none of the PCI devices added will be hotpluggable because
they'll all be put in the root complex as integrated end points,
whether PCI or PCIe devices.

To allow for hot-unplug, any cold plugged PCIe devices need to be
placed in unique pcie-root-port (one root port per device). The
PCI devices meanwhile have to be put into a pci-bridge, which is
in turn plugged into a pcie-to-pci-bridge.  QEMU doesn't do this
placement by default so nothing is hot-unpluggable.

To allow for hot-plug, it is needed to pre-create many pcie-root-port
devices - one for each device to be plugged.

Libvirt tried to make this a little easier by putting cold plugged
devices into a place that allows them to be hot-unplugged.

On the libvirt side, there's also the need to know about sata vs
ide. That ones fun because at the QEMU level we still refer to it
as 'ide' throughout, even though q35 is implementing sata. 

There was one other notable difference that impacted apps, but I
can't remember what it was offhand.


> While it is of course newer and shinier than it's old i440fx cousin,
> the main reasons are:

s/newer and shinier/slightly less ancient and obsolete/ ;-P

>   * PCIe support
>   * No default floppy or IDE
>   * More modern defaults for NIC
>   * Better OVMF support

These are fine reasons for recommending apps to prefer use of 'q35'
on an opt-in basis.

Given the semantic differences from 'i440fx', changing the default
machine type has effects that are equivalent to breaking command
line syntax compatibility, which is something we've always tried
to avoid.

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

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Tue, Feb 15, 2022 at 04:25:33PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > The 'q35' machine type series has been around for a few years now, and
> > is getting heavily used downstream without many problems;  lets flip
> > to using it as the default.
> 
> I don't think I'd claim 'without many problems'.  In RHEL while 'q35'
> is recommended, it explicitly isn't the default because it would havbe
> created compatibility problems for applications. Every non-trivial
> application needs to make code changes to cope with 'q35' if they're
> using 'i440fx' today.  Apps that Red Hat ships, either in RHEL or as
> add-on products, have been updated, but it was certainly not painfree
> and took considerable time for OpenStack at least. I worry about how
> ready the broader QEMU consumers are to work with 'q35' as an opt-in,
> let alone as a default.

How many of these consumers do all of:
  a) Don't specify the machine type
  b) and try and do hotplug etc

> PCI is the really big ticket item here. If keeping the same command
> line none of the PCI devices added will be hotpluggable because
> they'll all be put in the root complex as integrated end points,
> whether PCI or PCIe devices.
> 
> To allow for hot-unplug, any cold plugged PCIe devices need to be
> placed in unique pcie-root-port (one root port per device). The
> PCI devices meanwhile have to be put into a pci-bridge, which is
> in turn plugged into a pcie-to-pci-bridge.  QEMU doesn't do this
> placement by default so nothing is hot-unpluggable.
> 
> To allow for hot-plug, it is needed to pre-create many pcie-root-port
> devices - one for each device to be plugged.

I'm tempted to modify q35 to create a bunch of root ports by default
and then tweak the placement to pick them;  that would have no effect
on libvirt users but would make simple command line use easy again.

> Libvirt tried to make this a little easier by putting cold plugged
> devices into a place that allows them to be hot-unplugged.
> 
> On the libvirt side, there's also the need to know about sata vs
> ide. That ones fun because at the QEMU level we still refer to it
> as 'ide' throughout, even though q35 is implementing sata. 
> 
> There was one other notable difference that impacted apps, but I
> can't remember what it was offhand.

There be dragons, but I can't remember where :-)

> 
> > While it is of course newer and shinier than it's old i440fx cousin,
> > the main reasons are:
> 
> s/newer and shinier/slightly less ancient and obsolete/ ;-P
> 
> >   * PCIe support
> >   * No default floppy or IDE
> >   * More modern defaults for NIC
> >   * Better OVMF support
> 
> These are fine reasons for recommending apps to prefer use of 'q35'
> on an opt-in basis.
> 
> Given the semantic differences from 'i440fx', changing the default
> machine type has effects that are equivalent to breaking command
> line syntax compatibility, which is something we've always tried
> to avoid.

I'm not sure where to draw the line; it's always been legal for
new versions of machine types to have different slightly different
behaviour; for example it would be reasonable for a new version to use
up a PCIe bus slot/address that was previously unused and thus break
your command line that was trying to stuff another device in there.

To my mind, as long as -M pc  puts everything back the way it was, I
wasn't too worried about that breakage.

Dave

> 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 :|
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Gerd Hoffmann]

  Hi,
 
> Given the semantic differences from 'i440fx', changing the default
> machine type has effects that are equivalent to breaking command
> line syntax compatibility, which is something we've always tried
> to avoid.

And if we are fine breaking backward compatibility I'd rather *not* pick
a default, effectively making -M $something mandatory, similar to arm.

take care,
  Gerd

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert]

* Gerd Hoffmann (kraxel@redhat.com) wrote:
>   Hi,
>  
> > Given the semantic differences from 'i440fx', changing the default
> > machine type has effects that are equivalent to breaking command
> > line syntax compatibility, which is something we've always tried
> > to avoid.
> 
> And if we are fine breaking backward compatibility I'd rather *not* pick
> a default, effectively making -M $something mandatory, similar to arm.

Oh, that's probably easy to do;  what are other peoples thoughts on
that?

Dave

> take care,
>   Gerd
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Daniel P. Berrangé]

On Wed, Feb 16, 2022 at 11:01:24AM +0000, Dr. David Alan Gilbert wrote:
> * Gerd Hoffmann (kraxel@redhat.com) wrote:
> >   Hi,
> >  
> > > Given the semantic differences from 'i440fx', changing the default
> > > machine type has effects that are equivalent to breaking command
> > > line syntax compatibility, which is something we've always tried
> > > to avoid.
> > 
> > And if we are fine breaking backward compatibility I'd rather *not* pick
> > a default, effectively making -M $something mandatory, similar to arm.
> 
> Oh, that's probably easy to do;  what are other peoples thoughts on
> that?

On the libvirt side it won't matter & will have no effect. Libvirt ignores
QEMU defaults an explicitly sets 'pc' as the default, so that our users
are protected against QEMU changes in defaults that could break app usage.

We would of course suggest that all apps using libvirt explicitly pick
a machine type they want, but if they dont, we'll set it for them and
guarantee that default won't change as long as the machine type exists
in QEMU.

IOW, whether QEMU selects 'q35' or <nothing at all>  as the default
machine, libvirt will continue to set 'pc' as the default in absence
of a mgmt app explicit choice.

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

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Thomas Huth]

On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> * Gerd Hoffmann (kraxel@redhat.com) wrote:
>>    Hi,
>>   
>>> Given the semantic differences from 'i440fx', changing the default
>>> machine type has effects that are equivalent to breaking command
>>> line syntax compatibility, which is something we've always tried
>>> to avoid.
>>
>> And if we are fine breaking backward compatibility I'd rather *not* pick
>> a default, effectively making -M $something mandatory, similar to arm.
> 
> Oh, that's probably easy to do;  what are other peoples thoughts on
> that?

I agree with Gerd. Getting rid of a default machine on x86 is likely better 
than silently changing it to q35. But I'd maybe say that this should go 
through the deprecation process first?

  Thomas

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert]

* Thomas Huth (thuth@redhat.com) wrote:
> On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > >    Hi,
> > > > Given the semantic differences from 'i440fx', changing the default
> > > > machine type has effects that are equivalent to breaking command
> > > > line syntax compatibility, which is something we've always tried
> > > > to avoid.
> > > 
> > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > a default, effectively making -M $something mandatory, similar to arm.
> > 
> > Oh, that's probably easy to do;  what are other peoples thoughts on
> > that?
> 
> I agree with Gerd. Getting rid of a default machine on x86 is likely better
> than silently changing it to q35. But I'd maybe say that this should go
> through the deprecation process first?

So just adding something like the following under 'System emulator
machines':

x86 default machine type
------------------------

x86 currently defaults to the very old ```pc``` machine type
which is based on the very old ```i440f``` chipset.  This default
will be removed and the user will be required to specify a machine
type explicitly using -M; users are encouraged to switch to the
not quite as old ```q35``` machine types.


(This option is going to take a lot more work switching all the
test cases over; in my world here I'd only changed the tests that broke
on q35, now everything is going to need to specify a type).

Dave



>  Thomas
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Daniel P. Berrangé]

On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
> * Thomas Huth (thuth@redhat.com) wrote:
> > On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > >    Hi,
> > > > > Given the semantic differences from 'i440fx', changing the default
> > > > > machine type has effects that are equivalent to breaking command
> > > > > line syntax compatibility, which is something we've always tried
> > > > > to avoid.
> > > > 
> > > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > > a default, effectively making -M $something mandatory, similar to arm.
> > > 
> > > Oh, that's probably easy to do;  what are other peoples thoughts on
> > > that?
> > 
> > I agree with Gerd. Getting rid of a default machine on x86 is likely better
> > than silently changing it to q35. But I'd maybe say that this should go
> > through the deprecation process first?
> 
> So just adding something like the following under 'System emulator
> machines':
> 
> x86 default machine type
> ------------------------
> 
> x86 currently defaults to the very old ```pc``` machine type
> which is based on the very old ```i440f``` chipset.  This default
> will be removed and the user will be required to specify a machine
> type explicitly using -M; users are encouraged to switch to the
> not quite as old ```q35``` machine types.
> 
> 
> (This option is going to take a lot more work switching all the
> test cases over; in my world here I'd only changed the tests that broke
> on q35, now everything is going to need to specify a type).

....which is still nothing compared to how many users will be impacted
and the docs all over the internet we'll never be able to change, which
give illustrations using qemu command lines without '-M pc'

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

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
> > * Thomas Huth (thuth@redhat.com) wrote:
> > > On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > > > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > > >    Hi,
> > > > > > Given the semantic differences from 'i440fx', changing the default
> > > > > > machine type has effects that are equivalent to breaking command
> > > > > > line syntax compatibility, which is something we've always tried
> > > > > > to avoid.
> > > > > 
> > > > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > > > a default, effectively making -M $something mandatory, similar to arm.
> > > > 
> > > > Oh, that's probably easy to do;  what are other peoples thoughts on
> > > > that?
> > > 
> > > I agree with Gerd. Getting rid of a default machine on x86 is likely better
> > > than silently changing it to q35. But I'd maybe say that this should go
> > > through the deprecation process first?
> > 
> > So just adding something like the following under 'System emulator
> > machines':
> > 
> > x86 default machine type
> > ------------------------
> > 
> > x86 currently defaults to the very old ```pc``` machine type
> > which is based on the very old ```i440f``` chipset.  This default
> > will be removed and the user will be required to specify a machine
> > type explicitly using -M; users are encouraged to switch to the
> > not quite as old ```q35``` machine types.
> > 
> > 
> > (This option is going to take a lot more work switching all the
> > test cases over; in my world here I'd only changed the tests that broke
> > on q35, now everything is going to need to specify a type).
> 
> ....which is still nothing compared to how many users will be impacted
> and the docs all over the internet we'll never be able to change, which
> give illustrations using qemu command lines without '-M pc'

What's your prreference - it sounds like you'd prefer to leave the
default as 'pc' ?

aarch's message is:
qemu-system-aarch64: No machine specified, and there is no default
Use -machine help to list supported machines

We could add a:
'Use -machine pc for the old default behaviour'

Dave

> 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 :|
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Thomas Huth]

On 16/02/2022 18.57, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
>>> * Thomas Huth (thuth@redhat.com) wrote:
>>>> On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
>>>>> * Gerd Hoffmann (kraxel@redhat.com) wrote:
>>>>>>     Hi,
>>>>>>> Given the semantic differences from 'i440fx', changing the default
>>>>>>> machine type has effects that are equivalent to breaking command
>>>>>>> line syntax compatibility, which is something we've always tried
>>>>>>> to avoid.
>>>>>>
>>>>>> And if we are fine breaking backward compatibility I'd rather *not* pick
>>>>>> a default, effectively making -M $something mandatory, similar to arm.
>>>>>
>>>>> Oh, that's probably easy to do;  what are other peoples thoughts on
>>>>> that?
>>>>
>>>> I agree with Gerd. Getting rid of a default machine on x86 is likely better
>>>> than silently changing it to q35. But I'd maybe say that this should go
>>>> through the deprecation process first?
>>>
>>> So just adding something like the following under 'System emulator
>>> machines':
>>>
>>> x86 default machine type
>>> ------------------------
>>>
>>> x86 currently defaults to the very old ```pc``` machine type
>>> which is based on the very old ```i440f``` chipset.  This default
>>> will be removed and the user will be required to specify a machine
>>> type explicitly using -M; users are encouraged to switch to the
>>> not quite as old ```q35``` machine types.
>>>
>>>
>>> (This option is going to take a lot more work switching all the
>>> test cases over; in my world here I'd only changed the tests that broke
>>> on q35, now everything is going to need to specify a type).
>>
>> ....which is still nothing compared to how many users will be impacted
>> and the docs all over the internet we'll never be able to change, which
>> give illustrations using qemu command lines without '-M pc'
> 
> What's your prreference - it sounds like you'd prefer to leave the
> default as 'pc' ?
> 
> aarch's message is:
> qemu-system-aarch64: No machine specified, and there is no default
> Use -machine help to list supported machines
> 
> We could add a:
> 'Use -machine pc for the old default behaviour'

Sounds like a good idea, but then I'd also do:

'Use -M pc for the old default behaviour, or -M q35 for a more modern 
machine' or so.

  Thomas

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Michael S. Tsirkin]

On Thu, Feb 17, 2022 at 08:46:55AM +0100, Thomas Huth wrote:
> On 16/02/2022 18.57, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
> > > > * Thomas Huth (thuth@redhat.com) wrote:
> > > > > On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > > > > > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > > > > >     Hi,
> > > > > > > > Given the semantic differences from 'i440fx', changing the default
> > > > > > > > machine type has effects that are equivalent to breaking command
> > > > > > > > line syntax compatibility, which is something we've always tried
> > > > > > > > to avoid.
> > > > > > > 
> > > > > > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > > > > > a default, effectively making -M $something mandatory, similar to arm.
> > > > > > 
> > > > > > Oh, that's probably easy to do;  what are other peoples thoughts on
> > > > > > that?
> > > > > 
> > > > > I agree with Gerd. Getting rid of a default machine on x86 is likely better
> > > > > than silently changing it to q35. But I'd maybe say that this should go
> > > > > through the deprecation process first?
> > > > 
> > > > So just adding something like the following under 'System emulator
> > > > machines':
> > > > 
> > > > x86 default machine type
> > > > ------------------------
> > > > 
> > > > x86 currently defaults to the very old ```pc``` machine type
> > > > which is based on the very old ```i440f``` chipset.  This default
> > > > will be removed and the user will be required to specify a machine
> > > > type explicitly using -M; users are encouraged to switch to the
> > > > not quite as old ```q35``` machine types.
> > > > 
> > > > 
> > > > (This option is going to take a lot more work switching all the
> > > > test cases over; in my world here I'd only changed the tests that broke
> > > > on q35, now everything is going to need to specify a type).
> > > 
> > > ....which is still nothing compared to how many users will be impacted
> > > and the docs all over the internet we'll never be able to change, which
> > > give illustrations using qemu command lines without '-M pc'
> > 
> > What's your prreference - it sounds like you'd prefer to leave the
> > default as 'pc' ?
> > 
> > aarch's message is:
> > qemu-system-aarch64: No machine specified, and there is no default
> > Use -machine help to list supported machines
> > 
> > We could add a:
> > 'Use -machine pc for the old default behaviour'
> 
> Sounds like a good idea, but then I'd also do:
> 
> 'Use -M pc for the old default behaviour, or -M q35 for a more modern
> machine' or so.
> 
>  Thomas
> 


How about we start by going through a deprecation period then?  Make
attempts to create a machine without -machine produce a warning for a
couple of cycles.

-- 
MST

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Daniel P. Berrangé]

On Wed, Feb 16, 2022 at 05:57:02PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
> > > * Thomas Huth (thuth@redhat.com) wrote:
> > > > On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > > > > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > > > >    Hi,
> > > > > > > Given the semantic differences from 'i440fx', changing the default
> > > > > > > machine type has effects that are equivalent to breaking command
> > > > > > > line syntax compatibility, which is something we've always tried
> > > > > > > to avoid.
> > > > > > 
> > > > > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > > > > a default, effectively making -M $something mandatory, similar to arm.
> > > > > 
> > > > > Oh, that's probably easy to do;  what are other peoples thoughts on
> > > > > that?
> > > > 
> > > > I agree with Gerd. Getting rid of a default machine on x86 is likely better
> > > > than silently changing it to q35. But I'd maybe say that this should go
> > > > through the deprecation process first?
> > > 
> > > So just adding something like the following under 'System emulator
> > > machines':
> > > 
> > > x86 default machine type
> > > ------------------------
> > > 
> > > x86 currently defaults to the very old ```pc``` machine type
> > > which is based on the very old ```i440f``` chipset.  This default
> > > will be removed and the user will be required to specify a machine
> > > type explicitly using -M; users are encouraged to switch to the
> > > not quite as old ```q35``` machine types.
> > > 
> > > 
> > > (This option is going to take a lot more work switching all the
> > > test cases over; in my world here I'd only changed the tests that broke
> > > on q35, now everything is going to need to specify a type).
> > 
> > ....which is still nothing compared to how many users will be impacted
> > and the docs all over the internet we'll never be able to change, which
> > give illustrations using qemu command lines without '-M pc'
> 
> What's your prreference - it sounds like you'd prefer to leave the
> default as 'pc' ?

Yes.

> aarch's message is:
> qemu-system-aarch64: No machine specified, and there is no default
> Use -machine help to list supported machines
> 
> We could add a:
> 'Use -machine pc for the old default behaviour'

Does this really serve to benefit our users though ? As noted, this
will have no benefit to any mgmt app using libvirt, as it'll still
explicitly set 'pc' by default. So we only impact basic users who
are relying on QEMU's built-in defaults.

It feels like the arguments for removing the machine type default
apply equally to other aspects of QEMU defaults. ie lets make
-cpu, -accel, -m, -smp, and -display mandatory too, since the
defaults might not be right for some users ?

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

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Wed, Feb 16, 2022 at 05:57:02PM +0000, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > On Wed, Feb 16, 2022 at 05:40:44PM +0000, Dr. David Alan Gilbert wrote:
> > > > * Thomas Huth (thuth@redhat.com) wrote:
> > > > > On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
> > > > > > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > > > > >    Hi,
> > > > > > > > Given the semantic differences from 'i440fx', changing the default
> > > > > > > > machine type has effects that are equivalent to breaking command
> > > > > > > > line syntax compatibility, which is something we've always tried
> > > > > > > > to avoid.
> > > > > > > 
> > > > > > > And if we are fine breaking backward compatibility I'd rather *not* pick
> > > > > > > a default, effectively making -M $something mandatory, similar to arm.
> > > > > > 
> > > > > > Oh, that's probably easy to do;  what are other peoples thoughts on
> > > > > > that?
> > > > > 
> > > > > I agree with Gerd. Getting rid of a default machine on x86 is likely better
> > > > > than silently changing it to q35. But I'd maybe say that this should go
> > > > > through the deprecation process first?
> > > > 
> > > > So just adding something like the following under 'System emulator
> > > > machines':
> > > > 
> > > > x86 default machine type
> > > > ------------------------
> > > > 
> > > > x86 currently defaults to the very old ```pc``` machine type
> > > > which is based on the very old ```i440f``` chipset.  This default
> > > > will be removed and the user will be required to specify a machine
> > > > type explicitly using -M; users are encouraged to switch to the
> > > > not quite as old ```q35``` machine types.
> > > > 
> > > > 
> > > > (This option is going to take a lot more work switching all the
> > > > test cases over; in my world here I'd only changed the tests that broke
> > > > on q35, now everything is going to need to specify a type).
> > > 
> > > ....which is still nothing compared to how many users will be impacted
> > > and the docs all over the internet we'll never be able to change, which
> > > give illustrations using qemu command lines without '-M pc'
> > 
> > What's your prreference - it sounds like you'd prefer to leave the
> > default as 'pc' ?
> 
> Yes.

Damn, that means we've got a 3 way split of people preferring pc, q35
and no default.

> > aarch's message is:
> > qemu-system-aarch64: No machine specified, and there is no default
> > Use -machine help to list supported machines
> > 
> > We could add a:
> > 'Use -machine pc for the old default behaviour'
> 
> Does this really serve to benefit our users though ? As noted, this
> will have no benefit to any mgmt app using libvirt, as it'll still
> explicitly set 'pc' by default. So we only impact basic users who
> are relying on QEMU's built-in defaults.

Right; since none of this impacts libvirt users it's all about the
(substantial number of) users who run qemu manually.
Any change will surprise them, if the change (no default) is going to
produce an error, then it may as well be a helpful error telling them
how to fix it.

> It feels like the arguments for removing the machine type default
> apply equally to other aspects of QEMU defaults. ie lets make
> -cpu, -accel, -m, -smp, and -display mandatory too, since the
> defaults might not be right for some users ?

My preference here was changing the default to q35 rather than
requiring an explicit selection; however I don't think requiring -M is
unreasonable, since it's an easy enough selection.

Requiring '-m' might also be a reasonable requirement - there's very few
use cases on x86 where the default works (probably just running DOS).

Dave


> 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 :|
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Thomas Huth]

On 16/02/2022 18.40, Dr. David Alan Gilbert wrote:
> * Thomas Huth (thuth@redhat.com) wrote:
>> On 16/02/2022 12.01, Dr. David Alan Gilbert wrote:
>>> * Gerd Hoffmann (kraxel@redhat.com) wrote:
>>>>     Hi,
>>>>> Given the semantic differences from 'i440fx', changing the default
>>>>> machine type has effects that are equivalent to breaking command
>>>>> line syntax compatibility, which is something we've always tried
>>>>> to avoid.
>>>>
>>>> And if we are fine breaking backward compatibility I'd rather *not* pick
>>>> a default, effectively making -M $something mandatory, similar to arm.
>>>
>>> Oh, that's probably easy to do;  what are other peoples thoughts on
>>> that?
>>
>> I agree with Gerd. Getting rid of a default machine on x86 is likely better
>> than silently changing it to q35. But I'd maybe say that this should go
>> through the deprecation process first?
> 
> So just adding something like the following under 'System emulator
> machines':
> 
> x86 default machine type
> ------------------------
> 
> x86 currently defaults to the very old ```pc``` machine type

I'd scrath the "very old" above since you repeat it below...

> which is based on the very old ```i440f``` chipset.  This default
> will be removed and the user will be required to specify a machine
> type explicitly using -M; users are encouraged to switch to the
> not quite as old ```q35``` machine types.

... but otherwise this sounds good to me.

> (This option is going to take a lot more work switching all the
> test cases over; in my world here I'd only changed the tests that broke
> on q35, now everything is going to need to specify a type).

We've got a bunch of targets now already that do not have a default machine 
type yet (aarch64/arm, avr, rx, tricore), and some where the default machine 
type does not make too much sense for testing anyway (e.g. m68k) ... so it 
would maybe be good to have a global qtest_get_default_machine() function in 
the qtest framework anyway instead of re-encoding this in each and every 
test case.

Anyway, if we agree that the default machine type of x86 should go through 
the deprecation process, we've got plenty of time to fix this up in the 
tests, no need to rush this now before 7.0.

Other heretic question: Should we maybe get rid of the default machine type 
for *all* targets? ... so that we do not continue to run into this issue 
again and again and again...

  Thomas

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Gerd Hoffmann]

  Hi,

> Other heretic question: Should we maybe get rid of the default machine type
> for *all* targets? ... so that we do not continue to run into this issue
> again and again and again...

/me votes "yes".

take care,
  Gerd

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Michael S. Tsirkin]

On Thu, Feb 17, 2022 at 09:08:52AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Other heretic question: Should we maybe get rid of the default machine type
> > for *all* targets? ... so that we do not continue to run into this issue
> > again and again and again...
> 
> /me votes "yes".
> 
> take care,
>   Gerd

Well originally qemu tried to be friendly and to just create
a reasonable machine when given a disk, to the point where
it would even set up some networking by default.

And I think it's not a bad idea, forcing everyone to specify a bunch of
boilerplate does not really result in people researching which machine
type is good for them, people instead just copy paste from a random
website.

So maybe we can detect that basic usage somehow (I only have some
vague ideas) and then set a bunch of defaults that we consider
best?

-- 
MST

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Daniel P. Berrangé]

On Thu, Feb 17, 2022 at 03:17:04AM -0500, Michael S. Tsirkin wrote:
> On Thu, Feb 17, 2022 at 09:08:52AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > Other heretic question: Should we maybe get rid of the default machine type
> > > for *all* targets? ... so that we do not continue to run into this issue
> > > again and again and again...
> > 
> > /me votes "yes".
> > 
> > take care,
> >   Gerd
> 
> Well originally qemu tried to be friendly and to just create
> a reasonable machine when given a disk, to the point where
> it would even set up some networking by default.
> 
> And I think it's not a bad idea, forcing everyone to specify a bunch of
> boilerplate does not really result in people researching which machine
> type is good for them, people instead just copy paste from a random
> website.
> 
> So maybe we can detect that basic usage somehow (I only have some
> vague ideas) and then set a bunch of defaults that we consider
> best?

The "basic usage" is the majority of people using QEMU directly,
as opposed to via a MGMT app. As I pointed out elserwhere in this
thread, changing the machine type in QEMU won't have any effect
on apps using libvirt, since libvirt explicitly override's QEMU's
default. So with that in mind, this proposed QEMU level defaults
change is primarily going to impact the basic usage, as those
people aren't using libvirt. IOW, to avoid impacting basic usage
don't change the default in current QEMU binaries at all.


Separately, we've got active work going on to try to eliminate
the need for QemuOpts and enable entirely QAPI based runtime
configuration. That is intending to start off by adding new system
emulator binaries targetted exclusively at machine usage, to avoid
impacting back compat of existing binaries wrt human usage. That
will likely eliminate the notion of a default machine entirely,
because you'll be required to issue a QMP command to select a
machine as one of the very first config steps.

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

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Igor Mammedov]

On Thu, 17 Feb 2022 03:17:04 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Feb 17, 2022 at 09:08:52AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >   
> > > Other heretic question: Should we maybe get rid of the default machine type
> > > for *all* targets? ... so that we do not continue to run into this issue
> > > again and again and again...  
> > 
> > /me votes "yes".
> > 
> > take care,
> >   Gerd  
> 
> Well originally qemu tried to be friendly and to just create
> a reasonable machine when given a disk, to the point where
> it would even set up some networking by default.
> 
> And I think it's not a bad idea, forcing everyone to specify a bunch of
> boilerplate does not really result in people researching which machine
> type is good for them, people instead just copy paste from a random
> website.
> 
> So maybe we can detect that basic usage somehow (I only have some
> vague ideas) and then set a bunch of defaults that we consider
> best?
or ship with QEMU a wrapper script that does set defaults as
older QEMU used to do. But it will still require user to
change a called binary name, or maybe we can rename QEMU binary
to something else and name wrapper as original QEMU.

Re: [PATCH 3/3] x86: Switch to q35 as the default machine type

[Kashyap Chamarthy]

On Thu, Feb 17, 2022 at 08:57:19AM +0100, Thomas Huth wrote:
> On 16/02/2022 18.40, Dr. David Alan Gilbert wrote:

[...]

> > So just adding something like the following under 'System emulator
> > machines':
> > 
> > x86 default machine type
> > ------------------------
> > 
> > x86 currently defaults to the very old ```pc``` machine type
> 
> I'd scrath the "very old" above since you repeat it below...
> 
> > which is based on the very old ```i440f``` chipset.  This default
> > will be removed and the user will be required to specify a machine
> > type explicitly using -M; users are encouraged to switch to the
> > not quite as old ```q35``` machine types.
> 
> ... but otherwise this sounds good to me.

Yeah, _not_ picking a default sounds good to me.  As there might come a
better machine type for x86 too, just like how "virt" machine type was
expressly designed for AArch64 guests.
 
> > (This option is going to take a lot more work switching all the
> > test cases over; in my world here I'd only changed the tests that broke
> > on q35, now everything is going to need to specify a type).
> 
> We've got a bunch of targets now already that do not have a default machine
> type yet (aarch64/arm, avr, rx, tricore), and some where the default machine
> type does not make too much sense for testing anyway (e.g. m68k) ... so it
> would maybe be good to have a global qtest_get_default_machine() function in
> the qtest framework anyway instead of re-encoding this in each and every
> test case.
> 
> Anyway, if we agree that the default machine type of x86 should go through
> the deprecation process, we've got plenty of time to fix this up in the
> tests, no need to rush this now before 7.0.
> 
> Other heretic question: Should we maybe get rid of the default machine type
> for *all* targets? ... so that we do not continue to run into this issue
> again and again and again...

Not at all heretic :-) I think the same reasoning above works here too.
FWIW, I agree to make the behaviour consistent across all targets.

[...] 

-- 
/kashyap

[PATCH 0/3] x86: Switch over to q35 as the default machine type

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

We've been using q35 heavily for a while now and it generally works
quite nicely; downstream in RH we prefer it as our default, and I wanted
to see what people think of making it the default.

The only pain really is that it requires some more setup for hotplug;
so for now I've forced hotplug tests to stay on i440fx aka pc.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


Dr. David Alan Gilbert (3):
  tests/x86: Use 'pc' machine type for old hardware tests
  tests/x86: Use 'pc' machine type for hotplug tests
  x86: Switch to q35 as the default machine type

 hw/i386/pc_piix.c              | 12 +-----------
 hw/i386/pc_q35.c               |  2 ++
 tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
 tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
 tests/qtest/fdc-test.c         |  2 +-
 tests/qtest/hd-geo-test.c      | 16 +++++++++++-----
 tests/qtest/i440fx-test.c      |  2 +-
 tests/qtest/ide-test.c         |  3 ++-
 tests/qtest/ivshmem-test.c     |  7 ++++++-
 9 files changed, 72 insertions(+), 27 deletions(-)

-- 
2.35.1

[PATCH 1/3] tests/x86: Use 'pc' machine type for old hardware tests

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

For tests that rely on old hardware, e.g. floppies or IDE drives,
explicitly select the 'pc' machine type.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/fdc-test.c    |  2 +-
 tests/qtest/hd-geo-test.c | 12 +++++++++---
 tests/qtest/i440fx-test.c |  2 +-
 tests/qtest/ide-test.c    |  3 ++-
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 8f6eee84a4..b0d40012e6 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -598,7 +598,7 @@ int main(int argc, char **argv)
     /* Run the tests */
     g_test_init(&argc, &argv, NULL);
 
-    qtest_start("-device floppy,id=floppy0");
+    qtest_start("-machine pc -device floppy,id=floppy0");
     qtest_irq_intercept_in(global_qtest, "ioapic");
     qtest_add_func("/fdc/cmos", test_cmos);
     qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 771eaa741b..3554b5d500 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -178,9 +178,15 @@ static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
 
 static int setup_common(char *argv[], int argv_sz)
 {
+    int new_argc;
     memset(cur_ide, 0, sizeof(cur_ide));
-    return append_arg(0, argv, argv_sz,
-                      g_strdup("-nodefaults"));
+    new_argc = append_arg(0, argv, argv_sz,
+                          g_strdup("-nodefaults"));
+    new_argc = append_arg(new_argc, argv, argv_sz,
+                          g_strdup("-machine"));
+    new_argc = append_arg(new_argc, argv, argv_sz,
+                          g_strdup("pc"));
+    return new_argc;
 }
 
 static void setup_mbr(int img_idx, MBRcontents mbr)
@@ -697,7 +703,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
index 1f57d9684b..6d7d4d8d8f 100644
--- a/tests/qtest/i440fx-test.c
+++ b/tests/qtest/i440fx-test.c
@@ -35,7 +35,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
 {
     char *cmdline;
 
-    cmdline = g_strdup_printf("-smp %d", s->num_cpus);
+    cmdline = g_strdup_printf("-machine pc -smp %d", s->num_cpus);
     qtest_start(cmdline);
     g_free(cmdline);
     return qpci_new_pc(global_qtest, NULL);
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 3f8081e77d..84935578fb 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -128,10 +128,11 @@ static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
 static QTestState *ide_test_start(const char *cmdline_fmt, ...)
 {
     QTestState *qts;
+    g_autofree char *full_fmt = g_strdup_printf("-machine pc %s", cmdline_fmt);
     va_list ap;
 
     va_start(ap, cmdline_fmt);
-    qts = qtest_vinitf(cmdline_fmt, ap);
+    qts = qtest_vinitf(full_fmt, ap);
     va_end(ap);
 
     pc_alloc_init(&guest_malloc, qts, 0);
-- 
2.35.1

[PATCH 2/3] tests/x86: Use 'pc' machine type for hotplug tests

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Hotplug tests need a bridge setting up on q35, for now
keep them on 'pc'.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/device-plug-test.c | 20 +++++++++++++++++--
 tests/qtest/drive_del-test.c   | 35 +++++++++++++++++++++++++++++-----
 tests/qtest/hd-geo-test.c      |  4 ++--
 tests/qtest/ivshmem-test.c     |  7 ++++++-
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index ad79bd4c14..404a92e132 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -63,7 +63,15 @@ static void wait_device_deleted_event(QTestState *qtest, const char *id)
 
 static void test_pci_unplug_request(void)
 {
-    QTestState *qtest = qtest_initf("-device virtio-mouse-pci,id=dev0");
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
+
+    QTestState *qtest = qtest_initf("%s -device virtio-mouse-pci,id=dev0",
+                                    machine_addition);
 
     /*
      * Request device removal. As the guest is not running, the request won't
@@ -79,8 +87,16 @@ static void test_pci_unplug_request(void)
 
 static void test_pci_unplug_json_request(void)
 {
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
+
     QTestState *qtest = qtest_initf(
-        "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'");
+        "%s -device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'",
+        machine_addition);
 
     /*
      * Request device removal. As the guest is not running, the request won't
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 8d08ee9995..0cc18dfa4a 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -235,14 +235,21 @@ static void test_drive_del_device_del(void)
 static void test_cli_device_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
     /*
      * -drive/-device and device_del.  Start with a drive used by a
      * device that unplugs after reset.
      */
-    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
                       "file.read-zeroes=on,format=raw"
                       " -device virtio-blk-%s,drive=drive0,id=dev0",
+                      machine_addition,
                       qvirtio_get_dev_type());
 
     device_del(qts, true);
@@ -266,13 +273,19 @@ static void test_empty_device_del(void)
 static void test_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
     /*
      * -drive/device_add and device_del.  Start with a drive used by a
      * device that unplugs after reset.
      */
-    qts = qtest_init("-drive if=none,id=drive0,file=null-co://,"
-                     "file.read-zeroes=on,format=raw");
+    qts = qtest_initf("%s -drive if=none,id=drive0,file=null-co://,"
+                     "file.read-zeroes=on,format=raw", machine_addition);
 
     device_add(qts);
     device_del(qts, true);
@@ -284,8 +297,14 @@ static void test_device_add_and_del(void)
 static void test_drive_add_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
-    qts = qtest_init("");
+    qts = qtest_init(machine_addition);
 
     /*
      * drive_add/device_add and device_del.  The drive is used by a
@@ -302,8 +321,14 @@ static void test_drive_add_device_add_and_del(void)
 static void test_blockdev_add_device_add_and_del(void)
 {
     QTestState *qts;
+    const char *arch = qtest_get_arch();
+    const char *machine_addition = "";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        machine_addition = "-machine pc";
+    }
 
-    qts = qtest_init("");
+    qts = qtest_init(machine_addition);
 
     /*
      * blockdev_add/device_add and device_del.  The it drive is used by a
diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 3554b5d500..64023c0574 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -839,7 +839,7 @@ static void test_override_scsi_hot_unplug(void)
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
@@ -899,7 +899,7 @@ static void test_override_virtio_hot_unplug(void)
 
     joined_args = g_strjoinv(" ", args->argv);
 
-    qts = qtest_init(joined_args);
+    qts = qtest_initf("-machine pc %s", joined_args);
     fw_cfg = pc_fw_cfg_init(qts);
 
     read_bootdevices(fw_cfg, expected);
diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index fe94dd3b96..4e8af42a9d 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -385,7 +385,12 @@ static void test_ivshmem_hotplug(void)
     QTestState *qts;
     const char *arch = qtest_get_arch();
 
-    qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
+                         " -machine pc");
+    } else {
+        qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1");
+    }
 
     qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
                          "{'addr': %s, 'memdev': 'mb1'}",
-- 
2.35.1

[PATCH 3/3] x86: Switch to q35 as the default machine type

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The 'q35' machine type series has been around for a few years now, and
is getting heavily used downstream without many problems;  lets flip
to using it as the default.

While it is of course newer and shinier than it's old i440fx cousin,
the main reasons are:
  * PCIe support
  * No default floppy or IDE
  * More modern defaults for NIC
  * Better OVMF support

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/i386/pc_piix.c | 12 +-----------
 hw/i386/pc_q35.c  |  2 ++
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d9b344248d..50c7e6c2c0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -420,7 +420,7 @@ static void pc_i440fx_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_machine_options(m);
     m->alias = "pc";
-    m->is_default = true;
+    m->is_default = false;
     pcmc->default_cpu_version = 1;
 }
 
@@ -431,7 +431,6 @@ static void pc_i440fx_6_2_machine_options(MachineClass *m)
 {
     pc_i440fx_7_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
     compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
 }
@@ -443,7 +442,6 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
 {
     pc_i440fx_6_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
     m->smp_props.prefer_sockets = true;
@@ -456,7 +454,6 @@ static void pc_i440fx_6_0_machine_options(MachineClass *m)
 {
     pc_i440fx_6_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_0, hw_compat_6_0_len);
     compat_props_add(m->compat_props, pc_compat_6_0, pc_compat_6_0_len);
 }
@@ -468,7 +465,6 @@ static void pc_i440fx_5_2_machine_options(MachineClass *m)
 {
     pc_i440fx_6_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
     compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
 }
@@ -482,7 +478,6 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m)
 
     pc_i440fx_5_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
@@ -496,7 +491,6 @@ static void pc_i440fx_5_0_machine_options(MachineClass *m)
 {
     pc_i440fx_5_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     m->numa_mem_supported = true;
     compat_props_add(m->compat_props, hw_compat_5_0, hw_compat_5_0_len);
     compat_props_add(m->compat_props, pc_compat_5_0, pc_compat_5_0_len);
@@ -510,7 +504,6 @@ static void pc_i440fx_4_2_machine_options(MachineClass *m)
 {
     pc_i440fx_5_0_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
     compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
 }
@@ -522,7 +515,6 @@ static void pc_i440fx_4_1_machine_options(MachineClass *m)
 {
     pc_i440fx_4_2_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_4_1, hw_compat_4_1_len);
     compat_props_add(m->compat_props, pc_compat_4_1, pc_compat_4_1_len);
 }
@@ -535,7 +527,6 @@ static void pc_i440fx_4_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_i440fx_4_1_machine_options(m);
     m->alias = NULL;
-    m->is_default = false;
     pcmc->default_cpu_version = CPU_VERSION_LEGACY;
     compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
     compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
@@ -549,7 +540,6 @@ static void pc_i440fx_3_1_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 
     pc_i440fx_4_0_machine_options(m);
-    m->is_default = false;
     pcmc->do_not_add_smb_acpi = true;
     m->smbus_no_migration_support = true;
     m->alias = NULL;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1780f79bc1..aec4e09c9f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -365,6 +365,7 @@ static void pc_q35_7_0_machine_options(MachineClass *m)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
     pc_q35_machine_options(m);
     m->alias = "q35";
+    m->is_default = true;
     pcmc->default_cpu_version = 1;
 }
 
@@ -375,6 +376,7 @@ static void pc_q35_6_2_machine_options(MachineClass *m)
 {
     pc_q35_7_0_machine_options(m);
     m->alias = NULL;
+    m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_2, hw_compat_6_2_len);
     compat_props_add(m->compat_props, pc_compat_6_2, pc_compat_6_2_len);
 }
-- 
2.35.1