anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 0/5] libxl: Some fixes related to Qemu upstream command lines.
From: Anthony PERARD <anthony.perard@citrix.com> This series comes with some fixes for hvm guest with Qemu upstream, and completes the function libxl_build_device_model_args_new to make XenPV guest works with the Qemu upstream (when it''s needed). Anthony PERARD (5): libxl: fix double free of ifname, when makes args for qemu. libxl: strdup disk path before put it in qemu args array. libxl: Specify the target ram size to Qemu (new) when calling it libxl: Makes libxl be able to call Qemu upstream for XenPV guest. libxl: Lists qdisk device in libxl_device_disk_list tools/libxl/libxl.c | 78 +++++++++++++++++++++++++++++++++++++--------- tools/libxl/libxl.idl | 1 + tools/libxl/xl_cmdimpl.c | 1 + 3 files changed, 65 insertions(+), 15 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 1/5] libxl: fix double free of ifname, when makes args for qemu.
From: Anthony PERARD <anthony.perard@citrix.com>
In libxl_build_device_model_args_new, vifs[i].ifname can be free two
times, by the gc, and by freeing the vifs structures. This patch avoids
this.
---
tools/libxl/libxl.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 33e5a2a..0feb93f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1341,14 +1341,18 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
char *smac = libxl__sprintf(gc,
"%02x:%02x:%02x:%02x:%02x:%02x",
vifs[i].mac[0], vifs[i].mac[1],
vifs[i].mac[2],
vifs[i].mac[3], vifs[i].mac[4],
vifs[i].mac[5]);
- if (!vifs[i].ifname)
- vifs[i].ifname = libxl__sprintf(gc, "tap%d.%d",
info->domid, vifs[i].devid);
+ char *ifname;
+ if (!vifs[i].ifname) {
+ ifname = libxl__sprintf(gc, "tap%d.%d",
info->domid, vifs[i].devid);
+ } else {
+ ifname = vifs[i].ifname;
+ }
flexarray_set(dm_args, num++, "-net");
flexarray_set(dm_args, num++, libxl__sprintf(gc,
"nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
flexarray_set(dm_args, num++, "-net");
flexarray_set(dm_args, num++, libxl__sprintf(gc,
"tap,vlan=%d,ifname=%s,script=no",
- vifs[i].devid, vifs[i].ifname));
+ vifs[i].devid, ifname));
ioemu_vifs++;
}
}
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 2/5] libxl: strdup disk path before put it in qemu args array.
From: Anthony PERARD <anthony.perard@citrix.com>
In libxl_build_device_model_args_new, the path to the disk image are
freeed before there was actually use to make the arguments list of Qemu.
The patch strdups it.
This patch also changes argv[0] of the device model.
Now, it is the conventional argv[0], so is value come from
info->device_model.
---
tools/libxl/libxl.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0feb93f..18ea8b1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1256,9 +1256,9 @@ static char ** libxl_build_device_model_args_new(libxl__gc
*gc,
if (!dm_args)
return NULL;
- flexarray_set(dm_args, num++, "qemu-system-xen");
- flexarray_set(dm_args, num++, "-xen-domid");
+ flexarray_set(dm_args, num++, libxl__strdup(gc, info->device_model));
+ flexarray_set(dm_args, num++, "-xen-domid");
flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d",
info->domid));
if (info->dom_name) {
@@ -1378,10 +1378,10 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
for (i; i < nb; i++) {
if ( disks[i].is_cdrom ) {
flexarray_set(dm_args, num++, "-cdrom");
- flexarray_set(dm_args, num++, disks[i].physpath);
- }else{
+ flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
+ } else {
flexarray_set(dm_args, num++, libxl__sprintf(gc, "-%s",
disks[i].virtpath));
- flexarray_set(dm_args, num++, disks[i].physpath);
+ flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
}
libxl_device_disk_destroy(&disks[i]);
}
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 3/5] libxl: Specify the target ram size to Qemu (new) when calling it
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds target_ram in device_model_info structure, to be used in
libxl_build_device_model_args_new. Qemu upstream needs to know about it.
---
tools/libxl/libxl.c | 4 ++++
tools/libxl/libxl.idl | 1 +
tools/libxl/xl_cmdimpl.c | 1 +
3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 18ea8b1..f4fb2c6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1374,6 +1374,10 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
else
flexarray_set(dm_args, num++, "xenfv");
+ /* RAM Size */
+ flexarray_set(dm_args, num++, "-m");
+ flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d",
info->target_ram));
+
disks = libxl_device_disk_list(libxl__gc_owner(gc), info->domid,
&nb);
for (i; i < nb; i++) {
if ( disks[i].is_cdrom ) {
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 8dd7749..89694b1 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -139,6 +139,7 @@ libxl_device_model_info =
Struct("device_model_info",[
("device_model", string),
("saved_state", string),
("type", libxl_qemu_machine_type),
+ ("target_ram", uint32),
("videoram", integer, False, "size of the
videoram in MB"),
("stdvga", bool, False, "stdvga
enabled or disabled"),
("vnc", bool, False, "vnc enabled
or disabled"),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5555319..897c5bc 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -359,6 +359,7 @@ static void init_dm_info(libxl_device_model_info *dm_info,
dm_info->dom_name = strdup(c_info->name);
dm_info->device_model = strdup("qemu-dm");
+ dm_info->target_ram = b_info->target_memkb / 1024;
dm_info->videoram = b_info->video_memkb / 1024;
dm_info->apic = b_info->u.hvm.apic;
dm_info->vcpus = b_info->max_vcpus;
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.
From: Anthony PERARD <anthony.perard@citrix.com>
In libxl_build_device_model_args_new:
- Adds -xen-attach options to the list of arguments to Qemu.
- Adds -vga xenfb options when vnc and sdl are not set.
- Remove disk list from the command line for XenPV as they will be
read from xenstore by Qemu.
---
tools/libxl/libxl.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f4fb2c6..57f7051 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1261,6 +1261,10 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
flexarray_set(dm_args, num++, "-xen-domid");
flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d",
info->domid));
+ if (info->type == XENPV) {
+ flexarray_set(dm_args, num++, "-xen-attach");
+ }
+
if (info->dom_name) {
flexarray_set(dm_args, num++, "-name");
flexarray_set(dm_args, num++, info->dom_name);
@@ -1292,6 +1296,12 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
if (info->sdl) {
flexarray_set(dm_args, num++, "-sdl");
}
+
+ if (info->type == XENPV && !info->nographic) {
+ flexarray_set(dm_args, num++, "-vga");
+ flexarray_set(dm_args, num++, "xenfb");
+ }
+
if (info->keymap) {
flexarray_set(dm_args, num++, "-k");
flexarray_set(dm_args, num++, info->keymap);
@@ -1378,16 +1388,18 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
flexarray_set(dm_args, num++, "-m");
flexarray_set(dm_args, num++, libxl__sprintf(gc, "%d",
info->target_ram));
- disks = libxl_device_disk_list(libxl__gc_owner(gc), info->domid,
&nb);
- for (i; i < nb; i++) {
- if ( disks[i].is_cdrom ) {
- flexarray_set(dm_args, num++, "-cdrom");
- flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
- } else {
- flexarray_set(dm_args, num++, libxl__sprintf(gc, "-%s",
disks[i].virtpath));
- flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
+ if (info->type == XENFV) {
+ disks = libxl_device_disk_list(libxl__gc_owner(gc), info->domid,
&nb);
+ for (i; i < nb; i++) {
+ if (disks[i].is_cdrom) {
+ flexarray_set(dm_args, num++, "-cdrom");
+ flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
+ } else {
+ flexarray_set(dm_args, num++, libxl__sprintf(gc,
"-%s", disks[i].virtpath));
+ flexarray_set(dm_args, num++, libxl__strdup(gc,
disks[i].physpath));
+ }
+ libxl_device_disk_destroy(&disks[i]);
}
- libxl_device_disk_destroy(&disks[i]);
}
free(disks);
flexarray_set(dm_args, num++, NULL);
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
anthony.perard@citrix.com
2010-Dec-13 13:15 UTC
[Xen-devel] [PATCH V2 5/5] libxl: Lists qdisk device in libxl_device_disk_list
From: Anthony PERARD <anthony.perard@citrix.com>
As libxl switch to qdisk when blktap isn''t available, this patch makes
libxl_device_disk_list also list qdisk device. So
libxl_build_device_model_args_new will be able to add qdisk device to
the command line options of Qemu.
---
tools/libxl/libxl.c | 30 +++++++++++++++++++++++++++++-
1 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 57f7051..d4bf734 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2492,7 +2492,7 @@ int libxl_device_vkb_hard_shutdown(libxl_ctx *ctx,
uint32_t domid)
libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int
*num)
{
libxl__gc gc = LIBXL_INIT_GC(ctx);
- char *be_path_tap, *be_path_vbd;
+ char *be_path_tap, *be_path_vbd, *be_path_qdisk;
libxl_device_disk *dend, *disks, *ret = NULL;
char **b, **l = NULL;
unsigned int numl, len;
@@ -2500,6 +2500,7 @@ libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx,
uint32_t domid, int *n
be_path_vbd = libxl__sprintf(&gc, "%s/backend/vbd/%d",
libxl__xs_get_dompath(&gc, 0), domid);
be_path_tap = libxl__sprintf(&gc, "%s/backend/tap/%d",
libxl__xs_get_dompath(&gc, 0), domid);
+ be_path_qdisk = libxl__sprintf(&gc, "%s/backend/qdisk/%d",
libxl__xs_get_dompath(&gc, 0), domid);
b = l = libxl__xs_directory(&gc, XBT_NULL, be_path_vbd, &numl);
if (l) {
@@ -2542,6 +2543,33 @@ libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx,
uint32_t domid, int *n
disks->is_cdrom = !strcmp(type, "cdrom");
}
}
+ b = l = libxl__xs_directory(&gc, XBT_NULL, be_path_qdisk, &numl);
+ if (l) {
+ ret = realloc(ret, sizeof(libxl_device_disk) * (*num + numl));
+ disks = ret + *num;
+ *num += numl;
+ for (dend = ret + *num; disks < dend; ++disks, ++l) {
+ char *physpath_tmp;
+ disks->backend_domid = 0;
+ disks->domid = domid;
+ physpath_tmp = xs_read(ctx->xsh, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/params", be_path_qdisk, *l), &len);
+ if (strchr(physpath_tmp, '':'')) {
+ disks->physpath = strdup(strchr(physpath_tmp) + 1);
+ free(physpath_tmp);
+ } else {
+ disks->physpath = physpath_tmp;
+ }
+ libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/type", be_path_qdisk, *l)),
&(disks->phystype));
+ disks->virtpath = xs_read(ctx->xsh, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/dev", be_path_qdisk, *l), &len);
+ disks->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/removable", be_path_qdisk, *l)));
+ if (!strcmp(libxl__xs_read(&gc, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/mode", be_path_qdisk, *l)),
"w"))
+ disks->readwrite = 1;
+ else
+ disks->readwrite = 0;
+ type = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/device-type", libxl__xs_read(&gc, XBT_NULL,
libxl__sprintf(&gc, "%s/%s/frontend", be_path_qdisk, *l))));
+ disks->is_cdrom = !strcmp(type, "cdrom");
+ }
+ }
libxl__free_all(&gc);
return ret;
}
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Dec-13 13:54 UTC
Re: [Xen-devel] [PATCH V2 3/5] libxl: Specify the target ram size to Qemu (new) when calling it
On Mon, 2010-12-13 at 13:15 +0000, anthony.perard@citrix.com wrote:> > + dm_info->target_ram = b_info->target_memkb / 1024; > dm_info->videoram = b_info->video_memkb / 1024;Both of these end up rounding down, is that desirable? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2010-Dec-13 14:14 UTC
Re: [Xen-devel] [PATCH V2 3/5] libxl: Specify the target ram size to Qemu (new) when calling it
On Mon, 13 Dec 2010, Ian Campbell wrote:> On Mon, 2010-12-13 at 13:15 +0000, anthony.perard@citrix.com wrote: > > > > + dm_info->target_ram = b_info->target_memkb / 1024; > > dm_info->videoram = b_info->video_memkb / 1024; > > Both of these end up rounding down, is that desirable?They are both multiplied by 1024 from the config file before they are stored in the b_info->*_memkb variables. So have them rounding down or up will not change anything. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Dec-13 14:32 UTC
Re: [Xen-devel] [PATCH V2 3/5] libxl: Specify the target ram size to Qemu (new) when calling it
On Mon, 2010-12-13 at 14:14 +0000, Anthony Perard wrote:> On Mon, 13 Dec 2010, Ian Campbell wrote: > > > On Mon, 2010-12-13 at 13:15 +0000, anthony.perard@citrix.com wrote: > > > > > > + dm_info->target_ram = b_info->target_memkb / 1024; > > > dm_info->videoram = b_info->video_memkb / 1024; > > > > Both of these end up rounding down, is that desirable? > > They are both multiplied by 1024 from the config file before they are > stored in the b_info->*_memkb variables. So have them rounding down or > up will not change anything.Fair enough. Makes me wonder if b_info->foo_memkb has the correct units. Also since libxl is supposed to be usable by other than xl relying on particular subtle behaviour like this seems unwise. This patch does make things any worse though I guess. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Dec-13 18:05 UTC
Re: [Xen-devel] [PATCH V2 3/5] libxl: Specify the target ram size to Qemu (new) when calling it
On Mon, 13 Dec 2010, Ian Campbell wrote:> On Mon, 2010-12-13 at 14:14 +0000, Anthony Perard wrote: > > On Mon, 13 Dec 2010, Ian Campbell wrote: > > > > > On Mon, 2010-12-13 at 13:15 +0000, anthony.perard@citrix.com wrote: > > > > > > > > + dm_info->target_ram = b_info->target_memkb / 1024; > > > > dm_info->videoram = b_info->video_memkb / 1024; > > > > > > Both of these end up rounding down, is that desirable? > > > > They are both multiplied by 1024 from the config file before they are > > stored in the b_info->*_memkb variables. So have them rounding down or > > up will not change anything. > > Fair enough. > > Makes me wonder if b_info->foo_memkb has the correct units. Also since > libxl is supposed to be usable by other than xl relying on particular > subtle behaviour like this seems unwise. This patch does make things any > worse though I guess.I think we should define a macro or a simple inline function to do kb to mb conversions, rounding up the result. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Dec-13 18:07 UTC
Re: [Xen-devel] [PATCH V2 5/5] libxl: Lists qdisk device in libxl_device_disk_list
On Mon, 13 Dec 2010, anthony.perard@citrix.com wrote:> From: Anthony PERARD <anthony.perard@citrix.com> > > As libxl switch to qdisk when blktap isn''t available, this patch makes > libxl_device_disk_list also list qdisk device. So > libxl_build_device_model_args_new will be able to add qdisk device to > the command line options of Qemu.This patch fails to compile: libxl.c:2553:66: error: macro "strchr" requires 2 arguments, but only 1 given cc1: warnings being treated as errors libxl.c: In function ‘libxl_device_disk_list’: libxl.c:2553: error: passing argument 1 of ‘strlen’ from incompatible pointer type /usr/include/string.h:397: note: expected ‘const char *’ but argument is of type ‘char * (*)(const char *, int)’ libxl.c:2553: error: passing argument 1 of ‘__strdup’ from incompatible pointer type /usr/include/bits/string2.h:1303: note: expected ‘const char *’ but argument is of type ‘char * (*)(const char *, int)’ I have applied patch 1, 2 and 4. Next time please add an explicit signed-off-by line to your patches. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel