From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47815) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTIf2-0003K7-Ak for qemu-devel@nongnu.org; Mon, 07 Oct 2013 17:48:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTIex-0005uZ-3f for qemu-devel@nongnu.org; Mon, 07 Oct 2013 17:48:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47643) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTIew-0005uP-Rm for qemu-devel@nongnu.org; Mon, 07 Oct 2013 17:48:11 -0400 Message-ID: <52532C14.7090007@redhat.com> Date: Mon, 07 Oct 2013 17:48:04 -0400 From: Cole Robinson MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Fix pc migration from qemu <= 1.5 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bandan Das Cc: david@gibson.dropbear.id.au, qemu-devel@nongnu.org, Eduardo Habkost , mst@redhat.com On 10/07/2013 05:43 PM, Bandan Das wrote: > Hi Cole, > > Cole Robinson writes: > >> The following commit introduced a migration incompatibility: >> >> commit 568f0690fd9aa4d39d84b04c1a5dbb53a915c3fe >> Author: David Gibson >> Date: Thu Jun 6 18:48:49 2013 +1000 >> >> pci: Replace pci_find_domain() with more general pci_root_bus_path() >> >> The issue is that i440fx savevm idstr went from 0000:00:00.0/I440FX to >> 0000:00.0/I440FX. Unfortunately we are stuck with the breakage for >> 1.6 machine types. >> >> Add a compat property to maintain the busted idstr for the 1.6 machine >> types, but revert to the old style format for 1.7+, and <= 1.5. >> >> Tested with migration from qemu 1.5, qemu 1.6, and qemu.git. >> >> Signed-off-by: Cole Robinson >> --- >> hw/i386/pc_piix.c | 4 ++++ >> hw/i386/pc_q35.c | 4 ++++ >> hw/pci-host/piix.c | 9 ++++++++- >> hw/pci-host/q35.c | 10 ++++++++-- >> include/hw/i386/pc.h | 28 ++++++++++++++++++++++++++++ >> include/hw/pci-host/q35.h | 1 + >> 6 files changed, 53 insertions(+), 3 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index c6042c7..90f1ea4 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -346,6 +346,10 @@ static QEMUMachine pc_i440fx_machine_v1_7 = { >> .alias = "pc", >> .init = pc_init_pci, >> .is_default = 1, >> + .compat_props = (GlobalProperty[]) { >> + PC_COMPAT_1_7, >> + { /* end of list */ } >> + }, >> }; >> #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index ca84e1c..569f946 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -270,6 +270,10 @@ static QEMUMachine pc_q35_machine_v1_7 = { >> .name = "pc-q35-1.7", >> .alias = "q35", >> .init = pc_q35_init, >> + .compat_props = (GlobalProperty[]) { >> + PC_COMPAT_1_7, >> + { /* end of list */ } >> + }, >> }; >> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS >> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c >> index c041149..9dafe80 100644 >> --- a/hw/pci-host/piix.c >> +++ b/hw/pci-host/piix.c >> @@ -48,6 +48,7 @@ typedef struct I440FXState { >> PCIHostState parent_obj; >> PcPciInfo pci_info; >> uint64_t pci_hole64_size; >> + uint32_t short_root_bus; >> } I440FXState; >> >> #define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */ >> @@ -712,13 +713,19 @@ static const TypeInfo i440fx_info = { >> static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge, >> PCIBus *rootbus) >> { >> + I440FXState *s = I440FX_PCI_HOST_BRIDGE(host_bridge); >> + >> /* For backwards compat with old device paths */ >> - return "0000"; >> + if (s->short_root_bus) { >> + return "0000"; >> + } >> + return "0000:00"; >> } >> >> static Property i440fx_props[] = { >> DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState, >> pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE), >> + DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c >> index ad703a4..cb3abfd 100644 >> --- a/hw/pci-host/q35.c >> +++ b/hw/pci-host/q35.c >> @@ -61,8 +61,13 @@ static void q35_host_realize(DeviceState *dev, Error **errp) >> static const char *q35_host_root_bus_path(PCIHostState *host_bridge, >> PCIBus *rootbus) >> { >> - /* For backwards compat with old device paths */ >> - return "0000"; >> + Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge); >> + >> + /* For backwards compat with old device paths */ >> + if (s->mch.short_root_bus) { >> + return "0000"; >> + } >> + return "0000:00"; >> } >> >> static void q35_host_get_pci_hole_start(Object *obj, Visitor *v, >> @@ -114,6 +119,7 @@ static Property mch_props[] = { >> MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT), >> DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost, >> mch.pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE), >> + DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 9b2ddc4..f9266f0 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -225,11 +225,31 @@ void pvpanic_init(ISABus *bus); >> >> int e820_add_entry(uint64_t, uint64_t, uint32_t); >> >> +#define PC_COMPAT_1_7 \ >> + {\ >> + .driver = "i440FX-pcihost",\ >> + .property = "short_root_bus",\ >> + .value = stringify(0),\ >> + },{\ >> + .driver = "mch",\ >> + .property = "short_root_bus",\ >> + .value = stringify(0),\ >> + } >> + > > This is probably not needed since the default value of > short_root_bus is already 0. BTW, 1_7 shouldn't have > compat props since it's already the most uptodate > machine type and by definition, compat properties only apply > to older machine types. > Yeah I got confused here, I'll fix and resend. - Cole