[PATCH] qemu: Do not silently allow non-available timers on non-x86 systems

[Thomas Huth]

On 28/07/2020 17.13, Boris Fiuczynski wrote:
> On 7/22/20 1:21 PM, Thomas Huth wrote:
>> libvirt currently silently allows <timer name="kvmclock"/> and some
>> other timer tags in the guest XML definition for timers that do not
>> exist on non-x86 systems. We should not silently ignore these tags
>> since the users might not get what they expected otherwise.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1754887
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>   src/qemu/qemu_validate.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index 488f258d00..667ac5cc23 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -371,6 +371,18 @@ qemuValidateDomainDefClockTimers(const
>> virDomainDef *def,
>>           case VIR_DOMAIN_TIMER_NAME_TSC:
>>           case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
>>           case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
>> +            if (!ARCH_IS_X86(def->os.arch)) {
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                               _("Configuring the '%s' timer is not
>> supported "
>> +                                 "for virtType=%s arch=%s machine=%s
>> guests"),
>> +                              
>> virDomainTimerNameTypeToString(timer->name),
>> +                               virDomainVirtTypeToString(def->virtType),
>> +                               virArchToString(def->os.arch),
>> +                               def->os.machine);
>> +                return -1;
>> +            }
>> +            break;
>> +
>>           case VIR_DOMAIN_TIMER_NAME_LAST:
>>               break;
>>  
> 
> This would render previously as valid accepted domains invalid, e.g. on
> s390x using kvmclock: As long as the user does not specify the "present"
> attribute the domain starts without error since qemus cpu parameter is
> not extended.

Shall I turn it into a VIR_WARN() instead?

 Thomas