qdev: Improve netdev property override error a bit
qdev_prop_set_netdev() fails when the property already has a non-null
value. Seems to go back to commit 30c367ed44
"qdev-properties-system.c: Allow vlan or netdev for -device, not
both", v1.7.0. Board code doesn't expect failure, and crashes:
$ qemu-system-x86_64 --nodefaults -nic user -netdev user,id=nic0 -global e1000.netdev=nic0
Unexpected error in error_set_from_qdev_prop_error() at /work/armbru/qemu/hw/core/qdev-properties.c:1101:
qemu-system-x86_64: Property 'e1000.netdev' doesn't take value '__org.qemu.nic0
'
Aborted (core dumped)
-device and device_add handle the failure:
$ qemu-system-x86_64 -nodefaults -netdev user,id=net0 -netdev user,id=net1 -device e1000,netdev=net0,netdev=net1
qemu-system-x86_64: -device e1000,netdev=net0,netdev=net1: Property 'e1000.netdev' doesn't take value 'net1'
$ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -netdev user,id=net0 -netdev user,id=net1 -global e1000.netdev=net0
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) qemu-system-x86_64: warning: netdev net0 has no peer
qemu-system-x86_64: warning: netdev net1 has no peer
device_add e1000,netdev=net1
Error: Property 'e1000.netdev' doesn't take value 'net1'
Perhaps netdev property override could be made to work. Perhaps it
should. I'm not the right guy to figure this out. What I can do is
improve the error message a bit:
(qemu) device_add e1000,netdev=net1
Error: -global e1000.netdev=... conflicts with netdev=net1
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-11-armbru@redhat.com>
This commit is contained in:
parent
466c2983f8
commit
1bc133365e
|
@ -25,6 +25,28 @@
|
||||||
#include "sysemu/iothread.h"
|
#include "sysemu/iothread.h"
|
||||||
#include "sysemu/tpm_backend.h"
|
#include "sysemu/tpm_backend.h"
|
||||||
|
|
||||||
|
static bool check_prop_still_unset(DeviceState *dev, const char *name,
|
||||||
|
const void *old_val, const char *new_val,
|
||||||
|
Error **errp)
|
||||||
|
{
|
||||||
|
const GlobalProperty *prop = qdev_find_global_prop(dev, name);
|
||||||
|
|
||||||
|
if (!old_val) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (prop) {
|
||||||
|
error_setg(errp, "-global %s.%s=... conflicts with %s=%s",
|
||||||
|
prop->driver, prop->property, name, new_val);
|
||||||
|
} else {
|
||||||
|
/* Error message is vague, but a better one would be hard */
|
||||||
|
error_setg(errp, "%s=%s conflicts, and override is not implemented",
|
||||||
|
name, new_val);
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* --- drive --- */
|
/* --- drive --- */
|
||||||
|
|
||||||
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
|
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
|
||||||
|
@ -299,14 +321,16 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
|
||||||
}
|
}
|
||||||
|
|
||||||
for (i = 0; i < queues; i++) {
|
for (i = 0; i < queues; i++) {
|
||||||
|
|
||||||
if (peers[i]->peer) {
|
if (peers[i]->peer) {
|
||||||
err = -EEXIST;
|
err = -EEXIST;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (ncs[i]) {
|
/*
|
||||||
err = -EINVAL;
|
* TODO Should this really be an error? If no, the old value
|
||||||
|
* needs to be released before we store the new one.
|
||||||
|
*/
|
||||||
|
if (!check_prop_still_unset(dev, name, ncs[i], str, errp)) {
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1246,6 +1246,23 @@ void qdev_prop_register_global(GlobalProperty *prop)
|
||||||
g_ptr_array_add(global_props(), prop);
|
g_ptr_array_add(global_props(), prop);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
|
||||||
|
const char *name)
|
||||||
|
{
|
||||||
|
GPtrArray *props = global_props();
|
||||||
|
const GlobalProperty *p;
|
||||||
|
int i;
|
||||||
|
|
||||||
|
for (i = 0; i < props->len; i++) {
|
||||||
|
p = g_ptr_array_index(props, i);
|
||||||
|
if (object_dynamic_cast(OBJECT(dev), p->driver)
|
||||||
|
&& !strcmp(p->property, name)) {
|
||||||
|
return p;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
int qdev_prop_check_globals(void)
|
int qdev_prop_check_globals(void)
|
||||||
{
|
{
|
||||||
int i, ret = 0;
|
int i, ret = 0;
|
||||||
|
|
|
@ -251,6 +251,8 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char *name,
|
||||||
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
|
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
|
||||||
|
|
||||||
void qdev_prop_register_global(GlobalProperty *prop);
|
void qdev_prop_register_global(GlobalProperty *prop);
|
||||||
|
const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
|
||||||
|
const char *name);
|
||||||
int qdev_prop_check_globals(void);
|
int qdev_prop_check_globals(void);
|
||||||
void qdev_prop_set_globals(DeviceState *dev);
|
void qdev_prop_set_globals(DeviceState *dev);
|
||||||
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
|
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
|
||||||
|
|
Loading…
Reference in a new issue