> 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. -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list