From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lp0L2-0002oW-Tz for qemu-devel@nongnu.org; Wed, 01 Apr 2009 09:18:40 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lp0Ky-0002gi-54 for qemu-devel@nongnu.org; Wed, 01 Apr 2009 09:18:40 -0400 Received: from [199.232.76.173] (port=36212 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lp0Kx-0002gR-V6 for qemu-devel@nongnu.org; Wed, 01 Apr 2009 09:18:36 -0400 Received: from wa-out-1112.google.com ([209.85.146.179]:24352) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Lp0Kx-0003fJ-HP for qemu-devel@nongnu.org; Wed, 01 Apr 2009 09:18:35 -0400 Received: by wa-out-1112.google.com with SMTP id m33so14935wag.18 for ; Wed, 01 Apr 2009 06:18:34 -0700 (PDT) Message-ID: <49D369A0.50907@codemonkey.ws> Date: Wed, 01 Apr 2009 08:18:24 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20090316184929.GA20955@amd.home.annexia.org> <49CB7D33.70002@us.ibm.com> <20090331112354.GA18048@amd.home.annexia.org> <20090401084620.GA17093@amd.home.annexia.org> In-Reply-To: <20090401084620.GA17093@amd.home.annexia.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] Hardware watchdog patch, version 6 Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Richard W.M. Jones wrote: > On Tue, Mar 31, 2009 at 07:07:39PM +0300, Blue Swirl wrote: > >> + if (i > 0) exit (i == 1 ? 1 : 0); >> + if (!d->enabled) return; >> >> Please split these into two lines. >> > > Version 7 of the patch is attached: > > - Fixed the 'if' statements above, plus a couple more I found. > > - Rebased to latest SVN. > > Rich. > I swear I sent a note with some review comments but I cannot find it in the mailing list archives. Perhaps my mail server ate it :-( I'll reproduce below. First, there's still no Signed-off-by. > @@ -4727,6 +4732,17 @@ > serial_devices[serial_device_index] = optarg; > serial_device_index++; > break; > + case QEMU_OPTION_watchdog: > + i = select_watchdog (optarg); > You have extra spaces before all your punctuation. This is not how QEMU does it (just look below). > + if (i > 0) > + exit (i == 1 ? 1 : 0); > + break; > + case QEMU_OPTION_watchdog_action: > + if (select_watchdog_action (optarg) == -1) { > + fprintf (stderr, "Unknown -watchdog-action parameter\n"); > + exit (1); > + } > + break; > case QEMU_OPTION_virtiocon: > if (virtio_console_index >= MAX_VIRTIO_CONSOLES) { > fprintf(stderr, "qemu: too many virtio consoles\n"); > > +void register_watchdogs(void) > +{ > +#ifdef TARGET_I386 > + wdt_ib700_init (); > + wdt_i6300esb_init (); > +#endif > +} > Limiting to target-i386 is not correct. These are ISA and PCI devices so they should be limited based on that property. Really, you only need to limit it based on !CONFIG_USER_ONLY. It's not a huge problem to add unused devices to other platforms. After the config file stuff is ready, we can move to a more modular model where we can choose which devices are included for any given target. Regards, Anthony Liguori