Re: [PATCH 05/11] conf: Fix type of @present in _virDomainTimerDef struct

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Monday in 2022, Michal Privoznik wrote:
> In the _virDomainTimerDef structure we have @present member which
> is like virTristateBool, except it's an integer and has values
> shifted by one. This is harder to read. Retype the member to
> virTristateBool which we are familiar with.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c   | 27 ++++++++++-----------------
> src/conf/domain_conf.h   |  2 +-
> src/libxl/libxl_conf.c   |  2 +-
> src/libxl/xen_common.c   | 13 +++++++++----
> src/lxc/lxc_cgroup.c     |  2 +-
> src/lxc/lxc_controller.c |  2 +-
> src/qemu/qemu_command.c  | 20 ++++++++++----------
> src/qemu/qemu_validate.c |  6 +++---
> 8 files changed, 36 insertions(+), 38 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bb887d4a3b..fbe21c4fd2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -20481,8 +20474,9 @@ virDomainTimerDefCheckABIStability(virDomainTimerDef *src,
>
>     if (src->present != dst->present) {
>         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Target timer presence %d does not match source %d"),
> -                       dst->present, src->present);
> +                       _("Target timer presence %s does not match source %s"),

Since you're changing the translatable message, please add apostrophes
around the %s specifiers.

> +                       virTristateBoolTypeToString(dst->present),
> +                       virTristateBoolTypeToString(src->present));
>         return false;
>     }
>
> @@ -26119,10 +26113,9 @@ virDomainTimerDefFormat(virBuffer *buf,
>     }
>     virBufferAsprintf(buf, "<timer name='%s'", name);
>
> -    if (def->present == 0) {
> -        virBufferAddLit(buf, " present='no'");
> -    } else if (def->present == 1) {
> -        virBufferAddLit(buf, " present='yes'");
> +    if (def->present) {

Consider the more explicit
def->present != VIR_TRISTATE_BOOL_ABSENT

> +        virBufferAsprintf(buf, " present='%s'",
> +                          virTristateBoolTypeToString(def->present));
>     }
>
>     if (def->tickpolicy != -1) {
> @@ -625,7 +625,11 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def)
>
>             timer = g_new0(virDomainTimerDef, 1);
>             timer->name = VIR_DOMAIN_TIMER_NAME_HPET;
> -            timer->present = val;
> +            if (val == 1) {
> +                timer->present = VIR_TRISTATE_BOOL_YES;
> +            } else {
> +                timer->present = VIR_TRISTATE_BOOL_NO;
> +            }

You can use virTristateBoolFromBool - we're inside if (val != -1)

>             timer->tickpolicy = -1;
>             timer->mode = -1;
>             timer->track = -1;

Jano

Attachment: signature.asc
Description: PGP signature