Re: [libvirt PATCH 4/7] 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(a)redhat.com&gt; --- src/conf/node_device_conf.c | 30 +++++++++--- src/conf/node_device_conf.h | 20 +++++++- src/conf/virnodedeviceobj.h | 1 + src/hypervisor/domain_driver.c | 7 +-- src/node_device/node_device_driver.c | 68 +++++++++++++++++++++++++++- src/node_device/node_device_driver.h | 3 ++ src/node_device/node_device_udev.c | 2 + src/test/test_driver.c | 6 ++- tests/nodedevmdevctltest.c | 7 ++- tests/nodedevxml2xmltest.c | 3 +- 10 files changed, 129 insertions(+), 18 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 332b12f997..cd1c07092d 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2174,10 +2174,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), @@ -2185,25 +2187,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 556878b9a8..786de85060 100644 --- 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 0cb78748a4..1fdd4f65da 100644 --- 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 29e11c0447..2969d55173 100644 --- 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 ad2ca2a614..8f39d79c68 100644 --- 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) @@ -1376,7 +1377,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) @@ -1761,3 +1763,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; +} +