Re: infinite loop in xenstat_qmp.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/11/2020 14:36, Reiser, Hans wrote:
> Hi,
>
> I have seen several occasions with "dead" xentop processes consuming 100% CPU 
> time, and tracked this down
> to the following problem:
>
> When the QEMU process the qmp_read function is communicating with terminates, 
> qmp_read may enter an
> infinite loop:  poll signals EOF (POLLIN and POLLHUP set), the subsequent 
> read() call returns 0, and then the
> function calls poll again, which still sees the EOF condition and will return 
> again immediately with POLLIN and
> POLLHUP set, repeating ad infinitum.
>
> A simple fix is to terminate the loop when read returns 0 (under "normal" 
> instances, poll will return with POLLIN
> set only if there is data to read, so read will always read >0 bytes, except 
> if the socket has been closed).
>
> Cheers, Hans

Hi - this appears to have slipped through the cracks.

Thanks for the bugfix, but we require code submissions to conform to the
DCO[1] and have a Signed-off-by line.

If you're happy to agree to that, I can fix up the patch and get it
sorted.  We've moved this library in the time since you submitted the
bugfix.

Thanks, and sorry for the delay.

~Andrew

[1]
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Signed-off-by

>
> diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c 
> b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> index 19b236e7b6..0c5748ba68 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> @@ -298,7 +298,7 @@ static int qmp_read(int qfd, unsigned char **qstats)
>         pfd[0].events = POLLIN;
>         while ((n = poll(pfd, 1, 10)) > 0) {
>                 if (pfd[0].revents & POLLIN) {
> -                       if ((n = read(qfd, buf, sizeof(buf))) < 0) {
> +                       if ((n = read(qfd, buf, sizeof(buf))) <= 0) {
>                                 free(*qstats);
>                                 return 0;
>                         }
>
>