Re: [Qemu-devel] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize()

[Laszlo Ersek]

On 03/08/19 07:55, Thomas Huth wrote:
> On 08/03/2019 02.32, Philippe Mathieu-Daudé wrote:
>> Back in abe147e0ce4 when fw_cfg_add_file() was introduced, there
>> was no QOM design, object where not created and released at runtime.

s/object where/objects were/

>> Later 38f3adc34d finished the QOM conversion of the fw_cfg device,
>> adding the fw_cfg_common_realize() method.

(I'm not sure if 38f3adc34d "finished the QOM conversion", but that's
just my lack of QOM knowledge. Hopefully someone can confirm whether
this statement is accurate.)

>> The time has come to add the equivalent destructor and release the
>> memory allocated for 'files'.
> 
> You should mention that the unrealize function is currently never called
> since the object never gets destroyed (AFAIK). But I hope we can fix
> that in the not so distant future, so:
> 
> Reviewed-by: Thomas Huth <address@hidden>
> 

How about we delay this patch until such time the function is actually
called?

This series is already huge, and quite divergent considering its goals.
(Please refer to the blurb.)

I don't mind this patch, but I think it should either belong to a
separate fw_cfg cleanup series (or "wave" if you like).

Or else, we should have a bug reported somewhere public, ensuring that
we eventually call the function introduced here. And then the commit
message should spell out -- as you say -- that the function is not
called yet, but it will be, because of <ticket-reference>.

Thanks
Laszlo