thr3ads.net - Xen devel - [Xen-devel] [PATCH 0/5] libxl: Some fixes related to Qemu upstream command lines. [Dec 2010]

If this information is useful, please help other people find it:
Share via:

anthony.perard@citrix.com

2010-Dec-09 19:42 UTC

[Xen-devel] [PATCH 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(-)

-- 
Anthony PERARD

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

anthony.perard@citrix.com

2010-Dec-09 19:42 UTC

head link

[Xen-devel] [PATCH 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-09 19:42 UTC

head link

[Xen-devel] [PATCH 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.
---
 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..0f6e632 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++, "qemu-dm");
 
+    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-09 19:42 UTC

head link

[Xen-devel] [PATCH 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 0f6e632..0ddeac0 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-09 19:42 UTC

head link

[Xen-devel] [PATCH 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 0ddeac0..c8fe82c 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-09 19:42 UTC

head link

[Xen-devel] [PATCH 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 c8fe82c..c79cca7 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-10 09:09 UTC

head link

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

On Thu, 2010-12-09 at 19:42 +0000, anthony.perard@citrix.com
wrote:> 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.
> ---
>  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..0f6e632 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++, "qemu-dm");
This bit doesn''t seem to correspond to the patch description, was the
change deliberate or did it slip in by mistake?

(my kingdom for flexarray_append(...))

Ian.



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

Anthony PERARD

2010-Dec-10 12:29 UTC

head link

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

On Fri, 10 Dec 2010, Ian Campbell wrote:
> On Thu, 2010-12-09 at 19:42 +0000, anthony.perard@citrix.com wrote:
> > 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.
> > ---
> >  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..0f6e632 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++, "qemu-dm");
>
> This bit doesn''t seem to correspond to the patch description, was
the
> change deliberate or did it slip in by mistake?
This change was deliberate. As this hunk don''t really change anything,
I
just don''t explain it in the commit message. But I should have said the
following in the description.

Also it changes argv[0] of the device model from "qemu-system-xen" to
"qemu-dm".

I will resend the patch.
> (my kingdom for flexarray_append(...))
It will be too easy with this functions :)

-- 
Anthony PERARD

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

anthony.perard@citrix.com

2010-Dec-10 12:31 UTC

head link

[Xen-devel] [PATCH 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.

Also it changes argv[0] of the device model from "qemu-system-xen" to
"qemu-dm".

---
 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..0f6e632 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++, "qemu-dm");
 
+    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

Ian Campbell

2010-Dec-10 14:49 UTC

head link

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

On Fri, 2010-12-10 at 12:29 +0000, Anthony Perard wrote:> On Fri, 10 Dec 2010, Ian Campbell wrote:
> 
> > On Thu, 2010-12-09 at 19:42 +0000, anthony.perard@citrix.com wrote:
> > >
> > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > index 0feb93f..0f6e632 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++, "qemu-dm");
> >
> > This bit doesn''t seem to correspond to the patch description,
was the
> > change deliberate or did it slip in by mistake?
> 
> This change was deliberate. As this hunk don''t really change
anything, I
> just don''t explain it in the commit message. But I should have
said the
> following in the description.
> 
> Also it changes argv[0] of the device model from
"qemu-system-xen" to
> "qemu-dm".
In the non-stubdom case convention would be for argv[0] to contain
info->device_model (or is it "libxl__abs_path(&gc,
info->device_model,
libxl_libexec_path()),"? eurk).

In the stubdom case I guess it might as well be whatever we like.

Ian.


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

Anthony PERARD

2010-Dec-10 15:38 UTC

head link

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

On Fri, 10 Dec 2010, Ian Campbell wrote:
> On Fri, 2010-12-10 at 12:29 +0000, Anthony Perard wrote:
> > On Fri, 10 Dec 2010, Ian Campbell wrote:
> >
> > > On Thu, 2010-12-09 at 19:42 +0000, anthony.perard@citrix.com
wrote:
> > > >
> > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > index 0feb93f..0f6e632 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++, "qemu-dm");
> > >
> > > This bit doesn''t seem to correspond to the patch
description, was the
> > > change deliberate or did it slip in by mistake?
> >
> > This change was deliberate. As this hunk don''t really change
anything, I
> > just don''t explain it in the commit message. But I should
have said the
> > following in the description.
> >
> > Also it changes argv[0] of the device model from
"qemu-system-xen" to
> > "qemu-dm".
>
> In the non-stubdom case convention would be for argv[0] to contain
> info->device_model (or is it "libxl__abs_path(&gc,
info->device_model,
> libxl_libexec_path()),"? eurk).
Yes, this will be a little bit better, with info->device_model, like a
shell would do.
> In the stubdom case I guess it might as well be whatever we like.
-- 
Anthony PERARD

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

anthony.perard@citrix.com

2010-Dec-10 16:59 UTC

head link

[Xen-devel] [PATCH v1.3 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.

Also it changes argv[0] of the device model from "qemu-system-xen" to
"qemu-dm".

---
 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..0f6e632 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

Ian Jackson

2010-Dec-10 17:42 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
Is that understood by qemu-xen ?

Ian.

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

Anthony PERARD

2010-Dec-10 18:12 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

On Fri, 10 Dec 2010, Ian Jackson wrote:
> anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
>
> Is that understood by qemu-xen ?
Not really, it is understood by Qemu only when we have this:
#if defined(CONFIG_XEN) && !defined(CONFIG_DM).

And even in this case, it will do nothing because the variable that
receives the option is write only, it is never read.

So the answer is no, it is not understood by Qemu.

-- 
Anthony PERARD

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

Anthony PERARD

2010-Dec-10 18:14 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

On Fri, 10 Dec 2010, Anthony PERARD wrote:
> On Fri, 10 Dec 2010, Ian Jackson wrote:
>
> > anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
> >
> > Is that understood by qemu-xen ?
> Not really, it is understood by Qemu only when we have this:
> #if defined(CONFIG_XEN) && !defined(CONFIG_DM).
>
> And even in this case, it will do nothing because the variable that
> receives the option is write only, it is never read.
>
> So the answer is no, it is not understood by Qemu.
You will read qemux-xen, here ...

-- 
Anthony PERARD

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

Ian Jackson

2010-Dec-13 18:19 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Anthony PERARD writes ("Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be
able to call Qemu upstream for XenPV guest."):> On Fri, 10 Dec 2010, Ian Jackson wrote:
> 
> > anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
> >
> > Is that understood by qemu-xen ?
> 
> Not really, it is understood by Qemu only when we have this:
> #if defined(CONFIG_XEN) && !defined(CONFIG_DM).
> 
> And even in this case, it will do nothing because the variable that
> receives the option is write only, it is never read.
> 
> So the answer is no, it is not understood by Qemu.
Your reply seems confusing to me.  Are we talking about the same
things ?

By "qemu-xen" I mean the old qemu tree which we are still using for
xen-unstable.  By "understood" I mean that the program will accept the
option.  You write "Qemu" but by "Qemu" I would normally
understand
the current upstream qemu (presumably with your Xen patch series).

If the option is not understood by qemu-xen then current xen-unstable
will break if we apply your patch, surely ?

Ian.

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

Anthony PERARD

2010-Dec-13 18:43 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

On Mon, 13 Dec 2010, Ian Jackson wrote:
> Anthony PERARD writes ("Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl
be able to call Qemu upstream for XenPV guest."):
> > On Fri, 10 Dec 2010, Ian Jackson wrote:
> >
> > > anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
> > >
> > > Is that understood by qemu-xen ?
> >
> > Not really, it is understood by Qemu only when we have this:
> > #if defined(CONFIG_XEN) && !defined(CONFIG_DM).
> >
> > And even in this case, it will do nothing because the variable that
> > receives the option is write only, it is never read.
> >
> > So the answer is no, it is not understood by Qemu.
>
> Your reply seems confusing to me.  Are we talking about the same
> things ?
Sorry, I make a typo in my mail. I was speaking about qemu-xen in my
reply, and not qemu upstream.
> By "qemu-xen" I mean the old qemu tree which we are still using
for
> xen-unstable.  By "understood" I mean that the program will
accept the
> option.  You write "Qemu" but by "Qemu" I would
normally understand
> the current upstream qemu (presumably with your Xen patch series).
>
> If the option is not understood by qemu-xen then current xen-unstable
> will break if we apply your patch, surely ?
No, because the arguments for qemu-xen are not make by the same
function. There are two functions that do almost the same thing:
  - libxl_build_device_model_args_old
      this function make argument for qemu-xen, the one in xen-unstable.
  - libxl_build_device_model_args_new
      and this one make arguments for qemu (upstream).

-- 
Anthony PERARD

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

Ian Jackson

2010-Dec-14 17:08 UTC

head link

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Anthony PERARD writes ("Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be
able to call Qemu upstream for XenPV guest."):> No, because the arguments for qemu-xen are not make by the same
> function. There are two functions that do almost the same thing:
>   - libxl_build_device_model_args_old
>       this function make argument for qemu-xen, the one in xen-unstable.
>   - libxl_build_device_model_args_new
>       and this one make arguments for qemu (upstream).
Ah, thanks, I hadn''t spotted that.

Ian.

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

Ian Jackson

2010-Dec-14 19:26 UTC

head link

Re: [Xen-devel] [PATCH 5/5] libxl: Lists qdisk device in libxl_device_disk_list

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH 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.
Did you compile this ?

libxl.c:2559: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:2559: error: passing argument 1 of ''strlen'' from
incompatible pointer type
libxl.c:2559: error: passing argument 1 of ''__strdup'' from
incompatible pointer type
make: *** [libxl.o] Error 1

Ian.

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

Xen devel - Dec 2010 - [PATCH 0/5] libxl: Some fixes related to Qemu upstream command lines.

[Xen-devel] [PATCH 0/5] libxl: Some fixes related to Qemu upstream command lines.

[Xen-devel] [PATCH 1/5] libxl: fix double free of ifname, when makes args for qemu.

[Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

[Xen-devel] [PATCH 3/5] libxl: Specify the target ram size to Qemu (new) when calling it

[Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

[Xen-devel] [PATCH 5/5] libxl: Lists qdisk device in libxl_device_disk_list

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

[Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

Re: [Xen-devel] [PATCH 2/5] libxl: strdup disk path before put it in qemu args array.

[Xen-devel] [PATCH v1.3 2/5] libxl: strdup disk path before put it in qemu args array.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 4/5] libxl: Makes libxl be able to call Qemu upstream for XenPV guest.

Re: [Xen-devel] [PATCH 5/5] libxl: Lists qdisk device in libxl_device_disk_list