[libvirt] [PATCH 3/4] xenconfig: add support for more timers
Jim Fehlig
jfehlig at suse.com
Fri Jan 20 22:17:06 UTC 2017
On 01/20/2017 05:00 AM, Joao Martins wrote:
> On 01/20/2017 01:59 AM, Jim Fehlig wrote:
>> Currently xenconfig only supports the hpet timer for HVM domains.
>> Include support for tsc timer for both PV and HVM domains.
>>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>> src/xenconfig/xen_common.c | 87 ++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 77 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
>> index 7968d40..ad596d7 100644
>> --- a/src/xenconfig/xen_common.c
>> +++ b/src/xenconfig/xen_common.c
>> @@ -490,6 +490,7 @@ xenParseCPUFeatures(virConfPtr conf,
>> unsigned long count = 0;
>> const char *str = NULL;
>> int val = 0;
>> + virDomainTimerDefPtr timer;
>>
>> if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0)
>> return -1;
>> @@ -514,6 +515,29 @@ xenParseCPUFeatures(virConfPtr conf,
>> if (str && (virBitmapParse(str, &def->cpumask, 4096) < 0))
>> return -1;
>>
>> + if (xenConfigGetString(conf, "tsc_mode", &str, NULL) < 0)
>> + return -1;
>> +
>> + if (str) {
>> + if (VIR_EXPAND_N(def->clock.timers, def->clock.ntimers, 1) < 0 ||
>> + VIR_ALLOC(timer) < 0)
>> + return -1;
>> +
>> + timer->name = VIR_DOMAIN_TIMER_NAME_TSC;
>> + timer->present = 1;
>> + timer->tickpolicy = -1;
>> + timer->mode = VIR_DOMAIN_TIMER_MODE_AUTO;
>> + timer->track = -1;
>> + if (STREQ_NULLABLE(str, "always_emulate"))
>> + timer->mode = VIR_DOMAIN_TIMER_MODE_EMULATE;
>> + else if (STREQ_NULLABLE(str, "native"))
>> + timer->mode = VIR_DOMAIN_TIMER_MODE_NATIVE;
>> + else if (STREQ_NULLABLE(str, "native_paravirt"))
>> + timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
>> +
>> + def->clock.timers[def->clock.ntimers - 1] = timer;
>> + }
>> +
>> if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
>> if (xenConfigGetBool(conf, "pae", &val, 1) < 0)
>> return -1;
>> @@ -545,9 +569,7 @@ xenParseCPUFeatures(virConfPtr conf,
>> return -1;
>>
>> if (val != -1) {
>> - virDomainTimerDefPtr timer;
>> -
>> - if (VIR_ALLOC_N(def->clock.timers, 1) < 0 ||
>> + if (VIR_EXPAND_N(def->clock.timers, def->clock.ntimers, 1) < 0 ||
>> VIR_ALLOC(timer) < 0)
>> return -1;
>>
>> @@ -557,8 +579,7 @@ xenParseCPUFeatures(virConfPtr conf,
>> timer->mode = -1;
>> timer->track = -1;
>>
>> - def->clock.ntimers = 1;
>> - def->clock.timers[0] = timer;
>> + def->clock.timers[def->clock.ntimers - 1] = timer;
>> }
>> }
>>
>> @@ -1584,8 +1605,9 @@ static int
>> xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def)
>> {
>> size_t i;
>> + int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? 1 : 0;
> You can probably avoid the ternary expression style with turning this into a
> bool and do !!(def->os.type == VIR_DOMAIN_OSTYPE_HVM) or simply just
> (def->os.type == VIR_DOMAIN_OSTYPE_HVM).
I think this is the preferred style.
> Or is it that libvirt style prefers
> this way? Looking at the file it looks like the style you used above is kept
> throughout this file.
I guess a bad habit was started in this file and grew though copy and past :-).
>
>>
>> - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
>> + if (hvm) {
>> if (xenConfigSetInt(conf, "pae",
>> (def->features[VIR_DOMAIN_FEATURE_PAE] ==
>> VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0)
>> @@ -1610,12 +1632,57 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def)
>> (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] ==
>> VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0)
>> return -1;
>> + }
>>
>> - for (i = 0; i < def->clock.ntimers; i++) {
>> - if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET &&
>> - def->clock.timers[i]->present != -1 &&
>> - xenConfigSetInt(conf, "hpet", def->clock.timers[i]->present) < 0)
>> + for (i = 0; i < def->clock.ntimers; i++) {
>> + switch ((virDomainTimerNameType) def->clock.timers[i]->name) {
>> + case VIR_DOMAIN_TIMER_NAME_TSC:
>> + switch (def->clock.timers[i]->mode) {
>> + case VIR_DOMAIN_TIMER_MODE_NATIVE:
>> + if (xenConfigSetString(conf, "tsc_mode", "native") < 0)
>> + return -1;
>> + break;
>> + case VIR_DOMAIN_TIMER_MODE_PARAVIRT:
>> + if (xenConfigSetString(conf, "tsc_mode", "native_paravirt") < 0)
>> + return -1;
>> + break;
>> + case VIR_DOMAIN_TIMER_MODE_EMULATE:
>> + if (xenConfigSetString(conf, "tsc_mode", "always_emulate") < 0)
>> + return -1;
>> + break;
>> + default:
>> + if (xenConfigSetString(conf, "tsc_mode", "default") < 0)
>> + return -1;
>> + }
>> + break;
>> +
>> + case VIR_DOMAIN_TIMER_NAME_HPET:
>> + if (hvm) {
>> + int enable_hpet = def->clock.timers[i]->present == 0 ? 0 : 1;
> Same here i.e. (def->clock.timers[i]->present != 0) ?
Yep. I'll leave this as an int since it is used in xenConfigSetInt.
>
>> +
>> + /* disable hpet if 'present' is 0, enable otherwise */
>> + if (xenConfigSetInt(conf, "hpet", enable_hpet) < 0)
>> + return -1;
>> + } else {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported timer type (name) '%s'"),
>> + virDomainTimerNameTypeToString(def->clock.timers[i]->name));
>> return -1;
>> + }
>> + break;
>> +
>> + case VIR_DOMAIN_TIMER_NAME_PLATFORM:
>> + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
>> + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
>> + case VIR_DOMAIN_TIMER_NAME_RTC:
>> + case VIR_DOMAIN_TIMER_NAME_PIT:
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported timer type (name) '%s'"),
>> + virDomainTimerNameTypeToString(def->clock.timers[i]->name));
>> + return -1;
>> +
>> + case VIR_DOMAIN_TIMER_NAME_LAST:
>> + break;
>> }
>> }
I've squashed the below diff into my branch. Do you have any additional comments
on this series?
Regards,
Jim
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index ad596d7..72ffb30 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -1605,7 +1605,7 @@ static int
xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def)
{
size_t i;
- int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? 1 : 0;
+ bool hvm = !!(def->os.type == VIR_DOMAIN_OSTYPE_HVM);
if (hvm) {
if (xenConfigSetInt(conf, "pae",
@@ -1658,7 +1658,7 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def)
case VIR_DOMAIN_TIMER_NAME_HPET:
if (hvm) {
- int enable_hpet = def->clock.timers[i]->present == 0 ? 0 : 1;
+ int enable_hpet = def->clock.timers[i]->present != 0;
/* disable hpet if 'present' is 0, enable otherwise */
if (xenConfigSetInt(conf, "hpet", enable_hpet) < 0)
More information about the libvir-list
mailing list