Re: [Qemu-devel] [PATCH v5 2/6] qdev: introduce FWPathProvider interface

[Alexey Kardashevskiy]

On 03/13/2014 05:07 AM, Andreas Färber wrote:
> Am 20.02.2014 14:50, schrieb Alexey Kardashevskiy:
>> QEMU supports firmware names for all devices in the QEMU tree but
>> some architectures expect some parts of firmware path names in different
>> format.
>>
>> This introduces a firmware-pathname-change interface definition.
>> If some machines needs to redefine the firmware path format, it has
>> to add the TYPE_FW_PATH_PROVIDER interface to an object that is above
>> the device on the QOM tree (typically /machine).
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
> 
> I do not see a reply to my question on v4 and didn't catch bonzini on
> IRC - guessing the two lines should be swapped since both v4 and v5 were
> submitted by Alexey?

I'll switch.


> 
>> ---
>> Changes:
>> v5:
>> * fixed code design
>> * added license headers
>>
>> v4:
>> * added fw-path-provider.o into tests/Makefile
>> * fixed 80chars warning from checkpatch.pl
>> ---
>>  hw/core/Makefile.objs         |  1 +
>>  hw/core/fw-path-provider.c    | 51 
>> +++++++++++++++++++++++++++++++++++++++++++
>>  hw/core/qdev.c                | 18 ++++++++++++++-
>>  include/hw/fw-path-provider.h | 49 +++++++++++++++++++++++++++++++++++++++++
>>  tests/Makefile                |  1 +
>>  5 files changed, 119 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/core/fw-path-provider.c
>>  create mode 100644 include/hw/fw-path-provider.h
>>
>> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
>> index 9e324be..9f75ea3 100644
>> --- a/hw/core/Makefile.objs
>> +++ b/hw/core/Makefile.objs
>> @@ -1,5 +1,6 @@
>>  # core qdev-related obj files, also used by *-user:
>>  common-obj-y += qdev.o qdev-properties.o
>> +common-obj-y += fw-path-provider.o
>>  # irq.o needed for qdev GPIO handling:
>>  common-obj-y += irq.o
>>  common-obj-y += hotplug.o
>> diff --git a/hw/core/fw-path-provider.c b/hw/core/fw-path-provider.c
>> new file mode 100644
>> index 0000000..b117157
>> --- /dev/null
>> +++ b/hw/core/fw-path-provider.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + *  Firmware patch provider class and helpers.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; under version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "hw/fw-path-provider.h"
>> +
>> +char *fw_path_provider_get_dev_path(FWPathProvider *p, BusState *bus,
>> +                                    DeviceState *dev)
>> +{
>> +    FWPathProviderClass *k = FW_PATH_PROVIDER_GET_CLASS(p);
>> +
>> +    return k->get_dev_path(p, bus, dev);
>> +}
>> +
>> +char *fw_path_provider_try_get_dev_path(Object *o, BusState *bus,
>> +                                        DeviceState *dev)
>> +{
>> +    FWPathProvider *p = (FWPathProvider *)
>> +        object_dynamic_cast(o, TYPE_FW_PATH_PROVIDER);
>> +
>> +    if (p) {
>> +        return fw_path_provider_get_dev_path(p, bus, dev);
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +static const TypeInfo fw_path_provider_info = {
>> +    .name          = TYPE_FW_PATH_PROVIDER,
>> +    .parent        = TYPE_INTERFACE,
>> +    .class_size    = sizeof(FWPathProviderClass),
>> +};
>> +
>> +static void fw_path_provider_register_types(void)
>> +{
>> +    type_register_static(&fw_path_provider_info);
>> +}
>> +
>> +type_init(fw_path_provider_register_types)
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index ae30163..dd993b5 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -26,6 +26,7 @@
>>     this API directly.  */
>>  
>>  #include "hw/qdev.h"
>> +#include "hw/fw-path-provider.h"
>>  #include "sysemu/sysemu.h"
>>  #include "qapi/error.h"
>>  #include "qapi/qmp/qerror.h"
>> @@ -529,6 +530,18 @@ static char *bus_get_fw_dev_path(BusState *bus, 
>> DeviceState *dev)
>>      return NULL;
>>  }
>>  
>> +static char *qdev_get_fw_dev_path_from_handler(BusState *bus, DeviceState 
>> *dev)
>> +{
>> +    Object *obj = OBJECT(dev);
>> +    char *d = NULL;
>> +
>> +    while (!d && obj->parent) {
>> +        obj = obj->parent;
>> +        d = fw_path_provider_try_get_dev_path(obj, bus, dev);
>> +    }
>> +    return d;
>> +}
>> +
>>  static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>>  {
>>      int l = 0;
>> @@ -536,7 +549,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState 
>> *dev, char *p, int size)
>>      if (dev && dev->parent_bus) {
>>          char *d;
>>          l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
>> -        d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> +        d = qdev_get_fw_dev_path_from_handler(dev->parent_bus, dev);
>> +        if (!d) {
>> +            d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> +        }
>>          if (d) {
>>              l += snprintf(p + l, size - l, "%s", d);
>>              g_free(d);
>> diff --git a/include/hw/fw-path-provider.h b/include/hw/fw-path-provider.h
>> new file mode 100644
>> index 0000000..8889d8b
>> --- /dev/null
>> +++ b/include/hw/fw-path-provider.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + *  Firmware patch provider class and helpers definitions.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; under version 2 of the License.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef FW_PATH_PROVIDER_H
>> +#define FW_PATH_PROVIDER_H 1
>> +
>> +#include "qemu-common.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_FW_PATH_PROVIDER "fw-path-provider"
>> +
>> +#define FW_PATH_PROVIDER_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(FWPathProviderClass, (klass), TYPE_FW_PATH_PROVIDER)
>> +#define FW_PATH_PROVIDER_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(FWPathProviderClass, (obj), TYPE_FW_PATH_PROVIDER)
>> +#define FW_PATH_PROVIDER(obj) \
>> +     INTERFACE_CHECK(FWPathProvider, (obj), TYPE_FW_PATH_PROVIDER)
>> +
>> +typedef struct FWPathProvider {
>> +    Object parent_obj;
>> +} FWPathProvider;
> 
> Unused?


Not today, this is stub. Remove?


>> +
>> +typedef void (*StreamCanPushNotifyFn)(void *opaque);
> 
> Unused?

I'll remove, looks like cut-n-paste error.


> Hoping we can consider this a bugfix and get done during freeze.


Oops. Did it manage to make it to any branch already? Which one?



> Andreas
> 
>> +
>> +typedef struct FWPathProviderClass {
>> +    InterfaceClass parent_class;
>> +
>> +    char *(*get_dev_path)(FWPathProvider *p, BusState *bus, DeviceState 
>> *dev);
>> +} FWPathProviderClass;
>> +
>> +char *fw_path_provider_get_dev_path(FWPathProvider *p, BusState *bus,
>> +                                    DeviceState *dev);
>> +char *fw_path_provider_try_get_dev_path(Object *o, BusState *bus,
>> +                                        DeviceState *dev);
>> +
>> +#endif /* FW_PATH_PROVIDER_H */
>> diff --git a/tests/Makefile b/tests/Makefile
>> index 9a7d2f1..0692b11 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -165,6 +165,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
>>  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>>      hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
>>      hw/core/irq.o \
>> +    hw/core/fw-path-provider.o \
>>      $(qom-core-obj) \
>>      $(test-qapi-obj-y) \
>>      libqemuutil.a libqemustub.a
>>
> 
> 


-- 
Alexey