On Mon, May 10, 2021 at 15:16:11 +0200, Pavel Hrdina wrote:
> When QEMU introduces new firmware features libvirt will fail until we
> list that feature in our code as well which doesn't sound right.
>
> We should simply ignore the new feature until we add a proper support
> for it.
>
> Reported-by: Laszlo Ersek <lersek@xxxxxxxxxx>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/qemu/qemu_firmware.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index 94e88ebe4b..e37a7edefa 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -567,6 +567,7 @@ qemuFirmwareFeatureParse(const char *path,
> virJSONValue *featuresJSON;
> g_autoptr(qemuFirmwareFeature) features = NULL;
Not related to this patch, but a bug nevertheless. 'features' is an
array allocated by:
features = g_new0(qemuFirmwareFeature, nfeatures);
Using g_autoptr calls the proper destructor function only for the first
element!
The whole premise of declaring an autoptr function for an enum type
seems a bit flawed to me!
> size_t nfeatures;
> + size_t nparsed = 0;
> size_t i;
>
> if (!(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) {
> @@ -586,17 +587,16 @@ qemuFirmwareFeatureParse(const char *path,
> int tmp;
>
> if ((tmp = qemuFirmwareFeatureTypeFromString(tmpStr)) <= 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unknown feature %s"),
> - tmpStr);
> - return -1;
> + VIR_DEBUG("unknown feature %s", tmpStr);
> + continue;
> }
>
> - features[i] = tmp;
> + features[nparsed] = tmp;
> + nparsed++;
> }
>
> fw->features = g_steal_pointer(&features);
> - fw->nfeatures = nfeatures;
> + fw->nfeatures = nparsed;
> return 0;
> }
For this patch:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
