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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.03.2026 11:50, Teddy Astie wrote:
> 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)).

Anthony, can you please suggest which one better fits the toolstack as a
whole?

Jan