All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC/v2] libxl: postpone backend name resolution
@ 2012-12-13 21:47 Daniel De Graaf
  2012-12-17  9:57 ` Ian Campbell
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel De Graaf @ 2012-12-13 21:47 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, Ian Jackson, Ian Campbell, Stefano Stabellini

This replaces the backend_domid field in libxl devices with a structure
allowing either a domid or a domain name to be specified.  The domain
name is resolved into a domain ID in the _setdefault function when
adding the device.  This change allows the backend of the block devices
to be specified (which previously required passing the libxl_ctx down
into the block device parser).

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>

---

This patch does not include the changes to tools/libxl/libxlu_disk_l.c
and tools/libxl/libxlu_disk_l.h because the diffs contain unrelated
changes due to different generator versions.

 docs/misc/xl-disk-configuration.txt | 12 +++++++
 tools/libxl/libxl.c                 | 65 +++++++++++++++++++++++++------------
 tools/libxl/libxl_dm.c              |  6 ++--
 tools/libxl/libxl_types.idl         | 15 ++++++---
 tools/libxl/libxl_utils.c           |  2 +-
 tools/libxl/libxlu_disk_l.l         |  1 +
 tools/libxl/xl_cmdimpl.c            | 36 ++++----------------
 tools/libxl/xl_sxp.c                |  6 ++--
 8 files changed, 82 insertions(+), 61 deletions(-)

diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 86c16be..5bd456d 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -139,6 +139,18 @@ cdrom
 Convenience alias for "devtype=cdrom".
 
 
+backend=<domain-name>
+---------------------
+
+Description:           Designates a backend domain for the device
+Supported values:      Valid domain names
+Mandatory:             No
+
+Specifies the backend domain which this device should attach to. This
+defaults to domain 0. Specifying another domain requires setting up a
+driver domain which is outside the scope of this document.
+
+
 backendtype=<backend-type>
 --------------------------
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6c4455e..edb29b3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1153,7 +1153,7 @@ static void disk_eject_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
     sscanf(backend,
             "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
            "[a-z]/%*d/%*d",
-           &disk->backend_domid, backend_type);
+           &disk->backend_domain.domid, backend_type);
     if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
         disk->backend = LIBXL_DISK_BACKEND_TAP;
     } else if (!strcmp(backend_type, "qdisk")) {
@@ -1710,13 +1710,36 @@ out:
     return;
 }
 
+/* backend domain name-to-domid conversion utility */
+static int libxl__resolve_domain(libxl__gc *gc, struct libxl_domain_desc *desc)
+{
+    int i, rv;
+    const char *name = desc->name;
+    if (!name)
+        return 0;
+    for (i=0; name[i]; i++) {
+        if (!isdigit(name[i])) {
+            rv = libxl_name_to_domid(libxl__gc_owner(gc), name, &desc->domid);
+            if (rv)
+                return rv;
+            goto resolved;
+        }
+    }
+    desc->domid = atoi(name);
+
+ resolved:
+    free(desc->name);
+    desc->name = NULL;
+    return 0;
+}
+
 /******************************************************************************/
 int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
 {
    if(libxl_uuid_is_nil(&vtpm->uuid)) {
       libxl_uuid_generate(&vtpm->uuid);
    }
-   return 0;
+   return libxl__resolve_domain(gc, &vtpm->backend_domain);
 }
 
 static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
@@ -1724,7 +1747,7 @@ static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
                                    libxl__device *device)
 {
    device->backend_devid   = vtpm->devid;
-   device->backend_domid   = vtpm->backend_domid;
+   device->backend_domid   = vtpm->backend_domain.domid;
    device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
    device->devid           = vtpm->devid;
    device->domid           = domid;
@@ -1783,7 +1806,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
     flexarray_append(back, "False");
 
     flexarray_append(front, "backend-id");
-    flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domid));
+    flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domain.domid));
     flexarray_append(front, "state");
     flexarray_append(front, GCSPRINTF("%d", 1));
     flexarray_append(front, "handle");
@@ -1832,7 +1855,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *n
           tmp = libxl__xs_read(gc, XBT_NULL,
                 GCSPRINTF("%s/%s/backend_id",
                    fe_path, *dir));
-          vtpm->backend_domid = atoi(tmp);
+          vtpm->backend_domain.domid = atoi(tmp);
 
           tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", be_path));
           if(tmp) {
@@ -1934,7 +1957,7 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
     rc = 1;
     for (i = 0; i < nb; ++i) {
         if(devid == vtpms[i].devid) {
-            vtpm->backend_domid = vtpms[i].backend_domid;
+            vtpm->backend_domain.domid = vtpms[i].backend_domain.domid;
             vtpm->devid = vtpms[i].devid;
             libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
             rc = 0;
@@ -1956,7 +1979,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
     rc = libxl__device_disk_set_backend(gc, disk);
     if (rc) return rc;
 
-    return rc;
+    return libxl__resolve_domain(gc, &disk->backend_domain);
 }
 
 int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
@@ -1973,7 +1996,7 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
         return ERROR_INVAL;
     }
 
-    device->backend_domid = disk->backend_domid;
+    device->backend_domid = disk->backend_domain.domid;
     device->backend_devid = devid;
 
     switch (disk->backend) {
@@ -2133,7 +2156,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
 
         flexarray_append(front, "backend-id");
-        flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
+        flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domain.domid));
         flexarray_append(front, "state");
         flexarray_append(front, libxl__sprintf(gc, "%d", 1));
         flexarray_append(front, "virtual-device");
@@ -2298,7 +2321,7 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc,
             p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
             if ((rc=libxl__device_disk_from_xs_be(gc, p, pdisk)))
                 goto out;
-            pdisk->backend_domid = 0;
+            pdisk->backend_domain.domid = 0;
             *ndisks += 1;
         }
     }
@@ -2759,7 +2782,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
         LOG(ERROR, "unable to get current hotplug scripts execution setting");
         return run_hotplug_scripts;
     }
-    if (nic->backend_domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) {
+    if (nic->backend_domain.domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) {
         LOG(ERROR, "cannot use a backend domain different than %d if"
                    "hotplug scripts are executed from libxl",
                    LIBXL_TOOLSTACK_DOMID);
@@ -2784,7 +2807,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
         abort();
     }
 
-    return 0;
+    return libxl__resolve_domain(gc, &nic->backend_domain);
 }
 
 static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
@@ -2792,7 +2815,7 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
                                   libxl__device *device)
 {
     device->backend_devid    = nic->devid;
-    device->backend_domid    = nic->backend_domid;
+    device->backend_domid    = nic->backend_domain.domid;
     device->backend_kind     = LIBXL__DEVICE_KIND_VIF;
     device->devid            = nic->devid;
     device->domid            = domid;
@@ -2874,7 +2897,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
                                      libxl_nic_type_to_string(nic->nictype)));
 
     flexarray_append(front, "backend-id");
-    flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
+    flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domain.domid));
     flexarray_append(front, "state");
     flexarray_append(front, libxl__sprintf(gc, "%d", 1));
     flexarray_append(front, "handle");
@@ -2991,7 +3014,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
             const char *p;
             p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
             libxl__device_nic_from_xs_be(gc, p, pnic);
-            pnic->backend_domid = 0;
+            pnic->backend_domain.domid = 0;
         }
     }
     return 0;
@@ -3144,7 +3167,7 @@ out:
 
 int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
 {
-    return 0;
+    return libxl__resolve_domain(gc, &vkb->backend_domain);
 }
 
 static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
@@ -3152,7 +3175,7 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
                                   libxl__device *device)
 {
     device->backend_devid = vkb->devid;
-    device->backend_domid = vkb->backend_domid;
+    device->backend_domid = vkb->backend_domain.domid;
     device->backend_kind = LIBXL__DEVICE_KIND_VKBD;
     device->devid = vkb->devid;
     device->domid = domid;
@@ -3205,7 +3228,7 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(back, libxl__domid_to_name(gc, domid));
 
     flexarray_append(front, "backend-id");
-    flexarray_append(front, libxl__sprintf(gc, "%d", vkb->backend_domid));
+    flexarray_append(front, libxl__sprintf(gc, "%d", vkb->backend_domain.domid));
     flexarray_append(front, "state");
     flexarray_append(front, libxl__sprintf(gc, "%d", 1));
 
@@ -3236,7 +3259,7 @@ int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
     libxl_defbool_setdefault(&vfb->sdl.enable, false);
     libxl_defbool_setdefault(&vfb->sdl.opengl, false);
 
-    return 0;
+    return libxl__resolve_domain(gc, &vfb->backend_domain);
 }
 
 static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
@@ -3244,7 +3267,7 @@ static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
                                   libxl__device *device)
 {
     device->backend_devid = vfb->devid;
-    device->backend_domid = vfb->backend_domid;
+    device->backend_domid = vfb->backend_domain.domid;
     device->backend_kind = LIBXL__DEVICE_KIND_VFB;
     device->devid = vfb->devid;
     device->domid = domid;
@@ -3309,7 +3332,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
     }
 
     flexarray_append_pair(front, "backend-id",
-                          libxl__sprintf(gc, "%d", vfb->backend_domid));
+                          libxl__sprintf(gc, "%d", vfb->backend_domain.domid));
     flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c036dc1..82a55ea 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -644,13 +644,15 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
     libxl_device_vfb_init(vfb);
     libxl_device_vkb_init(vkb);
 
-    vfb->backend_domid = 0;
+    vfb->backend_domain.domid = 0;
+    vfb->backend_domain.name = NULL;
     vfb->devid = 0;
     vfb->vnc = b_info->u.hvm.vnc;
     vfb->keymap = b_info->u.hvm.keymap;
     vfb->sdl = b_info->u.hvm.sdl;
 
-    vkb->backend_domid = 0;
+    vkb->backend_domain.domid = 0;
+    vkb->backend_domain.name = NULL;
     vkb->devid = 0;
     return 0;
 }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 93524f0..6ec7967 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -136,6 +136,11 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
 # Complex libxl types
 #
 
+libxl_domain_desc = Struct("domain_desc", [
+    ("domid", libxl_domid),
+    ("name",  string),
+    ])
+
 libxl_ioport_range = Struct("ioport_range", [
     ("first", uint32),
     ("number", uint32),
@@ -344,7 +349,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 )
 
 libxl_device_vfb = Struct("device_vfb", [
-    ("backend_domid", libxl_domid),
+    ("backend_domain",libxl_domain_desc),
     ("devid",         libxl_devid),
     ("vnc",           libxl_vnc_info),
     ("sdl",           libxl_sdl_info),
@@ -353,12 +358,12 @@ libxl_device_vfb = Struct("device_vfb", [
     ])
 
 libxl_device_vkb = Struct("device_vkb", [
-    ("backend_domid", libxl_domid),
+    ("backend_domain", libxl_domain_desc),
     ("devid", libxl_devid),
     ])
 
 libxl_device_disk = Struct("device_disk", [
-    ("backend_domid", libxl_domid),
+    ("backend_domain", libxl_domain_desc),
     ("pdev_path", string),
     ("vdev", string),
     ("backend", libxl_disk_backend),
@@ -370,7 +375,7 @@ libxl_device_disk = Struct("device_disk", [
     ])
 
 libxl_device_nic = Struct("device_nic", [
-    ("backend_domid", libxl_domid),
+    ("backend_domain", libxl_domain_desc),
     ("devid", libxl_devid),
     ("mtu", integer),
     ("model", string),
@@ -397,7 +402,7 @@ libxl_device_pci = Struct("device_pci", [
     ])
 
 libxl_device_vtpm = Struct("device_vtpm", [
-    ("backend_domid",    libxl_domid),
+    ("backend_domain",   libxl_domain_desc),
     ("devid",            libxl_devid),
     ("uuid",             libxl_uuid),
 ])
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8f78790..86a310e 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -478,7 +478,7 @@ int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
     rc = 1;
     for (i = 0; i < nb; ++i) {
         if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
-            vtpm->backend_domid = vtpms[i].backend_domid;
+            vtpm->backend_domain.domid = vtpms[i].backend_domain.domid;
             vtpm->devid = vtpms[i].devid;
             libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
             rc = 0;
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index bee16a1..a65030f 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -168,6 +168,7 @@ devtype=disk,?	{ DPC->disk->is_cdrom = 0; }
 devtype=[^,]*,?	{ xlu__disk_err(DPC,yytext,"unknown value for type"); }
 
 access=[^,]*,?	{ STRIP(','); setaccess(DPC, FROMEQUALS); }
+backend=[^,]*,? { STRIP(','); SAVESTRING("backend", backend_domain.name, FROMEQUALS); }
 backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e964bf1..bc22f3c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1090,12 +1090,7 @@ static void parse_config_data(const char *config_source,
                      break;
                   *p2 = '\0';
                   if (!strcmp(p, "backend")) {
-                     if(domain_qualifier_to_domid(p2 + 1, &(vtpm->backend_domid), 0))
-                     {
-                        fprintf(stderr,
-                              "Specified vtpm backend domain `%s' does not exist!\n", p2 + 1);
-                        exit(1);
-                     }
+                     vtpm->backend_domain.name = strdup(p2 + 1);
                      got_backend = true;
                   } else if(!strcmp(p, "uuid")) {
                      if( libxl_uuid_from_string(&vtpm->uuid, p2 + 1) ) {
@@ -1190,11 +1185,8 @@ static void parse_config_data(const char *config_source,
                     free(nic->ifname);
                     nic->ifname = strdup(p2 + 1);
                 } else if (!strcmp(p, "backend")) {
-                    if(libxl_name_to_domid(ctx, (p2 + 1), &(nic->backend_domid))) {
-                        fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
-                        nic->backend_domid = 0;
-                    }
-                    if (nic->backend_domid != 0 && run_hotplug_scripts) {
+                    nic->backend_domain.name = strdup(p2 + 1);
+                    if (run_hotplug_scripts) {
                         fprintf(stderr, "ERROR: the vif 'backend=' option "
                                 "cannot be used in conjunction with "
                                 "run_hotplug_scripts, please set "
@@ -2431,8 +2423,6 @@ static void cd_insert(uint32_t domid, const char *virtdev, char *phys)
 
     parse_disk_config(&config, buf, &disk);
 
-    disk.backend_domid = 0;
-
     libxl_cdrom_insert(ctx, domid, &disk, NULL);
 
     libxl_device_disk_dispose(&disk);
@@ -5516,11 +5506,7 @@ int main_networkattach(int argc, char **argv)
         } else if (MATCH_OPTION("script", *argv, oparg)) {
             replace_string(&nic.script, oparg);
         } else if (MATCH_OPTION("backend", *argv, oparg)) {
-            if(libxl_name_to_domid(ctx, oparg, &val)) {
-                fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
-                val = 0;
-            }
-            nic.backend_domid = val;
+            replace_string(&nic.backend_domain.name, oparg);
         } else if (MATCH_OPTION("vifname", *argv, oparg)) {
             replace_string(&nic.ifname, oparg);
         } else if (MATCH_OPTION("model", *argv, oparg)) {
@@ -5623,8 +5609,8 @@ int main_networkdetach(int argc, char **argv)
 int main_blockattach(int argc, char **argv)
 {
     int opt;
-    uint32_t fe_domid, be_domid = 0;
-    libxl_device_disk disk = { 0 };
+    uint32_t fe_domid;
+    libxl_device_disk disk;
     XLU_Config *config = 0;
 
     if ((opt = def_getopt(argc, argv, "", "block-attach", 2)) != -1)
@@ -5639,8 +5625,6 @@ int main_blockattach(int argc, char **argv)
     parse_disk_config_multistring
         (&config, argc-optind, (const char* const*)argv + optind, &disk);
 
-    disk.backend_domid = be_domid;
-
     if (dryrun_only) {
         char *json = libxl_device_disk_to_json(ctx, &disk);
         printf("disk: %s\n", json);
@@ -5720,7 +5704,6 @@ int main_vtpmattach(int argc, char **argv)
     int opt;
     libxl_device_vtpm vtpm;
     char *oparg;
-    unsigned int val;
     uint32_t domid;
 
     if ((opt = def_getopt(argc, argv, "", "vtpm-attach", 1)) != -1)
@@ -5740,12 +5723,7 @@ int main_vtpmattach(int argc, char **argv)
                 return 1;
             }
         } else if (MATCH_OPTION("backend", *argv, oparg)) {
-            if(domain_qualifier_to_domid(oparg, &val, 0)) {
-                fprintf(stderr,
-                      "Specified backend domain does not exist, defaulting to Dom0\n");
-                val = 0;
-            }
-            vtpm.backend_domid = val;
+            replace_string(&vtpm.backend_domain.name, oparg);
         } else {
             fprintf(stderr, "unrecognized argument `%s'\n", *argv);
             return 1;
diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
index a16a025..6cb8d40 100644
--- a/tools/libxl/xl_sxp.c
+++ b/tools/libxl/xl_sxp.c
@@ -163,7 +163,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
     for (i = 0; i < d_config->num_disks; i++) {
         printf("\t(device\n");
         printf("\t\t(tap\n");
-        printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domid);
+        printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domain.domid);
         printf("\t\t\t(frontend_domid %d)\n", domid);
         printf("\t\t\t(physpath %s)\n", d_config->disks[i].pdev_path);
         printf("\t\t\t(phystype %d)\n", d_config->disks[i].backend);
@@ -180,7 +180,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
         printf("\t\t(vif\n");
         if (d_config->nics[i].ifname)
             printf("\t\t\t(vifname %s)\n", d_config->nics[i].ifname);
-        printf("\t\t\t(backend_domid %d)\n", d_config->nics[i].backend_domid);
+        printf("\t\t\t(backend_domid %d)\n", d_config->nics[i].backend_domain.domid);
         printf("\t\t\t(frontend_domid %d)\n", domid);
         printf("\t\t\t(devid %d)\n", d_config->nics[i].devid);
         printf("\t\t\t(mtu %d)\n", d_config->nics[i].mtu);
@@ -210,7 +210,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
     for (i = 0; i < d_config->num_vfbs; i++) {
         printf("\t(device\n");
         printf("\t\t(vfb\n");
-        printf("\t\t\t(backend_domid %d)\n", d_config->vfbs[i].backend_domid);
+        printf("\t\t\t(backend_domid %d)\n", d_config->vfbs[i].backend_domain.domid);
         printf("\t\t\t(frontend_domid %d)\n", domid);
         printf("\t\t\t(devid %d)\n", d_config->vfbs[i].devid);
         printf("\t\t\t(vnc %s)\n",
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC/v2] libxl: postpone backend name resolution
  2012-12-13 21:47 [PATCH RFC/v2] libxl: postpone backend name resolution Daniel De Graaf
@ 2012-12-17  9:57 ` Ian Campbell
  2012-12-17 15:08   ` Daniel De Graaf
  2012-12-17 15:10   ` [PATCH v3] " Daniel De Graaf
  0 siblings, 2 replies; 4+ messages in thread
From: Ian Campbell @ 2012-12-17  9:57 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: Stefano Stabellini, Ian Jackson, xen-devel

On Thu, 2012-12-13 at 21:47 +0000, Daniel De Graaf wrote:
> This replaces the backend_domid field in libxl devices with a structure
> allowing either a domid or a domain name to be specified.  The domain
> name is resolved into a domain ID in the _setdefault function when
> adding the device.  This change allows the backend of the block devices
> to be specified (which previously required passing the libxl_ctx down
> into the block device parser).

I didn't review this in detail yet but my first thought was that this is
a libxl API change, and I can't see any provision for backwards
compatibility.

If you are doing something clever which I've missed then it deserves a
comment ;-)

Assuming not then the simplest solution would be to remove the struct
and just add the name as a field to each affected device. Or maybe an
anonymous struct would do the job if the members were backend_domid and
backend_name?

You could also potentially do something with the provisions around
LIBXL_API_VERSION documented in libxl.h.

Ian.

> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> 
> ---
> 
> This patch does not include the changes to tools/libxl/libxlu_disk_l.c
> and tools/libxl/libxlu_disk_l.h because the diffs contain unrelated
> changes due to different generator versions.
> 
>  docs/misc/xl-disk-configuration.txt | 12 +++++++
>  tools/libxl/libxl.c                 | 65 +++++++++++++++++++++++++------------
>  tools/libxl/libxl_dm.c              |  6 ++--
>  tools/libxl/libxl_types.idl         | 15 ++++++---
>  tools/libxl/libxl_utils.c           |  2 +-
>  tools/libxl/libxlu_disk_l.l         |  1 +
>  tools/libxl/xl_cmdimpl.c            | 36 ++++----------------
>  tools/libxl/xl_sxp.c                |  6 ++--
>  8 files changed, 82 insertions(+), 61 deletions(-)
> 
> diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
> index 86c16be..5bd456d 100644
> --- a/docs/misc/xl-disk-configuration.txt
> +++ b/docs/misc/xl-disk-configuration.txt
> @@ -139,6 +139,18 @@ cdrom
>  Convenience alias for "devtype=cdrom".
> 
> 
> +backend=<domain-name>
> +---------------------
> +
> +Description:           Designates a backend domain for the device
> +Supported values:      Valid domain names
> +Mandatory:             No
> +
> +Specifies the backend domain which this device should attach to. This
> +defaults to domain 0. Specifying another domain requires setting up a
> +driver domain which is outside the scope of this document.
> +
> +
>  backendtype=<backend-type>
>  --------------------------
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 6c4455e..edb29b3 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1153,7 +1153,7 @@ static void disk_eject_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
>      sscanf(backend,
>              "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
>             "[a-z]/%*d/%*d",
> -           &disk->backend_domid, backend_type);
> +           &disk->backend_domain.domid, backend_type);
>      if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
>          disk->backend = LIBXL_DISK_BACKEND_TAP;
>      } else if (!strcmp(backend_type, "qdisk")) {
> @@ -1710,13 +1710,36 @@ out:
>      return;
>  }
> 
> +/* backend domain name-to-domid conversion utility */
> +static int libxl__resolve_domain(libxl__gc *gc, struct libxl_domain_desc *desc)
> +{
> +    int i, rv;
> +    const char *name = desc->name;
> +    if (!name)
> +        return 0;
> +    for (i=0; name[i]; i++) {
> +        if (!isdigit(name[i])) {
> +            rv = libxl_name_to_domid(libxl__gc_owner(gc), name, &desc->domid);
> +            if (rv)
> +                return rv;
> +            goto resolved;
> +        }
> +    }
> +    desc->domid = atoi(name);
> +
> + resolved:
> +    free(desc->name);
> +    desc->name = NULL;
> +    return 0;
> +}
> +
>  /******************************************************************************/
>  int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
>  {
>     if(libxl_uuid_is_nil(&vtpm->uuid)) {
>        libxl_uuid_generate(&vtpm->uuid);
>     }
> -   return 0;
> +   return libxl__resolve_domain(gc, &vtpm->backend_domain);
>  }
> 
>  static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
> @@ -1724,7 +1747,7 @@ static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
>                                     libxl__device *device)
>  {
>     device->backend_devid   = vtpm->devid;
> -   device->backend_domid   = vtpm->backend_domid;
> +   device->backend_domid   = vtpm->backend_domain.domid;
>     device->backend_kind    = LIBXL__DEVICE_KIND_VTPM;
>     device->devid           = vtpm->devid;
>     device->domid           = domid;
> @@ -1783,7 +1806,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
>      flexarray_append(back, "False");
> 
>      flexarray_append(front, "backend-id");
> -    flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domid));
> +    flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domain.domid));
>      flexarray_append(front, "state");
>      flexarray_append(front, GCSPRINTF("%d", 1));
>      flexarray_append(front, "handle");
> @@ -1832,7 +1855,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *n
>            tmp = libxl__xs_read(gc, XBT_NULL,
>                  GCSPRINTF("%s/%s/backend_id",
>                     fe_path, *dir));
> -          vtpm->backend_domid = atoi(tmp);
> +          vtpm->backend_domain.domid = atoi(tmp);
> 
>            tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", be_path));
>            if(tmp) {
> @@ -1934,7 +1957,7 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
>      rc = 1;
>      for (i = 0; i < nb; ++i) {
>          if(devid == vtpms[i].devid) {
> -            vtpm->backend_domid = vtpms[i].backend_domid;
> +            vtpm->backend_domain.domid = vtpms[i].backend_domain.domid;
>              vtpm->devid = vtpms[i].devid;
>              libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
>              rc = 0;
> @@ -1956,7 +1979,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
>      rc = libxl__device_disk_set_backend(gc, disk);
>      if (rc) return rc;
> 
> -    return rc;
> +    return libxl__resolve_domain(gc, &disk->backend_domain);
>  }
> 
>  int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
> @@ -1973,7 +1996,7 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
>          return ERROR_INVAL;
>      }
> 
> -    device->backend_domid = disk->backend_domid;
> +    device->backend_domid = disk->backend_domain.domid;
>      device->backend_devid = devid;
> 
>      switch (disk->backend) {
> @@ -2133,7 +2156,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
>          flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> 
>          flexarray_append(front, "backend-id");
> -        flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> +        flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domain.domid));
>          flexarray_append(front, "state");
>          flexarray_append(front, libxl__sprintf(gc, "%d", 1));
>          flexarray_append(front, "virtual-device");
> @@ -2298,7 +2321,7 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc,
>              p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
>              if ((rc=libxl__device_disk_from_xs_be(gc, p, pdisk)))
>                  goto out;
> -            pdisk->backend_domid = 0;
> +            pdisk->backend_domain.domid = 0;
>              *ndisks += 1;
>          }
>      }
> @@ -2759,7 +2782,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
>          LOG(ERROR, "unable to get current hotplug scripts execution setting");
>          return run_hotplug_scripts;
>      }
> -    if (nic->backend_domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) {
> +    if (nic->backend_domain.domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) {
>          LOG(ERROR, "cannot use a backend domain different than %d if"
>                     "hotplug scripts are executed from libxl",
>                     LIBXL_TOOLSTACK_DOMID);
> @@ -2784,7 +2807,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
>          abort();
>      }
> 
> -    return 0;
> +    return libxl__resolve_domain(gc, &nic->backend_domain);
>  }
> 
>  static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
> @@ -2792,7 +2815,7 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
>                                    libxl__device *device)
>  {
>      device->backend_devid    = nic->devid;
> -    device->backend_domid    = nic->backend_domid;
> +    device->backend_domid    = nic->backend_domain.domid;
>      device->backend_kind     = LIBXL__DEVICE_KIND_VIF;
>      device->devid            = nic->devid;
>      device->domid            = domid;
> @@ -2874,7 +2897,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
>                                       libxl_nic_type_to_string(nic->nictype)));
> 
>      flexarray_append(front, "backend-id");
> -    flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domid));
> +    flexarray_append(front, libxl__sprintf(gc, "%d", nic->backend_domain.domid));
>      flexarray_append(front, "state");
>      flexarray_append(front, libxl__sprintf(gc, "%d", 1));
>      flexarray_append(front, "handle");
> @@ -2991,7 +3014,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
>              const char *p;
>              p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
>              libxl__device_nic_from_xs_be(gc, p, pnic);
> -            pnic->backend_domid = 0;
> +            pnic->backend_domain.domid = 0;
>          }
>      }
>      return 0;
> @@ -3144,7 +3167,7 @@ out:
> 
>  int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
>  {
> -    return 0;
> +    return libxl__resolve_domain(gc, &vkb->backend_domain);
>  }
> 
>  static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
> @@ -3152,7 +3175,7 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
>                                    libxl__device *device)
>  {
>      device->backend_devid = vkb->devid;
> -    device->backend_domid = vkb->backend_domid;
> +    device->backend_domid = vkb->backend_domain.domid;
>      device->backend_kind = LIBXL__DEVICE_KIND_VKBD;
>      device->devid = vkb->devid;
>      device->domid = domid;
> @@ -3205,7 +3228,7 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
>      flexarray_append(back, libxl__domid_to_name(gc, domid));
> 
>      flexarray_append(front, "backend-id");
> -    flexarray_append(front, libxl__sprintf(gc, "%d", vkb->backend_domid));
> +    flexarray_append(front, libxl__sprintf(gc, "%d", vkb->backend_domain.domid));
>      flexarray_append(front, "state");
>      flexarray_append(front, libxl__sprintf(gc, "%d", 1));
> 
> @@ -3236,7 +3259,7 @@ int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
>      libxl_defbool_setdefault(&vfb->sdl.enable, false);
>      libxl_defbool_setdefault(&vfb->sdl.opengl, false);
> 
> -    return 0;
> +    return libxl__resolve_domain(gc, &vfb->backend_domain);
>  }
> 
>  static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
> @@ -3244,7 +3267,7 @@ static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
>                                    libxl__device *device)
>  {
>      device->backend_devid = vfb->devid;
> -    device->backend_domid = vfb->backend_domid;
> +    device->backend_domid = vfb->backend_domain.domid;
>      device->backend_kind = LIBXL__DEVICE_KIND_VFB;
>      device->devid = vfb->devid;
>      device->domid = domid;
> @@ -3309,7 +3332,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
>      }
> 
>      flexarray_append_pair(front, "backend-id",
> -                          libxl__sprintf(gc, "%d", vfb->backend_domid));
> +                          libxl__sprintf(gc, "%d", vfb->backend_domain.domid));
>      flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
> 
>      libxl__device_generic_add(gc, XBT_NULL, &device,
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index c036dc1..82a55ea 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -644,13 +644,15 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
>      libxl_device_vfb_init(vfb);
>      libxl_device_vkb_init(vkb);
> 
> -    vfb->backend_domid = 0;
> +    vfb->backend_domain.domid = 0;
> +    vfb->backend_domain.name = NULL;
>      vfb->devid = 0;
>      vfb->vnc = b_info->u.hvm.vnc;
>      vfb->keymap = b_info->u.hvm.keymap;
>      vfb->sdl = b_info->u.hvm.sdl;
> 
> -    vkb->backend_domid = 0;
> +    vkb->backend_domain.domid = 0;
> +    vkb->backend_domain.name = NULL;
>      vkb->devid = 0;
>      return 0;
>  }
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 93524f0..6ec7967 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -136,6 +136,11 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
>  # Complex libxl types
>  #
> 
> +libxl_domain_desc = Struct("domain_desc", [
> +    ("domid", libxl_domid),
> +    ("name",  string),
> +    ])
> +
>  libxl_ioport_range = Struct("ioport_range", [
>      ("first", uint32),
>      ("number", uint32),
> @@ -344,7 +349,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>  )
> 
>  libxl_device_vfb = Struct("device_vfb", [
> -    ("backend_domid", libxl_domid),
> +    ("backend_domain",libxl_domain_desc),
>      ("devid",         libxl_devid),
>      ("vnc",           libxl_vnc_info),
>      ("sdl",           libxl_sdl_info),
> @@ -353,12 +358,12 @@ libxl_device_vfb = Struct("device_vfb", [
>      ])
> 
>  libxl_device_vkb = Struct("device_vkb", [
> -    ("backend_domid", libxl_domid),
> +    ("backend_domain", libxl_domain_desc),
>      ("devid", libxl_devid),
>      ])
> 
>  libxl_device_disk = Struct("device_disk", [
> -    ("backend_domid", libxl_domid),
> +    ("backend_domain", libxl_domain_desc),
>      ("pdev_path", string),
>      ("vdev", string),
>      ("backend", libxl_disk_backend),
> @@ -370,7 +375,7 @@ libxl_device_disk = Struct("device_disk", [
>      ])
> 
>  libxl_device_nic = Struct("device_nic", [
> -    ("backend_domid", libxl_domid),
> +    ("backend_domain", libxl_domain_desc),
>      ("devid", libxl_devid),
>      ("mtu", integer),
>      ("model", string),
> @@ -397,7 +402,7 @@ libxl_device_pci = Struct("device_pci", [
>      ])
> 
>  libxl_device_vtpm = Struct("device_vtpm", [
> -    ("backend_domid",    libxl_domid),
> +    ("backend_domain",   libxl_domain_desc),
>      ("devid",            libxl_devid),
>      ("uuid",             libxl_uuid),
>  ])
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 8f78790..86a310e 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -478,7 +478,7 @@ int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
>      rc = 1;
>      for (i = 0; i < nb; ++i) {
>          if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
> -            vtpm->backend_domid = vtpms[i].backend_domid;
> +            vtpm->backend_domain.domid = vtpms[i].backend_domain.domid;
>              vtpm->devid = vtpms[i].devid;
>              libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
>              rc = 0;
> diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
> index bee16a1..a65030f 100644
> --- a/tools/libxl/libxlu_disk_l.l
> +++ b/tools/libxl/libxlu_disk_l.l
> @@ -168,6 +168,7 @@ devtype=disk,?      { DPC->disk->is_cdrom = 0; }
>  devtype=[^,]*,?        { xlu__disk_err(DPC,yytext,"unknown value for type"); }
> 
>  access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); }
> +backend=[^,]*,? { STRIP(','); SAVESTRING("backend", backend_domain.name, FROMEQUALS); }
>  backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
> 
>  vdev=[^,]*,?   { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index e964bf1..bc22f3c 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1090,12 +1090,7 @@ static void parse_config_data(const char *config_source,
>                       break;
>                    *p2 = '\0';
>                    if (!strcmp(p, "backend")) {
> -                     if(domain_qualifier_to_domid(p2 + 1, &(vtpm->backend_domid), 0))
> -                     {
> -                        fprintf(stderr,
> -                              "Specified vtpm backend domain `%s' does not exist!\n", p2 + 1);
> -                        exit(1);
> -                     }
> +                     vtpm->backend_domain.name = strdup(p2 + 1);
>                       got_backend = true;
>                    } else if(!strcmp(p, "uuid")) {
>                       if( libxl_uuid_from_string(&vtpm->uuid, p2 + 1) ) {
> @@ -1190,11 +1185,8 @@ static void parse_config_data(const char *config_source,
>                      free(nic->ifname);
>                      nic->ifname = strdup(p2 + 1);
>                  } else if (!strcmp(p, "backend")) {
> -                    if(libxl_name_to_domid(ctx, (p2 + 1), &(nic->backend_domid))) {
> -                        fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
> -                        nic->backend_domid = 0;
> -                    }
> -                    if (nic->backend_domid != 0 && run_hotplug_scripts) {
> +                    nic->backend_domain.name = strdup(p2 + 1);
> +                    if (run_hotplug_scripts) {
>                          fprintf(stderr, "ERROR: the vif 'backend=' option "
>                                  "cannot be used in conjunction with "
>                                  "run_hotplug_scripts, please set "
> @@ -2431,8 +2423,6 @@ static void cd_insert(uint32_t domid, const char *virtdev, char *phys)
> 
>      parse_disk_config(&config, buf, &disk);
> 
> -    disk.backend_domid = 0;
> -
>      libxl_cdrom_insert(ctx, domid, &disk, NULL);
> 
>      libxl_device_disk_dispose(&disk);
> @@ -5516,11 +5506,7 @@ int main_networkattach(int argc, char **argv)
>          } else if (MATCH_OPTION("script", *argv, oparg)) {
>              replace_string(&nic.script, oparg);
>          } else if (MATCH_OPTION("backend", *argv, oparg)) {
> -            if(libxl_name_to_domid(ctx, oparg, &val)) {
> -                fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
> -                val = 0;
> -            }
> -            nic.backend_domid = val;
> +            replace_string(&nic.backend_domain.name, oparg);
>          } else if (MATCH_OPTION("vifname", *argv, oparg)) {
>              replace_string(&nic.ifname, oparg);
>          } else if (MATCH_OPTION("model", *argv, oparg)) {
> @@ -5623,8 +5609,8 @@ int main_networkdetach(int argc, char **argv)
>  int main_blockattach(int argc, char **argv)
>  {
>      int opt;
> -    uint32_t fe_domid, be_domid = 0;
> -    libxl_device_disk disk = { 0 };
> +    uint32_t fe_domid;
> +    libxl_device_disk disk;
>      XLU_Config *config = 0;
> 
>      if ((opt = def_getopt(argc, argv, "", "block-attach", 2)) != -1)
> @@ -5639,8 +5625,6 @@ int main_blockattach(int argc, char **argv)
>      parse_disk_config_multistring
>          (&config, argc-optind, (const char* const*)argv + optind, &disk);
> 
> -    disk.backend_domid = be_domid;
> -
>      if (dryrun_only) {
>          char *json = libxl_device_disk_to_json(ctx, &disk);
>          printf("disk: %s\n", json);
> @@ -5720,7 +5704,6 @@ int main_vtpmattach(int argc, char **argv)
>      int opt;
>      libxl_device_vtpm vtpm;
>      char *oparg;
> -    unsigned int val;
>      uint32_t domid;
> 
>      if ((opt = def_getopt(argc, argv, "", "vtpm-attach", 1)) != -1)
> @@ -5740,12 +5723,7 @@ int main_vtpmattach(int argc, char **argv)
>                  return 1;
>              }
>          } else if (MATCH_OPTION("backend", *argv, oparg)) {
> -            if(domain_qualifier_to_domid(oparg, &val, 0)) {
> -                fprintf(stderr,
> -                      "Specified backend domain does not exist, defaulting to Dom0\n");
> -                val = 0;
> -            }
> -            vtpm.backend_domid = val;
> +            replace_string(&vtpm.backend_domain.name, oparg);
>          } else {
>              fprintf(stderr, "unrecognized argument `%s'\n", *argv);
>              return 1;
> diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
> index a16a025..6cb8d40 100644
> --- a/tools/libxl/xl_sxp.c
> +++ b/tools/libxl/xl_sxp.c
> @@ -163,7 +163,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
>      for (i = 0; i < d_config->num_disks; i++) {
>          printf("\t(device\n");
>          printf("\t\t(tap\n");
> -        printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domid);
> +        printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domain.domid);
>          printf("\t\t\t(frontend_domid %d)\n", domid);
>          printf("\t\t\t(physpath %s)\n", d_config->disks[i].pdev_path);
>          printf("\t\t\t(phystype %d)\n", d_config->disks[i].backend);
> @@ -180,7 +180,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
>          printf("\t\t(vif\n");
>          if (d_config->nics[i].ifname)
>              printf("\t\t\t(vifname %s)\n", d_config->nics[i].ifname);
> -        printf("\t\t\t(backend_domid %d)\n", d_config->nics[i].backend_domid);
> +        printf("\t\t\t(backend_domid %d)\n", d_config->nics[i].backend_domain.domid);
>          printf("\t\t\t(frontend_domid %d)\n", domid);
>          printf("\t\t\t(devid %d)\n", d_config->nics[i].devid);
>          printf("\t\t\t(mtu %d)\n", d_config->nics[i].mtu);
> @@ -210,7 +210,7 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
>      for (i = 0; i < d_config->num_vfbs; i++) {
>          printf("\t(device\n");
>          printf("\t\t(vfb\n");
> -        printf("\t\t\t(backend_domid %d)\n", d_config->vfbs[i].backend_domid);
> +        printf("\t\t\t(backend_domid %d)\n", d_config->vfbs[i].backend_domain.domid);
>          printf("\t\t\t(frontend_domid %d)\n", domid);
>          printf("\t\t\t(devid %d)\n", d_config->vfbs[i].devid);
>          printf("\t\t\t(vnc %s)\n",
> --
> 1.7.11.7
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC/v2] libxl: postpone backend name resolution
  2012-12-17  9:57 ` Ian Campbell
@ 2012-12-17 15:08   ` Daniel De Graaf
  2012-12-17 15:10   ` [PATCH v3] " Daniel De Graaf
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel De Graaf @ 2012-12-17 15:08 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Stefano Stabellini, Ian Jackson, xen-devel

On 12/17/2012 04:57 AM, Ian Campbell wrote:
> On Thu, 2012-12-13 at 21:47 +0000, Daniel De Graaf wrote:
>> This replaces the backend_domid field in libxl devices with a structure
>> allowing either a domid or a domain name to be specified.  The domain
>> name is resolved into a domain ID in the _setdefault function when
>> adding the device.  This change allows the backend of the block devices
>> to be specified (which previously required passing the libxl_ctx down
>> into the block device parser).
> 
> I didn't review this in detail yet but my first thought was that this is
> a libxl API change, and I can't see any provision for backwards
> compatibility.

Nope, I forgot to address that this version. Will send v3 soon.

> If you are doing something clever which I've missed then it deserves a
> comment ;-)
> 
> Assuming not then the simplest solution would be to remove the struct
> and just add the name as a field to each affected device. Or maybe an
> anonymous struct would do the job if the members were backend_domid and
> backend_name?

I think just adding the field would work better; the anonymous struct seems
like it might be compiler-dependent.  It's not significantly more code in
the setdefault functions to avoid using the struct.

> You could also potentially do something with the provisions around
> LIBXL_API_VERSION documented in libxl.h.
> 
> Ian.
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3] libxl: postpone backend name resolution
  2012-12-17  9:57 ` Ian Campbell
  2012-12-17 15:08   ` Daniel De Graaf
@ 2012-12-17 15:10   ` Daniel De Graaf
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel De Graaf @ 2012-12-17 15:10 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, Ian Jackson, Ian Campbell, Stefano Stabellini

This adds a backend_name field in addition to the backend_domid field in
libxl devices, allowing either a domid or a domain name to be specified.
The domain name is resolved into a domain ID in the _setdefault function
when adding the device.  This change allows the backend of the block
devices to be specified (which previously required passing the libxl_ctx
down into the block device parser), and should simplify users of libxl
that wish to use backend domains.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>

---

This patch does not include the changes to tools/libxl/libxlu_disk_l.c
and tools/libxl/libxlu_disk_l.h because the diffs contain unrelated
changes due to different generator versions.
---
 docs/misc/xl-disk-configuration.txt | 12 +++++++++
 tools/libxl/libxl.c                 | 49 ++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_types.idl         |  5 ++++
 tools/libxl/libxlu_disk_l.l         |  1 +
 tools/libxl/xl_cmdimpl.c            | 36 ++++++---------------------
 5 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt
index 86c16be..5bd456d 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -139,6 +139,18 @@ cdrom
 Convenience alias for "devtype=cdrom".
 
 
+backend=<domain-name>
+---------------------
+
+Description:           Designates a backend domain for the device
+Supported values:      Valid domain names
+Mandatory:             No
+
+Specifies the backend domain which this device should attach to. This
+defaults to domain 0. Specifying another domain requires setting up a
+driver domain which is outside the scope of this document.
+
+
 backendtype=<backend-type>
 --------------------------
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6c4455e..a04b435 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1710,12 +1710,35 @@ out:
     return;
 }
 
+/* backend domain name-to-domid conversion utility */
+static int libxl__resolve_domain(libxl__gc *gc, const char *name)
+{
+    int i, rv;
+    uint32_t domid;
+    for (i=0; name[i]; i++) {
+        if (!isdigit(name[i])) {
+            rv = libxl_name_to_domid(libxl__gc_owner(gc), name, &domid);
+            if (rv)
+                return rv;
+            return domid;
+        }
+    }
+    return atoi(name);
+}
+
 /******************************************************************************/
 int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
 {
+   int rc;
    if(libxl_uuid_is_nil(&vtpm->uuid)) {
       libxl_uuid_generate(&vtpm->uuid);
    }
+   if (vtpm->backend_domname) {
+       rc = libxl__resolve_domain(gc, vtpm->backend_domname);
+       if (rc < 0)
+           return rc;
+       vtpm->backend_domid = rc;
+   }
    return 0;
 }
 
@@ -1956,7 +1979,13 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
     rc = libxl__device_disk_set_backend(gc, disk);
     if (rc) return rc;
 
-    return rc;
+    if (disk->backend_domname) {
+        rc = libxl__resolve_domain(gc, disk->backend_domname);
+        if (rc < 0)
+            return rc;
+        disk->backend_domid = rc;
+    }
+    return 0;
 }
 
 int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
@@ -2784,6 +2813,12 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
         abort();
     }
 
+    if (nic->backend_domname) {
+        int rc = libxl__resolve_domain(gc, nic->backend_domname);
+        if (rc < 0)
+            return rc;
+        nic->backend_domid = rc;
+    }
     return 0;
 }
 
@@ -3144,6 +3179,12 @@ out:
 
 int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
 {
+    if (vkb->backend_domname) {
+        int rc = libxl__resolve_domain(gc, vkb->backend_domname);
+        if (rc < 0)
+            return rc;
+        vkb->backend_domid = rc;
+    }
     return 0;
 }
 
@@ -3236,6 +3277,12 @@ int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
     libxl_defbool_setdefault(&vfb->sdl.enable, false);
     libxl_defbool_setdefault(&vfb->sdl.opengl, false);
 
+    if (vfb->backend_domname) {
+        int rc = libxl__resolve_domain(gc, vfb->backend_domname);
+        if (rc < 0)
+            return rc;
+        vfb->backend_domid = rc;
+    }
     return 0;
 }
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 93524f0..131332a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -345,6 +345,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
 libxl_device_vfb = Struct("device_vfb", [
     ("backend_domid", libxl_domid),
+    ("backend_domname",string),
     ("devid",         libxl_devid),
     ("vnc",           libxl_vnc_info),
     ("sdl",           libxl_sdl_info),
@@ -354,11 +355,13 @@ libxl_device_vfb = Struct("device_vfb", [
 
 libxl_device_vkb = Struct("device_vkb", [
     ("backend_domid", libxl_domid),
+    ("backend_domname", string),
     ("devid", libxl_devid),
     ])
 
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", libxl_domid),
+    ("backend_domname", string),
     ("pdev_path", string),
     ("vdev", string),
     ("backend", libxl_disk_backend),
@@ -371,6 +374,7 @@ libxl_device_disk = Struct("device_disk", [
 
 libxl_device_nic = Struct("device_nic", [
     ("backend_domid", libxl_domid),
+    ("backend_domname", string),
     ("devid", libxl_devid),
     ("mtu", integer),
     ("model", string),
@@ -398,6 +402,7 @@ libxl_device_pci = Struct("device_pci", [
 
 libxl_device_vtpm = Struct("device_vtpm", [
     ("backend_domid",    libxl_domid),
+    ("backend_domname",  string),
     ("devid",            libxl_devid),
     ("uuid",             libxl_uuid),
 ])
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index bee16a1..7c4e7f1 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -168,6 +168,7 @@ devtype=disk,?	{ DPC->disk->is_cdrom = 0; }
 devtype=[^,]*,?	{ xlu__disk_err(DPC,yytext,"unknown value for type"); }
 
 access=[^,]*,?	{ STRIP(','); setaccess(DPC, FROMEQUALS); }
+backend=[^,]*,? { STRIP(','); SAVESTRING("backend", backend_domname, FROMEQUALS); }
 backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e964bf1..103e344 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1090,12 +1090,7 @@ static void parse_config_data(const char *config_source,
                      break;
                   *p2 = '\0';
                   if (!strcmp(p, "backend")) {
-                     if(domain_qualifier_to_domid(p2 + 1, &(vtpm->backend_domid), 0))
-                     {
-                        fprintf(stderr,
-                              "Specified vtpm backend domain `%s' does not exist!\n", p2 + 1);
-                        exit(1);
-                     }
+                     vtpm->backend_domname = strdup(p2 + 1);
                      got_backend = true;
                   } else if(!strcmp(p, "uuid")) {
                      if( libxl_uuid_from_string(&vtpm->uuid, p2 + 1) ) {
@@ -1190,11 +1185,8 @@ static void parse_config_data(const char *config_source,
                     free(nic->ifname);
                     nic->ifname = strdup(p2 + 1);
                 } else if (!strcmp(p, "backend")) {
-                    if(libxl_name_to_domid(ctx, (p2 + 1), &(nic->backend_domid))) {
-                        fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
-                        nic->backend_domid = 0;
-                    }
-                    if (nic->backend_domid != 0 && run_hotplug_scripts) {
+                    nic->backend_domname = strdup(p2 + 1);
+                    if (run_hotplug_scripts) {
                         fprintf(stderr, "ERROR: the vif 'backend=' option "
                                 "cannot be used in conjunction with "
                                 "run_hotplug_scripts, please set "
@@ -2431,8 +2423,6 @@ static void cd_insert(uint32_t domid, const char *virtdev, char *phys)
 
     parse_disk_config(&config, buf, &disk);
 
-    disk.backend_domid = 0;
-
     libxl_cdrom_insert(ctx, domid, &disk, NULL);
 
     libxl_device_disk_dispose(&disk);
@@ -5516,11 +5506,7 @@ int main_networkattach(int argc, char **argv)
         } else if (MATCH_OPTION("script", *argv, oparg)) {
             replace_string(&nic.script, oparg);
         } else if (MATCH_OPTION("backend", *argv, oparg)) {
-            if(libxl_name_to_domid(ctx, oparg, &val)) {
-                fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n");
-                val = 0;
-            }
-            nic.backend_domid = val;
+            replace_string(&nic.backend_domname, oparg);
         } else if (MATCH_OPTION("vifname", *argv, oparg)) {
             replace_string(&nic.ifname, oparg);
         } else if (MATCH_OPTION("model", *argv, oparg)) {
@@ -5623,8 +5609,8 @@ int main_networkdetach(int argc, char **argv)
 int main_blockattach(int argc, char **argv)
 {
     int opt;
-    uint32_t fe_domid, be_domid = 0;
-    libxl_device_disk disk = { 0 };
+    uint32_t fe_domid;
+    libxl_device_disk disk;
     XLU_Config *config = 0;
 
     if ((opt = def_getopt(argc, argv, "", "block-attach", 2)) != -1)
@@ -5639,8 +5625,6 @@ int main_blockattach(int argc, char **argv)
     parse_disk_config_multistring
         (&config, argc-optind, (const char* const*)argv + optind, &disk);
 
-    disk.backend_domid = be_domid;
-
     if (dryrun_only) {
         char *json = libxl_device_disk_to_json(ctx, &disk);
         printf("disk: %s\n", json);
@@ -5720,7 +5704,6 @@ int main_vtpmattach(int argc, char **argv)
     int opt;
     libxl_device_vtpm vtpm;
     char *oparg;
-    unsigned int val;
     uint32_t domid;
 
     if ((opt = def_getopt(argc, argv, "", "vtpm-attach", 1)) != -1)
@@ -5740,12 +5723,7 @@ int main_vtpmattach(int argc, char **argv)
                 return 1;
             }
         } else if (MATCH_OPTION("backend", *argv, oparg)) {
-            if(domain_qualifier_to_domid(oparg, &val, 0)) {
-                fprintf(stderr,
-                      "Specified backend domain does not exist, defaulting to Dom0\n");
-                val = 0;
-            }
-            vtpm.backend_domid = val;
+            replace_string(&vtpm.backend_domname, oparg);
         } else {
             fprintf(stderr, "unrecognized argument `%s'\n", *argv);
             return 1;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-12-17 15:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-13 21:47 [PATCH RFC/v2] libxl: postpone backend name resolution Daniel De Graaf
2012-12-17  9:57 ` Ian Campbell
2012-12-17 15:08   ` Daniel De Graaf
2012-12-17 15:10   ` [PATCH v3] " Daniel De Graaf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.