Xen devel - Apr 2013 - [PATCH v4 1/2] libxl: postpone backend name resolution

This adds a backend_domname field in libxl devices that contain a
backend_domid field, allowing either a domid or a domain name to be
specified in the configuration structures.  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 will simplify specification of backend domains in
other users of libxl.

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.

Changes since v3:
 - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
 - Create libxl_domain_qualifier_to_domid function in libxl_util.h

 docs/misc/xl-disk-configuration.txt | 12 ++++++
 tools/libxl/libxl.c                 | 17 +++++++--
 tools/libxl/libxl.h                 | 12 ++++++
 tools/libxl/libxl_types.idl         |  5 +++
 tools/libxl/libxl_utils.c           | 19 ++++++++++
 tools/libxl/libxl_utils.h           |  1 +
 tools/libxl/libxlu_disk_l.l         |  1 +
 tools/libxl/xl_cmdimpl.c            | 75 ++++++++-----------------------------
 8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc, uint32_t
domid, char *device)
     return nextid;
 }
 
+static int libxl__resolve_domid(libxl__gc *gc, const char *name,
+                                uint32_t *domid)
+{
+    if (!name)
+        return 0;
+    return libxl_domain_qualifier_to_domid(libxl__gc_owner(gc), name, domid);
+}
+
 /******************************************************************************/
 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_domid(gc, vtpm->backend_domname,
&vtpm->backend_domid);
 }
 
 static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
@@ -1968,6 +1976,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;
 
+    rc = libxl__resolve_domid(gc, disk->backend_domname,
&disk->backend_domid);
     return rc;
 }
 
@@ -2799,7 +2808,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc,
libxl_device_nic *nic,
         abort();
     }
 
-    return 0;
+    return libxl__resolve_domid(gc, nic->backend_domname,
&nic->backend_domid);
 }
 
 static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
@@ -3156,7 +3165,7 @@ out:
 
 int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
 {
-    return 0;
+    return libxl__resolve_domid(gc, vkb->backend_domname,
&vkb->backend_domid);
 }
 
 static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
@@ -3255,7 +3264,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_domid(gc, vfb->backend_domname,
&vfb->backend_domid);
 }
 
 static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d18d22c..b77e3a4 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -289,6 +289,18 @@
  */
 #define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
 
+/*
+ * LIBXL_HAVE_DEVICE_BACKEND_DOMNAME
+ *
+ * If this is defined, libxl_device_* structures containing a backend_domid
+ * field also contain a backend_domname field.  If backend_domname is set, it
is
+ * resolved to a domain ID when the device is used and takes precedence over
the
+ * backend_domid field.
+ *
+ * If this is not defined, the backend_domname field does not exist.
+ */
+#define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6cb6de6..5edc9cc 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -349,6 +349,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),
@@ -358,11 +359,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),
@@ -375,6 +378,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),
@@ -403,6 +407,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/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8f78790..35da71c 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -90,6 +90,25 @@ int libxl_name_to_domid(libxl_ctx *ctx, const char *name,
     return ret;
 }
 
+int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name,
+                                    uint32_t *domid)
+{
+    int i, rv;
+    for (i=0; name[i]; i++) {
+        if (!isdigit(name[i])) {
+            goto nondigit_found;
+        }
+    }
+    *domid = strtoul(name, NULL, 10);
+    return 0;
+
+ nondigit_found:
+    /* this could also check for uuids */
+    rv = libxl_name_to_domid(ctx, name, domid);
+    return rv;
+}
+
+
 char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid)
 {
     unsigned int len;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 40f3f30..a430362 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -21,6 +21,7 @@
 const char *libxl_basename(const char *name); /* returns string from strdup */
 unsigned long libxl_get_required_shadow_memory(unsigned long maxmem_kb,
unsigned int smp_cpus);
 int libxl_name_to_domid(libxl_ctx *ctx, const char *name, uint32_t *domid);
+int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name, uint32_t
*domid);
 char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
 int libxl_name_to_cpupoolid(libxl_ctx *ctx, const char *name, uint32_t
*poolid);
 char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid);
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 61f7b96..d916e90 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -162,29 +162,6 @@ static int qualifier_to_id(const char *p, uint32_t *id_r)
     return 1;
 }
 
-static int domain_qualifier_to_domid(const char *p, uint32_t *domid_r,
-                                     int *was_name_r)
-{
-    int was_name, rc;
-
-    was_name = qualifier_to_id(p, domid_r);
-    if (was_name_r)
-        *was_name_r = was_name;
-
-    if (was_name) {
-        rc = libxl_name_to_domid(ctx, p, domid_r);
-        if (rc)
-            return rc;
-    } else {
-        rc = libxl_domain_info(ctx, NULL, *domid_r);
-        /* error only if domain does not exist */
-        if (rc == ERROR_INVAL)
-            return rc;
-    }
-
-    return 0;
-}
-
 static int cpupool_qualifier_to_cpupoolid(const char *p, uint32_t *poolid_r,
                                      int *was_name_r)
 {
@@ -199,14 +176,14 @@ static uint32_t find_domain(const char *p)
__attribute__((warn_unused_result));
 static uint32_t find_domain(const char *p)
 {
     uint32_t domid;
-    int rc, was_name;
+    int rc;
 
-    rc = domain_qualifier_to_domid(p, &domid, &was_name);
+    rc = libxl_domain_qualifier_to_domid(ctx, p, &domid);
     if (rc) {
         fprintf(stderr, "%s is an invalid domain identifier
(rc=%d)\n", p, rc);
         exit(2);
     }
-    common_domname = was_name ? p : libxl_domid_to_name(ctx, domid);
+    common_domname = libxl_domid_to_name(ctx, domid);
     return domid;
 }
 
@@ -1095,12 +1072,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) ) {
@@ -1200,11 +1172,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 "
@@ -2556,8 +2525,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);
@@ -5534,11 +5501,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)) {
@@ -5643,15 +5606,15 @@ 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;
 
     SWITCH_FOREACH_OPT(opt, "", NULL, "block-attach", 2) {
         /* No options */
     }
 
-    if (domain_qualifier_to_domid(argv[optind], &fe_domid, 0) < 0) {
+    if (libxl_domain_qualifier_to_domid(ctx, argv[optind], &fe_domid) <
0) {
         fprintf(stderr, "%s is an invalid domain identifier\n",
argv[optind]);
         return 1;
     }
@@ -5660,8 +5623,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);
@@ -5691,7 +5652,7 @@ int main_blocklist(int argc, char **argv)
            "Vdev", "BE", "handle",
"state", "evt-ch", "ring-ref",
"BE-path");
     for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
         uint32_t domid;
-        if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
+        if (libxl_domain_qualifier_to_domid(ctx, *argv, &domid) < 0) {
             fprintf(stderr, "%s is an invalid domain identifier\n",
*argv);
             continue;
         }
@@ -5743,14 +5704,13 @@ int main_vtpmattach(int argc, char **argv)
     int opt;
     libxl_device_vtpm vtpm;
     char *oparg;
-    unsigned int val;
     uint32_t domid;
 
     SWITCH_FOREACH_OPT(opt, "", NULL, "vtpm-attach", 1) {
         /* No options */
     }
 
-    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+    if (libxl_domain_qualifier_to_domid(ctx, argv[optind], &domid) < 0)
{
         fprintf(stderr, "%s is an invalid domain identifier\n",
argv[optind]);
         return 1;
     }
@@ -5764,12 +5724,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;
@@ -5809,7 +5764,7 @@ int main_vtpmlist(int argc, char **argv)
            "Idx", "BE", "Uuid",
"handle", "state", "evt-ch", "ring-ref",
"BE-path");
     for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
         uint32_t domid;
-        if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
+        if (libxl_domain_qualifier_to_domid(ctx, *argv, &domid) < 0) {
             fprintf(stderr, "%s is an invalid domain identifier\n",
*argv);
             continue;
         }
@@ -6691,7 +6646,7 @@ int main_cpupoolmigrate(int argc, char **argv)
     dom = argv[optind++];
     pool = argv[optind];
 
-    if (domain_qualifier_to_domid(dom, &domid, NULL) ||
+    if (libxl_domain_qualifier_to_domid(ctx, dom, &domid) ||
         !libxl_domid_to_name(ctx, domid)) {
         fprintf(stderr, "unknown domain \''%s\''\n",
dom);
         return -ERROR_FAIL;
-- 
1.8.1.4

This avoids returning unallocated memory in the libxl_device_vtpm
structure in libxl_device_vtpm_list, and uses libxl_device_nic_init
instead of memset when initializing libxl_device_nics.

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>
---

Changes from v3:
 - Also fix some memset => libxl_device_*_init calls

 tools/libxl/libxl.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b994fea..f0d7d2f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1847,6 +1847,8 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx,
uint32_t domid, int *n
                 GCSPRINTF("%s/%s/backend",
                    fe_path, *dir));
 
+          libxl_device_vtpm_init(vtpm);
+
           vtpm->devid = atoi(*dir);
 
           tmp = libxl__xs_read(gc, XBT_NULL,
@@ -1950,7 +1952,7 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
     if (!vtpms)
         return ERROR_FAIL;
 
-    memset(vtpm, 0, sizeof (libxl_device_vtpm));
+    libxl_device_vtpm_init(vtpm);
     rc = 1;
     for (i = 0; i < nb; ++i) {
         if(devid == vtpms[i].devid) {
@@ -2927,7 +2929,7 @@ static void libxl__device_nic_from_xs_be(libxl__gc *gc,
     char *tmp;
     int rc;
 
-    memset(nic, 0, sizeof(*nic));
+    libxl_device_nic_init(nic);
 
     tmp = xs_read(ctx->xsh, XBT_NULL,
                   libxl__sprintf(gc, "%s/handle", be_path),
&len);
@@ -2966,7 +2968,7 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t
domid,
     char *dompath, *path;
     int rc = ERROR_FAIL;
 
-    memset(nic, 0, sizeof (libxl_device_nic));
+    libxl_device_nic_init(nic);
     dompath = libxl__xs_get_dompath(gc, domid);
     if (!dompath)
         goto out;
-- 
1.8.1.4

On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf
wrote:
> This adds a backend_domname field in libxl devices that contain a
> backend_domid field, allowing either a domid or a domain name to be
> specified in the configuration structures.  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 will simplify specification of backend domains in
> other users of libxl.
Looks good to me (some minor comments below). Given that the initial
version of this was posted ages ago I think this should be granted a
freeze exception. George?
> 
> 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.
> 
> Changes since v3:
>  - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
>  - Create libxl_domain_qualifier_to_domid function in libxl_util.h
> 
>  docs/misc/xl-disk-configuration.txt | 12 ++++++
>  tools/libxl/libxl.c                 | 17 +++++++--
>  tools/libxl/libxl.h                 | 12 ++++++
>  tools/libxl/libxl_types.idl         |  5 +++
>  tools/libxl/libxl_utils.c           | 19 ++++++++++
>  tools/libxl/libxl_utils.h           |  1 +
>  tools/libxl/libxlu_disk_l.l         |  1 +
>  tools/libxl/xl_cmdimpl.c            | 75
++++++++-----------------------------
>  8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc,
uint32_t domid, char *device)
>      return nextid;
>  }
> 
> +static int libxl__resolve_domid(libxl__gc *gc, const char *name,
> +                                uint32_t *domid)
> +{
> +    if (!name)
> +        return 0;
> +    return libxl_domain_qualifier_to_domid(libxl__gc_owner(gc), name,
domid);
You can use CTX for libxl__gc_owner(gc).
> +}
> +
> 
/******************************************************************************/
>  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_domid(gc, vtpm->backend_domname,
&vtpm->backend_domid);
I''m not terribly keen on this pattern, it makes it seem (at least to
me)
like this final call is somehow special and has to be a tail call,
rather than just happening to be the last item. That might be just me
though.

Anyway, both of those a just picking nits. However this...
> @@ -1200,11 +1172,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
"
... changes when this error (with the exit() which follows outside the
context presented here) happens. Before it was only if the specified
domain was non-zero but now it is if the backend is given at all.

That might be OK, if you want to use dom0 as the backend, don''t use the
option. But maybe we should just nuke the warning if something else will
also check later on once the domid is known in libxl so that we don''t
get strange behaviour? I think the check in
libxl__device_nic_setdefault is sufficient?

Are there any docs which need updating for this subtle change? I suppose
it ought to at least say "don''t specify backend=0 or
backend=Domain-0"?

Ian.

On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf
wrote:
> This avoids returning unallocated memory in the libxl_device_vtpm
> structure in libxl_device_vtpm_list, and uses libxl_device_nic_init
> instead of memset when initializing libxl_device_nics.
> 
> 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>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

On 04/15/2013 08:11 AM, Ian Campbell wrote:
> On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf wrote:
>> This adds a backend_domname field in libxl devices that contain a
>> backend_domid field, allowing either a domid or a domain name to be
>> specified in the configuration structures.  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 will simplify specification of backend domains in
>> other users of libxl.
>
> Looks good to me (some minor comments below). Given that the initial
> version of this was posted ages ago I think this should be granted a
> freeze exception. George?
If it helps, this was discussed prior to 4.2''s release as a feature
that
would be considered for backporting to 4.2.x. I don''t think
it''s suitable
for that due to the fact it changes the IDL - but it is functionality
whose absence blocks the use of disk driver 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.
>>
>> Changes since v3:
>>   - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
>>   - Create libxl_domain_qualifier_to_domid function in libxl_util.h
>>
>>   docs/misc/xl-disk-configuration.txt | 12 ++++++
>>   tools/libxl/libxl.c                 | 17 +++++++--
>>   tools/libxl/libxl.h                 | 12 ++++++
>>   tools/libxl/libxl_types.idl         |  5 +++
>>   tools/libxl/libxl_utils.c           | 19 ++++++++++
>>   tools/libxl/libxl_utils.h           |  1 +
>>   tools/libxl/libxlu_disk_l.l         |  1 +
>>   tools/libxl/xl_cmdimpl.c            | 75
++++++++-----------------------------
>>   8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc,
uint32_t domid, char *device)
>>       return nextid;
>>   }
>>
>> +static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>> +                                uint32_t *domid)
>> +{
>> +    if (!name)
>> +        return 0;
>> +    return libxl_domain_qualifier_to_domid(libxl__gc_owner(gc), name,
domid);
>
> You can use CTX for libxl__gc_owner(gc).
>
>> +}
>> +
>>  
/******************************************************************************/
>>   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_domid(gc, vtpm->backend_domname,
&vtpm->backend_domid);
>
> I''m not terribly keen on this pattern, it makes it seem (at least
to me)
> like this final call is somehow special and has to be a tail call,
> rather than just happening to be the last item. That might be just me
> though.
I also think it looks better assigning to rc and returning.
> Anyway, both of those a just picking nits. However this...
>
>> @@ -1200,11 +1172,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
"
>
> ... changes when this error (with the exit() which follows outside the
> context presented here) happens. Before it was only if the specified
> domain was non-zero but now it is if the backend is given at all.
>
> That might be OK, if you want to use dom0 as the backend, don''t
use the
> option. But maybe we should just nuke the warning if something else will
> also check later on once the domid is known in libxl so that we
don''t
> get strange behaviour? I think the check in
> libxl__device_nic_setdefault is sufficient?
Right, we do check later which should suffice - even though it will take a
bit longer to get to the error, it''s a configuration problem that
won''t
start happening by surprise so that shouldn''t be an issue. Removing
this
check will also make changes to LIBXL_TOOLSTACK_DOMID do the right thing.
> Are there any docs which need updating for this subtle change? I suppose
> it ought to at least say "don''t specify backend=0 or
backend=Domain-0"?
>
> Ian.
>
Removing the earlier check will resolve this.

-- 
Daniel De Graaf
National Security Agency

On 12/04/13 17:22, Daniel De Graaf wrote:
> This adds a backend_domname field in libxl devices that contain a
> backend_domid field, allowing either a domid or a domain name to be
> specified in the configuration structures.  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 will simplify specification of backend domains in
> other users of libxl.
> 
> 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.
> 
> Changes since v3:
>  - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
>  - Create libxl_domain_qualifier_to_domid function in libxl_util.h
> 
>  docs/misc/xl-disk-configuration.txt | 12 ++++++
>  tools/libxl/libxl.c                 | 17 +++++++--
>  tools/libxl/libxl.h                 | 12 ++++++
>  tools/libxl/libxl_types.idl         |  5 +++
>  tools/libxl/libxl_utils.c           | 19 ++++++++++
>  tools/libxl/libxl_utils.h           |  1 +
>  tools/libxl/libxlu_disk_l.l         |  1 +
>  tools/libxl/xl_cmdimpl.c            | 75
++++++++-----------------------------
>  8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc,
uint32_t domid, char *device)
>      return nextid;
>  }
> 
> +static int libxl__resolve_domid(libxl__gc *gc, const char *name,
> +                                uint32_t *domid)
> +{
> +    if (!name)
> +        return 0;
This is just my opinion, but I find this function ambiguous, it will
return 0 (success), if either name is NULL, or name != NULL and it can
be resolved to a domid, which means that even when returning 0 the value
in *domid might be uninitialized (if name == NULL). I would rather do
something like

if (!name || !domid)
	return ERROR_INVAL;

And make sure callers only call this function if there''s a real need to
resolve a domid.

Also, do we check somewhere that both backend_domid and backend_domname
are not specified at the same time?

On Mon, 2013-04-15 at 15:55 +0100, Roger Pau Monne
wrote:
> On 12/04/13 17:22, Daniel De Graaf wrote:
> > This adds a backend_domname field in libxl devices that contain a
> > backend_domid field, allowing either a domid or a domain name to be
> > specified in the configuration structures.  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 will simplify specification of backend domains in
> > other users of libxl.
> > 
> > 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.
> > 
> > Changes since v3:
> >  - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
> >  - Create libxl_domain_qualifier_to_domid function in libxl_util.h
> > 
> >  docs/misc/xl-disk-configuration.txt | 12 ++++++
> >  tools/libxl/libxl.c                 | 17 +++++++--
> >  tools/libxl/libxl.h                 | 12 ++++++
> >  tools/libxl/libxl_types.idl         |  5 +++
> >  tools/libxl/libxl_utils.c           | 19 ++++++++++
> >  tools/libxl/libxl_utils.h           |  1 +
> >  tools/libxl/libxlu_disk_l.l         |  1 +
> >  tools/libxl/xl_cmdimpl.c            | 75
++++++++-----------------------------
> >  8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc,
uint32_t domid, char *device)
> >      return nextid;
> >  }
> > 
> > +static int libxl__resolve_domid(libxl__gc *gc, const char *name,
> > +                                uint32_t *domid)
> > +{
> > +    if (!name)
> > +        return 0;
> 
> This is just my opinion, but I find this function ambiguous, it will
> return 0 (success), if either name is NULL, or name != NULL and it can
> be resolved to a domid, which means that even when returning 0 the value
> in *domid might be uninitialized (if name == NULL). I would rather do
> something like
> 
> if (!name || !domid)
> 	return ERROR_INVAL;
> 
> And make sure callers only call this function if there''s a real
need to
> resolve a domid.
Did you mean !name && !domid? i.e. fail if no name is given AND domid
isn''t already set? But domid == 0 is a valid domid...

The nice property which the existing function has is you can initialise
domid as you wish (including to 0) and then blindly call this on the
given name (including NULL) and it will correctly override the domid if
a name was given.
> Also, do we check somewhere that both backend_domid and backend_domname
> are not specified at the same time?
I think saw words somewhere in the patch which documented explicitly
that domname takes precedence. Which isn''t quite what you asked, but is
sufficient IMHO.

Ian.

On 04/15/2013 10:55 AM, Roger Pau Monné wrote:
> On 12/04/13 17:22, Daniel De Graaf wrote:
>> This adds a backend_domname field in libxl devices that contain a
>> backend_domid field, allowing either a domid or a domain name to be
>> specified in the configuration structures.  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 will simplify specification of backend domains in
>> other users of libxl.
>>
>> 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.
>>
>> Changes since v3:
>>   - Add #define LIBXL_HAVE_DEVICE_BACKEND_DOMNAME in libxl.h
>>   - Create libxl_domain_qualifier_to_domid function in libxl_util.h
>>
>>   docs/misc/xl-disk-configuration.txt | 12 ++++++
>>   tools/libxl/libxl.c                 | 17 +++++++--
>>   tools/libxl/libxl.h                 | 12 ++++++
>>   tools/libxl/libxl_types.idl         |  5 +++
>>   tools/libxl/libxl_utils.c           | 19 ++++++++++
>>   tools/libxl/libxl_utils.h           |  1 +
>>   tools/libxl/libxlu_disk_l.l         |  1 +
>>   tools/libxl/xl_cmdimpl.c            | 75
++++++++-----------------------------
>>   8 files changed, 78 insertions(+), 64 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 572c2c6..b994fea 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1730,13 +1730,21 @@ static int libxl__device_nextid(libxl__gc *gc,
uint32_t domid, char *device)
>>       return nextid;
>>   }
>>
>> +static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>> +                                uint32_t *domid)
>> +{
>> +    if (!name)
>> +        return 0;
>
> This is just my opinion, but I find this function ambiguous, it will
> return 0 (success), if either name is NULL, or name != NULL and it can
> be resolved to a domid, which means that even when returning 0 the value
> in *domid might be uninitialized (if name == NULL). I would rather do
> something like
>
> if (!name || !domid)
> 	return ERROR_INVAL;
>
> And make sure callers only call this function if there''s a real
need to
> resolve a domid.
This is an internal function which is called only when there is a real need
to resolve a domid. The value of domid is never uninitialized when calling
this function; it is always the value of backend_domid, and retains that
value when backend_domname is not specified.
> Also, do we check somewhere that both backend_domid and backend_domname
> are not specified at the same time?
Since backend_domid is an integer that defaults to 0 (which is a valid ID),
it''s impossible to tell if the 0 was specified or if it was simply the
default. This is resolved by having backend_domname take precedence over
backend_domid when it is present.

-- 
Daniel De Graaf
National Security Agency

On Mon, 2013-04-15 at 15:59 +0100, Ian Campbell wrote:
> > And make sure callers only call this function if there''s a
real need to
> > resolve a domid.
> 
> Did you mean !name && !domid? i.e. fail if no name is given AND
domid
> isn''t already set? But domid == 0 is a valid domid...
I think I missed the fact that domid is a pointer. In that case I
think !domid is an outright error in the caller (unless there is a
usecase for determining if a qualifier is valid?).

As I say though I think name == NULL is ok given the intended semantics.

Ian.

On 15/04/13 13:11, Ian Campbell wrote:
> On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf wrote:
>> This adds a backend_domname field in libxl devices that contain a
>> backend_domid field, allowing either a domid or a domain name to be
>> specified in the configuration structures.  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 will simplify specification of backend domains in
>> other users of libxl.
> Looks good to me (some minor comments below). Given that the initial
> version of this was posted ages ago I think this should be granted a
> freeze exception. George?
1. It seems like if there are any bugs they should be caught before the 
release
2. Allowing disk backends to be specified seems like a very good feature 
for 4.3; and it seems like if there is a bug it should be caught before 
the release and fairly easy to fix.

So re the code freeze:

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

On Mon, 2013-04-15 at 13:11 +0100, Ian Campbell wrote:
> On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf wrote:
> > This avoids returning unallocated memory in the libxl_device_vtpm
> > structure in libxl_device_vtpm_list, and uses libxl_device_nic_init
> > instead of memset when initializing libxl_device_nics.
> > 
> > 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>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
I didn''t see a v4.2 repost of this one like there was for 1/2 so
I''ve
applied this version, which I think is correct thing for me to have
done.

Ian.

On 04/17/2013 11:01 AM, Ian Campbell wrote:
> On Mon, 2013-04-15 at 13:11 +0100, Ian Campbell wrote:
>> On Fri, 2013-04-12 at 16:22 +0100, Daniel De Graaf wrote:
>>> This avoids returning unallocated memory in the libxl_device_vtpm
>>> structure in libxl_device_vtpm_list, and uses libxl_device_nic_init
>>> instead of memset when initializing libxl_device_nics.
>>>
>>> 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>
>>
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> I didn''t see a v4.2 repost of this one like there was for 1/2 so
I''ve
> applied this version, which I think is correct thing for me to have
> done.
>
> Ian.
>
Yes, this patch did not change so I did not repost it.

-- 
Daniel De Graaf
National Security Agency