[PATCH 1/2] domain: add tsc.on_reboot element

[Michal Prívozník]

On 3/24/22 10:48, Paolo Bonzini wrote:
> Some versions of Windows hang on reboot if their TSC value is greater
> than 2^54.  The workaround is to reset the TSC to a small value.  Add
> to the domain configuration an attribute for this.  It can be used
> by QEMU and in principle also by ESXi, which has a property called
> monitor_control.enable_softResetClearTSC as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  docs/formatdomain.rst             |  4 ++++
>  src/conf/domain_conf.c            | 22 ++++++++++++++++++++++
>  src/conf/domain_conf.h            | 10 ++++++++++
>  src/conf/schemas/domaincommon.rng |  9 +++++++++
>  4 files changed, 45 insertions(+)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index d188de4858..c9319e42ac 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2241,6 +2241,10 @@ Windows, however, expects it to be in so called 'localtime'.
>     ``frequency``
>        The ``frequency`` attribute is an unsigned integer specifying the
>        frequency at which ``name="tsc"`` runs.
> +   ``on_reboot``
> +      The ``on_reboot`` attribute controls how the ``name="tsc"`` timer behaves
> +      when the VM is reset, and can be "default", "clear" or "keep". The reset
> +      behavior of other timers is hardcoded, and depends on the type of timer.
>     ``mode``
>        The ``mode`` attribute controls how the ``name="tsc"`` timer is managed,
>        and can be "auto", "native", "emulate", "paravirt", or "smpsafe". Other
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 153954a0b0..1893b8bbca 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1217,6 +1217,13 @@ VIR_ENUM_IMPL(virDomainTimerMode,
>                "smpsafe",
>  );
>  
> +VIR_ENUM_IMPL(virDomainTimerRebootMode,
> +              VIR_DOMAIN_TIMER_REBOOT_MODE_LAST,
> +              "default",
> +              "keep",
> +              "clear",
> +);
> +
>  VIR_ENUM_IMPL(virDomainStartupPolicy,
>                VIR_DOMAIN_STARTUP_POLICY_LAST,
>                "default",
> @@ -12022,6 +12029,7 @@ virDomainTimerDefParseXML(xmlNodePtr node,
>      g_autofree char *tickpolicy = NULL;
>      g_autofree char *track = NULL;
>      g_autofree char *mode = NULL;
> +    g_autofree char *reboot = NULL;
>  
>      def = g_new0(virDomainTimerDef, 1);
>  
> @@ -12080,6 +12088,15 @@ virDomainTimerDefParseXML(xmlNodePtr node,
>          }
>      }
>  
> +    reboot = virXMLPropString(node, "on_reboot");
> +    if (reboot != NULL) {
> +        if ((def->reboot = virDomainTimerRebootModeTypeFromString(reboot)) <= 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unknown timer reboot mode '%s'"), reboot);
> +            goto error;
> +        }
> +    }

I know you just mimicked what is done for @mode attribute, but we have
this nice brand new virXMLPropEnum() which fits perfectly here as it
encapsulates these lines.

> +
>      catchup = virXPathNode("./catchup", ctxt);
>      if (catchup != NULL) {
>          ret = virXPathULong("string(./catchup/@threshold)", ctxt,
> @@ -26151,6 +26168,11 @@ virDomainTimerDefFormat(virBuffer *buf,
>              virBufferAsprintf(&timerAttr, " mode='%s'",
>                                virDomainTimerModeTypeToString(def->mode));
>          }
> +
> +        if (def->reboot) {
> +            virBufferAsprintf(&timerAttr, " on_reboot='%s'",
> +                              virDomainTimerRebootModeTypeToString(def->mode));
> +        }
>      }
>  
>      if (def->catchup.threshold > 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index b69abfa270..3618410f6c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2408,6 +2408,14 @@ typedef enum {
>      VIR_DOMAIN_TIMER_MODE_LAST
>  } virDomainTimerModeType;
>  
> +typedef enum {
> +    VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT = 0,
> +    VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP,
> +    VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR,
> +
> +    VIR_DOMAIN_TIMER_REBOOT_MODE_LAST
> +} virDomainTimerRebootModeType;
> +
>  typedef enum {
>      VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0,
>      VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO,
> @@ -2439,6 +2447,7 @@ struct _virDomainTimerDef {
>      /* frequency & mode are only valid for name='tsc' */
>      unsigned long long frequency; /* in Hz, unspecified = 0 */
>      int mode;   /* enum virDomainTimerModeType */
> +    int reboot;   /* enum virDomainTimerRebootModeType */

This can be then the actual enum instead of int.

>  };
>  
>  typedef enum {
> @@ -4032,6 +4041,7 @@ VIR_ENUM_DECL(virDomainClockBasis);
>  VIR_ENUM_DECL(virDomainTimerName);
>  VIR_ENUM_DECL(virDomainTimerTrack);
>  VIR_ENUM_DECL(virDomainTimerTickpolicy);
> +VIR_ENUM_DECL(virDomainTimerRebootMode);
>  VIR_ENUM_DECL(virDomainTimerMode);
>  VIR_ENUM_DECL(virDomainCpuPlacementMode);
>  
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 9c1b64a644..10d0404889 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -1285,6 +1285,15 @@
>                <ref name="unsignedLong"/>
>              </attribute>
>            </optional>
> +          <optional>
> +            <attribute name="on_reboot">
> +              <choice>
> +                <value>default</value>

And "default" is not accepted (because _DEFAULT = 0 and parser checks <= 0).

> +                <value>clear</value>
> +                <value>keep</value>
> +              </choice>
> +            </attribute>
> +          </optional>
>            <optional>
>              <attribute name="mode">
>                <choice>

Michal