On Mon, 2010-09-27 at 18:41 +0200, Michael S. Tsirkin wrote:
> Move all of vhost-net start/stop logic to a single routine,
> and call it from everywhere.
>
> Additionally, start/stop vhost-net on link up/down:
> we should not transmit anything if user asked us to
> put the link down.
>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
>
> Sent a wrong version, sorry about the noise.
>
> hw/virtio-net.c | 89 ++++++++++++++++++++++++++----------------------------
> 1 files changed, 43 insertions(+), 46 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 075f72d..f87c219 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -51,6 +51,7 @@ typedef struct VirtIONet
> uint8_t nouni;
> uint8_t nobcast;
> uint8_t vhost_started;
> + bool vm_running;
> VMChangeStateEntry *vmstate;
> struct {
> int in_use;
> @@ -95,6 +96,38 @@ static void virtio_net_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> }
> }
>
> +static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> +{
> + VirtIONet *n = to_virtio_net(vdev);
> + if (!n->nic->nc.peer) {
> + return;
> + }
> + if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
> + return;
> + }
> +
> + if (!tap_get_vhost_net(n->nic->nc.peer)) {
> + return;
> + }
> + if (!!n->vhost_started == (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> + (n->status & VIRTIO_NET_S_LINK_UP) &&
> + n->vm_running) {
I don't think this works...
#define VIRTIO_CONFIG_S_DRIVER_OK 4
!!(1) == (status & 4)?
> + return;
> + }
> + if (!n->vhost_started) {
> + int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer),
> &n->vdev);
> + if (r < 0) {
> + fprintf(stderr, "unable to start vhost net: %d: "
> + "falling back on userspace virtio\n", -r);
Definitely worth noting if not using vhost, but aren't we potentially
going to see this in the log a lot in that case? Thanks,
Alex
> + } else {
> + n->vhost_started = 1;
> + }
> + } else {
> + vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev);
> + n->vhost_started = 0;
> + }
> +}
> +
> static void virtio_net_set_link_status(VLANClientState *nc)
> {
> VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque;
> @@ -107,6 +140,8 @@ static void virtio_net_set_link_status(VLANClientState
> *nc)
>
> if (n->status != old_status)
> virtio_notify_config(&n->vdev);
> +
> + virtio_net_set_status(&n->vdev, n->vdev.status);
> }
>
> static void virtio_net_reset(VirtIODevice *vdev)
> @@ -120,10 +155,6 @@ static void virtio_net_reset(VirtIODevice *vdev)
> n->nomulti = 0;
> n->nouni = 0;
> n->nobcast = 0;
> - if (n->vhost_started) {
> - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev);
> - n->vhost_started = 0;
> - }
>
> /* Flush any MAC and VLAN filter table state */
> n->mac_table.in_use = 0;
> @@ -726,12 +757,9 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
> {
> VirtIONet *n = opaque;
>
> - if (n->vhost_started) {
> - /* TODO: should we really stop the backend?
> - * If we don't, it might keep writing to memory. */
> - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev);
> - n->vhost_started = 0;
> - }
> + /* At this point, backend must be stopped, otherwise
> + * it might keep writing to memory. */
> + assert(!n->vhost_started);
> virtio_save(&n->vdev, f);
>
> qemu_put_buffer(f, n->mac, ETH_ALEN);
> @@ -863,44 +891,14 @@ static NetClientInfo net_virtio_info = {
> .link_status_changed = virtio_net_set_link_status,
> };
>
> -static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> -{
> - VirtIONet *n = to_virtio_net(vdev);
> - if (!n->nic->nc.peer) {
> - return;
> - }
> - if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
> - return;
> - }
> -
> - if (!tap_get_vhost_net(n->nic->nc.peer)) {
> - return;
> - }
> - if (!!n->vhost_started == !!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> - return;
> - }
> - if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> - int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), vdev);
> - if (r < 0) {
> - fprintf(stderr, "unable to start vhost net: %d: "
> - "falling back on userspace virtio\n", -r);
> - } else {
> - n->vhost_started = 1;
> - }
> - } else {
> - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev);
> - n->vhost_started = 0;
> - }
> -}
> -
> static void virtio_net_vmstate_change(void *opaque, int running, int reason)
> {
> VirtIONet *n = opaque;
> - uint8_t status = running ? VIRTIO_CONFIG_S_DRIVER_OK : 0;
> + n->vm_running = running;
> /* This is called when vm is started/stopped,
> - * it will start/stop vhost backend if * appropriate
> + * it will start/stop vhost backend if appropriate
> * e.g. after migration. */
> - virtio_net_set_status(&n->vdev, n->vdev.status & status);
> + virtio_net_set_status(&n->vdev, n->vdev.status);
> }
>
> VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf)
> @@ -951,9 +949,8 @@ void virtio_net_exit(VirtIODevice *vdev)
> VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
> qemu_del_vm_change_state_handler(n->vmstate);
>
> - if (n->vhost_started) {
> - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev);
> - }
> + /* This will stop vhost backend if appropriate. */
> + virtio_net_set_status(vdev, 0);
>
> qemu_purge_queued_packets(&n->nic->nc);
>
