[PATCH 1/3] conf: Move virDomainCheckVirtioOptions() into domain_validate.c

[Michal Privoznik]

The aim of virDomainCheckVirtioOptions() function is to check
whether no virtio options are set, i.e. no @iommu no @ats and no
@packed attributes were present in given device's XML (yeah, the
function has very misleading name). Nevertheless, this kind of
check belongs to validation phase, but now is done in post parse
phase. Move the function and its calls to domain_validate.c so
that future code is not tempted to repeat this mistake.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/conf/domain_conf.c     | 40 +--------------------------
 src/conf/domain_validate.c | 55 +++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dab4f10326..69ebd5d66d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5132,34 +5132,6 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev,
 }
 
 
-static int
-virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio)
-{
-    if (!virtio)
-        return 0;
-
-    if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("iommu driver option is only supported "
-                         "for virtio devices"));
-        return -1;
-    }
-    if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("ats driver option is only supported "
-                         "for virtio devices"));
-        return -1;
-    }
-    if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("packed driver option is only supported "
-                         "for virtio devices"));
-        return -1;
-    }
-    return 0;
-}
-
-
 static void
 virDomainChrDefPostParse(virDomainChrDefPtr chr,
                          const virDomainDef *def)
@@ -5256,11 +5228,6 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk,
         virDomainPostParseCheckISCSIPath(&disk->src->path);
     }
 
-    if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
-        virDomainCheckVirtioOptions(disk->virtio) < 0) {
-        return -1;
-    }
-
     if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
         if (disk->src->nvme->managed == VIR_TRISTATE_BOOL_ABSENT)
             disk->src->nvme->managed = VIR_TRISTATE_BOOL_YES;
@@ -5310,13 +5277,8 @@ virDomainControllerDefPostParse(virDomainControllerDefPtr cdev)
 
 
 static int
-virDomainNetDefPostParse(virDomainNetDefPtr net)
+virDomainNetDefPostParse(virDomainNetDefPtr net G_GNUC_UNUSED)
 {
-    if (!virDomainNetIsVirtioModel(net) &&
-        virDomainCheckVirtioOptions(net->virtio) < 0) {
-        return -1;
-    }
-
     return 0;
 }
 
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 649fc335ac..37c09ff648 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -226,6 +226,34 @@ virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr *seclabels,
 }
 
 
+static int
+virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio)
+{
+    if (!virtio)
+        return 0;
+
+    if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("iommu driver option is only supported "
+                         "for virtio devices"));
+        return -1;
+    }
+    if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("ats driver option is only supported "
+                         "for virtio devices"));
+        return -1;
+    }
+    if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("packed driver option is only supported "
+                         "for virtio devices"));
+        return -1;
+    }
+    return 0;
+}
+
+
 #define VENDOR_LEN  8
 #define PRODUCT_LEN 16
 
@@ -277,15 +305,19 @@ virDomainDiskDefValidate(const virDomainDef *def,
         return -1;
     }
 
-    if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO &&
-        (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO ||
-         disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL ||
-         disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("disk model '%s' not supported for bus '%s'"),
-                       virDomainDiskModelTypeToString(disk->model),
-                       virDomainDiskBusTypeToString(disk->bus));
-        return -1;
+    if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
+        if (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO ||
+            disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL ||
+            disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("disk model '%s' not supported for bus '%s'"),
+                           virDomainDiskModelTypeToString(disk->model),
+                           virDomainDiskBusTypeToString(disk->bus));
+            return -1;
+        }
+
+        if (virDomainCheckVirtioOptions(disk->virtio) < 0)
+            return -1;
     }
 
     if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
@@ -1330,6 +1362,11 @@ virDomainNetDefValidate(const virDomainNetDef *net)
         return -1;
     }
 
+    if (!virDomainNetIsVirtioModel(net) &&
+        virDomainCheckVirtioOptions(net->virtio) < 0) {
+        return -1;
+    }
+
     return 0;
 }
 
-- 
2.26.2