1. Patchew
  2. Libvirt
  3. View series

[PATCH] hyperv: Handle long CPU models better.

Dawid Zamirski posted 1 patch 3 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210408155105.7253-1-dzamirski@datto.com
There is a newer version of this series
src/hyperv/hyperv_driver.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
[PATCH] hyperv: Handle long CPU models better.
Posted by Dawid Zamirski 3 years ago 
Apparenlly exising code was dealing with stripping down Intel CPU models
as reported by Hyper-V host but was still having issues with my AMD CPU
for which Hyper-V returns "AMD FX(tm)-8350 Eight-Core Processor".
Therefore, this patch deals with it by stripping out the "
Processor" part, and if there's another CPU that we don't know of
yet that causes trouble, trim the resulting string to 32 characters
rather than failing as the node info has other information that are
more useful than the model.
---
 src/hyperv/hyperv_driver.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 17f5be1f0d..6e03aa4f18 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1948,14 +1948,14 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
         if (STRPREFIX(tmp, "  ")) {
             memmove(tmp, tmp + 1, strlen(tmp + 1) + 1);
             continue;
-        } else if (STRPREFIX(tmp, "(R)") || STRPREFIX(tmp, "(C)")) {
+        } else if (STRCASEPREFIX(tmp, "(R)") || STRCASEPREFIX(tmp, "(C)")) {
             memmove(tmp, tmp + 3, strlen(tmp + 3) + 1);
             continue;
-        } else if (STRPREFIX(tmp, "(TM)")) {
+        } else if (STRCASEPREFIX(tmp, "(TM)")) {
             memmove(tmp, tmp + 4, strlen(tmp + 4) + 1);
             continue;
-        } else if (STRPREFIX(tmp, " @ ")) {
-            /* Remove " @ X.YZGHz" from the end. */
+        } else if (STRPREFIX(tmp, " @ ") || STRPREFIX(tmp, " Processor")) {
+            /* Remove " @ X.YZGHz" or " Processor" from the end. */
             *tmp = '\0';
             break;
         }
@@ -1963,13 +1963,12 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
         ++tmp;
     }
 
+    /* trim whatever is left to 32 characters - better this than nothing  */
+    processorList->data->Name[31] = '\0';
+
     /* Fill struct */
-    if (virStrcpyStatic(info->model, processorList->data->Name) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("CPU model %s too long for destination"),
-                       processorList->data->Name);
+    if (virStrcpyStatic(info->model, processorList->data->Name) < 0)
         return -1;
-    }
 
     info->memory = computerSystem->data->TotalPhysicalMemory / 1024; /* byte to kilobyte */
     info->mhz = processorList->data->MaxClockSpeed;
-- 
2.31.1
Re: [PATCH] hyperv: Handle long CPU models better.
Posted by Neal Gompa 3 years ago 
On Thu, Apr 8, 2021 at 11:52 AM Dawid Zamirski <dzrudy@gmail.com> wrote:
>
> Apparenlly exising code was dealing with stripping down Intel CPU models
> as reported by Hyper-V host but was still having issues with my AMD CPU
> for which Hyper-V returns "AMD FX(tm)-8350 Eight-Core Processor".
> Therefore, this patch deals with it by stripping out the "
> Processor" part, and if there's another CPU that we don't know of
> yet that causes trouble, trim the resulting string to 32 characters
> rather than failing as the node info has other information that are
> more useful than the model.
> ---
>  src/hyperv/hyperv_driver.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 17f5be1f0d..6e03aa4f18 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1948,14 +1948,14 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
>          if (STRPREFIX(tmp, "  ")) {
>              memmove(tmp, tmp + 1, strlen(tmp + 1) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, "(R)") || STRPREFIX(tmp, "(C)")) {
> +        } else if (STRCASEPREFIX(tmp, "(R)") || STRCASEPREFIX(tmp, "(C)")) {
>              memmove(tmp, tmp + 3, strlen(tmp + 3) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, "(TM)")) {
> +        } else if (STRCASEPREFIX(tmp, "(TM)")) {
>              memmove(tmp, tmp + 4, strlen(tmp + 4) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, " @ ")) {
> -            /* Remove " @ X.YZGHz" from the end. */
> +        } else if (STRPREFIX(tmp, " @ ") || STRPREFIX(tmp, " Processor")) {
> +            /* Remove " @ X.YZGHz" or " Processor" from the end. */
>              *tmp = '\0';
>              break;
>          }
> @@ -1963,13 +1963,12 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
>          ++tmp;
>      }
>
> +    /* trim whatever is left to 32 characters - better this than nothing  */
> +    processorList->data->Name[31] = '\0';
> +
>      /* Fill struct */
> -    if (virStrcpyStatic(info->model, processorList->data->Name) < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("CPU model %s too long for destination"),
> -                       processorList->data->Name);
> +    if (virStrcpyStatic(info->model, processorList->data->Name) < 0)
>          return -1;
> -    }
>
>      info->memory = computerSystem->data->TotalPhysicalMemory / 1024; /* byte to kilobyte */
>      info->mhz = processorList->data->MaxClockSpeed;
> --
> 2.31.1
>

Looks reasonable to me.

When you resend with the Signed-off-by statement added, you can also add:

Reviewed-by: Neal Gompa <ngompa13@gmail.com>


-- 
真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH] hyperv: Handle long CPU models better.
Posted by dzrudy@gmail.com 3 years ago 
On Thu, 2021-04-08 at 11:51 -0400, Dawid Zamirski wrote:
> Apparenlly exising code was dealing with stripping down Intel CPU
> models
> as reported by Hyper-V host but was still having issues with my AMD
> CPU
> for which Hyper-V returns "AMD FX(tm)-8350 Eight-Core Processor".
> Therefore, this patch deals with it by stripping out the "
> Processor" part, and if there's another CPU that we don't know of
> yet that causes trouble, trim the resulting string to 32 characters
> rather than failing as the node info has other information that are
> more useful than the model.
> ---
>  src/hyperv/hyperv_driver.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 17f5be1f0d..6e03aa4f18 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1948,14 +1948,14 @@ hypervNodeGetInfo(virConnectPtr conn,
> virNodeInfoPtr info)
>          if (STRPREFIX(tmp, "  ")) {
>              memmove(tmp, tmp + 1, strlen(tmp + 1) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, "(R)") || STRPREFIX(tmp, "(C)")) {
> +        } else if (STRCASEPREFIX(tmp, "(R)") || STRCASEPREFIX(tmp,
> "(C)")) {
>              memmove(tmp, tmp + 3, strlen(tmp + 3) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, "(TM)")) {
> +        } else if (STRCASEPREFIX(tmp, "(TM)")) {
>              memmove(tmp, tmp + 4, strlen(tmp + 4) + 1);
>              continue;
> -        } else if (STRPREFIX(tmp, " @ ")) {
> -            /* Remove " @ X.YZGHz" from the end. */
> +        } else if (STRPREFIX(tmp, " @ ") || STRPREFIX(tmp, "
> Processor")) {
> +            /* Remove " @ X.YZGHz" or " Processor" from the end. */
>              *tmp = '\0';
>              break;
>          }
> @@ -1963,13 +1963,12 @@ hypervNodeGetInfo(virConnectPtr conn,
> virNodeInfoPtr info)
>          ++tmp;
>      }
>  
> +    /* trim whatever is left to 32 characters - better this than
> nothing  */
> +    processorList->data->Name[31] = '\0';
> +
>      /* Fill struct */
> -    if (virStrcpyStatic(info->model, processorList->data->Name) < 0)
> {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("CPU model %s too long for destination"),
> -                       processorList->data->Name);
> +    if (virStrcpyStatic(info->model, processorList->data->Name) < 0)
>          return -1;
> -    }
>  
>      info->memory = computerSystem->data->TotalPhysicalMemory / 1024;
> /* byte to kilobyte */
>      info->mhz = processorList->data->MaxClockSpeed;

Apparently, I've forgotten to add --signed-off to my commit. I'll send
V2 after initial review.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.