On Tue, 2017-02-21 at 15:26 -0500, Laine Stump wrote:
> > @@ -2664,19 +2664,13 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> > break;
> >
> > case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> > - if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
> > - def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > - _("wrong function called for pci-root/pcie-root"));
> > - return NULL;
> > - }
>
> It makes sense that the above code would never happen (certainly one of
> the two current callers to qemuBuildControllerDevStr() guarantees that
> it won't happen by skipping the call in that case), but how much do you
> want to trues the caller.
Not at all! That's why I didn't drop the check but merely
moved it ;)
> > @@ -2917,6 +2911,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> > virBufferAsprintf(&buf, ",numa_node=%d",
> > def->opts.pciopts.numaNode);
> > break;
> > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
> > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("wrong function called"));
>
> Actually it will probably never get to here, because the code above
> where you removed this check also checks to be sure that def->idx != 0
> (and for root controllers it always does, unless the user has manually
> altered it). So instead of getting a "wrong function called" log, you
> would probably get the incorrect "index for pci controllers of model
> "pci[e]-root" must be > 0".
>
> You can solve this by putting the "if (def->idx == 0)" check down just
> below the "switch (def->model)".
Makes sense.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
