Re: [Qemu-devel] [PATCH v2 1/2] pvpanic: initialization cleanup

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 19, 2013 at 04:07:27PM +0200, Laszlo Ersek wrote:
> On 06/18/13 15:01, Michael S. Tsirkin wrote:
> > Avoid use of static variables: PC systems
> > initialize pvpanic device through pvpanic_init,
> > so we can simply create the fw_cfg file at that point.
> > This also makes it possible to assert if fw_cfg is not there
> > rather than skipping the device silently.
> > 
> > Reviewed-by:  Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> > Changes from v1:
> >     don't assert if !fw_cfg, simply skip fwcfg
> > 
> >  hw/misc/pvpanic.c | 26 ++++++++++++++------------
> >  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> Patches look good, however I think you missed to update the 1/2 commit
> message completely. You removed the sentence
> 
>   Others don't use fw_cfg at all.
> 
> but
> 
>   This also makes it possible to assert if fw_cfg is not there rather
>   than skipping the device silently.
> 
> remains, although it's exactly the assert() that's been erased.
> 
> Laszlo

Good point. It's going in through my tree so I'll tweak the
commit log but won't repost.
Thanks a lot for the thorough review!