[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] virsh Range check in Credit Scheduler



On Fri, Jul 20, 2007 at 05:17:34PM +0900, Saori Fukuta wrote:
> The schedinfo command said strange message.
> # virsh schedinfo 25 --weight 0
> libvir: Xen error : invalid argument in xenHypervisorSetSchedulerParameters: 
> Credit scheduler weight parameter (%d) is out of range (1-65535)
>                                   ^^^^here
> # virsh schedinfo 25 --cap -1
> libvir: Xen error : invalid argument in xenHypervisorSetSchedulerParameters: 
> Credit scheduler cap parameter (%d) is out of range (0-65535)
>                                ^^^^here
> 
> I think we do not need to print the specified number, so I removed
> "(%d)" from the code to solve this problem. If you want to print the 
> specified number, I will remake this patch !

  That would be nicer, we have the information and don't pass it to the user.
It may not be a big gain when used purely from virsh but for future automatic
tuning tools it may be interesting to get the value reported back. An
snprintf to a local buffer to generate the string may be sufficient. I 
didn't applied remove_perD.patch for this reason, if you send another
patch I will fix this or I could just do it.

> And I found some wrong code when I was checking about error message.
> So I fix them, but if I have a misunderstanding of them, please tell me. :-)
>  o I guess 10th argument of __virRaiseError should be specified the number
>    of the line. But the value, for example which is number of heypercall, 
>    is sometimes specified.

  Well not really it's just  str1 - str3/int1 - int2 are just extra 
informations. This is intended to populate the virError fields to be used
for more precise error handling than the default one. 

>    xen_internal.c-673-    errmsg = __virErrorMsg(error, info);
>    xen_internal.c:674:    __virRaiseError(NULL, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
>    xen_internal.c-675-                    errmsg, info, NULL, value, 0, errmsg, info);
>    xen_internal.c-676-}
> 
>    There is same mistake at xml.c, xmlrpc.c and xend_internal.c, too.

  There is still some point of the patch which make sense, for example 
adding value at the end of the argument list in virXenError(), I will apply
some of those but not remove the tenth value.

  thanks, and sorry for the mistake in xen_internal.c

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]