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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
