[libvirt] [PATCH] host-validate: Improve CPU flags processing

[Peter Krempa]

On Tue, Mar 29, 2016 at 18:09:44 +0200, Andrea Bolognani wrote:
> Instead of relying on substring search, tokenize the input
> and process each CPU flag separately. This ensures CPU flag
> detection will continue to work correctly even if we start
> looking for CPU flags whose name might appear as part of
> other CPU flags' names.
> 
> The result of processing is stored in a virBitmap, which
> means we don't have to parse /proc/cpuinfo in its entirety
> for each single CPU flag we want to check.
> 
> Moreover, use of the newly-introduced virHostValidateCPUFlag
> enumeration ensures we don't go looking for random CPU flags
> which might actually be simple typos.
> ---
> The concern was raised by John in
> 
>   https://www.redhat.com/archives/libvir-list/2016-March/msg01301.html
> 
> when discussing a new check on the "sie" CPU flag.
> 
>  tools/virt-host-validate-common.c | 50 +++++++++++++++++++++++++++++----------
>  tools/virt-host-validate-common.h | 13 +++++++++-
>  tools/virt-host-validate-qemu.c   | 12 ++++++++--
>  3 files changed, 60 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
> index 8ebf73e..57fa332 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c

[...]

> @@ -188,29 +191,47 @@ int virHostValidateNamespace(const char *hvname,
>  }
>  
>  
> -bool virHostValidateHasCPUFlag(const char *name)
> +virBitmapPtr virHostValidateGetCPUFlags(void)
>  {
> -    FILE *fp = fopen("/proc/cpuinfo", "r");
> -    bool ret = false;
> +    FILE *fp;
> +    virBitmapPtr flags;
>  
> -    if (!fp)
> -        return false;
> +    if (!(fp = fopen("/proc/cpuinfo", "r")))
> +        return NULL;
> +
> +    if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST)))

Since you are already using libvirt's utils ...

> +        return NULL;
>  
>      do {
>          char line[1024];
> +        char *saveptr;
> +        char *token;
>  
>          if (!fgets(line, sizeof(line), fp))
>              break;
>  
> -        if (strstr(line, name)) {
> -            ret = true;
> -            break;
> +        if (!(token = strtok_r(line, " \t\n", &saveptr)))

Why not virStringSplit ...

> +            continue;
> +
> +        /* The line we're interested in is marked either as "flags" or
> +         * as "Features" depending on the architecture, so check both
> +         * prefixes. It's very unlikely that there will be no whitespace
> +         * between the line name and the colon, but handle that as well */
> +        if (strcmp(token, "flags") && strcmp(token, "flags:") &&

And STREQ?

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160330/8f1c2346/attachment-0001.sig>