From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M9hwi-0005Lh-Gy for qemu-devel@nongnu.org; Thu, 28 May 2009 11:55:08 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M9hwd-0005KA-Uq for qemu-devel@nongnu.org; Thu, 28 May 2009 11:55:08 -0400 Received: from [199.232.76.173] (port=44862 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M9hwd-0005K7-P8 for qemu-devel@nongnu.org; Thu, 28 May 2009 11:55:03 -0400 Received: from lizzard.sbs.de ([194.138.37.39]:16273) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M9hwc-0000tO-U6 for qemu-devel@nongnu.org; Thu, 28 May 2009 11:55:03 -0400 Message-ID: <4A1EB3D4.7030804@siemens.com> Date: Thu, 28 May 2009 17:55:00 +0200 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 10/11] slirp: Rework external configuration interface References: <20090508103416.6080.44298.stgit@mchn012c.ww002.siemens.net> <20090508103418.6080.46645.stgit@mchn012c.ww002.siemens.net> <1243523247.4046.197.camel@blaa> In-Reply-To: <1243523247.4046.197.camel@blaa> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark McLoughlin Cc: qemu-devel@nongnu.org Mark McLoughlin wrote: > On Fri, 2009-05-08 at 12:34 +0200, Jan Kiszka wrote: >> With the internal IP configuration made more flexible, we can now >> enhance the user interface. This patch adds a number of new options to >> "-net user": net (address and mask), host, dhcpstart, dns and smbserver. >> It also renames "redir" to "hostfwd" and "channel" to "guestfwd" in >> order to (hopefully) clarify their meanings. The format of guestfwd is >> extended so that the user can define not only the port but also the >> virtual server's IP address the forwarding starts from. >> >> Signed-off-by: Jan Kiszka > > .... > >> @@ -1988,15 +2117,21 @@ int net_client_init(Monitor *mon, const char *device, const char *p) >> #ifdef CONFIG_SLIRP >> if (!strcmp(device, "user")) { >> static const char * const slirp_params[] = { >> - "vlan", "name", "hostname", "restrict", "ip", "tftp", "bootfile", >> - "smb", "redir", "channel", NULL >> + "vlan", "name", "hostname", "restrict", "net", "host", >> + "tftp", "bootfile", "dhcpstart", "dns", "smb", "smbserver", >> + "hostfwd", "guestfwd", NULL > > You're renaming "redir" and "channel" here which isn't a big deal since > you've only introduced them in the previous patch, but it would be > better to use the final names in the original patch. Well, the purpose of the original patch was only to pull "redir" and "channel" as-is into the -net parameter list, not to refactor the interface otherwise. > > More importantly, though, you've dropped the "ip" parameter which is in > the 0.10.x releases. We can't just drop existing parameters. OK, I see the problem - though this parameter was probable rarely used (it was undocumented and suffered from the poor configurability). Will have a closer look. > > By way of meta-comment, some of these patches are much harder to review > than they need to be, because e.g. you're doing lots of cleanups > together with the real changes, or not breaking changes into smaller > chunks. There might be a few coding style updates included, when I touch related lines. Or please give some (or the most annoying) example. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux