Re: [PATCH v2 08/10] hyperv: implement connectGetVersion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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, &micro) < 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!





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux