Patchwork [ANNOUNCE] QEMU 1.5.0-rc2 is now available

login
register
mail settings
Submitter fred.konrad@greensocs.com
Date May 16, 2013, 4:34 p.m.
Message ID <51950A81.8050703@greensocs.com>
Download mbox | patch
Permalink /patch/244396/
State New
Headers show

Comments

fred.konrad@greensocs.com - May 16, 2013, 4:34 p.m.
On 16/05/2013 18:07, Paolo Bonzini wrote:
> Il 16/05/2013 17:54, KONRAD Frédéric ha scritto:
>> I think this can do the job, any better idea?
>>
>> 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));
> You do not need to export pcibus_get_dev_path.  This should just return
> qdev_get_dev_path(proxy) and should be in TYPE_VIRTIO_BUS, not in the
> PCI-specific subclass.
>
> (The g_strdup is not needed, either).
>
> Paolo

True, I avoided it because of CCW and S390, but as they don't have there 
get_dev_path, it seems
not to change anything for them.

I think that's better and I get 0000:00:04.0/virtio-net for idstr.

  static void virtio_register_types(void)
Michael Roth - May 16, 2013, 4:49 p.m.
On Thu, May 16, 2013 at 06:34:09PM +0200, KONRAD Frédéric wrote:
> On 16/05/2013 18:07, Paolo Bonzini wrote:
> >Il 16/05/2013 17:54, KONRAD Frédéric ha scritto:
> >>I think this can do the job, any better idea?
> >>
> >>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));
> >You do not need to export pcibus_get_dev_path.  This should just return
> >qdev_get_dev_path(proxy) and should be in TYPE_VIRTIO_BUS, not in the
> >PCI-specific subclass.
> >
> >(The g_strdup is not needed, either).
> >
> >Paolo
> 
> True, I avoided it because of CCW and S390, but as they don't have
> there get_dev_path, it seems
> not to change anything for them.
> 
> I think that's better and I get 0000:00:04.0/virtio-net for idstr.

Sorry, my email seems to be malfunctioning this morning and I didn't see
this before sending mine. Not sure which one is better but I'll be happy
to test whatever we decide on.

> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index aab72ff..ea2e11a 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -154,12 +154,26 @@ void virtio_bus_set_vdev_config(VirtioBusState
> *bus, uint8_t *config)
>      }
>  }
> 
> +static char *virtio_bus_get_dev_path(DeviceState *dev)
> +{
> +    BusState *bus = qdev_get_parent_bus(dev);
> +    DeviceState *proxy = DEVICE(bus->parent);
> +    return qdev_get_dev_path(proxy);
> +}
> +
> +static void virtio_bus_class_init(ObjectClass *klass, void *data)
> +{
> +    BusClass *bus_class = BUS_CLASS(klass);
> +    bus_class->get_dev_path = virtio_bus_get_dev_path;
> +}
> +
>  static const TypeInfo virtio_bus_info = {
>      .name = TYPE_VIRTIO_BUS,
>      .parent = TYPE_BUS,
>      .instance_size = sizeof(VirtioBusState),
>      .abstract = true,
>      .class_size = sizeof(VirtioBusClass),
> +    .class_init = virtio_bus_class_init
>  };
> 
>  static void virtio_register_types(void)
> -- 
> 1.7.11.7
>
Paolo Bonzini - May 16, 2013, 4:52 p.m.
Il 16/05/2013 18:34, KONRAD Frédéric ha scritto:
> True, I avoided it because of CCW and S390, but as they don't have there
> get_dev_path, it seems
> not to change anything for them.
> 
> I think that's better and I get 0000:00:04.0/virtio-net for idstr.

Thanks,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index aab72ff..ea2e11a 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -154,12 +154,26 @@ void virtio_bus_set_vdev_config(VirtioBusState
> *bus, uint8_t *config)
>      }
>  }
> 
> +static char *virtio_bus_get_dev_path(DeviceState *dev)
> +{
> +    BusState *bus = qdev_get_parent_bus(dev);
> +    DeviceState *proxy = DEVICE(bus->parent);
> +    return qdev_get_dev_path(proxy);
> +}
> +
> +static void virtio_bus_class_init(ObjectClass *klass, void *data)
> +{
> +    BusClass *bus_class = BUS_CLASS(klass);
> +    bus_class->get_dev_path = virtio_bus_get_dev_path;
> +}
> +
>  static const TypeInfo virtio_bus_info = {
>      .name = TYPE_VIRTIO_BUS,
>      .parent = TYPE_BUS,
>      .instance_size = sizeof(VirtioBusState),
>      .abstract = true,
>      .class_size = sizeof(VirtioBusClass),
> +    .class_init = virtio_bus_class_init
>  };
> 
>  static void virtio_register_types(void)
fred.konrad@greensocs.com - May 16, 2013, 4:53 p.m.
On 16/05/2013 18:49, mdroth wrote:
> On Thu, May 16, 2013 at 06:34:09PM +0200, KONRAD Frédéric wrote:
>> On 16/05/2013 18:07, Paolo Bonzini wrote:
>>> Il 16/05/2013 17:54, KONRAD Frédéric ha scritto:
>>>> I think this can do the job, any better idea?
>>>>
>>>> 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));
>>> You do not need to export pcibus_get_dev_path.  This should just return
>>> qdev_get_dev_path(proxy) and should be in TYPE_VIRTIO_BUS, not in the
>>> PCI-specific subclass.
>>>
>>> (The g_strdup is not needed, either).
>>>
>>> Paolo
>> True, I avoided it because of CCW and S390, but as they don't have
>> there get_dev_path, it seems
>> not to change anything for them.
>>
>> I think that's better and I get 0000:00:04.0/virtio-net for idstr.
> Sorry, my email seems to be malfunctioning this morning and I didn't see
> this before sending mine. Not sure which one is better but I'll be happy
> to test whatever we decide on.

Well, seems our patches are sent at the same moment.
>> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
>> index aab72ff..ea2e11a 100644
>> --- a/hw/virtio/virtio-bus.c
>> +++ b/hw/virtio/virtio-bus.c
>> @@ -154,12 +154,26 @@ void virtio_bus_set_vdev_config(VirtioBusState
>> *bus, uint8_t *config)
>>       }
>>   }
>>
>> +static char *virtio_bus_get_dev_path(DeviceState *dev)
>> +{
>> +    BusState *bus = qdev_get_parent_bus(dev);
>> +    DeviceState *proxy = DEVICE(bus->parent);
>> +    return qdev_get_dev_path(proxy);
>> +}
>> +
>> +static void virtio_bus_class_init(ObjectClass *klass, void *data)
>> +{
>> +    BusClass *bus_class = BUS_CLASS(klass);
>> +    bus_class->get_dev_path = virtio_bus_get_dev_path;
>> +}
>> +
>>   static const TypeInfo virtio_bus_info = {
>>       .name = TYPE_VIRTIO_BUS,
>>       .parent = TYPE_BUS,
>>       .instance_size = sizeof(VirtioBusState),
>>       .abstract = true,
>>       .class_size = sizeof(VirtioBusClass),
>> +    .class_init = virtio_bus_class_init
>>   };
>>
>>   static void virtio_register_types(void)
>> -- 
>> 1.7.11.7
>>

Patch

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index aab72ff..ea2e11a 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -154,12 +154,26 @@  void virtio_bus_set_vdev_config(VirtioBusState 
*bus, uint8_t *config)
      }
  }

+static char *virtio_bus_get_dev_path(DeviceState *dev)
+{
+    BusState *bus = qdev_get_parent_bus(dev);
+    DeviceState *proxy = DEVICE(bus->parent);
+    return qdev_get_dev_path(proxy);
+}
+
+static void virtio_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *bus_class = BUS_CLASS(klass);
+    bus_class->get_dev_path = virtio_bus_get_dev_path;
+}
+
  static const TypeInfo virtio_bus_info = {
      .name = TYPE_VIRTIO_BUS,
      .parent = TYPE_BUS,
      .instance_size = sizeof(VirtioBusState),
      .abstract = true,
      .class_size = sizeof(VirtioBusClass),
+    .class_init = virtio_bus_class_init
  };