[libvirt] [PATCH 4/4] qemu: Add support for setting the TSEG size

[Ján Tomko]

On Thu, Jun 07, 2018 at 10:37:43AM +0200, Martin Kletzander wrote:
>The default is stable per machine type so there should be no need to keep that.
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1469338
>
>Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>---
> src/conf/domain_conf.c                        |  3 +-
> src/qemu/qemu_command.c                       | 18 ++++++++
> src/qemu/qemu_domain.c                        | 35 ++++++++++++++
> .../tseg-explicit-size.x86_64-latest.args     | 35 ++++++++++++++
> tests/qemuxml2argvdata/tseg-explicit-size.xml | 23 ++++++++++
> tests/qemuxml2argvdata/tseg-i440fx.xml        | 23 ++++++++++
> tests/qemuxml2argvdata/tseg-invalid-size.xml  | 23 ++++++++++
> tests/qemuxml2argvtest.c                      | 25 ++++++++++
> .../qemuxml2xmloutdata/tseg-explicit-size.xml | 46 +++++++++++++++++++
> .../tseg-old-machine-type.xml                 | 44 ++++++++++++++++++
> tests/qemuxml2xmloutdata/tseg.xml             | 44 ++++++++++++++++++
> tests/qemuxml2xmltest.c                       |  9 ++++
> 12 files changed, 327 insertions(+), 1 deletion(-)
> create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/tseg-explicit-size.xml
> create mode 100644 tests/qemuxml2argvdata/tseg-i440fx.xml
> create mode 100644 tests/qemuxml2argvdata/tseg-invalid-size.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg-explicit-size.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg-old-machine-type.xml
> create mode 100644 tests/qemuxml2xmloutdata/tseg.xml
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 62bf6bb803bb..e83487d6b0de 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -22043,7 +22043,8 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>             return false;
>         }
>
>-        if (src->tseg_size != dst->tseg_size) {
>+        if (src->tseg_specified &&

Why this change?

IIUC if they weren't specified on both sides, they should both be 0
here.

If you're sure it's needed, put it in the commit adding this check.

>+            src->tseg_size != dst->tseg_size) {
>             const char *unit_src, *unit_dst;
>             unsigned long long short_size_src = virFormatIntPretty(src->tseg_size,
>                                                                    &unit_src);
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 6bc9bf5ffab8..4a87b892b7c5 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -7295,6 +7295,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>     return ret;
> }
>
>+
>+static void
>+qemuBuildTSEGCommandLine(virCommandPtr cmd,
>+                         const virDomainDef *def)
>+{
>+    if (!def->tseg_size)
>+        return;

If you don't need the tseg_specified bool at all, I suggest just
dropping it. Does a size of 0 MiB make sense? It's divisible by 1 MiB.

>+
>+    virCommandAddArg(cmd, "-global");
>+
>+    /* PostParse callback guarantees that the size is divisible by 1 MiB */
>+    virCommandAddArgFormat(cmd, "mch.extended-tseg-mbytes=%llu",
>+                           def->tseg_size >> 20);
>+}
>+
>+
> static int
> qemuBuildSmpCommandLine(virCommandPtr cmd,
>                         virDomainDefPtr def)
>@@ -10108,6 +10124,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>     if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0)
>         goto error;
>
>+    qemuBuildTSEGCommandLine(cmd, def);
>+
>     if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps) < 0)
>         goto error;
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index a2c4d3a36090..643fca52c17b 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -3632,6 +3632,38 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def)
> }
>
>
>+static int
>+qemuDomainDefTsegPostParse(virDomainDefPtr def,

s/qemuDomainDefTsegPostParse/qemuDomainDefTSEGPostParse/

>+                           virQEMUCapsPtr qemuCaps)
>+{
>+    if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON)
>+        return 0;
>+

With the bool variable dropped or defended and consistently used:

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180607/a4b4693a/attachment-0001.sig>