Thanks for the review, Brian.

>
> I suspect we might want these two as cluster parameters like max_running_jobs.
>
> At first I thought of exposing these as luxid command line options, but
> then you'd have the problem of copying updates to these options in
> /etc/defaults/ganeti to all nodes, so having them as cluster params is better.
>

Yeah I'm planning to set them as cluster parameters, as we also
discussed (and I promised) at GanetiCon. I'm going to be working on a
couple of follow-up patches after this gets accepted.

>
> The type signature of this is quite odd: two timestamps, one integer and the
> other JQ.Timestamp. Could you make both be JQ.Timestamp (I know it's just a
> sec/usec tuple), and use JQueue.currentTimestamp to get the current time.
> JQ.Timestamp seems to be used pretty heavily in the rest of the scheduler 
> code.

Agreed, keeping it consistent makes it also flow much better, thanks
for pointing this out.

I'm sending out the fixed patch in a new message to keep things clean
as this has gotten a bit muddied and I prefer to keep more clarity in
the history.

Reply via email to