[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 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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