1:48 a.m.
On Thursday, 1 October 2020 23:47:16 CEST mcoleman(a)datto.com wrote:
From: Matt Coleman <matt(a)datto.com>
Hyper-V version numbers are not compatible with the encoding in
virParseVersionString():
https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virutil.c#L246
For example, the Windows Server 2016 Hyper-V version is 10.0.14393: its
micro is over 14 times larger than the encoding allows.
This commit repacks the Hyper-V version number in order to preserve all
of the digits. The major and minor are concatenated (with minor zero-
padded to two digits) to form the repacked major value. This works
because Microsoft's major and minor versions numbers are unlikely to
exceed 99. The repacked minor value is derived from the digits in the
thousands, ten-thousands, and hundred-thousands places of Hyper-V's
micro. The repacked micro is derived from the digits in the ones, tens,
and hundreds places of Hyper-V's micro.
[...]
+ /*
+ * Mangle the version to work with virParseVersionString() while retaining all the
digits.
+ *
+ * For example...
+ * 2008: 6.0.6001 => 600.6.1
+ * 2008 R2: 6.1.7600 => 601.7.600
+ * 2012: 6.2.9200 => 602.9.200
+ * 2012 R2: 6.3.9600 => 603.9.600
+ * 2016: 10.0.14393 => 1000.14.393
+ * 2019: 10.0.17763 => 1000.17.763
+ */
IMHO these explanations should be documented in the Hyper-V driver
page: docs/drvhyperv.html.in. This way, users know about the different
value returned by virConnectGetVersion() by the Hyper-V driver, and
can unmangle it to get the real Hyper-V version.
+ if (virStrToLong_ui(os->data.common->Version, &suffix,
10, &major) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not parse major from '%s'"),
+ os->data.common->Version);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ui(suffix + 1, &suffix, 10, &minor) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not parse minor from '%s'"),
+ os->data.common->Version);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ui(suffix + 1, NULL, 10, µ) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not parse micro from '%s'"),
+ os->data.common->Version);
+ goto cleanup;
+ }
+
+ if (major > 99 || minor > 99 || micro > 999999) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not produce mangled version number from
'%s'"),
+ os->data.common->Version);
+ goto cleanup;
+ }
I'd move the parsing code to an own helper, similar to
virParseVersionString():
int
hypervParseVersionString(const char *str, unsigned int *major,
unsigned int *minor, unsigned int *micro)
without virReportError() in it; then use a single virReportError() for
hypervParseVersionString() failure.
+
+ virBufferAsprintf(&mangled_version, "%d%02d.%d.%d", major, minor,
+ micro > 999 ? micro / 1000 : 0,
+ micro > 999 ? micro % 1000 : micro);
+
+ /* Parse version string to long */
+ if (virParseVersionString(virBufferCurrentContent(&mangled_version), version,
true) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not parse version number from '%s'"),
+ os->data.common->Version);
+ goto cleanup;
+ }
Considering the major, minor and micro parts of the version string are
already broken out here, why not directly encode the version number as
long? It seems a forced roundtrip to format it as string, and parse it
again.
diff --git a/src/hyperv/hyperv_wmi_generator.input
b/src/hyperv/hyperv_wmi_generator.input
index 77543cf6ef..bbca550790 100644
--- a/src/hyperv/hyperv_wmi_generator.input
+++ b/src/hyperv/hyperv_wmi_generator.input
@@ -673,7 +673,7 @@ class Win32_OperatingSystem
string CSCreationClassName
string CSDVersion
string CSName
- uint16 CurrentTimeZone
+ int16 CurrentTimeZone
boolean DataExecutionPrevention_Available
boolean DataExecutionPrevention_32BitApplications
boolean DataExecutionPrevention_Drivers
This seems unrelated to the patch? Or it is needed because this class
is used by the hypervGetWmiClass(Win32_OperatingSystem, ...) call, and
the field needed to be fixed?
I'd split it as separate commit before this one.
--
Pino Toscano