| Message ID | 20201006153636.2383248-2-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qom: Fix missing interface in qemu-storage-daemon | expand |
On 06/10/20 17:36, Philippe Mathieu-Daudé wrote: > While the FW_CFG_DATA_GENERATOR_INTERFACE is only consumed > by a device only available using system-mode (fw_cfg), it is > implemented by a crypto component (tls-cipher-suites) which > is always available when crypto is used. > > Commit 69699f3055 introduced the following error in the > qemu-storage-daemon binary: > > $ echo -e \ > '{"execute": "qmp_capabilities"}\r\n{"execute": "qom-list-types"}\r\n{"execute": "quit"}\r\n' \ > | storage-daemon/qemu-storage-daemon --chardev stdio,id=qmp0 --monitor qmp0 > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 5}, "package": ""}, "capabilities": ["oob"]}} > {"return": {}} > missing interface 'fw_cfg-data-generator' for object 'tls-creds' > Aborted (core dumped) > > Since QOM dependencies are resolved at runtime, this issue > could not be triggered at linktime, and we don't have test > running the qemu-storage-daemon binary. > > Fix by always registering the QOM interface. > > Reported-by: Kevin Wolf <kwolf@redhat.com> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Fixes: 69699f3055 ("crypto/tls-cipher-suites: Produce fw_cfg consumable blob") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/nvram/fw_cfg-interface.c | 15 +++++++++++++++ > hw/nvram/fw_cfg.c | 7 ------- > MAINTAINERS | 2 +- > hw/nvram/meson.build | 3 +++ > 4 files changed, 19 insertions(+), 8 deletions(-) > create mode 100644 hw/nvram/fw_cfg-interface.c > > diff --git a/hw/nvram/fw_cfg-interface.c b/hw/nvram/fw_cfg-interface.c > new file mode 100644 > index 0000000000..bcc7b1edbc > --- /dev/null > +++ b/hw/nvram/fw_cfg-interface.c > @@ -0,0 +1,15 @@ > +#include "qemu/osdep.h" > +#include "hw/nvram/fw_cfg.h" > + > +static const TypeInfo fw_cfg_data_generator_interface_info = { > + .parent = TYPE_INTERFACE, > + .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE, > + .class_size = sizeof(FWCfgDataGeneratorClass), > +}; > + > +static void fw_cfg_register_interfaces(void) > +{ > + type_register_static(&fw_cfg_data_generator_interface_info); > +} > + > +type_init(fw_cfg_register_interfaces) > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 0e95d057fd..08539a1aab 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -1360,18 +1360,11 @@ static const TypeInfo fw_cfg_mem_info = { > .class_init = fw_cfg_mem_class_init, > }; > > -static const TypeInfo fw_cfg_data_generator_interface_info = { > - .parent = TYPE_INTERFACE, > - .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE, > - .class_size = sizeof(FWCfgDataGeneratorClass), > -}; > - > static void fw_cfg_register_types(void) > { > type_register_static(&fw_cfg_info); > type_register_static(&fw_cfg_io_info); > type_register_static(&fw_cfg_mem_info); > - type_register_static(&fw_cfg_data_generator_interface_info); > } > > type_init(fw_cfg_register_types) > diff --git a/MAINTAINERS b/MAINTAINERS > index b76fb31861..a45d908ebd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2054,7 +2054,7 @@ R: Laszlo Ersek <lersek@redhat.com> > R: Gerd Hoffmann <kraxel@redhat.com> > S: Supported > F: docs/specs/fw_cfg.txt > -F: hw/nvram/fw_cfg.c > +F: hw/nvram/fw_cfg*.c > F: stubs/fw_cfg.c > F: include/hw/nvram/fw_cfg.h > F: include/standard-headers/linux/qemu_fw_cfg.h > diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build > index 1f2ed013b2..fd2951a860 100644 > --- a/hw/nvram/meson.build > +++ b/hw/nvram/meson.build > @@ -1,3 +1,6 @@ > +# QOM interfaces must be available anytime QOM is used. > +qom_ss.add(files('fw_cfg-interface.c')) > + > softmmu_ss.add(files('fw_cfg.c')) > softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c')) > softmmu_ss.add(when: 'CONFIG_DS1225Y', if_true: files('ds1225y.c')) > Queued, thanks. Paolo
On 10/6/20 10:36 AM, Philippe Mathieu-Daudé wrote: > +++ b/hw/nvram/fw_cfg-interface.c > @@ -0,0 +1,15 @@ > +#include "qemu/osdep.h" > +#include "hw/nvram/fw_cfg.h" License boilerplate missing. r~
On 10/6/20 5:51 PM, Richard Henderson wrote: > On 10/6/20 10:36 AM, Philippe Mathieu-Daudé wrote: >> +++ b/hw/nvram/fw_cfg-interface.c >> @@ -0,0 +1,15 @@ >> +#include "qemu/osdep.h" >> +#include "hw/nvram/fw_cfg.h" > > License boilerplate missing. Grr. Paolo since you queued this, do you mind fixing directly?
On 10/06/20 17:51, Richard Henderson wrote: > On 10/6/20 10:36 AM, Philippe Mathieu-Daudé wrote: >> +++ b/hw/nvram/fw_cfg-interface.c >> @@ -0,0 +1,15 @@ >> +#include "qemu/osdep.h" >> +#include "hw/nvram/fw_cfg.h" > > License boilerplate missing. > > r~ > Hrmpf, so easy to forget about that, especially in review :/ Thanks for catching it. Laszlo
diff --git a/hw/nvram/fw_cfg-interface.c b/hw/nvram/fw_cfg-interface.c new file mode 100644 index 0000000000..bcc7b1edbc --- /dev/null +++ b/hw/nvram/fw_cfg-interface.c @@ -0,0 +1,15 @@ +#include "qemu/osdep.h" +#include "hw/nvram/fw_cfg.h" + +static const TypeInfo fw_cfg_data_generator_interface_info = { + .parent = TYPE_INTERFACE, + .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE, + .class_size = sizeof(FWCfgDataGeneratorClass), +}; + +static void fw_cfg_register_interfaces(void) +{ + type_register_static(&fw_cfg_data_generator_interface_info); +} + +type_init(fw_cfg_register_interfaces) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 0e95d057fd..08539a1aab 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -1360,18 +1360,11 @@ static const TypeInfo fw_cfg_mem_info = { .class_init = fw_cfg_mem_class_init, }; -static const TypeInfo fw_cfg_data_generator_interface_info = { - .parent = TYPE_INTERFACE, - .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE, - .class_size = sizeof(FWCfgDataGeneratorClass), -}; - static void fw_cfg_register_types(void) { type_register_static(&fw_cfg_info); type_register_static(&fw_cfg_io_info); type_register_static(&fw_cfg_mem_info); - type_register_static(&fw_cfg_data_generator_interface_info); } type_init(fw_cfg_register_types) diff --git a/MAINTAINERS b/MAINTAINERS index b76fb31861..a45d908ebd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2054,7 +2054,7 @@ R: Laszlo Ersek <lersek@redhat.com> R: Gerd Hoffmann <kraxel@redhat.com> S: Supported F: docs/specs/fw_cfg.txt -F: hw/nvram/fw_cfg.c +F: hw/nvram/fw_cfg*.c F: stubs/fw_cfg.c F: include/hw/nvram/fw_cfg.h F: include/standard-headers/linux/qemu_fw_cfg.h diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build index 1f2ed013b2..fd2951a860 100644 --- a/hw/nvram/meson.build +++ b/hw/nvram/meson.build @@ -1,3 +1,6 @@ +# QOM interfaces must be available anytime QOM is used. +qom_ss.add(files('fw_cfg-interface.c')) + softmmu_ss.add(files('fw_cfg.c')) softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c')) softmmu_ss.add(when: 'CONFIG_DS1225Y', if_true: files('ds1225y.c'))
While the FW_CFG_DATA_GENERATOR_INTERFACE is only consumed by a device only available using system-mode (fw_cfg), it is implemented by a crypto component (tls-cipher-suites) which is always available when crypto is used. Commit 69699f3055 introduced the following error in the qemu-storage-daemon binary: $ echo -e \ '{"execute": "qmp_capabilities"}\r\n{"execute": "qom-list-types"}\r\n{"execute": "quit"}\r\n' \ | storage-daemon/qemu-storage-daemon --chardev stdio,id=qmp0 --monitor qmp0 {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 5}, "package": ""}, "capabilities": ["oob"]}} {"return": {}} missing interface 'fw_cfg-data-generator' for object 'tls-creds' Aborted (core dumped) Since QOM dependencies are resolved at runtime, this issue could not be triggered at linktime, and we don't have test running the qemu-storage-daemon binary. Fix by always registering the QOM interface. Reported-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Fixes: 69699f3055 ("crypto/tls-cipher-suites: Produce fw_cfg consumable blob") Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/nvram/fw_cfg-interface.c | 15 +++++++++++++++ hw/nvram/fw_cfg.c | 7 ------- MAINTAINERS | 2 +- hw/nvram/meson.build | 3 +++ 4 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 hw/nvram/fw_cfg-interface.c