Re: [PATCH v8 2/2] xenpm: Add get-core-temp subcommand

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Le 02/03/2026 à 17:52, Jan Beulich a écrit :
> On 27.02.2026 18:00, Teddy Astie wrote:
>> @@ -1354,6 +1358,127 @@ void enable_turbo_mode(int argc, char *argv[])
>>                   errno, strerror(errno));
>>   }
>>
>> +static int fetch_dts_temp(xc_interface *xch, uint32_t cpu, bool package, 
>> int *temp)
>> +{
>> +    xc_resource_entry_t entries[] = {
>> +        { .idx = package ? MSR_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS 
>> },
>> +        { .idx = MSR_TEMPERATURE_TARGET },
>> +    };
>> +    struct xc_resource_op ops = {
>> +        .cpu = cpu,
>> +        .entries = entries,
>> +        .nr_entries = ARRAY_SIZE(entries),
>> +    };
>> +    int tjmax;
>> +
>> +    int ret = xc_resource_op(xch, 1, &ops);
>> +
>> +    switch ( ret )
>> +    {
>> +    case 0:
>> +        /* This CPU isn't online or can't query this MSR */
>> +        return -1;
>
> Further down at the callers of this function you assume errno is set whenever
> an error indication is returned. As xc_resource_op() didn't fail, you will
> need to synthesize an errno value here.
>

ah yes indeed

>> +static void get_core_temp(int argc, char *argv[])
>> +{
>> +    int temp = -1, cpu = -1;
>> +    unsigned int socket;
>> +    bool has_data = false;
>> +
>> +    if ( argc > 0 )
>> +        parse_cpuid(argv[0], &cpu);
>> +
>> +    if ( cpu != -1 )
>> +    {
>> +        if ( !fetch_dts_temp(xc_handle, cpu, false, &temp) )
>> +            printf("CPU%d: %d°C\n", cpu, temp);
>> +        else
>> +        {
>> +            fprintf(stderr, "Unable to fetch temperature (%d - %s)\n",
>> +                    errno, strerror(errno));
>> +            printf("No data\n");
>> +            exit(ENODATA);
>
> In how far is using errno values as arguments to exit() a useful thing? (I
> think you had it like this before, and I merely forgot to ask.) Yes, I can
> see the tool using a number of exit(EINVAL), but I don't understand those
> either. This way you can't even document easily what particular exit codes
> mean, as the errno values may vary across OSes.
>

I reused the exit(...) pattern used in xenpm, but I'm also fine by
returning simpler errors (like exit(1) or exit(EXIT_FAILURE)).

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 981d3ec519..7d186c4837 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -1377,6 +1377,7 @@ static int fetch_dts_temp(xc_interface *xch,
uint32_t cpu, bool package, int *te
      {
      case 0:
          /* This CPU isn't online or can't query this MSR */
+        errno = ENODATA;
          return -1;

      case 1:
@@ -1434,7 +1435,7 @@ static void get_core_temp(int argc, char *argv[])
              fprintf(stderr, "Unable to fetch temperature (%d - %s)\n",
                      errno, strerror(errno));
              printf("No data\n");
-            exit(ENODATA);
+            exit(EXIT_FAILURE);
          }
          return;
      }
@@ -1475,7 +1476,7 @@ static void get_core_temp(int argc, char *argv[])
      if ( !has_data )
      {
          printf("No data\n");
-        exit(ENODATA);
+        exit(EXIT_FAILURE);
      }
  }

> Jan
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech