[PATCH 04/17] virDomainDeviceAddressParseXML: Switch to virXMLPropEnumDefault()

[Michal Prívozník]

On 5/24/22 13:50, Boris Fiuczynski wrote:
> On 5/23/22 3:08 PM, Michal Privoznik wrote:
>> @@ -6709,21 +6709,13 @@ static int
>>   virDomainDeviceAddressParseXML(xmlNodePtr address,
>>                                  virDomainDeviceInfo *info)
>>   {
>> -    g_autofree char *type = virXMLPropString(address, "type");
>> -
>> -    if (type) {
>> -        if ((info->type = virDomainDeviceAddressTypeFromString(type))
>> <= 0) {
>> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> -                           _("unknown address type '%s'"), type);
>> -            return -1;
>> -        }
>> -    } else {
>> -        virReportError(VIR_ERR_INTERNAL_ERROR,
>> -                       "%s", _("No type specified for device address"));
>> +    if (virXMLPropEnum(address, "type",
>> +                       virDomainDeviceAddressTypeFromString,
>> +                       VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
>> +                       &info->type) < 0)
>>           return -1;
>> -    }
>>   
> 
> For my taste the resulting generic error messages look a bit different
> and besides the different message texts the error numbers change:
> 
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                 _("unknown address type '%s'"), type);
> turns into
>  virReportError(VIR_ERR_XML_ERROR,
>                 _("Invalid value for attribute '%s' in element '%s':
> '%s'."),
>                        name, node->name, NULLSTR(tmp));
> 
> and
> 
>  virReportError(VIR_ERR_INTERNAL_ERROR,
>                 "%s", _("No type specified for device address"));
> turns into
>  virReportError(VIR_ERR_XML_ERROR,
>                 _("Missing required attribute '%s' in element '%s'"),
>                        name, node->name);
> 
> Don't changes like this have an impact on the user?
> 

They do. We don't really guarantee error codes stability though. But
okay, let me postpone pushing these until there's a broader agreement.

Michal