[libvirt] [PATCH v4 00/13] Move qemu command line controller checks to qemuDomainDeviceDefValidateController* checks
v3: https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html Differences since v3: * Pushed first 4 ACK'd patches of v3 * Rework/Separate out a few patches for the SCSI handling * Alter the PCI handling as requested by code review (although I still had a question to a review comment regarding whether or not the model should be handled in PostParse). * Alter the SATA code as indicated by code review * Add Andrea's patch into the series Andrea Bolognani (1): qemu: Add missing checks for pcie-root-port options John Ferlan (12): qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes qemu: Move model set for qemuBuildControllerDevStr qemu: Adjust SCSI controller switch in qemuBuildControllerDevStr qemu: Add check for iothread attribute in validate controller 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 src/qemu/qemu_command.c | 403 ++++------------------------------------------ src/qemu/qemu_domain.c | 420 +++++++++++++++++++++++++++++++++++++++++++++++- tests/qemumemlocktest.c | 14 ++ tests/qemuxml2xmltest.c | 5 +- 4 files changed, 466 insertions(+), 376 deletions(-) -- 2.13.6
Move the checks that various attributes are not set on any controller other than SCSI controller using virtio-scsi model into the common controller validate checks. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 24 ------------------------ src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2dd50a214..d9cbdff83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2667,30 +2667,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 347fc0742..120c013bd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3893,6 +3893,38 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) static int +qemuDomainDeviceDefValidateControllerAttributes(const virDomainControllerDef *controller, + int model) +{ + if (!(controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && + 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; + } + } + + return 0; +} + + +static int qemuDomainDeviceDefValidateControllerIDE(const virDomainControllerDef *controller, const virDomainDef *def) { @@ -3924,11 +3956,20 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, virQEMUCapsPtr qemuCaps) { int ret = 0; + int model = controller->model; if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, "controller")) return -1; + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + } + + if (qemuDomainDeviceDefValidateControllerAttributes(controller, model) < 0) + return -1; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: ret = qemuDomainDeviceDefValidateControllerIDE(controller, def); -- 2.13.6
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
Move the checks that various attributes are not set on any controller other than SCSI controller using virtio-scsi model into the common controller validate checks.
You not only move the checks, but also add the call 'qemuDomainSetSCSIControllerModel'.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 24 ------------------------ src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2dd50a214..d9cbdff83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2667,30 +2667,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 347fc0742..120c013bd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3893,6 +3893,38 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int +qemuDomainDeviceDefValidateControllerAttributes(const virDomainControllerDef *controller, + int model) +{ + if (!(controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && + 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; + } + } + + return 0; +} + + +static int qemuDomainDeviceDefValidateControllerIDE(const virDomainControllerDef *controller, const virDomainDef *def) { @@ -3924,11 +3956,20 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, virQEMUCapsPtr qemuCaps) { int ret = 0; + int model = controller->model;
if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, "controller")) return -1;
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + }
Didn't take a closer look, but is it the right place for this? (in a validation function)
+ + if (qemuDomainDeviceDefValidateControllerAttributes(controller, model) < 0) + return -1; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: ret = qemuDomainDeviceDefValidateControllerIDE(controller, def); -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 01/04/2018 05:23 AM, Marc Hartmayer wrote:
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
Move the checks that various attributes are not set on any controller other than SCSI controller using virtio-scsi model into the common controller validate checks.
You not only move the checks, but also add the call 'qemuDomainSetSCSIControllerModel'.
True, but it's required... Something noted in the v3 review regarding that this Set function is more like a Get function and a question over whether we could move it to post-parse. The question and my thoughts from that cut-n-pasted here since this is the more recent review:
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. ...
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 24 ------------------------ src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2dd50a214..d9cbdff83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2667,30 +2667,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 347fc0742..120c013bd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3893,6 +3893,38 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
static int +qemuDomainDeviceDefValidateControllerAttributes(const virDomainControllerDef *controller, + int model) +{ + if (!(controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && + 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; + } + } + + return 0; +} + + +static int qemuDomainDeviceDefValidateControllerIDE(const virDomainControllerDef *controller, const virDomainDef *def) { @@ -3924,11 +3956,20 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, virQEMUCapsPtr qemuCaps) { int ret = 0; + int model = controller->model;
if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, "controller")) return -1;
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + }
Didn't take a closer look, but is it the right place for this? (in a validation function)
As noted above - this ends up being required in order to "get" the right model type for SCSI because we don't set a default otherwise during post parse. I'm somewhat conflicted over what to do and how much (more) effort to make to this series which I originally thought would be a simple move from one place to another, but now is taking on a different direction/life <sigh>... John
+ + if (qemuDomainDeviceDefValidateControllerAttributes(controller, model) < 0) + return -1; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: ret = qemuDomainDeviceDefValidateControllerIDE(controller, def); -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Beste Grüße / Kind regards Marc Hartmayer
IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Thu, Jan 04, 2018 at 02:12 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
On 01/04/2018 05:23 AM, Marc Hartmayer wrote:
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
Move the checks that various attributes are not set on any controller other than SCSI controller using virtio-scsi model into the common controller validate checks.
[…snip…]
virQEMUCapsPtr qemuCaps) { int ret = 0; + int model = controller->model;
if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, "controller")) return -1;
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + }
Didn't take a closer look, but is it the right place for this? (in a validation function)
As noted above - this ends up being required in order to "get" the right model type for SCSI because we don't set a default otherwise during post parse. I'm somewhat conflicted over what to do and how much (more) effort to make to this series which I originally thought would be a simple move from one place to another, but now is taking on a different direction/life <sigh>...
Hmm - if you want to you can leave your patch at it is. It’s already much better than before. But I would definitely split 'qemuDomainSetSCSIControllerModel' into two separate functions as it does two distinct things. It either checks if the passed controller model is supported by the QEMU caps or it sets the controller model. “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.” Do you mean with your comment the same as I suggested above?
John
+ + if (qemuDomainDeviceDefValidateControllerAttributes(controller, model) < 0) + return -1; + switch ((virDomainControllerType) controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_IDE: ret = qemuDomainDeviceDefValidateControllerIDE(controller, def); -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Beste Grüße / Kind regards Marc Hartmayer
IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
-- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 01/05/2018 10:20 AM, Marc Hartmayer wrote:
On Thu, Jan 04, 2018 at 02:12 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
On 01/04/2018 05:23 AM, Marc Hartmayer wrote:
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
Move the checks that various attributes are not set on any controller other than SCSI controller using virtio-scsi model into the common controller validate checks.
[…snip…]
virQEMUCapsPtr qemuCaps) { int ret = 0; + int model = controller->model;
if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps, "controller")) return -1;
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0) + return -1; + }
Didn't take a closer look, but is it the right place for this? (in a validation function)
As noted above - this ends up being required in order to "get" the right model type for SCSI because we don't set a default otherwise during post parse. I'm somewhat conflicted over what to do and how much (more) effort to make to this series which I originally thought would be a simple move from one place to another, but now is taking on a different direction/life <sigh>...
Hmm - if you want to you can leave your patch at it is. It’s already much better than before.
But I would definitely split 'qemuDomainSetSCSIControllerModel' into two separate functions as it does two distinct things. It either checks if the passed controller model is supported by the QEMU caps or it sets the controller model.
“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.”
Do you mean with your comment the same as I suggested above?
The split-up is done - there's a new series to include it as well as the patch you posted... Should make for hours of entertaining reading ;-) John
We can move the @model reset inside the switch case for VIR_DOMAIN_CONTROLLER_TYPE_SCSI since it is the only case that uses an altered model name. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d9cbdff83..7aa5f5dd1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2662,13 +2662,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, *devstr = NULL; - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + switch ((virDomainControllerType) def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) return -1; - } - switch ((virDomainControllerType) def->type) { - case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { -- 2.13.6
Modify the SCSI controller switch during command line building to account for all virDomainControllerModelSCSI types rather than using the default label. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7aa5f5dd1..d3a993fb1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2667,7 +2667,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) return -1; - switch (model) { + 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"); @@ -2707,7 +2707,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)); -- 2.13.6
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan@redhat.com> wrote:
Modify the SCSI controller switch during command line building to account for all virDomainControllerModelSCSI types rather than using the default label.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7aa5f5dd1..d3a993fb1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2667,7 +2667,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) return -1;
- switch (model) { + 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"); @@ -2707,7 +2707,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)); -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> (haven’t tested it) -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Let's make sure that non SCSI virtio-scsi isn't used for any type other than a virtio-scsi controller. This includes removing the check from qemuCheckSCSIControllerIOThreads which is a very suble difference because although def->model was used in the original comparison and just @model is used in the new comparison, the comparison is the same. This is because qemuDomainSetSCSIControllerModel doesn't change the def->model, thus we know that the resultant @model would result in either the same as input or if not set, whatever the default model is when def->model == -1. In this second case, virtio-scsi is the last default chosen. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 8 -------- src/qemu/qemu_domain.c | 5 +++++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d3a993fb1..bdac01414 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2601,14 +2601,6 @@ qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef, 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", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 120c013bd..144439218 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3918,6 +3918,11 @@ qemuDomainDeviceDefValidateControllerAttributes(const virDomainControllerDef *co _("'ioeventfd' is only supported by virtio-scsi controller")); return -1; } + if (controller->iothread) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'iothread' is only supported for virtio-scsi controller")); + return -1; + } } return 0; -- 2.13.6
Move SCSI validation from qemu_command into qemu_domain. Rename/reorder the args in qemuCheckSCSIControllerIOThreads to match the caller as well as fixing up the comments to remove the previously removed qemuCaps arg. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 48 ++-------------------------------- src/qemu/qemu_domain.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bdac01414..5995ebe90 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2583,44 +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->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 @@ -2663,12 +2625,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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"); @@ -2677,12 +2636,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; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 144439218..37626a6f2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3955,6 +3955,69 @@ 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, + int model, + const virDomainDef *def) +{ + 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, @@ -3980,8 +4043,11 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, ret = qemuDomainDeviceDefValidateControllerIDE(controller, def); break; - case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + ret = qemuDomainDeviceDefValidateControllerSCSI(controller, model, 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: -- 2.13.6
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@redhat.com> --- src/qemu/qemu_command.c | 20 -------------------- src/qemu/qemu_domain.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5995ebe90..673924536 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2720,26 +2720,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 37626a6f2..787367ca0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4019,6 +4019,48 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll static int +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller, + const virDomainDef *def) +{ + virDomainControllerModelPCI model = controller->model; + + /* skip pcie-root */ + if (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->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) @@ -4047,12 +4089,15 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, ret = qemuDomainDeviceDefValidateControllerSCSI(controller, model, def); 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
From: Andrea Bolognani <abologna@redhat.com> We format the 'chassis' and 'port' properties on the QEMU command line later on, so we should make sure they've been set. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 673924536..85cb5a3b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2823,7 +2823,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (def->opts.pciopts.modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + def->opts.pciopts.chassis == -1 || + def->opts.pciopts.port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-root-port options not set")); goto error; -- 2.13.6
Shorten up a few characters and reference the pciopts pointer Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 118 ++++++++++++++++++++++++------------------------ 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 85cb5a3b3..811579246 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; @@ -2718,24 +2719,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' " @@ -2750,26 +2753,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' " @@ -2784,28 +2787,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' " @@ -2822,24 +2825,23 @@ 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 || - def->opts.pciopts.chassis == -1 || - def->opts.pciopts.port == -1) { + pciopts->chassis == -1 || pciopts->port == -1) { 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' " @@ -2847,7 +2849,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", @@ -2855,7 +2857,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", @@ -2865,24 +2867,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' " @@ -2900,24 +2902,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' " @@ -2933,26 +2935,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' " @@ -2967,32 +2969,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); @@ -3005,17 +3007,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
Move the various modelName == NAME_NONE from the command line generation into domain controller validation. Also rather than have multiple cases with the same check, let's make the code more generic, but also note that it was the modelName option that caused the failure. We also have to be sure not to check the PCI models that we don't care about. For the remaining checks in command line building, we can use the field name in the error message to be more specific about what causes the failure. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 36 ++++++------------------------------ src/qemu/qemu_domain.c | 12 ++++++++++++ 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 811579246..cc8cc1afe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2723,9 +2723,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->chassisNr == -1) { + if (pciopts->chassisNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-bridge options not set")); goto error; @@ -2757,9 +2755,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->busNr == -1) { + if (pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-expander-bus options not set")); goto error; @@ -2794,13 +2790,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated dmi-to-pci-bridge options not set")); - goto error; - } - modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2825,9 +2814,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->chassis == -1 || pciopts->port == -1) { + if (pciopts->chassis == -1 || pciopts->port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-root-port options not set")); goto error; @@ -2871,12 +2858,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-switch-upstream-port options not set")); - goto error; - } modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2902,9 +2883,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->chassis == -1 || + if (pciopts->chassis == -1 || pciopts->port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-switch-downstream-port " @@ -2939,9 +2918,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->chassis, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - if (pciopts->modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->busNr == -1) { + if (pciopts->busNr == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-expander-bus options not set")); goto error; @@ -2976,8 +2953,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || - pciopts->targetIndex == -1) { + if (pciopts->targetIndex == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pci-root options not set")); goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 787367ca0..9c843d8b5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4023,6 +4023,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle const virDomainDef *def) { virDomainControllerModelPCI model = controller->model; + const virDomainPCIControllerOpts *pciopts; /* skip pcie-root */ if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) @@ -4056,6 +4057,17 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle break; } + pciopts = &controller->opts.pciopts; + if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT && + controller->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST) { + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("autogenerated %s options not set"), + virDomainControllerModelPCITypeToString(controller->model)); + return -1; + } + } + return 0; } -- 2.13.6
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@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 cc8cc1afe..18ede87d6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2720,6 +2720,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: @@ -2729,13 +2732,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, @@ -2761,13 +2757,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, goto error; } - 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, @@ -2790,13 +2779,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, @@ -2819,13 +2801,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, _("autogenerated pcie-root-port options not set")); goto error; } - 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 != @@ -2858,13 +2833,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, @@ -2891,13 +2859,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, @@ -2924,13 +2885,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, @@ -2963,13 +2917,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 9c843d8b5..376ad63fc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4024,6 +4024,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle { virDomainControllerModelPCI model = controller->model; const virDomainPCIControllerOpts *pciopts; + const char *modelName = NULL; /* skip pcie-root */ if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) @@ -4066,6 +4067,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
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@redhat.com> --- src/qemu/qemu_command.c | 101 ----------------------------------- src/qemu/qemu_domain.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 101 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 18ede87d6..448fdd549 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2726,20 +2726,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 options 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 " @@ -2751,20 +2737,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 options not set")); - goto error; - } - - 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 " @@ -2779,14 +2751,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) " @@ -2796,21 +2760,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->chassis == -1 || pciopts->port == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("autogenerated pcie-root-port options not set")); - goto error; - } - 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)) { @@ -2833,14 +2782,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) " @@ -2851,22 +2792,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, "%s", - _("autogenerated pcie-switch-downstream-port " - "options not set")); - 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 " @@ -2879,20 +2804,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 options 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 " @@ -2907,22 +2818,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 options 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 376ad63fc..a1b5ee46d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4036,6 +4036,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: @@ -4078,6 +4080,143 @@ 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 options 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 options 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->chassis == -1 || pciopts->port == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pcie-root-port options not set")); + return -1; + } + + 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, "%s", + _("autogenerated pcie-switch-downstream-port " + "options not set")); + 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 options 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 options 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
Move the qemuCaps checks over to qemuDomainControllerDefValidatePCI. 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@redhat.com> --- src/qemu/qemu_command.c | 70 +------------------------------------------ src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++-- tests/qemumemlocktest.c | 14 +++++++++ tests/qemuxml2xmltest.c | 5 +++- 4 files changed, 97 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 448fdd549..d67cefa60 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2726,23 +2726,11 @@ 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); @@ -2751,65 +2739,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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; - } - 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; - } - 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); @@ -2822,25 +2767,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, 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: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a1b5ee46d..a579dba54 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4020,7 +4020,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; @@ -4097,6 +4098,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: @@ -4114,6 +4122,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: @@ -4125,6 +4140,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: @@ -4143,6 +4165,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: @@ -4154,6 +4192,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: @@ -4172,6 +4217,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: @@ -4189,6 +4242,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: @@ -4210,6 +4270,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: @@ -4251,7 +4326,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
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@redhat.com> --- src/qemu/qemu_command.c | 6 ------ src/qemu/qemu_domain.c | 24 +++++++++++++++++++++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d67cefa60..45325a587 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2700,12 +2700,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 a579dba54..feda3e5ed 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4297,6 +4297,24 @@ 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->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) @@ -4330,8 +4348,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
ping? Tks - John On 12/12/2017 10:06 AM, John Ferlan wrote:
v3: https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html
Differences since v3:
* Pushed first 4 ACK'd patches of v3
* Rework/Separate out a few patches for the SCSI handling
* Alter the PCI handling as requested by code review (although I still had a question to a review comment regarding whether or not the model should be handled in PostParse).
* Alter the SATA code as indicated by code review
* Add Andrea's patch into the series
Andrea Bolognani (1): qemu: Add missing checks for pcie-root-port options
John Ferlan (12): qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes qemu: Move model set for qemuBuildControllerDevStr qemu: Adjust SCSI controller switch in qemuBuildControllerDevStr qemu: Add check for iothread attribute in validate controller 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
src/qemu/qemu_command.c | 403 ++++------------------------------------------ src/qemu/qemu_domain.c | 420 +++++++++++++++++++++++++++++++++++++++++++++++- tests/qemumemlocktest.c | 14 ++ tests/qemuxml2xmltest.c | 5 +- 4 files changed, 466 insertions(+), 376 deletions(-)
Use a switch statement instead of if-else-if statements. Move the command line building of the iothread attribute into the common path as the SCSI controller attributes are already validated. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- This patch fits well to this series as it was created during the review :) The default cases in the switch statements should be replaced by the appropriate types. --- src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1c9a802de73d..c82497860eb4 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; + int address_type = def->info.type; const virDomainPCIControllerOpts *pciopts; const char *modelName = NULL; @@ -2624,23 +2625,25 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, switch ((virDomainControllerModelSCSI) model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: - if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + switch ((virDomainDeviceAddressType) address_type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: virBufferAddLit(&buf, "virtio-scsi-ccw"); - if (def->iothread) - virBufferAsprintf(&buf, ",iothread=iothread%u", - def->iothread); - } else if (def->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: virBufferAddLit(&buf, "virtio-scsi-s390"); - } else if (def->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: virBufferAddLit(&buf, "virtio-scsi-device"); - } else { + break; + default: virBufferAddLit(&buf, "virtio-scsi-pci"); - if (def->iothread) - virBufferAsprintf(&buf, ",iothread=iothread%u", - def->iothread); } + + if (def->iothread) { + virBufferAsprintf(&buf, ",iothread=iothread%u", + def->iothread); + } + if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) goto error; break; @@ -2668,18 +2671,20 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: - if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + switch ((virDomainDeviceAddressType) address_type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: virBufferAddLit(&buf, "virtio-serial-pci"); - } else if (def->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: virBufferAddLit(&buf, "virtio-serial-ccw"); - } else if (def->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: virBufferAddLit(&buf, "virtio-serial-s390"); - } else if (def->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: virBufferAddLit(&buf, "virtio-serial-device"); - } else { + break; + default: virBufferAddLit(&buf, "virtio-serial"); } virBufferAsprintf(&buf, ",id=%s", def->info.alias); -- 2.13.4
participants (2)
-
John Ferlan -
Marc Hartmayer