|
From: | Anthony Liguori |
Subject: | [Qemu-devel] Re: [PATCH] Hardware watchdog patch, version 6 |
Date: | Wed, 01 Apr 2009 08:18:24 -0500 |
User-agent: | Thunderbird 2.0.0.21 (X11/20090320) |
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
[Prev in Thread] | Current Thread | [Next in Thread] |