[libvirt] [PATCHv2 3/3] util: Produce friendlier error message to user

[lhuang]

On 10/21/2015 09:58 PM, John Ferlan wrote:
>
> On 10/21/2015 12:13 AM, Luyao Huang wrote:
>> Commit 1c24cfe9 fix the problem in virNumaSetPagePoolSize,
>> this patch just like it and fix the issue in another function.
>>
>> when user use freepages and specify a invalid node or page size,
>> will get error like this:
>>
>>    # virsh freepages 0 1
>>    error: Failed to open file '/sys/devices/system/node/node0/hugepages/hugepages-1kB/free_hugepages':No such file or directory
>>
>> Add two checks to catch this and therefore produce much more
>> friendlier error messages.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   src/util/virnuma.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
> Now that I see things in their final form, one more suggestion which I
> can take care of...   You've repeated the same "if (!virFileExists)"
> check and error messages for all 3 callers to virNumaGetHugePageInfo.
>
> So I think moving that check into virNumaGetHugePageInfo would now be
> appropriate (see the attached patch which I'll squash)
>
> I'll also fix up the commit messages and push some time later today

Oh, right, i should move the check in virNumaGetHugePageInfo to avoid 
code duplicated. Your patch looks good to me. And thanks a lot for your 
quick review and lots of help!

> John

Luyao

>> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
>> index 815cbc1..cd000f9 100644
>> --- a/src/util/virnuma.c
>> +++ b/src/util/virnuma.c
>> @@ -553,6 +553,25 @@ virNumaGetHugePageInfo(int node,
>>                                          page_size, "nr_hugepages") < 0)
>>               goto cleanup;
>>   
>> +        if (!virFileExists(path)) {
>> +            if (node != -1) {
>> +                if (!virNumaNodeIsAvailable(node)) {
>> +                    virReportError(VIR_ERR_OPERATION_FAILED,
>> +                                   _("NUMA node %d is not available"),
>> +                                   node);
>> +                } else {
>> +                    virReportError(VIR_ERR_OPERATION_FAILED,
>> +                                   _("page size %u is not available on node %d"),
>> +                                   page_size, node);
>> +                }
>> +            } else {
>> +                virReportError(VIR_ERR_OPERATION_FAILED,
>> +                               _("page size %u is not available"),
>> +                               page_size);
>> +            }
>> +            goto cleanup;
>> +        }
>> +
>>           if (virFileReadAll(path, 1024, &buf) < 0)
>>               goto cleanup;
>>   
>> @@ -572,6 +591,25 @@ virNumaGetHugePageInfo(int node,
>>                                          page_size, "free_hugepages") < 0)
>>               goto cleanup;
>>   
>> +        if (!virFileExists(path)) {
>> +            if (node != -1) {
>> +                if (!virNumaNodeIsAvailable(node)) {
>> +                    virReportError(VIR_ERR_OPERATION_FAILED,
>> +                                   _("NUMA node %d is not available"),
>> +                                   node);
>> +                } else {
>> +                    virReportError(VIR_ERR_OPERATION_FAILED,
>> +                                   _("page size %u is not available on node %d"),
>> +                                   page_size, node);
>> +                }
>> +            } else {
>> +                virReportError(VIR_ERR_OPERATION_FAILED,
>> +                               _("page size %u is not available"),
>> +                               page_size);
>> +            }
>> +            goto cleanup;
>> +        }
>> +
>>           if (virFileReadAll(path, 1024, &buf) < 0)
>>               goto cleanup;
>>   
>>