libxl: Move bdf parsing into libxlu

Config parsing functions do not properly belong in libxl.  Move them into
libxlu so that others can use them or not as they see fit.

No functional changes.  One side-effect was making public a private libxl
utility function which just set the elements of a structure from the  function
arguments passed in.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

tools/libxl/Makefile

@@ -57,7 +57,7 @@ $(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_lib
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
-	libxlu_disk_l.o libxlu_disk.o
+	libxlu_disk_l.o libxlu_disk.o libxlu_pci.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
 CLIENTS = xl testidl

tools/libxl/libxl.h

@@ -652,13 +652,6 @@ int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pc
 int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
 libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
 
-/*
- * Parse a PCI BDF into a PCI device structure.
- */
-int libxl_device_pci_parse_bdf(libxl_ctx *ctx,
-                               libxl_device_pci *pcidev,
-                               const char *str);
-
 /*
  * Similar to libxl_device_pci_list but returns all devices which
  * could be assigned to a domain (i.e. are bound to the backend

tools/libxl/libxl_pci.c

@@ -34,9 +34,9 @@ static unsigned int pcidev_encode_bdf(libxl_device_pci *pcidev)
     return value;
 }
 
-static int pcidev_init(libxl_device_pci *pcidev, unsigned int domain,
-                          unsigned int bus, unsigned int dev,
-                          unsigned int func, unsigned int vdevfn)
+static int pcidev_struct_fill(libxl_device_pci *pcidev, unsigned int domain,
+                               unsigned int bus, unsigned int dev,
+                               unsigned int func, unsigned int vdevfn)
 {
     pcidev->domain = domain;
     pcidev->bus = bus;
@@ -46,149 +46,6 @@ static int pcidev_init(libxl_device_pci *pcidev, unsigned int domain,
     return 0;
 }
 
-static int hex_convert(const char *str, unsigned int *val, unsigned int mask)
-{
-    unsigned long ret;
-    char *end;
-
-    ret = strtoul(str, &end, 16);
-    if ( end == str || *end != '\0' )
-        return -1;
-    if ( ret & ~mask )
-        return -1;
-    *val = (unsigned int)ret & mask;
-    return 0;
-}
-
-#define STATE_DOMAIN    0
-#define STATE_BUS       1
-#define STATE_DEV       2
-#define STATE_FUNC      3
-#define STATE_VSLOT     4
-#define STATE_OPTIONS_K 6
-#define STATE_OPTIONS_V 7
-#define STATE_TERMINAL  8
-int libxl_device_pci_parse_bdf(libxl_ctx *ctx, libxl_device_pci *pcidev, const char *str)
-{
-    unsigned state = STATE_DOMAIN;
-    unsigned dom, bus, dev, func, vslot = 0;
-    char *buf2, *tok, *ptr, *end, *optkey = NULL;
-
-    if ( NULL == (buf2 = ptr = strdup(str)) )
-        return ERROR_NOMEM;
-
-    for(tok = ptr, end = ptr + strlen(ptr) + 1; ptr < end; ptr++) {
-        switch(state) {
-        case STATE_DOMAIN:
-            if ( *ptr == ':' ) {
-                state = STATE_BUS;
-                *ptr = '\0';
-                if ( hex_convert(tok, &dom, 0xffff) )
-                    goto parse_error;
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_BUS:
-            if ( *ptr == ':' ) {
-                state = STATE_DEV;
-                *ptr = '\0';
-                if ( hex_convert(tok, &bus, 0xff) )
-                    goto parse_error;
-                tok = ptr + 1;
-            }else if ( *ptr == '.' ) {
-                state = STATE_FUNC;
-                *ptr = '\0';
-                if ( dom & ~0xff )
-                    goto parse_error;
-                bus = dom;
-                dom = 0;
-                if ( hex_convert(tok, &dev, 0xff) )
-                    goto parse_error;
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_DEV:
-            if ( *ptr == '.' ) {
-                state = STATE_FUNC;
-                *ptr = '\0';
-                if ( hex_convert(tok, &dev, 0xff) )
-                    goto parse_error;
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_FUNC:
-            if ( *ptr == '\0' || *ptr == '@' || *ptr == ',' ) {
-                switch( *ptr ) {
-                case '\0':
-                    state = STATE_TERMINAL;
-                    break;
-                case '@':
-                    state = STATE_VSLOT;
-                    break;
-                case ',':
-                    state = STATE_OPTIONS_K;
-                    break;
-                }
-                *ptr = '\0';
-                if ( !strcmp(tok, "*") ) {
-                    pcidev->vfunc_mask = LIBXL_PCI_FUNC_ALL;
-                }else{
-                    if ( hex_convert(tok, &func, 0x7) )
-                        goto parse_error;
-                    pcidev->vfunc_mask = (1 << 0);
-                }
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_VSLOT:
-            if ( *ptr == '\0' || *ptr == ',' ) {
-                state = ( *ptr == ',' ) ? STATE_OPTIONS_K : STATE_TERMINAL;
-                *ptr = '\0';
-                if ( hex_convert(tok, &vslot, 0xff) )
-                    goto parse_error;
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_OPTIONS_K:
-            if ( *ptr == '=' ) {
-                state = STATE_OPTIONS_V;
-                *ptr = '\0';
-                optkey = tok;
-                tok = ptr + 1;
-            }
-            break;
-        case STATE_OPTIONS_V:
-            if ( *ptr == ',' || *ptr == '\0' ) {
-                state = (*ptr == ',') ? STATE_OPTIONS_K : STATE_TERMINAL;
-                *ptr = '\0';
-                if ( !strcmp(optkey, "msitranslate") ) {
-                    pcidev->msitranslate = atoi(tok);
-                }else if ( !strcmp(optkey, "power_mgmt") ) {
-                    pcidev->power_mgmt = atoi(tok);
-                }else{
-                    LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
-                           "Unknown PCI BDF option: %s", optkey);
-                }
-                tok = ptr + 1;
-            }
-        default:
-            break;
-        }
-    }
-
-    free(buf2);
-
-    if ( tok != ptr || state != STATE_TERMINAL )
-        goto parse_error;
-
-    pcidev_init(pcidev, dom, bus, dev, func, vslot << 3);
-
-    return 0;
-
-parse_error:
-    return ERROR_INVAL;
-}
-
 static void libxl_create_pci_backend_device(libxl__gc *gc, flexarray_t *back, int num, libxl_device_pci *pcidev)
 {
     flexarray_append(back, libxl__sprintf(gc, "key-%d", num));
@@ -436,7 +293,7 @@ static int get_all_assigned_devices(libxl__gc *gc, libxl_device_pci **list, int
                     *list = realloc(*list, sizeof(libxl_device_pci) * ((*num) + 1));
                     if (*list == NULL)
                         return ERROR_NOMEM;
-                    pcidev_init(*list + *num, dom, bus, dev, func, 0);
+                    pcidev_struct_fill(*list + *num, dom, bus, dev, func, 0);
                     (*num)++;
                 }
             }
@@ -507,7 +364,7 @@ libxl_device_pci *libxl_device_pci_list_assignable(libxl_ctx *ctx, int *num)
         new = pcidevs + *num;
 
         memset(new, 0, sizeof(*new));
-        pcidev_init(new, dom, bus, dev, func, 0);
+        pcidev_struct_fill(new, dom, bus, dev, func, 0);
         (*num)++;
     }
 
@@ -1086,7 +943,7 @@ static void libxl__device_pci_from_xs_be(libxl__gc *gc,
     if (s)
         vdevfn = strtol(s, (char **) NULL, 16);
 
-    pcidev_init(pci, domain, bus, dev, func, vdevfn);
+    pcidev_struct_fill(pci, domain, bus, dev, func, vdevfn);
 
     s = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/opts-%d", be_path, nr));
     if (s) {

tools/libxl/libxlu_pci.c

@@ -0,0 +1,172 @@
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxlu_internal.h"
+#include "libxlu_disk_l.h"
+#include "libxlu_disk_i.h"
+#include "libxlu_cfg_i.h"
+
+
+#define XLU__PCI_ERR(_c, _x, _a...) \
+    if((_c) && (_c)->report) fprintf((_c)->report, _x, ##_a)
+
+static int hex_convert(const char *str, unsigned int *val, unsigned int mask)
+{
+    unsigned long ret;
+    char *end;
+
+    ret = strtoul(str, &end, 16);
+    if ( end == str || *end != '\0' )
+        return -1;
+    if ( ret & ~mask )
+        return -1;
+    *val = (unsigned int)ret & mask;
+    return 0;
+}
+
+static int pcidev_struct_fill(libxl_device_pci *pcidev, unsigned int domain,
+                               unsigned int bus, unsigned int dev,
+                               unsigned int func, unsigned int vdevfn)
+{
+    pcidev->domain = domain;
+    pcidev->bus = bus;
+    pcidev->dev = dev;
+    pcidev->func = func;
+    pcidev->vdevfn = vdevfn;
+    return 0;
+}
+
+#define STATE_DOMAIN    0
+#define STATE_BUS       1
+#define STATE_DEV       2
+#define STATE_FUNC      3
+#define STATE_VSLOT     4
+#define STATE_OPTIONS_K 6
+#define STATE_OPTIONS_V 7
+#define STATE_TERMINAL  8
+int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str)
+{
+    unsigned state = STATE_DOMAIN;
+    unsigned dom, bus, dev, func, vslot = 0;
+    char *buf2, *tok, *ptr, *end, *optkey = NULL;
+
+    if ( NULL == (buf2 = ptr = strdup(str)) )
+        return ERROR_NOMEM;
+
+    for(tok = ptr, end = ptr + strlen(ptr) + 1; ptr < end; ptr++) {
+        switch(state) {
+        case STATE_DOMAIN:
+            if ( *ptr == ':' ) {
+                state = STATE_BUS;
+                *ptr = '\0';
+                if ( hex_convert(tok, &dom, 0xffff) )
+                    goto parse_error;
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_BUS:
+            if ( *ptr == ':' ) {
+                state = STATE_DEV;
+                *ptr = '\0';
+                if ( hex_convert(tok, &bus, 0xff) )
+                    goto parse_error;
+                tok = ptr + 1;
+            }else if ( *ptr == '.' ) {
+                state = STATE_FUNC;
+                *ptr = '\0';
+                if ( dom & ~0xff )
+                    goto parse_error;
+                bus = dom;
+                dom = 0;
+                if ( hex_convert(tok, &dev, 0xff) )
+                    goto parse_error;
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_DEV:
+            if ( *ptr == '.' ) {
+                state = STATE_FUNC;
+                *ptr = '\0';
+                if ( hex_convert(tok, &dev, 0xff) )
+                    goto parse_error;
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_FUNC:
+            if ( *ptr == '\0' || *ptr == '@' || *ptr == ',' ) {
+                switch( *ptr ) {
+                case '\0':
+                    state = STATE_TERMINAL;
+                    break;
+                case '@':
+                    state = STATE_VSLOT;
+                    break;
+                case ',':
+                    state = STATE_OPTIONS_K;
+                    break;
+                }
+                *ptr = '\0';
+                if ( !strcmp(tok, "*") ) {
+                    pcidev->vfunc_mask = LIBXL_PCI_FUNC_ALL;
+                }else{
+                    if ( hex_convert(tok, &func, 0x7) )
+                        goto parse_error;
+                    pcidev->vfunc_mask = (1 << 0);
+                }
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_VSLOT:
+            if ( *ptr == '\0' || *ptr == ',' ) {
+                state = ( *ptr == ',' ) ? STATE_OPTIONS_K : STATE_TERMINAL;
+                *ptr = '\0';
+                if ( hex_convert(tok, &vslot, 0xff) )
+                    goto parse_error;
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_OPTIONS_K:
+            if ( *ptr == '=' ) {
+                state = STATE_OPTIONS_V;
+                *ptr = '\0';
+                optkey = tok;
+                tok = ptr + 1;
+            }
+            break;
+        case STATE_OPTIONS_V:
+            if ( *ptr == ',' || *ptr == '\0' ) {
+                state = (*ptr == ',') ? STATE_OPTIONS_K : STATE_TERMINAL;
+                *ptr = '\0';
+                if ( !strcmp(optkey, "msitranslate") ) {
+                    pcidev->msitranslate = atoi(tok);
+                }else if ( !strcmp(optkey, "power_mgmt") ) {
+                    pcidev->power_mgmt = atoi(tok);
+                }else{
+                    XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey);
+                }
+                tok = ptr + 1;
+            }
+        default:
+            break;
+        }
+    }
+
+    free(buf2);
+
+    if ( tok != ptr || state != STATE_TERMINAL )
+        goto parse_error;
+
+    /* Just a pretty way to fill in the values */
+    pcidev_struct_fill(pcidev, dom, bus, dev, func, vslot << 3);
+
+    return 0;
+
+parse_error:
+    return ERROR_INVAL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

tools/libxl/libxlutil.h

@@ -88,6 +88,11 @@ int xlu_disk_parse(XLU_Config *cfg, int nspecs, const char *const *specs,
    * resulting disk struct is used with libxl.
    */
 
+/*
+ * PCI specification parsing
+ */
+int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str);
+
 
 #endif /* LIBXLUTIL_H */
 

tools/libxl/xl_cmdimpl.c

@@ -1010,7 +1010,7 @@ skip_vfb:
 
             pcidev->msitranslate = pci_msitranslate;
             pcidev->power_mgmt = pci_power_mgmt;
-            if (!libxl_device_pci_parse_bdf(ctx, pcidev, buf))
+            if (!xlu_pci_parse_bdf(config, pcidev, buf))
                 d_config->num_pcidevs++;
         }
         if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
@@ -2253,11 +2253,16 @@ int main_pcilist(int argc, char **argv)
 static void pcidetach(const char *dom, const char *bdf, int force)
 {
     libxl_device_pci pcidev;
+    XLU_Config *config;
 
     find_domain(dom);
 
     memset(&pcidev, 0x00, sizeof(pcidev));
-    if (libxl_device_pci_parse_bdf(ctx, &pcidev, bdf)) {
+    
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) { perror("xlu_cfg_inig"); exit(-1); }
+
+    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-detach: malformed BDF specification \"%s\"\n", bdf);
         exit(2);
     }
@@ -2293,11 +2298,16 @@ int main_pcidetach(int argc, char **argv)
 static void pciattach(const char *dom, const char *bdf, const char *vs)
 {
     libxl_device_pci pcidev;
+    XLU_Config *config;
 
     find_domain(dom);
 
     memset(&pcidev, 0x00, sizeof(pcidev));
-    if (libxl_device_pci_parse_bdf(ctx, &pcidev, bdf)) {
+
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) { perror("xlu_cfg_inig"); exit(-1); }
+
+    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-attach: malformed BDF specification \"%s\"\n", bdf);
         exit(2);
     }

tools/python/xen/lowlevel/xl/xl.c

@@ -40,6 +40,7 @@
 
 #include <libxl.h>
 #include <libxl_utils.h>
+#include <libxlutil.h>
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
@@ -556,7 +557,7 @@ static PyObject *pyxl_pci_parse(XlObject *self, PyObject *args)
         return NULL;
     }
 
-    if ( libxl_device_pci_parse_bdf(self->ctx, &pci->obj, str) ) {
+    if ( xlu_pci_parse_bdf(NULL, &pci->obj, str) ) {
         PyErr_SetString(xl_error_obj, "cannot parse pci device spec (BDF)");
         Py_DECREF(pci);
         return NULL;