* [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both
@ 2013-11-08 2:13 Vlad Yasevich
2013-11-11 5:18 ` Jason Wang
2013-11-22 8:50 ` [Qemu-devel] [PATCH] " Stefan Hajnoczi
0 siblings, 2 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-11-08 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Vlad Yasevich, jasowang, stefanha
It is currently possible to specify things like:
-device e1000,netdev=foo,vlan=1
With this usage, whichever argument was specified last (vlan or netdev)
overwrites what was previousely set and results in a non-working
configuration. Even worse, when used with multiqueue devices,
it causes a segmentation fault on exit in qemu_free_net_client.
That patch treates the above command line options as invalid and
generates an error at start-up.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
hw/core/qdev-properties-system.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 0eada32..729efa8 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
goto err;
}
+ if (ncs[i]) {
+ ret = -EINVAL;
+ goto err;
+ }
+
ncs[i] = peers[i];
ncs[i]->queue_index = i;
}
@@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
*ptr = NULL;
return;
}
+ if (*ptr) {
+ error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
+ return;
+ }
hubport = net_hub_port_find(id);
if (!hubport) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both
2013-11-08 2:13 [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both Vlad Yasevich
@ 2013-11-11 5:18 ` Jason Wang
2013-11-11 7:50 ` [Qemu-devel] [PATCH for-1.7] " Paolo Bonzini
2013-11-22 8:50 ` [Qemu-devel] [PATCH] " Stefan Hajnoczi
1 sibling, 1 reply; 6+ messages in thread
From: Jason Wang @ 2013-11-11 5:18 UTC (permalink / raw)
To: Vlad Yasevich, qemu-devel; +Cc: stefanha
On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
> It is currently possible to specify things like:
> -device e1000,netdev=foo,vlan=1
> With this usage, whichever argument was specified last (vlan or netdev)
> overwrites what was previousely set and results in a non-working
> configuration. Even worse, when used with multiqueue devices,
> it causes a segmentation fault on exit in qemu_free_net_client.
>
> That patch treates the above command line options as invalid and
> generates an error at start-up.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> hw/core/qdev-properties-system.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
> index 0eada32..729efa8 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
> goto err;
> }
>
> + if (ncs[i]) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> ncs[i] = peers[i];
> ncs[i]->queue_index = i;
> }
> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
> *ptr = NULL;
> return;
> }
> + if (*ptr) {
> + error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
> + return;
> + }
>
> hubport = net_hub_port_find(id);
> if (!hubport) {
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-1.7] qdev-properties-system.c: Allow vlan or netdev for -device, not both
2013-11-11 5:18 ` Jason Wang
@ 2013-11-11 7:50 ` Paolo Bonzini
2013-11-21 19:47 ` Vlad Yasevich
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-11-11 7:50 UTC (permalink / raw)
To: Jason Wang; +Cc: Vlad Yasevich, qemu-devel, stefanha
Il 11/11/2013 06:18, Jason Wang ha scritto:
> On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
>> It is currently possible to specify things like:
>> -device e1000,netdev=foo,vlan=1
>> With this usage, whichever argument was specified last (vlan or netdev)
>> overwrites what was previousely set and results in a non-working
>> configuration. Even worse, when used with multiqueue devices,
>> it causes a segmentation fault on exit in qemu_free_net_client.
>>
>> That patch treates the above command line options as invalid and
>> generates an error at start-up.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> hw/core/qdev-properties-system.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
>> index 0eada32..729efa8 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
>> goto err;
>> }
>>
>> + if (ncs[i]) {
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> ncs[i] = peers[i];
>> ncs[i]->queue_index = i;
>> }
>> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
>> *ptr = NULL;
>> return;
>> }
>> + if (*ptr) {
>> + error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
>> + return;
>> + }
>>
>> hubport = net_hub_port_find(id);
>> if (!hubport) {
>
> Acked-by: Jason Wang <jasowang@redhat.com>
This patch is good for 1.7.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-1.7] qdev-properties-system.c: Allow vlan or netdev for -device, not both
2013-11-11 7:50 ` [Qemu-devel] [PATCH for-1.7] " Paolo Bonzini
@ 2013-11-21 19:47 ` Vlad Yasevich
2013-11-21 20:40 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Vlad Yasevich @ 2013-11-21 19:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Jason Wang, stefanha
On 11/11/2013 02:50 AM, Paolo Bonzini wrote:
> Il 11/11/2013 06:18, Jason Wang ha scritto:
>> On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
>>> It is currently possible to specify things like:
>>> -device e1000,netdev=foo,vlan=1
>>> With this usage, whichever argument was specified last (vlan or netdev)
>>> overwrites what was previousely set and results in a non-working
>>> configuration. Even worse, when used with multiqueue devices,
>>> it causes a segmentation fault on exit in qemu_free_net_client.
>>>
>>> That patch treates the above command line options as invalid and
>>> generates an error at start-up.
>>>
>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>> ---
>>> hw/core/qdev-properties-system.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
>>> index 0eada32..729efa8 100644
>>> --- a/hw/core/qdev-properties-system.c
>>> +++ b/hw/core/qdev-properties-system.c
>>> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
>>> goto err;
>>> }
>>>
>>> + if (ncs[i]) {
>>> + ret = -EINVAL;
>>> + goto err;
>>> + }
>>> +
>>> ncs[i] = peers[i];
>>> ncs[i]->queue_index = i;
>>> }
>>> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
>>> *ptr = NULL;
>>> return;
>>> }
>>> + if (*ptr) {
>>> + error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
>>> + return;
>>> + }
>>>
>>> hubport = net_hub_port_find(id);
>>> if (!hubport) {
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>
> This patch is good for 1.7.
>
> Paolo
>
Hi All
Just wondering what's to become of this patch? It has an ACK, but has
been abandoned. It does a fix a crash in qemu.
Thanks
-vlad
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH for-1.7] qdev-properties-system.c: Allow vlan or netdev for -device, not both
2013-11-21 19:47 ` Vlad Yasevich
@ 2013-11-21 20:40 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-11-21 20:40 UTC (permalink / raw)
To: vyasevic; +Cc: Jason Wang, qemu-devel, stefanha
Il 21/11/2013 20:47, Vlad Yasevich ha scritto:
> On 11/11/2013 02:50 AM, Paolo Bonzini wrote:
>> Il 11/11/2013 06:18, Jason Wang ha scritto:
>>> On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
>>>> It is currently possible to specify things like:
>>>> -device e1000,netdev=foo,vlan=1
>>>> With this usage, whichever argument was specified last (vlan or netdev)
>>>> overwrites what was previousely set and results in a non-working
>>>> configuration. Even worse, when used with multiqueue devices,
>>>> it causes a segmentation fault on exit in qemu_free_net_client.
>>>>
>>>> That patch treates the above command line options as invalid and
>>>> generates an error at start-up.
>>>>
>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>> ---
>>>> hw/core/qdev-properties-system.c | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
>>>> index 0eada32..729efa8 100644
>>>> --- a/hw/core/qdev-properties-system.c
>>>> +++ b/hw/core/qdev-properties-system.c
>>>> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
>>>> goto err;
>>>> }
>>>>
>>>> + if (ncs[i]) {
>>>> + ret = -EINVAL;
>>>> + goto err;
>>>> + }
>>>> +
>>>> ncs[i] = peers[i];
>>>> ncs[i]->queue_index = i;
>>>> }
>>>> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
>>>> *ptr = NULL;
>>>> return;
>>>> }
>>>> + if (*ptr) {
>>>> + error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
>>>> + return;
>>>> + }
>>>>
>>>> hubport = net_hub_port_find(id);
>>>> if (!hubport) {
>>>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>
>> This patch is good for 1.7.
>>
>> Paolo
>>
>
> Hi All
>
> Just wondering what's to become of this patch? It has an ACK, but has
> been abandoned. It does a fix a crash in qemu.
I missed this when going through pending patches. Stefan, are you
picking it up?
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both
2013-11-08 2:13 [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both Vlad Yasevich
2013-11-11 5:18 ` Jason Wang
@ 2013-11-22 8:50 ` Stefan Hajnoczi
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2013-11-22 8:50 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: jasowang, qemu-devel
On Thu, Nov 07, 2013 at 09:13:09PM -0500, Vlad Yasevich wrote:
> It is currently possible to specify things like:
> -device e1000,netdev=foo,vlan=1
> With this usage, whichever argument was specified last (vlan or netdev)
> overwrites what was previousely set and results in a non-working
> configuration. Even worse, when used with multiqueue devices,
> it causes a segmentation fault on exit in qemu_free_net_client.
>
> That patch treates the above command line options as invalid and
> generates an error at start-up.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> hw/core/qdev-properties-system.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-22 8:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-08 2:13 [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both Vlad Yasevich
2013-11-11 5:18 ` Jason Wang
2013-11-11 7:50 ` [Qemu-devel] [PATCH for-1.7] " Paolo Bonzini
2013-11-21 19:47 ` Vlad Yasevich
2013-11-21 20:40 ` Paolo Bonzini
2013-11-22 8:50 ` [Qemu-devel] [PATCH] " Stefan Hajnoczi
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).