[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3] tools/libxl: Improve videoram setting
Il 14/01/2013 19:21, Stefano Stabellini ha scritto:
On Mon, 14 Jan 2013, Fabio Fantoni wrote:
Il 14/01/2013 13:49, Stefano Stabellini ha scritto:
On Fri, 11 Jan 2013, Fabio Fantoni wrote:
tools/libxl: Improve videoram setting
- If videoram setting is less than 8 mb shows error.
- Added videoram setting for qemu upstream with cirrus (added in qemu
1.3).
- Updated xl.cfg man.
Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxxxxx>
---
docs/man/xl.cfg.pod.5 | 14 +++++---------
tools/libxl/libxl_dm.c | 6 ++++++
tools/libxl/xl_cmdimpl.c | 10 ++++++++--
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index caba162..9c5cdcd 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -974,19 +974,15 @@ in the B<VFB_SPEC_STRING> for configuring virtual
frame
buffer devices
Sets the amount of RAM which the emulated video card will contain,
which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available.
The default amount of video ram for stdvga is 8MB which is sufficient
-for e.g. 1600x1200 at 32bpp.
+for e.g. 1600x1200 at 32bpp and videoram option is currently working
+only when using the qemu-xen-traditional device-model.
When using the emulated Cirrus graphics card (B<stdvga=0>)
the amount of video ram is fixed at 4MB which is sufficient
-for 1024x768 at 32 bpp.
-
-videoram option is currently only available when using the
-qemu-xen-traditional device-model. Upstream qemu-xen device-model
-currently does not support changing the amount of video memory for the
-emulated graphics device.
+for 1024x768 at 32 bpp and videoram option is currently working
+only when using the upstream qemu-xen device-model.
=item B<stdvga=BOOLEAN>
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index c036dc1..d719130 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -430,6 +430,12 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
break;
case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ if (b_info->video_memkb) {
+ flexarray_vappend(dm_args, "-global",
+ libxl__sprintf(gc, "vga.vram_size_mb=%d",
+ libxl__sizekb_to_mb(b_info->video_memkb)),
+ NULL);
+ }
break;
}
Looking at the QEMU code, it seems to be that there another variable
with the same name to define the videoram size in vga-pci.c:
static Property vga_pci_properties[] = {
DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
Is it possible that this code works even for std-vga out of the box?
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e964bf1..6e11c8d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -831,8 +831,14 @@ static void parse_config_data(const char
*config_source,
xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);
- if (!xlu_cfg_get_long (config, "videoram", &l, 0))
- b_info->video_memkb = l * 1024;
+ if (!xlu_cfg_get_long (config, "videoram", &l, 0)){
+ if ( l < 8 ){
+ fprintf(stderr, "ERROR: videoram must be at least 8 mb\n");
+ exit (1);
+ } else {
+ b_info->video_memkb = l * 1024;
+ }
+ }
As Ian pointed out, it would be better if you moved the error check in
libxl, probably somewhere towards the beginning of
initiate_domain_create.
-----
Nessun virus nel messaggio.
Controllato da AVG - www.avg.com
Versione: 2013.0.2890 / Database dei virus: 2638/6030 - Data di rilascio:
13/01/2013
About std-vga videoram it seems defined at fixed value and I found only the
cirrus and qxl being custimisable.
For example this is the commit that make cirrus custimisable:
http://xenbits.xen.org/gitweb/?p=staging/qemu-upstream-unstable.git;a=commit;h=19403a68fb8eaefb2e1245b6a8384d3a3ffa7ca0
I did a quick test and it seems that it should be possible to change the
amount of videoram for stdvga too using the same command line option,
however at the moment it just errors out.
Therefore I am OK with this patch only taking care of Cirrus for the moment.
About error check I added it only in this last version of patch.
Why wouldn't check it near other domU configuration parameters?
This seems more a sanity check to do before initiate starting domU.
I know that it might seem counterintuitive but we try to keep all the
significant informations about the platform within libxl. So if 8MB is
an important limit of the platform that libxl should know about it,
while xl should stay as dumb as possible.
I tried to move the check within libxl_create.c, at the point where it
sets video_memkb default value, but I couldn't make it exit in case of
failure. Also I'm not sure if the check should go there. Can you help me
to do it please?
Attachment:
smime.p7s
Description: Firma crittografica S/MIME
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
