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

12 Feb 2019


      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@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

    