From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37191) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya8hA-00029N-8H for qemu-devel@nongnu.org; Mon, 23 Mar 2015 16:11:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ya8h6-0004Zv-Vg for qemu-devel@nongnu.org; Mon, 23 Mar 2015 16:11:32 -0400 Received: from mail-we0-x22c.google.com ([2a00:1450:400c:c03::22c]:33671) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya8h6-0004Zr-Oo for qemu-devel@nongnu.org; Mon, 23 Mar 2015 16:11:28 -0400 Received: by weop45 with SMTP id p45so147141727weo.0 for ; Mon, 23 Mar 2015 13:11:28 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5510736B.8000107@redhat.com> Date: Mon, 23 Mar 2015 21:11:23 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1427130328-3629-1-git-send-email-pbonzini@redhat.com> <55105971.3000708@redhat.com> <551059A0.9060602@suse.de> In-Reply-To: <551059A0.9060602@suse.de> Content-Type: text/plain; charset=windows-1252 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: Alexander Graf , Marcel Apfelbaum , qemu-devel@nongnu.org Cc: marcel.a@redhat.com On 23/03/2015 19:21, Alexander Graf wrote: > 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. So we do for kernel_irqchip_requested/allowed. Both approaches could be replaced by a tri-state on/off/auto. >> 2. Even for these 2 machines, the scenario of defaults=on and usb=off >> is not practical. Why? For example you could add a virtio-input device instead of a USB keyboard and mouse. > I'm personally fine either way, but I assumed that Paolo had a good > reason for writing the patch? Actually, two. One good reason is that no matter how you look at it, it's at least surprising and at worst a bug that "-machine usb=no" includes a default USB controller. The second good reason is that it's a guest ABI change for the versioned pSeries machines, and as such it breaks migration. Paolo