[Libvir] [PATCH] Another Report error in virsh.c code.

[Richard W.M. Jones]

On Fri, Mar 07, 2008 at 04:43:37PM +0900, S.Sakamoto wrote:
> Hi,
> 
> I am watching through the virsh code for same type bug check.
> http://git.et.redhat.com/?p=libvirt.git;a=commit;h=c857ace66df5a5068ed561aad913b29fd36160f9
> And I found another point it should report error.
> 
> Thanks,
> Shigeki Sakamoto.
> 
> 
> Index: src/virsh.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/virsh.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 virsh.c
> --- src/virsh.c	4 Mar 2008 19:59:56 -0000	1.135
> +++ src/virsh.c	7 Mar 2008 07:03:12 -0000
> @@ -1729,6 +1729,7 @@ cmdVcpupin(vshControl * ctl, vshCmd * cm
>      }
>  
>      if (!(cpulist = vshCommandOptString(cmd, "cpulist", NULL))) {
> +        vshError(ctl, FALSE, _("vcpupin: Invalid value of cpulist"));
>          virDomainFree(dom);
>          return FALSE;
>      }

Is this one necessary?  vshCommandOptString prints an error anyway
because the cpulist parameter is marked as required, ie:

  $ virsh vcpupin
  error: command 'vcpupin' requires <domain> option
  error: command 'vcpupin' requires <vcpu> option
  error: command 'vcpupin' requires <cpulist> option

> @@ -1744,6 +1745,7 @@ cmdVcpupin(vshControl * ctl, vshCmd * cm
>      }
>  
>      if (vcpu >= info.nrVirtCpu) {
> +        vshError(ctl, FALSE, _("vcpupin: Invalid vCPU number."));
>          virDomainFree(dom);
>          return FALSE;
>      }

+1

> @@ -4473,6 +4475,7 @@ cmdAttachDevice(vshControl * ctl, vshCmd
>  
>      from = vshCommandOptString(cmd, "file", &found);
>      if (!found) {
> +        vshError(ctl, FALSE, _("attach-device: Invalid value of <file> option"));
>          virDomainFree(dom);
>          return FALSE;
>      }

Again, vshCommandOptString prints an error:

  $ virsh attach-device
  error: command 'attach-device' requires <domain> option
  error: command 'attach-device' requires <file> option

> @@ -4529,6 +4532,7 @@ cmdDetachDevice(vshControl * ctl, vshCmd
>  
>      from = vshCommandOptString(cmd, "file", &found);
>      if (!found) {
> +        vshError(ctl, FALSE, _("detach-device: Invalid value of <file> option"));
>          virDomainFree(dom);
>          return FALSE;
>      }

And similarly.

Let me know if I'm missing something.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v