nodedev: Add parser validation for node devices

At the moment, this is only for mediated devices. When a new mediated
device is created or defined, the xml is expected specify the nodedev
name of an existing device as its parent. We were not previously
validating this and were simply accepting any string here.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 12ef18dc157d6d1ae36978a8a96f8bae755d99cf..f2d17b31f425a9858586198275291c3720b1b69c 100644 (file)
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2170,10 +2170,12 @@ static virNodeDeviceDef *
 virNodeDeviceDefParse(const char *str,
                       const char *filename,
                       int create,
-                      const char *virt_type)
+                      const char *virt_type,
+                      virNodeDeviceDefParserCallbacks *parserCallbacks,
+                      void *opaque)
 {
     xmlDocPtr xml;
-    virNodeDeviceDef *def = NULL;
+    g_autoptr(virNodeDeviceDef) def = NULL;
 
     if ((xml = virXMLParse(filename, str, _("(node_device_definition)")))) {
         def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml),
@@ -2181,25 +2183,39 @@ virNodeDeviceDefParse(const char *str,
         xmlFreeDoc(xml);
     }
 
-    return def;
+    if (parserCallbacks) {
+        int ret = 0;
+        /* validate definition */
+        if (parserCallbacks->validate) {
+            ret = parserCallbacks->validate(def, opaque);
+            if (ret < 0)
+                return NULL;
+        }
+    }
+
+    return g_steal_pointer(&def);
 }
 
 
 virNodeDeviceDef *
 virNodeDeviceDefParseString(const char *str,
                             int create,
-                            const char *virt_type)
+                            const char *virt_type,
+                            virNodeDeviceDefParserCallbacks *parserCallbacks,
+                            void *opaque)
 {
-    return virNodeDeviceDefParse(str, NULL, create, virt_type);
+    return virNodeDeviceDefParse(str, NULL, create, virt_type, parserCallbacks, opaque);
 }
 
 
 virNodeDeviceDef *
 virNodeDeviceDefParseFile(const char *filename,
                           int create,
-                          const char *virt_type)
+                          const char *virt_type,
+                          virNodeDeviceDefParserCallbacks *parserCallbacks,
+                          void *opaque)
 {
-    return virNodeDeviceDefParse(NULL, filename, create, virt_type);
+    return virNodeDeviceDefParse(NULL, filename, create, virt_type, parserCallbacks, opaque);
 }
 
 

diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 556878b9a8ff2ca210f7d42aae66370bac095417..786de850607a6ec8ba18faed51e1a47df7951dda 100644 (file)
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -349,15 +349,31 @@ struct _virNodeDeviceDef {
 char *
 virNodeDeviceDefFormat(const virNodeDeviceDef *def);
 
+
+typedef int (*virNodeDeviceDefPostParseCallback)(virNodeDeviceDef *dev,
+                                                 void *opaque);
+
+typedef int (*virNodeDeviceDefValidateCallback)(virNodeDeviceDef *dev,
+                                                void *opaque);
+
+typedef struct _virNodeDeviceDefParserCallbacks {
+    virNodeDeviceDefPostParseCallback postParse;
+    virNodeDeviceDefValidateCallback validate;
+} virNodeDeviceDefParserCallbacks;
+
 virNodeDeviceDef *
 virNodeDeviceDefParseString(const char *str,
                             int create,
-                            const char *virt_type);
+                            const char *virt_type,
+                            virNodeDeviceDefParserCallbacks *callbacks,
+                            void *opaque);
 
 virNodeDeviceDef *
 virNodeDeviceDefParseFile(const char *filename,
                           int create,
-                          const char *virt_type);
+                          const char *virt_type,
+                          virNodeDeviceDefParserCallbacks *callbacks,
+                          void *opaque);
 
 virNodeDeviceDef *
 virNodeDeviceDefParseNode(xmlDocPtr xml,

diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 0cb78748a442c00fae340a4981b6a3b42db52f59..1fdd4f65da9288a7d1c33a2b27befb83d88145d7 100644 (file)
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -47,6 +47,7 @@ struct _virNodeDeviceDriverState {
 
     /* Immutable pointer, self-locking APIs */
     virObjectEventState *nodeDeviceEventState;
+    virNodeDeviceDefParserCallbacks parserCallbacks;
 };
 
 void

diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 29e11c04474a644a4a3186d7f353395a978767c2..2969d551734a27775cc5e26009285e4ab397d2bc 100644 (file)
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -395,7 +395,8 @@ virDomainDriverNodeDeviceReset(virNodeDevicePtr dev,
     if (!xml)
         return -1;
 
-    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
+    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL,
+                                      NULL, NULL);
     if (!def)
         return -1;
 
@@ -440,7 +441,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
     if (!xml)
         return -1;
 
-    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
+    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL);
     if (!def)
         return -1;
 
@@ -488,7 +489,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
     if (!xml)
         return -1;
 
-    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
+    def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL);
     if (!def)
         return -1;
 

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index daeaa084bd8b8f430c32ddc93dfbf8ed0ebffa67..328819e778304b37aee5eef6d158c16a26b72917 100644 (file)
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -896,7 +896,8 @@ nodeDeviceCreateXML(virConnectPtr conn,
 
     virt_type  = virConnectGetType(conn);
 
-    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type)))
+    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type,
+                                            &driver->parserCallbacks, NULL)))
         return NULL;
 
     if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0)
@@ -1375,7 +1376,8 @@ nodeDeviceDefineXML(virConnect *conn,
 
     virt_type  = virConnectGetType(conn);
 
-    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type)))
+    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type,
+                                            &driver->parserCallbacks, NULL)))
         return NULL;
 
     if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0)
@@ -1760,3 +1762,65 @@ nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
 
     return ret;
 }
+
+
+/* validate that parent exists */
+static int nodeDeviceDefValidateMdev(virNodeDeviceDef *def,
+                                     G_GNUC_UNUSED virNodeDevCapMdev *mdev,
+                                     G_GNUC_UNUSED void *opaque)
+{
+    virNodeDeviceObj *obj = NULL;
+    if (!def->parent) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("missing parent device"));
+        return -1;
+    }
+    obj = virNodeDeviceObjListFindByName(driver->devs, def->parent);
+    if (!obj) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("invalid parent device '%s'"),
+                       def->parent);
+        return -1;
+    }
+
+    virNodeDeviceObjEndAPI(&obj);
+    return 0;
+}
+
+int nodeDeviceDefValidate(virNodeDeviceDef *def,
+                          G_GNUC_UNUSED void *opaque)
+{
+    virNodeDevCapsDef *caps = NULL;
+    for (caps = def->caps; caps != NULL; caps = caps->next) {
+        switch (caps->data.type) {
+            case VIR_NODE_DEV_CAP_MDEV:
+                if (nodeDeviceDefValidateMdev(def, &caps->data.mdev, opaque) < 0)
+                    return -1;
+                break;
+
+            case VIR_NODE_DEV_CAP_SYSTEM:
+            case VIR_NODE_DEV_CAP_PCI_DEV:
+            case VIR_NODE_DEV_CAP_USB_DEV:
+            case VIR_NODE_DEV_CAP_USB_INTERFACE:
+            case VIR_NODE_DEV_CAP_NET:
+            case VIR_NODE_DEV_CAP_SCSI_HOST:
+            case VIR_NODE_DEV_CAP_SCSI_TARGET:
+            case VIR_NODE_DEV_CAP_SCSI:
+            case VIR_NODE_DEV_CAP_STORAGE:
+            case VIR_NODE_DEV_CAP_FC_HOST:
+            case VIR_NODE_DEV_CAP_VPORTS:
+            case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+            case VIR_NODE_DEV_CAP_DRM:
+            case VIR_NODE_DEV_CAP_MDEV_TYPES:
+            case VIR_NODE_DEV_CAP_CCW_DEV:
+            case VIR_NODE_DEV_CAP_CSS_DEV:
+            case VIR_NODE_DEV_CAP_VDPA:
+            case VIR_NODE_DEV_CAP_AP_CARD:
+            case VIR_NODE_DEV_CAP_AP_QUEUE:
+            case VIR_NODE_DEV_CAP_AP_MATRIX:
+            case VIR_NODE_DEV_CAP_LAST:
+                break;
+        }
+    }
+    return 0;
+}

diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index edd763f0e40b6b8e9a1ff923f98b3009df2a9de5..d9b9b7a961deacfb4d13956cd3b4b6c31ebf695b 100644 (file)
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -171,3 +171,6 @@ bool nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
 int
 nodeDeviceCreate(virNodeDevice *dev,
                  unsigned int flags);
+
+int nodeDeviceDefValidate(virNodeDeviceDef *def,
+                          void *opaque);

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index e7db74b325d726cc6378a07bc386ba0769d866ae..ccf94d8e7da3ea054375533b5b572a71b4f81200 100644 (file)
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2243,6 +2243,8 @@ nodeStateInitialize(bool privileged,
     driver->privateData = priv;
     driver->nodeDeviceEventState = virObjectEventStateNew();
 
+    driver->parserCallbacks.validate = nodeDeviceDefValidate;
+
     if (udevPCITranslateInit(privileged) < 0)
         goto unlock;
 

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 16d70d9025b8b1c3389abd0c16b1d7e8ad21c7c3..7c3bb70be3fcb38c90652b3a883e6eee65243fee 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7558,7 +7558,8 @@ testNodeDeviceMockCreateVport(testDriver *driver,
     if (!xml)
         goto cleanup;
 
-    if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
+    if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL,
+                                            NULL)))
         goto cleanup;
 
     VIR_FREE(def->name);
@@ -7620,7 +7621,8 @@ testNodeDeviceCreateXML(virConnectPtr conn,
 
     virCheckFlags(0, NULL);
 
-    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL)))
+    if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL,
+                                            NULL, NULL)))
         goto cleanup;
 
     /* We run this simply for validation - it essentially validates that

diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 0a590405efee6955d9e017e405115a064e3f324c..f5f882e8b5a62fd1ba4619e3ab65903df92f4393 100644 (file)
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -12,6 +12,10 @@
 
 #define VIRT_TYPE "QEMU"
 
+static virNodeDeviceDefParserCallbacks parser_callbacks = {
+    .validate = nodeDeviceDefValidate
+};
+
 struct TestInfo {
     const char *filename;
     virMdevctlCommand command;
@@ -66,7 +70,8 @@ testMdevctlCmd(virMdevctlCommand cmd_type,
             return -1;
     }
 
-    if (!(def = virNodeDeviceDefParseFile(mdevxml, create, VIRT_TYPE)))
+    if (!(def = virNodeDeviceDefParseFile(mdevxml, create, VIRT_TYPE,
+                                          &parser_callbacks, NULL)))
         return -1;
 
     /* this function will set a stdin buffer containing the json configuration

diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index e9716726d71a3eceee0a1f5b048d817c034f857e..d8f81456eef344cb85746a3a3b0906bf12212e8f 100644 (file)
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -25,7 +25,8 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile)
     if (virTestLoadFile(xml, &xmlData) < 0)
         goto fail;
 
-    if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE, NULL)))
+    if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE, NULL,
+                                            NULL, NULL)))
         goto fail;
 
     /* Calculate some things that are not read in */