Re: [Qemu-devel] [ANNOUNCE] QEMU 1.5.0-rc2 is now available

["KONRAD Frédéric" <fred.konrad@greensocs.com>]

On 16/05/2013 18:33, Anthony Liguori wrote:
> KONRAD Frédéric <fred.konrad@greensocs.com> writes:
>
>> On 16/05/2013 16:51, Paolo Bonzini wrote:
>>> Il 16/05/2013 16:21, mdroth ha scritto:
>>>> commit e37da3945fa2fde161e1b217f937fc318c4b7639
>>>> Author: KONRAD Frederic <fred.konrad@greensocs.com>
>>>> Date:   Thu Apr 11 16:29:58 2013 +0200
>>>>
>>>>       virtio-net-pci: switch to the new API.
>>>>       
>>>>       Here the virtio-net-pci is modified for the new API. The device
>>>>       virtio-net-pci extends virtio-pci. It creates and connects a
>>>>       virtio-net-device during the init. The properties are not changed.
>>>>       
>>>>       Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>>>       Tested-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>>>       Message-id: 1365690602-22729-4-git-send-email-fred.konrad@greensocs.com
>>>>       Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>>>
>>>> And if we roll that back, we have similar failures for virtio-blk, and most
>>>> likely the other virtio devices touched by the refactoring.
>>>>
>>>> The issue seems to be a change the way section id strings are generated 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_path()
>>>> 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 used
>>>> pcibus_get_dev_path() to calculate this.
>>>>
>>>> I'm not sure what the best fix is for this. I looking at implementing
>>>> get_dev_path() for TYPE_VIRTIO_BUS, but to maintain migration
>>>> compatibility we'd end up baking in PCI-specific stuff which from what
>>>> 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 indent);
>> -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 = 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 = 1;
>>    }
>>
>> +static char *virtio_pci_bus_get_dev_path(DeviceState *dev)
>> +{
>> +    BusState *bus = qdev_get_parent_bus(dev);
>> +    DeviceState *proxy = DEVICE(bus->parent);
>> +    return g_strdup(pcibus_get_dev_path(proxy));
>> +}
>> +
>>    static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>>    {
>>        BusClass *bus_class = BUS_CLASS(klass);
>>        VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
>>        bus_class->max_dev = 1;
>> +    bus_class->get_dev_path = virtio_pci_bus_get_dev_path;
>>        k->notify = virtio_pci_notify;
>>        k->save_config = virtio_pci_save_config;
>>        k->load_config = 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       = (stringify(_field)),                               \
>>        .size       = sizeof(PCIDevice),                                 \
>> -- 
>> 1.7.11.7