| Message ID | 20170922080435.27342-1-jfreimann@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: fix check for number of parameters to -netdev socket | expand |
On 2017年09月22日 16:04, Jens Freimann wrote: > Since commit 0f8c289ad "net: fix -netdev socket,fd= for UDP sockets" > we allow more than one parameter for -netdev socket. But now > we run into an assert when no parameter at all is specified > >> qemu-system-x86_64 -netdev socket > socket.c:729: net_init_socket: Assertion `sock->has_udp' failed. > > Change the check and error message to reflect that at least > one of the options has to be specified. An example for when we need > more than on is when we hand in a UDP socket. Then we have fd= and udp= > as parameters. > > Cc: Jason Wang <jasowang@redhat.com> > Fixes: 0f8c289ad539feb5135c545bea947b310a893f4b > Reported-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > --- > net/socket.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/socket.c b/net/socket.c > index e6b471c63d..0a987bcf4f 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -695,9 +695,9 @@ int net_init_socket(const Netdev *netdev, const char *name, > assert(netdev->type == NET_CLIENT_DRIVER_SOCKET); > sock = &netdev->u.socket; > > - if (sock->has_listen + sock->has_connect + sock->has_mcast + > - sock->has_udp > 1) { > - error_setg(errp, "exactly one of listen=, connect=, mcast= or udp=" > + if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast + > + sock->has_udp == 0) { > + error_setg(errp, "at least one of fd=, listen=, connect=, mcast= or udp=" > " is required"); > return -1; > } Looks like this allows "listen" and "connect" to be used in the same time which is wrong I think? Thanks
On Fri, Sep 22, 2017 at 08:32:47AM +0000, Jason Wang wrote: > > >On 2017???09???22??? 16:04, Jens Freimann wrote: >>Since commit 0f8c289ad "net: fix -netdev socket,fd= for UDP sockets" >>we allow more than one parameter for -netdev socket. But now >>we run into an assert when no parameter at all is specified >> >>>qemu-system-x86_64 -netdev socket >>socket.c:729: net_init_socket: Assertion `sock->has_udp' failed. >> >>Change the check and error message to reflect that at least >>one of the options has to be specified. An example for when we need >>more than on is when we hand in a UDP socket. Then we have fd= and udp= >>as parameters. >> >>Cc: Jason Wang <jasowang@redhat.com> >>Fixes: 0f8c289ad539feb5135c545bea947b310a893f4b >>Reported-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> >>Signed-off-by: Jens Freimann <jfreimann@redhat.com> >>--- >> net/socket.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >>diff --git a/net/socket.c b/net/socket.c >>index e6b471c63d..0a987bcf4f 100644 >>--- a/net/socket.c >>+++ b/net/socket.c >>@@ -695,9 +695,9 @@ int net_init_socket(const Netdev *netdev, const char *name, >> assert(netdev->type == NET_CLIENT_DRIVER_SOCKET); >> sock = &netdev->u.socket; >>- if (sock->has_listen + sock->has_connect + sock->has_mcast + >>- sock->has_udp > 1) { >>- error_setg(errp, "exactly one of listen=, connect=, mcast= or udp=" >>+ if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast + >>+ sock->has_udp == 0) { >>+ error_setg(errp, "at least one of fd=, listen=, connect=, mcast= or udp=" >> " is required"); >> return -1; >> } > >Looks like this allows "listen" and "connect" to be used in the same >time which is wrong I think? I think connect would be silently dropped because netdev_sock_init() returns after the listen part is set up. There would be no warning/error message though, which is not nice. I can add a check to prevent that listen and connect at the same time. regards, Jens
diff --git a/net/socket.c b/net/socket.c index e6b471c63d..0a987bcf4f 100644 --- a/net/socket.c +++ b/net/socket.c @@ -695,9 +695,9 @@ int net_init_socket(const Netdev *netdev, const char *name, assert(netdev->type == NET_CLIENT_DRIVER_SOCKET); sock = &netdev->u.socket; - if (sock->has_listen + sock->has_connect + sock->has_mcast + - sock->has_udp > 1) { - error_setg(errp, "exactly one of listen=, connect=, mcast= or udp=" + if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast + + sock->has_udp == 0) { + error_setg(errp, "at least one of fd=, listen=, connect=, mcast= or udp=" " is required"); return -1; }