Re: [PATCH v3 05/12] qemu: Introduce qemuDomainDeviceDefValidateControllerSCSI

[Ján Tomko <jtomko@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> 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");

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list