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>

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);
 }
 
 

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,

src/conf/virnodedeviceobj.h

@@ -47,6 +47,7 @@ struct _virNodeDeviceDriverState {
 
     /* Immutable pointer, self-locking APIs */
     virObjectEventState *nodeDeviceEventState;
+    virNodeDeviceDefParserCallbacks parserCallbacks;
 };
 
 void

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;
 

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;
+}

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);

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;
 

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

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

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 */