Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

[Igor Mammedov]

On Fri, 7 Jul 2017 17:20:25 +0100
Mark Cave-Ayland <address@hidden> wrote:

> On 07/07/17 16:07, Eduardo Habkost wrote:
> 
> >> looks fine,
> >>
> >> so what I'd do is:
> >>  * drop 4/6  
> 
> Yes.
> 
> > Agreed on this point.  But:
> >   
> >>  * make fw_cfg_find() use ambiguous argument and error_abort if ambiguous 
> >> == true  
> 
> During my latest tests I've found that everything works fine without the
> ambiguous argument.
> 
> Do we still want to keep it? And I don't think error_abort() is the
> right thing to do here, I'd much rather return NULL and add a suitable
> comment.
I'd still use ambiguous argument and since you prefer not to assert
I'd add errp argument to fw_cfg_find() and handle error at callsites.

Just returning NULL isn't sufficient if you need to distinguish
'not found' vs 'duplicate' usecases, additionally  'not found'
in most cases isn't even error but 'duplicate' definitely is.

Aborting on diplicate in fw_cfg_find() is fine and would
help to avoid touching current callers if you wish to limit
patches scope, but you can go with proper error propagating
route if you wish.