Re: [PATCH] virsh: Error prompt if one passes negative value for scheduler setting

Osier Yang <jyang@xxxxxxxxxx> · Fri, 28 Jan 2011 20:11:03 +0800

ä 2011å01æ28æ 20:02, Daniel P. Berrange åé:
On Fri, Jan 28, 2011 at 07:53:44PM +0800, Osier Yang wrote:
As cgroup doesn't allow one writes negative into files like cpu.shares,
(e.g. echo -1>  /cgroup/cpu/libvirt/qemu/rhel6/cpu.shares), user will be
confused if libvirt accepts negative value and converts it into unsigned
int (or long int, etc) silently.

* tools/virsh.c
---
  tools/virsh.c |    6 ++++--
  1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index cd54174..a0f2527 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1652,7 +1652,8 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
              }
              break;
          case VIR_DOMAIN_SCHED_FIELD_UINT:
-            if (virStrToLong_ui(val, NULL, 10,&param->value.ui)<  0) {
+            if (STRPREFIX(val, "-") ||
+                virStrToLong_ui(val, NULL, 10,&param->value.ui)<  0) {
                  vshError(ctl, "%s",
                           _("Invalid value for parameter, expecting an unsigned int"));
                  return -1;
@@ -1666,7 +1667,8 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
              }
              break;
          case VIR_DOMAIN_SCHED_FIELD_ULLONG:
-            if (virStrToLong_ull(val, NULL, 10,&param->value.ul)<  0) {
+            if (STRPREFIX(val, "-") ||
+                virStrToLong_ull(val, NULL, 10,&param->value.ul)<  0) {
                  vshError(ctl, "%s",
                           _("Invalid value for parameter, expecting an unsigned long long"));
                  return -1;

Surely this check should be done inside  virStrToLong_{ui,ul,ull}

Is there some codes that expects virStrToLong_{ui,ul,ull} to convert
the negative to {ui, ul, ull}? I was worried about it, so didn't make
the changes inside those functions, if no codes expect that, would
like make changes those functions with a v2 patch then.

Thanks
Osier

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






