Patchwork [for-1.4] net: fix infinite loop on exit

login
register
mail settings
Submitter Michael Roth
Date Feb. 7, 2013, 12:25 a.m.
Message ID <1360196748-15023-1-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/218807/
State New
Headers show

Comments

Michael Roth - Feb. 7, 2013, 12:25 a.m.
1ceef9f27359cbe92ef124bf74de6f792e71f6fb added handling for cleaning
up multiple queues in qemu_del_nic() for cases where multiqueue is in
use. To determine the number of queues it looks at nic->conf->queues,
then iterates through all the queues to cleanup the associated
NetClientStates. If no queues are found, no NetClientStates are deleted.

However, nic->conf->queues is only set when a peer is created via
-netdev or netdev_add, and is otherwise 0. This causes us to spin in
net_cleanup() if we attempt to shut down qemu before adding a host
device.

Since qemu_new_nic() unconditionally creates at least 1
queue/NetClientState at queue idx 0, make qemu_del_nic() always attempt
to clean it up.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 net/net.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Jason Wang - Feb. 7, 2013, 6:24 a.m.
On 02/07/2013 08:25 AM, Michael Roth wrote:
> 1ceef9f27359cbe92ef124bf74de6f792e71f6fb added handling for cleaning
> up multiple queues in qemu_del_nic() for cases where multiqueue is in
> use. To determine the number of queues it looks at nic->conf->queues,
> then iterates through all the queues to cleanup the associated
> NetClientStates. If no queues are found, no NetClientStates are deleted.
>
> However, nic->conf->queues is only set when a peer is created via
> -netdev or netdev_add, and is otherwise 0. This causes us to spin in
> net_cleanup() if we attempt to shut down qemu before adding a host
> device.
>
> Since qemu_new_nic() unconditionally creates at least 1
> queue/NetClientState at queue idx 0, make qemu_del_nic() always attempt
> to clean it up.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  net/net.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/net.c b/net/net.c
> index 9806862..f9e7136 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -351,7 +351,7 @@ void qemu_del_net_client(NetClientState *nc)
>  
>  void qemu_del_nic(NICState *nic)
>  {
> -    int i, queues = nic->conf->queues;
> +    int i, queues = MAX(nic->conf->queues, 1);
>  
>      /* If this is a peer NIC and peer has already been deleted, free it now. */
>      if (nic->peer_deleted) {

Acked-by: Jason Wang <jasowang@redhat.com>

Alternatively, we can set the conf->queues to 1 in qemu_new_nic() if we
find it was 0. But this fix is ok enough.

Thanks
Amos Kong - Feb. 7, 2013, 8:47 a.m.
On Wed, Feb 06, 2013 at 06:25:48PM -0600, Michael Roth wrote:
> 1ceef9f27359cbe92ef124bf74de6f792e71f6fb added handling for cleaning
> up multiple queues in qemu_del_nic() for cases where multiqueue is in
> use. To determine the number of queues it looks at nic->conf->queues,
> then iterates through all the queues to cleanup the associated
> NetClientStates. If no queues are found, no NetClientStates are deleted.
> 
> However, nic->conf->queues is only set when a peer is created via
> -netdev or netdev_add, and is otherwise 0. This causes us to spin in
> net_cleanup() if we attempt to shut down qemu before adding a host
> device.
> 
> Since qemu_new_nic() unconditionally creates at least 1
> queue/NetClientState at queue idx 0, make qemu_del_nic() always attempt
> to clean it up.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Acked-by: Amos Kong <akong@redhat.com>

> ---
>  net/net.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 9806862..f9e7136 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -351,7 +351,7 @@ void qemu_del_net_client(NetClientState *nc)
>  
>  void qemu_del_nic(NICState *nic)
>  {
> -    int i, queues = nic->conf->queues;
> +    int i, queues = MAX(nic->conf->queues, 1);
>  
>      /* If this is a peer NIC and peer has already been deleted, free it now. */
>      if (nic->peer_deleted) {
> -- 
> 1.7.9.5
>
Stefan Hajnoczi - Feb. 7, 2013, 8:57 a.m.
On Wed, Feb 06, 2013 at 06:25:48PM -0600, Michael Roth wrote:
> 1ceef9f27359cbe92ef124bf74de6f792e71f6fb added handling for cleaning
> up multiple queues in qemu_del_nic() for cases where multiqueue is in
> use. To determine the number of queues it looks at nic->conf->queues,
> then iterates through all the queues to cleanup the associated
> NetClientStates. If no queues are found, no NetClientStates are deleted.
> 
> However, nic->conf->queues is only set when a peer is created via
> -netdev or netdev_add, and is otherwise 0. This causes us to spin in
> net_cleanup() if we attempt to shut down qemu before adding a host
> device.
> 
> Since qemu_new_nic() unconditionally creates at least 1
> queue/NetClientState at queue idx 0, make qemu_del_nic() always attempt
> to clean it up.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  net/net.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Anthony Liguori - Feb. 8, 2013, 6:57 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/net/net.c b/net/net.c
index 9806862..f9e7136 100644
--- a/net/net.c
+++ b/net/net.c
@@ -351,7 +351,7 @@  void qemu_del_net_client(NetClientState *nc)
 
 void qemu_del_nic(NICState *nic)
 {
-    int i, queues = nic->conf->queues;
+    int i, queues = MAX(nic->conf->queues, 1);
 
     /* If this is a peer NIC and peer has already been deleted, free it now. */
     if (nic->peer_deleted) {