Convert non-blocking connect and fix its error reporting

[Qemu-devel] [PATCH v2 0/4] Convert non-blocking connect and fix its error reporting

Posted by Mao Zhongyi 6 years, 11 months ago

v2:
* PATCH 02 reworking of patch 2 following Markus's suggestion that convert error_report()
  in the function called by net_socket_*_init() to Error. Also add many error handling
  information.
* PATCH 03 net_socket_mcast_create(), net_socket_fd_init_dgram() and net_socket_fd_init() 
  use the function such as fprintf, perror to report an error message. Convert it to Error.
* PATCH 04 parse_host_port() may fail without reporting an error. Now, fix it to set an
  error when it fails.

CC: berrange@redhat.com, kraxel@redhat.com, pbonzini@redhat.com, jasowang@redhat.com, armbru@redhat.com

Mao Zhongyi (4):
  net/socket: Convert the non-blocking connection mechanism to
    QIOchannel
  net/socket: Improve -net socket error reporting
  net/socket: Convert error report message to Error
  net/net: Convert parse_host_port() to Error

 include/qemu/sockets.h |   4 +-
 net/net.c              |  21 +++++--
 net/socket.c           | 159 ++++++++++++++++++++++++++++++-------------------
 util/qemu-sockets.c    |   2 +-
 4 files changed, 117 insertions(+), 69 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 0/4] Convert non-blocking connect and fix its error reporting

Posted by Daniel P. Berrange 6 years, 11 months ago

On Wed, Apr 26, 2017 at 04:04:14PM +0800, Mao Zhongyi wrote:
> v2:
> * PATCH 02 reworking of patch 2 following Markus's suggestion that convert error_report()
>   in the function called by net_socket_*_init() to Error. Also add many error handling
>   information.
> * PATCH 03 net_socket_mcast_create(), net_socket_fd_init_dgram() and net_socket_fd_init() 
>   use the function such as fprintf, perror to report an error message. Convert it to Error.
> * PATCH 04 parse_host_port() may fail without reporting an error. Now, fix it to set an
>   error when it fails.

FYI, I discovered that previous change

  commit 883e4f7624e10b98d16d9adaffb8b1795664d899
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   Sat Jun 18 13:24:02 2016 +0530

    Change net/socket.c to use socket_*() functions

has seriously broken the current code because net_socket_fd_init()
was not called from the right place. Fixing the current code is
somewhat painful, so I've sent a revert of that broken patch.

To demo the problem first run:

  $ ./x86_64-softmmu/qemu-system-x86_64  \
      -device e1000,id=e0,netdev=user.0,mac=DE:AD:BE:EF:AF:04 \
      -netdev socket,id=user.0,listen=:1234 

and then run:

   $ ./x86_64-softmmu/qemu-system-x86_64 \
           -device e1000,id=e0,netdev=hn0,mac=DE:AD:BE:EF:AF:05 \
           -netdev socket,id=hn0,connect=localhost:1234

currently the second command fails with

  qemu-system-x86_64: -device e1000,id=e0,netdev=hn0,mac=DE:AD:BE:EF:AF:05: Property 'e1000.netdev' can't find value 'hn0'

and my revert fixes that. Just something for you to test with your
new patch series...

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 0/4] Convert non-blocking connect and fix its error reporting

Posted by Mao Zhongyi 6 years, 11 months ago


On 05/06/2017 12:39 AM, Daniel P. Berrange wrote:
> On Wed, Apr 26, 2017 at 04:04:14PM +0800, Mao Zhongyi wrote:
>> v2:
>> * PATCH 02 reworking of patch 2 following Markus's suggestion that convert error_report()
>>   in the function called by net_socket_*_init() to Error. Also add many error handling
>>   information.
>> * PATCH 03 net_socket_mcast_create(), net_socket_fd_init_dgram() and net_socket_fd_init()
>>   use the function such as fprintf, perror to report an error message. Convert it to Error.
>> * PATCH 04 parse_host_port() may fail without reporting an error. Now, fix it to set an
>>   error when it fails.
>
> FYI, I discovered that previous change
>
>   commit 883e4f7624e10b98d16d9adaffb8b1795664d899
>   Author: Marc-André Lureau <marcandre.lureau@redhat.com>
>   Date:   Sat Jun 18 13:24:02 2016 +0530
>
>     Change net/socket.c to use socket_*() functions
>
> has seriously broken the current code because net_socket_fd_init()
> was not called from the right place. Fixing the current code is
> somewhat painful, so I've sent a revert of that broken patch.
>
> To demo the problem first run:
>
>   $ ./x86_64-softmmu/qemu-system-x86_64  \
>       -device e1000,id=e0,netdev=user.0,mac=DE:AD:BE:EF:AF:04 \
>       -netdev socket,id=user.0,listen=:1234
>
> and then run:
>
>    $ ./x86_64-softmmu/qemu-system-x86_64 \
>            -device e1000,id=e0,netdev=hn0,mac=DE:AD:BE:EF:AF:05 \
>            -netdev socket,id=hn0,connect=localhost:1234
>
> currently the second command fails with
>
>   qemu-system-x86_64: -device e1000,id=e0,netdev=hn0,mac=DE:AD:BE:EF:AF:05: Property 'e1000.netdev' can't find value 'hn0'
>
> and my revert fixes that. Just something for you to test with your
> new patch series...
>
> Regards,
> Daniel

Hi, Daniel

According to your latest advice, run these two commands in my new
patch, the second command reported the same error.

Now, you sent a revert of that broken patch to avoid this problem.
It also means that my first patch based on socket_*() is disabled.
Currently, there is no good way to convert it use QIOchannel. So I
have an idea to use the reverted patch instead of the first of my
series, and then based on the first to fix the rest. Latter I will
convert it to QIOchannel in a separated patch. Do you think that's
ok?

Looking forward to your opinion.

Thanks
Mao