Daniel De Graaf
2013-Apr-15 14:33 UTC
[PATCH v4.2 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.
The check on run_hotplug_scripts in parse_config_data is removed because
it is a duplicate of the one in libxl__device_nic_setdefault, and is
removed here because it no longer has the resolved domain ID to check.
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 | 32 +++++++++++----
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 | 80 +++++++------------------------------
8 files changed, 89 insertions(+), 73 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..06e4d09 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1730,13 +1730,23 @@ 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(CTX, 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;
+ int rc;
+ if (libxl_uuid_is_nil(&vtpm->uuid)) {
+ libxl_uuid_generate(&vtpm->uuid);
+ }
+ rc = libxl__resolve_domid(gc, vtpm->backend_domname,
&vtpm->backend_domid);
+ return rc;
}
static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
@@ -1968,6 +1978,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;
}
@@ -2739,6 +2750,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc,
libxl_device_nic *nic,
uint32_t domid)
{
int run_hotplug_scripts;
+ int rc;
if (!nic->mtu)
nic->mtu = 1492;
@@ -2799,7 +2811,8 @@ int libxl__device_nic_setdefault(libxl__gc *gc,
libxl_device_nic *nic,
abort();
}
- return 0;
+ rc = libxl__resolve_domid(gc, nic->backend_domname,
&nic->backend_domid);
+ return rc;
}
static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
@@ -3156,7 +3169,9 @@ out:
int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
{
- return 0;
+ int rc;
+ rc = libxl__resolve_domid(gc, vkb->backend_domname,
&vkb->backend_domid);
+ return rc;
}
static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
@@ -3240,6 +3255,8 @@ out:
int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
{
+ int rc;
+
libxl_defbool_setdefault(&vfb->vnc.enable, true);
if (libxl_defbool_val(vfb->vnc.enable)) {
if (!vfb->vnc.listen) {
@@ -3255,7 +3272,8 @@ 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;
+ rc = libxl__resolve_domid(gc, vfb->backend_domname,
&vfb->backend_domid);
+ return rc;
}
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..22e85bf 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,17 +1172,7 @@ 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) {
- fprintf(stderr, "ERROR: the vif
''backend='' option "
- "cannot be used in conjunction with "
- "run_hotplug_scripts, please set "
- "run_hotplug_scripts to 0 in
xl.conf\n");
- exit(EXIT_FAILURE);
- }
+ nic->backend_domname = strdup(p2 + 1);
} else if (!strcmp(p, "rate")) {
parse_vif_rate(&config, (p2 + 1), nic);
} else if (!strcmp(p, "accel")) {
@@ -2556,8 +2518,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 +5494,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 +5599,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 +5616,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 +5645,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 +5697,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 +5717,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 +5757,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 +6639,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
Ian Campbell
2013-Apr-17 15:01 UTC
Re: [PATCH v4.2 1/2] libxl: postpone backend name resolution
On Mon, 2013-04-15 at 15:33 +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. > > The check on run_hotplug_scripts in parse_config_data is removed because > it is a duplicate of the one in libxl__device_nic_setdefault, and is > removed here because it no longer has the resolved domain ID to check. > > 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>Acked + Ian J acked on IRC + applied, thanks.> --- > > 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.I reran flex as part of the commit, thanks for the heads up.