Patchwork net: Fix VM start with '-net none'

login
register
mail settings
Submitter Amit Shah
Date June 15, 2010, 8 a.m.
Message ID <22a96312232a0458fc04268b79d17828c824df42.1276588830.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/55617/
State New
Headers show

Comments

Amit Shah - June 15, 2010, 8 a.m.
Commit 50e32ea8f31035877decc10f1075aa0e619e09cb changed the behaviour
for the return type of net_client_init() when a nic type with no init
method was specified. 'none' is one such nic type. Instead of returning
0, which gets interpreted as an index into the nd_table[] array, we
switched to returning -1, which signifies an error as well.

That broke VM start with '-net none'. Testing was only done with the
monitor command 'pci_add', which doesn't fail.

The correct fix would still be to return 0+ values from
net_client_init() only when the return value can be used as an index to
refer to an entry in nd_table[]. With the current code, callers can
erroneously poke into nd_table[0] when -net nic is used, which can lead
to badness.

However, this commit just returns to the previous behaviour before the
offending commit.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Stefan Hajnoczi - June 24, 2010, 2:31 p.m.
On Tue, Jun 15, 2010 at 9:00 AM, Amit Shah <amit.shah@redhat.com> wrote:
> Commit 50e32ea8f31035877decc10f1075aa0e619e09cb changed the behaviour
> for the return type of net_client_init() when a nic type with no init
> method was specified. 'none' is one such nic type. Instead of returning
> 0, which gets interpreted as an index into the nd_table[] array, we
> switched to returning -1, which signifies an error as well.
>
> That broke VM start with '-net none'. Testing was only done with the
> monitor command 'pci_add', which doesn't fail.

I stumbled across the broken -net none in qemu.git and qemu-kvm.git.
It does a silent exit(1) on startup.

It would be nice to merge this patch.

Stefan
Aurelien Jarno - June 30, 2010, 8:36 p.m.
On Tue, Jun 15, 2010 at 01:30:39PM +0530, Amit Shah wrote:
> Commit 50e32ea8f31035877decc10f1075aa0e619e09cb changed the behaviour
> for the return type of net_client_init() when a nic type with no init
> method was specified. 'none' is one such nic type. Instead of returning
> 0, which gets interpreted as an index into the nd_table[] array, we
> switched to returning -1, which signifies an error as well.
> 
> That broke VM start with '-net none'. Testing was only done with the
> monitor command 'pci_add', which doesn't fail.
> 
> The correct fix would still be to return 0+ values from
> net_client_init() only when the return value can be used as an index to
> refer to an entry in nd_table[]. With the current code, callers can
> erroneously poke into nd_table[0] when -net nic is used, which can lead
> to badness.
> 
> However, this commit just returns to the previous behaviour before the
> offending commit.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  net.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/net.c b/net.c
> index 4cb93ed..b681233 100644
> --- a/net.c
> +++ b/net.c
> @@ -1119,7 +1119,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
>                  vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
>              }
>  
> -            ret = -1;
> +            ret = 0;
>              if (net_client_types[i].init) {
>                  ret = net_client_types[i].init(opts, mon, name, vlan);
>                  if (ret < 0) {
> -- 
> 1.7.0.1
> 
> 
>

Patch

diff --git a/net.c b/net.c
index 4cb93ed..b681233 100644
--- a/net.c
+++ b/net.c
@@ -1119,7 +1119,7 @@  int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
                 vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
             }
 
-            ret = -1;
+            ret = 0;
             if (net_client_types[i].init) {
                 ret = net_client_types[i].init(opts, mon, name, vlan);
                 if (ret < 0) {