Skip to content

Commit

Permalink
Revert "tap: setting error appropriately when calling net_init_tap_on…
Browse files Browse the repository at this point in the history
…e()"

This reverts commit 46d4d36.

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 5430a28 ("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 1e7398a ("vhost: enable vhost without without MSI-X") and
changed to behave identical with the vhost option for compatibility.

Worse, commit bf769f7 ("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
bf769f7 ("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>
  • Loading branch information
akihikodaki authored and jasowang committed Mar 29, 2024
1 parent decfde6 commit d9b3301
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 20 deletions.
3 changes: 0 additions & 3 deletions include/net/vhost_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
22 changes: 5 additions & 17 deletions net/tap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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)) {
Expand All @@ -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) {
Expand Down

0 comments on commit d9b3301

Please sign in to comment.