Re: [libvirt] [PATCH v2 24/25] qemu: Support scsi controller model=virtio-{non-}transitional
On Wed, 2019-01-23 at 16:32 -0500, Cole Robinson wrote:
[...]
+++ b/docs/schemas/domaincommon.rng
@@ -2153,6 +2153,8 @@
<value>ibmvscsi</value>
<value>virtio-scsi</value>
<value>lsisas1078</value>
+ <value>virtio-transitional</value>
+ <value>virtio-non-transitional</value>
As mentioned during the previous round of reviews, I think we should
support model='virtio' (which would behave the same as the existing
model='virtio-scsi') in order to have a nice, consistent experience
for users and management application developers.
[...]
@@ -4859,11 +4862,12 @@
virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
virDomainControllerDefPtr cdev = dev->data.controller;
if (cdev->iothread &&
- cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) {
- virReportError(VIR_ERR_XML_ERROR,
+ cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI &&
+ cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL
&&
+ cdev->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL)
{
+ virReportError(VIR_ERR_XML_ERROR, "%s",
_("'iothread' attribute only supported for
"
- "controller model '%s'"),
-
virDomainControllerModelSCSITypeToString(VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI));
+ "virtio scsi controllers"));
return -1;
}
You could also use
virReportError(VIR_ERR_XML_ERROR,
_("'iothread' attribute not supported for "
"controller model '%s'"),
virDomainControllerModelSCSITypeToString(cdev->model));
I usually prefer that pattern, but either way works.
A more interesting change would be to move this check...
[...]
qemuDomainDeviceDefValidateControllerAttributes(const
virDomainControllerDef *controller)
{
if (!(controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
+ (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI ||
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL
||
+ controller->model ==
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL))) {
if (controller->queues) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'queues' is only supported by virtio-scsi
controller"));
... here in a small preparatory patch, which will also lead to
adding the new models to one less location in this one. But again,
that's just bikeshedding and either way is fine :)
--
Andrea Bolognani / Red Hat / Virtualization