qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Laszlo Ersek
Subject: Re: [Qemu-devel] [PATCH v2 07/18] hw/nvram/fw_cfg: Add fw_cfg_common_unrealize()
Date: Fri, 8 Mar 2019 11:29:51 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]