qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest
@ 2012-03-13  8:56 Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 1/5] net: reset the count after rounds of announcing Jason Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

This an update of series that let guest and qemu to be co-operated to send
gratuitous packets when needed such as after migration, loadvm and continuing.

As it's hard for qemu to track the network configuration in guest such as
bondings, vlans or ipv6. So current gratuitous may not work under those
situations.

The series first introduce a model specific function in order to let nic models
to use a device specific way to announce the link presence. With this,
virtio-net backend were modified to notify the guest (through config update
interrupt) and let guest send the gratuitous packet when needed.

---

Jason Wang (5):
      net: reset the count after rounds of announcing
      net: announce self after vm start
      net: model specific announcing support
      virtio-net: notify guest to annouce itself
      virtio-net: compat guest announce support.


 gdbstub.c       |    2 +-
 hw/pc_piix.c    |   35 +++++++++++++++++++++++++++++++++++
 hw/virtio-net.c |   19 ++++++++++++++++++-
 hw/virtio-net.h |    3 +++
 migration.c     |    5 ++---
 monitor.c       |    2 +-
 net.h           |    2 ++
 qmp.c           |    2 +-
 savevm.c        |   11 +++++++----
 sysemu.h        |    2 +-
 vl.c            |    7 +++++--
 11 files changed, 76 insertions(+), 14 deletions(-)

-- 
Jason Wang

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [V4 PATCH 1/5] net: reset the count after rounds of announcing
  2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
@ 2012-03-13  8:56 ` Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start Jason Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

As it would be called after continue a stopped guest.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 savevm.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/savevm.c b/savevm.c
index 80be1ff..5b59826 100644
--- a/savevm.c
+++ b/savevm.c
@@ -144,6 +144,7 @@ static void qemu_announce_self_once(void *opaque)
     } else {
 	    qemu_del_timer(timer);
 	    qemu_free_timer(timer);
+            count = SELF_ANNOUNCE_ROUNDS;
     }
 }
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start
  2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 1/5] net: reset the count after rounds of announcing Jason Wang
@ 2012-03-13  8:56 ` Jason Wang
  2012-03-13  9:18   ` Paolo Bonzini
  2012-03-13 14:23   ` Michael S. Tsirkin
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support Jason Wang
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

This patch moves qemu_announce_self() to vm_start() and add a new parameters to
control whether sending gratuitous packet is needed. There are several reasons
to do this:

- Gratuitous packet is also needed when we resume a stopped vm or successfully
  load a state.
- Sending gratuitous packets may be done through co-operation with guest, so
  this work should be done after vm is started.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 gdbstub.c   |    2 +-
 migration.c |    5 ++---
 monitor.c   |    2 +-
 qmp.c       |    2 +-
 savevm.c    |    2 +-
 sysemu.h    |    2 +-
 vl.c        |    7 +++++--
 7 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ef95ac2..f4d22e5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s)
 #ifdef CONFIG_USER_ONLY
     s->running_state = 1;
 #else
-    vm_start();
+    vm_start(false);
 #endif
 }
 
diff --git a/migration.c b/migration.c
index 00fa1e3..40332d2 100644
--- a/migration.c
+++ b/migration.c
@@ -88,14 +88,13 @@ void process_incoming_migration(QEMUFile *f)
         fprintf(stderr, "load of migration failed\n");
         exit(0);
     }
-    qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
 
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all();
 
     if (autostart) {
-        vm_start();
+        vm_start(true);
     } else {
         runstate_set(RUN_STATE_PRELAUNCH);
     }
@@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque)
         }
         if (s->state != MIG_STATE_COMPLETED) {
             if (old_vm_running) {
-                vm_start();
+                vm_start(false);
             }
         }
     }
diff --git a/monitor.c b/monitor.c
index cbdfbad..0b63c11 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
     vm_stop(RUN_STATE_RESTORE_VM);
 
     if (load_vmstate(name) == 0 && saved_vm_running) {
-        vm_start();
+        vm_start(true);
     }
 }
 
diff --git a/qmp.c b/qmp.c
index a182b51..252c842 100644
--- a/qmp.c
+++ b/qmp.c
@@ -160,7 +160,7 @@ void qmp_cont(Error **errp)
         return;
     }
 
-    vm_start();
+    vm_start(true);
 }
 
 void qmp_system_wakeup(Error **errp)
diff --git a/savevm.c b/savevm.c
index 5b59826..82b9d3a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 
  the_end:
     if (saved_vm_running)
-        vm_start();
+        vm_start(false);
 }
 
 int load_vmstate(const char *name)
diff --git a/sysemu.h b/sysemu.h
index 98118cc..787edd4 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state);
 #define VMRESET_SILENT   false
 #define VMRESET_REPORT   true
 
-void vm_start(void);
+void vm_start(bool announce);
 void vm_stop(RunState state);
 void vm_stop_force_state(RunState state);
 
diff --git a/vl.c b/vl.c
index 65f11f2..4e04f82 100644
--- a/vl.c
+++ b/vl.c
@@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state)
     }
 }
 
-void vm_start(void)
+void vm_start(bool announce)
 {
     if (!runstate_is_running()) {
         cpu_enable_ticks();
@@ -1266,6 +1266,9 @@ void vm_start(void)
         vm_state_notify(1, RUN_STATE_RUNNING);
         resume_all_vcpus();
         monitor_protocol_event(QEVENT_RESUME, NULL);
+        if (announce) {
+            qemu_announce_self();
+        }
     }
 }
 
@@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp)
             exit(ret);
         }
     } else if (autostart) {
-        vm_start();
+        vm_start(false);
     }
 
     os_setup_post();

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support
  2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 1/5] net: reset the count after rounds of announcing Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start Jason Wang
@ 2012-03-13  8:56 ` Jason Wang
  2012-03-13 14:20   ` Michael S. Tsirkin
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself Jason Wang
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 5/5] virtio-net: compat guest announce support Jason Wang
  4 siblings, 1 reply; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

This patch introduces a function pointer in NetClientInfo which is
called during self announcement. With this, each kind of card can announce the
link with a specific way. The old method is still kept for cards that have not
implemented this or old guest. The first user would be virtio-net.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net.h    |    2 ++
 savevm.c |    8 +++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/net.h b/net.h
index 75a8c15..7195bfc 100644
--- a/net.h
+++ b/net.h
@@ -48,6 +48,7 @@ typedef ssize_t (NetReceive)(VLANClientState *, const uint8_t *, size_t);
 typedef ssize_t (NetReceiveIOV)(VLANClientState *, const struct iovec *, int);
 typedef void (NetCleanup) (VLANClientState *);
 typedef void (LinkStatusChanged)(VLANClientState *);
+typedef int (NetAnnounce)(VLANClientState *);
 
 typedef struct NetClientInfo {
     net_client_type type;
@@ -59,6 +60,7 @@ typedef struct NetClientInfo {
     NetCleanup *cleanup;
     LinkStatusChanged *link_status_changed;
     NetPoll *poll;
+    NetAnnounce *announce;
 } NetClientInfo;
 
 struct VLANClientState {
diff --git a/savevm.c b/savevm.c
index 82b9d3a..0a901dc 100644
--- a/savevm.c
+++ b/savevm.c
@@ -123,10 +123,12 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
 {
     uint8_t buf[60];
     int len;
+    NetAnnounce *func = nic->nc.info->announce;
 
-    len = announce_self_create(buf, nic->conf->macaddr.a);
-
-    qemu_send_packet_raw(&nic->nc, buf, len);
+    if (func == NULL || func(&nic->nc) != 0) {
+        len = announce_self_create(buf, nic->conf->macaddr.a);
+        qemu_send_packet_raw(&nic->nc, buf, len);
+    }
 }
 
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself
  2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
                   ` (2 preceding siblings ...)
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support Jason Wang
@ 2012-03-13  8:56 ` Jason Wang
  2012-03-13 14:18   ` Michael S. Tsirkin
  2012-03-13 14:20   ` Michael S. Tsirkin
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 5/5] virtio-net: compat guest announce support Jason Wang
  4 siblings, 2 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

It's hard to track all mac addresses and their usage (vlan, bondings,
ipv6) in qemu to send proper gratuitous packet. The better choice is
to let guest to send them.

So, this patch introduces a new rw config status bit of virtio-net,
VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
presence of its link through config update interrupt. When gust have
done the announcement, it should clear that bit. The feature is negotiated by
a new feature bit VIRTIO_NET_F_ANNOUNCE.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio-net.c |   19 ++++++++++++++++++-
 hw/virtio-net.h |    3 +++
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index bc5e3a8..a591a48 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -95,6 +95,8 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
         memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(&n->nic->nc, n->mac);
     }
+
+    memcpy(&n->status, &netcfg.status, sizeof(n->status));
 }
 
 static bool virtio_net_started(VirtIONet *n, uint8_t status)
@@ -227,7 +229,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
 {
     VirtIONet *n = to_virtio_net(vdev);
 
-    features |= (1 << VIRTIO_NET_F_MAC);
+    features |= (1 << VIRTIO_NET_F_MAC | 1 << VIRTIO_NET_F_GUEST_ANNOUNCE);
 
     if (peer_has_vnet_hdr(n)) {
         tap_using_vnet_hdr(n->nic->nc.peer, 1);
@@ -983,6 +985,20 @@ static void virtio_net_cleanup(VLANClientState *nc)
     n->nic = NULL;
 }
 
+static int virtio_net_announce(VLANClientState *nc)
+{
+    VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque;
+
+    if (n->vdev.guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE)
+        && n->status & VIRTIO_NET_S_LINK_UP) {
+        n->status |= VIRTIO_NET_S_ANNOUNCE;
+        virtio_notify_config(&n->vdev);
+        return 0;
+    }
+
+    return 1;
+}
+
 static NetClientInfo net_virtio_info = {
     .type = NET_CLIENT_TYPE_NIC,
     .size = sizeof(NICState),
@@ -990,6 +1006,7 @@ static NetClientInfo net_virtio_info = {
     .receive = virtio_net_receive,
         .cleanup = virtio_net_cleanup,
     .link_status_changed = virtio_net_set_link_status,
+    .announce = virtio_net_announce,
 };
 
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 4468741..9f8cea7 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -44,8 +44,10 @@
 #define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
 #define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering */
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
+#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Guest can announce itself */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
+#define VIRTIO_NET_S_ANNOUNCE   2       /* Announcement is needed */
 
 #define TX_TIMER_INTERVAL 150000 /* 150 us */
 
@@ -176,6 +178,7 @@ struct virtio_net_ctrl_mac {
         DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
         DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
         DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
+        DEFINE_PROP_BIT("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
         DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
         DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
         DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [V4 PATCH 5/5] virtio-net: compat guest announce support.
  2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
                   ` (3 preceding siblings ...)
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself Jason Wang
@ 2012-03-13  8:56 ` Jason Wang
  4 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-13  8:56 UTC (permalink / raw)
  To: aliguori, stefanha, mst, rusty, qemu-devel, quintela

Disable guest announce for compat machine types.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/pc_piix.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6c5c40f..780b607 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -389,6 +389,11 @@ static QEMUMachine pc_machine_v1_0 = {
             .property = "check_media_rate",
             .value    = "off",
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -408,6 +413,11 @@ static QEMUMachine pc_machine_v0_15 = {
             .property = "check_media_rate",
             .value    = "off",
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -452,6 +462,11 @@ static QEMUMachine pc_machine_v0_14 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -508,6 +523,11 @@ static QEMUMachine pc_machine_v0_13 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -568,6 +588,11 @@ static QEMUMachine pc_machine_v0_12 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     }
 };
@@ -636,6 +661,11 @@ static QEMUMachine pc_machine_v0_11 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     }
 };
@@ -716,6 +746,11 @@ static QEMUMachine pc_machine_v0_10 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start Jason Wang
@ 2012-03-13  9:18   ` Paolo Bonzini
  2012-03-15  6:08     ` Jason Wang
  2012-03-13 14:23   ` Michael S. Tsirkin
  1 sibling, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2012-03-13  9:18 UTC (permalink / raw)
  To: Jason Wang; +Cc: aliguori, stefanha, quintela, rusty, qemu-devel, mst

Il 13/03/2012 09:56, Jason Wang ha scritto:
> This patch moves qemu_announce_self() to vm_start() and add a new parameters to
> control whether sending gratuitous packet is needed. There are several reasons
> to do this:
> 
> - Gratuitous packet is also needed when we resume a stopped vm or successfully
>   load a state.
> - Sending gratuitous packets may be done through co-operation with guest, so
>   this work should be done after vm is started.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  gdbstub.c   |    2 +-
>  migration.c |    5 ++---
>  monitor.c   |    2 +-
>  qmp.c       |    2 +-
>  savevm.c    |    2 +-
>  sysemu.h    |    2 +-
>  vl.c        |    7 +++++--
>  7 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index ef95ac2..f4d22e5 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s)
>  #ifdef CONFIG_USER_ONLY
>      s->running_state = 1;
>  #else
> -    vm_start();
> +    vm_start(false);
>  #endif

Here we're switching from RUNSTATE_DEBUG.

>  }
>  
> diff --git a/migration.c b/migration.c
> index 00fa1e3..40332d2 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -88,14 +88,13 @@ void process_incoming_migration(QEMUFile *f)
>          fprintf(stderr, "load of migration failed\n");
>          exit(0);
>      }
> -    qemu_announce_self();
>      DPRINTF("successfully loaded vm state\n");
>  
>      /* Make sure all file formats flush their mutable metadata */
>      bdrv_invalidate_cache_all();
>  
>      if (autostart) {
> -        vm_start();
> +        vm_start(true);

Here from RUN_STATE_INMIGRATE.

>      } else {
>          runstate_set(RUN_STATE_PRELAUNCH);
>      }
> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque)
>          }
>          if (s->state != MIG_STATE_COMPLETED) {
>              if (old_vm_running) {
> -                vm_start();
> +                vm_start(false);

Here from RUN_STATE_FINISH_MIGRATE.

>              }
>          }
>      }
> diff --git a/monitor.c b/monitor.c
> index cbdfbad..0b63c11 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
>      vm_stop(RUN_STATE_RESTORE_VM);
>  
>      if (load_vmstate(name) == 0 && saved_vm_running) {
> -        vm_start();
> +        vm_start(true);

Here from RUN_STATE_RESTORE_VM.

>      }
>  }
>  
> diff --git a/qmp.c b/qmp.c
> index a182b51..252c842 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp)
>          return;
>      }
>  
> -    vm_start();
> +    vm_start(true);

Here from RUN_STATE_PAUSED or RUN_STATE_PRELAUNCH.

This introduces a difference here with "qemu -S" + cont, and "qemu".
The former will send a gratuitous ARP, the latter won't.  Is this
desired/harmless/...?

>  }
>  
>  void qmp_system_wakeup(Error **errp)
> diff --git a/savevm.c b/savevm.c
> index 5b59826..82b9d3a 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>  
>   the_end:
>      if (saved_vm_running)
> -        vm_start();
> +        vm_start(false);
>  }

This is RUN_STATE_SAVE_VM.

>  
>  int load_vmstate(const char *name)
> diff --git a/sysemu.h b/sysemu.h
> index 98118cc..787edd4 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state);
>  #define VMRESET_SILENT   false
>  #define VMRESET_REPORT   true
>  
> -void vm_start(void);
> +void vm_start(bool announce);
>  void vm_stop(RunState state);
>  void vm_stop_force_state(RunState state);
>  
> diff --git a/vl.c b/vl.c
> index 65f11f2..4e04f82 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state)
>      }
>  }
>  
> -void vm_start(void)
> +void vm_start(bool announce)
>  {
>      if (!runstate_is_running()) {
>          cpu_enable_ticks();
> @@ -1266,6 +1266,9 @@ void vm_start(void)
>          vm_state_notify(1, RUN_STATE_RUNNING);
>          resume_all_vcpus();
>          monitor_protocol_event(QEVENT_RESUME, NULL);
> +        if (announce) {
> +            qemu_announce_self();
> +        }
>      }
>  }
>  
> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp)
>              exit(ret);
>          }
>      } else if (autostart) {
> -        vm_start();
> +        vm_start(false);
>      }

This is RUN_STATE_PRELAUNCH.

To some up, it seems like whether to send an announcement depends on the
previous runstate: it should be sent only for RUN_STATE_INMIGRATE,
RUN_STATE_RESTORE_VM and (new with your patch) RUN_STATE_PAUSED.  So
perhaps the new argument to vm_start is not needed.

Paolo

>  
>      os_setup_post();
> 
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself Jason Wang
@ 2012-03-13 14:18   ` Michael S. Tsirkin
  2012-03-13 14:20   ` Michael S. Tsirkin
  1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2012-03-13 14:18 UTC (permalink / raw)
  To: Jason Wang; +Cc: quintela, aliguori, rusty, stefanha, qemu-devel

On Tue, Mar 13, 2012 at 04:56:40PM +0800, Jason Wang wrote:
> It's hard to track all mac addresses and their usage (vlan, bondings,
> ipv6) in qemu to send proper gratuitous packet. The better choice is
> to let guest to send them.
> 
> So, this patch introduces a new rw config status bit of virtio-net,
> VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
> presence of its link through config update interrupt. When gust have
> done the announcement, it should clear that bit. The feature is negotiated by
> a new feature bit VIRTIO_NET_F_ANNOUNCE.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/virtio-net.c |   19 ++++++++++++++++++-
>  hw/virtio-net.h |    3 +++
>  2 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index bc5e3a8..a591a48 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -95,6 +95,8 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>          memcpy(n->mac, netcfg.mac, ETH_ALEN);
>          qemu_format_nic_info_str(&n->nic->nc, n->mac);
>      }
> +
> +    memcpy(&n->status, &netcfg.status, sizeof(n->status));

This overwrites all of status, which seems wrong as
it will also overwrite the link state - that should
have been read-only.

>  }
>  
>  static bool virtio_net_started(VirtIONet *n, uint8_t status)
> @@ -227,7 +229,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>  {
>      VirtIONet *n = to_virtio_net(vdev);
>  
> -    features |= (1 << VIRTIO_NET_F_MAC);
> +    features |= (1 << VIRTIO_NET_F_MAC | 1 << VIRTIO_NET_F_GUEST_ANNOUNCE);
>  

Why do you force this bit on?
We have a property below ...

>      if (peer_has_vnet_hdr(n)) {
>          tap_using_vnet_hdr(n->nic->nc.peer, 1);
> @@ -983,6 +985,20 @@ static void virtio_net_cleanup(VLANClientState *nc)
>      n->nic = NULL;
>  }
>  
> +static int virtio_net_announce(VLANClientState *nc)
> +{
> +    VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque;
> +
> +    if (n->vdev.guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE)
> +        && n->status & VIRTIO_NET_S_LINK_UP) {
> +        n->status |= VIRTIO_NET_S_ANNOUNCE;
> +        virtio_notify_config(&n->vdev);
> +        return 0;
> +    }
> +
> +    return 1;
> +}
> +
>  static NetClientInfo net_virtio_info = {
>      .type = NET_CLIENT_TYPE_NIC,
>      .size = sizeof(NICState),
> @@ -990,6 +1006,7 @@ static NetClientInfo net_virtio_info = {
>      .receive = virtio_net_receive,
>          .cleanup = virtio_net_cleanup,
>      .link_status_changed = virtio_net_set_link_status,
> +    .announce = virtio_net_announce,
>  };
>  
>  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 4468741..9f8cea7 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -44,8 +44,10 @@
>  #define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
>  #define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering */
>  #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
> +#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Guest can announce itself */
>  
>  #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
> +#define VIRTIO_NET_S_ANNOUNCE   2       /* Announcement is needed */
>  
>  #define TX_TIMER_INTERVAL 150000 /* 150 us */
>  
> @@ -176,6 +178,7 @@ struct virtio_net_ctrl_mac {
>          DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
>          DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
>          DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
> +        DEFINE_PROP_BIT("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
>          DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
>          DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
>          DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support Jason Wang
@ 2012-03-13 14:20   ` Michael S. Tsirkin
  0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2012-03-13 14:20 UTC (permalink / raw)
  To: Jason Wang; +Cc: quintela, aliguori, rusty, stefanha, qemu-devel

On Tue, Mar 13, 2012 at 04:56:31PM +0800, Jason Wang wrote:
> This patch introduces a function pointer in NetClientInfo which is
> called during self announcement. With this, each kind of card can announce the
> link with a specific way. The old method is still kept for cards that have not
> implemented this or old guest. The first user would be virtio-net.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  net.h    |    2 ++
>  savevm.c |    8 +++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net.h b/net.h
> index 75a8c15..7195bfc 100644
> --- a/net.h
> +++ b/net.h
> @@ -48,6 +48,7 @@ typedef ssize_t (NetReceive)(VLANClientState *, const uint8_t *, size_t);
>  typedef ssize_t (NetReceiveIOV)(VLANClientState *, const struct iovec *, int);
>  typedef void (NetCleanup) (VLANClientState *);
>  typedef void (LinkStatusChanged)(VLANClientState *);
> +typedef int (NetAnnounce)(VLANClientState *);
>  
>  typedef struct NetClientInfo {
>      net_client_type type;
> @@ -59,6 +60,7 @@ typedef struct NetClientInfo {
>      NetCleanup *cleanup;
>      LinkStatusChanged *link_status_changed;
>      NetPoll *poll;
> +    NetAnnounce *announce;
>  } NetClientInfo;
>  
>  struct VLANClientState {
> diff --git a/savevm.c b/savevm.c
> index 82b9d3a..0a901dc 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -123,10 +123,12 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
>  {
>      uint8_t buf[60];
>      int len;
> +    NetAnnounce *func = nic->nc.info->announce;
>  
> -    len = announce_self_create(buf, nic->conf->macaddr.a);
> -
> -    qemu_send_packet_raw(&nic->nc, buf, len);
> +    if (func == NULL || func(&nic->nc) != 0) {

Shorter as !func || func(&nic->nc)

> +        len = announce_self_create(buf, nic->conf->macaddr.a);
> +        qemu_send_packet_raw(&nic->nc, buf, len);
> +    }
>  }
>  
>  

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself Jason Wang
  2012-03-13 14:18   ` Michael S. Tsirkin
@ 2012-03-13 14:20   ` Michael S. Tsirkin
  1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2012-03-13 14:20 UTC (permalink / raw)
  To: Jason Wang; +Cc: quintela, aliguori, rusty, stefanha, qemu-devel

On Tue, Mar 13, 2012 at 04:56:40PM +0800, Jason Wang wrote:
> It's hard to track all mac addresses and their usage (vlan, bondings,
> ipv6) in qemu to send proper gratuitous packet. The better choice is
> to let guest to send them.
> 
> So, this patch introduces a new rw config status bit of virtio-net,
> VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
> presence of its link through config update interrupt. When gust have
> done the announcement, it should clear that bit. The feature is negotiated by
> a new feature bit VIRTIO_NET_F_ANNOUNCE.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/virtio-net.c |   19 ++++++++++++++++++-
>  hw/virtio-net.h |    3 +++
>  2 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index bc5e3a8..a591a48 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -95,6 +95,8 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>          memcpy(n->mac, netcfg.mac, ETH_ALEN);
>          qemu_format_nic_info_str(&n->nic->nc, n->mac);
>      }
> +
> +    memcpy(&n->status, &netcfg.status, sizeof(n->status));
>  }
>  
>  static bool virtio_net_started(VirtIONet *n, uint8_t status)
> @@ -227,7 +229,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>  {
>      VirtIONet *n = to_virtio_net(vdev);
>  
> -    features |= (1 << VIRTIO_NET_F_MAC);
> +    features |= (1 << VIRTIO_NET_F_MAC | 1 << VIRTIO_NET_F_GUEST_ANNOUNCE);
>  
>      if (peer_has_vnet_hdr(n)) {
>          tap_using_vnet_hdr(n->nic->nc.peer, 1);
> @@ -983,6 +985,20 @@ static void virtio_net_cleanup(VLANClientState *nc)
>      n->nic = NULL;
>  }
>  
> +static int virtio_net_announce(VLANClientState *nc)
> +{
> +    VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque;
> +
> +    if (n->vdev.guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE)
> +        && n->status & VIRTIO_NET_S_LINK_UP) {
> +        n->status |= VIRTIO_NET_S_ANNOUNCE;
> +        virtio_notify_config(&n->vdev);
> +        return 0;
> +    }
> +
> +    return 1;
> +}
> +

Returning 1 on error is confusing. return -1 would be better.

>  static NetClientInfo net_virtio_info = {
>      .type = NET_CLIENT_TYPE_NIC,
>      .size = sizeof(NICState),
> @@ -990,6 +1006,7 @@ static NetClientInfo net_virtio_info = {
>      .receive = virtio_net_receive,
>          .cleanup = virtio_net_cleanup,
>      .link_status_changed = virtio_net_set_link_status,
> +    .announce = virtio_net_announce,
>  };
>  
>  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 4468741..9f8cea7 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -44,8 +44,10 @@
>  #define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
>  #define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering */
>  #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
> +#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Guest can announce itself */
>  
>  #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
> +#define VIRTIO_NET_S_ANNOUNCE   2       /* Announcement is needed */
>  
>  #define TX_TIMER_INTERVAL 150000 /* 150 us */
>  
> @@ -176,6 +178,7 @@ struct virtio_net_ctrl_mac {
>          DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
>          DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
>          DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
> +        DEFINE_PROP_BIT("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
>          DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
>          DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
>          DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start
  2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start Jason Wang
  2012-03-13  9:18   ` Paolo Bonzini
@ 2012-03-13 14:23   ` Michael S. Tsirkin
  2012-03-15  6:11     ` Jason Wang
  1 sibling, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2012-03-13 14:23 UTC (permalink / raw)
  To: Jason Wang; +Cc: quintela, aliguori, rusty, stefanha, qemu-devel

On Tue, Mar 13, 2012 at 04:56:22PM +0800, Jason Wang wrote:
> This patch moves qemu_announce_self() to vm_start() and add a new parameters to
> control whether sending gratuitous packet is needed. There are several reasons
> to do this:
> 
> - Gratuitous packet is also needed when we resume a stopped vm or successfully
>   load a state.

Why is it needed when we continue a stopped vm?

> - Sending gratuitous packets may be done through co-operation with guest, so
>   this work should be done after vm is started.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  gdbstub.c   |    2 +-
>  migration.c |    5 ++---
>  monitor.c   |    2 +-
>  qmp.c       |    2 +-
>  savevm.c    |    2 +-
>  sysemu.h    |    2 +-
>  vl.c        |    7 +++++--
>  7 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index ef95ac2..f4d22e5 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s)
>  #ifdef CONFIG_USER_ONLY
>      s->running_state = 1;
>  #else
> -    vm_start();
> +    vm_start(false);
>  #endif
>  }
>  
> diff --git a/migration.c b/migration.c
> index 00fa1e3..40332d2 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -88,14 +88,13 @@ void process_incoming_migration(QEMUFile *f)
>          fprintf(stderr, "load of migration failed\n");
>          exit(0);
>      }
> -    qemu_announce_self();
>      DPRINTF("successfully loaded vm state\n");
>  
>      /* Make sure all file formats flush their mutable metadata */
>      bdrv_invalidate_cache_all();
>  
>      if (autostart) {
> -        vm_start();
> +        vm_start(true);
>      } else {
>          runstate_set(RUN_STATE_PRELAUNCH);
>      }
> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque)
>          }
>          if (s->state != MIG_STATE_COMPLETED) {
>              if (old_vm_running) {
> -                vm_start();
> +                vm_start(false);
>              }
>          }
>      }
> diff --git a/monitor.c b/monitor.c
> index cbdfbad..0b63c11 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
>      vm_stop(RUN_STATE_RESTORE_VM);
>  
>      if (load_vmstate(name) == 0 && saved_vm_running) {
> -        vm_start();
> +        vm_start(true);
>      }
>  }
>  
> diff --git a/qmp.c b/qmp.c
> index a182b51..252c842 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp)
>          return;
>      }
>  
> -    vm_start();
> +    vm_start(true);
>  }
>  
>  void qmp_system_wakeup(Error **errp)
> diff --git a/savevm.c b/savevm.c
> index 5b59826..82b9d3a 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>  
>   the_end:
>      if (saved_vm_running)
> -        vm_start();
> +        vm_start(false);
>  }
>  
>  int load_vmstate(const char *name)
> diff --git a/sysemu.h b/sysemu.h
> index 98118cc..787edd4 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state);
>  #define VMRESET_SILENT   false
>  #define VMRESET_REPORT   true
>  
> -void vm_start(void);
> +void vm_start(bool announce);
>  void vm_stop(RunState state);
>  void vm_stop_force_state(RunState state);
>  
> diff --git a/vl.c b/vl.c
> index 65f11f2..4e04f82 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state)
>      }
>  }
>  
> -void vm_start(void)
> +void vm_start(bool announce)
>  {
>      if (!runstate_is_running()) {
>          cpu_enable_ticks();
> @@ -1266,6 +1266,9 @@ void vm_start(void)
>          vm_state_notify(1, RUN_STATE_RUNNING);
>          resume_all_vcpus();
>          monitor_protocol_event(QEVENT_RESUME, NULL);
> +        if (announce) {
> +            qemu_announce_self();
> +        }
>      }
>  }
>  
> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp)
>              exit(ret);
>          }
>      } else if (autostart) {
> -        vm_start();
> +        vm_start(false);
>      }
>  
>      os_setup_post();

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start
  2012-03-13  9:18   ` Paolo Bonzini
@ 2012-03-15  6:08     ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-15  6:08 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: aliguori, stefanha, quintela, rusty, qemu-devel, mst

On 03/13/2012 05:18 PM, Paolo Bonzini wrote:
> Il 13/03/2012 09:56, Jason Wang ha scritto:
>> This patch moves qemu_announce_self() to vm_start() and add a new parameters to
>> control whether sending gratuitous packet is needed. There are several reasons
>> to do this:
>>
>> - Gratuitous packet is also needed when we resume a stopped vm or successfully
>>    load a state.
>> - Sending gratuitous packets may be done through co-operation with guest, so
>>    this work should be done after vm is started.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>>   gdbstub.c   |    2 +-
>>   migration.c |    5 ++---
>>   monitor.c   |    2 +-
>>   qmp.c       |    2 +-
>>   savevm.c    |    2 +-
>>   sysemu.h    |    2 +-
>>   vl.c        |    7 +++++--
>>   7 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index ef95ac2..f4d22e5 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s)
>>   #ifdef CONFIG_USER_ONLY
>>       s->running_state = 1;
>>   #else
>> -    vm_start();
>> +    vm_start(false);
>>   #endif
> Here we're switching from RUNSTATE_DEBUG.
>
>>   }
>>
>> diff --git a/migration.c b/migration.c
>> index 00fa1e3..40332d2 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -88,14 +88,13 @@ void process_incoming_migration(QEMUFile *f)
>>           fprintf(stderr, "load of migration failed\n");
>>           exit(0);
>>       }
>> -    qemu_announce_self();
>>       DPRINTF("successfully loaded vm state\n");
>>
>>       /* Make sure all file formats flush their mutable metadata */
>>       bdrv_invalidate_cache_all();
>>
>>       if (autostart) {
>> -        vm_start();
>> +        vm_start(true);
> Here from RUN_STATE_INMIGRATE.
>
>>       } else {
>>           runstate_set(RUN_STATE_PRELAUNCH);
>>       }
>> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque)
>>           }
>>           if (s->state != MIG_STATE_COMPLETED) {
>>               if (old_vm_running) {
>> -                vm_start();
>> +                vm_start(false);
> Here from RUN_STATE_FINISH_MIGRATE.
>
>>               }
>>           }
>>       }
>> diff --git a/monitor.c b/monitor.c
>> index cbdfbad..0b63c11 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
>>       vm_stop(RUN_STATE_RESTORE_VM);
>>
>>       if (load_vmstate(name) == 0&&  saved_vm_running) {
>> -        vm_start();
>> +        vm_start(true);
> Here from RUN_STATE_RESTORE_VM.
>
>>       }
>>   }
>>
>> diff --git a/qmp.c b/qmp.c
>> index a182b51..252c842 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp)
>>           return;
>>       }
>>
>> -    vm_start();
>> +    vm_start(true);
> Here from RUN_STATE_PAUSED or RUN_STATE_PRELAUNCH.
>
> This introduces a difference here with "qemu -S" + cont, and "qemu".
> The former will send a gratuitous ARP, the latter won't.  Is this
> desired/harmless/...?

Not desired but harmless I think.

And indeed there're two possibilities:

- start a fresh vm with -S, and then continue the vm.
- migrate a guest with -S option used in destination, and then continue 
the vm.

It's a little hard to differentiate one from another just in vm_stop().
>
>>   }
>>
>>   void qmp_system_wakeup(Error **errp)
>> diff --git a/savevm.c b/savevm.c
>> index 5b59826..82b9d3a 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>>
>>    the_end:
>>       if (saved_vm_running)
>> -        vm_start();
>> +        vm_start(false);
>>   }
> This is RUN_STATE_SAVE_VM.
>
>>
>>   int load_vmstate(const char *name)
>> diff --git a/sysemu.h b/sysemu.h
>> index 98118cc..787edd4 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state);
>>   #define VMRESET_SILENT   false
>>   #define VMRESET_REPORT   true
>>
>> -void vm_start(void);
>> +void vm_start(bool announce);
>>   void vm_stop(RunState state);
>>   void vm_stop_force_state(RunState state);
>>
>> diff --git a/vl.c b/vl.c
>> index 65f11f2..4e04f82 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state)
>>       }
>>   }
>>
>> -void vm_start(void)
>> +void vm_start(bool announce)
>>   {
>>       if (!runstate_is_running()) {
>>           cpu_enable_ticks();
>> @@ -1266,6 +1266,9 @@ void vm_start(void)
>>           vm_state_notify(1, RUN_STATE_RUNNING);
>>           resume_all_vcpus();
>>           monitor_protocol_event(QEVENT_RESUME, NULL);
>> +        if (announce) {
>> +            qemu_announce_self();
>> +        }
>>       }
>>   }
>>
>> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp)
>>               exit(ret);
>>           }
>>       } else if (autostart) {
>> -        vm_start();
>> +        vm_start(false);
>>       }
> This is RUN_STATE_PRELAUNCH.
>
> To some up, it seems like whether to send an announcement depends on the
> previous runstate: it should be sent only for RUN_STATE_INMIGRATE,
> RUN_STATE_RESTORE_VM and (new with your patch) RUN_STATE_PAUSED.  So
> perhaps the new argument to vm_start is not needed.
>
> Paolo

Make sense, thanks.
>>
>>       os_setup_post();
>>
>>
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start
  2012-03-13 14:23   ` Michael S. Tsirkin
@ 2012-03-15  6:11     ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2012-03-15  6:11 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: quintela, aliguori, rusty, stefanha, qemu-devel

On 03/13/2012 10:23 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 13, 2012 at 04:56:22PM +0800, Jason Wang wrote:
>> This patch moves qemu_announce_self() to vm_start() and add a new parameters to
>> control whether sending gratuitous packet is needed. There are several reasons
>> to do this:
>>
>> - Gratuitous packet is also needed when we resume a stopped vm or successfully
>>    load a state.
> Why is it needed when we continue a stopped vm?

If we stop a vm for a little long time, the mac table entry in the 
switch would also be expired. So we need to announce again.
>
>> - Sending gratuitous packets may be done through co-operation with guest, so
>>    this work should be done after vm is started.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>>   gdbstub.c   |    2 +-
>>   migration.c |    5 ++---
>>   monitor.c   |    2 +-
>>   qmp.c       |    2 +-
>>   savevm.c    |    2 +-
>>   sysemu.h    |    2 +-
>>   vl.c        |    7 +++++--
>>   7 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index ef95ac2..f4d22e5 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -371,7 +371,7 @@ static inline void gdb_continue(GDBState *s)
>>   #ifdef CONFIG_USER_ONLY
>>       s->running_state = 1;
>>   #else
>> -    vm_start();
>> +    vm_start(false);
>>   #endif
>>   }
>>
>> diff --git a/migration.c b/migration.c
>> index 00fa1e3..40332d2 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -88,14 +88,13 @@ void process_incoming_migration(QEMUFile *f)
>>           fprintf(stderr, "load of migration failed\n");
>>           exit(0);
>>       }
>> -    qemu_announce_self();
>>       DPRINTF("successfully loaded vm state\n");
>>
>>       /* Make sure all file formats flush their mutable metadata */
>>       bdrv_invalidate_cache_all();
>>
>>       if (autostart) {
>> -        vm_start();
>> +        vm_start(true);
>>       } else {
>>           runstate_set(RUN_STATE_PRELAUNCH);
>>       }
>> @@ -274,7 +273,7 @@ static void migrate_fd_put_ready(void *opaque)
>>           }
>>           if (s->state != MIG_STATE_COMPLETED) {
>>               if (old_vm_running) {
>> -                vm_start();
>> +                vm_start(false);
>>               }
>>           }
>>       }
>> diff --git a/monitor.c b/monitor.c
>> index cbdfbad..0b63c11 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2260,7 +2260,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
>>       vm_stop(RUN_STATE_RESTORE_VM);
>>
>>       if (load_vmstate(name) == 0&&  saved_vm_running) {
>> -        vm_start();
>> +        vm_start(true);
>>       }
>>   }
>>
>> diff --git a/qmp.c b/qmp.c
>> index a182b51..252c842 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -160,7 +160,7 @@ void qmp_cont(Error **errp)
>>           return;
>>       }
>>
>> -    vm_start();
>> +    vm_start(true);
>>   }
>>
>>   void qmp_system_wakeup(Error **errp)
>> diff --git a/savevm.c b/savevm.c
>> index 5b59826..82b9d3a 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -2107,7 +2107,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>>
>>    the_end:
>>       if (saved_vm_running)
>> -        vm_start();
>> +        vm_start(false);
>>   }
>>
>>   int load_vmstate(const char *name)
>> diff --git a/sysemu.h b/sysemu.h
>> index 98118cc..787edd4 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -34,7 +34,7 @@ void vm_state_notify(int running, RunState state);
>>   #define VMRESET_SILENT   false
>>   #define VMRESET_REPORT   true
>>
>> -void vm_start(void);
>> +void vm_start(bool announce);
>>   void vm_stop(RunState state);
>>   void vm_stop_force_state(RunState state);
>>
>> diff --git a/vl.c b/vl.c
>> index 65f11f2..4e04f82 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1258,7 +1258,7 @@ void vm_state_notify(int running, RunState state)
>>       }
>>   }
>>
>> -void vm_start(void)
>> +void vm_start(bool announce)
>>   {
>>       if (!runstate_is_running()) {
>>           cpu_enable_ticks();
>> @@ -1266,6 +1266,9 @@ void vm_start(void)
>>           vm_state_notify(1, RUN_STATE_RUNNING);
>>           resume_all_vcpus();
>>           monitor_protocol_event(QEVENT_RESUME, NULL);
>> +        if (announce) {
>> +            qemu_announce_self();
>> +        }
>>       }
>>   }
>>
>> @@ -3619,7 +3622,7 @@ int main(int argc, char **argv, char **envp)
>>               exit(ret);
>>           }
>>       } else if (autostart) {
>> -        vm_start();
>> +        vm_start(false);
>>       }
>>
>>       os_setup_post();

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-03-15  6:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-13  8:56 [Qemu-devel] [V4 PATCH 0/5] Send the gratuitous packets by guest Jason Wang
2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 1/5] net: reset the count after rounds of announcing Jason Wang
2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 2/5] net: announce self after vm start Jason Wang
2012-03-13  9:18   ` Paolo Bonzini
2012-03-15  6:08     ` Jason Wang
2012-03-13 14:23   ` Michael S. Tsirkin
2012-03-15  6:11     ` Jason Wang
2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 3/5] net: model specific announcing support Jason Wang
2012-03-13 14:20   ` Michael S. Tsirkin
2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 4/5] virtio-net: notify guest to annouce itself Jason Wang
2012-03-13 14:18   ` Michael S. Tsirkin
2012-03-13 14:20   ` Michael S. Tsirkin
2012-03-13  8:56 ` [Qemu-devel] [V4 PATCH 5/5] virtio-net: compat guest announce support Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).