8:31 a.m.
Pavel Hrdina (3):
docs: fix qemu version for hyperv features
qemu_process: skip only cpu features
qemu_process: add check for hyperv features
docs/formatdomain.html.in | 4 ++--
src/cpu/cpu_x86.c | 8 +++++++
src/cpu/cpu_x86_data.h | 8 +++++++
src/qemu/qemu_process.c | 57 +++++++++++++++++++++++++++++++++++------------
4 files changed, 61 insertions(+), 16 deletions(-)
--
2.7.4
8:31 a.m.
New subject: [libvirt] [PATCH 1/3] docs: fix qemu version for hyperv features
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
docs/formatdomain.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a1d7c4a..957b839 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1558,13 +1558,13 @@
<td>synic</td>
<td>Enable Synthetic Interrupt Controller (SyNIC)</td>
<td> on, off</td>
- <td><span class="since">1.3.3 (QEMU
2.5)</span></td>
+ <td><span class="since">1.3.3 (QEMU
2.6)</span></td>
</tr>
<tr>
<td>stimer</td>
<td>Enable SyNIC timers</td>
<td> on, off</td>
- <td><span class="since">1.3.3 (QEMU
2.5)</span></td>
+ <td><span class="since">1.3.3 (QEMU
2.6)</span></td>
</tr>
<tr>
<td>reset</td>
--
2.7.4
9:45 a.m.
New subject: [libvirt] [PATCH 1/3] docs: fix qemu version for hyperv features
On 03/29/2016 09:31 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
docs/formatdomain.html.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK -
Read that one wrong yesterday
John
8:31 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_process: skip only cpu features
This check is there to allow restore saved domain with older libvirt
where we included invtsc by default for host-passthrough model. Don't
skip the whole function, but only the part that checks for invtsc.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_process.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 205d9ae..9334a75 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3904,11 +3904,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
bool ret = false;
size_t i;
- /* no features are passed to QEMU with -cpu host
- * so it makes no sense to verify them */
- if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
- return true;
-
switch (arch) {
case VIR_ARCH_I686:
case VIR_ARCH_X86_64:
@@ -3933,17 +3928,20 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
}
- for (i = 0; def->cpu && i < def->cpu->nfeatures; i++) {
- virCPUFeatureDefPtr feature = &def->cpu->features[i];
- if (feature->policy != VIR_CPU_FEATURE_REQUIRE)
- continue;
+ if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH)
{
+ for (i = 0; i < def->cpu->nfeatures; i++) {
+ virCPUFeatureDefPtr feature = &def->cpu->features[i];
- if (STREQ(feature->name, "invtsc") &&
- !cpuHasFeature(guestcpu, feature->name)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("host doesn't support invariant TSC"));
- goto cleanup;
+ if (feature->policy != VIR_CPU_FEATURE_REQUIRE)
+ continue;
+
+ if (STREQ(feature->name, "invtsc") &&
+ !cpuHasFeature(guestcpu, feature->name)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("host doesn't support invariant
TSC"));
+ goto cleanup;
+ }
}
}
break;
--
2.7.4
8:31 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
Commit 7068b56c introduced several hyperv features. Not all hyperv
features are supported by old enough kernels and we shouldn't allow to
start a guest if kernel doesn't support any of the hyperv feature.
There is one exception, for backward compatibility we cannot error out
if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
reason we ignore invtsc, to not break restoring saved domains with older
libvirt.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/cpu/cpu_x86.c | 8 ++++++++
src/cpu/cpu_x86_data.h | 8 ++++++++
src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 90949f6..b7f1690 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] =
{VIR_CPU_x86_KVM_PV_UNHALT, { .function = 0x40000001, .eax = 0x00000080 }},
{VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
{ .function = 0x40000001, .eax = 0x01000000 }},
+ {VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x40000003, .eax = 0x00000001 }},
+ {VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x40000003, .eax = 0x00000004 }},
+ {VIR_CPU_x86_KVM_HV_STIMER, { .function = 0x40000003, .eax = 0x00000008 }},
+ {VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x40000003, .eax = 0x00000020 }},
+ {VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x40000003, .eax = 0x00000022 }},
+ {VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x40000003, .eax = 0x00000030 }},
+ {VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x40000003, .eax = 0x00000040 }},
+ {VIR_CPU_x86_KVM_HV_RESET, { .function = 0x40000003, .eax = 0x00000080 }},
};
struct x86_model {
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 88dccf6..777cc8d 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -48,6 +48,14 @@ struct _virCPUx86CPUID {
# define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi"
# define VIR_CPU_x86_KVM_PV_UNHALT "__kvm_pv_unhalt"
# define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
+# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime"
+# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
+# define VIR_CPU_x86_KVM_HV_STIMER "__kvm_hv_stimer"
+# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed"
+# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
+# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex"
+# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"
typedef struct _virCPUx86Data virCPUx86Data;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9334a75..07b9df2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
}
+ for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
+ if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) {
+ char *cpuFeature;
+ if (virAsprintf(&cpuFeature, "__kvm_hv_%s",
+ virDomainHypervTypeToString(i)) < 0)
+ goto cleanup;
+ if (!cpuHasFeature(guestcpu, cpuFeature)) {
+ switch ((virDomainHyperv) i) {
+ case VIR_DOMAIN_HYPERV_RELAXED:
+ case VIR_DOMAIN_HYPERV_VAPIC:
+ case VIR_DOMAIN_HYPERV_SPINLOCKS:
+ VIR_WARN("host doesn't support hyperv '%s'
feature",
+ virDomainHypervTypeToString(i));
+ break;
+ case VIR_DOMAIN_HYPERV_VPINDEX:
+ case VIR_DOMAIN_HYPERV_RUNTIME:
+ case VIR_DOMAIN_HYPERV_SYNIC:
+ case VIR_DOMAIN_HYPERV_STIMER:
+ case VIR_DOMAIN_HYPERV_RESET:
+ case VIR_DOMAIN_HYPERV_VENDOR_ID:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("host doesn't support hyperv
'%s' feature"),
+ virDomainHypervTypeToString(i));
+ goto cleanup;
+ break;
+ case VIR_DOMAIN_HYPERV_LAST:
+ break;
+ }
+ }
+ }
+ }
if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH)
{
for (i = 0; i < def->cpu->nfeatures; i++) {
--
2.7.4
10:10 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On 03/29/2016 09:31 AM, Pavel Hrdina wrote:
Commit 7068b56c introduced several hyperv features. Not all hyperv
features are supported by old enough kernels and we shouldn't allow to
start a guest if kernel doesn't support any of the hyperv feature.
There is one exception, for backward compatibility we cannot error out
if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
reason we ignore invtsc, to not break restoring saved domains with older
libvirt.
From yesterday's dialog, there's also commit id
'59fc0d06' which adds
"hv_crash" and commit id '600bca59'
which adds "hv_time".
Neither is handled via these bits, but wouldn't both fall into the same
trap since both were added after commit '2e8f9080'?
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/cpu/cpu_x86.c | 8 ++++++++
src/cpu/cpu_x86_data.h | 8 ++++++++
src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
Now I was going to ask about a capability bit for this, but seeing none
in previous commits, I thought I was safe... I guess not.
The rest looks OK to me, although I wonder if there's a way to avoid
missing this for future changes
John
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 90949f6..b7f1690 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] =
{VIR_CPU_x86_KVM_PV_UNHALT, { .function = 0x40000001, .eax = 0x00000080 }},
{VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
{ .function = 0x40000001, .eax = 0x01000000 }},
+ {VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x40000003, .eax = 0x00000001 }},
+ {VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x40000003, .eax = 0x00000004 }},
+ {VIR_CPU_x86_KVM_HV_STIMER, { .function = 0x40000003, .eax = 0x00000008 }},
+ {VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x40000003, .eax = 0x00000020 }},
+ {VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x40000003, .eax = 0x00000022 }},
+ {VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x40000003, .eax = 0x00000030 }},
+ {VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x40000003, .eax = 0x00000040 }},
+ {VIR_CPU_x86_KVM_HV_RESET, { .function = 0x40000003, .eax = 0x00000080 }},
};
struct x86_model {
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 88dccf6..777cc8d 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -48,6 +48,14 @@ struct _virCPUx86CPUID {
# define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi"
# define VIR_CPU_x86_KVM_PV_UNHALT "__kvm_pv_unhalt"
# define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
+# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime"
+# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
+# define VIR_CPU_x86_KVM_HV_STIMER "__kvm_hv_stimer"
+# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed"
+# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
+# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex"
+# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"
typedef struct _virCPUx86Data virCPUx86Data;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9334a75..07b9df2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
}
+ for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
+ if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) {
+ char *cpuFeature;
+ if (virAsprintf(&cpuFeature, "__kvm_hv_%s",
+ virDomainHypervTypeToString(i)) < 0)
+ goto cleanup;
+ if (!cpuHasFeature(guestcpu, cpuFeature)) {
+ switch ((virDomainHyperv) i) {
+ case VIR_DOMAIN_HYPERV_RELAXED:
+ case VIR_DOMAIN_HYPERV_VAPIC:
+ case VIR_DOMAIN_HYPERV_SPINLOCKS:
+ VIR_WARN("host doesn't support hyperv '%s'
feature",
+ virDomainHypervTypeToString(i));
+ break;
+ case VIR_DOMAIN_HYPERV_VPINDEX:
+ case VIR_DOMAIN_HYPERV_RUNTIME:
+ case VIR_DOMAIN_HYPERV_SYNIC:
+ case VIR_DOMAIN_HYPERV_STIMER:
+ case VIR_DOMAIN_HYPERV_RESET:
+ case VIR_DOMAIN_HYPERV_VENDOR_ID:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("host doesn't support hyperv
'%s' feature"),
+ virDomainHypervTypeToString(i));
+ goto cleanup;
+ break;
+ case VIR_DOMAIN_HYPERV_LAST:
+ break;
+ }
+ }
+ }
+ }
if (def->cpu && def->cpu->mode !=
VIR_CPU_MODE_HOST_PASSTHROUGH) {
for (i = 0; i < def->cpu->nfeatures; i++) {
10:31 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On Tue, Mar 29, 2016 at 11:10:42AM -0400, John Ferlan wrote:
On 03/29/2016 09:31 AM, Pavel Hrdina wrote:
> Commit 7068b56c introduced several hyperv features. Not all hyperv
> features are supported by old enough kernels and we shouldn't allow to
> start a guest if kernel doesn't support any of the hyperv feature.
>
> There is one exception, for backward compatibility we cannot error out
> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> reason we ignore invtsc, to not break restoring saved domains with older
> libvirt.
From yesterday's dialog, there's also commit id '59fc0d06' which adds
"hv_crash" and commit id '600bca59' which adds "hv_time".
Neither is handled via these bits, but wouldn't both fall into the same
trap since both were added after commit '2e8f9080'?
Even though libvirt passes those options to QEMU in the same -cpu argument, they
are unrelated to this patch series. And for the same reason as for RELAXED,
VAPIC and SPINLOCKS, we cannot error out to not break things so there is nothing
to do about it.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/cpu/cpu_x86.c | 8 ++++++++
> src/cpu/cpu_x86_data.h | 8 ++++++++
> src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
>
Now I was going to ask about a capability bit for this, but seeing none
in previous commits, I thought I was safe... I guess not.
I don't know what you mean by this, what capability?
Pavel
10:36 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
[...]
>
> Now I was going to ask about a capability bit for this, but seeing none
> in previous commits, I thought I was safe... I guess not.
I don't know what you mean by this, what capability?
Sorry - context was I was thinking about the code I looked at yesterday
that didn't have any capability checking for the bits in previous
commits for hyperv feature bits. Not this patch.
John
11:06 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
29.03.2016 16:31, Pavel Hrdina пишет:
Commit 7068b56c introduced several hyperv features. Not all hyperv
features are supported by old enough kernels and we shouldn't allow to
start a guest if kernel doesn't support any of the hyperv feature.
There is one exception, for backward compatibility we cannot error out
if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
reason we ignore invtsc, to not break restoring saved domains with older
libvirt.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/cpu/cpu_x86.c | 8 ++++++++
src/cpu/cpu_x86_data.h | 8 ++++++++
src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 90949f6..b7f1690 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] =
{VIR_CPU_x86_KVM_PV_UNHALT, { .function = 0x40000001, .eax = 0x00000080 }},
{VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
{ .function = 0x40000001, .eax = 0x01000000 }},
+ {VIR_CPU_x86_KVM_HV_RUNTIME, { .function = 0x40000003, .eax = 0x00000001 }},
+ {VIR_CPU_x86_KVM_HV_SYNIC, { .function = 0x40000003, .eax = 0x00000004 }},
+ {VIR_CPU_x86_KVM_HV_STIMER, { .function = 0x40000003, .eax = 0x00000008 }},
+ {VIR_CPU_x86_KVM_HV_RELAXED, { .function = 0x40000003, .eax = 0x00000020 }},
+ {VIR_CPU_x86_KVM_HV_SPINLOCK, { .function = 0x40000003, .eax = 0x00000022 }},
+ {VIR_CPU_x86_KVM_HV_VAPIC, { .function = 0x40000003, .eax = 0x00000030 }},
+ {VIR_CPU_x86_KVM_HV_VPINDEX, { .function = 0x40000003, .eax = 0x00000040 }},
+ {VIR_CPU_x86_KVM_HV_RESET, { .function = 0x40000003, .eax = 0x00000080 }},
};
struct x86_model {
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 88dccf6..777cc8d 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -48,6 +48,14 @@ struct _virCPUx86CPUID {
# define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi"
# define VIR_CPU_x86_KVM_PV_UNHALT "__kvm_pv_unhalt"
# define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
+# define VIR_CPU_x86_KVM_HV_RUNTIME "__kvm_hv_runtime"
+# define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
+# define VIR_CPU_x86_KVM_HV_STIMER "__kvm_hv_stimer"
+# define VIR_CPU_x86_KVM_HV_RELAXED "__kvm_hv_relaxed"
+# define VIR_CPU_x86_KVM_HV_SPINLOCK "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
+# define VIR_CPU_x86_KVM_HV_VPINDEX "__kvm_hv_vpindex"
+# define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"
typedef struct _virCPUx86Data virCPUx86Data;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9334a75..07b9df2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
}
+ for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
+ if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) {
+ char *cpuFeature;
+ if (virAsprintf(&cpuFeature, "__kvm_hv_%s",
+ virDomainHypervTypeToString(i)) < 0)
+ goto cleanup;
+ if (!cpuHasFeature(guestcpu, cpuFeature)) {
+ switch ((virDomainHyperv) i) {
+ case VIR_DOMAIN_HYPERV_RELAXED:
+ case VIR_DOMAIN_HYPERV_VAPIC:
+ case VIR_DOMAIN_HYPERV_SPINLOCKS:
+ VIR_WARN("host doesn't support hyperv '%s'
feature",
+ virDomainHypervTypeToString(i));
+ break;
+ case VIR_DOMAIN_HYPERV_VPINDEX:
+ case VIR_DOMAIN_HYPERV_RUNTIME:
+ case VIR_DOMAIN_HYPERV_SYNIC:
+ case VIR_DOMAIN_HYPERV_STIMER:
+ case VIR_DOMAIN_HYPERV_RESET:
+ case VIR_DOMAIN_HYPERV_VENDOR_ID:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("host doesn't support hyperv
'%s' feature"),
+ virDomainHypervTypeToString(i));
+ goto cleanup;
+ break;
+ case VIR_DOMAIN_HYPERV_LAST:
+ break;
+ }
+ }
+ }
+ }
if (def->cpu && def->cpu->mode !=
VIR_CPU_MODE_HOST_PASSTHROUGH) {
for (i = 0; i < def->cpu->nfeatures; i++) {
Hmm, qemu already checks them and simply ignores most of them and
doesn't prevent guest from starting in case they are not supported and
optional. In case they are reqired it fails. Why should we check them
here? At least we should follow the logic qemu has.
Extract from target-i386/kvm.c does the following:
if (cpu->hyperv_relaxed_timing) {
c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
}
if (cpu->hyperv_vapic) {
c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
has_msr_hv_vapic = true;
}
...
if (cpu->hyperv_crash && has_msr_hv_crash) {
c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
}
if (cpu->hyperv_reset && has_msr_hv_reset) {
c->eax |= HV_X64_MSR_RESET_AVAILABLE;
}
if (cpu->hyperv_vpindex && has_msr_hv_vpindex) {
c->eax |= HV_X64_MSR_VP_INDEX_AVAILABLE;
}
if (cpu->hyperv_runtime && has_msr_hv_runtime) {
c->eax |= HV_X64_MSR_VP_RUNTIME_AVAILABLE;
}
thus, features hv_vapic, hv_relaxed, hv_reset, hv_runtime, hv_crash,
hv_vpindex are ignored if not supported. Only absence of hv_synic,
hv_stimer leads to error.
Maxim
2:50 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:
29.03.2016 16:31, Pavel Hrdina пишет:
> Commit 7068b56c introduced several hyperv features. Not all hyperv
> features are supported by old enough kernels and we shouldn't allow to
> start a guest if kernel doesn't support any of the hyperv feature.
>
> There is one exception, for backward compatibility we cannot error out
> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> reason we ignore invtsc, to not break restoring saved domains with older
> libvirt.
>
[...]
Hmm, qemu already checks them and simply ignores most of them and
doesn't prevent guest from starting in case they are not supported and
optional. In case they are reqired it fails. Why should we check them
here? At least we should follow the logic qemu has.
Yes, that's true that QEMU do some checks and ignores most of the features
missing in kernel, but that's no reason why we should do the same. Libvirt
tries to present in domain XML only those features and devices that are actually
present in the guest. Thus if you tell libvirt that you want some hyperv
feature but your host kernel doesn't support it, we should let the user know
that this feature isn't supported instead of ignoring that fact and start the
guest anyway.
Pavel
4:05 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
30.03.2016 10:50, Pavel Hrdina пишет:
On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:
> 29.03.2016 16:31, Pavel Hrdina пишет:
>> Commit 7068b56c introduced several hyperv features. Not all hyperv
>> features are supported by old enough kernels and we shouldn't allow to
>> start a guest if kernel doesn't support any of the hyperv feature.
>>
>> There is one exception, for backward compatibility we cannot error out
>> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
>> reason we ignore invtsc, to not break restoring saved domains with older
>> libvirt.
>>
[...]
> Hmm, qemu already checks them and simply ignores most of them and
> doesn't prevent guest from starting in case they are not supported and
> optional. In case they are reqired it fails. Why should we check them
> here? At least we should follow the logic qemu has.
Yes, that's true that QEMU do some checks and ignores most of the features
missing in kernel, but that's no reason why we should do the same. Libvirt
tries to present in domain XML only those features and devices that are actually
present in the guest. Thus if you tell libvirt that you want some hyperv
feature but your host kernel doesn't support it, we should let the user know
that this feature isn't supported instead of ignoring that fact and start the
guest anyway.
Pavel
I see your point. And what if a user wants to define some features just
to ask libvirt/qemu to do its best if possible and ignore if it isn't
without redefining domain xml? Does such case make sense?
Maxim
5:07 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: add check for hyperv features
On Wed, Mar 30, 2016 at 12:05:28PM +0300, Maxim Nestratov wrote:
30.03.2016 10:50, Pavel Hrdina пишет:
> On Tue, Mar 29, 2016 at 07:06:14PM +0300, Maxim Nestratov wrote:
>> 29.03.2016 16:31, Pavel Hrdina пишет:
>>> Commit 7068b56c introduced several hyperv features. Not all hyperv
>>> features are supported by old enough kernels and we shouldn't allow to
>>> start a guest if kernel doesn't support any of the hyperv feature.
>>>
>>> There is one exception, for backward compatibility we cannot error out
>>> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
>>> reason we ignore invtsc, to not break restoring saved domains with older
>>> libvirt.
>>>
> [...]
>
>> Hmm, qemu already checks them and simply ignores most of them and
>> doesn't prevent guest from starting in case they are not supported and
>> optional. In case they are reqired it fails. Why should we check them
>> here? At least we should follow the logic qemu has.
> Yes, that's true that QEMU do some checks and ignores most of the features
> missing in kernel, but that's no reason why we should do the same. Libvirt
> tries to present in domain XML only those features and devices that are actually
> present in the guest. Thus if you tell libvirt that you want some hyperv
> feature but your host kernel doesn't support it, we should let the user know
> that this feature isn't supported instead of ignoring that fact and start the
> guest anyway.
>
> Pavel
I see your point. And what if a user wants to define some features just
to ask libvirt/qemu to do its best if possible and ignore if it isn't
without redefining domain xml? Does such case make sense?
Maxim
Sure, this makes sense and we do this for cpu features [1], for example:
<feature policy='optional' name='lahf_lm'/>
But I'm not that sure whether we want to do this also for guest features like
those hyperv features. If your guest OS is Windows you probably want to know
what features are supported and to be sure, that all of the specified features
are enabled.
[1] http://libvirt.org/formatdomain.html#elementsCPU
Pavel
10:03 a.m.
On Tue, Mar 29, 2016 at 15:31:14 +0200, Pavel Hrdina wrote:
Pavel Hrdina (3):
docs: fix qemu version for hyperv features
qemu_process: skip only cpu features
qemu_process: add check for hyperv features
docs/formatdomain.html.in | 4 ++--
src/cpu/cpu_x86.c | 8 +++++++
src/cpu/cpu_x86_data.h | 8 +++++++
src/qemu/qemu_process.c | 57 +++++++++++++++++++++++++++++++++++------------
4 files changed, 61 insertions(+), 16 deletions(-)
ACK series
Jirka
3189
days inactive
3190
days old
12 comments
4 participants
participants (4)
-
Jiri Denemark -
John Ferlan -
Maxim Nestratov -
Pavel Hrdina