[libvirt] [PATCH v3 08/12] qemu: Move PCI command modelName check to controller def validate

Ján Tomko jtomko at redhat.com
Fri Dec 8 15:48:31 UTC 2017


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 at redhat.com>
>---
> 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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171208/57c4f7c4/attachment-0001.sig>


More information about the libvir-list mailing list