Re: [PATCH v1 07/14] conf: Parse user supplied aliases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 19, 2017 at 10:11:02AM +0200, Michal Privoznik wrote:
If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/conf/domain_conf.c | 13 +++++++++++--
src/conf/domain_conf.h |  1 +
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f4de4e288..f1386c116 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6398,6 +6398,10 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
}


+#define USER_ALIAS_PREFIX "ua-"
+#define USER_ALIAS_CHARS \
+    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
+
/* Parse the XML definition for a device address
 * @param node XML nodeset to parse for device address definition
 */
@@ -6424,7 +6428,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
    while (cur != NULL) {
        if (cur->type == XML_ELEMENT_NODE) {
            if (alias == NULL &&
-                !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
                virXMLNodeNameEqual(cur, "alias")) {
                alias = cur;
            } else if (address == NULL &&
@@ -6446,8 +6449,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
        cur = cur->next;
    }

-    if (alias)
+    if (alias) {
        info->alias = virXMLPropString(alias, "name");
+        if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
+            (!STRPREFIX(info->alias, USER_ALIAS_PREFIX) ||
+             !(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_USER_ALIAS) ||
+             strspn(info->alias, USER_ALIAS_CHARS) < strlen(info->alias)))
+            VIR_FREE(info->alias);

Assigning the pointer just so you can clear it after some hard to read condition
seems just plain wrong.  Only assign it if the condition is false.

+    }

    if (master) {
        info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1e007346f..5ff85057f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2508,6 +2508,7 @@ typedef enum {
    VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN = (1 << 2),
    VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
    VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
+    VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),

Can you elaborate on why is this not the default and whydo we need to have a
flag for it?  If it's just so that individual drivers can decide this, adding
this to the commit message is enough and ACK with this and the above fixed.

Also ACK to previous patches.

} virDomainDefFeatures;


--
2.13.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux