[libvirt] [PATCH] networkStartNetworkVirtual: Don't overwrite error in 'err5'

[Peter Krempa]

On Tue, Apr 23, 2019 at 17:14:27 +0200, Peter Krempa wrote:
> Now the proper review:
> 
> On Tue, Apr 23, 2019 at 16:21:49 +0200, Michal Privoznik wrote:
> > If there's an error when setting up QoS on a bridge the control
> > jumps over to 'err5' label. Here, the virNetDevBandwidthClear()
> > is called to clear out any partially set QoS. This function can
> > also report an error which would overwrite the actual error that
> > caused us jumping here. 🤦 Use virErrorPreserveLast() to preserve
> 
> Drop the emoji.
> 
> > the original error.
> > 
> > Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> > ---
> >  src/network/bridge_driver.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> > index ce4f4890f1..77206b4584 100644
> > --- a/src/network/bridge_driver.c
> > +++ b/src/network/bridge_driver.c
> > @@ -2493,6 +2493,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
> >      return 0;
> >  
> >   err5:
> > +    virErrorPreserveLast(&save_err);
> 
> 
> You [1] said:
> 
> "2) there are several places where we call virSetError() + virFreeError() even
> though we've caleld virErrorPreserveLast() earlier. Now, it's not incorrect
> (strictly speaking), but it would look much nicer if we called
> virErrorRestore() instead. But this is something for a separate patch."
> 
> And then Daniel added [2]:
> 
> "IMHO it should be part of this patch.  virErrorPreserveLast is intended
> to be matched with virErrorRestore, so any conversion that adds the
> former, should add the latter in the same method."
> 
> networkStartNetworkVirtual uses virSaveLastError() in other cases and
> virSetError/virFreeError on the cleanup path. Either convert it before
> adding the above call or use the old way.

Sorry. I didn't notice the other patch which also fixes this function
which was already pushed.

ACK
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190423/8e2a49d9/attachment-0001.sig>