From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40953) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya6zi-0007ns-6V for qemu-devel@nongnu.org; Mon, 23 Mar 2015 14:22:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ya6zf-00046c-1T for qemu-devel@nongnu.org; Mon, 23 Mar 2015 14:22:34 -0400 Received: from cantor2.suse.de ([195.135.220.15]:55174 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya6ze-00046W-Rp for qemu-devel@nongnu.org; Mon, 23 Mar 2015 14:22:30 -0400 Message-ID: <551059A0.9060602@suse.de> Date: Mon, 23 Mar 2015 19:21:20 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1427130328-3629-1-git-send-email-pbonzini@redhat.com> <55105971.3000708@redhat.com> In-Reply-To: <55105971.3000708@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.3] powerpc: fix -machine usb=no for newworld and pseries machines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , Paolo Bonzini , qemu-devel@nongnu.org Cc: marcel.a@redhat.com On 03/23/2015 07:20 PM, Marcel Apfelbaum wrote: > On 03/23/2015 07:05 PM, Paolo Bonzini wrote: >> Capture the explicit setting of "usb=no" into a separate bool, and >> use it to skip the update of machine->usb in the board init function. >> >> Signed-off-by: Paolo Bonzini >> --- >> hw/core/machine.c | 1 + >> hw/ppc/mac_newworld.c | 2 +- >> hw/ppc/spapr.c | 2 +- >> include/hw/boards.h | 1 + >> 4 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index cb1185a..25c45e6 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -223,6 +223,7 @@ static void machine_set_usb(Object *obj, bool >> value, Error **errp) >> MachineState *ms = MACHINE(obj); >> >> ms->usb = value; >> + ms->usb_disabled = !value; > Maybe is too late now, but I really not like this pollution of > MachineState > with 'usb_disabled'. (Imagine we have this kind of fields for lots of > objects and lots > of corner cases...) > I know it comes to solve a bug, but we talked about it in another mail > thread and > this change in semantics was approved. > > Let me explain *why* I don't like it. > 1. We add an "usb_disabled" field to a base class (actually object) > of all the machines and the only place it is interesting is > for 2 machines on ppc. > 2. Even for these 2 machines, the scenario of defaults=on and usb=off > is not practical. I'm personally fine either way, but I assumed that Paolo had a good reason for writing the patch? Alex