Move SCSI validation from qemu_command into qemu_domain.
This includes the @model reset/check when the controller
model hasn't yet been set. While at it modify the switch
to account for all virDomainControllerModelSCSI types
rather than using the default label.
Rename/reorder the args in qemuCheckSCSIControllerIOThreads
to match the caller and also remove the unnecessary model
check as well as fixing up the comments to remove the previously
removed qemuCaps arg.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/qemu/qemu_command.c | 94 +++++------------------------------------------
src/qemu/qemu_domain.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 86 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2dd50a214..cdd267675 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2583,52 +2583,6 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
}
-/* qemuCheckSCSIControllerIOThreads:
- * @domainDef: Pointer to domain def
- * @def: Pointer to controller def
- * @qemuCaps: Capabilities
- *
- * If this controller definition has iothreads set, let's make sure the
- * configuration is right before adding to the command line
- *
- * Returns true if either supported or there are no iothreads for controller;
- * otherwise, returns false if configuration is not quite right.
- */
-static bool
-qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef,
- virDomainControllerDefPtr def)
-{
- if (!def->iothread)
- return true;
-
- if (def->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("IOThreads only supported for virtio-scsi "
- "controllers model is '%s'"),
- virDomainControllerModelSCSITypeToString(def->model));
- return false;
- }
-
- if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
- def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("IOThreads only available for virtio pci and "
- "virtio ccw controllers"));
- return false;
- }
-
- /* Can we find the controller iothread in the iothreadid list? */
- if (!virDomainIOThreadIDFind(domainDef, def->iothread)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("controller iothread '%u' not defined in iothreadid"),
- def->iothread);
- return false;
- }
-
- return true;
-}
-
-
/**
* qemuBuildControllerDevStr:
* @domainDef: domain definition
@@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
*devstr = NULL;
- if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
- return -1;
- }
-
- if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
- if (def->queues) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'queues' is only supported by virtio-scsi controller"));
- return -1;
- }
- if (def->cmd_per_lun) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'cmd_per_lun' is only supported by virtio-scsi controller"));
- return -1;
- }
- if (def->max_sectors) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'max_sectors' is only supported by virtio-scsi controller"));
- return -1;
- }
- if (def->ioeventfd) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'ioeventfd' is only supported by virtio-scsi controller"));
- return -1;
- }
- }
-
switch ((virDomainControllerType) def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- switch (model) {
+ if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
+ return -1;
+ switch ((virDomainControllerModelSCSI) model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
virBufferAddLit(&buf, "virtio-scsi-ccw");
- if (def->iothread) {
- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
- goto error;
+ if (def->iothread)
virBufferAsprintf(&buf, ",iothread=iothread%u",
def->iothread);
- }
} else if (def->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
virBufferAddLit(&buf, "virtio-scsi-s390");
@@ -2711,12 +2635,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
virBufferAddLit(&buf, "virtio-scsi-device");
} else {
virBufferAddLit(&buf, "virtio-scsi-pci");
- if (def->iothread) {
- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
- goto error;
+ if (def->iothread)
virBufferAsprintf(&buf, ",iothread=iothread%u",
def->iothread);
- }
}
if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0)
goto error;
@@ -2733,7 +2654,10 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
virBufferAddLit(&buf, "megasas");
break;
- default:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported controller model: %s"),
virDomainControllerModelSCSITypeToString(def->model));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 076c68c9f..428db1193 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3918,6 +3918,97 @@ qemuDomainDeviceDefValidateControllerIDE(const virDomainControllerDef *controlle
}
+/* qemuDomainCheckSCSIControllerIOThreads:
+ * @controller: Pointer to controller def
+ * @def: Pointer to domain def
+ *
+ * If this controller definition has iothreads set, let's make sure the
+ * configuration is right before adding to the command line
+ *
+ * Returns true if either supported or there are no iothreads for controller;
+ * otherwise, returns false if configuration is not quite right.
+ */
+static bool
+qemuDomainCheckSCSIControllerIOThreads(const virDomainControllerDef *controller,
+ const virDomainDef *def)
+{
+ if (!controller->iothread)
+ return true;
+
+ if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+ controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("IOThreads only available for virtio pci and "
+ "virtio ccw controllers"));
+ return false;
+ }
+
+ /* Can we find the controller iothread in the iothreadid list? */
+ if (!virDomainIOThreadIDFind(def, controller->iothread)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("controller iothread '%u' not defined in iothreadid"),
+ controller->iothread);
+ return false;
+ }
+
+ return true;
+}
+
+
+static int
+qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controller,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ int model = controller->model;
+
+ if ((qemuDomainSetSCSIControllerModel(def, qemuCaps, &model)) < 0)
+ return -1;
+
+ if (model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) {
+ if (controller->queues) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'queues' is only supported by virtio-scsi controller"));
+ return -1;
+ }
+ if (controller->cmd_per_lun) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'cmd_per_lun' is only supported by virtio-scsi controller"));
+ return -1;
+ }
+ if (controller->max_sectors) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'max_sectors' is only supported by virtio-scsi controller"));
+ return -1;
+ }
+ if (controller->ioeventfd) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'ioeventfd' is only supported by virtio-scsi controller"));
+ return -1;
+ }
+ }
+
+ switch ((virDomainControllerModelSCSI) model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!qemuDomainCheckSCSIControllerIOThreads(controller, def))
+ return -1;
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+ break;
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
const virDomainDef *def,
@@ -3934,8 +4025,12 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
ret = qemuDomainDeviceDefValidateControllerIDE(controller, def);
break;
- case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
+ ret = qemuDomainDeviceDefValidateControllerSCSI(controller, def,
+ qemuCaps);
+ break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Dec 06, 2017 at 08:14:01PM -0500, John Ferlan wrote:
>Move SCSI validation from qemu_command into qemu_domain.
>This includes the @model reset/check when the controller
>model hasn't yet been set. While at it modify the switch
>to account for all virDomainControllerModelSCSI types
>rather than using the default label.
'While at it' in the commit message is usually an indicator
of a change that should have been in a separate commit.
>
>Rename/reorder the args in qemuCheckSCSIControllerIOThreads
>to match the caller and also remove the unnecessary model
>check as well as fixing up the comments to remove the previously
>removed qemuCaps arg.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/qemu/qemu_command.c | 94 +++++------------------------------------------
> src/qemu/qemu_domain.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 105 insertions(+), 86 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 2dd50a214..cdd267675 100644
[...]
>@@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>
> *devstr = NULL;
>
>- if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
>- if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
>- return -1;
>- }
>-
>- if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
>- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
These errors are reported for non-SCSI controllers too.
>- if (def->queues) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>- _("'queues' is only supported by virtio-scsi controller"));
>- return -1;
>- }
>- if (def->cmd_per_lun) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>- _("'cmd_per_lun' is only supported by virtio-scsi controller"));
>- return -1;
>- }
>- if (def->max_sectors) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>- _("'max_sectors' is only supported by virtio-scsi controller"));
>- return -1;
>- }
>- if (def->ioeventfd) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>- _("'ioeventfd' is only supported by virtio-scsi controller"));
>- return -1;
>- }
>- }
>-
> switch ((virDomainControllerType) def->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
>- switch (model) {
>+ if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
>+ return -1;
After this patch, this function is called twice. Also it's called
SetModel, while it's behaving as GetModel. Can we start assigning the
default model in post-parse without breaking backwards compatibility?
Jan
>+ switch ((virDomainControllerModelSCSI) model) {
> case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
> if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> virBufferAddLit(&buf, "virtio-scsi-ccw");
>- if (def->iothread) {
>- if (!qemuCheckSCSIControllerIOThreads(domainDef, def))
>- goto error;
>+ if (def->iothread)
> virBufferAsprintf(&buf, ",iothread=iothread%u",
> def->iothread);
>- }
> } else if (def->info.type ==
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
> virBufferAddLit(&buf, "virtio-scsi-s390");
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12/08/2017 10:37 AM, Ján Tomko wrote:
> On Wed, Dec 06, 2017 at 08:14:01PM -0500, John Ferlan wrote:
>> Move SCSI validation from qemu_command into qemu_domain.
>> This includes the @model reset/check when the controller
>> model hasn't yet been set. While at it modify the switch
>> to account for all virDomainControllerModelSCSI types
>> rather than using the default label.
>
> 'While at it' in the commit message is usually an indicator
> of a change that should have been in a separate commit.
>
Sure... That's fine. Couple more patches added...
>>
>> Rename/reorder the args in qemuCheckSCSIControllerIOThreads
>> to match the caller and also remove the unnecessary model
>> check as well as fixing up the comments to remove the previously
>> removed qemuCaps arg.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/qemu/qemu_command.c | 94
>> +++++------------------------------------------
>> src/qemu/qemu_domain.c | 97
>> ++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 105 insertions(+), 86 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 2dd50a214..cdd267675 100644
>
> [...]
>
>> @@ -2662,47 +2616,17 @@ qemuBuildControllerDevStr(const virDomainDef
>> *domainDef,
>>
>> *devstr = NULL;
>>
>> - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
>> - if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps,
>> &model)) < 0)
>> - return -1;
>> - }
Oh and of course I cannot remove this since this is called from hotplug
code we need to perhaps alter the model here...
>> -
>> - if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
>> - model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
>
> These errors are reported for non-SCSI controllers too.
>
Hmm.. true, dang compound conditionals...
>> - if (def->queues) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("'queues' is only supported by
>> virtio-scsi controller"));
>> - return -1;
>> - }
>> - if (def->cmd_per_lun) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("'cmd_per_lun' is only supported by
>> virtio-scsi controller"));
>> - return -1;
>> - }
>> - if (def->max_sectors) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("'max_sectors' is only supported by
>> virtio-scsi controller"));
>> - return -1;
>> - }
>> - if (def->ioeventfd) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> - _("'ioeventfd' is only supported by
>> virtio-scsi controller"));
>> - return -1;
>> - }
>> - }
>> -
>> switch ((virDomainControllerType) def->type) {
>> case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
>> - switch (model) {
>> + if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps,
>> &model)) < 0)
>> + return -1;
>
> After this patch, this function is called twice. Also it's called
> SetModel, while it's behaving as GetModel. Can we start assigning the
> default model in post-parse without breaking backwards compatibility?
>
> Jan
>
Are you asking that during PostParse callback for us to set def->model
so that we can just use it later on? That'd make things a lot easier
later on, but that seems a bit outside of what I was trying to do though.
Anyway based on this series and another I have posted... That means for
someone that didn't set the model, that a model would then be set and
written out... Meaning adjustment of a number of tests because now there
would be a model defined. It also could mean we cannot adjust the
default model - one could say for example that using lsi as the default
model for scsi_host passthrough is perhaps less optimal than using
virtio-scsi... In fact, based on a different series and some testing
done - it may not work either.
As for GetModel vs. SetModel - I think that's a different problem and
can be put on a list of things to do after this is done.
Tks -
John
>> + switch ((virDomainControllerModelSCSI) model) {
>> case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
>> if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
>> virBufferAddLit(&buf, "virtio-scsi-ccw");
>> - if (def->iothread) {
>> - if (!qemuCheckSCSIControllerIOThreads(domainDef,
>> def))
>> - goto error;
>> + if (def->iothread)
>> virBufferAsprintf(&buf, ",iothread=iothread%u",
>> def->iothread);
>> - }
>> } else if (def->info.type ==
>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
>> virBufferAddLit(&buf, "virtio-scsi-s390");
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.