Re: [libvirt PATCH v2 05/15] conf: put hostdev PCI backend into a struct
On Sun, Dec 17, 2023 at 21:28:46 -0500, Laine Stump wrote:
On 11/27/23 9:53 AM, Peter Krempa wrote:
> > @@ -29973,14 +29973,10 @@ virDomainNetDefActualToNetworkPort(virDomainDef
*dom,
> > break;
> > case VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN:
> > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > - _("Unexpected PCI backend
'xen'"));
> > - break;
> > -
>
> Falling through to virReportEnumRangeError from such cases isn't
> something we normally do as virReportEnumRangeError is usually meant for
> cases when the value is unknown.
(Finally going through all of these)
the ..._XEN value does exist in the enum for the value in the NetDef object,
but there's no equivalent for it in the virNetworkForwardDriverNameType enum
that's used on the NetworkPort object being converted *to*, so this actually
*is* a range error (although you're correct that it's not the normal case of
a completely unknown value).
The virReportEnumRangeError error reporting macro is in all other cases
used to report values out of range of the enum, the 'switch' statement
uses as an argument. Whatever it's then doing with the value is IMO out
of scope as it is in range of the enum being tested for values.
If it doesn't have any mapping where it is being converted *to*, it's
either an UNSUPORTED_CONFIG or INTERNAL_ERROR, but clearly the value is
in range of the source enum.
So I would take the time to make a separate case / error message for
this
particular value, however the patch immediately after this patch completely
removes all this code anyway (it switches to using the same enum for both
objects), so I'm just leaving it as it is.
Fair enough in this case, I just don't want to keep wrong examples in
the repo, so if you remove it it's okay. I was just reviewing these
chronologically.