* [PATCH 1/3] move stubdom make into a proper function
2010-07-07 13:00 [PATCH 0/3] libxl stubdom API cleanup Vincent Hanquez
@ 2010-07-07 13:00 ` Vincent Hanquez
2010-07-07 13:00 ` [PATCH 2/3] let the user of libxenlight choose explicitely if it want to start a stubdom or not Vincent Hanquez
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-07 13:00 UTC (permalink / raw)
To: Xen Devel; +Cc: Vincent Hanquez
[-- Attachment #1: Type: text/plain, Size: 243 bytes --]
Signed-off-by: Vincent Hanquez <vincent.hanquez@eu.citrix.com>
---
tools/libxl/libxl.c | 72 ++++++++++++++++++++++++++++++++------------------
tools/libxl/libxl.h | 10 +++++++
2 files changed, 56 insertions(+), 26 deletions(-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-move-stubdom-make-into-a-proper-function.patch --]
[-- Type: text/x-patch; name="0001-move-stubdom-make-into-a-proper-function.patch", Size: 5500 bytes --]
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 293aaa7..bf2c7e8 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -80,7 +80,7 @@ int libxl_ctx_free(struct libxl_ctx *ctx)
/******************************************************************************/
int libxl_domain_make(struct libxl_ctx *ctx, libxl_domain_create_info *info,
- uint32_t *domid)
+ uint32_t *domid)
{
int flags, ret, i, rc;
char *uuid_string;
@@ -1020,6 +1020,41 @@ retry_transaction:
return 0;
}
+int libxl_stubdom_make(struct libxl_ctx *ctx, libxl_stubdom_create_info *info,
+ libxl_domain_build_state *state, uint32_t *domid)
+{
+ int ret, i;
+ libxl_domain_create_info c_info;
+ libxl_domain_build_info b_info;
+
+ memset(&c_info, 0x00, sizeof(libxl_domain_create_info));
+ c_info.hvm = 0;
+ c_info.name = libxl_sprintf(ctx, "%s-dm", libxl_domid_to_name(ctx, info->target_domid));
+ for (i = 0; i < 16; i++)
+ c_info.uuid[i] = info->uuid[i];
+
+ memset(&b_info, 0x00, sizeof(libxl_domain_build_info));
+ b_info.max_vcpus = 1;
+ b_info.max_memkb = info->max_memkb;
+ b_info.target_memkb = b_info.max_memkb;
+ b_info.kernel = libxl_abs_path(ctx, "ioemu-stubdom.gz", libxl_xenfirmwaredir_path());
+ b_info.u.pv.cmdline = libxl_sprintf(ctx, " -d %d", info->target_domid);
+ b_info.u.pv.ramdisk = "";
+ b_info.u.pv.features = "";
+ b_info.hvm = 0;
+
+ ret = libxl_domain_make(ctx, &c_info, domid);
+ if (ret)
+ return ret;
+ ret = libxl_domain_build(ctx, &b_info, *domid, state);
+ if (ret)
+ return ret;
+
+ xc_domain_set_target(ctx->xch, *domid, info->target_domid);
+ xs_set_target(ctx->xsh, *domid, info->target_domid);
+ return 0;
+}
+
static int libxl_create_stubdom(struct libxl_ctx *ctx,
libxl_device_model_info *info,
libxl_device_disk *disks, int num_disks,
@@ -1030,40 +1065,27 @@ static int libxl_create_stubdom(struct libxl_ctx *ctx,
{
int i, num_console = 1, ret;
libxl_device_console *console;
- libxl_domain_create_info c_info;
- libxl_domain_build_info b_info;
libxl_domain_build_state state;
uint32_t domid;
char **args;
struct xs_permissions perm[2];
xs_transaction_t t;
libxl_device_model_starting *dm_starting = 0;
+ libxl_stubdom_create_info stubinfo;
+
+ /* initialize stubinfo create structure */
+ stubinfo.target_domid = info->domid;
+ stubinfo.max_memkb = 32 * 1024;
+ for (i = 0; i < 16; i++)
+ stubinfo.uuid[i] = info->uuid[i];
+ ret = libxl_stubdom_make(ctx, &stubinfo, &state, &domid);
+ if (ret)
+ return ret;
args = libxl_build_device_model_args(ctx, info, vifs, num_vifs);
if (!args)
return ERROR_FAIL;
- memset(&c_info, 0x00, sizeof(libxl_domain_create_info));
- c_info.hvm = 0;
- c_info.name = libxl_sprintf(ctx, "%s-dm", libxl_domid_to_name(ctx, info->domid));
- for (i = 0; i < 16; i++)
- c_info.uuid[i] = info->uuid[i];
-
- memset(&b_info, 0x00, sizeof(libxl_domain_build_info));
- b_info.max_vcpus = 1;
- b_info.max_memkb = 32 * 1024;
- b_info.target_memkb = b_info.max_memkb;
- b_info.kernel = libxl_abs_path(ctx, "ioemu-stubdom.gz", libxl_xenfirmwaredir_path());
- b_info.u.pv.cmdline = libxl_sprintf(ctx, " -d %d", info->domid);
- b_info.u.pv.ramdisk = "";
- b_info.u.pv.features = "";
- b_info.hvm = 0;
-
- ret = libxl_domain_make(ctx, &c_info, &domid);
- if (ret) return ret;
- ret = libxl_domain_build(ctx, &b_info, domid, &state);
- if (ret) return ret;
-
libxl_write_dmargs(ctx, domid, info->domid, args);
libxl_xs_write(ctx, XBT_NULL,
libxl_sprintf(ctx, "%s/image/device-model-domid", libxl_xs_get_dompath(ctx, info->domid)),
@@ -1071,8 +1093,6 @@ static int libxl_create_stubdom(struct libxl_ctx *ctx,
libxl_xs_write(ctx, XBT_NULL,
libxl_sprintf(ctx, "%s/target", libxl_xs_get_dompath(ctx, domid)),
"%d", info->domid);
- xc_domain_set_target(ctx->xch, domid, info->domid);
- xs_set_target(ctx->xsh, domid, info->domid);
perm[0].id = domid;
perm[0].perms = XS_PERM_NONE;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 85f8196..a0a3a1a 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -121,6 +121,13 @@ typedef struct {
} libxl_domain_build_info;
typedef struct {
+ uint8_t uuid[16];
+ char *name;
+ uint32_t max_memkb;
+ uint32_t target_domid;
+} libxl_stubdom_create_info;
+
+typedef struct {
uint32_t store_port;
unsigned long store_mfn;
uint32_t console_port;
@@ -364,6 +371,9 @@ struct libxl_dominfo * libxl_list_domain(struct libxl_ctx*, int *nb_domain);
struct libxl_poolinfo * libxl_list_pool(struct libxl_ctx*, int *nb_pool);
struct libxl_vminfo * libxl_list_vm(struct libxl_ctx *ctx, int *nb_vm);
+int libxl_stubdom_make(struct libxl_ctx *ctx, libxl_stubdom_create_info *info,
+ libxl_domain_build_state *state, uint32_t *domid);
+
typedef struct libxl_device_model_starting libxl_device_model_starting;
int libxl_create_device_model(struct libxl_ctx *ctx,
libxl_device_model_info *info,
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/3] let the user of libxenlight choose explicitely if it want to start a stubdom or not.
2010-07-07 13:00 [PATCH 0/3] libxl stubdom API cleanup Vincent Hanquez
2010-07-07 13:00 ` [PATCH 1/3] move stubdom make into a proper function Vincent Hanquez
@ 2010-07-07 13:00 ` Vincent Hanquez
2010-07-07 13:00 ` [PATCH 3/3] stubdom_create returns stubdomain domid so that unpause is done by the user of libxenlight Vincent Hanquez
2010-07-07 16:53 ` [PATCH 0/3] libxl stubdom API cleanup Stefano Stabellini
3 siblings, 0 replies; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-07 13:00 UTC (permalink / raw)
To: Xen Devel; +Cc: Vincent Hanquez
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
the same mechanism of choosing stubdomain or not, depending on the qemu-dm
filename that was embedded in the libxenlight is moved to xl so that nothing
change. also expose more functions that the caller is expected to used now.
Signed-off-by: Vincent Hanquez <vincent.hanquez@eu.citrix.com>
---
tools/libxl/libxl.c | 30 +++++++++++-------------------
tools/libxl/libxl.h | 15 ++++++++++++++-
tools/libxl/xl_cmdimpl.c | 12 ++++++++++--
3 files changed, 35 insertions(+), 22 deletions(-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-let-the-user-of-libxenlight-choose-explicitely-if-it.patch --]
[-- Type: text/x-patch; name="0002-let-the-user-of-libxenlight-choose-explicitely-if-it.patch", Size: 5252 bytes --]
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bf2c7e8..811678f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -949,10 +949,10 @@ void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
xs_daemon_close(xsh);
}
-static int libxl_vfb_and_vkb_from_device_model_info(struct libxl_ctx *ctx,
- libxl_device_model_info *info,
- libxl_device_vfb *vfb,
- libxl_device_vkb *vkb)
+int libxl_vfb_and_vkb_from_device_model_info(struct libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_vfb *vfb,
+ libxl_device_vkb *vkb)
{
memset(vfb, 0x00, sizeof(libxl_device_vfb));
memset(vkb, 0x00, sizeof(libxl_device_vkb));
@@ -1055,13 +1055,13 @@ int libxl_stubdom_make(struct libxl_ctx *ctx, libxl_stubdom_create_info *info,
return 0;
}
-static int libxl_create_stubdom(struct libxl_ctx *ctx,
- libxl_device_model_info *info,
- libxl_device_disk *disks, int num_disks,
- libxl_device_nic *vifs, int num_vifs,
- libxl_device_vfb *vfb,
- libxl_device_vkb *vkb,
- libxl_device_model_starting **starting_r)
+int libxl_stubdom_create(struct libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_disk *disks, int num_disks,
+ libxl_device_nic *vifs, int num_vifs,
+ libxl_device_vfb *vfb,
+ libxl_device_vkb *vkb,
+ libxl_device_model_starting **starting_r)
{
int i, num_console = 1, ret;
libxl_device_console *console;
@@ -1178,14 +1178,6 @@ int libxl_create_device_model(struct libxl_ctx *ctx,
char *vm_path;
char **pass_stuff;
- if (strstr(info->device_model, "stubdom-dm")) {
- libxl_device_vfb vfb;
- libxl_device_vkb vkb;
-
- libxl_vfb_and_vkb_from_device_model_info(ctx, info, &vfb, &vkb);
- return libxl_create_stubdom(ctx, info, disks, num_disks, vifs, num_vifs, &vfb, &vkb, starting_r);
- }
-
args = libxl_build_device_model_args(ctx, info, vifs, num_vifs);
if (!args)
return ERROR_FAIL;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a0a3a1a..76f9b5c 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -336,6 +336,7 @@ typedef struct {
char *token;
} libxl_waiter;
+typedef struct libxl_device_model_starting libxl_device_model_starting;
int libxl_get_wait_fd(struct libxl_ctx *ctx, int *fd);
/* waiter is allocated by the caller */
@@ -374,7 +375,19 @@ struct libxl_vminfo * libxl_list_vm(struct libxl_ctx *ctx, int *nb_vm);
int libxl_stubdom_make(struct libxl_ctx *ctx, libxl_stubdom_create_info *info,
libxl_domain_build_state *state, uint32_t *domid);
-typedef struct libxl_device_model_starting libxl_device_model_starting;
+int libxl_stubdom_create(struct libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_disk *disks, int num_disks,
+ libxl_device_nic *vifs, int num_vifs,
+ libxl_device_vfb *vfb,
+ libxl_device_vkb *vkb,
+ libxl_device_model_starting **starting_r);
+
+int libxl_vfb_and_vkb_from_device_model_info(struct libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_vfb *vfb,
+ libxl_device_vkb *vkb);
+
int libxl_create_device_model(struct libxl_ctx *ctx,
libxl_device_model_info *info,
libxl_device_disk *disk, int num_disks,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 208ecd6..a20e77a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1151,8 +1151,16 @@ start:
}
if (info1.hvm) {
dm_info.domid = domid;
- MUST( libxl_create_device_model(&ctx, &dm_info, disks, num_disks,
- vifs, num_vifs, &dm_starting) );
+ if (strstr(dm_info.device_model, "stubdom-dm")) {
+ libxl_device_vfb vfb;
+ libxl_device_vkb vkb;
+
+ libxl_vfb_and_vkb_from_device_model_info(&ctx, &dm_info, &vfb, &vkb);
+ MUST( libxl_stubdom_create(&ctx, &dm_info, disks, num_disks, vifs, num_vifs, &vfb, &vkb, &dm_starting) );
+ } else {
+ MUST( libxl_create_device_model(&ctx, &dm_info, disks, num_disks,
+ vifs, num_vifs, &dm_starting) );
+ }
} else {
for (i = 0; i < num_vfbs; i++) {
vfbs[i].domid = domid;
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/3] stubdom_create returns stubdomain domid so that unpause is done by the user of libxenlight.
2010-07-07 13:00 [PATCH 0/3] libxl stubdom API cleanup Vincent Hanquez
2010-07-07 13:00 ` [PATCH 1/3] move stubdom make into a proper function Vincent Hanquez
2010-07-07 13:00 ` [PATCH 2/3] let the user of libxenlight choose explicitely if it want to start a stubdom or not Vincent Hanquez
@ 2010-07-07 13:00 ` Vincent Hanquez
2010-07-07 16:53 ` [PATCH 0/3] libxl stubdom API cleanup Stefano Stabellini
3 siblings, 0 replies; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-07 13:00 UTC (permalink / raw)
To: Xen Devel; +Cc: Vincent Hanquez
[-- Attachment #1: Type: text/plain, Size: 242 bytes --]
Signed-off-by: Vincent Hanquez <vincent.hanquez@eu.citrix.com>
---
tools/libxl/libxl.c | 7 ++++---
tools/libxl/libxl.h | 2 +-
tools/libxl/xl_cmdimpl.c | 4 +++-
3 files changed, 8 insertions(+), 5 deletions(-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-stubdom_create-returns-stubdomain-domid-so-that-unpa.patch --]
[-- Type: text/x-patch; name="0003-stubdom_create-returns-stubdomain-domid-so-that-unpa.patch", Size: 2819 bytes --]
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 811678f..29fb10f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1060,7 +1060,7 @@ int libxl_stubdom_create(struct libxl_ctx *ctx,
libxl_device_disk *disks, int num_disks,
libxl_device_nic *vifs, int num_vifs,
libxl_device_vfb *vfb,
- libxl_device_vkb *vkb,
+ libxl_device_vkb *vkb, uint32_t *stubdomid,
libxl_device_model_starting **starting_r)
{
int i, num_console = 1, ret;
@@ -1141,6 +1141,9 @@ retry_transaction:
ret = libxl_device_console_add(ctx, domid, &console[i]);
if (ret) return ret;
}
+
+ *stubdomid = domid;
+
if (libxl_create_xenpv_qemu(ctx, vfb, num_console, console, &dm_starting) < 0) {
free(args);
return -1;
@@ -1150,8 +1153,6 @@ retry_transaction:
return -1;
}
- libxl_domain_unpause(ctx, domid);
-
if (starting_r) {
*starting_r = libxl_calloc(ctx, sizeof(libxl_device_model_starting), 1);
(*starting_r)->domid = info->domid;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 76f9b5c..597d91b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -381,7 +381,7 @@ int libxl_stubdom_create(struct libxl_ctx *ctx,
libxl_device_nic *vifs, int num_vifs,
libxl_device_vfb *vfb,
libxl_device_vkb *vkb,
- libxl_device_model_starting **starting_r);
+ uint32_t *stubdomid, libxl_device_model_starting **starting_r);
int libxl_vfb_and_vkb_from_device_model_info(struct libxl_ctx *ctx,
libxl_device_model_info *info,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a20e77a..6ed8e26 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1154,9 +1154,11 @@ start:
if (strstr(dm_info.device_model, "stubdom-dm")) {
libxl_device_vfb vfb;
libxl_device_vkb vkb;
+ uint32_t stubdomid;
libxl_vfb_and_vkb_from_device_model_info(&ctx, &dm_info, &vfb, &vkb);
- MUST( libxl_stubdom_create(&ctx, &dm_info, disks, num_disks, vifs, num_vifs, &vfb, &vkb, &dm_starting) );
+ MUST( libxl_stubdom_create(&ctx, &dm_info, disks, num_disks, vifs, num_vifs, &vfb, &vkb, &stubdomid, &dm_starting) );
+ libxl_domain_unpause(&ctx, stubdomid);
} else {
MUST( libxl_create_device_model(&ctx, &dm_info, disks, num_disks,
vifs, num_vifs, &dm_starting) );
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-07 13:00 [PATCH 0/3] libxl stubdom API cleanup Vincent Hanquez
` (2 preceding siblings ...)
2010-07-07 13:00 ` [PATCH 3/3] stubdom_create returns stubdomain domid so that unpause is done by the user of libxenlight Vincent Hanquez
@ 2010-07-07 16:53 ` Stefano Stabellini
2010-07-08 11:17 ` Vincent Hanquez
3 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2010-07-07 16:53 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: Xen Devel
On Wed, 7 Jul 2010, Vincent Hanquez wrote:
> This patch serie cleans the stubdom API, and remove some of the policy embedded
> in libxl. libxl users are expected to start explicitely a stubdomain instead of relying
> on a magic value in devicemodel_create.
>
> Vincent Hanquez (3):
> move stubdom make into a proper function
> let the user of libxenlight choose explicitely if it want to start a
> stubdom or not.
> stubdom_create returns stubdomain domid so that unpause is done by
> the user of libxenlight.
>
I though we wanted to make stubdoms transparent to libxenlight users,
why do you want to expose them now?
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-07 16:53 ` [PATCH 0/3] libxl stubdom API cleanup Stefano Stabellini
@ 2010-07-08 11:17 ` Vincent Hanquez
2010-07-08 14:03 ` Stefano Stabellini
0 siblings, 1 reply; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-08 11:17 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Xen Devel
On 07/07/10 17:53, Stefano Stabellini wrote:
> I though we wanted to make stubdoms transparent to libxenlight users,
> why do you want to expose them now?
>
From the users yes, from the libxenlight users (aka developers) no.
It's also a good way to get the policy out of libxenlight. For example the
32mb value which might or might not change in future.
--
Vincent
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 11:17 ` Vincent Hanquez
@ 2010-07-08 14:03 ` Stefano Stabellini
2010-07-08 14:18 ` Ian Campbell
0 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2010-07-08 14:03 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: Xen Devel, Stefano Stabellini
On Thu, 8 Jul 2010, Vincent Hanquez wrote:
> On 07/07/10 17:53, Stefano Stabellini wrote:
> > I though we wanted to make stubdoms transparent to libxenlight users,
> > why do you want to expose them now?
> >
> From the users yes, from the libxenlight users (aka developers) no.
> It's also a good way to get the policy out of libxenlight. For example the
> 32mb value which might or might not change in future.
Fair enough.
I ack the whole series then.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 14:03 ` Stefano Stabellini
@ 2010-07-08 14:18 ` Ian Campbell
2010-07-08 16:27 ` Ian Jackson
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Ian Campbell @ 2010-07-08 14:18 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Xen Devel, Vincent Hanquez
On Thu, 2010-07-08 at 15:03 +0100, Stefano Stabellini wrote:
> On Thu, 8 Jul 2010, Vincent Hanquez wrote:
> > On 07/07/10 17:53, Stefano Stabellini wrote:
> > > I though we wanted to make stubdoms transparent to libxenlight users,
> > > why do you want to expose them now?
> > >
> > From the users yes, from the libxenlight users (aka developers) no.
> > It's also a good way to get the policy out of libxenlight. For example the
> > 32mb value which might or might not change in future.
>
> Fair enough.
> I ack the whole series then.
Is it necessary to pull the mechanism out along with the policy though?
Could the libxl user not specify one of nostubdom, stubdom or
libxlchooses (the default?) and let the internals of libxl take care of
actually starting it etc?
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 14:18 ` Ian Campbell
@ 2010-07-08 16:27 ` Ian Jackson
2010-07-08 17:18 ` Vincent Hanquez
2010-07-09 8:17 ` Tim Deegan
2 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2010-07-08 16:27 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen Devel, Vincent Hanquez, Stefano Stabellini
Ian Campbell writes ("Re: [Xen-devel] [PATCH 0/3] libxl stubdom API cleanup"):
> Could the libxl user not specify one of nostubdom, stubdom or
> libxlchooses (the default?) and let the internals of libxl take care of
> actually starting it etc?
Quite so.
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 14:18 ` Ian Campbell
2010-07-08 16:27 ` Ian Jackson
@ 2010-07-08 17:18 ` Vincent Hanquez
2010-07-09 7:52 ` Ian Campbell
2010-07-09 8:17 ` Tim Deegan
2 siblings, 1 reply; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-08 17:18 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen Devel, Stefano Stabellini
On 08/07/10 15:18, Ian Campbell wrote:
> On Thu, 2010-07-08 at 15:03 +0100, Stefano Stabellini wrote:
>> On Thu, 8 Jul 2010, Vincent Hanquez wrote:
>>> On 07/07/10 17:53, Stefano Stabellini wrote:
>>>> I though we wanted to make stubdoms transparent to libxenlight users,
>>>> why do you want to expose them now?
>>>>
>>> From the users yes, from the libxenlight users (aka developers) no.
>>> It's also a good way to get the policy out of libxenlight. For example the
>>> 32mb value which might or might not change in future.
>>
>> Fair enough.
>> I ack the whole series then.
>
> Is it necessary to pull the mechanism out along with the policy though?
Necessary is quite a strong word.
> Could the libxl user not specify one of nostubdom, stubdom or
> libxlchooses (the default?) and let the internals of libxl take care of
> actually starting it etc?
Starting a stubdom or not, imply 2 very different side effects (e.g.
memory wise). Separating the API give better error reporting, more room
for action (e.g. creating a domain without stubdom if you don't have
those N mb to spare), and it also simplify the ocaml bindings not having
to encode complex semantics on the ocaml side.
--
Vincent
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 17:18 ` Vincent Hanquez
@ 2010-07-09 7:52 ` Ian Campbell
0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2010-07-09 7:52 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: Xen Devel, Stefano Stabellini
On Thu, 2010-07-08 at 18:18 +0100, Vincent Hanquez wrote:
> On 08/07/10 15:18, Ian Campbell wrote:
> > On Thu, 2010-07-08 at 15:03 +0100, Stefano Stabellini wrote:
> >> On Thu, 8 Jul 2010, Vincent Hanquez wrote:
> >>> On 07/07/10 17:53, Stefano Stabellini wrote:
> >>>> I though we wanted to make stubdoms transparent to libxenlight users,
> >>>> why do you want to expose them now?
> >>>>
> >>> From the users yes, from the libxenlight users (aka developers) no.
> >>> It's also a good way to get the policy out of libxenlight. For example the
> >>> 32mb value which might or might not change in future.
> >>
> >> Fair enough.
> >> I ack the whole series then.
> >
> > Is it necessary to pull the mechanism out along with the policy though?
>
> Necessary is quite a strong word.
>
> > Could the libxl user not specify one of nostubdom, stubdom or
> > libxlchooses (the default?) and let the internals of libxl take care of
> > actually starting it etc?
>
> Starting a stubdom or not, imply 2 very different side effects (e.g.
> memory wise). Separating the API give better error reporting, more room
> for action (e.g. creating a domain without stubdom if you don't have
> those N mb to spare), and it also simplify the ocaml bindings not having
> to encode complex semantics on the ocaml side.
Fair enough.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-08 14:18 ` Ian Campbell
2010-07-08 16:27 ` Ian Jackson
2010-07-08 17:18 ` Vincent Hanquez
@ 2010-07-09 8:17 ` Tim Deegan
2010-07-09 10:44 ` Vincent Hanquez
2 siblings, 1 reply; 21+ messages in thread
From: Tim Deegan @ 2010-07-09 8:17 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen Devel, Vincent Hanquez, Stefano Stabellini
At 15:18 +0100 on 08 Jul (1278602309), Ian Campbell wrote:
> On Thu, 2010-07-08 at 15:03 +0100, Stefano Stabellini wrote:
> > On Thu, 8 Jul 2010, Vincent Hanquez wrote:
> > > On 07/07/10 17:53, Stefano Stabellini wrote:
> > > > I though we wanted to make stubdoms transparent to libxenlight users,
> > > > why do you want to expose them now?
> > > >
> > > From the users yes, from the libxenlight users (aka developers) no.
> > > It's also a good way to get the policy out of libxenlight. For example the
> > > 32mb value which might or might not change in future.
> >
> > Fair enough.
> > I ack the whole series then.
>
> Is it necessary to pull the mechanism out along with the policy though?
Or, if we're taking some mechanism out, couldn't we take _all_ the
mechanism out? The idea of a stub domain doesn't seem like one that
libxl necessarily needs to know about.
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 8:17 ` Tim Deegan
@ 2010-07-09 10:44 ` Vincent Hanquez
2010-07-09 10:51 ` Tim Deegan
0 siblings, 1 reply; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-09 10:44 UTC (permalink / raw)
To: Tim Deegan; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
On 09/07/10 09:17, Tim Deegan wrote:
>> Is it necessary to pull the mechanism out along with the policy though?
>>
> Or, if we're taking some mechanism out, couldn't we take _all_ the
> mechanism out?
Which one do you have in minds ?
> The idea of a stub domain doesn't seem like one that
> libxl necessarily needs to know about.
>
yes, indeed. the stubdom create could move as a xl helper.
On the ocaml side reimplementing stubdom create is a trivial composition
of smaller libxl function (create/build/add devs) which are already bound.
--
Vincent
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 10:44 ` Vincent Hanquez
@ 2010-07-09 10:51 ` Tim Deegan
2010-07-09 10:59 ` Stefano Stabellini
2010-07-09 11:05 ` Vincent Hanquez
0 siblings, 2 replies; 21+ messages in thread
From: Tim Deegan @ 2010-07-09 10:51 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
At 11:44 +0100 on 09 Jul (1278675850), Vincent Hanquez wrote:
> On 09/07/10 09:17, Tim Deegan wrote:
> >> Is it necessary to pull the mechanism out along with the policy though?
> >>
> > Or, if we're taking some mechanism out, couldn't we take _all_ the
> > mechanism out?
>
> Which one do you have in minds ?
It looks like your patch leaves some "create a stubdom" functions in the
libxl API. I'd have thought libxl should either handle stubdoms
entirely or not at all. (Unless stubdom creation needs some low-level
grunge that will uglify the libxl API if it's exposed that far up - I
can't think of any except PRIV_FOR though).
> > The idea of a stub domain doesn't seem like one that
> > libxl necessarily needs to know about.
> >
> yes, indeed. the stubdom create could move as a xl helper.
> On the ocaml side reimplementing stubdom create is a trivial composition
> of smaller libxl function (create/build/add devs) which are already bound.
That sounds cleaner to me.
Cheers,
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 10:51 ` Tim Deegan
@ 2010-07-09 10:59 ` Stefano Stabellini
2010-07-09 14:44 ` Tim Deegan
2010-07-09 11:05 ` Vincent Hanquez
1 sibling, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2010-07-09 10:59 UTC (permalink / raw)
To: Tim Deegan; +Cc: Ian Campbell, Xen Devel, Vincent Hanquez, Stefano Stabellini
On Fri, 9 Jul 2010, Tim Deegan wrote:
> At 11:44 +0100 on 09 Jul (1278675850), Vincent Hanquez wrote:
> > On 09/07/10 09:17, Tim Deegan wrote:
> > >> Is it necessary to pull the mechanism out along with the policy though?
> > >>
> > > Or, if we're taking some mechanism out, couldn't we take _all_ the
> > > mechanism out?
> >
> > Which one do you have in minds ?
>
> It looks like your patch leaves some "create a stubdom" functions in the
> libxl API. I'd have thought libxl should either handle stubdoms
> entirely or not at all. (Unless stubdom creation needs some low-level
> grunge that will uglify the libxl API if it's exposed that far up - I
> can't think of any except PRIV_FOR though).
>
> > > The idea of a stub domain doesn't seem like one that
> > > libxl necessarily needs to know about.
> > >
> > yes, indeed. the stubdom create could move as a xl helper.
> > On the ocaml side reimplementing stubdom create is a trivial composition
> > of smaller libxl function (create/build/add devs) which are already bound.
>
> That sounds cleaner to me.
>
I am not so sure about this: a stubdom is not just a normal PV guest, it
also needs some special plumbings in xenstore.
By design libxl clients are not required to know about xenstore,
therefore we cannot have stubdoms entirely built by libxl clients.
I think that working around this would be worse than Vincent's current
patch series.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 10:59 ` Stefano Stabellini
@ 2010-07-09 14:44 ` Tim Deegan
0 siblings, 0 replies; 21+ messages in thread
From: Tim Deegan @ 2010-07-09 14:44 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Ian Campbell, Xen Devel, Vincent Hanquez
At 11:59 +0100 on 09 Jul (1278676763), Stefano Stabellini wrote:
> I am not so sure about this: a stubdom is not just a normal PV guest, it
> also needs some special plumbings in xenstore.
> By design libxl clients are not required to know about xenstore,
> therefore we cannot have stubdoms entirely built by libxl clients.
> I think that working around this would be worse than Vincent's current
> patch series.
Fair enough.
Tim.
--
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 10:51 ` Tim Deegan
2010-07-09 10:59 ` Stefano Stabellini
@ 2010-07-09 11:05 ` Vincent Hanquez
2010-07-09 17:04 ` Ian Jackson
1 sibling, 1 reply; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-09 11:05 UTC (permalink / raw)
To: Tim Deegan; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
On 09/07/10 11:51, Tim Deegan wrote:
> At 11:44 +0100 on 09 Jul (1278675850), Vincent Hanquez wrote:
>
>> On 09/07/10 09:17, Tim Deegan wrote:
>>
>>>> Is it necessary to pull the mechanism out along with the policy though?
>>>>
>>>>
>>> Or, if we're taking some mechanism out, couldn't we take _all_ the
>>> mechanism out?
>>>
>> Which one do you have in minds ?
>>
> It looks like your patch leaves some "create a stubdom" functions in the
> libxl API. I'd have thought libxl should either handle stubdoms
> entirely or not at all. (Unless stubdom creation needs some low-level
> grunge that will uglify the libxl API if it's exposed that far up - I
> can't think of any except PRIV_FOR though).
>
I think that either is fine from my point of view; as long as I don't
have to capture two very different semantics (starting a program |
starting a domain) in one call.
--
Vincent
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 11:05 ` Vincent Hanquez
@ 2010-07-09 17:04 ` Ian Jackson
2010-07-09 17:58 ` Stefano Stabellini
2010-07-11 22:14 ` Vincent Hanquez
0 siblings, 2 replies; 21+ messages in thread
From: Ian Jackson @ 2010-07-09 17:04 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: Tim Deegan, Ian Campbell, Xen Devel, Stefano Stabellini
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 0/3] libxl stubdom API cleanup"):
> I think that either is fine from my point of view; as long as I don't
> have to capture two very different semantics (starting a program |
> starting a domain) in one call.
I still disagree. I think it would be better to hide this distinction
as much as possible.
Your key motive seems to be some problem with the ocaml bindings.
Perhaps you could explain that in more detail ?
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 17:04 ` Ian Jackson
@ 2010-07-09 17:58 ` Stefano Stabellini
2010-07-12 16:46 ` Ian Jackson
2010-07-11 22:14 ` Vincent Hanquez
1 sibling, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2010-07-09 17:58 UTC (permalink / raw)
To: Ian Jackson
Cc: Devel, Stefano Stabellini, Tim Deegan, Xen, Vincent Hanquez,
Ian Campbell
On Fri, 9 Jul 2010, Ian Jackson wrote:
> Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 0/3] libxl stubdom API cleanup"):
> > I think that either is fine from my point of view; as long as I don't
> > have to capture two very different semantics (starting a program |
> > starting a domain) in one call.
>
> I still disagree. I think it would be better to hide this distinction
> as much as possible.
>
> Your key motive seems to be some problem with the ocaml bindings.
> Perhaps you could explain that in more detail ?
I think Vincent wanted a different API to make memory accounting easier.
What about extending the current create_device_model API with a
more explicit stubdom flag, and a way to return the stubdom domid to the
caller?
Also the caller should be able to know in advance the amount of memory
used for the stubdom: another libxl function could be added for that
purpose.
Would that interface be flexible enough for you?
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 17:58 ` Stefano Stabellini
@ 2010-07-12 16:46 ` Ian Jackson
0 siblings, 0 replies; 21+ messages in thread
From: Ian Jackson @ 2010-07-12 16:46 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Tim Deegan, Ian Campbell, Xen, Devel, Vincent Hanquez
--text follows this line--
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 0/3] libxl stubdom API cleanup"):
> I think Vincent wanted a different API to make memory accounting easier.
Right.
> What about extending the current create_device_model API with a
> more explicit stubdom flag, and a way to return the stubdom domid to the
> caller?
>
> Also the caller should be able to know in advance the amount of memory
> used for the stubdom: another libxl function could be added for that
> purpose.
>
> Would that interface be flexible enough for you?
That would address my concerns.
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] libxl stubdom API cleanup
2010-07-09 17:04 ` Ian Jackson
2010-07-09 17:58 ` Stefano Stabellini
@ 2010-07-11 22:14 ` Vincent Hanquez
1 sibling, 0 replies; 21+ messages in thread
From: Vincent Hanquez @ 2010-07-11 22:14 UTC (permalink / raw)
To: Ian Jackson; +Cc: Tim Deegan, Ian Campbell, Xen Devel, Stefano Stabellini
On 09/07/10 18:04, Ian Jackson wrote:
> Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 0/3] libxl stubdom API cleanup"):
>> I think that either is fine from my point of view; as long as I don't
>> have to capture two very different semantics (starting a program |
>> starting a domain) in one call.
>
> I still disagree. I think it would be better to hide this distinction
> as much as possible.
>
> Your key motive seems to be some problem with the ocaml bindings.
> Perhaps you could explain that in more detail ?
This has nothing to do with the ocaml bindings, but this has to do with
who is using those bindings (i.e. a fully featured toolstack).
--
Vincent
^ permalink raw reply [flat|nested] 21+ messages in thread