Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/arm/aspeed.c | 2 +-
hw/arm/collie.c | 2 +-
hw/arm/cubieboard.c | 2 +-
hw/arm/digic_boards.c | 7 ++++---
hw/arm/highbank.c | 3 ++-
hw/arm/imx25_pdk.c | 2 +-
hw/arm/integratorcp.c | 2 +-
hw/arm/kzm.c | 2 +-
hw/arm/mcimx6ul-evk.c | 2 +-
hw/arm/mcimx7d-sabre.c | 2 +-
hw/arm/mps2-tz.c | 4 ++--
hw/arm/mps2.c | 4 ++--
hw/arm/musicpal.c | 2 +-
hw/arm/nseries.c | 2 +-
hw/arm/omap_sx1.c | 2 +-
hw/arm/palm.c | 2 +-
hw/arm/raspi.c | 2 +-
hw/arm/sabrelite.c | 2 +-
hw/arm/sbsa-ref.c | 2 +-
hw/arm/versatilepb.c | 2 +-
hw/arm/vexpress.c | 4 ++--
hw/arm/virt.c | 2 +-
hw/arm/xilinx_zynq.c | 2 +-
hw/arm/xlnx-versal-virt.c | 2 +-
hw/arm/xlnx-zcu102.c | 2 +-
25 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 52993f84b4..f2a52e1ade 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -212,7 +212,7 @@ static void aspeed_board_init(MachineState *machine,
ram_size = object_property_get_uint(OBJECT(&bmc->soc), "ram-size",
&error_abort);
- memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
+ memory_region_allocate_system_memory(&bmc->ram, machine, "ram", ram_size);
memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
memory_region_add_subregion(get_system_memory(),
sc->memmap[ASPEED_SDRAM],
diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 970a4405cc..632531a88d 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -31,7 +31,7 @@ static void collie_init(MachineState *machine)
s = sa1110_init(machine->cpu_type);
- memory_region_allocate_system_memory(sdram, NULL, "strongarm.sdram",
+ memory_region_allocate_system_memory(sdram, machine, "strongarm.sdram",
collie_binfo.ram_size);
memory_region_add_subregion(get_system_memory(), SA_SDCS0, sdram);
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 6dc2f1d6b6..b76ad7ef69 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -65,7 +65,7 @@ static void cubieboard_init(MachineState *machine)
exit(1);
}
- memory_region_allocate_system_memory(&s->sdram, NULL, "cubieboard.ram",
+ memory_region_allocate_system_memory(&s->sdram, machine, "cubieboard.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
&s->sdram);
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index ef3fc2b6a5..f3d1febe87 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -53,7 +53,7 @@ typedef struct DigicBoard {
const char *rom1_def_filename;
} DigicBoard;
-static void digic4_board_init(DigicBoard *board)
+static void digic4_board_init(MachineState *machine, DigicBoard *board)
{
Error *err = NULL;
@@ -66,7 +66,8 @@ static void digic4_board_init(DigicBoard *board)
exit(1);
}
- memory_region_allocate_system_memory(&s->ram, NULL, "ram", board->ram_size);
+ memory_region_allocate_system_memory(&s->ram, machine, "ram",
+ board->ram_size);
memory_region_add_subregion(get_system_memory(), 0, &s->ram);
if (board->add_rom0) {
@@ -142,7 +143,7 @@ static DigicBoard digic4_board_canon_a1100 = {
static void canon_a1100_init(MachineState *machine)
{
- digic4_board_init(&digic4_board_canon_a1100);
+ digic4_board_init(machine, &digic4_board_canon_a1100);
}
static void canon_a1100_machine_init(MachineClass *mc)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index f1724d6929..57e549ec00 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -290,7 +290,8 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
sysmem = get_system_memory();
dram = g_new(MemoryRegion, 1);
- memory_region_allocate_system_memory(dram, NULL, "highbank.dram", ram_size);
+ memory_region_allocate_system_memory(dram, machine, "highbank.dram",
+ ram_size);
/* SDRAM at address zero. */
memory_region_add_subregion(sysmem, 0, dram);
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index c76fc2bd94..e88d325e10 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -84,7 +84,7 @@ static void imx25_pdk_init(MachineState *machine)
machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
}
- memory_region_allocate_system_memory(&s->ram, NULL, "imx25.ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "imx25.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(), FSL_IMX25_SDRAM0_ADDR,
&s->ram);
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 524970840d..a8053be396 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -605,7 +605,7 @@ static void integratorcp_init(MachineState *machine)
cpu = ARM_CPU(cpuobj);
- memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
+ memory_region_allocate_system_memory(ram, machine, "integrator.ram",
ram_size);
/* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */
/* ??? RAM should repeat to fill physical memory space. */
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 1d5ef289d5..e81bd8e0dc 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -84,7 +84,7 @@ static void kzm_init(MachineState *machine)
machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
}
- memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "kzm.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(), FSL_IMX31_SDRAM0_ADDR,
&s->ram);
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index e90b393a44..95653db599 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -48,7 +48,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
- memory_region_allocate_system_memory(&s->ram, NULL, "mcimx6ul-evk.ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "mcimx6ul-evk.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(),
FSL_IMX6UL_MMDC_ADDR, &s->ram);
diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 0d1f62d30a..d535a119f7 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -50,7 +50,7 @@ static void mcimx7d_sabre_init(MachineState *machine)
TYPE_FSL_IMX7, &error_fatal, NULL);
object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
- memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "mcimx7d-sabre.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(),
FSL_IMX7_MMDC_ADDR, &s->ram);
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index f8b620bcc6..96fe815361 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -458,8 +458,8 @@ static void mps2tz_common_init(MachineState *machine)
* tradeoffs. For QEMU they're all just RAM, though. We arbitrarily
* call the 16MB our "system memory", as it's the largest lump.
*/
- memory_region_allocate_system_memory(&mms->psram,
- NULL, "mps.ram", 16 * MiB);
+ memory_region_allocate_system_memory(&mms->psram, machine,
+ "mps.ram", 16 * MiB);
memory_region_add_subregion(system_memory, 0x80000000, &mms->psram);
/* The overflow IRQs for all UARTs are ORed together.
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index d002b126d3..4687bc7ed1 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -146,8 +146,8 @@ static void mps2_common_init(MachineState *machine)
* This is of no use for QEMU so we don't implement it (as if
* zbt_boot_ctrl is always zero).
*/
- memory_region_allocate_system_memory(&mms->psram,
- NULL, "mps.ram", 16 * MiB);
+ memory_region_allocate_system_memory(&mms->psram, machine,
+ "mps.ram", 16 * MiB);
memory_region_add_subregion(system_memory, 0x21000000, &mms->psram);
switch (mmc->fpga_type) {
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index f68a399a98..2a2c7b3abf 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1596,7 +1596,7 @@ static void musicpal_init(MachineState *machine)
cpu = ARM_CPU(cpu_create(machine->cpu_type));
/* For now we use a fixed - the original - RAM size */
- memory_region_allocate_system_memory(ram, NULL, "musicpal.ram",
+ memory_region_allocate_system_memory(ram, machine, "musicpal.ram",
MP_RAM_DEFAULT_SIZE);
memory_region_add_subregion(address_space_mem, 0, ram);
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 11f2c193f3..8ab30fde24 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1315,7 +1315,7 @@ static void n8x0_init(MachineState *machine,
struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
uint64_t sdram_size = binfo->ram_size;
- memory_region_allocate_system_memory(sdram, NULL, "omap2.dram",
+ memory_region_allocate_system_memory(sdram, machine, "omap2.dram",
sdram_size);
memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, sdram);
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index be245714db..26f4e1834b 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -119,7 +119,7 @@ static void sx1_init(MachineState *machine, const int version)
flash_size = flash2_size;
}
- memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+ memory_region_allocate_system_memory(dram, machine, "omap1.dram",
sx1_binfo.ram_size);
memory_region_add_subregion(address_space, OMAP_EMIFF_BASE, dram);
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 72eca8cc55..dceb4ab6a8 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -199,7 +199,7 @@ static void palmte_init(MachineState *machine)
MemoryRegion *flash = g_new(MemoryRegion, 1);
MemoryRegion *cs = g_new(MemoryRegion, 4);
- memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+ memory_region_allocate_system_memory(dram, machine, "omap1.dram",
palmte_binfo.ram_size);
memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram);
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index f76b6eaad3..6f1520bad6 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -182,7 +182,7 @@ static void raspi_init(MachineState *machine, int version)
&error_abort, NULL);
/* Allocate and map RAM */
- memory_region_allocate_system_memory(&s->ram, NULL, "ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "ram",
machine->ram_size);
/* FIXME: Remove when we have custom CPU address space support */
memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index 96cc455c5c..dfeffcb2a7 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -64,7 +64,7 @@ static void sabrelite_init(MachineState *machine)
exit(1);
}
- memory_region_allocate_system_memory(&s->ram, NULL, "sabrelite.ram",
+ memory_region_allocate_system_memory(&s->ram, machine, "sabrelite.ram",
machine->ram_size);
memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR,
&s->ram);
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 27046cc284..1d4db8d7fd 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -687,7 +687,7 @@ static void sbsa_ref_init(MachineState *machine)
object_unref(cpuobj);
}
- memory_region_allocate_system_memory(ram, NULL, "sbsa-ref.ram",
+ memory_region_allocate_system_memory(ram, machine, "sbsa-ref.ram",
machine->ram_size);
memory_region_add_subregion(sysmem, sbsa_ref_memmap[SBSA_MEM].base, ram);
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index e86af01537..5cde8e3677 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -220,7 +220,7 @@ static void versatile_init(MachineState *machine, int board_id)
cpu = ARM_CPU(cpuobj);
- memory_region_allocate_system_memory(ram, NULL, "versatile.ram",
+ memory_region_allocate_system_memory(ram, machine, "versatile.ram",
machine->ram_size);
/* ??? RAM should repeat to fill physical memory space. */
/* SDRAM at address zero. */
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 4673a88a8d..b97770cc68 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -283,7 +283,7 @@ static void a9_daughterboard_init(const VexpressMachineState *vms,
exit(1);
}
- memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+ memory_region_allocate_system_memory(ram, machine, "vexpress.highmem",
ram_size);
low_ram_size = ram_size;
if (low_ram_size > 0x4000000) {
@@ -375,7 +375,7 @@ static void a15_daughterboard_init(const VexpressMachineState *vms,
}
}
- memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+ memory_region_allocate_system_memory(ram, machine, "vexpress.highmem",
ram_size);
/* RAM is from 0x80000000 upwards; there is no low-memory alias for it. */
memory_region_add_subregion(sysmem, 0x80000000, ram);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d4bedc2607..917d5549c1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1702,7 +1702,7 @@ static void machvirt_init(MachineState *machine)
}
}
- memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
+ memory_region_allocate_system_memory(ram, machine, "mach-virt.ram",
machine->ram_size);
memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram);
if (machine->device_memory) {
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3a0fa5b23f..abb3162cfc 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -190,7 +190,7 @@ static void zynq_init(MachineState *machine)
}
/* DDR remapped to address zero. */
- memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
+ memory_region_allocate_system_memory(ext_ram, machine, "zynq.ext_ram",
ram_size);
memory_region_add_subregion(address_space_mem, 0, ext_ram);
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 462493c467..565a52bdab 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -414,7 +414,7 @@ static void versal_virt_init(MachineState *machine)
psci_conduit = QEMU_PSCI_CONDUIT_SMC;
}
- memory_region_allocate_system_memory(&s->mr_ddr, NULL, "ddr",
+ memory_region_allocate_system_memory(&s->mr_ddr, machine, "ddr",
machine->ram_size);
sysbus_init_child_obj(OBJECT(machine), "xlnx-ve", &s->soc,
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 53cfe7c1f1..0de41adb75 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -87,7 +87,7 @@ static void xlnx_zcu102_init(MachineState *machine)
ram_size);
}
- memory_region_allocate_system_memory(&s->ddr_ram, NULL, "ddr-ram",
+ memory_region_allocate_system_memory(&s->ddr_ram, machine, "ddr-ram",
ram_size);
object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
--
2.21.0
On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/arm/virt.c | 2 +- I think from a quick code scan that this is ok, but did you test that migration compat from old to new still works? I vaguely recall that there are some cases when adding an owner to a memory region changes the name string used for identifying the ramblock in migration. thanks -- PMM
On 10/21/19 11:22 AM, Peter Maydell wrote: > On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- > >> hw/arm/virt.c | 2 +- > > I think from a quick code scan that this is ok, but did > you test that migration compat from old to new still works? > I vaguely recall that there are some cases when adding an > owner to a memory region changes the name string used for > identifying the ramblock in migration. It seems to still works: $ make check-qtest-aarch64 V=1 QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 QTEST_QEMU_IMG=qemu-img tests/migration-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="migration-test" PASS 1 migration-test /aarch64/migration/deprecated PASS 2 migration-test /aarch64/migration/bad_dest PASS 3 migration-test /aarch64/migration/fd_proto PASS 4 migration-test /aarch64/migration/validate_uuid PASS 5 migration-test /aarch64/migration/validate_uuid_error PASS 6 migration-test /aarch64/migration/validate_uuid_src_not_set PASS 7 migration-test /aarch64/migration/validate_uuid_dst_not_set PASS 8 migration-test /aarch64/migration/auto_converge PASS 9 migration-test /aarch64/migration/postcopy/unix PASS 10 migration-test /aarch64/migration/postcopy/recovery PASS 11 migration-test /aarch64/migration/precopy/unix PASS 12 migration-test /aarch64/migration/precopy/tcp PASS 13 migration-test /aarch64/migration/xbzrle/unix This test migrate the virt machine. Is this enough? Regards, Phil.
On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > On 10/21/19 11:22 AM, Peter Maydell wrote: > > On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> --- > > > >> hw/arm/virt.c | 2 +- > > > > I think from a quick code scan that this is ok, but did > > you test that migration compat from old to new still works? > > I vaguely recall that there are some cases when adding an > > owner to a memory region changes the name string used for > > identifying the ramblock in migration. > > It seems to still works: > > $ make check-qtest-aarch64 V=1 > This test migrate the virt machine. > > Is this enough? No, you need to test migration from a QEMU binary without this patchset to a QEMU binary that has it. Otherwise you're only checking that the new version can migrate from itself to itself. I find the easiest way to test this is just to use the 'savevm' command to save a state snapshot to a qcow2 disk image while running the old binary, and then run 'loadvm' with the new binary and check it restored OK. thanks -- PMM
Cc'ing Paolo/David. On 10/21/19 11:39 AM, Peter Maydell wrote: > On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> >> On 10/21/19 11:22 AM, Peter Maydell wrote: >>> On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>>> --- >>> >>>> hw/arm/virt.c | 2 +- >>> >>> I think from a quick code scan that this is ok, but did >>> you test that migration compat from old to new still works? >>> I vaguely recall that there are some cases when adding an >>> owner to a memory region changes the name string used for >>> identifying the ramblock in migration. >> >> It seems to still works: >> >> $ make check-qtest-aarch64 V=1 > >> This test migrate the virt machine. >> >> Is this enough? > > No, you need to test migration from a QEMU binary without > this patchset to a QEMU binary that has it. Otherwise you're > only checking that the new version can migrate from itself > to itself. I find the easiest way to test this is just to > use the 'savevm' command to save a state snapshot to a > qcow2 disk image while running the old binary, and then run > 'loadvm' with the new binary and check it restored OK. I did not think if this case. I followed your blog post [*] and tested the migration works OK. Paolo, now thinking about regular testing, we should add this testing to patchew too. Something like: - when mainstream/master is updated, patchew build QEMU (it should be already mostly ccached) and generate some vm dumps with 'savevm'. - build/test the series - if series succeeded testing, run 'loadvm' tests [*] https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote: > Cc'ing Paolo/David. > > On 10/21/19 11:39 AM, Peter Maydell wrote: > > On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > > > On 10/21/19 11:22 AM, Peter Maydell wrote: > > > > On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > > > > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > > > --- > > > > > > > > > hw/arm/virt.c | 2 +- > > > > > > > > I think from a quick code scan that this is ok, but did > > > > you test that migration compat from old to new still works? > > > > I vaguely recall that there are some cases when adding an > > > > owner to a memory region changes the name string used for > > > > identifying the ramblock in migration. > > > > > > It seems to still works: > > > > > > $ make check-qtest-aarch64 V=1 > > > > > This test migrate the virt machine. > > > > > > Is this enough? > > > > No, you need to test migration from a QEMU binary without > > this patchset to a QEMU binary that has it. Otherwise you're > > only checking that the new version can migrate from itself > > to itself. I find the easiest way to test this is just to > > use the 'savevm' command to save a state snapshot to a > > qcow2 disk image while running the old binary, and then run > > 'loadvm' with the new binary and check it restored OK. > > I did not think if this case. > > I followed your blog post [*] and tested the migration works OK. > > Paolo, now thinking about regular testing, we should add this testing to > patchew too. Something like: > > - when mainstream/master is updated, patchew build QEMU (it should be > already mostly ccached) and generate some vm dumps with 'savevm'. > > - build/test the series > > - if series succeeded testing, run 'loadvm' tests > > [*] https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/ Avocado certainly already has an option for specifying source and destination qemu separately; I've used that for testing cross version in the past. The challenge is finding a command line/set of devices for each architecture that's expected to be stable. You want a command line with as big a set of devices as possible (for coverage) yet really is tied to machine type. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Cc'ing avocado-devel for test idea. On 10/21/19 4:57 PM, Dr. David Alan Gilbert wrote: > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote: >> Cc'ing Paolo/David. >> >> On 10/21/19 11:39 AM, Peter Maydell wrote: >>> On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >>>> >>>> On 10/21/19 11:22 AM, Peter Maydell wrote: >>>>> On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >>>>>> >>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>>>>> --- >>>>> >>>>>> hw/arm/virt.c | 2 +- >>>>> >>>>> I think from a quick code scan that this is ok, but did >>>>> you test that migration compat from old to new still works? >>>>> I vaguely recall that there are some cases when adding an >>>>> owner to a memory region changes the name string used for >>>>> identifying the ramblock in migration. >>>> >>>> It seems to still works: >>>> >>>> $ make check-qtest-aarch64 V=1 >>> >>>> This test migrate the virt machine. >>>> >>>> Is this enough? >>> >>> No, you need to test migration from a QEMU binary without >>> this patchset to a QEMU binary that has it. Otherwise you're >>> only checking that the new version can migrate from itself >>> to itself. I find the easiest way to test this is just to >>> use the 'savevm' command to save a state snapshot to a >>> qcow2 disk image while running the old binary, and then run >>> 'loadvm' with the new binary and check it restored OK. >> >> I did not think if this case. >> >> I followed your blog post [*] and tested the migration works OK. >> >> Paolo, now thinking about regular testing, we should add this testing to >> patchew too. Something like: >> >> - when mainstream/master is updated, patchew build QEMU (it should be >> already mostly ccached) and generate some vm dumps with 'savevm'. >> >> - build/test the series >> >> - if series succeeded testing, run 'loadvm' tests >> >> [*] https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/ > > Avocado certainly already has an option for specifying source and > destination qemu separately; I've used that for testing > cross version in the past. > > The challenge is finding a command line/set of devices for each > architecture that's expected to be stable. > You want a command line with as big a set of devices as possible (for > coverage) yet really is tied to machine type. > > Dave > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > _______________________________________________ Patchew-devel mailing list Patchew-devel@redhat.com https://www.redhat.com/mailman/listinfo/patchew-devel
© 2016 - 2026 Red Hat, Inc.