| Message ID | 1460732057-30032-3-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Juergen Gross writes ("[PATCH 2/3] libxc: adjust retry loop in xc_cpupool_removecpu()"):
> Commit 1ef6beea187b ("libxc: do some retries in xc_cpupool_removecpu()
> for EBUSY case") added a retry loop in xc_cpupool_removecpu() for the
> EBUSY case. As EBUSY was returned in multiple error situations the
> loop would have been executed in situations where a retry would not
> be successful. Additionally calling sleep(1) between the rerires is a
> bad idea when being called in a daemon.
>
> The hypervisor has been changed to return different error values now.
> The retry added in above mentioned commit should be done in the
> EADDRINUSE case now. As the error condition should last only for a
> very short time, the sleep(1) call can be removed.
>
> Requested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Thanks.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
I will follow up with further patches to improve the behaviour of xl.
Ian.
On Fri, 2016-04-15 at 16:19 +0100, Ian Jackson wrote: > Juergen Gross writes ("[PATCH 2/3] libxc: adjust retry loop in > xc_cpupool_removecpu()"): > > > > Commit 1ef6beea187b ("libxc: do some retries in > > xc_cpupool_removecpu() > > for EBUSY case") added a retry loop in xc_cpupool_removecpu() for > > the > > EBUSY case. As EBUSY was returned in multiple error situations the > > loop would have been executed in situations where a retry would not > > be successful. Additionally calling sleep(1) between the rerires is > > a > > bad idea when being called in a daemon. > > > > The hypervisor has been changed to return different error values > > now. > > The retry added in above mentioned commit should be done in the > > EADDRINUSE case now. As the error condition should last only for a > > very short time, the sleep(1) call can be removed. > > > > Requested-by: Ian Jackson <Ian.Jackson@eu.citrix.com> > > Signed-off-by: Juergen Gross <jgross@suse.com> > Thanks. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> Regards, Dario
On Fri, Apr 15, 2016 at 04:54:16PM +0200, Juergen Gross wrote: > Subject: [Xen-devel] [PATCH 2/3] libxc: adjust retry loop in > xc_cpupool_removecpu() > List-Id: Xen developer discussion <xen-devel.lists.xen.org> > > Commit 1ef6beea187b ("libxc: do some retries in xc_cpupool_removecpu() > for EBUSY case") added a retry loop in xc_cpupool_removecpu() for the > EBUSY case. As EBUSY was returned in multiple error situations the > loop would have been executed in situations where a retry would not > be successful. Additionally calling sleep(1) between the rerires is a s/rerires/retries/ Reviewed-by: Alan Robinson <alan.robinson@ts.fujitsu.com> Alan
Wei Liu writes ("Re: [PATCH 0/3] adjust error return values for cpupool operations"):
> In principle I agree we need better semantics and documentation of this
> hypercall, and because this is new code so it's risk free, so:
>
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
>
> I will leave this series for Ian to review.
Thanks for all the reviews and acks. I have queued these three
patches.
Ian.
diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c index 261b9c9..bb99e3b 100644 --- a/tools/libxc/xc_cpupool.c +++ b/tools/libxc/xc_cpupool.c @@ -139,7 +139,7 @@ int xc_cpupool_addcpu(xc_interface *xch, } /* - * The hypervisor might return EBUSY when trying to remove a cpu from a + * The hypervisor might return EADDRINUSE when trying to remove a cpu from a * cpupool when a domain running in this cpupool has pinned a vcpu * temporarily. Do some retries in this case, perhaps the situation * cleans up. @@ -160,9 +160,7 @@ int xc_cpupool_removecpu(xc_interface *xch, sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY : cpu; for ( retries = 0; retries < NUM_RMCPU_BUSY_RETRIES; retries++ ) { err = do_sysctl_save(xch, &sysctl); - if ( err < 0 && errno == EBUSY ) - sleep(1); - else + if ( err == 0 || errno != EADDRINUSE ) break; } return err;
Commit 1ef6beea187b ("libxc: do some retries in xc_cpupool_removecpu() for EBUSY case") added a retry loop in xc_cpupool_removecpu() for the EBUSY case. As EBUSY was returned in multiple error situations the loop would have been executed in situations where a retry would not be successful. Additionally calling sleep(1) between the rerires is a bad idea when being called in a daemon. The hypervisor has been changed to return different error values now. The retry added in above mentioned commit should be done in the EADDRINUSE case now. As the error condition should last only for a very short time, the sleep(1) call can be removed. Requested-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/libxc/xc_cpupool.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)