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.