Re: [Qemu-devel] [PATCH 2/5] spapr: Refactor spapr_drc_detach()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

On Thu, Jun 22, 2017 at 11:32:15AM +0200, Greg Kurz wrote:
> On Wed, 21 Jun 2017 17:18:45 +0800
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > This function has two unused parameters - remove them.
> > 
> 
> It's ok for the d argument but I'm not sure about errp... Indeed it isn't used
> in the current code, but looking at the paths below spapr_drc_detach(), we
> have:
> 
> spapr_drc_detach()
>  spapr_drc_release()
>   spapr_core_release()
>    hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
> 
> and
> 
> spapr_drc_detach()
>  spapr_drc_release()
>   spapr_lmb_release()
>    hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
> 
> Shouldn't we pass the errp down to hotplug_handler_unplug() instead of
> aborting ?

So, at first I thought you were right, and was going to rewrite on
that basis.

Then I realized that calling hotplug_handler_unplug() is a bizarrely
roundabout way of doing what we need anyway (usually the unplug
handler is never called if there is an unplug_request() handler).
Once we expand those out to what they actually call, the errors
disappear again.  I'll do that in the next spin.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]