From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53878) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrAbz-0003CT-MB for qemu-devel@nongnu.org; Wed, 19 Nov 2014 14:08:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrAbt-0003Cj-Hg for qemu-devel@nongnu.org; Wed, 19 Nov 2014 14:08:19 -0500 Received: from omzsmtpe04.verizonbusiness.com ([199.249.25.207]:55936) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrAbt-0003CX-90 for qemu-devel@nongnu.org; Wed, 19 Nov 2014 14:08:13 -0500 From: Don Slutz Message-ID: <546CEA98.40505@terremark.com> Date: Wed, 19 Nov 2014 14:08:08 -0500 MIME-Version: 1.0 References: <1416418257-10166-1-git-send-email-dslutz@verizon.com> <546CD4F1.4040602@redhat.com> <546CDC46.8050603@terremark.com> <546CDCAD.9030800@redhat.com> In-Reply-To: <546CDCAD.9030800@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 1/1] hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Don Slutz , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , "Michael S. Tsirkin" , Eduardo Habkost Cc: Michael Tokarev , Anthony Liguori , Stefan Hajnoczi On 11/19/14 13:08, Paolo Bonzini wrote: > > On 19/11/2014 19:07, Don Slutz wrote: >>> "-M pc -machine accel=xen" should work and, if that's what you want, >>> disable the vmport device. I think this patch is wrong. >>> >>> Paolo >> Well, I also want "-M pc -machine accel=xen,vmport=on" to work. > Right. So let's start by deciding what the desired semantics are for > all six cases: -M pc/xenfv, -machine vmport=on/off/absent. > > Paolo I get 12 cases (PCMachineState *pcms = PC_MACHINE(obj)): -M pc pcms->vmport is true -M pc -machine vmport=on pcms->vmport is true -M pc -machine vmport=off pcms->vmport is false -M xenfv pcms->vmport is false -M xenfv -machine vmport=on pcms->vmport is true -M xenfv -machine vmport=off pcms->vmport is false -M pc -machine accel=xen pcms->vmport is false -M pc -machine vmport=on,accel=xen pcms->vmport is true -M pc -machine vmport=off,accel=xen pcms->vmport is false -M xenfv -machine accel=xen pcms->vmport is false -M xenfv -machine vmport=on,accel=xen pcms->vmport is true -M xenfv -machine vmport=off,accel=xen pcms->vmport is false Which look like to me to solve down to xen_init() called via xen_accel_class_init() need to see if vmport has been specified and change it if needed. Which leads me to: commit 436d056420bb209d066d2378b628d297777d9f20 Author: Don Slutz Date: Wed Nov 19 10:01:16 2014 -0500 hw/i386/pc_piix.c: Also pass vmport=off for xenfv machine c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4 or c/s b154537ad07598377ebf98252fb7d2aff127983b moved the testing of xen_enabled() from pc_init1() to pc_machine_initfn(). xen_enabled() does not return the correct value in pc_machine_initfn(). Add vmport_changed to track the state of vmport so that accel=xen can do the right thing. Drop the call to xen_enabled() in pc_machine_initfn() to reduce potential confusion. Signed-off-by: Don Slutz diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 1205db8..b400ac8 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1723,6 +1723,7 @@ static void pc_machine_set_vmport(Object *obj, bool value, Error **errp) PCMachineState *pcms = PC_MACHINE(obj); pcms->vmport = value; + pcms->vmport_changed = true; } static void pc_machine_initfn(Object *obj) @@ -1737,7 +1738,7 @@ static void pc_machine_initfn(Object *obj) pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, NULL, NULL, NULL); - pcms->vmport = !xen_enabled(); + pcms->vmport = true; object_property_add_bool(obj, PC_MACHINE_VMPORT, pc_machine_get_vmport, pc_machine_set_vmport, diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 7c3731f..5782406 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -38,6 +38,7 @@ struct PCMachineState { uint64_t max_ram_below_4g; bool vmport; + bool vmport_changed; }; #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" diff --git a/xen-common.c b/xen-common.c index 56359ca..bb11bb7 100644 --- a/xen-common.c +++ b/xen-common.c @@ -12,6 +12,7 @@ #include "qmp-commands.h" #include "sysemu/char.h" #include "sysemu/accel.h" +#include "hw/i386/pc.h" //#define DEBUG_XEN @@ -112,6 +113,11 @@ static void xen_change_state_handler(void *opaque, int running, static int xen_init(MachineState *ms) { + PCMachineState *pcms = PC_MACHINE(ms); + + if (!pcms->vmport_changed) { + pcms->vmport = false; + } xen_xc = xen_xc_interface_open(0, 0, 0); if (xen_xc == XC_HANDLER_INITIAL_VALUE) { xen_be_printf(NULL, 0, "can't open xen interface\n"); -Don Slutz >> However you bring up a good point and I will see if I can quickly handled it.