virDomainDefCheckDuplicateDiskInfo() and virDomainDefCheckDuplicateDriveAddresses() are static functions used by virDomainDefValidateInternal(). Let's move them to domain_validate.c to start clearing up the path to move virDomainDefValidateInternal(). Change the functions name slightly to be more on par with their new home. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 129 +------------------------------------ src/conf/domain_conf.h | 5 ++ src/conf/domain_validate.c | 125 +++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 2 + 4 files changed, 135 insertions(+), 126 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 23415b323c..0f5d314e2c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4867,7 +4867,7 @@ virDomainDefPostParseGraphics(virDomainDef *def) * Return true if any disk is already using the given address on the * given bus, false otherwise. */ -static bool +bool virDomainDriveAddressIsUsedByDisk(const virDomainDef *def, virDomainDiskBus bus_type, const virDomainDeviceDriveAddress *addr) @@ -6599,129 +6599,6 @@ virDomainDefValidateDeviceIterator(virDomainDefPtr def, } -static int -virDomainDefCheckDuplicateDiskInfo(const virDomainDef *def) -{ - size_t i; - size_t j; - - for (i = 0; i < def->ndisks; i++) { - for (j = i + 1; j < def->ndisks; j++) { - if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], - def->disks[j]) < 0) - return -1; - } - } - - return 0; -} - -/** - * virDomainDefCheckDuplicateDriveAddresses: - * @def: domain definition to check against - * - * This function checks @def for duplicate drive addresses. Drive - * addresses are only in use for disks and hostdevs at the moment. - * - * Returns 0 in case of there are no duplicate drive addresses, -1 - * otherwise. - */ -static int -virDomainDefCheckDuplicateDriveAddresses(const virDomainDef *def) -{ - size_t i; - size_t j; - - for (i = 0; i < def->ndisks; i++) { - virDomainDiskDefPtr disk_i = def->disks[i]; - virDomainDeviceInfoPtr disk_info_i = &disk_i->info; - - if (disk_info_i->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) - continue; - - for (j = i + 1; j < def->ndisks; j++) { - virDomainDiskDefPtr disk_j = def->disks[j]; - virDomainDeviceInfoPtr disk_info_j = &disk_j->info; - - if (disk_i->bus != disk_j->bus) - continue; - - if (disk_info_j->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) - continue; - - if (virDomainDeviceInfoAddressIsEqual(disk_info_i, disk_info_j)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Found duplicate drive address for disk with " - "target name '%s' controller='%u' bus='%u' " - "target='%u' unit='%u'"), - disk_i->dst, - disk_info_i->addr.drive.controller, - disk_info_i->addr.drive.bus, - disk_info_i->addr.drive.target, - disk_info_i->addr.drive.unit); - return -1; - } - } - - /* Note: There is no need to check for conflicts with SCSI - * hostdevs above, because conflicts with hostdevs are checked - * in the next loop. - */ - } - - for (i = 0; i < def->nhostdevs; i++) { - virDomainHostdevDefPtr hdev_i = def->hostdevs[i]; - virDomainDeviceInfoPtr hdev_info_i = hdev_i->info; - virDomainDeviceDriveAddressPtr hdev_addr_i; - - if (!virHostdevIsSCSIDevice(hdev_i)) - continue; - - if (hdev_i->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) - continue; - - hdev_addr_i = &hdev_info_i->addr.drive; - for (j = i + 1; j < def->nhostdevs; j++) { - virDomainHostdevDefPtr hdev_j = def->hostdevs[j]; - virDomainDeviceInfoPtr hdev_info_j = hdev_j->info; - - if (!virHostdevIsSCSIDevice(hdev_j)) - continue; - - /* Address type check for hdev_j will be done implicitly - * in virDomainDeviceInfoAddressIsEqual() */ - - if (virDomainDeviceInfoAddressIsEqual(hdev_info_i, hdev_info_j)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("SCSI host address controller='%u' " - "bus='%u' target='%u' unit='%u' in " - "use by another SCSI host device"), - hdev_addr_i->bus, - hdev_addr_i->controller, - hdev_addr_i->target, - hdev_addr_i->unit); - return -1; - } - } - - if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI, - hdev_addr_i)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("SCSI host address controller='%u' " - "bus='%u' target='%u' unit='%u' in " - "use by another SCSI disk"), - hdev_addr_i->bus, - hdev_addr_i->controller, - hdev_addr_i->target, - hdev_addr_i->unit); - return -1; - } - } - - return 0; -} - - bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action) @@ -6899,10 +6776,10 @@ static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOptionPtr xmlopt) { - if (virDomainDefCheckDuplicateDiskInfo(def) < 0) + if (virDomainDefDuplicateDiskInfoValidate(def) < 0) return -1; - if (virDomainDefCheckDuplicateDriveAddresses(def) < 0) + if (virDomainDefDuplicateDriveAddressesValidate(def) < 0) return -1; if (virDomainDefGetVcpusTopology(def, NULL) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 72771c46b9..71e42d6617 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3915,6 +3915,11 @@ bool virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a, const virDomainBlockIoTuneInfo *b); +bool +virDomainDriveAddressIsUsedByDisk(const virDomainDef *def, + virDomainDiskBus bus_type, + const virDomainDeviceDriveAddress *addr); + bool virHostdevIsSCSIDevice(const virDomainHostdevDef *hostdev) ATTRIBUTE_NONNULL(1); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 0eed1ba982..8bbd60c836 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -635,3 +635,128 @@ virDomainDefIdMapValidate(const virDomainDef *def) return 0; } + + +int +virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def) +{ + size_t i; + size_t j; + + for (i = 0; i < def->ndisks; i++) { + for (j = i + 1; j < def->ndisks; j++) { + if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], + def->disks[j]) < 0) + return -1; + } + } + + return 0; +} + + + +/** + * virDomainDefDuplicateDriveAddressesValidate: + * @def: domain definition to check against + * + * This function checks @def for duplicate drive addresses. Drive + * addresses are only in use for disks and hostdevs at the moment. + * + * Returns 0 in case of there are no duplicate drive addresses, -1 + * otherwise. + */ +int +virDomainDefDuplicateDriveAddressesValidate(const virDomainDef *def) +{ + size_t i; + size_t j; + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk_i = def->disks[i]; + virDomainDeviceInfoPtr disk_info_i = &disk_i->info; + + if (disk_info_i->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + continue; + + for (j = i + 1; j < def->ndisks; j++) { + virDomainDiskDefPtr disk_j = def->disks[j]; + virDomainDeviceInfoPtr disk_info_j = &disk_j->info; + + if (disk_i->bus != disk_j->bus) + continue; + + if (disk_info_j->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + continue; + + if (virDomainDeviceInfoAddressIsEqual(disk_info_i, disk_info_j)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Found duplicate drive address for disk with " + "target name '%s' controller='%u' bus='%u' " + "target='%u' unit='%u'"), + disk_i->dst, + disk_info_i->addr.drive.controller, + disk_info_i->addr.drive.bus, + disk_info_i->addr.drive.target, + disk_info_i->addr.drive.unit); + return -1; + } + } + + /* Note: There is no need to check for conflicts with SCSI + * hostdevs above, because conflicts with hostdevs are checked + * in the next loop. + */ + } + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr hdev_i = def->hostdevs[i]; + virDomainDeviceInfoPtr hdev_info_i = hdev_i->info; + virDomainDeviceDriveAddressPtr hdev_addr_i; + + if (!virHostdevIsSCSIDevice(hdev_i)) + continue; + + if (hdev_i->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + continue; + + hdev_addr_i = &hdev_info_i->addr.drive; + for (j = i + 1; j < def->nhostdevs; j++) { + virDomainHostdevDefPtr hdev_j = def->hostdevs[j]; + virDomainDeviceInfoPtr hdev_info_j = hdev_j->info; + + if (!virHostdevIsSCSIDevice(hdev_j)) + continue; + + /* Address type check for hdev_j will be done implicitly + * in virDomainDeviceInfoAddressIsEqual() */ + + if (virDomainDeviceInfoAddressIsEqual(hdev_info_i, hdev_info_j)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("SCSI host address controller='%u' " + "bus='%u' target='%u' unit='%u' in " + "use by another SCSI host device"), + hdev_addr_i->bus, + hdev_addr_i->controller, + hdev_addr_i->target, + hdev_addr_i->unit); + return -1; + } + } + + if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI, + hdev_addr_i)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("SCSI host address controller='%u' " + "bus='%u' target='%u' unit='%u' in " + "use by another SCSI disk"), + hdev_addr_i->bus, + hdev_addr_i->controller, + hdev_addr_i->target, + hdev_addr_i->unit); + return -1; + } + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 497a02b9b3..1f1a5b1bd1 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -45,3 +45,5 @@ int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, int virDomainDefTunablesValidate(const virDomainDef *def); int virDomainControllerDefValidate(const virDomainControllerDef *controller); int virDomainDefIdMapValidate(const virDomainDef *def); +int virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def); +int virDomainDefDuplicateDriveAddressesValidate(const virDomainDef *def); -- 2.26.2

virDomainDefValidateAliases() is one of the static functions that needs to be handled before moving virDomainDefValidateInternal(). Let's move all related validate functions to domain_validate.c at the same time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 144 +------------------------------------ src/conf/domain_conf.h | 14 ++-- src/conf/domain_validate.c | 138 +++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 5 ++ src/libvirt_private.syms | 5 +- src/qemu/qemu_driver.c | 1 + 6 files changed, 159 insertions(+), 148 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3ea79b708..0425fbaca3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4213,16 +4213,11 @@ virDomainSkipBackcompatConsole(virDomainDefPtr def, } -enum { - DOMAIN_DEVICE_ITERATE_ALL_CONSOLES = 1 << 0, - DOMAIN_DEVICE_ITERATE_MISSING_INFO = 1 << 1, -} virDomainDeviceIterateFlags; - /* * Iterates over domain devices calling @cb on each device. The default * behaviour can be altered with virDomainDeviceIterateFlags. */ -static int +int virDomainDeviceInfoIterateFlags(virDomainDefPtr def, virDomainDeviceInfoCallback cb, unsigned int iteratorFlags, @@ -6427,143 +6422,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, } -struct virDomainDefValidateAliasesData { - GHashTable *aliases; -}; - - -static int -virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def, - virDomainDeviceDefPtr dev, - virDomainDeviceInfoPtr info, - void *opaque) -{ - struct virDomainDefValidateAliasesData *data = opaque; - const char *alias = info->alias; - - if (!virDomainDeviceAliasIsUserAlias(alias)) - return 0; - - /* Some crazy backcompat for consoles. */ - if (def->nserials && def->nconsoles && - def->consoles[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && - dev->type == VIR_DOMAIN_DEVICE_CHR && - virDomainChrEquals(def->serials[0], dev->data.chr)) - return 0; - - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->parentnet) { - /* This hostdev is a copy of some previous interface. - * Aliases are duplicated. */ - return 0; - } - - if (virHashLookup(data->aliases, alias)) { - virReportError(VIR_ERR_XML_ERROR, - _("non unique alias detected: %s"), - alias); - return -1; - } - - if (virHashAddEntry(data->aliases, alias, (void *) 1) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to construct table of device aliases")); - return -1; - } - - return 0; -} - - -/** - * virDomainDefValidateAliases: - * - * Check for uniqueness of device aliases. If @aliases is not - * NULL return hash table of all the aliases in it. - * - * Returns 0 on success, - * -1 otherwise (with error reported). - */ -static int -virDomainDefValidateAliases(const virDomainDef *def, - GHashTable **aliases) -{ - struct virDomainDefValidateAliasesData data; - int ret = -1; - - /* We are not storing copies of aliases. Don't free them. */ - if (!(data.aliases = virHashNew(NULL))) - goto cleanup; - - if (virDomainDeviceInfoIterateFlags((virDomainDefPtr) def, - virDomainDeviceDefValidateAliasesIterator, - DOMAIN_DEVICE_ITERATE_ALL_CONSOLES, - &data) < 0) - goto cleanup; - - if (aliases) - *aliases = g_steal_pointer(&data.aliases); - - ret = 0; - cleanup: - virHashFree(data.aliases); - return ret; -} - - -static int -virDomainDeviceValidateAliasImpl(const virDomainDef *def, - virDomainDeviceDefPtr dev) -{ - GHashTable *aliases = NULL; - virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); - int ret = -1; - - if (!info || !info->alias) - return 0; - - if (virDomainDefValidateAliases(def, &aliases) < 0) - goto cleanup; - - if (virHashLookup(aliases, info->alias)) { - virReportError(VIR_ERR_XML_ERROR, - _("non unique alias detected: %s"), - info->alias); - goto cleanup; - } - - ret = 0; - cleanup: - - virHashFree(aliases); - return ret; -} - - -int -virDomainDeviceValidateAliasForHotplug(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - unsigned int flags) -{ - virDomainDefPtr persDef = NULL; - virDomainDefPtr liveDef = NULL; - - if (virDomainObjGetDefs(vm, flags, &liveDef, &persDef) < 0) - return -1; - - if (persDef && - virDomainDeviceValidateAliasImpl(persDef, dev) < 0) - return -1; - - if (liveDef && - virDomainDeviceValidateAliasImpl(liveDef, dev) < 0) - return -1; - - return 0; -} - - static int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 71e42d6617..ba03a55c7b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2759,6 +2759,11 @@ typedef enum { VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; +typedef enum { + DOMAIN_DEVICE_ITERATE_ALL_CONSOLES = 1 << 0, + DOMAIN_DEVICE_ITERATE_MISSING_INFO = 1 << 1, +} virDomainDeviceIterateFlags; + /* Guest VM runtime state */ typedef struct _virDomainStateReason virDomainStateReason; struct _virDomainStateReason { @@ -2987,10 +2992,6 @@ int virDomainDefPostParse(virDomainDefPtr def, void *parseOpaque); bool virDomainDefHasUSB(const virDomainDef *def); -int virDomainDeviceValidateAliasForHotplug(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - unsigned int flags); - bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); int virDomainDefValidate(virDomainDefPtr def, @@ -3130,6 +3131,11 @@ virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt); virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt); +int virDomainDeviceInfoIterateFlags(virDomainDefPtr def, + virDomainDeviceInfoCallback cb, + unsigned int iteratorFlags, + void *opaque); + virDomainGraphicsDefPtr virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 8bbd60c836..962336033f 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -760,3 +760,141 @@ virDomainDefDuplicateDriveAddressesValidate(const virDomainDef *def) return 0; } + + + +struct virDomainDefValidateAliasesData { + GHashTable *aliases; +}; + + +static int +virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info, + void *opaque) +{ + struct virDomainDefValidateAliasesData *data = opaque; + const char *alias = info->alias; + + if (!virDomainDeviceAliasIsUserAlias(alias)) + return 0; + + /* Some crazy backcompat for consoles. */ + if (def->nserials && def->nconsoles && + def->consoles[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + dev->type == VIR_DOMAIN_DEVICE_CHR && + virDomainChrEquals(def->serials[0], dev->data.chr)) + return 0; + + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->parentnet) { + /* This hostdev is a copy of some previous interface. + * Aliases are duplicated. */ + return 0; + } + + if (virHashLookup(data->aliases, alias)) { + virReportError(VIR_ERR_XML_ERROR, + _("non unique alias detected: %s"), + alias); + return -1; + } + + if (virHashAddEntry(data->aliases, alias, (void *) 1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to construct table of device aliases")); + return -1; + } + + return 0; +} + + +/** + * virDomainDefValidateAliases: + * + * Check for uniqueness of device aliases. If @aliases is not + * NULL return hash table of all the aliases in it. + * + * Returns 0 on success, + * -1 otherwise (with error reported). + */ +int +virDomainDefValidateAliases(const virDomainDef *def, + GHashTable **aliases) +{ + struct virDomainDefValidateAliasesData data; + int ret = -1; + + /* We are not storing copies of aliases. Don't free them. */ + if (!(data.aliases = virHashNew(NULL))) + goto cleanup; + + if (virDomainDeviceInfoIterateFlags((virDomainDefPtr) def, + virDomainDeviceDefValidateAliasesIterator, + DOMAIN_DEVICE_ITERATE_ALL_CONSOLES, + &data) < 0) + goto cleanup; + + if (aliases) + *aliases = g_steal_pointer(&data.aliases); + + ret = 0; + cleanup: + virHashFree(data.aliases); + return ret; +} + + +static int +virDomainDeviceValidateAliasImpl(const virDomainDef *def, + virDomainDeviceDefPtr dev) +{ + GHashTable *aliases = NULL; + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + int ret = -1; + + if (!info || !info->alias) + return 0; + + if (virDomainDefValidateAliases(def, &aliases) < 0) + goto cleanup; + + if (virHashLookup(aliases, info->alias)) { + virReportError(VIR_ERR_XML_ERROR, + _("non unique alias detected: %s"), + info->alias); + goto cleanup; + } + + ret = 0; + cleanup: + + virHashFree(aliases); + return ret; +} + + +int +virDomainDeviceValidateAliasForHotplug(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned int flags) +{ + virDomainDefPtr persDef = NULL; + virDomainDefPtr liveDef = NULL; + + if (virDomainObjGetDefs(vm, flags, &liveDef, &persDef) < 0) + return -1; + + if (persDef && + virDomainDeviceValidateAliasImpl(persDef, dev) < 0) + return -1; + + if (liveDef && + virDomainDeviceValidateAliasImpl(liveDef, dev) < 0) + return -1; + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 1f1a5b1bd1..521a98ef11 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -47,3 +47,8 @@ int virDomainControllerDefValidate(const virDomainControllerDef *controller); int virDomainDefIdMapValidate(const virDomainDef *def); int virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def); int virDomainDefDuplicateDriveAddressesValidate(const virDomainDef *def); +int virDomainDefValidateAliases(const virDomainDef *def, + GHashTable **aliases); +int virDomainDeviceValidateAliasForHotplug(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 992488f754..c3056232db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -354,7 +354,6 @@ virDomainDeviceGetInfo; virDomainDeviceInfoIterate; virDomainDeviceSetData; virDomainDeviceTypeToString; -virDomainDeviceValidateAliasForHotplug; virDomainDiskBackingStoreFormat; virDomainDiskBackingStoreParse; virDomainDiskBusTypeToString; @@ -744,6 +743,10 @@ virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; +# conf/domain_validate.h +virDomainDeviceValidateAliasForHotplug; + + # conf/interface_conf.h virInterfaceDefFormat; virInterfaceDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 58c376fbe5..5f0fb0a55f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -70,6 +70,7 @@ #include "domain_audit.h" #include "domain_cgroup.h" #include "domain_driver.h" +#include "domain_validate.h" #include "node_device_conf.h" #include "virpci.h" #include "virusb.h" -- 2.26.2

Move virDomainDeviceDefValidate() and all its helper functions to domain_validate.c. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 137 +------------------------------------ src/conf/domain_conf.h | 16 +++-- src/conf/domain_validate.c | 129 ++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 4 ++ src/libvirt_private.syms | 2 +- src/qemu/qemu_process.c | 1 + 6 files changed, 147 insertions(+), 142 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1cd7733652..bf9d00094c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5643,13 +5643,6 @@ virDomainDeviceDefPostParseOne(virDomainDeviceDefPtr dev, } -struct virDomainDefPostParseDeviceIteratorData { - virDomainXMLOptionPtr xmlopt; - void *parseOpaque; - unsigned int parseFlags; -}; - - static int virDomainDefPostParseDeviceIterator(virDomainDefPtr def, virDomainDeviceDefPtr dev, @@ -6398,7 +6391,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, } -static int +int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, unsigned int parseFlags, @@ -6420,19 +6413,6 @@ virDomainDeviceDefValidate(const virDomainDeviceDef *dev, } -static int -virDomainDefValidateDeviceIterator(virDomainDefPtr def, - virDomainDeviceDefPtr dev, - virDomainDeviceInfoPtr info G_GNUC_UNUSED, - void *opaque) -{ - struct virDomainDefPostParseDeviceIteratorData *data = opaque; - return virDomainDeviceDefValidate(dev, def, - data->parseFlags, data->xmlopt, - data->parseOpaque); -} - - bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action) @@ -6465,121 +6445,6 @@ virDomainDefLifecycleActionAllowed(virDomainLifecycle type, } -static int -virDomainDefValidateInternal(const virDomainDef *def, - virDomainXMLOptionPtr xmlopt) -{ - if (virDomainDefDuplicateDiskInfoValidate(def) < 0) - return -1; - - if (virDomainDefDuplicateDriveAddressesValidate(def) < 0) - return -1; - - if (virDomainDefGetVcpusTopology(def, NULL) < 0) - return -1; - - if (virDomainDefValidateAliases(def, NULL) < 0) - return -1; - - if (def->iommu && - def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && - def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("IOMMU interrupt remapping requires split I/O APIC " - "(ioapic driver='qemu')")); - return -1; - } - - if (def->iommu && - def->iommu->eim == VIR_TRISTATE_SWITCH_ON && - def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("IOMMU eim requires interrupt remapping to be enabled")); - return -1; - } - - if (virDomainDefLifecycleActionValidate(def) < 0) - return -1; - - if (virDomainDefMemtuneValidate(def) < 0) - return -1; - - if (virDomainDefOSValidate(def, xmlopt) < 0) - return -1; - - if (virDomainDefCputuneValidate(def) < 0) - return -1; - - if (virDomainDefBootValidate(def) < 0) - return -1; - - if (virDomainDefVideoValidate(def) < 0) - return -1; - - if (virDomainDefTunablesValidate(def) < 0) - return -1; - - if (virDomainDefIdMapValidate(def) < 0) - return -1; - - if (virDomainNumaDefValidate(def->numa) < 0) - return -1; - - return 0; -} - - -/** - * virDomainDefValidate: - * @def: domain definition - * @caps: driver capabilities object - * @parseFlags: virDomainDefParseFlags - * @xmlopt: XML parser option object - * @parseOpaque: hypervisor driver specific data for this validation run - * - * This validation function is designed to take checks of globally invalid - * configurations that the parser needs to accept so that VMs don't vanish upon - * daemon restart. Such definition can be rejected upon startup or define, where - * this function shall be called. - * - * Returns 0 if domain definition is valid, -1 on error and reports an - * appropriate message. - */ -int -virDomainDefValidate(virDomainDefPtr def, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque) -{ - struct virDomainDefPostParseDeviceIteratorData data = { - .xmlopt = xmlopt, - .parseFlags = parseFlags, - .parseOpaque = parseOpaque, - }; - - /* validate configuration only in certain places */ - if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) - return 0; - - /* call the domain config callback */ - if (xmlopt->config.domainValidateCallback && - xmlopt->config.domainValidateCallback(def, xmlopt->config.priv, parseOpaque) < 0) - return -1; - - /* iterate the devices */ - if (virDomainDeviceInfoIterateFlags(def, - virDomainDefValidateDeviceIterator, - (DOMAIN_DEVICE_ITERATE_ALL_CONSOLES | - DOMAIN_DEVICE_ITERATE_MISSING_INFO), - &data) < 0) - return -1; - - if (virDomainDefValidateInternal(def, xmlopt) < 0) - return -1; - - return 0; -} - int virDomainObjCheckActive(virDomainObjPtr dom) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5f8ec16b7d..1c1533ed2f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3006,6 +3006,12 @@ struct _virDomainXMLOption { virDomainMomentPostParseCallback momentPostParse; }; +struct virDomainDefPostParseDeviceIteratorData { + virDomainXMLOptionPtr xmlopt; + void *parseOpaque; + unsigned int parseFlags; +}; + bool virDomainSCSIDriveAddressIsUsed(const virDomainDef *def, const virDomainDeviceDriveAddress *addr); @@ -3018,11 +3024,6 @@ bool virDomainDefHasUSB(const virDomainDef *def); bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); -int virDomainDefValidate(virDomainDefPtr def, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque); - int virDomainActualNetDefValidate(const virDomainNetDef *net); @@ -3124,6 +3125,11 @@ void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); void virDomainShmemDefFree(virDomainShmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); +int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, const virDomainDef *def, diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 02cd761ee2..47aa3e4375 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1040,3 +1040,132 @@ virDomainDefCputuneValidate(const virDomainDef *def) } #undef CPUTUNE_VALIDATE_PERIOD #undef CPUTUNE_VALIDATE_QUOTA + + +static int +virDomainDefValidateInternal(const virDomainDef *def, + virDomainXMLOptionPtr xmlopt) +{ + if (virDomainDefDuplicateDiskInfoValidate(def) < 0) + return -1; + + if (virDomainDefDuplicateDriveAddressesValidate(def) < 0) + return -1; + + if (virDomainDefGetVcpusTopology(def, NULL) < 0) + return -1; + + if (virDomainDefValidateAliases(def, NULL) < 0) + return -1; + + if (def->iommu && + def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && + def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU interrupt remapping requires split I/O APIC " + "(ioapic driver='qemu')")); + return -1; + } + + if (def->iommu && + def->iommu->eim == VIR_TRISTATE_SWITCH_ON && + def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU eim requires interrupt remapping to be enabled")); + return -1; + } + + if (virDomainDefLifecycleActionValidate(def) < 0) + return -1; + + if (virDomainDefMemtuneValidate(def) < 0) + return -1; + + if (virDomainDefOSValidate(def, xmlopt) < 0) + return -1; + + if (virDomainDefCputuneValidate(def) < 0) + return -1; + + if (virDomainDefBootValidate(def) < 0) + return -1; + + if (virDomainDefVideoValidate(def) < 0) + return -1; + + if (virDomainDefTunablesValidate(def) < 0) + return -1; + + if (virDomainDefIdMapValidate(def) < 0) + return -1; + + if (virDomainNumaDefValidate(def->numa) < 0) + return -1; + + return 0; +} + + +static int +virDomainDefValidateDeviceIterator(virDomainDefPtr def, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info G_GNUC_UNUSED, + void *opaque) +{ + struct virDomainDefPostParseDeviceIteratorData *data = opaque; + return virDomainDeviceDefValidate(dev, def, + data->parseFlags, data->xmlopt, + data->parseOpaque); +} + + +/** + * virDomainDefValidate: + * @def: domain definition + * @caps: driver capabilities object + * @parseFlags: virDomainDefParseFlags + * @xmlopt: XML parser option object + * @parseOpaque: hypervisor driver specific data for this validation run + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(virDomainDefPtr def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) +{ + struct virDomainDefPostParseDeviceIteratorData data = { + .xmlopt = xmlopt, + .parseFlags = parseFlags, + .parseOpaque = parseOpaque, + }; + + /* validate configuration only in certain places */ + if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) + return 0; + + /* call the domain config callback */ + if (xmlopt->config.domainValidateCallback && + xmlopt->config.domainValidateCallback(def, xmlopt->config.priv, parseOpaque) < 0) + return -1; + + /* iterate the devices */ + if (virDomainDeviceInfoIterateFlags(def, + virDomainDefValidateDeviceIterator, + (DOMAIN_DEVICE_ITERATE_ALL_CONSOLES | + DOMAIN_DEVICE_ITERATE_MISSING_INFO), + &data) < 0) + return -1; + + if (virDomainDefValidateInternal(def, xmlopt) < 0) + return -1; + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 4247dfd758..309ae5f950 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -58,3 +58,7 @@ int virDomainDefMemtuneValidate(const virDomainDef *def); int virDomainDefOSValidate(const virDomainDef *def, virDomainXMLOptionPtr xmlopt); int virDomainDefCputuneValidate(const virDomainDef *def); +int virDomainDefValidate(virDomainDefPtr def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3056232db..3cbc336bad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -342,7 +342,6 @@ virDomainDefSave; virDomainDefSetMemoryTotal; virDomainDefSetVcpus; virDomainDefSetVcpusMax; -virDomainDefValidate; virDomainDefVcpuOrderClear; virDomainDeleteConfig; virDomainDeviceAliasIsUserAlias; @@ -744,6 +743,7 @@ virDomainConfVMNWFilterTeardown; # conf/domain_validate.h +virDomainDefValidate; virDomainDeviceValidateAliasForHotplug; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3b64caa619..bb78967ca3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -74,6 +74,7 @@ #include "virhostcpu.h" #include "domain_audit.h" #include "domain_nwfilter.h" +#include "domain_validate.h" #include "locking/domain_lock.h" #include "viruuid.h" #include "virprocess.h" -- 2.26.2

The next objective is to move virDomainDeviceDefValidate() to domain_validate.c. First let's move all the static helpers. The net device validation functions are used across multiple drivers, so let's move them separately first. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 153 ------------------------------------- src/conf/domain_conf.h | 3 - src/conf/domain_validate.c | 152 ++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 2 + src/libvirt_private.syms | 2 +- src/libxl/libxl_domain.c | 1 + src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/lxc/lxc_process.c | 1 + src/qemu/qemu_domain.c | 1 + 10 files changed, 160 insertions(+), 157 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf9d00094c..a541e7bca6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6022,159 +6022,6 @@ virDomainDefHasUSB(const virDomainDef *def) } - -static int -virDomainNetDefValidatePortOptions(const char *macstr, - virDomainNetType type, - const virNetDevVPortProfile *vport, - virTristateBool isolatedPort) -{ - /* - * This function can be called for either a config interface - * object (NetDef) or a runtime interface object (ActualNetDef), - * by calling it with either, e.g., the "type" (what is in the - * config) or the "actualType" (what is determined at runtime by - * acquiring a port from the network). - */ - /* - * port isolation can only be set for an interface that is - * connected to a Linux host bridge (either a libvirt-managed - * network, or plain type='bridge') - */ - if (isolatedPort == VIR_TRISTATE_BOOL_YES) { - if (!(type == VIR_DOMAIN_NET_TYPE_NETWORK || - type == VIR_DOMAIN_NET_TYPE_BRIDGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("interface %s - <port isolated='yes'/> is not supported for network interfaces with type='%s'"), - macstr, virDomainNetTypeToString(type)); - return -1; - } - /* - * also not allowed for anything with <virtualport> setting - * (openvswitch or 802.11Qb[gh]) - */ - if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("interface %s - <port isolated='yes'/> is not supported for network interfaces with virtualport type='%s'"), - macstr, virNetDevVPortTypeToString(vport->virtPortType)); - return -1; - } - } - return 0; -} - - -int -virDomainActualNetDefValidate(const virDomainNetDef *net) -{ - /* Unlike virDomainNetDefValidate(), which is a static function - * called internally to this file, virDomainActualNetDefValidate() - * is a public function that can be called from a hypervisor after - * it has completely setup the NetDef for use by a domain, - * including possibly allocating a port from the network driver - * (which could change the effective/"actual" type of the NetDef, - * thus changing what should/shouldn't be allowed by validation). - * - * This function should contain validations not specific to a - * particular hypervisor (e.g. whether or not specifying bandwidth - * is allowed for a type of interface), but *not* - * hypervisor-specific things. - */ - char macstr[VIR_MAC_STRING_BUFLEN]; - virDomainNetType actualType = virDomainNetGetActualType(net); - const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); - const virNetDevBandwidth *bandwidth = virDomainNetGetActualBandwidth(net); - - virMacAddrFormat(&net->mac, macstr); - - if (virDomainNetGetActualVlan(net)) { - /* vlan configuration via libvirt is only supported for PCI - * Passthrough SR-IOV devices (hostdev or macvtap passthru - * mode) and openvswitch bridges. Otherwise log an error and - * fail - */ - if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV || - (actualType == VIR_DOMAIN_NET_TYPE_DIRECT && - virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || - (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && - vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("interface %s - vlan tag not supported for this connection type"), - macstr); - return -1; - } - } - - /* bandwidth configuration via libvirt is not supported for - * hostdev network devices - */ - if (bandwidth && actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("interface %s - bandwidth settings are not supported " - "for hostdev interfaces"), - macstr); - return -1; - } - - if (virDomainNetDefValidatePortOptions(macstr, actualType, vport, - virDomainNetGetActualPortOptionsIsolated(net)) < 0) { - return -1; - } - - return 0; -} - - -static int -virDomainNetDefValidate(const virDomainNetDef *net) -{ - char macstr[VIR_MAC_STRING_BUFLEN]; - - virMacAddrFormat(&net->mac, macstr); - - if ((net->hostIP.nroutes || net->hostIP.nips) && - net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid attempt to set network interface " - "host-side IP route and/or address info on " - "interface of type '%s'. This is only supported " - "on interfaces of type 'ethernet'"), - virDomainNetTypeToString(net->type)); - return -1; - } - if (net->managed_tap == VIR_TRISTATE_BOOL_NO && - net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unmanaged target dev is not supported on " - "interfaces of type '%s'"), - virDomainNetTypeToString(net->type)); - return -1; - } - - if (net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) { - if (!net->teaming.persistent) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("teaming persistent attribute must be set if teaming type is 'transient'")); - return -1; - } - } else { - if (net->teaming.persistent) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("teaming persistent attribute not allowed if teaming type is '%s'"), - virDomainNetTeamingTypeToString(net->teaming.type)); - return -1; - } - } - - if (virDomainNetDefValidatePortOptions(macstr, net->type, net->virtPortProfile, - net->isolatedPort) < 0) { - return -1; - } - - return 0; -} - - static int virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1c1533ed2f..e35c4206df 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3024,9 +3024,6 @@ bool virDomainDefHasUSB(const virDomainDef *def); bool virDomainDeviceAliasIsUserAlias(const char *aliasStr); -int -virDomainActualNetDefValidate(const virDomainNetDef *net); - static inline bool virDomainObjIsActive(virDomainObjPtr dom) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 6632262ca2..6ca4ebb0ea 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1169,3 +1169,155 @@ virDomainDefValidate(virDomainDefPtr def, return 0; } + + +static int +virDomainNetDefValidatePortOptions(const char *macstr, + virDomainNetType type, + const virNetDevVPortProfile *vport, + virTristateBool isolatedPort) +{ + /* + * This function can be called for either a config interface + * object (NetDef) or a runtime interface object (ActualNetDef), + * by calling it with either, e.g., the "type" (what is in the + * config) or the "actualType" (what is determined at runtime by + * acquiring a port from the network). + */ + /* + * port isolation can only be set for an interface that is + * connected to a Linux host bridge (either a libvirt-managed + * network, or plain type='bridge') + */ + if (isolatedPort == VIR_TRISTATE_BOOL_YES) { + if (!(type == VIR_DOMAIN_NET_TYPE_NETWORK || + type == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - <port isolated='yes'/> is not supported for network interfaces with type='%s'"), + macstr, virDomainNetTypeToString(type)); + return -1; + } + /* + * also not allowed for anything with <virtualport> setting + * (openvswitch or 802.11Qb[gh]) + */ + if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - <port isolated='yes'/> is not supported for network interfaces with virtualport type='%s'"), + macstr, virNetDevVPortTypeToString(vport->virtPortType)); + return -1; + } + } + return 0; +} + + +int +virDomainActualNetDefValidate(const virDomainNetDef *net) +{ + /* Unlike virDomainNetDefValidate(), which is a static function + * called internally to this file, virDomainActualNetDefValidate() + * is a public function that can be called from a hypervisor after + * it has completely setup the NetDef for use by a domain, + * including possibly allocating a port from the network driver + * (which could change the effective/"actual" type of the NetDef, + * thus changing what should/shouldn't be allowed by validation). + * + * This function should contain validations not specific to a + * particular hypervisor (e.g. whether or not specifying bandwidth + * is allowed for a type of interface), but *not* + * hypervisor-specific things. + */ + char macstr[VIR_MAC_STRING_BUFLEN]; + virDomainNetType actualType = virDomainNetGetActualType(net); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); + const virNetDevBandwidth *bandwidth = virDomainNetGetActualBandwidth(net); + + virMacAddrFormat(&net->mac, macstr); + + if (virDomainNetGetActualVlan(net)) { + /* vlan configuration via libvirt is only supported for PCI + * Passthrough SR-IOV devices (hostdev or macvtap passthru + * mode) and openvswitch bridges. Otherwise log an error and + * fail + */ + if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV || + (actualType == VIR_DOMAIN_NET_TYPE_DIRECT && + virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || + (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && + vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - vlan tag not supported for this connection type"), + macstr); + return -1; + } + } + + /* bandwidth configuration via libvirt is not supported for + * hostdev network devices + */ + if (bandwidth && actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - bandwidth settings are not supported " + "for hostdev interfaces"), + macstr); + return -1; + } + + if (virDomainNetDefValidatePortOptions(macstr, actualType, vport, + virDomainNetGetActualPortOptionsIsolated(net)) < 0) { + return -1; + } + + return 0; +} + + +int +virDomainNetDefValidate(const virDomainNetDef *net) +{ + char macstr[VIR_MAC_STRING_BUFLEN]; + + virMacAddrFormat(&net->mac, macstr); + + if ((net->hostIP.nroutes || net->hostIP.nips) && + net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid attempt to set network interface " + "host-side IP route and/or address info on " + "interface of type '%s'. This is only supported " + "on interfaces of type 'ethernet'"), + virDomainNetTypeToString(net->type)); + return -1; + } + if (net->managed_tap == VIR_TRISTATE_BOOL_NO && + net->type != VIR_DOMAIN_NET_TYPE_ETHERNET) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unmanaged target dev is not supported on " + "interfaces of type '%s'"), + virDomainNetTypeToString(net->type)); + return -1; + } + + if (net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) { + if (!net->teaming.persistent) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("teaming persistent attribute must be set if teaming type is 'transient'")); + return -1; + } + } else { + if (net->teaming.persistent) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("teaming persistent attribute not allowed if teaming type is '%s'"), + virDomainNetTeamingTypeToString(net->teaming.type)); + return -1; + } + } + + if (virDomainNetDefValidatePortOptions(macstr, net->type, net->virtPortProfile, + net->isolatedPort) < 0) { + return -1; + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index e89cadab75..d38d2f4e06 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -49,3 +49,5 @@ int virDomainDefValidate(virDomainDefPtr def, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt, void *parseOpaque); +int virDomainActualNetDefValidate(const virDomainNetDef *net); +int virDomainNetDefValidate(const virDomainNetDef *net); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3cbc336bad..d0e548b0a3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -226,7 +226,6 @@ virDiskNameParse; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; -virDomainActualNetDefValidate; virDomainAudioTypeTypeFromString; virDomainAudioTypeTypeToString; virDomainBlockedReasonTypeFromString; @@ -743,6 +742,7 @@ virDomainConfVMNWFilterTeardown; # conf/domain_validate.h +virDomainActualNetDefValidate; virDomainDefValidate; virDomainDeviceValidateAliasForHotplug; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 3669b358f6..ab838b317c 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -35,6 +35,7 @@ #include "locking/domain_lock.h" #include "xen_common.h" #include "driver.h" +#include "domain_validate.h" #define VIR_FROM_THIS VIR_FROM_LIBXL diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6af274cb1b..c2563ef872 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -55,6 +55,7 @@ #include "virnetdevtap.h" #include "cpu/cpu.h" #include "virutil.h" +#include "domain_validate.h" #define VIR_FROM_THIS VIR_FROM_LIBXL diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b1295e71da..88d3890de7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -58,6 +58,7 @@ #include "domain_cgroup.h" #include "domain_driver.h" #include "domain_nwfilter.h" +#include "domain_validate.h" #include "virinitctl.h" #include "virnetdev.h" #include "virnetdevtap.h" diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0f818e2ee0..937e9a3fc1 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -42,6 +42,7 @@ #include "domain_nwfilter.h" #include "viralloc.h" #include "domain_audit.h" +#include "domain_validate.h" #include "virerror.h" #include "virlog.h" #include "vircommand.h" diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 766f76020c..bfb6e23942 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -48,6 +48,7 @@ #include "domain_capabilities.h" #include "domain_driver.h" #include "domain_event.h" +#include "domain_validate.h" #include "virtime.h" #include "virnetdevopenvswitch.h" #include "virstoragefile.h" -- 2.26.2

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 88 -------------------------------------- src/conf/domain_conf.h | 6 +-- src/conf/domain_validate.c | 88 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 5 +++ 4 files changed, 94 insertions(+), 93 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ebe895948f..26c48ef38c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6022,94 +6022,6 @@ virDomainDefHasUSB(const virDomainDef *def) } -static int -virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, - const virDomainDef *def) -{ - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - return virDomainDiskDefValidate(def, dev->data.disk); - - case VIR_DOMAIN_DEVICE_REDIRDEV: - return virDomainRedirdevDefValidate(def, dev->data.redirdev); - - case VIR_DOMAIN_DEVICE_NET: - return virDomainNetDefValidate(dev->data.net); - - case VIR_DOMAIN_DEVICE_CONTROLLER: - return virDomainControllerDefValidate(dev->data.controller); - - case VIR_DOMAIN_DEVICE_CHR: - return virDomainChrDefValidate(dev->data.chr, def); - - case VIR_DOMAIN_DEVICE_SMARTCARD: - return virDomainSmartcardDefValidate(dev->data.smartcard, def); - - case VIR_DOMAIN_DEVICE_RNG: - return virDomainRNGDefValidate(dev->data.rng, def); - - case VIR_DOMAIN_DEVICE_HOSTDEV: - return virDomainHostdevDefValidate(dev->data.hostdev); - - case VIR_DOMAIN_DEVICE_VIDEO: - return virDomainVideoDefValidate(dev->data.video, def); - - case VIR_DOMAIN_DEVICE_MEMORY: - return virDomainMemoryDefValidate(dev->data.memory, def); - - case VIR_DOMAIN_DEVICE_VSOCK: - return virDomainVsockDefValidate(dev->data.vsock); - - case VIR_DOMAIN_DEVICE_INPUT: - return virDomainInputDefValidate(dev->data.input); - - case VIR_DOMAIN_DEVICE_SHMEM: - return virDomainShmemDefValidate(dev->data.shmem); - - case VIR_DOMAIN_DEVICE_AUDIO: - /* TODO: validate? */ - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_LAST: - break; - } - - return 0; -} - - -int -virDomainDeviceDefValidate(const virDomainDeviceDef *dev, - const virDomainDef *def, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque) -{ - /* validate configuration only in certain places */ - if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) - return 0; - - if (xmlopt->config.deviceValidateCallback && - xmlopt->config.deviceValidateCallback(dev, def, xmlopt->config.priv, parseOpaque)) - return -1; - - if (virDomainDeviceDefValidateInternal(dev, def) < 0) - return -1; - - return 0; -} - - bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e35c4206df..ce8ed48a04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3122,11 +3122,7 @@ void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); void virDomainShmemDefFree(virDomainShmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); -int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, - const virDomainDef *def, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque); + G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, const virDomainDef *def, diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index ec955daf66..8acb7c1044 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1471,3 +1471,91 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem) return 0; } + + +static int +virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, + const virDomainDef *def) +{ + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + return virDomainDiskDefValidate(def, dev->data.disk); + + case VIR_DOMAIN_DEVICE_REDIRDEV: + return virDomainRedirdevDefValidate(def, dev->data.redirdev); + + case VIR_DOMAIN_DEVICE_NET: + return virDomainNetDefValidate(dev->data.net); + + case VIR_DOMAIN_DEVICE_CONTROLLER: + return virDomainControllerDefValidate(dev->data.controller); + + case VIR_DOMAIN_DEVICE_CHR: + return virDomainChrDefValidate(dev->data.chr, def); + + case VIR_DOMAIN_DEVICE_SMARTCARD: + return virDomainSmartcardDefValidate(dev->data.smartcard, def); + + case VIR_DOMAIN_DEVICE_RNG: + return virDomainRNGDefValidate(dev->data.rng, def); + + case VIR_DOMAIN_DEVICE_HOSTDEV: + return virDomainHostdevDefValidate(dev->data.hostdev); + + case VIR_DOMAIN_DEVICE_VIDEO: + return virDomainVideoDefValidate(dev->data.video, def); + + case VIR_DOMAIN_DEVICE_MEMORY: + return virDomainMemoryDefValidate(dev->data.memory, def); + + case VIR_DOMAIN_DEVICE_VSOCK: + return virDomainVsockDefValidate(dev->data.vsock); + + case VIR_DOMAIN_DEVICE_INPUT: + return virDomainInputDefValidate(dev->data.input); + + case VIR_DOMAIN_DEVICE_SHMEM: + return virDomainShmemDefValidate(dev->data.shmem); + + case VIR_DOMAIN_DEVICE_AUDIO: + /* TODO: validate? */ + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + + return 0; +} + + +int +virDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) +{ + /* validate configuration only in certain places */ + if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) + return 0; + + if (xmlopt->config.deviceValidateCallback && + xmlopt->config.deviceValidateCallback(dev, def, xmlopt->config.priv, parseOpaque)) + return -1; + + if (virDomainDeviceDefValidateInternal(dev, def) < 0) + return -1; + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index e04ecab065..710515a6ff 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -57,3 +57,8 @@ int virDomainMemoryDefValidate(const virDomainMemoryDef *mem, int virDomainVsockDefValidate(const virDomainVsockDef *vsock); int virDomainInputDefValidate(const virDomainInputDef *input); int virDomainShmemDefValidate(const virDomainShmemDef *shmem); +int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque); -- 2.26.2

All other validations from virDomainDefValidateInternal() are done in their own functions. Take IOMMU validation out of the function body and into its own function. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_validate.c | 41 ++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 2a881f1f06..4cc1d11647 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1042,6 +1042,31 @@ virDomainDefCputuneValidate(const virDomainDef *def) #undef CPUTUNE_VALIDATE_QUOTA +static int +virDomainDefIOMMUValidate(const virDomainDef *def) +{ + if (!def->iommu) + return 0; + + if (def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && + def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU interrupt remapping requires split I/O APIC " + "(ioapic driver='qemu')")); + return -1; + } + + if (def->iommu->eim == VIR_TRISTATE_SWITCH_ON && + def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU eim requires interrupt remapping to be enabled")); + return -1; + } + + return 0; +} + + static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOptionPtr xmlopt) @@ -1058,22 +1083,8 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefValidateAliases(def, NULL) < 0) return -1; - if (def->iommu && - def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && - def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("IOMMU interrupt remapping requires split I/O APIC " - "(ioapic driver='qemu')")); - return -1; - } - - if (def->iommu && - def->iommu->eim == VIR_TRISTATE_SWITCH_ON && - def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("IOMMU eim requires interrupt remapping to be enabled")); + if (virDomainDefIOMMUValidate(def) < 0) return -1; - } if (virDomainDefLifecycleActionValidate(def) < 0) return -1; -- 2.26.2