On Mon, Oct 5, 2020 at 12:22 PM Matt Coleman <mcoleman@xxxxxxxxx> wrote: > > 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. > > Co-authored-by: Sri Ramanujam <sramanujam@xxxxxxxxx> > Signed-off-by: Matt Coleman <matt@xxxxxxxxx> > --- > docs/drvhyperv.html.in | 50 +++++++++++++++++++++++ > src/hyperv/hyperv_driver.c | 81 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 131 insertions(+) > > diff --git a/docs/drvhyperv.html.in b/docs/drvhyperv.html.in > index 78f61aaee2..e8e2113f70 100644 > --- a/docs/drvhyperv.html.in > +++ b/docs/drvhyperv.html.in > @@ -112,4 +112,54 @@ winrm set winrm/config/service @{AllowUnencrypted="true"} > </pre> > > > + <h2><a id="versions">Version Numbers</a></h2> > + <p> > + Since Microsoft's build numbers are almost always over 1000, this driver > + needs to pack the value differently compared to the format defined by > + <code>virConnectGetVersion</code>. > + To preserve all of the digits, the following format is used: > + </p> > + <pre>major * 100000000 + minor * 1000000 + micro</pre> > + <p> > + This results in <code>virsh version</code> producing unexpected output. > + </p> > + <table class="top_table"> > + <thead> > + <th>Windows Release</th> > + <th>Kernel Version</th> > + <th>libvirt Representation</th> > + </thead> > + <tr> > + <td>Windows Server 2008</td> > + <td>6.0.6001</td> > + <td>600.6.1</td> > + </tr> > + <tr> > + <td>Windows Server 2008 R2</td> > + <td>6.1.7600</td> > + <td>601.7.600</td> > + </tr> > + <tr> > + <td>Windows Server 2012</td> > + <td>6.2.9200</td> > + <td>602.9.200</td> > + </tr> > + <tr> > + <td>Windows Server 2012 R2</td> > + <td>6.3.9600</td> > + <td>603.9.600</td> > + </tr> > + <tr> > + <td>Windows Server 2016</td> > + <td>10.0.14393</td> > + <td>1000.14.393</td> > + </tr> > + <tr> > + <td>Windows Server 2019</td> > + <td>10.0.17763</td> > + <td>1000.17.763</td> > + </tr> > + </table> > + > + > </body></html> > diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c > index bbe892fd62..528c826e16 100644 > --- a/src/hyperv/hyperv_driver.c > +++ b/src/hyperv/hyperv_driver.c > @@ -29,6 +29,7 @@ > #include "viralloc.h" > #include "virlog.h" > #include "viruuid.h" > +#include "virutil.h" > #include "hyperv_driver.h" > #include "hyperv_private.h" > #include "hyperv_util.h" > @@ -288,6 +289,24 @@ hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id, > * API-specific utility functions > */ > > +static int > +hypervParseVersionString(const char *str, unsigned int *major, > + unsigned int *minor, unsigned int *micro) > +{ > + char *suffix = NULL; > + > + if (virStrToLong_ui(str, &suffix, 10, major) < 0) > + return -1; > + > + if (virStrToLong_ui(suffix + 1, &suffix, 10, minor) < 0) > + return -1; > + > + if (virStrToLong_ui(suffix + 1, NULL, 10, micro) < 0) > + return -1; > + > + return 0; > +} > + > static int > hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) > { > @@ -523,6 +542,67 @@ hypervConnectGetType(virConnectPtr conn G_GNUC_UNUSED) > > > > +static int > +hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) > +{ > + int result = -1; > + hypervPrivate *priv = conn->privateData; > + Win32_OperatingSystem *os = NULL; > + g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 }; > + unsigned int major, minor, micro; > + > + if (hypervGetWmiClass(Win32_OperatingSystem, &os) < 0 || > + !os) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Could not get version information for host %s"), > + conn->uri->server); > + goto cleanup; > + } > + > + if (hypervParseVersionString(os->data.common->Version, > + &major, &minor, µ) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Could not parse version from '%s'"), > + os->data.common->Version); > + goto cleanup; > + } > + > + /* > + * Pack the version into an unsigned long while retaining all the digits. > + * > + * Since Microsoft's build numbers are almost always over 1000, this driver > + * needs to pack the value differently compared to the format defined by > + * virConnectGetVersion(). > + * > + * This results in `virsh version` producing unexpected output. > + * > + * 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 > + */ > + if (major > 99 || minor > 99 || micro > 999999) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Could not produce packed version number from '%s'"), > + os->data.common->Version); > + goto cleanup; > + } > + > + *version = major * 100000000 + minor * 1000000 + micro; > + > + result = 0; > + > + cleanup: > + hypervFreeObject(priv, (hypervObject *) os); > + > + return result; > +} > + > + > + > static char * > hypervConnectGetHostname(virConnectPtr conn) > { > @@ -1721,6 +1801,7 @@ static virHypervisorDriver hypervHypervisorDriver = { > .connectOpen = hypervConnectOpen, /* 0.9.5 */ > .connectClose = hypervConnectClose, /* 0.9.5 */ > .connectGetType = hypervConnectGetType, /* 0.9.5 */ > + .connectGetVersion = hypervConnectGetVersion, /* 6.9.0 */ > .connectGetHostname = hypervConnectGetHostname, /* 0.9.5 */ > .connectGetMaxVcpus = hypervConnectGetMaxVcpus, /* 6.9.0 */ > .nodeGetInfo = hypervNodeGetInfo, /* 0.9.5 */ > -- > 2.27.0 > > Ugh, this is kind of gross, but it makes sense... Reviewed-by: Neal Gompa <ngompa13@xxxxxxxxx> -- 真実はいつも一つ！/ Always, there's only one truth!
