Re: [Qemu-devel] [PATCH 10/11] slirp: Rework external configuration interface

[Mark McLoughlin <markmc@redhat.com>]

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 <jan.kiszka@siemens.com>

...

> @@ -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.

More importantly, though, you've dropped the "ip" parameter which is in
the 0.10.x releases. We can't just drop existing parameters.

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.

Cheers,
Mark.