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