> -----Original Message----- > From: Jason Wang [mailto:jasow...@redhat.com] > Sent: Friday, March 02, 2018 2:30 PM > To: Zhoujian (jay) <jianjay.z...@huawei.com>; qemu-devel@nongnu.org > Cc: Huangweidong (C) <weidong.hu...@huawei.com>; m...@redhat.com; Gonglei > (Arei) <arei.gong...@huawei.com>; imamm...@redhat.com; wangxin (U) > <wangxinxin.w...@huawei.com> > Subject: Re: [Qemu-devel] [PATCH v4] tap: setting error appropriately when > calling net_init_tap_one() > > > > On 2018年02月06日 20:53, Jay Zhou wrote: > > If netdev_add tap,id=net0,...,vhost=on failed in net_init_tap_one(), > > the followed up device_add virtio-net-pci,netdev=net0 will fail too, > > prints: > > > > TUNSETOFFLOAD ioctl() failed: Bad file descriptor TUNSETOFFLOAD > > ioctl() failed: Bad file descriptor > > > > The reason is that the fd of tap is closed when error occured after > > calling net_init_tap_one(). > > > > The fd should be closed when calling net_init_tap_one failed: > > - if tap_set_sndbuf() failed > > - if tap_set_sndbuf() succeeded but vhost failed to open or > > initialize with vhostforce flag on The fd should not be closed > > just because vhost failed to open or initialize but without vhostforce > > flag. So the followed up device_add can fall back to userspace virtio > > successfully. > > > > Suggested-by: Michael S. Tsirkin <m...@redhat.com> > > Suggested-by: Igor Mammedov <imamm...@redhat.com> > > Suggested-by: Jason Wang <jasow...@redhat.com> > > Signed-off-by: Jay Zhou <jianjay.z...@huawei.com> > > --- > > v4: - reduce duplication > > - close the fd by caller > > - tweak the title > > > > v3: - set errp appropriately > > --- > > include/net/vhost_net.h | 3 +++ > > net/tap.c | 24 ++++++++++++++++++------ > > 2 files changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h index > > afc1499..77e4739 100644 > > --- a/include/net/vhost_net.h > > +++ b/include/net/vhost_net.h > > @@ -4,6 +4,9 @@ > > #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 979e622..14d230f 100644 > > --- a/net/tap.c > > +++ b/net/tap.c > > @@ -686,14 +686,23 @@ static void net_init_tap_one(const NetdevTapOptions > *tap, NetClientState *peer, > > if (vhostfdname) { > > vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err); > > if (vhostfd == -1) { > > - error_propagate(errp, err); > > + if (tap->has_vhostforce && tap->vhostforce) { > > + error_propagate(errp, err); > > + } else { > > + warn_report_err(err); > > + } > > return; > > } > > } else { > > vhostfd = open("/dev/vhost-net", O_RDWR); > > if (vhostfd < 0) { > > - error_setg_errno(errp, errno, > > - "tap: open vhost char device failed"); > > + 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)); > > + } > > return; > > } > > fcntl(vhostfd, F_SETFL, O_NONBLOCK); @@ -702,12 +711,15 > > @@ static void net_init_tap_one(const NetdevTapOptions *tap, > > NetClientState *peer, > > > > s->vhost_net = vhost_net_init(&options); > > if (!s->vhost_net) { > > - error_setg(errp, > > - "vhost-net requested but could not be initialized"); > > + if (tap->has_vhostforce && tap->vhostforce) { > > + error_setg(errp, VHOST_NET_INIT_FAILED); > > + } else { > > + warn_report(VHOST_NET_INIT_FAILED); > > + } > > return; > > } > > } else if (vhostfdname) { > > - error_setg(errp, "vhostfd(s)= is not valid without vhost"); > > + warn_report("vhostfd(s)= is not valid without vhost"); > > Do we need to keep the error here consider it was a wrong command line > parameter?
Fair enough, thanks! Regards, Jay > > Thanks > > > } > > } > >
