xenParsePCI() does a lot of stuff and, in order to make things cleaner,
let's split it in two new functions:
- xenParsePCI(): it's a new function that keeps the old name. It's
responsible for the whole per-PCI logic from the old xenParsePCI();
- xenParsePCIList(): it's basically the old xenParsePCI(), but now it
just iterates over the list of PCIs, calling xenParsePCI() per each PCI.

This patch is basically preparing the ground for the future when
typesafe virConf acessors will be used.

Signed-off-by: Fabiano Fidêncio <fabiano@fidencio.org>
---
 src/xenconfig/xen_common.c | 167 ++++++++++++++++++++++++---------------------
 1 file changed, 89 insertions(+), 78 deletions(-)

diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index a2b0708ee3..fc7b0683b8 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -389,92 +389,103 @@ xenParseEventsActions(virConfPtr conf, virDomainDefPtr def)
 }
 
 
-static int
-xenParsePCI(virConfPtr conf, virDomainDefPtr def)
+static virDomainHostdevDefPtr
+xenParsePCI(char *entry)
 {
-    virConfValuePtr list = virConfGetValue(conf, "pci");
     virDomainHostdevDefPtr hostdev = NULL;
+    char domain[5];
+    char bus[3];
+    char slot[3];
+    char func[2];
+    char *key, *nextkey;
+    int domainID;
+    int busID;
+    int slotID;
+    int funcID;
+
+    domain[0] = bus[0] = slot[0] = func[0] = '\0';
+
+    /* pci=['0000:00:1b.0','0000:00:13.0'] */
+    if (!(key = entry))
+        return NULL;
+    if (!(nextkey = strchr(key, ':')))
+        return NULL;
+    if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Domain %s too big for destination"), key);
+        return NULL;
+    }
 
-    if (list && list->type == VIR_CONF_LIST) {
-        list = list->list;
-        while (list) {
-            char domain[5];
-            char bus[3];
-            char slot[3];
-            char func[2];
-            char *key, *nextkey;
-            int domainID;
-            int busID;
-            int slotID;
-            int funcID;
-
-            domain[0] = bus[0] = slot[0] = func[0] = '\0';
+    key = nextkey + 1;
+    if (!(nextkey = strchr(key, ':')))
+        return NULL;
+    if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Bus %s too big for destination"), key);
+        return NULL;
+    }
 
-            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
-                goto skippci;
-            /* pci=['0000:00:1b.0','0000:00:13.0'] */
-            if (!(key = list->str))
-                goto skippci;
-            if (!(nextkey = strchr(key, ':')))
-                goto skippci;
-            if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Domain %s too big for destination"), key);
-                goto skippci;
-            }
+    key = nextkey + 1;
+    if (!(nextkey = strchr(key, '.')))
+        return NULL;
+    if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Slot %s too big for destination"), key);
+        return NULL;
+    }
 
-            key = nextkey + 1;
-            if (!(nextkey = strchr(key, ':')))
-                goto skippci;
-            if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Bus %s too big for destination"), key);
-                goto skippci;
-            }
+    key = nextkey + 1;
+    if (strlen(key) != 1)
+        return NULL;
+    if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Function %s too big for destination"), key);
+        return NULL;
+    }
 
-            key = nextkey + 1;
-            if (!(nextkey = strchr(key, '.')))
-                goto skippci;
-            if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Slot %s too big for destination"), key);
-                goto skippci;
-            }
+    if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
+        return NULL;
+    if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
+        return NULL;
+    if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
+        return NULL;
+    if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
+        return NULL;
 
-            key = nextkey + 1;
-            if (strlen(key) != 1)
-                goto skippci;
-            if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Function %s too big for destination"), key);
-                goto skippci;
-            }
+    if (!(hostdev = virDomainHostdevDefNew()))
+       return NULL;
 
-            if (virStrToLong_i(domain, NULL, 16, &domainID) < 0)
-                goto skippci;
-            if (virStrToLong_i(bus, NULL, 16, &busID) < 0)
-                goto skippci;
-            if (virStrToLong_i(slot, NULL, 16, &slotID) < 0)
-                goto skippci;
-            if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
-                goto skippci;
-            if (!(hostdev = virDomainHostdevDefNew()))
-               return -1;
-
-            hostdev->managed = false;
-            hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
-            hostdev->source.subsys.u.pci.addr.domain = domainID;
-            hostdev->source.subsys.u.pci.addr.bus = busID;
-            hostdev->source.subsys.u.pci.addr.slot = slotID;
-            hostdev->source.subsys.u.pci.addr.function = funcID;
-
-            if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) {
-                virDomainHostdevDefFree(hostdev);
-                return -1;
-            }
+    hostdev->managed = false;
+    hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
+    hostdev->source.subsys.u.pci.addr.domain = domainID;
+    hostdev->source.subsys.u.pci.addr.bus = busID;
+    hostdev->source.subsys.u.pci.addr.slot = slotID;
+    hostdev->source.subsys.u.pci.addr.function = funcID;
 
-        skippci:
-            list = list->next;
+    return hostdev;
+}
+
+
+static int
+xenParsePCIList(virConfPtr conf, virDomainDefPtr def)
+{
+    virConfValuePtr list = virConfGetValue(conf, "pci");
+
+    if (!list || list->type != VIR_CONF_LIST)
+        return 0;
+
+    for (list = list->list; list; list = list->next) {
+        virDomainHostdevDefPtr hostdev;
+
+        if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+            continue;
+
+        if (!(hostdev = xenParsePCI(list->str)))
+            return -1;
+
+        if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) {
+            virDomainHostdevDefFree(hostdev);
+            return -1;
         }
     }
 
@@ -1126,7 +1137,7 @@ xenParseConfigCommon(virConfPtr conf,
         return -1;
     }
 
-    if (xenParsePCI(conf, def) < 0)
+    if (xenParsePCIList(conf, def) < 0)
         return -1;
 
     if (xenParseEmulatedDevices(conf, def) < 0)
-- 
2.14.3

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

On Mon, May 28, 2018 at 12:28:23AM +0200, Fabiano Fidêncio wrote:
>xenParsePCI() does a lot of stuff and, in order to make things cleaner,
>let's split it in two new functions:
>- xenParsePCI(): it's a new function that keeps the old name. It's
>responsible for the whole per-PCI logic from the old xenParsePCI();
>- xenParsePCIList(): it's basically the old xenParsePCI(), but now it
>just iterates over the list of PCIs, calling xenParsePCI() per each PCI.
>
>This patch is basically preparing the ground for the future when
>typesafe virConf acessors will be used.
>
>Signed-off-by: Fabiano Fidêncio <fabiano@fidencio.org>
>---
> src/xenconfig/xen_common.c | 167 ++++++++++++++++++++++++---------------------
> 1 file changed, 89 insertions(+), 78 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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