From patchwork Mon Jun 18 07:54:23 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Peter A. G. Crosthwaite" X-Patchwork-Id: 165410 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1EA4EB72C3 for ; Mon, 18 Jun 2012 17:54:47 +1000 (EST) Received: from localhost ([::1]:49173 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgWnN-0004KN-1I for incoming@patchwork.ozlabs.org; Mon, 18 Jun 2012 03:54:45 -0400 Received: from eggs.gnu.org ([208.118.235.92]:37102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgWn6-00041K-8c for qemu-devel@nongnu.org; Mon, 18 Jun 2012 03:54:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SgWn4-0003f0-C9 for qemu-devel@nongnu.org; Mon, 18 Jun 2012 03:54:27 -0400 Received: from mail-bk0-f45.google.com ([209.85.214.45]:45050) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgWn4-0003em-53 for qemu-devel@nongnu.org; Mon, 18 Jun 2012 03:54:26 -0400 Received: by bkwj10 with SMTP id j10so3973673bkw.4 for ; Mon, 18 Jun 2012 00:54:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding:x-gm-message-state; bh=VqZf96H6TQKUvV2g6eRpVGynYCl+BMaGNZMzwhGd8Vw=; b=nbwyBB0Obovpp5A+jAf24GuJH7yTBkBafFf80M71FS0w3ADhwYUjWY7j8tJjDzJ87D PP4wq8eZDGoh9gu2a9eMT1kxe8XMt9bg7T0CpTMq3jlgQYYRilLVd/dBEr7ksVmqn0ZC U/6o9S9wUtUGKKCCqmL01no6pcLc7ac6REkh9+Orvp91kZVEZFV73hQfxKGz3rLz13Nl IDmGChKrXg7R0AbRJcKUN9ykk4/BpUs1FTIMHqY/6q5xwWBIRZlOL6EGQMq7o55gRbKb S0MYBSVWtKntoVT2tOX75M2Qu79t3R7GPKT2NHNzwkAGwv5Mz/HW0aVUFkHjBZV1rmN3 bLHA== MIME-Version: 1.0 Received: by 10.204.9.199 with SMTP id m7mr5956050bkm.66.1340006063376; Mon, 18 Jun 2012 00:54:23 -0700 (PDT) Received: by 10.204.119.199 with HTTP; Mon, 18 Jun 2012 00:54:23 -0700 (PDT) In-Reply-To: References: <1339754783-14341-1-git-send-email-zhlcindy@linux.vnet.ibm.com> Date: Mon, 18 Jun 2012 17:54:23 +1000 Message-ID: From: Peter Crosthwaite To: Markus Armbruster X-Gm-Message-State: ALoCoQlNWNDY0+MogWCW1CqWYfx+ZPEjmSbRIsQ7g59sNG3Xw6eJui33qVrrIFOnqGv3fKkaHOi7 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.45 Cc: aliguori@us.ibm.com, benh@au1.ibm.com, qemu-devel@nongnu.org, Li Zhang , qemu-ppc@nongnu.org, Li Zhang Subject: Re: [Qemu-devel] [PATCHv2 1/1] Add usb option in machine options to enable/disable usb X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org > > 3. Can't have USB: fail if the user tries to enable it. > > Code sketch: > >    /* init USB devices */ >    if (!machine->has_usb) { >        if (usb_enabled) >            [report error; should point to the offending options] >            exit(1); >        } >    } else { >        if (machine->has_usb > 0) { >            usb_enabled = 1; >        } >        if (usb_enabled) { >            if (foreach_device_config(DEV_USB, usb_parse) < 0) >                exit(1); >        } >    } > > >>> Anyway, I don't see why we need to update opts.  Who's using the updated >>> opts? >>> >> psereis will use this opts. >> usb kbd and mouse will be needed  with vga enabled. > > Do they use the updated QemuOpts *opts?  I'd expect them to use usb_on, > or whatever flag variable governs USB (now: usb_enabled). > I think whats going on here is Li is trying to do the right thing by using QEMU opts for this new machine functionality, however, its getting tangled with all this global state replication of -usb. Isnt there predecessor work here in getting rid of usb_enabled first? To that end, I think what is being proposed here is two (somewhat independent) patches. One patch for changing usb to QEMU_OPTS that primarily does this: [6] Done gedit ./sysemu.h And the second patch which is the pseries machine model stuff. Which way round probably doesnt matter right? You could make your machine model use the extern int usb_enabled initially then move it across to machine opts along with the rest of the usb subsystem. Or you could fix USB first (globally) then build on top of it. But I think that this patch as is, is going to do is introduce is a duplicate -usb implementation which is a little messy (even if it is only an intermediary state). Regards, Peter > [...] > diff --git a/sysemu.h b/sysemu.h index bc2c788..9f5ce2c 100644 --- a/sysemu.h +++ b/sysemu.h @@ -117,7 +117,6 @@ extern const char *keyboard_layout; extern int win2k_install_hack; extern int alt_grab; extern int ctrl_grab; -extern int usb_enabled; extern int smp_cpus; extern int max_cpus; extern int cursor_hide;