Re: [Xen-devel] [PATCH 1/3] libxc: Revert "do some retries in xc_cpupool_removecpu() for EBUSY case"

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Fri, 2016-04-15 at 07:15 +0200, Juergen Gross wrote:
> On 14/04/16 19:07, Ian Jackson wrote:
> > 
> > libxc may be called from within long-running daemons such as
> > libvirt.
> > 
> > In such a system this sleep would enable an uncooperative or buggy
> > guest to block all toolstack operations for an extended period.
> > 
> > Sadly, therefore, such a retry loop is not feasible without a lot
> > of
> > engineering which is probably not appropriate.
> I understand your concerns. OTOH you should consider that libvirt has
> no support for cpupool operations today, so it won't run this code.
> 
True, and it does not even have the concept of pooling (not in a
compatible way with what we provide with cpupools), so it probably
won't be start supporting them anytime soon.

HOWEVER, Ian's concern applies to any daemon based toolstack (i.e., not
necessarily libvirt) which may want to support cpupools, and would then
fall into this.

> > diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c
> > index 261b9c9..c42273e 100644
> > --- a/tools/libxc/xc_cpupool.c
> > +++ b/tools/libxc/xc_cpupool.c
> > @@ -20,7 +20,6 @@
> > 
> >  int xc_cpupool_removecpu(xc_interface *xch,
> >                           uint32_t poolid,
> >                           int cpu)
> >  {
> > -    unsigned retries;
> > -    int err;
> >      DECLARE_SYSCTL;
> >  
> >      sysctl.cmd = XEN_SYSCTL_cpupool_op;
> >      sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU;
> >      sysctl.u.cpupool_op.cpupool_id = poolid;
> >      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);
> I'd rather just remove this sleep() call than the whole retry logic.
> EBUSY cases should be very very very very rare and last for some
> msecs or usecs only.
> 
+1

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel