qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCHv4] net: delay freeing peer host device


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCHv4] net: delay freeing peer host device
Date: Wed, 6 Oct 2010 18:58:23 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Wed, Oct 06, 2010 at 11:35:08AM -0500, Anthony Liguori wrote:
> On 10/06/2010 11:04 AM, Michael S. Tsirkin wrote:
> >With -netdev, virtio devices present offload
> >features to guest, depending on the backend used.
> >Thus, removing host netdev peer while guest is
> >active leads to guest-visible inconsistency and/or crashes.
> >
> >As a solution, while guest (NIC) peer device exists,
> >we prevent the host peer from being deleted.
> >This patch does this by adding peer_deleted flag in nic state:
> >if host device is going away while guest device
> >is around, set this flag and keep a shell of
> >the host device around for as long as guest device exists.
> >
> >The link is put down so all packets will get discarded.
> >
> >At the moment, management can detect that device deletion
> >is delayed by doing info net. As a next step, we shall add
> >commands that control hotplug/unplug without
> >removing the device, and an event to report that
> >guest has responded to the hotplug event.
> >
> >Signed-off-by: Michael S. Tsirkin<address@hidden>
> 
> I don't think this is a very good idea.  Making netdev_del
> conditionally succeed is going to break management tools and is very
> non-intuitive.
> 
> Regards,
> 
> Anthony Liguori

But this is not what this does. With this patch netdev_del always
succeeds, and clears the device state leaving just an empty shell around.
The shell goes away together with the NIC.

> >---
> >
> >  Changes from v3:
> >    make sure ->cleanup is called for NICs
> >  Changes from v2:
> >    fix crash on repeated netdev_del
> >
> >  net.c |   49 ++++++++++++++++++++++++++++++++++++++++++-------
> >  net.h |    1 +
> >  2 files changed, 43 insertions(+), 7 deletions(-)
> >
> >diff --git a/net.c b/net.c
> >index 3d0fde7..ed74c7f 100644
> >--- a/net.c
> >+++ b/net.c
> >@@ -281,29 +281,64 @@ NICState *qemu_new_nic(NetClientInfo *info,
> >      return nic;
> >  }
> >
> >-void qemu_del_vlan_client(VLANClientState *vc)
> >+static void qemu_cleanup_vlan_client(VLANClientState *vc)
> >  {
> >      if (vc->vlan) {
> >          QTAILQ_REMOVE(&vc->vlan->clients, vc, next);
> >      } else {
> >-        if (vc->send_queue) {
> >-            qemu_del_net_queue(vc->send_queue);
> >-        }
> >          QTAILQ_REMOVE(&non_vlan_clients, vc, next);
> >-        if (vc->peer) {
> >-            vc->peer->peer = NULL;
> >-        }
> >      }
> >
> >      if (vc->info->cleanup) {
> >          vc->info->cleanup(vc);
> >      }
> >+}
> >
> >+static void qemu_free_vlan_client(VLANClientState *vc)
> >+{
> >+    if (!vc->vlan) {
> >+        if (vc->send_queue) {
> >+            qemu_del_net_queue(vc->send_queue);
> >+        }
> >+        if (vc->peer) {
> >+            vc->peer->peer = NULL;
> >+        }
> >+    }
> >      qemu_free(vc->name);
> >      qemu_free(vc->model);
> >      qemu_free(vc);
> >  }
> >
> >+void qemu_del_vlan_client(VLANClientState *vc)
> >+{
> >+    /* If there is a peer NIC, delete and cleanup client, but do not free. 
> >*/
> >+    if (!vc->vlan&&  vc->peer&&  vc->peer->info->type == 
> >NET_CLIENT_TYPE_NIC) {
> >+        NICState *nic = DO_UPCAST(NICState, nc, vc->peer);
> >+        if (nic->peer_deleted) {
> >+            return;
> >+        }
> >+        nic->peer_deleted = true;
> >+        /* Let NIC know peer is gone. */
> >+        vc->peer->link_down = true;
> >+        if (vc->peer->info->link_status_changed) {
> >+            vc->peer->info->link_status_changed(vc->peer);
> >+        }
> >+        qemu_cleanup_vlan_client(vc);
> >+        return;
> >+    }
> >+
> >+    /* If this is a peer NIC and peer has already been deleted, free it 
> >now. */
> >+    if (!vc->vlan&&  vc->peer&&  vc->info->type == NET_CLIENT_TYPE_NIC) {
> >+        NICState *nic = DO_UPCAST(NICState, nc, vc);
> >+        if (nic->peer_deleted) {
> >+            qemu_free_vlan_client(vc->peer);
> >+        }
> >+    }
> >+
> >+    qemu_cleanup_vlan_client(vc);
> >+    qemu_free_vlan_client(vc);
> >+}
> >+
> >  VLANClientState *
> >  qemu_find_vlan_client_by_name(Monitor *mon, int vlan_id,
> >                                const char *client_str)
> >diff --git a/net.h b/net.h
> >index 518cf9c..44c31a9 100644
> >--- a/net.h
> >+++ b/net.h
> >@@ -72,6 +72,7 @@ typedef struct NICState {
> >      VLANClientState nc;
> >      NICConf *conf;
> >      void *opaque;
> >+    bool peer_deleted;
> >  } NICState;
> >
> >  struct VLANState {



reply via email to

[Prev in Thread] Current Thread [Next in Thread]