On 02/04/13 18:53, John Ferlan wrote:
On 04/02/2013 01:42 AM, Osier Yang wrote:
> This prohibits all clear cpumap eariler in virsh, for both vcpupin
> and emulatorpin.
> ---
> tools/virsh-domain.c | 35 ++++++++++++++++++++++++++++++-----
> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 965f92c..8d3d63f 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -5552,6 +5552,20 @@ cleanup:
> }
>
> static bool
> +vshIsCpumapAllClear(const unsigned char *cpumap,
> + int maxcpu)
> +{
> + int i;
> +
> + for (i = 0; i < maxcpu; i++) {
> + if (VIR_CPU_USED(cpumap, i))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool
> cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> {
> virDomainInfo info;
> @@ -5620,7 +5634,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>
> cpumaplen = VIR_CPU_MAPLEN(maxcpu);
>
> - /* Query mode: show CPU affinity information then exit.*/
> + /* Query mode: show CPU affinity information then exit. */
> if (query) {
> /* When query mode and neither "live", "config" nor
"current"
> * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */
> @@ -5649,10 +5663,15 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - /* Pin mode: pinning specified vcpu to specified physical cpus*/
> + /* Pin mode: pinning specified vcpu to specified physical cpus. */
> if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> goto cleanup;
>
> + if (vshIsCpumapAllClear(cpumap, maxcpu)) {
> + vshError(ctl, "%s", _("No CPU specified"));
> + goto cleanup;
> + }
> +
> if (flags == -1) {
> if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0)
> goto cleanup;
> @@ -5755,10 +5774,11 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
>
> cpumaplen = VIR_CPU_MAPLEN(maxcpu);
>
> - /* Query mode: show CPU affinity information then exit.*/
> + /* Query mode: show CPU affinity information then exit. */
> if (query) {
> /* When query mode and neither "live", "config" nor
"current"
> - * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */
> + * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags.
> + */
> if (flags == -1)
> flags = VIR_DOMAIN_AFFECT_CURRENT;
>
> @@ -5775,10 +5795,15 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - /* Pin mode: pinning emulator threads to specified physical cpus*/
> + /* Pin mode: pinning emulator threads to specified physical cpus. */
> if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> goto cleanup;
>
> + if (vshIsCpumapAllClear(cpumap, maxcpu)) {
> + vshError(ctl, "%s", _("No CPU is specified"));
> + goto cleanup;
> + }
> +
> if (flags == -1)
> flags = VIR_DOMAIN_AFFECT_LIVE;
>
>
Unlike the virReportError() which provides the function trace - how
would one know which of the two functions had the error here? OK other
than looking at the code and noting one error has an "is" verb and the
other doesn't...
Something like "vcpupin: No CPU is specified" should be clear
enough to tell which function throws the error. But what I was
thinking is the prefix "vcpupin:" is redundant. As one must known
he is executing a "vcpupin" command. E.g.
% virsh vcpupin toy 0 0,^0
error: No CPU specified
One message could indicate "No domain vCPU affinity" while the other
could indicate "No domain emulator affinity"...
John
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list