Re: [PATCH] Fix scheduler support check

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Fri, Nov 18, 2016 at 10:59:58AM +0100, Erik Skultety wrote:
> On Fri, Nov 18, 2016 at 10:34:47AM +0100, Martin Kletzander wrote:
> > Commit 94cc577807ba tried fixing build on systems that did not have
> > SCHED_BATCH or SCHED_IDLE defined.  But instead of changing it to
> > conditional support, it rather completely disabled the support for
> > setting any scheduler.  Since then, such old systems are not
> > supported, but rather than reverting that commit, let's change that to
> > the conditional support.  That way any addition to the list of
> > schedulers can follow the same style so that we're consistent in the
> > future.
> >
> > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> > ---
> >
> > Notes:
> >     Of course there are various ways how to address that, I went with
> >     case.  Also defining undefined SCHED_* to -1 makes some gnulib
> >     syntax-check go haywire.
> >
> >  src/util/virprocess.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > index 718c4a2664e1..39d6b30c40f2 100644
> > --- a/src/util/virprocess.c
> > +++ b/src/util/virprocess.c
> > @@ -1183,7 +1183,7 @@ virProcessExitWithStatus(int status)
> >      exit(value);
> >  }
> >
> > -#if HAVE_SCHED_SETSCHEDULER && defined(SCHED_BATCH) && defined(SCHED_IDLE)
> > +#if HAVE_SCHED_SETSCHEDULER
> >
> >  static int
> >  virProcessSchedTranslatePolicy(virProcessSchedPolicy policy)
> > @@ -1196,10 +1196,18 @@ virProcessSchedTranslatePolicy(virProcessSchedPolicy policy)
> >          return SCHED_BATCH;
> >
> >      case VIR_PROC_POLICY_IDLE:
> > +# ifdef SCHED_IDLE
> >          return SCHED_IDLE;
> > +# else
> > +        return -1;
> > +# endif
> >
> >      case VIR_PROC_POLICY_FIFO:
> > +# ifdef SCHED_FIFO
> >          return SCHED_FIFO;
> > +# else
> > +        return -1;
> > +# endif
>
> Didn't you by any chance mean to make SCHED_BATCH conditional instead of
> SCHED_FIFO which might have probably been part of the kernel since forever (at
> least man 7 sched doesn't say anything about when it was added).

Um... well, I wonder in what version of this patch I changed that.
Probably somewhere between 3rd and 7th version.  I'll fix that back.
Thanks.

> ACK with that fixed.
>
> Erik
>
> >      case VIR_PROC_POLICY_RR:
> >          return SCHED_RR;
> > @@ -1225,6 +1233,13 @@ virProcessSetScheduler(pid_t pid,
> >      if (!policy)
> >          return 0;
> >
> > +    if (pol < 0) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Scheduler '%s' is not supported on this platform"),
> > +                       virProcessSchedPolicyTypeToString(policy));
> > +        return -1;
> > +    }
> > +
> >      if (pol == SCHED_FIFO || pol == SCHED_RR) {
> >          int min = 0;
> >          int max = 0;
> > --
> > 2.10.2
> >
> > --
> > libvir-list mailing list
> > libvir-list@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/libvir-list



> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list