Re: [libvirt] [PATCH v3 02/12] qemu: Create new qemuDomainDeviceDefValidateControllerPCI()
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. :-)