Mailing List Archive: [PATCH v2] libxl: Support backend domain ID for disks

[PATCH v2] libxl: Support backend domain ID for disks

Sep 5, 2012, 10:05 AM

Post #1 of 5 (332 views)

Allow specification of backend domains for disks, either in the config
file or via xl block-attach. This functionality was supported in xend
(via an optional command line parameter to block-attach), so should also
be supported with libxl.

In order to support named backend domains like network-attach, a valid
libxl_ctx must now be passed to xlu_cfg_init.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---

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/libxlu_cfg.c | 4 +++-
tools/libxl/libxlu_disk_l.l | 9 +++++++++
tools/libxl/libxlu_internal.h | 1 +
tools/libxl/libxlutil.h | 3 ++-
tools/libxl/xl.c | 2 +-
tools/libxl/xl_cmdimpl.c | 20 +++++++++-----------
7 files changed, 37 insertions(+), 14 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/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 22adcb0..1d086b4 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -25,13 +25,15 @@
#include "libxlu_cfg_l.h"
#include "libxlu_cfg_i.h"

-XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) {
+XLU_Config *xlu_cfg_init(FILE *report, const char *report_source,
+ libxl_ctx *ctx) {
XLU_Config *cfg;

cfg= malloc(sizeof(*cfg));
if (!cfg) return 0;

cfg->report= report;
+ cfg->ctx = ctx;
cfg->config_source= strdup(report_source);
if (!cfg->config_source) { free(cfg); return 0; }

diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index bee16a1..8cfc16e 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -33,6 +33,7 @@

%{
#include "libxlu_disk_i.h"
+#include "libxl_utils.h"

#define YY_NO_INPUT

@@ -113,6 +114,13 @@ static void setbackendtype(DiskParseContext *dpc, const char *str) {
else xlu__disk_err(dpc,str,"unknown value for backendtype");
}

+/* Sets ->backend_domid from the string. */
+static void setbackend(DiskParseContext *dpc, const char *str) {
+ if (libxl_name_to_domid(dpc->cfg->ctx, str, &dpc->disk->backend_domid)) {
+ xlu__disk_err(dpc,str,"unknown domain for backend");
+ }
+}
+
#define DEPRECATE(usewhatinstead) /* not currently reported */

/* Handles a vdev positional parameter which includes a devtype. */
@@ -168,6 +176,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(','); setbackend(DPC,FROMEQUALS); }
backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }

vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
index 7579158..5a9cf6d 100644
--- a/tools/libxl/libxlu_internal.h
+++ b/tools/libxl/libxlu_internal.h
@@ -39,6 +39,7 @@ struct XLU_Config {
XLU_ConfigSetting *settings;
FILE *report;
char *config_source;
+ libxl_ctx *ctx;
};

typedef struct {
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index 0333e55..1de23e7 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -24,7 +24,8 @@
typedef struct XLU_Config XLU_Config;
typedef struct XLU_ConfigList XLU_ConfigList;

-XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename);
+XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename,
+ libxl_ctx *ctx);
/* 0 means we got ENOMEM. */
/* report_filename is copied; report is saved and must remain valid
* until the Config is destroyed. */
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index f31e836..32a5c32 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -56,7 +56,7 @@ static void parse_global_config(const char *configfile,
int e;
const char *buf;

- config = xlu_cfg_init(stderr, configfile);
+ config = xlu_cfg_init(stderr, configfile, ctx);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
exit(1);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2d6ab97..eaebbeb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -427,7 +427,7 @@ static void parse_disk_config_multistring(XLU_Config **config,
libxl_device_disk_init(disk);

if (!*config) {
- *config = xlu_cfg_init(stderr, "command line");
+ *config = xlu_cfg_init(stderr, "command line", ctx);
if (!*config) { perror("xlu_cfg_init"); exit(-1); }
}

@@ -583,7 +583,7 @@ static void parse_config_data(const char *config_source,
libxl_domain_create_info *c_info = &d_config->c_info;
libxl_domain_build_info *b_info = &d_config->b_info;

- config= xlu_cfg_init(stderr, config_source);
+ config= xlu_cfg_init(stderr, config_source, ctx);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
exit(1);
@@ -2473,7 +2473,7 @@ static void pcidetach(const char *dom, const char *bdf, int force)

libxl_device_pci_init(&pcidev);

- config = xlu_cfg_init(stderr, "command line");
+ config = xlu_cfg_init(stderr, "command line", ctx);
if (!config) { perror("xlu_cfg_inig"); exit(-1); }

if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
@@ -2520,7 +2520,7 @@ static void pciattach(const char *dom, const char *bdf, const char *vs)

libxl_device_pci_init(&pcidev);

- config = xlu_cfg_init(stderr, "command line");
+ config = xlu_cfg_init(stderr, "command line", ctx);
if (!config) { perror("xlu_cfg_inig"); exit(-1); }

if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
@@ -2586,7 +2586,7 @@ static void pciassignable_add(const char *bdf, int rebind)

libxl_device_pci_init(&pcidev);

- config = xlu_cfg_init(stderr, "command line");
+ config = xlu_cfg_init(stderr, "command line", ctx);
if (!config) { perror("xlu_cfg_init"); exit(-1); }

if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
@@ -2624,7 +2624,7 @@ static void pciassignable_remove(const char *bdf, int rebind)

libxl_device_pci_init(&pcidev);

- config = xlu_cfg_init(stderr, "command line");
+ config = xlu_cfg_init(stderr, "command line", ctx);
if (!config) { perror("xlu_cfg_init"); exit(-1); }

if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
@@ -5321,7 +5321,7 @@ int main_networkattach(int argc, char **argv)
return 1;
}

- config= xlu_cfg_init(stderr, "command line");
+ config= xlu_cfg_init(stderr, "command line", ctx);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
return 1;
@@ -5467,7 +5467,7 @@ int main_networkdetach(int argc, char **argv)
int main_blockattach(int argc, char **argv)
{
int opt;
- uint32_t fe_domid, be_domid = 0;
+ uint32_t fe_domid;
libxl_device_disk disk = { 0 };
XLU_Config *config = 0;

@@ -5483,8 +5483,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);
@@ -6078,7 +6076,7 @@ int main_cpupoolcreate(int argc, char **argv)
config_len += strlen(extra_config) + 1;
}

- config = xlu_cfg_init(stderr, config_src);
+ config = xlu_cfg_init(stderr, config_src, ctx);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
goto out;
--
1.7.11.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] libxl: Support backend domain ID for disks [ In reply to ]

Ian.Campbell at citrix

Sep 6, 2012, 12:26 AM

Post #2 of 5 (311 views)

Permalink

On Wed, 2012-09-05 at 18:05 +0100, Daniel De Graaf wrote:
> Allow specification of backend domains for disks, either in the config
> file or via xl block-attach. This functionality was supported in xend
> (via an optional command line parameter to block-attach), so should also
> be supported with libxl.
>
> In order to support named backend domains like network-attach, a valid
> libxl_ctx must now be passed to xlu_cfg_init.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Sorry but I think in the end this has missed the cut-off for 4.2.0 (we
are cutting the final RC tomorrow).

We should be branching soon after the final RC which means 4.3
development will open in unstable shortly. We should take this into
unstable then and consider it for 4.2.1.

In the meantime I suppose we should mention this in the release notes.

>
> ---
>
> 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/libxlu_cfg.c | 4 +++-
> tools/libxl/libxlu_disk_l.l | 9 +++++++++
> tools/libxl/libxlu_internal.h | 1 +
> tools/libxl/libxlutil.h | 3 ++-
> tools/libxl/xl.c | 2 +-
> tools/libxl/xl_cmdimpl.c | 20 +++++++++-----------
> 7 files changed, 37 insertions(+), 14 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/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
> index 22adcb0..1d086b4 100644
> --- a/tools/libxl/libxlu_cfg.c
> +++ b/tools/libxl/libxlu_cfg.c
> @@ -25,13 +25,15 @@
> #include "libxlu_cfg_l.h"
> #include "libxlu_cfg_i.h"
>
> -XLU_Config *xlu_cfg_init(FILE *report, const char *report_source) {
> +XLU_Config *xlu_cfg_init(FILE *report, const char *report_source,
> + libxl_ctx *ctx) {
> XLU_Config *cfg;
>
> cfg= malloc(sizeof(*cfg));
> if (!cfg) return 0;
>
> cfg->report= report;
> + cfg->ctx = ctx;
> cfg->config_source= strdup(report_source);
> if (!cfg->config_source) { free(cfg); return 0; }
>
> diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
> index bee16a1..8cfc16e 100644
> --- a/tools/libxl/libxlu_disk_l.l
> +++ b/tools/libxl/libxlu_disk_l.l
> @@ -33,6 +33,7 @@
>
> %{
> #include "libxlu_disk_i.h"
> +#include "libxl_utils.h"
>
> #define YY_NO_INPUT
>
> @@ -113,6 +114,13 @@ static void setbackendtype(DiskParseContext *dpc, const char *str) {
> else xlu__disk_err(dpc,str,"unknown value for backendtype");
> }
>
> +/* Sets ->backend_domid from the string. */
> +static void setbackend(DiskParseContext *dpc, const char *str) {
> + if (libxl_name_to_domid(dpc->cfg->ctx, str, &dpc->disk->backend_domid)) {
> + xlu__disk_err(dpc,str,"unknown domain for backend");
> + }
> +}
> +
> #define DEPRECATE(usewhatinstead) /* not currently reported */
>
> /* Handles a vdev positional parameter which includes a devtype. */
> @@ -168,6 +176,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(','); setbackend(DPC,FROMEQUALS); }
> backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); }
>
> vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
> diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
> index 7579158..5a9cf6d 100644
> --- a/tools/libxl/libxlu_internal.h
> +++ b/tools/libxl/libxlu_internal.h
> @@ -39,6 +39,7 @@ struct XLU_Config {
> XLU_ConfigSetting *settings;
> FILE *report;
> char *config_source;
> + libxl_ctx *ctx;
> };
>
> typedef struct {
> diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
> index 0333e55..1de23e7 100644
> --- a/tools/libxl/libxlutil.h
> +++ b/tools/libxl/libxlutil.h
> @@ -24,7 +24,8 @@
> typedef struct XLU_Config XLU_Config;
> typedef struct XLU_ConfigList XLU_ConfigList;
>
> -XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename);
> +XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename,
> + libxl_ctx *ctx);
> /* 0 means we got ENOMEM. */
> /* report_filename is copied; report is saved and must remain valid
> * until the Config is destroyed. */
> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
> index f31e836..32a5c32 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -56,7 +56,7 @@ static void parse_global_config(const char *configfile,
> int e;
> const char *buf;
>
> - config = xlu_cfg_init(stderr, configfile);
> + config = xlu_cfg_init(stderr, configfile, ctx);
> if (!config) {
> fprintf(stderr, "Failed to allocate for configuration\n");
> exit(1);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 2d6ab97..eaebbeb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -427,7 +427,7 @@ static void parse_disk_config_multistring(XLU_Config **config,
> libxl_device_disk_init(disk);
>
> if (!*config) {
> - *config = xlu_cfg_init(stderr, "command line");
> + *config = xlu_cfg_init(stderr, "command line", ctx);
> if (!*config) { perror("xlu_cfg_init"); exit(-1); }
> }
>
> @@ -583,7 +583,7 @@ static void parse_config_data(const char *config_source,
> libxl_domain_create_info *c_info = &d_config->c_info;
> libxl_domain_build_info *b_info = &d_config->b_info;
>
> - config= xlu_cfg_init(stderr, config_source);
> + config= xlu_cfg_init(stderr, config_source, ctx);
> if (!config) {
> fprintf(stderr, "Failed to allocate for configuration\n");
> exit(1);
> @@ -2473,7 +2473,7 @@ static void pcidetach(const char *dom, const char *bdf, int force)
>
> libxl_device_pci_init(&pcidev);
>
> - config = xlu_cfg_init(stderr, "command line");
> + config = xlu_cfg_init(stderr, "command line", ctx);
> if (!config) { perror("xlu_cfg_inig"); exit(-1); }
>
> if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> @@ -2520,7 +2520,7 @@ static void pciattach(const char *dom, const char *bdf, const char *vs)
>
> libxl_device_pci_init(&pcidev);
>
> - config = xlu_cfg_init(stderr, "command line");
> + config = xlu_cfg_init(stderr, "command line", ctx);
> if (!config) { perror("xlu_cfg_inig"); exit(-1); }
>
> if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> @@ -2586,7 +2586,7 @@ static void pciassignable_add(const char *bdf, int rebind)
>
> libxl_device_pci_init(&pcidev);
>
> - config = xlu_cfg_init(stderr, "command line");
> + config = xlu_cfg_init(stderr, "command line", ctx);
> if (!config) { perror("xlu_cfg_init"); exit(-1); }
>
> if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> @@ -2624,7 +2624,7 @@ static void pciassignable_remove(const char *bdf, int rebind)
>
> libxl_device_pci_init(&pcidev);
>
> - config = xlu_cfg_init(stderr, "command line");
> + config = xlu_cfg_init(stderr, "command line", ctx);
> if (!config) { perror("xlu_cfg_init"); exit(-1); }
>
> if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> @@ -5321,7 +5321,7 @@ int main_networkattach(int argc, char **argv)
> return 1;
> }
>
> - config= xlu_cfg_init(stderr, "command line");
> + config= xlu_cfg_init(stderr, "command line", ctx);
> if (!config) {
> fprintf(stderr, "Failed to allocate for configuration\n");
> return 1;
> @@ -5467,7 +5467,7 @@ int main_networkdetach(int argc, char **argv)
> int main_blockattach(int argc, char **argv)
> {
> int opt;
> - uint32_t fe_domid, be_domid = 0;
> + uint32_t fe_domid;
> libxl_device_disk disk = { 0 };
> XLU_Config *config = 0;
>
> @@ -5483,8 +5483,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);
> @@ -6078,7 +6076,7 @@ int main_cpupoolcreate(int argc, char **argv)
> config_len += strlen(extra_config) + 1;
> }
>
> - config = xlu_cfg_init(stderr, config_src);
> + config = xlu_cfg_init(stderr, config_src, ctx);
> if (!config) {
> fprintf(stderr, "Failed to allocate for configuration\n");
> goto out;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] libxl: Support backend domain ID for disks [ In reply to ]

dgdegra at tycho

Sep 6, 2012, 5:24 AM

Post #3 of 5 (308 views)

Permalink

On 09/06/2012 03:26 AM, Ian Campbell wrote:
> On Wed, 2012-09-05 at 18:05 +0100, Daniel De Graaf wrote:
>> Allow specification of backend domains for disks, either in the config
>> file or via xl block-attach. This functionality was supported in xend
>> (via an optional command line parameter to block-attach), so should also
>> be supported with libxl.
>>
>> In order to support named backend domains like network-attach, a valid
>> libxl_ctx must now be passed to xlu_cfg_init.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>
> Sorry but I think in the end this has missed the cut-off for 4.2.0 (we
> are cutting the final RC tomorrow).
>
> We should be branching soon after the final RC which means 4.3
> development will open in unstable shortly. We should take this into
> unstable then and consider it for 4.2.1.
>
> In the meantime I suppose we should mention this in the release notes.
>

That's fine; I just wanted to be sure that the change to xlu_cfg_init
was noticed and that the API change was noted in 4.2.0 if that is needed
to make this acceptable for 4.2.1. Sorry I didn't get this in earlier; I
didn't notice that you wanted me to resubmit until it was too late.

--
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] libxl: Support backend domain ID for disks [ In reply to ]

Ian.Jackson at eu

Oct 9, 2012, 8:23 AM

Post #4 of 5 (299 views)

Permalink

Daniel De Graaf writes ("[PATCH v2] libxl: Support backend domain ID for disks"):
> Allow specification of backend domains for disks, either in the config
> file or via xl block-attach. This functionality was supported in xend
> (via an optional command line parameter to block-attach), so should also
> be supported with libxl.
>
> In order to support named backend domains like network-attach, a valid
> libxl_ctx must now be passed to xlu_cfg_init.

I've been thinking about this and I'm afraid I've come to the
conclusion that the way your new API specifies backend domains is not
the way I think it should be done.

In particular, I think translating the config file from a source text
into an idl configuration structure shouldn't depend on looking up
information like domids. (The same would be true of DNS names, or
other runtime lookups.) So I think the backend domain _name_ should
be in the IDL structure.

But of course it should also be possible to specify a domid.

I can think of three (at least superficially) plausible ways to define
this API:

1. The backend domain is specified as a string. If specifying a domid
is desired, the string is the domid number in ascii. Domains whose
names are entirely valid numbers according to strtoul(,,0) cannot
be specified as backends by name (or should perhaps be prohibited
entirely - xl can't handle them anyway).

2. The IDL contains both a string and a number. If the string is
provided, it is used; otherwise the number is used.

3. The IDL contains a variadic "domspec" structure which allows the
domain to be specified (a) not at all (b) as a domid (c) as a
domain name (d) as a uuid.

Of these I think 3 is probably overkill and either 1 or 2 is
acceptable and I have a marginal preference for 2.

Before you implement any of this I think we should agree whether my
qualm about domain name lookups during config parsing is justified,
and what the right API is.

NB that this complaint does seem perhaps contrary to my intent to add
a libxl context to libxlu parsing calls. But, having thought about
it, this libxl context should be a "dummy" one which can be used for
memory allocation and logging but which does not support actual Xen
functionality.

Thanks, and sorry to block your useful new functionality on cans of
works.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] libxl: Support backend domain ID for disks [ In reply to ]

dgdegra at tycho

Oct 9, 2012, 11:41 AM

Post #5 of 5 (296 views)

Permalink

On 10/09/2012 11:23 AM, Ian Jackson wrote:
> Daniel De Graaf writes ("[PATCH v2] libxl: Support backend domain ID for disks"):
>> Allow specification of backend domains for disks, either in the config
>> file or via xl block-attach. This functionality was supported in xend
>> (via an optional command line parameter to block-attach), so should also
>> be supported with libxl.
>>
>> In order to support named backend domains like network-attach, a valid
>> libxl_ctx must now be passed to xlu_cfg_init.
>
> I've been thinking about this and I'm afraid I've come to the
> conclusion that the way your new API specifies backend domains is not
> the way I think it should be done.
>
> In particular, I think translating the config file from a source text
> into an idl configuration structure shouldn't depend on looking up
> information like domids. (The same would be true of DNS names, or
> other runtime lookups.) So I think the backend domain _name_ should
> be in the IDL structure.
>
> But of course it should also be possible to specify a domid.
>
> I can think of three (at least superficially) plausible ways to define
> this API:
>
> 1. The backend domain is specified as a string. If specifying a domid
> is desired, the string is the domid number in ascii. Domains whose
> names are entirely valid numbers according to strtoul(,,0) cannot
> be specified as backends by name (or should perhaps be prohibited
> entirely - xl can't handle them anyway).
>
> 2. The IDL contains both a string and a number. If the string is
> provided, it is used; otherwise the number is used.
>
> 3. The IDL contains a variadic "domspec" structure which allows the
> domain to be specified (a) not at all (b) as a domid (c) as a
> domain name (d) as a uuid.
>
> Of these I think 3 is probably overkill and either 1 or 2 is
> acceptable and I have a marginal preference for 2.
>
> Before you implement any of this I think we should agree whether my
> qualm about domain name lookups during config parsing is justified,
> and what the right API is.
>
> NB that this complaint does seem perhaps contrary to my intent to add
> a libxl context to libxlu parsing calls. But, having thought about
> it, this libxl context should be a "dummy" one which can be used for
> memory allocation and logging but which does not support actual Xen
> functionality.
>
> Thanks, and sorry to block your useful new functionality on cans of
> works.
>
> Ian.
>

I assume that this change would also apply to vfb, vkb, nic, and any new
devices that allow backend domains to be specified (i.e. vtpm). Currently,
the domains have to be specified by name in xl's config file, but I think
that allowing domid there in addition is a good idea.

Method (2) is also simpler for backwards compatibility as it allows the
existing backend_domid fields to remain, supplemented by an entry like
backend_name - so that's my preference.

--
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel