* [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
@ 2014-08-28 7:41 Jason Wang
2014-09-02 10:38 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2014-08-28 7:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Jason Wang, afaerber, Stefan Hajnoczi, mst
Commit 1ceef9f27359cbe92ef124bf74de6f792e71f6fb (net: multiqueue
support) tries to use set_pointer() and get_pointer() to set and get
NICPeers which is not a pointer defined in DEFINE_PROP_NETDEV. This
trick works but result a unclean and fragile implementation (e.g
print_netdev and parse_netdev).
This patch solves this issue by not using set/get_pinter() and set and
get netdev directly in set_netdev() and get_netdev(). After this the
parse_netdev() and print_netdev() were no longer used and dropped from
the source.
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/core/qdev-properties-system.c | 71 ++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 32 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index ae0900f..b3753ce 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -176,41 +176,67 @@ PropertyInfo qdev_prop_chr = {
};
/* --- netdev device --- */
+static void get_netdev(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
+ char *p = g_strdup(peers_ptr->ncs[0]->name);
-static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
+ visit_type_str(v, &p, name, errp);
+ g_free(p);
+}
+
+static void set_netdev(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
- NICPeers *peers_ptr = (NICPeers *)ptr;
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
NetClientState **ncs = peers_ptr->ncs;
NetClientState *peers[MAX_QUEUE_NUM];
- int queues, i = 0;
- int ret;
+ Error *local_err = NULL;
+ int err, queues, i = 0;
+ char *str;
+
+ if (dev->realized) {
+ qdev_prop_set_after_realize(dev, name, errp);
+ return;
+ }
+
+ visit_type_str(v, &str, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
queues = qemu_find_net_clients_except(str, peers,
NET_CLIENT_OPTIONS_KIND_NIC,
MAX_QUEUE_NUM);
if (queues == 0) {
- ret = -ENOENT;
+ err = -ENOENT;
goto err;
}
if (queues > MAX_QUEUE_NUM) {
- ret = -E2BIG;
+ err = -E2BIG;
goto err;
}
for (i = 0; i < queues; i++) {
if (peers[i] == NULL) {
- ret = -ENOENT;
+ err = -ENOENT;
goto err;
}
if (peers[i]->peer) {
- ret = -EEXIST;
+ err = -EEXIST;
goto err;
}
if (ncs[i]) {
- ret = -EINVAL;
+ err = -EINVAL;
goto err;
}
@@ -219,31 +245,12 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
}
peers_ptr->queues = queues;
-
- return 0;
+ g_free(str);
+ return;
err:
- return ret;
-}
-
-static char *print_netdev(void *ptr)
-{
- NetClientState *netdev = ptr;
- const char *val = netdev->name ? netdev->name : "";
-
- return g_strdup(val);
-}
-
-static void get_netdev(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
- get_pointer(obj, v, opaque, print_netdev, name, errp);
-}
-
-static void set_netdev(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
- set_pointer(obj, v, opaque, parse_netdev, name, errp);
+ error_set_from_qdev_prop_error(errp, err, dev, prop, str);
+ g_free(str);
}
PropertyInfo qdev_prop_netdev = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
2014-08-28 7:41 [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev() Jason Wang
@ 2014-09-02 10:38 ` Stefan Hajnoczi
2014-09-04 16:21 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-09-02 10:38 UTC (permalink / raw)
To: Jason Wang; +Cc: Markus Armbruster, qemu-devel, afaerber, mst
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
On Thu, Aug 28, 2014 at 03:41:25PM +0800, Jason Wang wrote:
> Commit 1ceef9f27359cbe92ef124bf74de6f792e71f6fb (net: multiqueue
> support) tries to use set_pointer() and get_pointer() to set and get
> NICPeers which is not a pointer defined in DEFINE_PROP_NETDEV. This
> trick works but result a unclean and fragile implementation (e.g
> print_netdev and parse_netdev).
>
> This patch solves this issue by not using set/get_pinter() and set and
> get netdev directly in set_netdev() and get_netdev(). After this the
> parse_netdev() and print_netdev() were no longer used and dropped from
> the source.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> hw/core/qdev-properties-system.c | 71 ++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 32 deletions(-)
Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
2014-09-02 10:38 ` Stefan Hajnoczi
@ 2014-09-04 16:21 ` Stefan Hajnoczi
2014-09-04 16:40 ` Peter Maydell
2014-09-09 3:01 ` Jason Wang
0 siblings, 2 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-09-04 16:21 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Jason Wang, Michael S. Tsirkin, Markus Armbruster,
Andreas Färber, qemu-devel
Unfortunately this patch breaks aarch64-softmmu qtests:
GTESTER check-qtest-aarch64
Broken pipe
GTester: last random seed: R02S6d8ab263ca56f8ae7a4b47bdf93fbc73
Please take a look at what is causing this.
Dropped from the net branch.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
2014-09-04 16:21 ` Stefan Hajnoczi
@ 2014-09-04 16:40 ` Peter Maydell
2014-09-09 3:03 ` Jason Wang
2014-09-09 3:01 ` Jason Wang
1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2014-09-04 16:40 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Michael S. Tsirkin, Jason Wang, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, Andreas Färber
On 4 September 2014 17:21, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> Unfortunately this patch breaks aarch64-softmmu qtests:
> GTESTER check-qtest-aarch64
> Broken pipe
> GTester: last random seed: R02S6d8ab263ca56f8ae7a4b47bdf93fbc73
>
> Please take a look at what is causing this.
Specifically, it breaks on all the ARM boards which have
more than one built in ethernet device. Something in your
patch is probably assuming there is only one (there's
a somewhat suspicious "[0]" array reference, for instance).
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
2014-09-04 16:21 ` Stefan Hajnoczi
2014-09-04 16:40 ` Peter Maydell
@ 2014-09-09 3:01 ` Jason Wang
1 sibling, 0 replies; 6+ messages in thread
From: Jason Wang @ 2014-09-09 3:01 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefan Hajnoczi
Cc: qemu-devel, Markus Armbruster, Andreas Färber, Michael S. Tsirkin
On 09/05/2014 12:21 AM, Stefan Hajnoczi wrote:
> Unfortunately this patch breaks aarch64-softmmu qtests:
> GTESTER check-qtest-aarch64
> Broken pipe
> GTester: last random seed: R02S6d8ab263ca56f8ae7a4b47bdf93fbc73
>
> Please take a look at what is causing this.
>
> Dropped from the net branch.
>
> Stefan
>
Ok, will post V2 later.
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev()
2014-09-04 16:40 ` Peter Maydell
@ 2014-09-09 3:03 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2014-09-09 3:03 UTC (permalink / raw)
To: Peter Maydell, Stefan Hajnoczi
Cc: Markus Armbruster, Andreas Färber, qemu-devel,
Stefan Hajnoczi, Michael S. Tsirkin
On 09/05/2014 12:40 AM, Peter Maydell wrote:
> On 4 September 2014 17:21, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> Unfortunately this patch breaks aarch64-softmmu qtests:
>> GTESTER check-qtest-aarch64
>> Broken pipe
>> GTester: last random seed: R02S6d8ab263ca56f8ae7a4b47bdf93fbc73
>>
>> Please take a look at what is causing this.
> Specifically, it breaks on all the ARM boards which have
> more than one built in ethernet device. Something in your
> patch is probably assuming there is only one (there's
> a somewhat suspicious "[0]" array reference, for instance).
>
> thanks
> -- PMM
>
The '[0]' dereference was introduced after multiqueue is supported.
Since there may be multiple pairs of peers for each nic.
The failure reason looks like the ncs[0] pointer should be validated
before trying to access them.
Will post V2.
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-09 3:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-28 7:41 [Qemu-devel] [PATCH] net: don't use set/get_pointer() in set/get_netdev() Jason Wang
2014-09-02 10:38 ` Stefan Hajnoczi
2014-09-04 16:21 ` Stefan Hajnoczi
2014-09-04 16:40 ` Peter Maydell
2014-09-09 3:03 ` Jason Wang
2014-09-09 3:01 ` Jason Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.