From: Jason Wang <jasowang@redhat.com>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org,
Akihiko Odaki <akihiko.odaki@daynix.com>,
Jason Wang <jasowang@redhat.com>
Subject: [PULL 5/5] Revert "tap: setting error appropriately when calling net_init_tap_one()"
Date: Fri, 29 Mar 2024 15:11:00 +0800 [thread overview]
Message-ID: <20240329071100.31376-6-jasowang@redhat.com> (raw)
In-Reply-To: <20240329071100.31376-1-jasowang@redhat.com>
From: Akihiko Odaki <akihiko.odaki@daynix.com>
This reverts commit 46d4d36d0bf2b24b205f2f604f0905db80264eef.
The reverted commit changed to emit warnings instead of errors when
vhost is requested but vhost initialization fails if vhostforce option
is not set.
However, vhostforce is not meant to ignore vhost errors. It was once
introduced as an option to commit 5430a28fe4 ("vhost: force vhost off
for non-MSI guests") to force enabling vhost for non-MSI guests, which
will have worse performance with vhost. The option was deprecated with
commit 1e7398a140 ("vhost: enable vhost without without MSI-X") and
changed to behave identical with the vhost option for compatibility.
Worse, commit bf769f742c ("virtio: del net client if net_init_tap_one
failed") changed to delete the client when vhost fails even when the
failure only results in a warning. The leads to an assertion failure
for the -netdev command line option.
The reverted commit was intended to avoid that the vhost initialization
failure won't result in a corrupted netdev. This problem should have
been fixed by deleting netdev when the initialization fails instead of
ignoring the failure with an arbitrary option. Fortunately, commit
bf769f742c ("virtio: del net client if net_init_tap_one failed"),
mentioned earlier, implements this behavior.
Restore the correct semantics and fix the assertion failure for the
-netdev command line option by reverting the problematic commit.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/net/vhost_net.h | 3 ---
net/tap.c | 22 +++++-----------------
2 files changed, 5 insertions(+), 20 deletions(-)
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index c37aba35e6..c6a5361a2a 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -4,9 +4,6 @@
#include "net/net.h"
#include "hw/virtio/vhost-backend.h"
-#define VHOST_NET_INIT_FAILED \
- "vhost-net requested but could not be initialized"
-
struct vhost_net;
typedef struct vhost_net VHostNetState;
diff --git a/net/tap.c b/net/tap.c
index c698b70475..baaa2f7a9a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -743,11 +743,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
if (vhostfdname) {
vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, &err);
if (vhostfd == -1) {
- if (tap->has_vhostforce && tap->vhostforce) {
- error_propagate(errp, err);
- } else {
- warn_report_err(err);
- }
+ error_propagate(errp, err);
goto failed;
}
if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
@@ -758,13 +754,8 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
} else {
vhostfd = open("/dev/vhost-net", O_RDWR);
if (vhostfd < 0) {
- if (tap->has_vhostforce && tap->vhostforce) {
- error_setg_errno(errp, errno,
- "tap: open vhost char device failed");
- } else {
- warn_report("tap: open vhost char device failed: %s",
- strerror(errno));
- }
+ error_setg_errno(errp, errno,
+ "tap: open vhost char device failed");
goto failed;
}
if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
@@ -777,11 +768,8 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
- if (tap->has_vhostforce && tap->vhostforce) {
- error_setg(errp, VHOST_NET_INIT_FAILED);
- } else {
- warn_report(VHOST_NET_INIT_FAILED);
- }
+ error_setg(errp,
+ "vhost-net requested but could not be initialized");
goto failed;
}
} else if (vhostfdname) {
--
2.42.0
next prev parent reply other threads:[~2024-03-29 7:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 7:10 [PULL 0/5] Net patches Jason Wang
2024-03-29 7:10 ` [PULL 1/5] virtio-net: Fix vhost virtqueue notifiers for RSS Jason Wang
2024-03-29 7:10 ` [PULL 2/5] ebpf: Fix indirections table setting Jason Wang
2024-03-29 7:10 ` [PULL 3/5] hw/net/net_tx_pkt: Fix virtio header without checksum offloading Jason Wang
2024-03-29 7:10 ` [PULL 4/5] tap-win32: Remove unnecessary stubs Jason Wang
2024-03-29 7:11 ` Jason Wang [this message]
2024-03-31 15:42 ` [PULL 0/5] Net patches Peter Maydell
2024-03-31 19:21 ` Michael Tokarev
2024-04-08 6:42 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240329071100.31376-6-jasowang@redhat.com \
--to=jasowang@redhat.com \
--cc=akihiko.odaki@daynix.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).