Re: [Qemu-stable] [PATCH v2] qdev: don't add typename to fw_dev_path when get_fw_dev_path isn't implemented

[Paolo Bonzini]

Il 29/05/2013 06:41, Amos Kong ha scritto:
> On Tue, May 28, 2013 at 01:15:57PM +0200, Paolo Bonzini wrote:
>> Il 28/05/2013 13:00, Amos Kong ha scritto:
>>> Recent virtio refactoring in QEMU made virtio-bus become the parent bus
>>> of scsi-bus, and virtio-bus doesn't have get_fw_dev_path implementation,
>>> so redundant typename will be added to fw_dev_path. It causes that
>>> bootindex parameter of scsi device doesn't work.
>>>
>>> This patch changes qdev_get_fw_dev_path_helper() to add nothing if
>>> implemented get_fw_dev_path() returns NULL.
>>>
>>> Signed-off-by: Amos Kong <address@hidden>
>>> --
>>> v2: only add nothing to fw_dev_path when get_fw_dev_path() is
>>>     implemented and return NULL. then it will not effect other devices
>>>     don't have get_fw_dev_path() implementation.
> 
> Hi Paolo,
>  
>> To achieve the same result, you can simply ensure that all classes
>> implement the method.
> 
> Do you mean if some classes didn't have an implement, I need to add
> a implement (just returns NULL) for all of them, then nothing will be
> added to fw_dev_path for those classes?

No, you can have an implementation in BusClass.  See how
device_class_init provides a default implementation for
realize/unrealize, and do the same in bus_class_init.

> Currently I'm sure 'typename' is redundant for virio-bus. 'typename'
> might be needed for other class (have no get_fw_dev_path implementation)
> 
> Applied patch v2, there are three conditions: 
>  1) implement get_fw_dev_path, return specific string, add to
>     fw_dev_path (same as original)
>  2) don't implement get_fw_dev_path, add typename to fw_dev_path.
>     (same as original)
>  3) implement get_fw_dev_path and return NULL, nothing is added
>     to fw_dev_path (new behavior)
> 
> If some other class wants to add nothing to path, just define a method
> to return NULL.
> 
>> The default implementation is just
>> g_strdup(object_get_typename(OBJECT(dev))).
> 
> If we implement the method for all classes, the default implementation
> in qdev_get_fw_dev_path_helper() will not be called.

Yes, but we shouldn't implement the method for all classes.  We should
implement it just once in BusClass.  Then the default implementation in
qdev_get_fw_dev_path_helper() can be removed.  It will live in BusClass
instead

Paolo

>> Remember that if you do not specify a method in a BusClass's class_init
>> function, it is inherited from the superclass.
>>
>> Paolo
>