[PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[anthony.perard]

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.

It introduces also libxl__sizekb_to_mb to convert size from KB to MB by
rounding up the result.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.c       |    4 ++++
 tools/libxl/libxl.idl     |    1 +
 tools/libxl/libxl_utils.h |    4 ++++
 tools/libxl/xl_cmdimpl.c  |    3 ++-
 4 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index aa28c72..9dfd211 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1384,6 +1384,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));
+
     if (info->type == XENFV) {
         disks = libxl_device_disk_list(libxl__gc_owner(gc), info->domid, &nb);
         for (i; i < nb; i++) {
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/libxl_utils.h b/tools/libxl/libxl_utils.h
index 7846c42..940fecd 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -82,5 +82,9 @@ void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_reset(libxl_cpumap *cpumap, int cpu);
 #define libxl_for_each_cpu(var, map) for (var = 0; var < (map).size * 8; var++)
 
+static inline uint32_t libxl__sizekb_to_mb(uint32_t s) {
+    return (s + 1023) / 1024;
+}
+
 #endif
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5555319..3718a5a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -359,7 +359,8 @@ 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->videoram = b_info->video_memkb / 1024;
+    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;
-- 
1.7.1

[PATCH] libxl: Lists qdisk device in libxl_device_disk_list

[anthony.perard]

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.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 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 9dfd211..78dcf62 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

Re: [PATCH] libxl: Lists qdisk device in libxl_device_disk_list

[Ian Jackson]

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH] libxl: Lists qdisk device in libxl_device_disk_list"):
> 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.

Thanks for this contribution.  But what you have done is introduced
yet a third copy of a section of code which has been cloned-and-hacked
one already.

Can you please make a patch which breaks the common code out into a
separate function, or macro, or uses a loop, or something ?  I don't
much mind how it's done but to my mind a function is probably going to
be easiest.

Ian.

[PATCH V2 1/2] libxl: Factorize function libxl_device_disk_list

[anthony.perard]

From: Anthony PERARD <anthony.perard@citrix.com>

This patch adds function libxl_append_disk_list_of_type to get disks
parameter of one backend type.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.c |   99 ++++++++++++++++++++++++++-------------------------
 1 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 10d4527..79ad06c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2518,61 +2518,62 @@ int libxl_device_vkb_hard_shutdown(libxl_ctx *ctx, uint32_t domid)
     return ERROR_NI;
 }
 
-libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
+static unsigned int libxl_append_disk_list_of_type(libxl_ctx *ctx,
+                                                   uint32_t domid,
+                                                   const char *type,
+                                                   libxl_device_disk **disks,
+                                                   unsigned int *ndisks)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    char *be_path_tap, *be_path_vbd;
-    libxl_device_disk *dend, *disks, *ret = NULL;
-    char **b, **l = NULL;
-    unsigned int numl, len;
-    char *type;
-
-    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);
-
-    b = l = libxl__xs_directory(&gc, XBT_NULL, be_path_vbd, &numl);
-    if (l) {
-        ret = realloc(ret, sizeof(libxl_device_disk) * numl);
-        disks = ret;
-        *num = numl;
-        dend = ret + *num;
-        for (; disks < dend; ++disks, ++l) {
-            disks->backend_domid = 0;
-            disks->domid = domid;
-            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path_vbd, *l), &len);
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path_vbd, *l)), &(disks->phystype));
-            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path_vbd, *l), &len);
-            disks->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path_vbd, *l)));
-            if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path_vbd, *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_vbd, *l))));
-            disks->is_cdrom = !strcmp(type, "cdrom");
-        }
-    }
-    b = l = libxl__xs_directory(&gc, XBT_NULL, be_path_tap, &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) {
-            disks->backend_domid = 0;
-            disks->domid = domid;
-            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path_tap, *l), &len);
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path_tap, *l)), &(disks->phystype));
-            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path_tap, *l), &len);
-            disks->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path_tap, *l)));
-            if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path_tap, *l)), "w"))
-                disks->readwrite = 1;
+    char *be_path = NULL;
+    char **dir = NULL;
+    unsigned int n = 0, len = 0;
+    libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
+    char *physpath_tmp = NULL;
+
+    be_path = libxl__sprintf(&gc, "%s/backend/%s/%d",
+                             libxl__xs_get_dompath(&gc, 0), type, domid);
+    dir = libxl__xs_directory(&gc, XBT_NULL, be_path, &n);
+    if (dir) {
+        *disks = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
+        pdisk = *disks + *ndisks;
+        *ndisks += n;
+        pdisk_end = *disks + *ndisks;
+        for (; pdisk < pdisk_end; pdisk++, dir++) {
+            pdisk->backend_domid = 0;
+            pdisk->domid = domid;
+            physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
+            if (strchr(physpath_tmp, ':')) {
+                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                free(physpath_tmp);
+            } else {
+                pdisk->physpath = physpath_tmp;
+            }
+            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
+            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
+            if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
+                pdisk->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_tap, *l))));
-            disks->is_cdrom = !strcmp(type, "cdrom");
+                pdisk->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, *dir))));
+            pdisk->is_cdrom = !strcmp(type, "cdrom");
         }
     }
+
     libxl__free_all(&gc);
-    return ret;
+    return n;
+}
+
+libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
+{
+    libxl_device_disk *disks = NULL;
+    unsigned int ndisks = 0;
+
+    *num = libxl_append_disk_list_of_type(ctx, domid, "vbd", &disks, &ndisks);
+    *num += libxl_append_disk_list_of_type(ctx, domid, "tap", &disks, &ndisks);
+
+    return disks;
 }
 
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
-- 
1.7.1

Re: [PATCH V2 1/2] libxl: Factorize function libxl_device_disk_list

[Ian Jackson]

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH V2 1/2] libxl: Factorize function libxl_device_disk_list"):
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> This patch adds function libxl_append_disk_list_of_type to get disks
> parameter of one backend type.

Great, thanks.

The creation and destruction of a private gc in the internal function
is perhaps unusual but in this case it doesn't lead to a bug, and yes
the outer function doesn't need a gc.

Applied.

Ian.

[PATCH V2 2/2] libxl: Lists qdisk device in libxl_device_disk_list

[anthony.perard]

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.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 79ad06c..fa42d98 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2572,6 +2572,7 @@ libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *n
 
     *num = libxl_append_disk_list_of_type(ctx, domid, "vbd", &disks, &ndisks);
     *num += libxl_append_disk_list_of_type(ctx, domid, "tap", &disks, &ndisks);
+    *num += libxl_append_disk_list_of_type(ctx, domid, "qdisk", &disks, &ndisks);
 
     return disks;
 }
-- 
1.7.1

Re: [PATCH V2 2/2] libxl: Lists qdisk device in libxl_device_disk_list

[Ian Jackson]

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH V2 2/2] libxl: Lists qdisk device in libxl_device_disk_list"):
> 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.

Applied, thanks.

Ian.

Re: [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[Ian Jackson]

anthony.perard@citrix.com writes ("[Xen-devel] [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it"):
> 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.

I have applied this patch, thanks.

I'm just a little concerned, though: can you explain _why_ qemu
upstream needs to know the target ram size ?

Ian.

Re: [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[Keir Fraser]

On 06/01/2011 14:28, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

> anthony.perard@citrix.com writes ("[Xen-devel] [PATCH] libxl: Specify the
> target ram size to Qemu (new) when calling it"):
>> 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.
> 
> I have applied this patch, thanks.
> 
> I'm just a little concerned, though: can you explain _why_ qemu
> upstream needs to know the target ram size ?

Qemu used to be responsible for initialising BIOS info tables, including
memory size information such as e820, in lieu of the virtual BIOS doing it.
I ripped all that out from our diverged tree, and put it in
hvmloader/rombios, but maybe they still do it in upstream qemu. I think it
would be harmless if so, since hvmloader/rombios will overwrite/ignore.

 -- Keir

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

Re: [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[Anthony PERARD]

On Thu, 6 Jan 2011, Keir Fraser wrote:

> On 06/01/2011 14:28, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:
>
> > anthony.perard@citrix.com writes ("[Xen-devel] [PATCH] libxl: Specify the
> > target ram size to Qemu (new) when calling it"):
> >> 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.
> >
> > I have applied this patch, thanks.
> >
> > I'm just a little concerned, though: can you explain _why_ qemu
> > upstream needs to know the target ram size ?
>
> Qemu used to be responsible for initialising BIOS info tables, including
> memory size information such as e820, in lieu of the virtual BIOS doing it.
> I ripped all that out from our diverged tree, and put it in
> hvmloader/rombios, but maybe they still do it in upstream qemu. I think it
> would be harmless if so, since hvmloader/rombios will overwrite/ignore.

There are another reason, qemu "allocate" the memory for the vga at a
dynamic address. In qemu-xen, this address is static. So qemu need to
know the size of the ram to put the memory of the vga after the ram.
Qemu do the same for other memory like the rom of a nic.

Information about the ram is put in the struct RAMBlock of qemu.

-- 
Anthony PERARD

Re: [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it

[Ian Jackson]

Anthony Perard writes ("Re: [Xen-devel] [PATCH] libxl: Specify the target ram size to Qemu (new) when calling it"):
> On Thu, 6 Jan 2011, Keir Fraser wrote:
> > Qemu used to be responsible for initialising BIOS info tables, including
> > memory size information such as e820, in lieu of the virtual BIOS doing it.
> > I ripped all that out from our diverged tree, and put it in
> > hvmloader/rombios, but maybe they still do it in upstream qemu. I think it
> > would be harmless if so, since hvmloader/rombios will overwrite/ignore.
> 
> There are another reason, qemu "allocate" the memory for the vga at a
> dynamic address. In qemu-xen, this address is static. So qemu need to
> know the size of the ram to put the memory of the vga after the ram.
> Qemu do the same for other memory like the rom of a nic.
> 
> Information about the ram is put in the struct RAMBlock of qemu.

Right.  OK, I'll stop worrying :-).

Ian.