Gianni Tedesco
2011-Jan-11 11:00 UTC
[Xen-devel] [PATCH]: xl: move domain struct init functions to libxl
This allows libxl users to get some sane default values for this complex
set of structures. This is purely code movement and there are no
functional changes.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r feb198f3c97f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Mon Jan 10 16:03:39 2011 +0000
+++ b/tools/libxl/libxl.h Tue Jan 11 10:53:36 2011 +0000
@@ -280,6 +280,9 @@ int libxl_ctx_set_log(libxl_ctx *ctx, xe
int libxl_ctx_postfork(libxl_ctx *ctx);
/* domain related functions */
+void libxl_init_create_info(libxl_domain_create_info *c_info);
+void libxl_init_build_info(libxl_domain_build_info *b_info,
libxl_domain_create_info *c_info);
+void libxl_init_dm_info(libxl_device_model_info *dm_info,
libxl_domain_create_info *c_info, libxl_domain_build_info *b_info);
typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid);
int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
diff -r feb198f3c97f tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Jan 10 16:03:39 2011 +0000
+++ b/tools/libxl/libxl_create.c Tue Jan 11 10:53:36 2011 +0000
@@ -60,6 +60,75 @@ void libxl_domain_config_destroy(libxl_d
libxl_device_model_info_destroy(&d_config->dm_info);
}
+void libxl_init_create_info(libxl_domain_create_info *c_info)
+{
+ memset(c_info, ''\0'', sizeof(*c_info));
+ c_info->xsdata = NULL;
+ c_info->platformdata = NULL;
+ c_info->hap = 1;
+ c_info->hvm = 1;
+ c_info->oos = 1;
+ c_info->ssidref = 0;
+ c_info->poolid = 0;
+}
+
+void libxl_init_build_info(libxl_domain_build_info *b_info,
libxl_domain_create_info *c_info)
+{
+ memset(b_info, ''\0'', sizeof(*b_info));
+ b_info->max_vcpus = 1;
+ b_info->max_memkb = 32 * 1024;
+ b_info->target_memkb = b_info->max_memkb;
+ b_info->disable_migrate = 0;
+ b_info->cpuid = NULL;
+ b_info->shadow_memkb = 0;
+ if (c_info->hvm) {
+ b_info->video_memkb = 8 * 1024;
+ b_info->kernel.path = strdup("hvmloader");
+ b_info->hvm = 1;
+ b_info->u.hvm.pae = 1;
+ b_info->u.hvm.apic = 1;
+ b_info->u.hvm.acpi = 1;
+ b_info->u.hvm.nx = 1;
+ b_info->u.hvm.viridian = 0;
+ b_info->u.hvm.hpet = 1;
+ b_info->u.hvm.vpt_align = 1;
+ b_info->u.hvm.timer_mode = 1;
+ } else {
+ b_info->u.pv.slack_memkb = 8 * 1024;
+ }
+}
+
+void libxl_init_dm_info(libxl_device_model_info *dm_info,
+ libxl_domain_create_info *c_info, libxl_domain_build_info *b_info)
+{
+ memset(dm_info, ''\0'', sizeof(*dm_info));
+
+ libxl_uuid_generate(&dm_info->uuid);
+
+ dm_info->dom_name = strdup(c_info->name);
+ dm_info->device_model = strdup("qemu-dm");
+ dm_info->target_ram = libxl__sizekb_to_mb(b_info->target_memkb);
+ dm_info->videoram = libxl__sizekb_to_mb(b_info->video_memkb);
+ dm_info->apic = b_info->u.hvm.apic;
+ dm_info->vcpus = b_info->max_vcpus;
+ dm_info->vcpu_avail = b_info->cur_vcpus;
+
+ dm_info->stdvga = 0;
+ dm_info->vnc = 1;
+ dm_info->vnclisten = strdup("127.0.0.1");
+ dm_info->vncdisplay = 0;
+ dm_info->vncunused = 1;
+ dm_info->keymap = NULL;
+ dm_info->sdl = 0;
+ dm_info->opengl = 0;
+ dm_info->nographic = 0;
+ dm_info->serial = NULL;
+ dm_info->boot = strdup("cda");
+ dm_info->usb = 0;
+ dm_info->usbdevice = NULL;
+ dm_info->xen_platform_pci = 1;
+}
+
static int init_console_info(libxl_device_console *console, int dev_num,
libxl_domain_build_state *state)
{
memset(console, 0x00, sizeof(libxl_device_console));
diff -r feb198f3c97f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Jan 10 16:03:39 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c Tue Jan 11 10:53:36 2011 +0000
@@ -249,75 +249,6 @@ static void dolog(const char *file, int
libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
}
-static void init_create_info(libxl_domain_create_info *c_info)
-{
- memset(c_info, ''\0'', sizeof(*c_info));
- c_info->xsdata = NULL;
- c_info->platformdata = NULL;
- c_info->hap = 1;
- c_info->hvm = 1;
- c_info->oos = 1;
- c_info->ssidref = 0;
- c_info->poolid = 0;
-}
-
-static void init_build_info(libxl_domain_build_info *b_info,
libxl_domain_create_info *c_info)
-{
- memset(b_info, ''\0'', sizeof(*b_info));
- b_info->max_vcpus = 1;
- b_info->max_memkb = 32 * 1024;
- b_info->target_memkb = b_info->max_memkb;
- b_info->disable_migrate = 0;
- b_info->cpuid = NULL;
- b_info->shadow_memkb = 0;
- if (c_info->hvm) {
- b_info->video_memkb = 8 * 1024;
- b_info->kernel.path = strdup("hvmloader");
- b_info->hvm = 1;
- b_info->u.hvm.pae = 1;
- b_info->u.hvm.apic = 1;
- b_info->u.hvm.acpi = 1;
- b_info->u.hvm.nx = 1;
- b_info->u.hvm.viridian = 0;
- b_info->u.hvm.hpet = 1;
- b_info->u.hvm.vpt_align = 1;
- b_info->u.hvm.timer_mode = 1;
- } else {
- b_info->u.pv.slack_memkb = 8 * 1024;
- }
-}
-
-static void init_dm_info(libxl_device_model_info *dm_info,
- libxl_domain_create_info *c_info, libxl_domain_build_info *b_info)
-{
- memset(dm_info, ''\0'', sizeof(*dm_info));
-
- libxl_uuid_generate(&dm_info->uuid);
-
- dm_info->dom_name = strdup(c_info->name);
- dm_info->device_model = strdup("qemu-dm");
- dm_info->target_ram = libxl__sizekb_to_mb(b_info->target_memkb);
- dm_info->videoram = libxl__sizekb_to_mb(b_info->video_memkb);
- dm_info->apic = b_info->u.hvm.apic;
- dm_info->vcpus = b_info->max_vcpus;
- dm_info->vcpu_avail = b_info->cur_vcpus;
-
- dm_info->stdvga = 0;
- dm_info->vnc = 1;
- dm_info->vnclisten = strdup("127.0.0.1");
- dm_info->vncdisplay = 0;
- dm_info->vncunused = 1;
- dm_info->keymap = NULL;
- dm_info->sdl = 0;
- dm_info->opengl = 0;
- dm_info->nographic = 0;
- dm_info->serial = NULL;
- dm_info->boot = strdup("cda");
- dm_info->usb = 0;
- dm_info->usbdevice = NULL;
- dm_info->xen_platform_pci = 1;
-}
-
static void init_nic_info(libxl_device_nic *nic_info, int devnum)
{
const uint8_t *r;
@@ -726,7 +657,7 @@ static void parse_config_data(const char
exit(1);
}
- init_create_info(c_info);
+ libxl_init_create_info(c_info);
c_info->hvm = 0;
if (!xlu_cfg_get_string (config, "builder", &buf) &&
@@ -761,7 +692,7 @@ static void parse_config_data(const char
exit(1);
}
- init_build_info(b_info, c_info);
+ libxl_init_build_info(b_info, c_info);
/* the following is the actual config parsing with overriding values in the
structures */
if (!xlu_cfg_get_long (config, "vcpus", &l)) {
@@ -1187,7 +1118,7 @@ skip_vfb:
if (c_info->hvm == 1) {
/* init dm from c and b */
- init_dm_info(dm_info, c_info, b_info);
+ libxl_init_dm_info(dm_info, c_info, b_info);
/* then process config related to dm */
xlu_cfg_replace_string (config, "device_model",
&dm_info->device_model);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jan-11 12:53 UTC
[Xen-devel] Re: [PATCH]: xl: move domain struct init functions to libxl
On Tue, 11 Jan 2011, Gianni Tedesco wrote:> This allows libxl users to get some sane default values for this complex > set of structures. This is purely code movement and there are no > functional changes. > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com> > > > diff -r feb198f3c97f tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Mon Jan 10 16:03:39 2011 +0000 > +++ b/tools/libxl/libxl.h Tue Jan 11 10:53:36 2011 +0000 > @@ -280,6 +280,9 @@ int libxl_ctx_set_log(libxl_ctx *ctx, xe > int libxl_ctx_postfork(libxl_ctx *ctx); > > /* domain related functions */ > +void libxl_init_create_info(libxl_domain_create_info *c_info); > +void libxl_init_build_info(libxl_domain_build_info *b_info, libxl_domain_create_info *c_info); > +void libxl_init_dm_info(libxl_device_model_info *dm_info, libxl_domain_create_info *c_info, libxl_domain_build_info *b_info); > typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv); > int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid); > int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd); > diff -r feb198f3c97f tools/libxl/libxl_create.cWhat about init_nic_info, init_net2_info, init_vfb_info, init_vkb_info and init_console_info? Wouldn''t make sense to have a libxl_init_domain_config? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2011-Jan-11 14:24 UTC
[Xen-devel] Re: [PATCH]: xl: move domain struct init functions to libxl
On Tue, 2011-01-11 at 12:53 +0000, Stefano Stabellini wrote:> On Tue, 11 Jan 2011, Gianni Tedesco wrote: > > This allows libxl users to get some sane default values for this complex > > set of structures. This is purely code movement and there are no > > functional changes. > > > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com> > > > > > > diff -r feb198f3c97f tools/libxl/libxl.h > > --- a/tools/libxl/libxl.h Mon Jan 10 16:03:39 2011 +0000 > > +++ b/tools/libxl/libxl.h Tue Jan 11 10:53:36 2011 +0000 > > @@ -280,6 +280,9 @@ int libxl_ctx_set_log(libxl_ctx *ctx, xe > > int libxl_ctx_postfork(libxl_ctx *ctx); > > > > /* domain related functions */ > > +void libxl_init_create_info(libxl_domain_create_info *c_info); > > +void libxl_init_build_info(libxl_domain_build_info *b_info, libxl_domain_create_info *c_info); > > +void libxl_init_dm_info(libxl_device_model_info *dm_info, libxl_domain_create_info *c_info, libxl_domain_build_info *b_info); > > typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv); > > int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid); > > int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd); > > diff -r feb198f3c97f tools/libxl/libxl_create.c > > > What about init_nic_info, init_net2_info, init_vfb_info, init_vkb_info > and init_console_info? > Wouldn''t make sense to have a libxl_init_domain_configYeah good point, I can re-spin to include the various device info''s. Not sure about an init_domain_config, I don''t see how it would work, currently we have: libxl_domain_config x; init_create_info(&x.c_info); // do stuff to c_info init_build_info(&x.b_info, &x.c_info); // do stuff to b_info init_dm_info(&x.dm_info, &x.c_info, &x.b_info); // do stuff to dm_info for each device { init device; do stuff to device; } You could init them all in one functions but it breaks IMO the main purpose of these functions which is that when you ''do stuff'' to b_info then init_dm_info() depends on that to set the sane defaults (eg. for vidmem). So either we would have every field as a parameter to this function or add callbacks for the ''do stuff'' bits which seems a very rigid interface. I think we should wait for 4.2 before we try to make any more-sweeping changes in this area. Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jan-11 14:39 UTC
[Xen-devel] Re: [PATCH]: xl: move domain struct init functions to libxl
On Tue, 11 Jan 2011, Gianni Tedesco wrote:> On Tue, 2011-01-11 at 12:53 +0000, Stefano Stabellini wrote: > > On Tue, 11 Jan 2011, Gianni Tedesco wrote: > > > This allows libxl users to get some sane default values for this complex > > > set of structures. This is purely code movement and there are no > > > functional changes. > > > > > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com> > > > > > > > > > diff -r feb198f3c97f tools/libxl/libxl.h > > > --- a/tools/libxl/libxl.h Mon Jan 10 16:03:39 2011 +0000 > > > +++ b/tools/libxl/libxl.h Tue Jan 11 10:53:36 2011 +0000 > > > @@ -280,6 +280,9 @@ int libxl_ctx_set_log(libxl_ctx *ctx, xe > > > int libxl_ctx_postfork(libxl_ctx *ctx); > > > > > > /* domain related functions */ > > > +void libxl_init_create_info(libxl_domain_create_info *c_info); > > > +void libxl_init_build_info(libxl_domain_build_info *b_info, libxl_domain_create_info *c_info); > > > +void libxl_init_dm_info(libxl_device_model_info *dm_info, libxl_domain_create_info *c_info, libxl_domain_build_info *b_info); > > > typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv); > > > int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid); > > > int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd); > > > diff -r feb198f3c97f tools/libxl/libxl_create.c > > > > > > What about init_nic_info, init_net2_info, init_vfb_info, init_vkb_info > > and init_console_info? > > Wouldn''t make sense to have a libxl_init_domain_config > > Yeah good point, I can re-spin to include the various device info''s. > > Not sure about an init_domain_config, I don''t see how it would work, > currently we have: > > libxl_domain_config x; > init_create_info(&x.c_info); > // do stuff to c_info > init_build_info(&x.b_info, &x.c_info); > // do stuff to b_info > init_dm_info(&x.dm_info, &x.c_info, &x.b_info); > // do stuff to dm_info > for each device { init device; do stuff to device; } > > You could init them all in one functions but it breaks IMO the main > purpose of these functions which is that when you ''do stuff'' to b_info > then init_dm_info() depends on that to set the sane defaults (eg. for > vidmem). >No I mean instead of having: libxl_init_nic_info, libxl_init_net2_info, etc. we would have a single libxl_init_domain_config that does it all. However we cannot actually do that because we don''t know exactly the number of devices of each kind we are going to have. So forget about libxl_init_domain_config. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel