Hi
On Fri, Aug 26, 2022 at 10:27 PM Thomas Huth <th...@redhat.com> wrote:
> On 26/08/2022 16.59, Bin Meng wrote:
> > On Thu, Aug 25, 2022 at 9:04 PM Thomas Huth <th...@redhat.com> wrote:
> >>
> >> On 24/08/2022 11.40, Bin Meng wrote:
> >>> From: Xuzhou Cheng <xuzhou.ch...@windriver.com>
> >>>
> >>> Socket communication in the libqtest and libqmp codes uses read()
> >>> and write() which work on any file descriptor on *nix, and sockets
> >>> in *nix are an example of a file descriptor.
> >>>
> >>> However sockets on Windows do not use *nix-style file descriptors,
> >>> so read() and write() cannot be used on sockets on Windows.
> >>> Switch over to use send() and recv() instead which work on both
> >>> Windows and *nix.
> >>>
> >>> Signed-off-by: Xuzhou Cheng <xuzhou.ch...@windriver.com>
> >>> Signed-off-by: Bin Meng <bin.m...@windriver.com>
> >>> ---
> >>>
> >>> tests/qtest/libqmp.c | 4 ++--
> >>> tests/qtest/libqtest.c | 4 ++--
> >>> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
> >>> index ade26c15f0..995a39c1f8 100644
> >>> --- a/tests/qtest/libqmp.c
> >>> +++ b/tests/qtest/libqmp.c
> >>> @@ -36,7 +36,7 @@ typedef struct {
> >>>
> >>> static void socket_send(int fd, const char *buf, size_t size)
> >>> {
> >>> - size_t res = qemu_write_full(fd, buf, size);
> >>> + ssize_t res = send(fd, buf, size, 0);
> >>
> >> This way we're losing the extra logic from qemu_write_full() here (i.e.
> the
> >> looping and EINTR handling) ... not sure whether that's really OK?
> Maybe you
> >> have to introduce a qemu_send_full() first?
> >>
> >
> > I am not sure if qemu_send_full() is really needed because there is an
> > assert() right after the send() call.
>
> That's just a sanity check ... I think this function still has to take
> care
> of EINTR - it originally looked like this:
>
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c3e5704af19ac6
>
> ... and you can also see the while loop there.
>
>
Agree, that would be the correct thing to do.
Fwiw, the SOCKET vs fd situation is giving me some nervous feelings,
sometimes.
For ex, as I checked recently, it seems close(fd) correctly closes the
underlying SOCKET - as if closesocket() was called on it.. but this is not
really documented.
And it's easy to mix fd vs SOCKET in QEMU code paths (we cast/map SOCKET to
"int fd" in general), and reach a close() on a SOCKET. That wouldn't be
valid, and would likely create leaks or other issues.
Maybe we should introduce a type for safety / documentation purposes...
--
Marc-André Lureau
