[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] caps: Improve error if passed an unknown arch



On 02/20/2012 05:55 PM, Eric Blake wrote:
> On 02/20/2012 11:54 AM, Cole Robinson wrote:
>> Previously we would have:
>>
>> "os type 'hvm' & arch 'idontexist' combination is not supported"
>>
>> Now we get
>>
>> "No guest options available for arch 'idontexist'"
>>
>> or if options available but guest OS type not applicable:
>>
>> "No os type 'xen' available for arch 'x86_64'"
> 
> Looks nicer.
> 
>> ---
>>  src/conf/capabilities.c |   28 ++++++++++++++++++++++++----
>>  src/conf/capabilities.h |    9 ++++++---
>>  src/conf/domain_conf.c  |   13 +++++++++++--
>>  3 files changed, 41 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index d44ce1b..542bf03 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -497,6 +497,26 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
>>      return NULL;
>>  }
>>  
>> +/**
>> + * virCapabilitiesSupportsGuestArch:
>> + * @caps: capabilities to query
>> + * @arch: Architecture to search for (eg, 'i686', 'x86_64')
>> + *
>> + * Returns non-zero if the capabilities support the
>> + * requested architecture
>> + */
>> +extern int
>> +virCapabilitiesSupportsGuestArch(virCapsPtr caps,
>> +                                 const char *arch)
>> +{
>> +    int i;
>> +    for (i = 0 ; i < caps->nguests ; i++) {
>> +        if (STREQ(caps->guests[i]->arch.name, arch))
>> +            return 1;
>> +    }
>> +    return 0;
> 
> Given how you are using this, I'd rather see it with a return type of
> 'bool', and return true or false in the method.
> 
>> @@ -529,9 +549,9 @@ virCapabilitiesSupportsGuestOSType(virCapsPtr caps,
>>   * requested operating system type
>>   */
>>  extern int
>> -virCapabilitiesSupportsGuestArch(virCapsPtr caps,
>> -                                 const char *ostype,
>> -                                 const char *arch)
>> +virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps,
>> +                                       const char *ostype,
>> +                                       const char *arch)
>>  {
> 
> Then again, you were just copying existing style, so maybe we should
> update this whole file to use bool where appropriate, which would imply
> doing it as a separate patch.
> 
> So, I guess that means I've effectively given:
> 
> ACK.
> 

Thanks, pushed (on Monday, just forgot to mail).

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]