[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 02/16] tools part of add vmware_hw to xl.cfg




On 09/11/14 07:23, George Dunlap wrote:
On Mon, Sep 8, 2014 at 2:15 PM, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
If non-zero then
   Force use of VMware's VGA in QEMU.

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
I think the title would be better "tools: Add vmware_hw support"

Ok.

---
  docs/man/xl.cfg.pod.5           |  6 ++++++
  tools/libxc/xc_domain_restore.c | 14 ++++++++++++++
  tools/libxc/xc_domain_save.c    | 11 +++++++++++
  tools/libxc/xg_save_restore.h   |  2 ++
  tools/libxl/libxl_create.c      |  4 +++-
  tools/libxl/libxl_dm.c          | 33 +++++++++++++++++++++------------
  tools/libxl/libxl_dom.c         |  2 ++
  tools/libxl/libxl_types.idl     |  1 +
  tools/libxl/xl_cmdimpl.c        |  2 ++
  9 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 517ae2f..7f7319a 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1147,6 +1147,12 @@ some other Operating Systems and in some circumstance 
can prevent
  Xen's own paravirtualisation interfaces for HVM guests from being
  used.

+=item B<vmware_hw=NUMBER>
+
+Turns on or off the exposure of VMware cpuid.  The number is the
+VMware's hardware version number, where 0 is off.  If on it also
+forces the use of VMware's VGA in QEMU.
You should also say what values are supported.

Here is what I have in v4:


=item B<vmware_hw=NUMBER>

Turns on or off the exposure of VMware cpuid.  The number is the
VMware's hardware version number, where 0 is off.  If not zero it
changes the default VGA to VMware's VGA.

The hardware version number (vmware_hw) come from VMware config files.

=over 4

In a .vmx it is virtualHW.version

In a .ovf it is part of the value of vssd:VirtualSystemType.
For vssd:VirtualSystemType == vmx-07, vmware_hw = 7.

=back


There is a lot in the different thread:

Message-ID: <54108DFB.8030804@xxxxxxxxxxxxx>
Date: Wed, 10 Sep 2014 13:44:27 -0400
User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:24.0) Gecko/20100101 
Thunderbird/24.6.0
To: Ian Campbell <Ian.Campbell@xxxxxxxxxx>, Don Slutz <dslutz@xxxxxxxxxxx>
CC: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Kevin
 Tian <kevin.tian@xxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Jun Nakajima
        <jun.nakajima@xxxxxxxxx>, Stefano Stabellini
        <stefano.stabellini@xxxxxxxxxxxxx>, Ian Jackson 
<ian.jackson@xxxxxxxxxxxxx>,
        Eddie Dong <eddie.dong@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxx>, "Aravind
 Gopalakrishnan" <Aravind.Gopalakrishnan@xxxxxxx>, Jan Beulich
        <jbeulich@xxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, 
"Suravee
 Suthikulpanit" <suravee.suthikulpanit@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH v2 1/3] Add vmware_hw to xl.cfg
References: <1409585629-25840-1-git-send-email-dslutz@xxxxxxxxxxx>
 <1409585629-25840-2-git-send-email-dslutz@xxxxxxxxxxx>
 <1410182256.3680.16.camel@xxxxxxxxxxxxxxxxxxxxxx>
 <540E00AB.1000501@xxxxxxxxxxxxx>
 <1410255568.8217.65.camel@xxxxxxxxxxxxxxxxxxxxxx>
 <540F32A0.2070609@xxxxxxxxxxxxx>
 <1410341443.8217.260.camel@xxxxxxxxxxxxxxxxxxxxxx>
In-Reply-To: <1410341443.8217.260.camel@xxxxxxxxxxxxxxxxxxxxxx>




diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 103cbca..c79274b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -542,19 +542,28 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,
              }
          }

-        switch (b_info->u.hvm.vga.kind) {
-        case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_append_pair(dm_args, "-device",
-                GCSPRINTF("VGA,vgamem_mb=%d",
-                libxl__sizekb_to_mb(b_info->video_memkb)));
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+        if (b_info->u.hvm.vmware_hw) {
              flexarray_append_pair(dm_args, "-device",
-                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
-                libxl__sizekb_to_mb(b_info->video_memkb)));
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_NONE:
-            break;
+                                  GCSPRINTF("vmware-svga,vgamem_mb=%d",
+                                            libxl__sizekb_to_mb(
+                                                b_info->video_memkb)));
+        } else {
+            switch (b_info->u.hvm.vga.kind) {
+            case LIBXL_VGA_INTERFACE_TYPE_STD:
+                flexarray_append_pair(dm_args, "-device",
+                                      GCSPRINTF("VGA,vgamem_mb=%d",
+                                                libxl__sizekb_to_mb(
+                                                    b_info->video_memkb)));
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+                flexarray_append_pair(dm_args, "-device",
+                                      GCSPRINTF("cirrus-vga,vgamem_mb=%d",
+                                                libxl__sizekb_to_mb(
+                                                    b_info->video_memkb)));
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_NONE:
+                break;
+            }
          }
So if we set vmware_hw, then we:
1. always add a vmware-svga device, regargless of whether vga has been requested
2. Ignore the vga parameter and only add vmware-svga, even if someone
may want something else?

I think at the libxl level, we should add a VMWARE interface type.
Then in xl_cmdimpl.c, we should:
* Add a vga="vmware" type in xl.cfg
* honor the vga= setting (perhaps with a warning if vmware_hw is true?)
* if vga is not set, and vmware_hw is true, default to vga=VMWARE

In v2 and v3 yes.  In v4 no.  What you are suggesting I have already coded.

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8a38077..c30341a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1033,6 +1033,8 @@ static void parse_config_data(const char *config_source,
          xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.acpi_s4, 0);
          xlu_cfg_get_defbool(config, "nx", &b_info->u.hvm.nx, 0);
          xlu_cfg_get_defbool(config, "viridian", &b_info->u.hvm.viridian, 0);
+        if (!xlu_cfg_get_long(config, "vmware_hw",  &l, 1))
+            b_info->u.hvm.vmware_hw = l;
          xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
          xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
This is a really minor thing, but is there a reason you put this in
the middle of a bunch of other get_defbool()s, instead of putting it
just after?  It makes the code just look a bit more ugly. :-)

Not a good one, I was just following where viridian was done. Will move it down a
few lines.

   -Don Slutz

  -George


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.