Patchwork [7/7] net: fix qemu_announce_self()

login
register
mail settings
Submitter Mark McLoughlin
Date Nov. 12, 2009, 8:29 p.m.
Message ID <1258057742-18699-8-git-send-email-markmc@redhat.com>
Download mbox | patch
Permalink /patch/38274/
State New
Headers show

Comments

Mark McLoughlin - Nov. 12, 2009, 8:29 p.m.
Now that we have a sane way of iterating over NICs.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 savevm.c |   43 ++++++++++++++++++++++++++-----------------
 1 files changed, 26 insertions(+), 17 deletions(-)
Mark McLoughlin - Nov. 13, 2009, 7:41 a.m.
On Thu, 2009-11-12 at 20:29 +0000, Mark McLoughlin wrote:
...
> +    printf("qemu_announce_self_iter() mac = %p\n", mac);
> +
> +    len = announce_self_create(buf, mac);
> +
> +    printf("sending packet from %s\n", client->name);

Ooops, some debugging leftovers. removed in my tree.

Cheers,
Mark.
Markus Armbruster - Nov. 13, 2009, 8:14 a.m.
Mark McLoughlin <markmc@redhat.com> writes:

> Now that we have a sane way of iterating over NICs.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  savevm.c |   43 ++++++++++++++++++++++++++-----------------
>  1 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 039740c..3736588 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -75,6 +75,7 @@
>  
>  #include "qemu-common.h"
>  #include "hw/hw.h"
> +#include "hw/qdev.h"
>  #include "net.h"
>  #include "monitor.h"
>  #include "sysemu.h"
> @@ -123,28 +124,36 @@ static int announce_self_create(uint8_t *buf,
>      return 60; /* len (FCS will be added by hardware) */
>  }
>  
> -static void qemu_announce_self_once(void *opaque)
> +static void qemu_announce_self_iter(DeviceState *dev, void *opaque)
>  {
> -    int i, len;
> -#ifdef FIXME
> -    VLANState *vlan;
> -    VLANClientState *vc;
> -#endif
> +    VLANClientState *client;
> +    uint8_t *mac;
>      uint8_t buf[60];
> +    int len;
> +
> +    if (!qdev_prop_exists(dev, "net-client")) {
> +        return;
> +    }

This is an ad-hoc "is this a NIC" test.  It assumes all NICs have a
"net-client" property (trivial), and that no other kind of device ever
has a "net-client" property (reasonable).  But, as we accumulate more of
this type of tests, we'll accumulate more of these assumptions, and
having them spread over the code could be confusing.  What about
collecting the "is this a DEVICE_KIND" tests in one place?

For the record, I still believe that identifying device kind by testing
presence of properties is suboptimal[*], but we got more important fish
to fry.

> +
> +    client = qdev_prop_get_net_client(dev, "net-client");
> +    mac = qdev_prop_get_macaddr(dev, "mac");
> +
> +    printf("qemu_announce_self_iter() mac = %p\n", mac);
> +
> +    len = announce_self_create(buf, mac);
> +
> +    printf("sending packet from %s\n", client->name);
> +
> +    qemu_send_packet_raw(client, buf, len);
> +}
[...]

[*] There's no way to test "is this a watchdog" by testing presence of
properties.  Ability to identify watchdog devices would let us kill
watchdog_list.

Patch

diff --git a/savevm.c b/savevm.c
index 039740c..3736588 100644
--- a/savevm.c
+++ b/savevm.c
@@ -75,6 +75,7 @@ 
 
 #include "qemu-common.h"
 #include "hw/hw.h"
+#include "hw/qdev.h"
 #include "net.h"
 #include "monitor.h"
 #include "sysemu.h"
@@ -123,28 +124,36 @@  static int announce_self_create(uint8_t *buf,
     return 60; /* len (FCS will be added by hardware) */
 }
 
-static void qemu_announce_self_once(void *opaque)
+static void qemu_announce_self_iter(DeviceState *dev, void *opaque)
 {
-    int i, len;
-#ifdef FIXME
-    VLANState *vlan;
-    VLANClientState *vc;
-#endif
+    VLANClientState *client;
+    uint8_t *mac;
     uint8_t buf[60];
+    int len;
+
+    if (!qdev_prop_exists(dev, "net-client")) {
+        return;
+    }
+
+    client = qdev_prop_get_net_client(dev, "net-client");
+    mac = qdev_prop_get_macaddr(dev, "mac");
+
+    printf("qemu_announce_self_iter() mac = %p\n", mac);
+
+    len = announce_self_create(buf, mac);
+
+    printf("sending packet from %s\n", client->name);
+
+    qemu_send_packet_raw(client, buf, len);
+}
+
+static void qemu_announce_self_once(void *opaque)
+{
     static int count = SELF_ANNOUNCE_ROUNDS;
     QEMUTimer *timer = *(QEMUTimer **)opaque;
 
-    for (i = 0; i < MAX_NICS; i++) {
-        if (!nd_table[i].used)
-            continue;
-        len = announce_self_create(buf, nd_table[i].macaddr);
-#ifdef FIXME
-        vlan = nd_table[i].vlan;
-        QTAILQ_FOREACH(vc, &vlan->clients, next) {
-            qemu_send_packet_raw(vc, buf, len);
-        }
-#endif
-    }
+    qdev_foreach(qemu_announce_self_iter, NULL);
+
     if (--count) {
         /* delay 50ms, 150ms, 250ms, ... */
         qemu_mod_timer(timer, qemu_get_clock(rt_clock) +