On Fri, May 26, 2006 at 10:51:55AM -0500, Anthony Liguori wrote:
> Just some random feedback.
>
> Xen patchbot-unstable wrote:
> >
> >+static PyObject *pyxc_csched_domain_set(XcObject *self,
> >+ PyObject *args,
> >+ PyObject *kwds)
> >+{
> >+ uint32_t domid;
> >+ uint16_t weight;
> >+ uint16_t cap;
> >+ static char *kwd_list[] = { "dom", "weight", "cap", NULL };
> >+ static char kwd_type[] = "I|HH";
> >+ struct csched_domain sdom;
> >+
> >+ weight = 0;
> >+ cap = (uint16_t)~0U;
> >+ if( !PyArg_ParseTupleAndKeywords(args, kwds, kwd_type, kwd_list,
> >+ &domid, &weight, &cap) )
> >+ return NULL;
> >+
> >+ sdom.weight = weight;
> >+ sdom.cap = cap;
> >+
> >+ if ( xc_csched_domain_set(self->xc_handle, domid, &sdom) != 0 )
> >+ return PyErr_SetFromErrno(xc_error);
> >+
> >+ Py_INCREF(zero);
> >+ return zero;
> >+}
> >
>
> It's always seemed odd that we return zero here instead of Py_RETURN_NONE.
Emmanuel will simply have followed the existing practice. I agree that
there's no sense in what's there at the moment -- feel free to patch all of
these.
> >
> >+ def domain_csched_get(self, domid):
> >+ """Get credit scheduler parameters for a domain.
> >+ """
> >+ dominfo = self.domain_lookup_by_name_or_id_nr(domid)
> >+ if not dominfo:
> >+ raise XendInvalidDomain(str(domid))
> >+ try:
> >+ return xc.csched_domain_get(dominfo.getDomid())
> >+ except Exception, ex:
> >+ raise XendError(str(ex))
> >+
> >+ def domain_csched_set(self, domid, weight, cap):
> >+ """Set credit scheduler parameters for a domain.
> >+ """
> >+ dominfo = self.domain_lookup_by_name_or_id_nr(domid)
> >+ if not dominfo:
> >+ raise XendInvalidDomain(str(domid))
> >+ try:
> >+ return xc.csched_domain_set(dominfo.getDomid(), weight, cap)
> >+ except Exception, ex:
> >+ raise XendError(str(ex))
> >+
> >
>
> Please don't catch Exception. The XML-RPC now properly propagates all
> exceptions so there's no need to rewrap things in XendError. Just let
> the normal exception propagate.
Again, feel free to patch these in the existing code as well as
Emmanuel's new code.
> >diff -r b6937b931419 -r e539abd27a0f tools/python/xen/xm/main.py
> >--- a/tools/python/xen/xm/main.py Fri May 26 09:44:29 2006 +0100
> >+++ b/tools/python/xen/xm/main.py Fri May 26 11:14:36 2006 +0100
> >@@ -99,6 +99,7 @@ sched_sedf_help = "sched-sedf [DOM] [OPT
> > specifies another way of setting a
> > domain's\n\
> > cpu period/slice."
> >
> >+csched_help = "csched Set or get credit
> >scheduler parameters"
> > block_attach_help = """block-attach <DomId> <BackDev> <FrontDev> <Mode>
> > [BackDomId] Create a new virtual block device"""
> > block_detach_help = """block-detach <DomId> <DevId> Destroy a
> > domain's virtual block device,
> >@@ -174,6 +175,7 @@ host_commands = [
> > ]
> >
> > scheduler_commands = [
> >+ "csched",
> > "sched-bvt",
> > "sched-bvt-ctxallow",
> > "sched-sedf",
> >@@ -735,6 +737,48 @@ def xm_sched_sedf(args):
> > else:
> > print_sedf(sedf_info)
>
> Seem to be breaking naming convention here. sched-csched may seem
> redundant but that's what you get for choosing a non-descriptive name
> for the scheduler in the first place ;-)
sched-credit seems appropriate.
Ewan.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home