diff mbox

net: roll back qdev_prop_vlan

Message ID 1339864232-9171-1-git-send-email-zwu.kernel@gmail.com
State New
Headers show

Commit Message

Zhiyong Wu June 16, 2012, 4:30 p.m. UTC 
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

We're trying to preserve backward compatibility.  This
command-line break:

x86_64-softmmu/qemu-system-x86_64 -net user,vlan=1 -device virtio-net-pci,vlan=1

Instead of dropping the qdev_prop_vlan completely the
hw/qdev-properties.c code needs to call net/hub.h external functions
to implement equivalent functionality.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 hw/qdev-properties.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    3 ++
 net.h                |    1 +
 net/hub.c            |   25 ++++++++++++++++
 net/hub.h            |    1 +
 5 files changed, 106 insertions(+), 0 deletions(-)

Comments

Stefan Hajnoczi June 18, 2012, 10:22 a.m. UTC | #1 
On Sun, Jun 17, 2012 at 12:30:32AM +0800, zwu.kernel@gmail.com wrote:
> +static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
> +{
> +    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
> +
> +    if (*ptr) {
> +        unsigned int id;
> +        if (!net_hub_id_for_client(*ptr, &id)) {
> +            return snprintf(dest, len, "%d", id);

Unsigned int should be %u.  Source code scanners or the compiler could
warn about this so it's worth changing.

> +        }
> +    }
> +
> +    return snprintf(dest, len, "<null>");
> +}
> +
> +static void get_vlan(Object *obj, Visitor *v, void *opaque,
> +                     const char *name, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
> +    int64_t id = -1;
> +
> +    if (*ptr) {
> +        unsigned int hub_id;
> +        net_hub_id_for_client(*ptr, &hub_id);

It's unclear what happens if net_hub_id_for_client() fails but it looks
like hub_id may be uninitialized.

Stefan
Zhiyong Wu June 18, 2012, 12:48 p.m. UTC | #2 
Good catch, thanks, please next version.

On Mon, Jun 18, 2012 at 6:22 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Sun, Jun 17, 2012 at 12:30:32AM +0800, zwu.kernel@gmail.com wrote:
>> +static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
>> +{
>> +    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
>> +
>> +    if (*ptr) {
>> +        unsigned int id;
>> +        if (!net_hub_id_for_client(*ptr, &id)) {
>> +            return snprintf(dest, len, "%d", id);
>
> Unsigned int should be %u.  Source code scanners or the compiler could
> warn about this so it's worth changing.
>
>> +        }
>> +    }
>> +
>> +    return snprintf(dest, len, "<null>");
>> +}
>> +
>> +static void get_vlan(Object *obj, Visitor *v, void *opaque,
>> +                     const char *name, Error **errp)
>> +{
>> +    DeviceState *dev = DEVICE(obj);
>> +    Property *prop = opaque;
>> +    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
>> +    int64_t id = -1;
>> +
>> +    if (*ptr) {
>> +        unsigned int hub_id;
>> +        net_hub_id_for_client(*ptr, &hub_id);
>
> It's unclear what happens if net_hub_id_for_client() fails but it looks
> like hub_id may be uninitialized.
>
> Stefan
>
diff mbox

Patch

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1c13bda..91328eb 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -2,6 +2,7 @@ 
 #include "qdev.h"
 #include "qerror.h"
 #include "blockdev.h"
+#include "net/hub.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
@@ -623,6 +624,81 @@  PropertyInfo qdev_prop_netdev = {
     .set   = set_netdev,
 };
 
+/* --- vlan --- */
+
+static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        unsigned int id;
+        if (!net_hub_id_for_client(*ptr, &id)) {
+            return snprintf(dest, len, "%d", id);
+        }
+    }
+
+    return snprintf(dest, len, "<null>");
+}
+
+static void get_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    int64_t id = -1;
+
+    if (*ptr) {
+        unsigned int hub_id;
+        net_hub_id_for_client(*ptr, &hub_id);
+        id = (int64_t)hub_id;
+    }
+
+    visit_type_int(v, &id, name, errp);
+}
+
+static void set_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    Error *local_err = NULL;
+    int64_t id;
+    NetClientState *hubport;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_int(v, &id, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (id == -1) {
+        *ptr = NULL;
+        return;
+    }
+
+    hubport = net_hub_port_find(id);
+    if (!hubport) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  name, prop->info->name);
+        return;
+    }
+    *ptr = hubport;
+}
+
+PropertyInfo qdev_prop_vlan = {
+    .name  = "vlan",
+    .print = print_vlan,
+    .get   = get_vlan,
+    .set   = set_vlan,
+};
+
 /* --- pointer --- */
 
 /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
diff --git a/hw/qdev.h b/hw/qdev.h
index edbf8fa..f4aea27 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -222,6 +222,7 @@  extern PropertyInfo qdev_prop_macaddr;
 extern PropertyInfo qdev_prop_losttickpolicy;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
 extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 
@@ -276,6 +277,8 @@  extern PropertyInfo qdev_prop_blocksize;
     DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
 #define DEFINE_PROP_NETDEV(_n, _s, _f)             \
     DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
+#define DEFINE_PROP_VLAN(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
 #define DEFINE_PROP_DRIVE(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
 #define DEFINE_PROP_MACADDR(_n, _s, _f)         \
diff --git a/net.h b/net.h
index 08306a4..c4e56cc 100644
--- a/net.h
+++ b/net.h
@@ -22,6 +22,7 @@  typedef struct NICConf {
 
 #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
     DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr),                \
+    DEFINE_PROP_VLAN("vlan",     _state, _conf.peer),                   \
     DEFINE_PROP_NETDEV("netdev", _state, _conf.peer),                   \
     DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)
 
diff --git a/net/hub.c b/net/hub.c
index efd90b5..001f818 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -205,6 +205,31 @@  NetClientState *net_hub_find_client_by_name(unsigned int hub_id,
 }
 
 /**
+ * Find a available port on a hub; otherwise create one new port
+ */
+NetClientState *net_hub_port_find(unsigned int hub_id)
+{
+    NetHub *hub;
+    NetHubPort *port;
+    NetClientState *nc;
+
+    QLIST_FOREACH(hub, &hubs, next) {
+        if (hub->id == hub_id) {
+            QLIST_FOREACH(port, &hub->ports, next) {
+                nc = port->nc.peer;
+                if (!nc) {
+                    return &(port->nc);
+                }
+            }
+            break;
+        }
+    }
+
+    nc = net_hub_add_port(hub_id);
+    return nc;
+}
+
+/**
  * Determine if one nc peers with one hub port
  */
 bool net_hub_port_peer_nc(NetClientState *nc)
diff --git a/net/hub.h b/net/hub.h
index 550189b..54d6803 100644
--- a/net/hub.h
+++ b/net/hub.h
@@ -23,6 +23,7 @@  NetClientState *net_hub_find_client_by_name(unsigned int hub_id,
 void net_hub_info(Monitor *mon);
 int net_hub_id_for_client(NetClientState *nc, unsigned int *id);
 void net_hub_check_clients(void);
+NetClientState *net_hub_port_find(unsigned int hub_id);
 bool net_hub_port_peer_nc(NetClientState *nc);
 
 #endif /* NET_HUB_H */