[libvirt] [PATCH v4 00/13] Move qemu command line controller checks to qemuDomainDeviceDefValidateController* checks
9:06 a.m.
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
9:06 a.m.
New subject: [libvirt] [PATCH v4 01/13] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes
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(a)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
4:23 a.m.
New subject: [libvirt] [PATCH v4 01/13] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan(a)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(a)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(a)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
7:12 a.m.
New subject: [libvirt] [PATCH v4 01/13] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes
On 01/04/2018 05:23 AM, Marc Hartmayer wrote:
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan
<jferlan(a)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(a)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(a)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
9:20 a.m.
New subject: [libvirt] [PATCH v4 01/13] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes
On Thu, Jan 04, 2018 at 02:12 PM +0100, John Ferlan <jferlan(a)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(a)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(a)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
5:49 p.m.
New subject: [libvirt] [PATCH v4 01/13] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes
On 01/05/2018 10:20 AM, Marc Hartmayer wrote:
On Thu, Jan 04, 2018 at 02:12 PM +0100, John Ferlan
<jferlan(a)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(a)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
9:06 a.m.
New subject: [libvirt] [PATCH v4 02/13] qemu: Move model set for qemuBuildControllerDevStr
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(a)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
9:06 a.m.
New subject: [libvirt] [PATCH v4 03/13] qemu: Adjust SCSI controller switch in qemuBuildControllerDevStr
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(a)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
4:28 a.m.
New subject: [libvirt] [PATCH v4 03/13] qemu: Adjust SCSI controller switch in qemuBuildControllerDevStr
On Tue, Dec 12, 2017 at 04:06 PM +0100, John Ferlan <jferlan(a)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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-by: Marc Hartmayer <mhartmay(a)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
9:06 a.m.
New subject: [libvirt] [PATCH v4 04/13] qemu: Add check for iothread attribute in validate controller
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(a)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
9:07 a.m.
New subject: [libvirt] [PATCH v4 05/13] qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI
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(a)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
9:07 a.m.
New subject: [libvirt] [PATCH v4 06/13] 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 | 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
9:07 a.m.
New subject: [libvirt] [PATCH v4 07/13] qemu: Add missing checks for pcie-root-port options
From: Andrea Bolognani <abologna(a)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(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)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
9:07 a.m.
New subject: [libvirt] [PATCH v4 08/13] 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 | 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
9:07 a.m.
New subject: [libvirt] [PATCH v4 09/13] 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 | 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
9:07 a.m.
New subject: [libvirt] [PATCH v4 10/13] 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 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
9:07 a.m.
New subject: [libvirt] [PATCH v4 11/13] 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 | 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
9:07 a.m.
New subject: [libvirt] [PATCH v4 12/13] qemu: Complete PCI command checks to controller def validate
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(a)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
9:07 a.m.
New subject: [libvirt] [PATCH v4 13/13] 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 | 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
8:47 a.m.
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(-)
5:41 a.m.
New subject: [libvirt] [PATCH] qemu: Use switch statement for address types in qemuBuildControllerDevStr
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(a)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
2547
days inactive
2571
days old
20 comments
2 participants
participants (2)
-
John Ferlan -
Marc Hartmayer