| Message ID | 20210210174518.2493928-4-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | failover: trivial cleanup and fix | expand |
On 2021/2/11 上午1:45, Laurent Vivier wrote: > In failover_add_primary(), we search the id of the failover device by > scanning the list of the devices in the opts list to find a device with > a failover_pair_id equals to the id of the virtio-net device. > > If the failover_pair_id is not found, QEMU ignores the primary > device silently (which also means it will not be hidden and > it will be enabled directly at boot). > > After that, we search the id in the opts list to do a qdev_device_add() > with it. The device will be always found as otherwise we had exited > before, and thus the warning is never displayed. > > Fix that by moving the error report to the first exit condition. > Also add a g_assert() to be sure the compiler will not complain > about a possibly NULL pointer. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > hw/net/virtio-net.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 1c5af08dc556..439f823b190c 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -855,21 +855,19 @@ static void failover_add_primary(VirtIONet *n, Error **errp) > > id = failover_find_primary_device_id(n); > if (!id) { > + error_setg(errp, "Primary device not found"); > + error_append_hint(errp, "Virtio-net failover will not work. Make " > + "sure primary device has parameter" > + " failover_pair_id=%s\n", n->netclient_name); > return; > } > opts = qemu_opts_find(qemu_find_opts("device"), id); > - if (opts) { > - dev = qdev_device_add(opts, &err); > - if (err) { > - qemu_opts_del(opts); > - } else { > - object_unref(OBJECT(dev)); > - } > + g_assert(opts); /* cannot be NULL because id was found using opts list */ > + dev = qdev_device_add(opts, &err); > + if (err) { > + qemu_opts_del(opts); > } else { > - error_setg(errp, "Primary device not found"); > - error_append_hint(errp, "Virtio-net failover will not work. Make " > - "sure primary device has parameter" > - " failover_pair_id=<virtio-net-id>\n"); > + object_unref(OBJECT(dev)); > } > error_propagate(errp, err); > }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 1c5af08dc556..439f823b190c 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -855,21 +855,19 @@ static void failover_add_primary(VirtIONet *n, Error **errp) id = failover_find_primary_device_id(n); if (!id) { + error_setg(errp, "Primary device not found"); + error_append_hint(errp, "Virtio-net failover will not work. Make " + "sure primary device has parameter" + " failover_pair_id=%s\n", n->netclient_name); return; } opts = qemu_opts_find(qemu_find_opts("device"), id); - if (opts) { - dev = qdev_device_add(opts, &err); - if (err) { - qemu_opts_del(opts); - } else { - object_unref(OBJECT(dev)); - } + g_assert(opts); /* cannot be NULL because id was found using opts list */ + dev = qdev_device_add(opts, &err); + if (err) { + qemu_opts_del(opts); } else { - error_setg(errp, "Primary device not found"); - error_append_hint(errp, "Virtio-net failover will not work. Make " - "sure primary device has parameter" - " failover_pair_id=<virtio-net-id>\n"); + object_unref(OBJECT(dev)); } error_propagate(errp, err); }
In failover_add_primary(), we search the id of the failover device by scanning the list of the devices in the opts list to find a device with a failover_pair_id equals to the id of the virtio-net device. If the failover_pair_id is not found, QEMU ignores the primary device silently (which also means it will not be hidden and it will be enabled directly at boot). After that, we search the id in the opts list to do a qdev_device_add() with it. The device will be always found as otherwise we had exited before, and thus the warning is never displayed. Fix that by moving the error report to the first exit condition. Also add a g_assert() to be sure the compiler will not complain about a possibly NULL pointer. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/net/virtio-net.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)