Re: [PATCH v2] xfs_copy: flush target devices before exiting

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Tue, Jun 09, 2020 at 11:31:52PM -0500, Eric Sandeen wrote:
> 
> On 6/8/20 1:47 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Flush the devices we're copying to before exiting, so that we can report
> > any write errors.
> 
> Hm, I hadn't really looked at xfs_copy in depth, funky stuff.
> 
> So normally errors look something like:
> 
> THE FOLLOWING COPIES FAILED TO COMPLETE
>     $TARGET -- write error at offset ABC
>     $TARGET -- seek error at offset XYZ
> 
> if for some reason we didn't detect any errors until this final
> device flush, we'll only see:
> 
>     $TARGET -- flush error -Q
> 
> and no header...
> 
> Seems like this error needs to be integrated w/ the other error reporting,
> something like:
> 
> check_errors(void)
> {
>         int     i, flush_error, first_error = 0;
> 
>         for (i = 0; i < num_targets; i++)  {
>                 flush_error = platform_flush_device(target[i].fd, 0);

<shrug> I was gonna skip the flush error if the target already died,
but yeah I agree this should come before THE FOLLOWING COPIES WERE EATEN
BY CIRCUMSTANCE.

--D

> 
>                 if (flush_error || target[i].state == INACTIVE)  {
>                         if (first_error == 0)  {
>                                 first_error++;
>                                 do_log(
>                                 _("THE FOLLOWING COPIES FAILED TO COMPLETE\n"));
>                         }
>                         if (target[i].state == INACTIVE) {
>                                 do_log("    %s -- ", target[i].name);
>                                 if (target[i].err_type == 0)
>                                         do_log(_("write error"));
>                                 else
>                                         do_log(_("lseek error"));
>                                 do_log(_(" at offset %lld\n"), target[i].position);
>                         }
>                         if (flush_error)
>                                 do_log(_("    %s -- flush error %d"),
>                                                 target[i].name, errno);
>                 }
>         }
>         if (first_error == 0)  {
>                 fprintf(stdout, _("All copies completed.\n"));
>                 fflush(NULL);
>         } else  {
>                 fprintf(stderr, _("See \"%s\" for more details.\n"),
>                         logfile_name);
>                 exit(1);
>         }
> }
> 
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  copy/xfs_copy.c |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
> > index 2d087f71..7657ad3e 100644
> > --- a/copy/xfs_copy.c
> > +++ b/copy/xfs_copy.c
> > @@ -12,6 +12,7 @@
> >  #include <stdarg.h>
> >  #include "xfs_copy.h"
> >  #include "libxlog.h"
> > +#include "libfrog/platform.h"
> >  
> >  #define	rounddown(x, y)	(((x)/(y))*(y))
> >  #define uuid_equal(s,d) (platform_uuid_compare((s),(d)) == 0)
> > @@ -150,6 +151,10 @@ check_errors(void)
> >  			else
> >  				do_log(_("lseek error"));
> >  			do_log(_(" at offset %lld\n"), target[i].position);
> > +		} else if (platform_flush_device(target[i].fd, 0)) {
> > +			do_log(_("    %s -- flush error %d"),
> > +					target[i].name, errno);
> > +			first_error++;
> >  		}
> >  	}
> >  	if (first_error == 0)  {
> >