[libvirt] [PATCH v2 03/11] conf: Validate user supplied aliases

[Ján Tomko]

On Fri, Oct 20, 2017 at 04:52:13PM +0200, Michal Privoznik wrote:
>They have to be unique within the domain. As usual, backwards
>compatibility takes its price. In this particular situation we
>have a device that is represented twice in a domain and so is its
>alias.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/conf/domain_conf.c   | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h   |   5 ++
> src/libvirt_private.syms |   1 +
> src/qemu/qemu_driver.c   |   3 +
> 4 files changed, 155 insertions(+), 2 deletions(-)
>

ACK with the 'xmlopt' and 'parseFlags' arguments removed, see below:

>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 40fcbc7df..ad71e951b 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -5457,6 +5457,145 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
> }
>
>
>+struct virDomainDefValidateAliasesData {
>+    virHashTablePtr aliases;
>+};
>+
>+
>+static int
>+virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def,
>+                                          virDomainDeviceDefPtr dev,
>+                                          virDomainDeviceInfoPtr info,
>+                                          void *opaque)
>+{
>+    struct virDomainDefValidateAliasesData *data = opaque;
>+    const char *alias = info->alias;
>+
>+    if (!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 (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(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,

xmlopt is unused here and all the other occurences in this patch.

>+                            const virDomainDef *def,
>+                            virHashTablePtr *aliases,
>+                            unsigned int parseFlags)
>+{
>+    struct virDomainDefValidateAliasesData data;
>+    int ret = -1;
>+
>+    /* validate configuration only in certain places */
>+    if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)
>+        return 0;

This is checked from the virDomainDefValidateInternal path
before virDomainDefValidateInternal is even called.

And virDomainDeviceValidateAliasImpl always passes 0,
so this condition is redundant and 'parseFlags' can be dropped too.

>+
>+    /* We are not storing copies of aliases. Don't free them. */
>+    if (!(data.aliases = virHashCreate(10, NULL)))
>+        goto cleanup;
>+

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171023/b61ef6a9/attachment-0001.sig>