[libvirt] [PATCH v3 00/12] Move qemu command line controller checks to qemuDomainDeviceDefValidateController* checks
7:13 p.m.
v2:
https://www.redhat.com/archives/libvir-list/2017-December/msg00173.html
Changes since v2:
* Push a few patches from v1 that were ACK'd
* Make the Skip helper private to qemu_command.c and just copied
the various skip checks from the new helper into the specific
controller validate helper to make it painfully obvious what
was being skipped rather than having common code handle that
and possibly obsfuscate those details from the casual reader.
John Ferlan (11):
qemu: Introduce qemuBuildSkipController
qemu: Introduce qemuDomainDeviceDefValidateController
qemu: Move CCW S390 Address check to controller def validate
qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI
qemu: Introduce qemuDomainDeviceDefValidateControllerPCI
qemu: Use virDomainPCIControllerOpts in qemuBuildControllerDevStr
qemu: Move PCI command modelName check to controller def validate
qemu: Move PCI command modelName TypeToString to controller def
validate
qemu: Move PCI more command checks to controller def validate
qemu: Complete PCI command checks to controller def validate
qemu: Introduce qemuDomainDeviceDefValidateControllerSATA
Lin Ma (1):
qemu: Introduce qemuDomainDeviceDefValidateControllerIDE
src/qemu/qemu_command.c | 497 ++++++++----------------------------------------
src/qemu/qemu_domain.c | 469 ++++++++++++++++++++++++++++++++++++++++++++-
tests/qemumemlocktest.c | 14 ++
tests/qemuxml2xmltest.c | 5 +-
4 files changed, 564 insertions(+), 421 deletions(-)
--
2.13.6
7:13 p.m.
New subject: [libvirt] [PATCH v3 01/12] qemu: Introduce qemuBuildSkipController
Move the non USB implicit controller checks into their own
helper to reduce the cruft in qemuBuildControllerDevCommandLine.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 65 +++++++++++++++++++++++++++++++------------------
1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 645802795..ffa89aa4a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3175,6 +3175,44 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
}
+/**
+ * qemuBuildSkipController:
+ * @controller: Controller to check
+ * @def: Domain definition
+ *
+ * Returns true if this controller can be skipped for command line
+ * generation or device validation.
+ */
+static bool
+qemuBuildSkipController(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ /* skip pcie-root */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return true;
+
+ /* Skip pci-root, except for pSeries guests (which actually
+ * support more than one PCI Host Bridge per guest) */
+ if (!qemuDomainIsPSeries(def) &&
+ controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+ return true;
+
+ /* first SATA controller on Q35 machines is implicit */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
+ controller->idx == 0 && qemuDomainIsQ35(def))
+ return true;
+
+ /* first IDE controller is implicit on various machines */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
+ controller->idx == 0 && qemuDomainHasBuiltinIDE(def))
+ return true;
+
+ return false;
+}
+
+
static int
qemuBuildControllerDevCommandLine(virCommandPtr cmd,
const virDomainDef *def,
@@ -3217,6 +3255,9 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
if (cont->type != contOrder[j])
continue;
+ if (qemuBuildSkipController(cont, def))
+ continue;
+
/* skip USB controllers with type none.*/
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
@@ -3224,30 +3265,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
continue;
}
- /* skip pcie-root */
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
- continue;
- }
-
- /* Skip pci-root, except for pSeries guests (which actually
- * support more than one PCI Host Bridge per guest) */
- if (!qemuDomainIsPSeries(def) &&
- cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
- continue;
- }
-
- /* first SATA controller on Q35 machines is implicit */
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
- cont->idx == 0 && qemuDomainIsQ35(def))
- continue;
-
- /* first IDE controller is implicit on various machines */
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
- cont->idx == 0 && qemuDomainHasBuiltinIDE(def))
- continue;
-
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == -1 &&
!qemuDomainIsQ35(def) &&
--
2.13.6
8:48 a.m.
New subject: [libvirt] [PATCH v3 01/12] qemu: Introduce qemuBuildSkipController
On Wed, Dec 06, 2017 at 08:13:57PM -0500, John Ferlan wrote:
Move the non USB implicit controller checks into their own
helper to reduce the cruft in qemuBuildControllerDevCommandLine.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 65 +++++++++++++++++++++++++++++++------------------
1 file changed, 41 insertions(+), 24 deletions(-)
ACK
Jan
7:13 p.m.
New subject: [libvirt] [PATCH v3 02/12] qemu: Introduce qemuDomainDeviceDefValidateController
Introduce the bare bones helper to validate whether the controller
definition is valid.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 75cecb1bb..877a0a322 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3893,6 +3893,26 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int
+qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller)
+{
+ switch ((virDomainControllerType) controller->type) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
+ case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+ case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+ case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
+ case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
+static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
void *opaque ATTRIBUTE_UNUSED)
@@ -3936,11 +3956,14 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
ret = qemuDomainDeviceDefValidateDisk(dev->data.disk);
break;
+ case VIR_DOMAIN_DEVICE_CONTROLLER:
+ ret = qemuDomainDeviceDefValidateController(dev->data.controller);
+ break;
+
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
- case VIR_DOMAIN_DEVICE_CONTROLLER:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
--
2.13.6
8:49 a.m.
New subject: [libvirt] [PATCH v3 02/12] qemu: Introduce qemuDomainDeviceDefValidateController
On Wed, Dec 06, 2017 at 08:13:58PM -0500, John Ferlan wrote:
Introduce the bare bones helper to validate whether the controller
definition is valid.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
ACK
Jan
7:13 p.m.
New subject: [libvirt] [PATCH v3 03/12] qemu: Move CCW S390 Address check to controller def validate
Move the call to qemuDomainCheckCCWS390AddressSupport from
qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController.
This means we will get the qemuCaps from the driver opaque
variable passed to qemuDomainDeviceDefValidate.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ----
src/qemu/qemu_domain.c | 20 +++++++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ffa89aa4a..894c3f323 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2662,10 +2662,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
*devstr = NULL;
- if (!qemuDomainCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps,
- "controller"))
- return -1;
-
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 877a0a322..8d966156d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3893,8 +3893,14 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int
-qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller)
+qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
+ if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps,
+ "controller"))
+ return -1;
+
switch ((virDomainControllerType) controller->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
@@ -3915,9 +3921,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller)
static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
int ret = 0;
+ virQEMUDriverPtr driver = opaque;
+ virQEMUCapsPtr qemuCaps = NULL;
+
+ if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
+ def->emulator)))
+ return -1;
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_NET:
@@ -3957,7 +3969,8 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
break;
case VIR_DOMAIN_DEVICE_CONTROLLER:
- ret = qemuDomainDeviceDefValidateController(dev->data.controller);
+ ret = qemuDomainDeviceDefValidateController(dev->data.controller, def,
+ qemuCaps);
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -3978,6 +3991,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
break;
}
+ virObjectUnref(qemuCaps);
return ret;
}
--
2.13.6
8:49 a.m.
New subject: [libvirt] [PATCH v3 03/12] qemu: Move CCW S390 Address check to controller def validate
On Wed, Dec 06, 2017 at 08:13:59PM -0500, John Ferlan wrote:
Move the call to qemuDomainCheckCCWS390AddressSupport from
qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController.
This means we will get the qemuCaps from the driver opaque
variable passed to qemuDomainDeviceDefValidate.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ----
src/qemu/qemu_domain.c | 20 +++++++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
ACK
Jan
9:57 a.m.
New subject: [libvirt] [PATCH v3 03/12] qemu: Move CCW S390 Address check to controller def validate
This looks okay to me.
Reviewed-by: Farhan Ali <alifm(a)linux.vnet.ibm.com>
On 12/06/2017 08:13 PM, John Ferlan wrote:
Move the call to qemuDomainCheckCCWS390AddressSupport from
qemuBuildControllerDevStr to qemuDomainDeviceDefValidateController.
This means we will get the qemuCaps from the driver opaque
variable passed to qemuDomainDeviceDefValidate.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ----
src/qemu/qemu_domain.c | 20 +++++++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ffa89aa4a..894c3f323 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2662,10 +2662,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
*devstr = NULL;
- if (!qemuDomainCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps,
- "controller"))
- return -1;
-
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) <
0)
return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 877a0a322..8d966156d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3893,8 +3893,14 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int
-qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller)
+qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
+ if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps,
+ "controller"))
+ return -1;
+
switch ((virDomainControllerType) controller->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
@@ -3915,9 +3921,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller)
static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
- void *opaque ATTRIBUTE_UNUSED)
+ void *opaque)
{
int ret = 0;
+ virQEMUDriverPtr driver = opaque;
+ virQEMUCapsPtr qemuCaps = NULL;
+
+ if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
+ def->emulator)))
+ return -1;
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_NET:
@@ -3957,7 +3969,8 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
break;
case VIR_DOMAIN_DEVICE_CONTROLLER:
- ret = qemuDomainDeviceDefValidateController(dev->data.controller);
+ ret = qemuDomainDeviceDefValidateController(dev->data.controller, def,
+ qemuCaps);
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -3978,6 +3991,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
break;
}
+ virObjectUnref(qemuCaps);
return ret;
}
7:14 p.m.
New subject: [libvirt] [PATCH v3 04/12] qemu: Introduce qemuDomainDeviceDefValidateControllerIDE
From: Lin Ma <lma(a)suse.com>
Move the IDE controller check from command line building to
controller def validation. Also explicitly include the avoidance
check for the implicit IDE controller from qemuBuildSkipController.
Cause the IDE case for command line building to generate a
failure if called to add an IDE since that shouldn't happen
if the Validate code did the right thing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 16 ----------------
src/qemu/qemu_domain.c | 33 ++++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 894c3f323..2dd50a214 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3120,22 +3120,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
- /* Since we currently only support the integrated IDE
- * controller on various boards, if we ever get to here, it's
- * because some other machinetype had an IDE controller
- * specified, or one with a single IDE contraller had multiple
- * ide controllers specified.
- */
- if (qemuDomainHasBuiltinIDE(domainDef))
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only a single IDE controller is supported "
- "for this machine type"));
- else
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("IDE controllers are unsupported for "
- "this QEMU binary or machine type"));
- goto error;
-
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8d966156d..076c68c9f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3893,16 +3893,47 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int
+qemuDomainDeviceDefValidateControllerIDE(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ /* first IDE controller is implicit on various machines */
+ if (controller->idx == 0 && qemuDomainHasBuiltinIDE(def))
+ return 0;
+
+ /* Since we currently only support the integrated IDE
+ * controller on various boards, if we ever get to here, it's
+ * because some other machinetype had an IDE controller
+ * specified, or one with a single IDE controller had multiple
+ * IDE controllers specified.
+ */
+ if (qemuDomainHasBuiltinIDE(def))
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Only a single IDE controller is supported "
+ "for this machine type"));
+ else
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("IDE controllers are unsupported for "
+ "this QEMU binary or machine type"));
+ return -1;
+}
+
+
+static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
+ int ret = 0;
+
if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps,
"controller"))
return -1;
switch ((virDomainControllerType) controller->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
+ ret = qemuDomainDeviceDefValidateControllerIDE(controller, def);
+ break;
+
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
@@ -3914,7 +3945,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
break;
}
- return 0;
+ return ret;
}
--
2.13.6
8:49 a.m.
New subject: [libvirt] [PATCH v3 04/12] qemu: Introduce qemuDomainDeviceDefValidateControllerIDE
On Wed, Dec 06, 2017 at 08:14:00PM -0500, John Ferlan wrote:
From: Lin Ma <lma(a)suse.com>
Move the IDE controller check from command line building to
controller def validation. Also explicitly include the avoidance
check for the implicit IDE controller from qemuBuildSkipController.
Cause the IDE case for command line building to generate a
failure if called to add an IDE since that shouldn't happen
if the Validate code did the right thing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 16 ----------------
src/qemu/qemu_domain.c | 33 ++++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 17 deletions(-)
ACK
Jan
7:14 p.m.
New subject: [libvirt] [PATCH v3 05/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI
Move SCSI validation from qemu_command into qemu_domain.
This includes the @model reset/check when the controller
model hasn't yet been set. While at it modify the switch
to account for all virDomainControllerModelSCSI types
rather than using the default label.
Rename/reorder the args in qemuCheckSCSIControllerIOThreads
to match the caller and also remove the unnecessary model
check as well as fixing up the comments to remove the previously
removed qemuCaps arg.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 94 +++++------------------------------------------
src/qemu/qemu_domain.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 86 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2dd50a214..cdd267675 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2583,52 +2583,6 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
}
-/* qemuCheckSCSIControllerIOThreads:
- * @domainDef: Pointer to domain def
- * @def: Pointer to controller def
- * @qemuCaps: Capabilities
- *
- * If this controller definition has iothreads set, let's make sure the
- * configuration is right before adding to the command line
- *
- * Returns true if either supported or there are no iothreads for controller;
- * otherwise, returns false if configuration is not quite right.
- */
-static bool
-qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef,
- virDomainControllerDefPtr def)
-{
- if (!def->iothread)
- return true;
-
- if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("IOThreads only supported for virtio-scsi "
- "controllers model is '%s'"),
- virDomainControllerModelSCSITypeToString(def->model));
- return false;
- }
-
- if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
- def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("IOThreads only available for virtio pci and "
- "virtio ccw controllers"));
- return false;
- }
-
- /* Can we find the controller iothread in the iothreadid list? */
- if (!virDomainIOThreadIDFind(domainDef, def->iothread)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("controller iothread '%u' not defined in
iothreadid"),
- def->iothread);
- return false;
- }
-
- return true;
-}
-
-
/**
* qemuBuildControllerDevStr:
* @domainDef: domain definition
@@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
*devstr = NULL;
- if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
- return -1;
- }
-
- if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
- if (def->queues) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'queues' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->cmd_per_lun) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'cmd_per_lun' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->max_sectors) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'max_sectors' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->ioeventfd) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'ioeventfd' is only supported by virtio-scsi
controller"));
- return -1;
- }
- }
-
switch ((virDomainControllerType) def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- switch (model) {
+ if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
+ return -1;
+ switch ((virDomainControllerModelSCSI) model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
virBufferAddLit(&buf, "virtio-scsi-ccw");
- if (def->iothread) {
- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
- goto error;
+ if (def->iothread)
virBufferAsprintf(&buf, ",iothread=iothread%u",
def->iothread);
- }
} else if (def->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
virBufferAddLit(&buf, "virtio-scsi-s390");
@@ -2711,12 +2635,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAddLit(&buf, "virtio-scsi-device");
} else {
virBufferAddLit(&buf, "virtio-scsi-pci");
- if (def->iothread) {
- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
- goto error;
+ if (def->iothread)
virBufferAsprintf(&buf, ",iothread=iothread%u",
def->iothread);
- }
}
if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0)
goto error;
@@ -2733,7 +2654,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
virBufferAddLit(&buf, "megasas");
break;
- default:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported controller model: %s"),
virDomainControllerModelSCSITypeToString(def->model));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 076c68c9f..428db1193 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3918,6 +3918,97 @@ qemuDomainDeviceDefValidateControllerIDE(const
virDomainControllerDef *controlle
}
+/* qemuDomainCheckSCSIControllerIOThreads:
+ * @controller: Pointer to controller def
+ * @def: Pointer to domain def
+ *
+ * If this controller definition has iothreads set, let's make sure the
+ * configuration is right before adding to the command line
+ *
+ * Returns true if either supported or there are no iothreads for controller;
+ * otherwise, returns false if configuration is not quite right.
+ */
+static bool
+qemuDomainCheckSCSIControllerIOThreads(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ if (!controller->iothread)
+ return true;
+
+ if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+ controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("IOThreads only available for virtio pci and "
+ "virtio ccw controllers"));
+ return false;
+ }
+
+ /* Can we find the controller iothread in the iothreadid list? */
+ if (!virDomainIOThreadIDFind(def, controller->iothread)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("controller iothread '%u' not defined in
iothreadid"),
+ controller->iothread);
+ return false;
+ }
+
+ return true;
+}
+
+
+static int
+qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ int model = controller->model;
+
+ if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0)
+ return -1;
+
+ if (model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) {
+ if (controller->queues) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'queues' is only supported by virtio-scsi
controller"));
+ return -1;
+ }
+ if (controller->cmd_per_lun) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'cmd_per_lun' is only supported by virtio-scsi
controller"));
+ return -1;
+ }
+ if (controller->max_sectors) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'max_sectors' is only supported by virtio-scsi
controller"));
+ return -1;
+ }
+ if (controller->ioeventfd) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'ioeventfd' is only supported by virtio-scsi
controller"));
+ return -1;
+ }
+ }
+
+ switch ((virDomainControllerModelSCSI) model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!qemuDomainCheckSCSIControllerIOThreads(controller, def))
+ return -1;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
@@ -3934,8 +4025,12 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
ret = qemuDomainDeviceDefValidateControllerIDE(controller, def);
break;
- case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+ ret = qemuDomainDeviceDefValidateControllerSCSI(controller, def,
+ qemuCaps);
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
--
2.13.6
9:37 a.m.
New subject: [libvirt] [PATCH v3 05/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI
On Wed, Dec 06, 2017 at 08:14:01PM -0500, John Ferlan wrote:
Move SCSI validation from qemu_command into qemu_domain.
This includes the @model reset/check when the controller
model hasn't yet been set. While at it modify the switch
to account for all virDomainControllerModelSCSI types
rather than using the default label.
'While at it' in the commit message is usually an indicator
of a change that should have been in a separate commit.
Rename/reorder the args in qemuCheckSCSIControllerIOThreads
to match the caller and also remove the unnecessary model
check as well as fixing up the comments to remove the previously
removed qemuCaps arg.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 94 +++++------------------------------------------
src/qemu/qemu_domain.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 86 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2dd50a214..cdd267675 100644
[...]
@@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef
*domainDef,
*devstr = NULL;
- if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
- return -1;
- }
-
- if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
These errors are reported for non-SCSI controllers too.
- if (def->queues) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'queues' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->cmd_per_lun) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'cmd_per_lun' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->max_sectors) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'max_sectors' is only supported by virtio-scsi
controller"));
- return -1;
- }
- if (def->ioeventfd) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'ioeventfd' is only supported by virtio-scsi
controller"));
- return -1;
- }
- }
-
switch ((virDomainControllerType) def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- switch (model) {
+ if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
+ return -1;
After this patch, this function is called twice. Also it's called
SetModel, while it's behaving as GetModel. Can we start assigning the
default model in post-parse without breaking backwards compatibility?
Jan
+ switch ((virDomainControllerModelSCSI) model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
virBufferAddLit(&buf, "virtio-scsi-ccw");
- if (def->iothread) {
- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
- goto error;
+ if (def->iothread)
virBufferAsprintf(&buf, ",iothread=iothread%u",
def->iothread);
- }
} else if (def->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
virBufferAddLit(&buf, "virtio-scsi-s390");
10:06 a.m.
New subject: [libvirt] [PATCH v3 05/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI
On 12/08/2017 10:37 AM, Ján Tomko wrote:
On Wed, Dec 06, 2017 at 08:14:01PM -0500, John Ferlan wrote:
> Move SCSI validation from qemu_command into qemu_domain.
> This includes the @model reset/check when the controller
> model hasn't yet been set. While at it modify the switch
> to account for all virDomainControllerModelSCSI types
> rather than using the default label.
'While at it' in the commit message is usually an indicator
of a change that should have been in a separate commit.
Sure... That's fine. Couple more patches added...
>
> Rename/reorder the args in qemuCheckSCSIControllerIOThreads
> to match the caller and also remove the unnecessary model
> check as well as fixing up the comments to remove the previously
> removed qemuCaps arg.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 94
> +++++------------------------------------------
> src/qemu/qemu_domain.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 105 insertions(+), 86 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2dd50a214..cdd267675 100644
[...]
> @@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef
> *domainDef,
>
> *devstr = NULL;
>
> - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
> - if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps,
> &model)) < 0)
> - return -1;
> - }
Oh and of course I cannot remove this since this is called from hotplug
code we need to perhaps alter the model here...
> -
> - if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
> - model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
These errors are reported for non-SCSI controllers too.
Hmm.. true, dang compound conditionals...
> - if (def->queues) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("'queues' is only supported by
> virtio-scsi controller"));
> - return -1;
> - }
> - if (def->cmd_per_lun) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("'cmd_per_lun' is only supported by
> virtio-scsi controller"));
> - return -1;
> - }
> - if (def->max_sectors) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("'max_sectors' is only supported by
> virtio-scsi controller"));
> - return -1;
> - }
> - if (def->ioeventfd) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("'ioeventfd' is only supported by
> virtio-scsi controller"));
> - return -1;
> - }
> - }
> -
> switch ((virDomainControllerType) def->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> - switch (model) {
> + if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps,
> &model)) < 0)
> + return -1;
After this patch, this function is called twice. Also it's called
SetModel, while it's behaving as GetModel. Can we start assigning the
default model in post-parse without breaking backwards compatibility?
Jan
Are you asking that during PostParse callback for us to set def->model
so that we can just use it later on? That'd make things a lot easier
later on, but that seems a bit outside of what I was trying to do though.
Anyway based on this series and another I have posted... That means for
someone that didn't set the model, that a model would then be set and
written out... Meaning adjustment of a number of tests because now there
would be a model defined. It also could mean we cannot adjust the
default model - one could say for example that using lsi as the default
model for scsi_host passthrough is perhaps less optimal than using
virtio-scsi... In fact, based on a different series and some testing
done - it may not work either.
As for GetModel vs. SetModel - I think that's a different problem and
can be put on a list of things to do after this is done.
Tks -
John
> + switch ((virDomainControllerModelSCSI) model) {
> case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> virBufferAddLit(&buf, "virtio-scsi-ccw");
> - if (def->iothread) {
> - if (!qemuCheckSCSIControllerIOThreads(domainDef,
> def))
> - goto error;
> + if (def->iothread)
> virBufferAsprintf(&buf, ",iothread=iothread%u",
> def->iothread);
> - }
> } else if (def->info.type ==
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
> virBufferAddLit(&buf, "virtio-scsi-s390");
7:14 p.m.
New subject: [libvirt] [PATCH v3 06/12] qemu: Introduce qemuDomainDeviceDefValidateControllerPCI
Move PCI validation checks out of qemu_command into the proper
qemu_domain validation helper.
Since there's a lot to move, we'll start slow by replicating the
pcie-root and pci-root avoidance from qemuBuildSkipController and
the first switch found in qemuBuildControllerDevStr.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 20 --------------------
src/qemu/qemu_domain.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cdd267675..e844dfe7c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2719,26 +2719,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (def->idx == 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("index for pci controllers of model '%s'
must be > 0"),
- virDomainControllerModelPCITypeToString(def->model));
- goto error;
- }
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
- break;
- }
- switch ((virDomainControllerModelPCI) def->model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
if (def->opts.pciopts.modelName
== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
def->opts.pciopts.chassisNr == -1) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 428db1193..ceb03a0cd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4010,6 +4010,50 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ virDomainControllerModelPCI model = controller->model;
+
+ /* skip pcie-root */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return 0;
+
+ /* Skip pci-root, except for pSeries guests (which actually
+ * support more than one PCI Host Bridge per guest) */
+ if (!qemuDomainIsPSeries(def) &&
+ controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+ return 0;
+
+ switch (model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ if (controller->idx == 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("index for pci controllers of model '%s' must
be > 0"),
+ virDomainControllerModelPCITypeToString(model));
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
+static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
@@ -4030,12 +4074,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
qemuCaps);
break;
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ ret = qemuDomainDeviceDefValidateControllerPCI(controller, def);
+ break;
+
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
- case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
break;
}
--
2.13.6
9:38 a.m.
New subject: [libvirt] [PATCH v3 06/12] qemu: Introduce qemuDomainDeviceDefValidateControllerPCI
On Wed, Dec 06, 2017 at 08:14:02PM -0500, John Ferlan wrote:
Move PCI validation checks out of qemu_command into the proper
qemu_domain validation helper.
Since there's a lot to move, we'll start slow by replicating the
pcie-root and pci-root avoidance from qemuBuildSkipController and
the first switch found in qemuBuildControllerDevStr.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 20 --------------------
src/qemu/qemu_domain.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 428db1193..ceb03a0cd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4010,6 +4010,50 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
+qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ virDomainControllerModelPCI model = controller->model;
+
+ /* skip pcie-root */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
We already checked that the controller type is PCI in the caller.
+ controller->model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return 0;
+
+ /* Skip pci-root, except for pSeries guests (which actually
+ * support more than one PCI Host Bridge per guest) */
+ if (!qemuDomainIsPSeries(def) &&
+ controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
Same here.
+ controller->model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+ return 0;
+
Jan
10:07 a.m.
New subject: [libvirt] [PATCH v3 06/12] qemu: Introduce qemuDomainDeviceDefValidateControllerPCI
On 12/08/2017 10:38 AM, Ján Tomko wrote:
On Wed, Dec 06, 2017 at 08:14:02PM -0500, John Ferlan wrote:
> Move PCI validation checks out of qemu_command into the proper
> qemu_domain validation helper.
>
> Since there's a lot to move, we'll start slow by replicating the
> pcie-root and pci-root avoidance from qemuBuildSkipController and
> the first switch found in qemuBuildControllerDevStr.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 20 --------------------
> src/qemu/qemu_domain.c | 49
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 428db1193..ceb03a0cd 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4010,6 +4010,50 @@ qemuDomainDeviceDefValidateControllerSCSI(const
> virDomainControllerDef *controll
>
>
> static int
> +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef
> *controller,
> + const virDomainDef *def)
> +{
> + virDomainControllerModelPCI model = controller->model;
> +
> + /* skip pcie-root */
> + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
We already checked that the controller type is PCI in the caller.
Oh right, hence my distaste for cut-n-paste code ;-)
John
> + controller->model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
> + return 0;
> +
> + /* Skip pci-root, except for pSeries guests (which actually
> + * support more than one PCI Host Bridge per guest) */
> + if (!qemuDomainIsPSeries(def) &&
> + controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
Same here.
> + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
> + return 0;
> +
Jan
7:14 p.m.
New subject: [libvirt] [PATCH v3 07/12] qemu: Use virDomainPCIControllerOpts in qemuBuildControllerDevStr
Shorten up a few characters and reference the pciopts pointer
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 115 +++++++++++++++++++++++++-----------------------
1 file changed, 59 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e844dfe7c..4e292e446 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2612,6 +2612,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
int model = def->model;
+ const virDomainPCIControllerOpts *pciopts;
const char *modelName = NULL;
*devstr = NULL;
@@ -2717,24 +2718,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ pciopts = &def->opts.pciopts;
+
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- def->opts.pciopts.chassisNr == -1) {
+ pciopts->chassisNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("autogenerated pci-bridge options not set"));
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pci-bridge model name value %d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2749,26 +2752,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
- modelName, def->opts.pciopts.chassisNr,
+ modelName, pciopts->chassisNr,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- def->opts.pciopts.busNr == -1) {
+ pciopts->busNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("autogenerated pci-expander-bus options not
set"));
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pci-expander-bus model name value
%d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2783,28 +2786,28 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
- modelName, def->opts.pciopts.busNr,
+ modelName, pciopts->busNr,
def->info.alias);
- if (def->opts.pciopts.numaNode != -1)
+ if (pciopts->numaNode != -1)
virBufferAsprintf(&buf, ",numa_node=%d",
- def->opts.pciopts.numaNode);
+ pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (def->opts.pciopts.modelName
+ 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(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown dmi-to-pci-bridge model name value
%d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2821,22 +2824,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, "%s,id=%s", modelName,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- if (def->opts.pciopts.modelName
+ 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(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pcie-root-port model name value
%d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if ((def->opts.pciopts.modelName !=
+ if ((pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
- (def->opts.pciopts.modelName !=
+ (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2844,7 +2847,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
modelName);
goto error;
}
- if ((def->opts.pciopts.modelName ==
+ if ((pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -2852,7 +2855,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
"controller is not supported in this QEMU
binary"));
goto error;
}
- if ((def->opts.pciopts.modelName ==
+ if ((pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -2862,24 +2865,24 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
}
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
- modelName, def->opts.pciopts.port,
- def->opts.pciopts.chassis, def->info.alias);
+ modelName, pciopts->port,
+ pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (def->opts.pciopts.modelName
+ 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(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pcie-switch-upstream-port model name value
%d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2897,24 +2900,24 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, "%s,id=%s", modelName,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- def->opts.pciopts.chassis == -1 ||
- def->opts.pciopts.port == -1) {
+ pciopts->chassis == -1 ||
+ pciopts->port == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("autogenerated pcie-switch-downstream-port "
"options not set"));
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pcie-switch-downstream-port model name
value %d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2930,26 +2933,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
- modelName, def->opts.pciopts.port,
- def->opts.pciopts.chassis, def->info.alias);
+ modelName, pciopts->port,
+ pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
== VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- def->opts.pciopts.busNr == -1) {
+ pciopts->busNr == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("autogenerated pcie-expander-bus options not
set"));
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pcie-expander-bus model name value
%d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName
+ if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' "
@@ -2964,32 +2967,32 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
- modelName, def->opts.pciopts.busNr,
+ modelName, pciopts->busNr,
def->info.alias);
- if (def->opts.pciopts.numaNode != -1)
+ if (pciopts->numaNode != -1)
virBufferAsprintf(&buf, ",numa_node=%d",
- def->opts.pciopts.numaNode);
+ pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- if (def->opts.pciopts.modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
- def->opts.pciopts.targetIndex == -1) {
+ if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE ||
+ pciopts->targetIndex == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("autogenerated pci-root options not set"));
goto error;
}
/* Skip the implicit one */
- if (def->opts.pciopts.targetIndex == 0)
+ if (pciopts->targetIndex == 0)
goto done;
- modelName =
virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName);
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
if (!modelName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown pci-root model name value %d"),
- def->opts.pciopts.modelName);
+ pciopts->modelName);
goto error;
}
- if (def->opts.pciopts.modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' is not
valid for a pci-root"),
modelName);
@@ -3002,17 +3005,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
virBufferAsprintf(&buf, "%s,index=%d,id=%s",
- modelName, def->opts.pciopts.targetIndex,
+ modelName, pciopts->targetIndex,
def->info.alias);
- if (def->opts.pciopts.numaNode != -1) {
+ if (pciopts->numaNode != -1) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE))
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the spapr-pci-host-bridge controller "
"doesn't support numa_node on this QEMU
binary"));
goto error;
}
- virBufferAsprintf(&buf, ",numa_node=%d",
def->opts.pciopts.numaNode);
+ virBufferAsprintf(&buf, ",numa_node=%d",
pciopts->numaNode);
}
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
--
2.13.6
9:39 a.m.
New subject: [libvirt] [PATCH v3 07/12] qemu: Use virDomainPCIControllerOpts in qemuBuildControllerDevStr
On Wed, Dec 06, 2017 at 08:14:03PM -0500, John Ferlan wrote:
Shorten up a few characters and reference the pciopts pointer
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 115 +++++++++++++++++++++++++-----------------------
1 file changed, 59 insertions(+), 56 deletions(-)
ACK
Jan
7:14 p.m.
New subject: [libvirt] [PATCH v3 08/12] qemu: Move PCI command modelName check to controller def validate
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(a)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"),
+
virDomainControllerModelPCITypeToString(controller->model));
+ return -1;
+ }
+ }
+
return 0;
}
--
2.13.6
9:48 a.m.
New subject: [libvirt] [PATCH v3 08/12] qemu: Move PCI command modelName check to controller def validate
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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:10 a.m.
New subject: [libvirt] [PATCH v3 08/12] qemu: Move PCI command modelName check to controller def validate
On 12/08/2017 10:48 AM, Ján Tomko wrote:
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.
Hmmm... I see... Although virDomainControllerDefParseXML does only parse
them if they exist, the generation of <model name='%s'/> is
"hidden", so
splatting the error for what should never happen doesn't seem like a
good idea. Similarly for the other options as well. So I'll make them
all the same generic error message (even though it does seem a bit
strange personally).
>
> Signed-off-by: John Ferlan <jferlan(a)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"
In this case the "autogenerated %s" wasn't single quoted before. I
could single quote it, but that makes it "different" than before. I did
check that all the %s names matched the virDomainControllerModelPCI list
of names. I think it should stay without the single quote.
John
Jan
> +
> virDomainControllerModelPCITypeToString(controller->model));
> + return -1;
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.13.6
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
7:14 p.m.
New subject: [libvirt] [PATCH v3 09/12] qemu: Move PCI command modelName TypeToString to controller def validate
Similar to the checking the modelName vs. NAME_NONE, let's make the
ModelNameTypeToString check more generic too within the checking done
in controller validation (with the same ignore certain models.
NB: We need to keep the ModelNameTypeToString fetch in command line
validation since we use it, but at least we can assume it returns
something valid now.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 59 +++----------------------------------------------
src/qemu/qemu_domain.c | 10 +++++++++
2 files changed, 13 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 45cbf5381..e038abc9d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2719,6 +2719,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
pciopts = &def->opts.pciopts;
+ if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT &&
+ def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
+ modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
@@ -2728,13 +2731,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pci-bridge model name value %d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2759,13 +2755,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
_("autogenerated pci-expander-bus option busNr not
set"));
}
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pci-expander-bus model name value
%d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2788,13 +2777,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown dmi-to-pci-bridge model name value
%d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2812,13 +2794,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, "%s,id=%s", modelName,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pcie-root-port model name value
%d"),
- pciopts->modelName);
- goto error;
- }
if ((pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
(pciopts->modelName !=
@@ -2851,13 +2826,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pcie-switch-upstream-port model name value
%d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2885,13 +2853,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pcie-switch-downstream-port model name
value %d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2918,13 +2879,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
goto error;
}
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pcie-expander-bus model name value
%d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName
!= VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2957,13 +2911,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
if (pciopts->targetIndex == 0)
goto done;
- modelName =
virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
- if (!modelName) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pci-root model name value %d"),
- pciopts->modelName);
- goto error;
- }
if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("PCI controller model name '%s' is not
valid for a pci-root"),
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ecfff4209..050355fe3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4015,6 +4015,7 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
{
virDomainControllerModelPCI model = controller->model;
const virDomainPCIControllerOpts *pciopts;
+ const char *modelName = NULL;
/* skip pcie-root */
if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
@@ -4059,6 +4060,15 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
virDomainControllerModelPCITypeToString(controller->model));
return -1;
}
+
+ modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ if (!modelName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown %s modelName value %d"),
+
virDomainControllerModelPCITypeToString(controller->model),
+ pciopts->modelName);
+ return -1;
+ }
}
return 0;
--
2.13.6
7:14 p.m.
New subject: [libvirt] [PATCH v3 10/12] qemu: Move PCI more command checks to controller def validate
Excluding the qemuCaps checks, move the remainder of the checks
that validate whether the PCI definition is valid or not into
qemuDomainControllerDefValidatePCI.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 96 ----------------------------------
src/qemu/qemu_domain.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e038abc9d..92a7f8641 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2725,20 +2725,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (pciopts->chassisNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-bridge option chassisNr not
set"));
- goto error;
- }
-
- if (pciopts->modelName
- != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a pci-bridge"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the pci-bridge controller "
@@ -2750,19 +2736,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (pciopts->busNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-expander-bus option busNr not
set"));
- }
-
- if (pciopts->modelName
- != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a pci-expander-bus"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the pxb controller "
@@ -2777,14 +2750,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_I82801B11_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a dmi-to-pci-bridge"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the dmi-to-pci-bridge (i82801b11-bridge) "
@@ -2794,16 +2759,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_IOH3420) &&
- (pciopts->modelName !=
- VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a pcie-root-port"),
- modelName);
- goto error;
- }
if ((pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
@@ -2826,14 +2781,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_X3130_UPSTREAM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a
pcie-switch-upstream-port"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the pcie-switch-upstream-port (x3130-upstream)
"
@@ -2844,23 +2791,6 @@ 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->chassis == -1 ||
- pciopts->port == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("autogenerated pcie-switch-downstream-port "
- "options chassis=%d or port=%d not set"),
- pciopts->chassis, pciopts->port);
- goto error;
- }
-
- if (pciopts->modelName
- != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a
pcie-switch-downstream-port"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("The pcie-switch-downstream-port "
@@ -2873,20 +2803,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->chassis, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (pciopts->busNr == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pcie-expander-bus option busNr not
set"));
- goto error;
- }
-
- if (pciopts->modelName
- != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' "
- "is not valid for a pcie-expander-bus"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the pxb-pcie controller "
@@ -2901,22 +2817,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
pciopts->numaNode);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- if (pciopts->targetIndex == -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("autogenerated pci-root option targetIndex not
set"));
- goto error;
- }
-
/* Skip the implicit one */
if (pciopts->targetIndex == 0)
goto done;
- if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("PCI controller model name '%s' is not
valid for a pci-root"),
- modelName);
- goto error;
- }
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("the spapr-pci-host-bridge controller "
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 050355fe3..5007f0029 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4029,6 +4029,8 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
return 0;
+ /* First pass - just check the controller index for the model's
+ * that we care to check... */
switch (model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
@@ -4071,6 +4073,138 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
}
}
+ /* Second pass - now the model specific checks */
+ switch (model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ if (pciopts->chassisNr == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated pci-bridge option chassisNr not
set"));
+ return -1;
+ }
+
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pci-bridge"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
+ if (pciopts->busNr == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated pci-expander-bus option busNr not
set"));
+ return -1;
+ }
+
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pci-expander-bus"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a dmi-to-pci-bridge"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ if ((pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420)
&&
+ (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pcie-root-port"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pcie-switch-upstream-port"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ if (pciopts->chassis == -1 || pciopts->port == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("autogenerated pcie-switch-downstream-port "
+ "options chassis=%d or port=%d not set"),
+ pciopts->chassis, pciopts->port);
+ return -1;
+ }
+
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pcie-switch-downstream-port"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+ if (pciopts->busNr == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated pcie-expander-bus option busNr not
set"));
+ return -1;
+ }
+
+ if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pcie-expander-bus"),
+ modelName);
+ return -1;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ if (pciopts->targetIndex == -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated pci-root option targetIndex not
set"));
+ return -1;
+ }
+
+ /* Skip the implicit one */
+ if (pciopts->targetIndex == 0)
+ return 0;
+
+ if (pciopts->modelName !=
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("PCI controller model name '%s' is not valid
"
+ "for a pci-root"),
+ modelName);
+ return 0;
+ }
+
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+
return 0;
}
--
2.13.6
7:14 p.m.
New subject: [libvirt] [PATCH v3 11/12] qemu: Complete PCI command checks to controller def validate
Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI
and add empty lines between cases so it's a bit easier on the eyes
to read.
This requires two test updates in order to set the correct capability
bit for an xml2xml test as well as setting up the similar capability
for the pseries memlocktest.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 78 ++++++-----------------------------------------
src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
tests/qemumemlocktest.c | 14 +++++++++
tests/qemuxml2xmltest.c | 5 +++-
4 files changed, 105 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 92a7f8641..83431f458 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2725,23 +2725,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
switch ((virDomainControllerModelPCI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pci-bridge controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
modelName, pciopts->chassisNr,
def->info.alias);
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
modelName, pciopts->busNr,
def->info.alias);
@@ -2749,66 +2738,28 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, ",numa_node=%d",
pciopts->numaNode);
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the dmi-to-pci-bridge (i82801b11-bridge) "
- "controller is not supported in this QEMU
binary"));
- goto error;
- }
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_IOH3420) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (ioh3420) "
- "controller is not supported in this QEMU
binary"));
- goto error;
- }
- if ((pciopts->modelName ==
- VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (pcie-root-port) "
- "controller is not supported in this QEMU
binary"));
- goto error;
- }
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
modelName, pciopts->port,
pciopts->chassis, def->info.alias);
break;
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-switch-upstream-port (x3130-upstream)
"
- "controller is not supported in this QEMU
binary"));
- goto error;
- }
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
virBufferAsprintf(&buf, "%s,id=%s", modelName,
def->info.alias);
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The pcie-switch-downstream-port "
- "(xio3130-downstream) controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s",
modelName, pciopts->port,
pciopts->chassis, def->info.alias);
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb-pcie controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s",
modelName, pciopts->busNr,
def->info.alias);
@@ -2816,31 +2767,20 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAsprintf(&buf, ",numa_node=%d",
pciopts->numaNode);
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
/* Skip the implicit one */
if (pciopts->targetIndex == 0)
goto done;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller "
- "is not supported in this QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "%s,index=%d,id=%s",
modelName, pciopts->targetIndex,
def->info.alias);
- if (pciopts->numaNode != -1) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE))
{
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller "
- "doesn't support numa_node on this QEMU
binary"));
- goto error;
- }
+ if (pciopts->numaNode != -1)
virBufferAsprintf(&buf, ",numa_node=%d",
pciopts->numaNode);
- }
break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5007f0029..0b2295129 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4011,7 +4011,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const
virDomainControllerDef *controll
static int
qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller,
- const virDomainDef *def)
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
virDomainControllerModelPCI model = controller->model;
const virDomainPCIControllerOpts *pciopts;
@@ -4090,6 +4091,13 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pci-bridge controller is not supported "
+ "in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
@@ -4107,6 +4115,13 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pxb controller is not supported in this "
+ "QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
@@ -4118,6 +4133,13 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the dmi-to-pci-bridge (i82801b11-bridge) "
+ "controller is not supported in this QEMU
binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
@@ -4130,6 +4152,22 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420)
&&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-root-port (ioh3420) controller "
+ "is not supported in this QEMU binary"));
+ return -1;
+ }
+
+ if ((pciopts->modelName ==
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-root-port (pcie-root-port) controller "
+ "is not supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
@@ -4141,6 +4179,13 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pcie-switch-upstream-port (x3130-upstream) "
+ "controller is not supported in this QEMU
binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
@@ -4160,6 +4205,14 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-switch-downstream-port "
+ "(xio3130-downstream) controller is not "
+ "supported in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
@@ -4177,6 +4230,13 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pxb-pcie controller is not supported "
+ "in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
@@ -4198,6 +4258,21 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
return 0;
}
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the spapr-pci-host-bridge controller is not "
+ "supported in this QEMU binary"));
+ return -1;
+ }
+
+ if (pciopts->numaNode != -1 &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the spapr-pci-host-bridge controller doesn't
"
+ "support numa_node in this QEMU binary"));
+ return -1;
+ }
+
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
@@ -4231,7 +4306,8 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
- ret = qemuDomainDeviceDefValidateControllerPCI(controller, def);
+ ret = qemuDomainDeviceDefValidateControllerPCI(controller, def,
+ qemuCaps);
break;
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
index 62bd25450..bc0b53e6f 100644
--- a/tests/qemumemlocktest.c
+++ b/tests/qemumemlocktest.c
@@ -63,6 +63,7 @@ mymain(void)
{
int ret = 0;
char *fakerootdir;
+ virQEMUCapsPtr qemuCaps = NULL;
if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
fprintf(stderr, "Out of memory\n");
@@ -127,6 +128,16 @@ mymain(void)
DO_TEST("pc-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64);
+ if (!(qemuCaps = virQEMUCapsNew())) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
+ if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0) {
+ ret = -1;
+ goto cleanup;
+ };
DO_TEST("pseries-kvm", 20971520);
DO_TEST("pseries-tcg", 0);
@@ -140,6 +151,9 @@ mymain(void)
DO_TEST("pseries-hardlimit+locked+hostdev", 2147483648);
DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
+ cleanup:
+ virObjectUnref(qemuCaps);
+
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2be8eb2c1..e866fb724 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1310,7 +1310,10 @@ mymain(void)
DO_TEST("intel-iommu-machine",
QEMU_CAPS_MACHINE_OPT,
QEMU_CAPS_MACHINE_IOMMU);
- DO_TEST("intel-iommu-caching-mode", NONE);
+ DO_TEST("intel-iommu-caching-mode",
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420);
DO_TEST("intel-iommu-eim", NONE);
DO_TEST("intel-iommu-device-iotlb", NONE);
--
2.13.6
9:49 a.m.
New subject: [libvirt] [PATCH v3 11/12] qemu: Complete PCI command checks to controller def validate
On Wed, Dec 06, 2017 at 08:14:07PM -0500, John Ferlan wrote:
Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI
and add empty lines between cases so it's a bit easier on the eyes
to read.
Same comment as for 5/12.
Jan
This requires two test updates in order to set the correct capability
bit for an xml2xml test as well as setting up the similar capability
for the pseries memlocktest.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 78 ++++++-----------------------------------------
src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
tests/qemumemlocktest.c | 14 +++++++++
tests/qemuxml2xmltest.c | 5 +++-
4 files changed, 105 insertions(+), 72 deletions(-)
10:15 a.m.
New subject: [libvirt] [PATCH v3 11/12] qemu: Complete PCI command checks to controller def validate
On 12/08/2017 10:49 AM, Ján Tomko wrote:
On Wed, Dec 06, 2017 at 08:14:07PM -0500, John Ferlan wrote:
> Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI
> and add empty lines between cases so it's a bit easier on the eyes
> to read.
>
Same comment as for 5/12.
Jan
For this one it's overkill IMO to generate another patch to add the
spaces, so I'll just drop them as they weren't important other than
aesthetics.
Tks -
John
> This requires two test updates in order to set the correct
capability
> bit for an xml2xml test as well as setting up the similar capability
> for the pseries memlocktest.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 78
> ++++++-----------------------------------------
> src/qemu/qemu_domain.c | 80
> +++++++++++++++++++++++++++++++++++++++++++++++--
> tests/qemumemlocktest.c | 14 +++++++++
> tests/qemuxml2xmltest.c | 5 +++-
> 4 files changed, 105 insertions(+), 72 deletions(-)
>
7:14 p.m.
New subject: [libvirt] [PATCH v3 12/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSATA
Move the SATA controller check from command line building to
controller def validation. This includes copying the SATA
skip check found in qemuBuildSkipController.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 6 ------
src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++++-
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 83431f458..f41b42aac 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2699,12 +2699,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("SATA is not supported with this "
- "QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "ahci,id=%s", def->info.alias);
break;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0b2295129..159876279 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4285,6 +4285,25 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
static int
+qemuDomainDeviceDefValidateControllerSATA(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ /* first SATA controller on Q35 machines is implicit */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
+ controller->idx == 0 && qemuDomainIsQ35(def))
+ return 0;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("SATA is not supported with this QEMU binary"));
+ return -1;
+ }
+ return 0;
+}
+
+
+static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
@@ -4310,8 +4329,12 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
qemuCaps);
break;
- case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+ ret = qemuDomainDeviceDefValidateControllerSATA(controller, def,
+ qemuCaps);
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
--
2.13.6
9:50 a.m.
New subject: [libvirt] [PATCH v3 12/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSATA
On Wed, Dec 06, 2017 at 08:14:08PM -0500, John Ferlan wrote:
Move the SATA controller check from command line building to
controller def validation. This includes copying the SATA
skip check found in qemuBuildSkipController.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 6 ------
src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++++-
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 83431f458..f41b42aac 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2699,12 +2699,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("SATA is not supported with this "
- "QEMU binary"));
- goto error;
- }
virBufferAsprintf(&buf, "ahci,id=%s", def->info.alias);
break;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0b2295129..159876279 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4285,6 +4285,25 @@ qemuDomainDeviceDefValidateControllerPCI(const
virDomainControllerDef *controlle
static int
+qemuDomainDeviceDefValidateControllerSATA(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ /* first SATA controller on Q35 machines is implicit */
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
The type is _SATA, it's in the name fo this function
Jan
+ controller->idx == 0 && qemuDomainIsQ35(def))
+ return 0;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("SATA is not supported with this QEMU binary"));
+ return -1;
+ }
+ return 0;
+}
+
+
+static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
2545
days inactive
2547
days old
27 comments
3 participants
participants (3)
-
Farhan Ali -
John Ferlan -
Ján Tomko