Re: [Qemu-devel] [PATCH v8 4/4] net/socket: Improve -net socket error reporting

[Mao Zhongyi]

Hi, Markus

On 07/06/2017 03:24 PM, Markus Armbruster wrote:

Mao Zhongyi <maozy.f...@cn.fujitsu.com> writes:

When -net socket fails, it first reports a specific error, then
a generic one, like this:

    $ qemu-system-x86_64 -net socket,
    qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required
    qemu-system-x86_64: -net socket: Device 'socket' could not be initialized

Convert net_socket_*_init() to Error to get rid of the superfluous second
error message. After the patch, the effect like this:

    $ qemu-system-x86_64 -net socket,
    qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required

At the same time, add many explicit error handling message when it fails.

Suggest "This also fixes a few silent failures to report an error."

Perhaps Jason can touch this up for you.

Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Signed-off-by: Mao Zhongyi <maozy.f...@cn.fujitsu.com>
---
 net/socket.c | 92 +++++++++++++++++++++++++++++-------------------------------
 1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 1d4c515..ca1878f 100644
--- a/net/socket.c
+++ b/net/socket.c

[...]

@@ -746,7 +742,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
         /* if sock->localaddr is missing, it has been initialized to "all bits
          * zero" */
         if (net_socket_mcast_init(peer, "socket", name, sock->mcast,
-            sock->localaddr) == -1) {
+            sock->localaddr, errp) < 0) {

Indent like this:

                                     sock->localaddr, errp) < 0) {

Perhaps Jason can touch this up for you.

             return -1;
         }
         return 0;
@@ -754,11 +750,11 @@ int net_init_socket(const Netdev *netdev, const char 
*name,

     assert(sock->has_udp);
     if (!sock->has_localaddr) {
-        error_report("localaddr= is mandatory with udp=");
+        error_setg(errp, "localaddr= is mandatory with udp=");
         return -1;
     }
-    if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr) 
==
-        -1) {
+    if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr,
+        errp) < 0) {

Likewise.

         return -1;
     }
     return 0;

With the indentation fixed:

Thanks for your review :)
I will fix and resend it as soon as possible.

Thanks,
Mao

Reviewed-by: Markus Armbruster <arm...@redhat.com>