[libvirt] [PATCH v3 0/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm
This patch series adds inbound migrate capability from qemu-kvm version 1.0. The main ideas are those set out in Cole Robinson's patch here: http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qem... however, rather than patching statically (and breaking inbound migration on existing machine types), I have added a new machine type (pc-1.0-qemu-kvm) without affecting any other machine types. The existing pc-1.0 machine type is renamed to pc-1.0-qemu-git, with pc-1.0 becoming an alias for one or another, as selected by a configure option (defaulting to pc-1.0-qemu-git, IE no change). Two aproaches are taken: * In hw/timer/i8254_common.c, the VMSTATE_UINT32_TEST macro is used to test the version for the irq_disable flags, allowing version 3 or more, or version 2 for an inbound migrate from qemu-kvm (only). * In hw/acpi/piix4.c, qemu-kvm incorrectly uses version 2 for a version 3 structure, causing acpi_load_old to be used. acpi_load_old detects this situation based on the machine type and restarts the attempt to load the vmstate using a customised VMStateDescription. The above cleaner approach is unavailable here. I developed this on qemu 2.0 but have forward ported it (trivially) to master. My testing has been on a VM live-migrated-to-file from Ubuntu Precise qemu-kvm 1.0. I have given this a moderate degree of testing but it could do with more. Note that certain hardware devices (including QXL) will not migrate properly due to a fundamental difference in their internal state between versions. Also note that (as expected) migration from qemu-2.x to qemu-1.0 will not work, even if the machine types are the same. Alex Bligh (2): Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm Add configure option --enable-pc-1-0-qemu-kvm configure | 12 ++++++++++++ hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- hw/i386/pc_piix.c | 38 +++++++++++++++++++++++++++++++++++++- hw/timer/i8254_common.c | 10 +++++++++- 4 files changed, 103 insertions(+), 4 deletions(-) -- 1.7.9.5
Add a machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm version 1.0. Signed-off-by: Alex Bligh <alex@alex.org.uk> --- hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- hw/i386/pc_piix.c | 30 ++++++++++++++++++++++++++++++ hw/timer/i8254_common.c | 10 +++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..e016005 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -37,6 +37,8 @@ #include "hw/acpi/memory_hotplug.h" #include "hw/acpi/acpi_dev_interface.h" +extern int qemu_kvm_1_0_compat; + //#define DEBUG #ifdef DEBUG @@ -200,12 +202,24 @@ static const VMStateDescription vmstate_pci_status = { } }; +static const VMStateDescription vmstate_acpi_compat; + static int acpi_load_old(QEMUFile *f, void *opaque, int version_id) { PIIX4PMState *s = opaque; int ret, i; uint16_t temp; + /* If we are expecting the inbound migration to come from + * qemu-kvm 1.0, it will have a version_id of 2 but really + * be version 3, so call back the original vmstate_load_state + * with a different more tolerant vmstate descriptor + */ + if ((version_id == 2) && (qemu_kvm_1_0_compat)) { + return vmstate_load_state(f, &vmstate_acpi_compat, + opaque, version_id); + } + ret = pci_device_load(PCI_DEVICE(s), f); if (ret < 0) { return ret; @@ -267,8 +281,9 @@ static const VMStateDescription vmstate_memhp_state = { }; /* qemu-kvm 1.2 uses version 3 but advertised as 2 - * To support incoming qemu-kvm 1.2 migration, change version_id - * and minimum_version_id to 2 below (which breaks migration from + * To support incoming qemu-kvm 1.2 migration, we support + * via a command line option a change to minimum_version_id + * of 2 in a _compat structure (which breaks migration from * qemu 1.2). * */ @@ -307,6 +322,34 @@ static const VMStateDescription vmstate_acpi = { } }; +static const VMStateDescription vmstate_acpi_compat = { + .name = "piix4_pm", + .version_id = 3, + .minimum_version_id = 2, + .minimum_version_id_old = 1, + .load_state_old = NULL, /* to avoid recursion */ + .post_load = vmstate_acpi_post_load, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(parent_obj, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.evt.sts, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState), + VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState), + VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState), + VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState), + VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), + VMSTATE_STRUCT_TEST( + acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT], + PIIX4PMState, + vmstate_test_no_use_acpi_pci_hotplug, + 2, vmstate_pci_status, + struct AcpiPciHpPciStatus), + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState, + vmstate_test_use_acpi_pci_hotplug), + VMSTATE_END_OF_LIST() + } +}; + static void piix4_reset(void *opaque) { PIIX4PMState *s = opaque; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7081c08..48a4942 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -70,6 +70,8 @@ static bool smbios_legacy_mode; static bool gigabyte_align = true; static bool has_reserved_memory = true; +int qemu_kvm_1_0_compat; + /* PC hardware initialisation */ static void pc_init1(MachineState *machine, int pci_enabled, @@ -386,6 +388,14 @@ static void pc_init_pci_1_2(MachineState *machine) pc_init_pci(machine); } +/* PC machine init function for qemu-kvm 1.0 */ +static void pc_init_pci_1_2_qemu_kvm(MachineState *machine) +{ + qemu_kvm_1_0_compat = 1; + pc_compat_1_2(machine); + pc_init_pci(machine); +} + /* PC init function for pc-0.10 to pc-0.13 */ static void pc_init_pci_no_kvmclock(MachineState *machine) { @@ -644,6 +654,25 @@ static QEMUMachine pc_machine_v1_0 = { .hw_version = "1.0", }; +#define PC_COMPAT_1_0_QEMU_KVM \ + PC_COMPAT_1_0,\ + {\ + .driver = "cirrus-vga",\ + .property = "vgamem_mb",\ + .value = stringify(16),\ + } + +static QEMUMachine pc_machine_v1_0_qemu_kvm = { + PC_I440FX_1_2_MACHINE_OPTIONS, + .name = "pc-1.0-qemu-kvm", + .init = pc_init_pci_1_2_qemu_kvm, + .compat_props = (GlobalProperty[]) { + PC_COMPAT_1_0_QEMU_KVM, + { /* end of list */ } + }, + .hw_version = "1.0", +}; + #define PC_COMPAT_0_15 \ PC_COMPAT_1_0 @@ -886,6 +915,7 @@ static void pc_machine_init(void) qemu_register_pc_machine(&pc_machine_v1_2); qemu_register_pc_machine(&pc_machine_v1_1); qemu_register_pc_machine(&pc_machine_v1_0); + qemu_register_pc_machine(&pc_machine_v1_0_qemu_kvm); qemu_register_pc_machine(&pc_machine_v0_15); qemu_register_pc_machine(&pc_machine_v0_14); qemu_register_pc_machine(&pc_machine_v0_13); diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c index 07345f6..b4deead 100644 --- a/hw/timer/i8254_common.c +++ b/hw/timer/i8254_common.c @@ -29,6 +29,8 @@ #include "hw/timer/i8254.h" #include "hw/timer/i8254_internal.h" +extern int qemu_kvm_1_0_compat; + /* val must be 0 or 1 */ void pit_set_gate(ISADevice *dev, int channel, int val) { @@ -257,6 +259,11 @@ static int pit_dispatch_post_load(void *opaque, int version_id) return 0; } +static bool has_irq_disabled(void *opaque, int version_id) +{ + return (version_id >= 3) || ((version_id == 2) && qemu_kvm_1_0_compat); +} + static const VMStateDescription vmstate_pit_common = { .name = "i8254", .version_id = 3, @@ -266,7 +273,8 @@ static const VMStateDescription vmstate_pit_common = { .pre_save = pit_dispatch_pre_save, .post_load = pit_dispatch_post_load, .fields = (VMStateField[]) { - VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), + VMSTATE_UINT32_TEST(channels[0].irq_disabled, PITCommonState, + has_irq_disabled), VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2, vmstate_pit_channel, PITChannelState), VMSTATE_INT64(channels[0].next_transition_time, -- 1.7.9.5
On Sun, Sep 21, 2014 at 03:38:58PM +0100, Alex Bligh wrote:
Add a machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm version 1.0.
Signed-off-by: Alex Bligh <alex@alex.org.uk> --- hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- hw/i386/pc_piix.c | 30 ++++++++++++++++++++++++++++++ hw/timer/i8254_common.c | 10 +++++++++- 3 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..e016005 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -37,6 +37,8 @@ #include "hw/acpi/memory_hotplug.h" #include "hw/acpi/acpi_dev_interface.h"
+extern int qemu_kvm_1_0_compat; + //#define DEBUG
#ifdef DEBUG
Please dont' put extern declarations in C files. You lose a useful type-safety compile-time check.
@@ -200,12 +202,24 @@ static const VMStateDescription vmstate_pci_status = { } };
+static const VMStateDescription vmstate_acpi_compat; +
Don't declare things like this, re-order them in a separate patch so forward declarations are not necessary.
static int acpi_load_old(QEMUFile *f, void *opaque, int version_id) { PIIX4PMState *s = opaque; int ret, i; uint16_t temp;
+ /* If we are expecting the inbound migration to come from + * qemu-kvm 1.0, it will have a version_id of 2 but really + * be version 3, so call back the original vmstate_load_state + * with a different more tolerant vmstate descriptor + */ + if ((version_id == 2) && (qemu_kvm_1_0_compat)) { + return vmstate_load_state(f, &vmstate_acpi_compat, + opaque, version_id); + } + ret = pci_device_load(PCI_DEVICE(s), f); if (ret < 0) { return ret; @@ -267,8 +281,9 @@ static const VMStateDescription vmstate_memhp_state = { };
/* qemu-kvm 1.2 uses version 3 but advertised as 2 - * To support incoming qemu-kvm 1.2 migration, change version_id - * and minimum_version_id to 2 below (which breaks migration from + * To support incoming qemu-kvm 1.2 migration, we support + * via a command line option a change to minimum_version_id + * of 2 in a _compat structure (which breaks migration from * qemu 1.2). * */ @@ -307,6 +322,34 @@ static const VMStateDescription vmstate_acpi = { } };
+static const VMStateDescription vmstate_acpi_compat = { + .name = "piix4_pm", + .version_id = 3, + .minimum_version_id = 2, + .minimum_version_id_old = 1, + .load_state_old = NULL, /* to avoid recursion */ + .post_load = vmstate_acpi_post_load, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(parent_obj, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.evt.sts, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState), + VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState), + VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState), + VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState), + VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState), + VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), + VMSTATE_STRUCT_TEST( + acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT], + PIIX4PMState, + vmstate_test_no_use_acpi_pci_hotplug, + 2, vmstate_pci_status, + struct AcpiPciHpPciStatus), + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState, + vmstate_test_use_acpi_pci_hotplug), + VMSTATE_END_OF_LIST() + } +}; + static void piix4_reset(void *opaque) { PIIX4PMState *s = opaque; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7081c08..48a4942 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -70,6 +70,8 @@ static bool smbios_legacy_mode; static bool gigabyte_align = true; static bool has_reserved_memory = true;
+int qemu_kvm_1_0_compat; + /* PC hardware initialisation */ static void pc_init1(MachineState *machine, int pci_enabled, @@ -386,6 +388,14 @@ static void pc_init_pci_1_2(MachineState *machine) pc_init_pci(machine); }
+/* PC machine init function for qemu-kvm 1.0 */ +static void pc_init_pci_1_2_qemu_kvm(MachineState *machine) +{ + qemu_kvm_1_0_compat = 1; + pc_compat_1_2(machine); + pc_init_pci(machine); +} + /* PC init function for pc-0.10 to pc-0.13 */ static void pc_init_pci_no_kvmclock(MachineState *machine) { @@ -644,6 +654,25 @@ static QEMUMachine pc_machine_v1_0 = { .hw_version = "1.0", };
+#define PC_COMPAT_1_0_QEMU_KVM \ + PC_COMPAT_1_0,\ + {\ + .driver = "cirrus-vga",\ + .property = "vgamem_mb",\ + .value = stringify(16),\ + } + +static QEMUMachine pc_machine_v1_0_qemu_kvm = { + PC_I440FX_1_2_MACHINE_OPTIONS, + .name = "pc-1.0-qemu-kvm",
I think pc-qemu-kvm-1.0 is slightly better - more consistent with other machine names.
+ .init = pc_init_pci_1_2_qemu_kvm, + .compat_props = (GlobalProperty[]) { + PC_COMPAT_1_0_QEMU_KVM, + { /* end of list */ } + }, + .hw_version = "1.0", +}; + #define PC_COMPAT_0_15 \ PC_COMPAT_1_0
@@ -886,6 +915,7 @@ static void pc_machine_init(void) qemu_register_pc_machine(&pc_machine_v1_2); qemu_register_pc_machine(&pc_machine_v1_1); qemu_register_pc_machine(&pc_machine_v1_0); + qemu_register_pc_machine(&pc_machine_v1_0_qemu_kvm); qemu_register_pc_machine(&pc_machine_v0_15); qemu_register_pc_machine(&pc_machine_v0_14); qemu_register_pc_machine(&pc_machine_v0_13); diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c index 07345f6..b4deead 100644 --- a/hw/timer/i8254_common.c +++ b/hw/timer/i8254_common.c @@ -29,6 +29,8 @@ #include "hw/timer/i8254.h" #include "hw/timer/i8254_internal.h"
+extern int qemu_kvm_1_0_compat; + /* val must be 0 or 1 */ void pit_set_gate(ISADevice *dev, int channel, int val) {
Can you add a device property like we do for other compat behaviours? That's better than a global variable. Same for other devices.
@@ -257,6 +259,11 @@ static int pit_dispatch_post_load(void *opaque, int version_id) return 0; }
+static bool has_irq_disabled(void *opaque, int version_id) +{ + return (version_id >= 3) || ((version_id == 2) && qemu_kvm_1_0_compat); +} + static const VMStateDescription vmstate_pit_common = { .name = "i8254", .version_id = 3, @@ -266,7 +273,8 @@ static const VMStateDescription vmstate_pit_common = { .pre_save = pit_dispatch_pre_save, .post_load = pit_dispatch_post_load, .fields = (VMStateField[]) { - VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), + VMSTATE_UINT32_TEST(channels[0].irq_disabled, PITCommonState, + has_irq_disabled), VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2, vmstate_pit_channel, PITChannelState), VMSTATE_INT64(channels[0].next_transition_time, -- 1.7.9.5
Michael, (Reordering) On 22 Sep 2014, at 12:50, Michael S. Tsirkin <mst@redhat.com> wrote:
@@ -257,6 +259,11 @@ static int pit_dispatch_post_load(void *opaque, int version_id) return 0; }
+static bool has_irq_disabled(void *opaque, int version_id) +{ + return (version_id >= 3) || ((version_id == 2) && qemu_kvm_1_0_compat); +} + Can you add a device property like we do for other compat behaviours? That's better than a global variable. Same for other devices.
I can see 'opaque' points to PIIX4PMState or PITCommonState. Do you mean to put them in there? Could you point to an example to copy. -- Alex Bligh
On Mon, Sep 22, 2014 at 01:28:30PM +0100, Alex Bligh wrote:
Michael,
(Reordering)
On 22 Sep 2014, at 12:50, Michael S. Tsirkin <mst@redhat.com> wrote:
@@ -257,6 +259,11 @@ static int pit_dispatch_post_load(void *opaque, int version_id) return 0; }
+static bool has_irq_disabled(void *opaque, int version_id) +{ + return (version_id >= 3) || ((version_id == 2) && qemu_kvm_1_0_compat); +} + Can you add a device property like we do for other compat behaviours? That's better than a global variable. Same for other devices.
I can see 'opaque' points to PIIX4PMState or PITCommonState. Do you mean to put them in there? Could you point to an example to copy.
Look for PC_COMPAT_ macros, we have a ton of flags like this.
-- Alex Bligh
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled. Rename machine type pc-1.0 to pc-1.0-qemu-git. Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option. Signed-off-by: Alex Bligh <alex@alex.org.uk> --- configure | 12 ++++++++++++ hw/i386/pc_piix.c | 8 +++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/configure b/configure index f7685b5..b143302 100755 --- a/configure +++ b/configure @@ -335,6 +335,7 @@ libssh2="" vhdx="" quorum="" numa="" +pc_1_0_qemu_kvm="no" # parse CC options first for opt do @@ -1125,6 +1126,10 @@ for opt do ;; --enable-numa) numa="yes" ;; + --disable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="no" + ;; + --enable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="yes" + ;; *) echo "ERROR: unknown option $opt" echo "Try '$0 --help' for more information" @@ -1394,6 +1399,8 @@ Advanced options (experts only): --enable-quorum enable quorum block filter support --disable-numa disable libnuma support --enable-numa enable libnuma support + --disable-pc-1-0-qemu-kvm disable pc-1.0 machine type reflecting qemu-kvm + --enable-pc-1-0-qemu-kvm enable pc-1.0 machine type reflecting qemu-kvm NOTE: The object files are built at the place where configure is launched EOF @@ -4262,6 +4269,7 @@ echo "Quorum $quorum" echo "lzo support $lzo" echo "snappy support $snappy" echo "NUMA host support $numa" +echo "pc-1.0 qemu-kvm $pc_1_0_qemu_kvm" if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -5241,6 +5249,10 @@ if test "$numa" = "yes"; then echo "CONFIG_NUMA=y" >> $config_host_mak fi +if test "$pc_1_0_qemu_kvm" = "yes"; then + echo "CONFIG_PC_1_0_QEMU_KVM=y" >> $config_host_mak +fi + # build tree in object directory in case the source is not in the current directory DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests" DIRS="$DIRS fsdev" diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 48a4942..b7a4af0 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -646,7 +646,10 @@ static QEMUMachine pc_machine_v1_1 = { static QEMUMachine pc_machine_v1_0 = { PC_I440FX_1_2_MACHINE_OPTIONS, - .name = "pc-1.0", + .name = "pc-1.0-qemu-git", +#ifndef CONFIG_PC_1_0_QEMU_KVM + .alias = "pc-1.0", +#endif .compat_props = (GlobalProperty[]) { PC_COMPAT_1_0, { /* end of list */ } @@ -665,6 +668,9 @@ static QEMUMachine pc_machine_v1_0 = { static QEMUMachine pc_machine_v1_0_qemu_kvm = { PC_I440FX_1_2_MACHINE_OPTIONS, .name = "pc-1.0-qemu-kvm", +#ifdef CONFIG_PC_1_0_QEMU_KVM + .alias = "pc-1.0", +#endif .init = pc_init_pci_1_2_qemu_kvm, .compat_props = (GlobalProperty[]) { PC_COMPAT_1_0_QEMU_KVM, -- 1.7.9.5
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference. Users also don't get qemu from git so I don't see why does git make sense in the name? Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types? It might make sense to also set -enable-kvm and change default CPU to kvm64 in this case.
--- configure | 12 ++++++++++++ hw/i386/pc_piix.c | 8 +++++++- 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/configure b/configure index f7685b5..b143302 100755 --- a/configure +++ b/configure @@ -335,6 +335,7 @@ libssh2="" vhdx="" quorum="" numa="" +pc_1_0_qemu_kvm="no"
# parse CC options first for opt do @@ -1125,6 +1126,10 @@ for opt do ;; --enable-numa) numa="yes" ;; + --disable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="no" + ;; + --enable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm="yes" + ;; *) echo "ERROR: unknown option $opt" echo "Try '$0 --help' for more information" @@ -1394,6 +1399,8 @@ Advanced options (experts only): --enable-quorum enable quorum block filter support --disable-numa disable libnuma support --enable-numa enable libnuma support + --disable-pc-1-0-qemu-kvm disable pc-1.0 machine type reflecting qemu-kvm + --enable-pc-1-0-qemu-kvm enable pc-1.0 machine type reflecting qemu-kvm
NOTE: The object files are built at the place where configure is launched EOF @@ -4262,6 +4269,7 @@ echo "Quorum $quorum" echo "lzo support $lzo" echo "snappy support $snappy" echo "NUMA host support $numa" +echo "pc-1.0 qemu-kvm $pc_1_0_qemu_kvm"
if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -5241,6 +5249,10 @@ if test "$numa" = "yes"; then echo "CONFIG_NUMA=y" >> $config_host_mak fi
+if test "$pc_1_0_qemu_kvm" = "yes"; then + echo "CONFIG_PC_1_0_QEMU_KVM=y" >> $config_host_mak +fi + # build tree in object directory in case the source is not in the current directory DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests" DIRS="$DIRS fsdev" diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 48a4942..b7a4af0 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -646,7 +646,10 @@ static QEMUMachine pc_machine_v1_1 = {
static QEMUMachine pc_machine_v1_0 = { PC_I440FX_1_2_MACHINE_OPTIONS, - .name = "pc-1.0", + .name = "pc-1.0-qemu-git", +#ifndef CONFIG_PC_1_0_QEMU_KVM + .alias = "pc-1.0", +#endif .compat_props = (GlobalProperty[]) { PC_COMPAT_1_0, { /* end of list */ } @@ -665,6 +668,9 @@ static QEMUMachine pc_machine_v1_0 = { static QEMUMachine pc_machine_v1_0_qemu_kvm = { PC_I440FX_1_2_MACHINE_OPTIONS, .name = "pc-1.0-qemu-kvm", +#ifdef CONFIG_PC_1_0_QEMU_KVM + .alias = "pc-1.0", +#endif .init = pc_init_pci_1_2_qemu_kvm, .compat_props = (GlobalProperty[]) { PC_COMPAT_1_0_QEMU_KVM, -- 1.7.9.5
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Sep 22, 2014 at 12:42:47PM +0100, Daniel P. Berrange wrote:
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host.
Regards, Daniel
Right. However, the fact remains that distros shipped qemu and qemu-kvm that had pc-1.0 meaning different things. What do you think about my idea to detect the meaning for pc-1.0 based on argv[0]?
-- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
"Daniel P. Berrange" <berrange@redhat.com> writes:
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host.
You're right. However, this particular horse left the barn a long time ago: the pc-* machine types differ in qemu-kvm and upstream QEMU. Sure, when qemu-kvm was merged back into QEMU, its machine type variants were dropped. But they live on in various downstreams that just like QEMU had to pick between compatibility with upstream QEMU and qemu-kvm, but unlike QEMU picked compatibility with qemu-kvm. So this patch does *not* break any management apps by letting them "no longer simply compare the machine type name to determine if it is a match with the same name on a different host". They never could for these messed up machine types, at least not without knowing exactly what kind of QEMU runs on the hosts in question. All this patch does is adding another facet to "exactly what kind of QEMU".
On Mon, Sep 22, 2014 at 05:24:55PM +0200, Markus Armbruster wrote:
"Daniel P. Berrange" <berrange@redhat.com> writes:
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host.
You're right. However, this particular horse left the barn a long time ago: the pc-* machine types differ in qemu-kvm and upstream QEMU.
Sure, when qemu-kvm was merged back into QEMU, its machine type variants were dropped. But they live on in various downstreams that just like QEMU had to pick between compatibility with upstream QEMU and qemu-kvm, but unlike QEMU picked compatibility with qemu-kvm.
So this patch does *not* break any management apps by letting them "no longer simply compare the machine type name to determine if it is a match with the same name on a different host". They never could for these messed up machine types, at least not without knowing exactly what kind of QEMU runs on the hosts in question.
All this patch does is adding another facet to "exactly what kind of QEMU".
Right, but IMHO doing it at compile-time is wrong. If distros want compatiblity with both sometimes, what then? Build two binaries with different flags? Should be a runtime option that management sets after (somehow?) figuring out what's going on, on source. How does it do that? Not sure - but I'm sure destination distro has no way to figure it out. -- MST
Quoting Michael S. Tsirkin (mst@redhat.com):
On Mon, Sep 22, 2014 at 05:24:55PM +0200, Markus Armbruster wrote:
"Daniel P. Berrange" <berrange@redhat.com> writes:
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host.
You're right. However, this particular horse left the barn a long time ago: the pc-* machine types differ in qemu-kvm and upstream QEMU.
Sure, when qemu-kvm was merged back into QEMU, its machine type variants were dropped. But they live on in various downstreams that just like QEMU had to pick between compatibility with upstream QEMU and qemu-kvm, but unlike QEMU picked compatibility with qemu-kvm.
So this patch does *not* break any management apps by letting them "no longer simply compare the machine type name to determine if it is a match with the same name on a different host". They never could for these messed up machine types, at least not without knowing exactly what kind of QEMU runs on the hosts in question.
All this patch does is adding another facet to "exactly what kind of QEMU".
Right, but IMHO doing it at compile-time is wrong. If distros want compatiblity with both sometimes, what then? Build two binaries with different flags? Should be a runtime option that management sets after (somehow?) figuring out what's going on, on source.
How does it do that? Not sure - but I'm sure destination distro has no way to figure it out.
It's not even just between distributions. Anyone who used qemu-2.0 to start a pc-1.0 machine type will have a different mt than someone who starts a vm under Ubuntu's (qemu-)kvm 1.0. Sadly. So in the packages at https://launchpad.net/~serge-hallyn/+archive/ubuntu/qemu-p-migration the default can be configured at build-time, but it can be specified on the command-line (which is then controlled by a new libvirt flag). -serge
Il 22/09/2014 17:24, Markus Armbruster ha scritto:
You're right. However, this particular horse left the barn a long time ago: the pc-* machine types differ in qemu-kvm and upstream QEMU.
Sure, when qemu-kvm was merged back into QEMU, its machine type variants were dropped. But they live on in various downstreams that just like QEMU had to pick between compatibility with upstream QEMU and qemu-kvm, but unlike QEMU picked compatibility with qemu-kvm.
And fixing this should have been downstream's job. Fedora did it at the time, RHEL7 did it. Ubuntu didn't, and probably neither did Debian. This patch singles out pc-1.0 just because it used to be the default in Ubuntu 12.04. So basically it's making upstream carry the burden of a decision of the Ubuntu folks. It's understandable that Alex disagrees with the decision, but nevertheless it's not something that upstream should agree with. Also, another horse that has left the barn: it's already too late to apply this patch to upstream Ubuntu. If you do that, any machine created with 12.04 and reused with 14.04 will fail to migrate to another 14.04 machine that includes this patch, as I understand it. So as things stand, I don't see a reason to apply this patch upstream. Paolo
On 22 Sep 2014, at 16:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
time, RHEL7 did it. Ubuntu didn't, and probably neither did Debian.
This patch singles out pc-1.0 just because it used to be the default in Ubuntu 12.04. So basically it's making upstream carry the burden of a decision of the Ubuntu folks. It's understandable that Alex disagrees with the decision, but nevertheless it's not something that upstream should agree with.
Also, another horse that has left the barn: it's already too late to apply this patch to upstream Ubuntu. If you do that, any machine created with 12.04 and reused with 14.04 will fail to migrate to another 14.04 machine that includes this patch, as I understand it.
So as things stand, I don't see a reason to apply this patch upstream.
Well, Ubuntu (Serge I think) said in the Ubuntu bug report he'd be quite willing to break migration of pc-1.0 machine types from 14.04 to 14.04 because that machine type isn't the default anyway on 14.04. But that isn't the point, as the patch (as a whole) doesn't break anything - it merely gives the possibility to import pc-1.0 machines from qemu-kvm. That's useful even if you build qemu from source every time. Or were you just arguing against the ./configure option? -- Alex Bligh
Il 22/09/2014 19:30, Alex Bligh ha scritto:
Well, Ubuntu (Serge I think) said in the Ubuntu bug report he'd be quite willing to break migration of pc-1.0 machine types from 14.04 to 14.04 because that machine type isn't the default anyway on 14.04.
But that isn't the point, as the patch (as a whole) doesn't break anything - it merely gives the possibility to import pc-1.0 machines from qemu-kvm. That's useful even if you build qemu from source every time.
Or were you just arguing against the ./configure option?
I'm arguing against special-casing pc-1.0. Just apply the patch to Ubuntu downstream and call it a day. It's perfectly normal for machine types to be part of the downstream (not so secret) sauce. Paolo
On 22 Sep 2014, at 20:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
I'm arguing against special-casing pc-1.0. Just apply the patch to Ubuntu downstream and call it a day.
It's perfectly normal for machine types to be part of the downstream (not so secret) sauce.
Well, I've just sent through a version that works as a machine parameter instead. If upstream doesn't like this, I'd prefer downstream took v2 of the patch (which makes -M pc-1.0 work) instead. That's also what Serge tested. -- Alex Bligh
Quoting Alex Bligh (alex@alex.org.uk):
On 22 Sep 2014, at 20:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
I'm arguing against special-casing pc-1.0. Just apply the patch to Ubuntu downstream and call it a day.
It's perfectly normal for machine types to be part of the downstream (not so secret) sauce.
Well, I've just sent through a version that works as a machine parameter instead. If upstream doesn't like this, I'd prefer downstream took v2 of the patch (which makes -M pc-1.0 work) instead. That's also what Serge tested.
Yeah, prefer v2 as well. Sorry if that means wasted time on your part. Though I'm also thinking I prefer to have pc-1.0-precise defined in both precise's qemu-kvm 1.0, and later qemu 2.x, and requiring some purely-on-precise action to switch the machine type from pc-1.0 to pc-1.0-precise before migrating to trusty/later. But as I mentioned elsewhere there's still the concerns of (a) how much other still (like virito-net) will still break, and (b) the practical issues of how to ship the old roms. -serge
On 22 Sep 2014, at 12:36, Michael S. Tsirkin <mst@redhat.com> wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
The whole point is to make it easy for distributions to make pc-1.0 do an appropriate thing for them, an appropriate thing being what their previous release(s) meant by pc-1.0. Sadly, pc-1.0 means more than one thing - on Ubuntu it really means "qemu-kvm's pc-1.0" and on other systems (and upstream) it means "qemu-git's pc-1.0". This is horrible, but it's not the fault of this patch, is simply current reality. Right now, you can't tell what pc-1.0 does anyway, as it does a different thing on Ubuntu Precise to Ubuntu Trusty (for instance). If we're going to provide compatibility to the machine type name level (which is the purpose here) then sadly we need something like this. As for 'not being able to predict', actually post this patch the situation is far clearer, as '-M ?' tells you exactly what pc-1.0 will do, by showing what it's an alias for.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Agree with that, but I couldn't think of anything better. 'qemu-upstream'? 'qemu'?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types? It might make sense to also set -enable-kvm a
Sadly that is not true. For instance on Ubuntu Precise it's invoked as qemu-system-x86_64 by at least one management application known to me. I'm not quite sure why you say "legacy management applications". This applies to /any/ management application. Unless we're going to burden every management application with this problem, we need to fix it. Just as a reminder, the ./configure value defaults to off, which means there is no change in current behaviour. Only if the ./configure value is changed does it result in the behaviour of pc-1.0 changing, and that is evident from the help line. And it's possible to specify a specific variant of 1.0 (irrespective of the default) from the command line. -- Alex Bligh
On Mon, Sep 22, 2014 at 12:50:14PM +0100, Alex Bligh wrote:
On 22 Sep 2014, at 12:36, Michael S. Tsirkin <mst@redhat.com> wrote:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
The whole point is to make it easy for distributions to make pc-1.0 do an appropriate thing for them, an appropriate thing being what their previous release(s) meant by pc-1.0. Sadly, pc-1.0 means more than one thing - on Ubuntu it really means "qemu-kvm's pc-1.0" and on other systems (and upstream) it means "qemu-git's pc-1.0". This is horrible, but it's not the fault of this patch, is simply current reality. Right now, you can't tell what pc-1.0 does anyway, as it does a different thing on Ubuntu Precise to Ubuntu Trusty (for instance).
If we're going to provide compatibility to the machine type name level (which is the purpose here) then sadly we need something like this.
Ack for "something" but not like this, I suspect.
As for 'not being able to predict', actually post this patch the situation is far clearer, as '-M ?' tells you exactly what pc-1.0 will do, by showing what it's an alias for.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Agree with that, but I couldn't think of anything better. 'qemu-upstream'? 'qemu'?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types? It might make sense to also set -enable-kvm a
Sadly that is not true. For instance on Ubuntu Precise it's invoked as qemu-system-x86_64 by at least one management application known to me.
Well change it to call qemu-kvm then :) Also what happens if you install qemu as well? Does it conflict?
I'm not quite sure why you say "legacy management applications".
Because any non legacy one can be patched.
This applies to /any/ management application. Unless we're going to burden every management application with this problem, we need to fix it.
Just as a reminder, the ./configure value defaults to off, which means there is no change in current behaviour.
Yes but this still perpetuates the mess. If you prefer using -M pc-1.0, add a new property and teach management to set it. But no silent compile-time behind the scenes changes please.
Only if the ./configure value is changed does it result in the behaviour of pc-1.0 changing, and that is evident from the help line. And it's possible to specify a specific variant of 1.0 (irrespective of the default) from the command line.
-- Alex Bligh
Sadly that is not true. For instance on Ubuntu Precise it's invoked as qemu-system-x86_64 by at least one management application known to me.
Well change it to call qemu-kvm then :) Also what happens if you install qemu as well? Does it conflict?
I'm not an Ubuntu maintainer but AFAIK qemu-kvm is deprecated. It may only be there to support upgrades. We still need to support migration from qemu running on Precise to qemu running on Trusty. The trusty instance may not have qemu-kvm installed. If we were looking at argv[0], we'd really want to look at it on the /sending/ machine. Forcing qemu to be involved as qemu-kvm solely on the basis some users might want to migrate a VM in from a previous version does not sound practical.
I'm not quite sure why you say "legacy management applications".
Because any non legacy one can be patched.
Well this is where we diverge. I think the distro needs a way to change the default behaviour, i.e. so the existing command line will do something different.
This applies to /any/ management application. Unless we're going to burden every management application with this problem, we need to fix it.
Just as a reminder, the ./configure value defaults to off, which means there is no change in current behaviour.
Yes but this still perpetuates the mess.
If you prefer using -M pc-1.0, add a new property and teach management to set it.
But no silent compile-time behind the scenes changes please.
OK, how about we keep the aliases, and make pc-1.0 default to the pc-1.0-qemu-git. We then add a command line option to make pc-1.0 mean pc-1.0-qemu-kvm, with that obviously defaulting to off. Then distros can then put the option in /etc/qemu/target-x86_64.conf or whatever. -- Alex Bligh
Am 22.09.2014 um 15:05 schrieb Alex Bligh:
Sadly that is not true. For instance on Ubuntu Precise it's invoked as qemu-system-x86_64 by at least one management application known to me.
Well change it to call qemu-kvm then :) Also what happens if you install qemu as well? Does it conflict?
I'm not an Ubuntu maintainer but AFAIK qemu-kvm is deprecated. It may only be there to support upgrades.
We still need to support migration from qemu running on Precise to qemu running on Trusty. The trusty instance may not have qemu-kvm installed. If we were looking at argv[0], we'd really want to look at it on the /sending/ machine.
Forcing qemu to be involved as qemu-kvm solely on the basis some users might want to migrate a VM in from a previous version does not sound practical.
I'm not quite sure why you say "legacy management applications".
Because any non legacy one can be patched.
Well this is where we diverge. I think the distro needs a way to change the default behaviour, i.e. so the existing command line will do something different.
This applies to /any/ management application. Unless we're going to burden every management application with this problem, we need to fix it.
Just as a reminder, the ./configure value defaults to off, which means there is no change in current behaviour.
Yes but this still perpetuates the mess.
If you prefer using -M pc-1.0, add a new property and teach management to set it.
But no silent compile-time behind the scenes changes please.
OK, how about we keep the aliases, and make pc-1.0 default to the pc-1.0-qemu-git. We then add a command line option to make pc-1.0 mean pc-1.0-qemu-kvm, with that obviously defaulting to off.
Then distros can then put the option in /etc/qemu/target-x86_64.conf or whatever.
What about adding a bool property "qemu-kvm-compat" to the MachineClass? Then a qemu-kvm shell script (like SUSE uses) can pass -global machine.qemu-kvm-compat=on whereas qemu-system-x86_64 would run in the default non-qemu-kvm mode (config on disk would affect both). It would also allow running -machine pc-0.15,qemu-kvm-compat=on, ditching lots of new machine names and avoiding the name bikeshedding. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On 22 Sep 2014, at 16:45, Andreas Färber <afaerber@suse.de> wrote:
What about adding a bool property "qemu-kvm-compat" to the MachineClass? Then a qemu-kvm shell script (like SUSE uses) can pass -global machine.qemu-kvm-compat=on whereas qemu-system-x86_64 would run in the default non-qemu-kvm mode (config on disk would affect both). It would also allow running -machine pc-0.15,qemu-kvm-compat=on, ditching lots of new machine names and avoiding the name bikeshedding.
I'd be happy with that. Presumably downstream can then patch things so qemu-kvm-compat defaults in the way they want (if we don't like the configure option). However, that's not compatible with using PC_COMPAT as far as I know (unless there is some cunning way you can make a machine parameter change compat_props things). -- Alex Bligh
Alex Bligh <alex@alex.org.uk> writes:
On 22 Sep 2014, at 16:45, Andreas Färber <afaerber@suse.de> wrote:
What about adding a bool property "qemu-kvm-compat" to the MachineClass? Then a qemu-kvm shell script (like SUSE uses) can pass -global machine.qemu-kvm-compat=on whereas qemu-system-x86_64 would run in the
No need to mess with -global, just use -machine qemu-kvm-compat=off. You can have multiple -machine, and the last key=value wins.
default non-qemu-kvm mode (config on disk would affect both). It would also allow running -machine pc-0.15,qemu-kvm-compat=on, ditching lots of new machine names and avoiding the name bikeshedding.
I'd be happy with that. Presumably downstream can then patch things so qemu-kvm-compat defaults in the way they want (if we don't like the configure option).
However, that's not compatible with using PC_COMPAT as far as I know (unless there is some cunning way you can make a machine parameter change compat_props things).
Monkey-patching MACHINE_GET_CLASS(machine)->compat_props? Gross, but I don't have better ideas to offer.
On Mon, Sep 22, 2014 at 05:54:02PM +0100, Alex Bligh wrote:
However, that's not compatible with using PC_COMPAT as far as I know (unless there is some cunning way you can make a machine parameter change compat_props things).
Of course not, PC_COMPAT is the reverse: have machine type influence properties.
On Mon, Sep 22, 2014 at 05:45:28PM +0200, Andreas Färber wrote:
Am 22.09.2014 um 15:05 schrieb Alex Bligh:
Sadly that is not true. For instance on Ubuntu Precise it's invoked as qemu-system-x86_64 by at least one management application known to me.
Well change it to call qemu-kvm then :) Also what happens if you install qemu as well? Does it conflict?
I'm not an Ubuntu maintainer but AFAIK qemu-kvm is deprecated. It may only be there to support upgrades.
We still need to support migration from qemu running on Precise to qemu running on Trusty. The trusty instance may not have qemu-kvm installed. If we were looking at argv[0], we'd really want to look at it on the /sending/ machine.
Forcing qemu to be involved as qemu-kvm solely on the basis some users might want to migrate a VM in from a previous version does not sound practical.
I'm not quite sure why you say "legacy management applications".
Because any non legacy one can be patched.
Well this is where we diverge. I think the distro needs a way to change the default behaviour, i.e. so the existing command line will do something different.
This applies to /any/ management application. Unless we're going to burden every management application with this problem, we need to fix it.
Just as a reminder, the ./configure value defaults to off, which means there is no change in current behaviour.
Yes but this still perpetuates the mess.
If you prefer using -M pc-1.0, add a new property and teach management to set it.
But no silent compile-time behind the scenes changes please.
OK, how about we keep the aliases, and make pc-1.0 default to the pc-1.0-qemu-git. We then add a command line option to make pc-1.0 mean pc-1.0-qemu-kvm, with that obviously defaulting to off.
Then distros can then put the option in /etc/qemu/target-x86_64.conf or whatever.
What about adding a bool property "qemu-kvm-compat" to the MachineClass? Then a qemu-kvm shell script (like SUSE uses) can pass -global machine.qemu-kvm-compat=on whereas qemu-system-x86_64 would run in the default non-qemu-kvm mode (config on disk would affect both). It would also allow running -machine pc-0.15,qemu-kvm-compat=on, ditching lots of new machine names and avoiding the name bikeshedding.
Regards, Andreas
Ack. Exactly.
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types?
Ugh! I like that even less than a configure option.
It might make sense to also set -enable-kvm and change default CPU to kvm64 in this case.
Yup.
On Mon, Sep 22, 2014 at 05:32:16PM +0200, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types?
Ugh! I like that even less than a configure option.
configure options are tested even less than runtime ones: each distro has to pick up a set of options and all users are stuck with it. So let's assume Ubuntu sets this and something is broken. How is a Fedora user to test this? Find out configure flags that Ubuntu uses and rebuild from source? It's hard to get people to triage bugs as it is. That's why changing behaviour in subtle ways depending on configure options is a very bad idea. So a flag might be better, but if we want to be compatible with qemu-kvm, poking at argv[0] still better than configure.
It might make sense to also set -enable-kvm and change default CPU to kvm64 in this case.
Yup.
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Mon, Sep 22, 2014 at 05:32:16PM +0200, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types?
Ugh! I like that even less than a configure option.
configure options are tested even less than runtime ones: each distro has to pick up a set of options and all users are stuck with it. So let's assume Ubuntu sets this and something is broken. How is a Fedora user to test this? Find out configure flags that Ubuntu uses and rebuild from source? It's hard to get people to triage bugs as it is. That's why changing behaviour in subtle ways depending on configure options is a very bad idea.
All it changes is a default machine type. I wouldn't classify that as "very bad". Fortunately, our difference of opinion doesn't matter, because the conclusion of the thread is "leave it to the distros".
So a flag might be better, but if we want to be compatible with qemu-kvm, poking at argv[0] still better than configure.
That I do consider a genuinely bad idea. Example of usage messed up by it: I symlink from my ~/bin to executables in various build trees. If one of these programs changed behavior depending on what it finds in argv[0], I'd have to know *exactly* how it matches argv[0] to choose suitable names for my symlinks. The name "qemu-kvm" is not a useful indicator of compatibility with the qemu-kvm fork of QEMU anyway. There are programs out there calling themselves qemu-kvm that aren't compatible with the qemu-kvm fork. And there are programs out there that are compatible, but call themselves something else.
On Tue, Sep 23, 2014 at 09:59:17AM +0200, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Mon, Sep 22, 2014 at 05:32:16PM +0200, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference.
Users also don't get qemu from git so I don't see why does git make sense in the name?
Legacy management applications invoked qemu as qemu-kvm - how about detecting that name and switching the machine types?
Ugh! I like that even less than a configure option.
configure options are tested even less than runtime ones: each distro has to pick up a set of options and all users are stuck with it. So let's assume Ubuntu sets this and something is broken. How is a Fedora user to test this? Find out configure flags that Ubuntu uses and rebuild from source? It's hard to get people to triage bugs as it is. That's why changing behaviour in subtle ways depending on configure options is a very bad idea.
All it changes is a default machine type. I wouldn't classify that as "very bad". Fortunately, our difference of opinion doesn't matter, because the conclusion of the thread is "leave it to the distros".
So a flag might be better, but if we want to be compatible with qemu-kvm, poking at argv[0] still better than configure.
That I do consider a genuinely bad idea.
Example of usage messed up by it: I symlink from my ~/bin to executables in various build trees. If one of these programs changed behavior depending on what it finds in argv[0], I'd have to know *exactly* how it matches argv[0] to choose suitable names for my symlinks.
The name "qemu-kvm" is not a useful indicator of compatibility with the qemu-kvm fork of QEMU anyway. There are programs out there calling themselves qemu-kvm that aren't compatible with the qemu-kvm fork. And there are programs out there that are compatible, but call themselves something else.
I think the approach v4 takes is reasonable, though. I didn't look at the implementation yet. -- MST
participants (7)
-
Alex Bligh -
Andreas Färber -
Daniel P. Berrange -
Markus Armbruster -
Michael S. Tsirkin -
Paolo Bonzini -
Serge Hallyn