[libvirt] [PATCH v5 11/16] qemu: Move PCI command modelName check to controller def validate

John Ferlan posted 16 patches 7 years, 4 months ago
There is a newer version of this series
[libvirt] [PATCH v5 11/16] qemu: Move PCI command modelName check to controller def validate
Posted by John Ferlan 7 years, 4 months ago
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.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_command.c | 36 ++++++------------------------------
 src/qemu/qemu_domain.c  | 12 ++++++++++++
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0175daee3..58f6bee3a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2723,9 +2723,7 @@ 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"));
                 goto error;
@@ -2757,9 +2755,7 @@ 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;
@@ -2794,13 +2790,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,
@@ -2825,9 +2814,7 @@ 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 ||
-                pciopts->chassis == -1 || pciopts->port == -1) {
+            if (pciopts->chassis == -1 || pciopts->port == -1) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("autogenerated pcie-root-port options not set"));
                 goto error;
@@ -2871,12 +2858,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,
@@ -2902,9 +2883,7 @@ 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",
                                _("autogenerated pcie-switch-downstream-port "
@@ -2939,9 +2918,7 @@ 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"));
                 goto error;
@@ -2976,8 +2953,7 @@ 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"));
                 goto error;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index df265dc50..eb533f4b7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4122,6 +4122,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
                                          const virDomainDef *def)
 {
     virDomainControllerModelPCI model = controller->model;
+    const virDomainPCIControllerOpts *pciopts;
 
     /* skip pcie-root */
     if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
@@ -4155,6 +4156,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 options not set"),
+                           virDomainControllerModelPCITypeToString(controller->model));
+            return -1;
+        }
+    }
+
     return 0;
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 11/16] qemu: Move PCI command modelName check to controller def validate
Posted by Michal Privoznik 7 years, 3 months ago
On 01/06/2018 12:47 AM, 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.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/qemu/qemu_command.c | 36 ++++++------------------------------
>  src/qemu/qemu_domain.c  | 12 ++++++++++++
>  2 files changed, 18 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0175daee3..58f6bee3a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2723,9 +2723,7 @@ 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"));

I don't quite understand why break this check and move just one part of
it into DefValidate and leave the other here.

EDIT: Ah, you're doing that in one of the next patches. ACK then.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list