[libvirt] [PATCH v3 02/12] qemu: Create new qemuDomainDeviceDefValidateControllerPCI()

[Laine Stump]

On 03/02/2018 08:05 AM, Andrea Bolognani wrote:
> On Fri, 2018-03-02 at 07:34 -0500, Laine Stump wrote:
>> On 02/21/2018 09:14 AM, Andrea Bolognani wrote:
> [...]
>>> +static int
>>> +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
>>> +                                         const virDomainDef *def,
>>> +                                         virQEMUCapsPtr qemuCaps)
>>> +
>>> +{
>>> +    const virDomainPCIControllerOpts *pciopts = &cont->opts.pciopts;
>>> +    const char *model = virDomainControllerModelPCITypeToString(cont->model);
>>> +    const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
>>> +
>>> +    if (!model) {
>>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +                       _("Unknown virDomainControllerModelPCI value: %d"),
>>> +                       cont->model);
>>> +        return -1;
>>> +    }
>>> +    if (!modelName) {
>>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +                       _("Unknown virDomainControllerPCIModelName value: %d"),
>>> +                       pciopts->modelName);
>>> +        return -1;
>>> +    }
>> (meant to send this before, but kept forgetting...)
>>
>> 1) I thought modelName wasn't set for pci-root. Doesn't the above cause
>> a validation error in that case? (too early in the day, haven't tried it)
> The default value is _MODEL_NAME_NONE aka zero, which is still part
> of the enumeration, so virDomainControllerPCIModelNameTypeToString()
> won't return NULL and no error will be raised. For pSeries guests,
> it will be set to _MODEL_NAME_SPAPR_PCI_HOST_BRIDGE so once again no
> problem there.

Ah, right. So the name is "none", but when we're formatting for dumpxml
we skip it if the value is 0, so that never shows up. "Nevermind"
</EmilyLitella>

>
>> 2) danpb made a nice new function to standardize/simplify errors of the
>> above type: virReportEnumRangeError().
> His efforts on switch normalization and me rebasing this series
> happened pretty much at the same time; more specifically, the
> function you're talking about was introduced in 3b1020ac805e, while
> my series is based on the earlier f565321b26df.

Yep, I saw his patches at about the same time as yours, and since you're
respinning I thought I'd point it out.

> I guess this means another rebase! Yay! \o/

Someday you'll learn to do what I'm doing right now - I want to add some
extra validation of pci controller options, but I'm waiting to write any
code until you've pushed your patches. :-)