[libvirt] [PATCH 3/3] virsh: Prohibit all clear cpumap

[Osier Yang]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list