[PATCH v2] tools/hv: terminate fcopy daemon if read from uio fails

[PATCH v2] tools/hv: terminate fcopy daemon if read from uio fails

[Olaf Hering]

Terminate endless loop in reading fails, to avoid flooding syslog.

This happens if the state of "Guest services" integration service
is changed from "enabled" to "disabled" at runtime in the VM
settings. In this case pread returns EIO.

Also handle an interrupted system call, and continue in this case.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
v2: update commit message

A more complete fix is to handle this properly in the kernel,
by making the file descriptor unavailable for further operations.

 tools/hv/hv_fcopy_uio_daemon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 7a00f3066a98..281fd95dc0d8 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -468,8 +468,10 @@ int main(int argc, char *argv[])
 		 */
 		ret = pread(fcopy_fd, &tmp, sizeof(int), 0);
 		if (ret < 0) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
 			syslog(LOG_ERR, "pread failed: %s", strerror(errno));
-			continue;
+			goto close;
 		}
 
 		len = HV_RING_SIZE;

Re: [PATCH v2] tools/hv: terminate fcopy daemon if read from uio fails

[Saurabh Singh Sengar]

On Tue, Nov 05, 2024 at 09:14:04AM +0100, Olaf Hering wrote:
> Terminate endless loop in reading fails, to avoid flooding syslog.
> 
> This happens if the state of "Guest services" integration service
> is changed from "enabled" to "disabled" at runtime in the VM
> settings. In this case pread returns EIO.

My comment on V1 was meant to log this information in the syslog, so the
user can look for the Guest Service status incase of EIO. Capturing these
details in the commit will require additional effort from user.

Nevertheless, thanks for fixing this:

Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>

> 
> Also handle an interrupted system call, and continue in this case.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> v2: update commit message
> 
> A more complete fix is to handle this properly in the kernel,
> by making the file descriptor unavailable for further operations.
> 
>  tools/hv/hv_fcopy_uio_daemon.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 7a00f3066a98..281fd95dc0d8 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -468,8 +468,10 @@ int main(int argc, char *argv[])
>  		 */
>  		ret = pread(fcopy_fd, &tmp, sizeof(int), 0);
>  		if (ret < 0) {
> +			if (errno == EINTR || errno == EAGAIN)
> +				continue;
>  			syslog(LOG_ERR, "pread failed: %s", strerror(errno));
> -			continue;
> +			goto close;
>  		}
>  
>  		len = HV_RING_SIZE;

Re: [PATCH v2] tools/hv: terminate fcopy daemon if read from uio fails

[Wei Liu]

On Tue, Nov 05, 2024 at 09:14:04AM +0100, Olaf Hering wrote:
> Terminate endless loop in reading fails, to avoid flooding syslog.
> 
> This happens if the state of "Guest services" integration service
> is changed from "enabled" to "disabled" at runtime in the VM
> settings. In this case pread returns EIO.
> 
> Also handle an interrupted system call, and continue in this case.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Applied to hyperv-fixes. Thanks.