Re: [PATCH v3 13/17] conf: Add <input model='virtio-{non-}transitional'/>

[Andrea Bolognani <abologna@xxxxxxxxxx>]

On Fri, 2019-02-08 at 17:12 -0500, Cole Robinson wrote:
[...]
> @@ -13208,6 +13217,13 @@ virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt,
>          goto error;
>      }
>  
> +    if (model &&
> +        (def->model = virDomainInputModelTypeFromString(model)) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("unknown input model '%s'"), model);
> +        goto error;
> +    }

Same comment as 9/17 as far as parsing the model is concerned.

[...]
> @@ -26810,6 +26836,15 @@ virDomainInputDefFormat(virBufferPtr buf,
>      virBufferAsprintf(buf, "<input type='%s' bus='%s'",
>                        type, bus);
>  
> +    if (def->model) {
> +        if (!model) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("unexpected input model %d"), def->model);
> +            goto cleanup;
> +        }
> +        virBufferAsprintf(buf, " model='%s'", model);
> +    }

Since def->model is optional, I think this would be a bit more
legible as

  if (def->model) {
      const char *model = virDomainInputModelTypeToString(def->model);

      if (!model) {
          virReportError(VIR_ERR_INTERNAL_ERROR,
                         _("unexpected input model %d"), def->model);
          goto cleanup;
      }

      virBufferAsprintf(buf, " model='%s'", model);
  }

but either way is fine.

[...]
> +++ b/src/conf/domain_conf.h
> @@ -1376,9 +1376,19 @@ typedef enum {
>      VIR_DOMAIN_INPUT_BUS_LAST
>  } virDomainInputBus;
>  
> +typedef enum {
> +    VIR_DOMAIN_INPUT_MODEL_DEFAULT,
> +    VIR_DOMAIN_INPUT_MODEL_VIRTIO,
> +    VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL,
> +    VIR_DOMAIN_INPUT_MODEL_VIRTIO_NON_TRANSITIONAL,
> +
> +    VIR_DOMAIN_INPUT_MODEL_LAST
> +} virDomainInputModel;

Please explicitly assign 0 to the first enum value.


Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>

-- 
Andrea Bolognani / Red Hat / Virtualization