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

[S.Sakamoto]

> 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
> 
> > @@ -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.
> 
Surely, the message of "Invalid value" is not pertinent since
the validity of value is checked at vshCommandCheckOpts and shown
the message there.

BTW, I think another message is needed here to inform the internal
error to user.
For example, the migrate command shows the following message when
"desturi" is missing:
    migrate: Missing desturi
But it does not show the error-message even though "migrateuri"
is missing because "migrateuri" is *not* a necessary option.

So, I want to unify "virsh.c" with the following rules,
  - for necessary option
    show the message if error occur
  - for unnecessary option
    do not show the message even if error occur

How do you think ?


Regards,
Shigeki Sakamoto.