[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