Re: [libvirt PATCH 06/38] domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainDeviceInfoParseXML

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 3/18/21 9:00 AM, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
>   src/conf/domain_conf.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)

Had to rebase this, because meanwhile I pushed another patch that 
touched this area.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3a4b01ad1e..e1b2baf621 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6620,8 +6620,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt,
>       xmlNodePtr boot = NULL;
>       xmlNodePtr rom = NULL;
>       int ret = -1;
> -    g_autofree char *romenabled = NULL;
> -    g_autofree char *rombar = NULL;
>       g_autofree char *aliasStr = NULL;
>   
>       virDomainDeviceInfoClear(info);
> @@ -6673,18 +6671,17 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt,
>       }
>   
>       if (rom) {
> -        if ((romenabled = virXMLPropString(rom, "enabled")) &&
> -            ((info->romenabled = virTristateBoolTypeFromString(romenabled)) <= 0)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("unknown rom enabled value '%s'"), romenabled);
> +        virTristateBool romenabled = VIR_TRISTATE_BOOL_ABSENT;
> +        virTristateSwitch rombar = VIR_TRISTATE_SWITCH_ABSENT;

Okay, so the reason these variables are here is because 
virXMLPropYesNo()/virXMLPropOnOff() expects 
virTristateBool/virTristateSwitch variable, but info->romenabled and 
info->rombar are ints. They had to be, because previously we stored 
virTristateXXXTypeFromString() retval - which is type of int - directly 
into them. But I guess after these patches are merged we can finally 
switch them to the proper type.

Same applies to the next patch (I haven't looked further). It's 
perfectly okay to do it in a follow up patch.

> +
> +        if (virXMLPropYesNo(rom, "enabled", &romenabled) < 0)
>               goto cleanup;
> -        }
> -        if ((rombar = virXMLPropString(rom, "bar")) &&
> -            ((info->rombar = virTristateSwitchTypeFromString(rombar)) <= 0)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("unknown rom bar value '%s'"), rombar);
> +
> +        if (virXMLPropOnOff(rom, "bar", &rombar) < 0)
>               goto cleanup;
> -        }
> +
> +        info->romenabled = romenabled;
> +        info->rombar = rombar;
>           info->romfile = virXMLPropString(rom, "file");
>   
>           if (info->romenabled == VIR_TRISTATE_BOOL_NO &&


Michal