From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ud18v-00070O-Cm for qemu-devel@nongnu.org; Thu, 16 May 2013 12:35:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ud18p-0000H2-Fm for qemu-devel@nongnu.org; Thu, 16 May 2013 12:35:01 -0400 Received: from ns232118.ovh.net ([178.33.234.66]:53150) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ud18p-0000Gw-5w for qemu-devel@nongnu.org; Thu, 16 May 2013 12:34:55 -0400 Message-ID: <51950AAC.4070305@greensocs.com> Date: Thu, 16 May 2013 18:34:52 +0200 From: =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= MIME-Version: 1.0 References: <1368662027-14213-1-git-send-email-aliguori@us.ibm.com> <20130516142151.GC23880@vm> <5194F279.30200@redhat.com> <51950139.5000209@greensocs.com> <87li7ezxxt.fsf@codemonkey.ws> In-Reply-To: <87li7ezxxt.fsf@codemonkey.ws> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [ANNOUNCE] QEMU 1.5.0-rc2 is now available List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Paolo Bonzini , mdroth , qemu-devel@nongnu.org On 16/05/2013 18:33, Anthony Liguori wrote: > KONRAD Fr=C3=A9d=C3=A9ric writes: > >> On 16/05/2013 16:51, Paolo Bonzini wrote: >>> Il 16/05/2013 16:21, mdroth ha scritto: >>>> commit e37da3945fa2fde161e1b217f937fc318c4b7639 >>>> Author: KONRAD Frederic >>>> Date: Thu Apr 11 16:29:58 2013 +0200 >>>> >>>> virtio-net-pci: switch to the new API. >>>> =20 >>>> Here the virtio-net-pci is modified for the new API. The devic= e >>>> virtio-net-pci extends virtio-pci. It creates and connects a >>>> virtio-net-device during the init. The properties are not chan= ged. >>>> =20 >>>> Signed-off-by: KONRAD Frederic >>>> Tested-by: Cornelia Huck >>>> Message-id: 1365690602-22729-4-git-send-email-fred.konrad@gree= nsocs.com >>>> Signed-off-by: Anthony Liguori >>>> >>>> And if we roll that back, we have similar failures for virtio-blk, a= nd most >>>> likely the other virtio devices touched by the refactoring. >>>> >>>> The issue seems to be a change the way section id strings are genera= ted in >>>> vmstate_register(). In v1.4.x we had: >>>> >>>> se->instance_id: 0, se->idstr: 0000:00:03.0/virtio-net >>>> >>>> In v1.5.0-rc2 we have: >>>> >>>> se->instance_id: 0, se->idstr: virtio-net >>>> >>>> This seems to be due to the fact that these devices now sit on a >>>> TYPE_VIRTIO_BUS that has no implementation of TYPE_BUS's get_dev_pat= h() >>>> interface, which is what savevm uses to calculate the id prefix for >>>> se->idstr. >>>> >>>> Prior to the refactoring, the device sat on a TYPE_PCI_BUS which use= d >>>> pcibus_get_dev_path() to calculate this. >>>> >>>> I'm not sure what the best fix is for this. I looking at implementin= g >>>> get_dev_path() for TYPE_VIRTIO_BUS, but to maintain migration >>>> compatibility we'd end up baking in PCI-specific stuff which from wh= at >>>> I gather is exactly what we were trying to avoid there. >>> I think get_dev_path for TYPE_VIRTIO_BUS could simply forward to the >>> parent device's parent bus. >>> >>> Paolo >> I think this can do the job, any better idea? > Monkey patching is a little ugly but it is a concise way of fixing the > problem. > > Mike, can you confirm the patch? > > Fred, can you add a Signed-off-by and send as a top-level? > > Regards, > > Anthony Liguori I just send a better way. > >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index d5257ed..e033b53 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -43,7 +43,6 @@ >> #endif >> >> static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int in= dent); >> -static char *pcibus_get_dev_path(DeviceState *dev); >> static char *pcibus_get_fw_dev_path(DeviceState *dev); >> static int pcibus_reset(BusState *qbus); >> >> @@ -2129,7 +2128,7 @@ static char *pcibus_get_fw_dev_path(DeviceState = *dev) >> return g_strdup(path); >> } >> >> -static char *pcibus_get_dev_path(DeviceState *dev) >> +char *pcibus_get_dev_path(DeviceState *dev) >> { >> PCIDevice *d =3D container_of(dev, PCIDevice, qdev); >> PCIDevice *t; >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 70d2c6b..0241223 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1514,11 +1514,19 @@ static void virtio_pci_bus_new(VirtioBusState >> *bus, VirtIOPCIProxy *dev) >> qbus->allow_hotplug =3D 1; >> } >> >> +static char *virtio_pci_bus_get_dev_path(DeviceState *dev) >> +{ >> + BusState *bus =3D qdev_get_parent_bus(dev); >> + DeviceState *proxy =3D DEVICE(bus->parent); >> + return g_strdup(pcibus_get_dev_path(proxy)); >> +} >> + >> static void virtio_pci_bus_class_init(ObjectClass *klass, void *dat= a) >> { >> BusClass *bus_class =3D BUS_CLASS(klass); >> VirtioBusClass *k =3D VIRTIO_BUS_CLASS(klass); >> bus_class->max_dev =3D 1; >> + bus_class->get_dev_path =3D virtio_pci_bus_get_dev_path; >> k->notify =3D virtio_pci_notify; >> k->save_config =3D virtio_pci_save_config; >> k->load_config =3D virtio_pci_load_config; >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index 8d075ab..fb5723c 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -708,6 +708,8 @@ static inline void pci_dma_sglist_init(QEMUSGList >> *qsg, PCIDevice *dev, >> >> extern const VMStateDescription vmstate_pci_device; >> >> +char *pcibus_get_dev_path(DeviceState *dev); >> + >> #define VMSTATE_PCI_DEVICE(_field, _state) { = \ >> .name =3D (stringify(_field)), = \ >> .size =3D sizeof(PCIDevice), = \ >> --=20 >> 1.7.11.7