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@xxxxxxxx>
---
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)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
