[i-g-t, v2, 1/4] runner/executor: Check for error at writing dmesg dump

[Kamil Konieczny]

Hi Ryszard,
On 2024-11-27 at 16:05:11 +0100, Ryszard Knop wrote:
> On Wed, 2024-11-20 at 19:32 +0100, Kamil Konieczny wrote:
> > In processing kernel dmesg there are checks for error at reading
> > so add also one for writing.
> > 
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > Reviewed-by: Krzysztof Karas <krzysztof.karas at intel.com>
> > ---
> >  runner/executor.c | 29 ++++++++++++++++++++++++++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> > 
> > diff --git a/runner/executor.c b/runner/executor.c
> > index ac73e1dde..e4d1fc323 100644
> > --- a/runner/executor.c
> > +++ b/runner/executor.c
> > @@ -600,7 +600,7 @@ static long dump_dmesg(int kmsgfd, int outfd)
> >  	bool underflow_once = false;
> >  	char cont;
> >  	char buf[2048];
> > -	ssize_t r;
> > +	ssize_t r, w;
> >  	long written = 0;
> >  
> >  	if (kmsgfd < 0)
> > @@ -654,9 +654,32 @@ static long dump_dmesg(int kmsgfd, int outfd)
> >  			return written;
> >  		}
> >  
> > -		write(outfd, buf, r);
> > -		written += r;
> > +		w = write(outfd, buf, r);
> > +		if (w < r) {
> 
> You could rewrite this entire block to have a single write call, or to handle multiple
> interrupts/better error messages:
> 
> int encountered_errors = 0;
> while (written < r) {
>   int w = write(outfd, buf + written, r - written);
>   if (w > 0) {
>     written += w;
>     continue;
>   }
> 
>   encountered_errors += 1;
>   if (encountered_errors >= 100) {
>     errf("Too many errors while writing out dmesg, aborting\n");
>     break;
>   }
> 
>   // Recoverable errors:
>   if ((errno == EINTR) || (errno == EAGAIN)) continue;
> 
>   // Non-recoverable:
>   errf("Error writing out dmesg (written %ld < %ld bytes): %s\n", written, r, strerror(errno));
>   break;
> }
> 

Good idea but mayb it should start small with jyst reporting
error and bail out? I will drop this patch for now.

Regards,
Kamil

> > +			if (w == -1 && errno == EINTR)
> > +				w = write(outfd, buf, r);
> > +
> > +			if (w < r && w >= 0)
> > +				w += write(outfd, buf + w, r - w);
> > +
> > +			if (w < r) {
> > +				long err = -errno;
> > +
> > +				if (err) {
> > +					errf("Write error while processing dmesg, errno=%d %ld <
> > %ld\n", errno, w, r);
> > +				} else {
> > +					errf("Cannot write dmesg chunk: %ld < %ld\n", w, r);
> > +					err = -1;
> > +				}
> >  
> > +				if (comparefd >= 0)
> > +					close(comparefd);
> > +
> > +				return err;
> > +			}
> > +		}
> > +
> > +		written += r;
> >  		if (comparefd < 0 && sscanf(buf, "%u,%llu,%llu,%c;",
> >  					    &flags, &seq, &usec, &cont) == 4) {
> >  			/*
>