On Wed, Dec 06, 2017 at 08:14:04PM -0500, John Ferlan wrote:
Move the various modelName == NAME_NONE from the command line generation into domain controller validation. Also rather than have multiple cases with the same check, let's make the code more generic, but also note that it was the modelName option that caused the failure. We also have to be sure not to check the PCI models that we don't care about. For the remaining checks in command line building, we can use the field name in the error message to be more specific about what causes the failure.
The errors should all be unreachable. I don't see a need to make them more specific and bother the translators with changes in them.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/qemu/qemu_command.c | 52 ++++++++++++-------------------------------------
src/qemu/qemu_domain.c | 12 ++++++++++++
2 files changed, 24 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4e292e446..45cbf5381 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2722,11 +2722,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- pciopts->chassisNr == -1) {
+ if (pciopts->chassisNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-bridge options not set"));
+ _("autogenerated pci-bridge option chassisNr not set"));
goto error;
}
@@ -2756,12 +2754,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- pciopts->busNr == -1) {
+ if (pciopts->busNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-expander-bus options not set"));
- goto error;
+ _("autogenerated pci-expander-bus option busNr not set"));
}
modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
@@ -2793,13 +2788,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated dmi-to-pci-bridge options not set"));
- goto error;
- }
-
modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2824,12 +2812,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-root-port options not set"));
- goto error;
- }
modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2869,12 +2851,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-switch-upstream-port options not set"));
- goto error;
- }
modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2900,13 +2876,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- pciopts->chassis == -1 ||
+ if (pciopts->chassis == -1 ||
pciopts->port == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_INTERNAL_ERROR,
_("autogenerated pcie-switch-downstream-port "
- "options not set"));
+ "options chassis=%d or port=%d not set"),
+ pciopts->chassis, pciopts->port);
goto error;
}
@@ -2937,11 +2912,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (pciopts->modelName
- == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- pciopts->busNr == -1) {
+ if (pciopts->busNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-expander-bus options not set"));
+ _("autogenerated pcie-expander-bus option busNr not set"));
goto error;
}
@@ -2974,10 +2947,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- pciopts->targetIndex == -1) {
+ if (pciopts->targetIndex == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-root options not set"));
+ _("autogenerated pci-root option targetIndex not set"));
goto error;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ceb03a0cd..ecfff4209 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4014,6 +4014,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
const virDomainDef *def)
{
virDomainControllerModelPCI model = controller->model;
+ const virDomainPCIControllerOpts *pciopts;
/* skip pcie-root */
if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
@@ -4049,6 +4050,17 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
break;
}
+ pciopts = &controller->opts.pciopts;
+ if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
+ controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST) {
+ if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("autogenerated %s option modelName not set"),
%s not surrounded by any quotes. IMO 'autogenerated pci controller %option not set' should be enough for all these "errors" Jan
+ virDomainControllerModelPCITypeToString(controller->model));
+ return -1;
+ }
+ }
+
return 0;
}
--
2.13.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
