From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37896) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrBRX-0001k6-KV for qemu-devel@nongnu.org; Wed, 19 Nov 2014 15:01:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XrBRR-00008C-Hn for qemu-devel@nongnu.org; Wed, 19 Nov 2014 15:01:35 -0500 Received: from omzsmtpe04.verizonbusiness.com ([199.249.25.207]:53614) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XrBRR-00007Y-Cp for qemu-devel@nongnu.org; Wed, 19 Nov 2014 15:01:29 -0500 From: Don Slutz Message-ID: <546CF714.4030505@terremark.com> Date: Wed, 19 Nov 2014 15:01:24 -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> <546CEA98.40505@terremark.com> <20141119193009.GF3243@thinpad.lan.raisama.net> In-Reply-To: <20141119193009.GF3243@thinpad.lan.raisama.net> 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: Eduardo Habkost , Don Slutz Cc: Anthony Liguori , "Michael S. Tsirkin" , Michael Tokarev , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Stefan Hajnoczi , Paolo Bonzini On 11/19/14 14:30, Eduardo Habkost wrote: > On Wed, Nov 19, 2014 at 02:08:08PM -0500, Don Slutz wrote: >> 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: >> > [...] >> 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; > So, now the setting have three possible states: unset, on, and off. > > If we can't avoid that (which seems to be the case, as "accel" is > changed after we already set the default value for "vmport" in instance > init), maybe we should make it an enum property that accepts on/off/auto > as values, instead of faking a boolean property that is not really a > boolean? > I would be happy to look into this. I have not done anything that used an enum property, and so it is hard to guess on how long it will take to convert the change to this. -Don Slutz