[libvirt PATCH 0/2] Addendum to hyperv-passthrough

Spotted by Daniel. Patches were already merged though, hence this addendum. Tim Wiederhake (2): docs: domain: Clarify on the dangers of migrating with hyperv-passthrough enabled virDomainFeaturesHyperVDefParse: Compare hyperv mode docs/formatdomain.rst | 9 ++++++++- src/conf/domain_conf.c | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 0c5e33c78f..2e9c450606 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1929,7 +1929,14 @@ are: Set exactly the specified features. ``passthrough`` - Enable all features currently supported by the hypervisor. + Enable all features currently supported by the hypervisor, even those that + libvirt does not understand. Migration of a guest using passthrough is + dangerous if the source and destination hosts are not identical in both + hardware, QEMU version, microcode version and configuration. If such a + migration is attempted then the guest may hang or crash upon resuming + execution on the destination host. Depending on hypervisor version the + virtual CPU may or may not contain features which may block migration + even to an identical host. The ``mode`` attribute can be omitted and will default to ``custom``. -- 2.31.1

On 12/14/21 17:53, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

Previous patch neglected the possibility of different modes for hyperv (e.g. "custom" and "passthrough"). Fixes: 6e83fafe331dd0b4fb19aa384c3dd36b3af62933 Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a21ac10ce..2d8851fa11 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21734,6 +21734,15 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, } /* hyperv */ + if (src->features[VIR_DOMAIN_FEATURE_HYPERV] != dst->features[VIR_DOMAIN_FEATURE_HYPERV]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV enlightenment mode differs: " + "source: '%s', destination: '%s'"), + virDomainHyperVModeTypeToString(src->features[VIR_DOMAIN_FEATURE_HYPERV]), + virDomainHyperVModeTypeToString(dst->features[VIR_DOMAIN_FEATURE_HYPERV])); + return false; + } + if (src->features[VIR_DOMAIN_FEATURE_HYPERV] != VIR_DOMAIN_HYPERV_MODE_NONE) { for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) { -- 2.31.1

On 12/14/21 17:53, Tim Wiederhake wrote:
Previous patch neglected the possibility of different modes for hyperv (e.g. "custom" and "passthrough").
Fixes: 6e83fafe331dd0b4fb19aa384c3dd36b3af62933 Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a21ac10ce..2d8851fa11 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21734,6 +21734,15 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, }
/* hyperv */ + if (src->features[VIR_DOMAIN_FEATURE_HYPERV] != dst->features[VIR_DOMAIN_FEATURE_HYPERV]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of HyperV enlightenment mode differs: " + "source: '%s', destination: '%s'"), + virDomainHyperVModeTypeToString(src->features[VIR_DOMAIN_FEATURE_HYPERV]), + virDomainHyperVModeTypeToString(dst->features[VIR_DOMAIN_FEATURE_HYPERV])); + return false; + } + if (src->features[VIR_DOMAIN_FEATURE_HYPERV] != VIR_DOMAIN_HYPERV_MODE_NONE) { for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((virDomainHyperv) i) {
I worry that this is effectively a dead code. In this function, just a couple of lines earlier we have: for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { const char *featureName = virDomainFeatureTypeToString(i); switch ((virDomainFeature) i) { ... case VIR_DOMAIN_FEATURE_HYPERV: ... if (src->features[i] != dst->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%s' differs: " "source: '%s', destination: '%s'"), featureName, .... return false; } break; So if VIR_DOMAIN_FEATURE_HYPERV is not the same on src and dst we will never get to this code you're adding. Michal
participants (2)
-
Michal Prívozník
-
Tim Wiederhake