Date: Thu, 18 Feb 2016 22:12:55 -0800 From: Maxim Sobolev <sobomax@FreeBSD.org> To: Thomas Quinot <thomas@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn: head/bin/dd Message-ID: <CAH7qZft0YH0KLgkPydZVgppSsA=0c122j0w03eu0m_g0M1M_zA@mail.gmail.com> In-Reply-To: <201602180844.u1I8iGTN082641@repo.freebsd.org> References: <201602180844.u1I8iGTN082641@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Thanks, Thomas. IMHO this is quite serious bug in one of the core utilities, so I suggest it gets faster MFC before 10.3 goes into beta stage on Feb 26. The old code in RELENG_10 is just plain broken anyway now, so it could not make it any worse. Thanks! On Thu, Feb 18, 2016 at 12:44 AM, Thomas Quinot <thomas@freebsd.org> wrote: > Author: thomas > Date: Thu Feb 18 08:44:16 2016 > New Revision: 295749 > URL: https://svnweb.freebsd.org/changeset/base/295749 > > Log: > Reorganize the handling all-zeroes terminal block in sparse mode > > The intent of the previous code in that case was to force > an explicit write, but the implementation was incorrect, and > as a result the write was never performed. This new implementation > instead uses ftruncate(2) to extend the file with a trailing hole. > > Also introduce regression tests for these cases. > > PR: 189284 > (original PR whose fix introduced this bug) > > PR: 207092 > > Differential Revision: D5248 > Reviewed by: sobomax,kib > MFC after: 2 weeks > > Added: > head/bin/dd/ref.obs_zeroes (contents, props changed) > Modified: > head/bin/dd/Makefile > head/bin/dd/dd.c > head/bin/dd/dd.h > head/bin/dd/gen.c > > Modified: head/bin/dd/Makefile > > ============================================================================== > --- head/bin/dd/Makefile Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/Makefile Thu Feb 18 08:44:16 2016 (r295749) > @@ -24,7 +24,18 @@ test: ${PROG} gen > LC_ALL=en_US.US-ASCII hexdump -C | \ > diff -I FreeBSD - ${.CURDIR}/ref.${conv} > .endfor > - @rm -f gen > + @${ECHO} "testing sparse file (obs zeroes)" > + @./gen 189284 | ./dd ibs=16 obs=8 conv=sparse of=obs_zeroes 2> > /dev/null > + @hexdump -C obs_zeroes | diff -I FreeBSD - > ${.CURDIR}/ref.obs_zeroes > + > + @${ECHO} "testing spase file (all zeroes)" > + @./dd if=/dev/zero of=1M_zeroes bs=1048576 count=1 2> /dev/null > + @./dd if=1M_zeroes of=1M_zeroes.1 bs=1048576 conv=sparse 2> > /dev/null > + @./dd if=1M_zeroes of=1M_zeroes.2 bs=1048576 2> /dev/null > + @diff 1M_zeroes 1M_zeroes.1 > + @diff 1M_zeroes 1M_zeroes.2 > + > + @rm -f gen 1M_zeroes* obs_zeroes > > .if ${MK_TESTS} != "no" > SUBDIR+= tests > > Modified: head/bin/dd/dd.c > > ============================================================================== > --- head/bin/dd/dd.c Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/dd.c Thu Feb 18 08:44:16 2016 (r295749) > @@ -77,7 +77,6 @@ STAT st; /* statistics */ > void (*cfunc)(void); /* conversion function */ > uintmax_t cpy_cnt; /* # of blocks to copy */ > static off_t pending = 0; /* pending seek if sparse */ > -static off_t last_sp = 0; /* size of last added sparse block */ > u_int ddflags = 0; /* conversion options */ > size_t cbsz; /* conversion block size */ > uintmax_t files_cnt = 1; /* # of files to copy */ > @@ -409,6 +408,15 @@ dd_close(void) > } > if (out.dbcnt || pending) > dd_out(1); > + > + /* > + * If the file ends with a hole, ftruncate it to extend its size > + * up to the end of the hole (without having to write any data). > + */ > + if (out.seek_offset > 0 && (out.flags & ISTRUNC)) { > + if (ftruncate(out.fd, out.seek_offset) == -1) > + err(1, "truncating %s", out.name); > + } > } > > void > @@ -457,29 +465,27 @@ dd_out(int force) > } > if (sparse && !force) { > pending += cnt; > - last_sp = cnt; > nw = cnt; > } else { > if (pending != 0) { > - /* If forced to write, and we have > no > - * data left, we need to write the > last > - * sparse block explicitly. > + /* > + * Seek past hole. Note that we > need to record the > + * reached offset, because we > might have no more data > + * to write, in which case we'll > need to call > + * ftruncate to extend the file > size. > */ > - if (force && cnt == 0) { > - pending -= last_sp; > - assert(outp == out.db); > - memset(outp, 0, cnt); > - } > - if (lseek(out.fd, pending, > SEEK_CUR) == > - -1) > + out.seek_offset = lseek(out.fd, > pending, SEEK_CUR); > + if (out.seek_offset == -1) > err(2, "%s: seek error > creating sparse file", > out.name); > - pending = last_sp = 0; > + pending = 0; > } > - if (cnt) > + if (cnt) { > nw = write(out.fd, outp, cnt); > - else > + out.seek_offset = 0; > + } else { > return; > + } > } > > if (nw <= 0) { > > Modified: head/bin/dd/dd.h > > ============================================================================== > --- head/bin/dd/dd.h Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/dd.h Thu Feb 18 08:44:16 2016 (r295749) > @@ -54,6 +54,7 @@ typedef struct { > const char *name; /* name */ > int fd; /* file descriptor */ > off_t offset; /* # of blocks to skip */ > + off_t seek_offset; /* offset of last seek past output > hole */ > } IO; > > typedef struct { > > Modified: head/bin/dd/gen.c > > ============================================================================== > --- head/bin/dd/gen.c Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/gen.c Thu Feb 18 08:44:16 2016 (r295749) > @@ -5,13 +5,20 @@ > */ > > #include <stdio.h> > +#include <string.h> > > int > -main(int argc __unused, char **argv __unused) > +main(int argc, char **argv) > { > int i; > > - for (i = 0; i < 256; i++) > - putchar(i); > + if (argc > 1 && !strcmp(argv[1], "189284")) { > + fputs("ABCDEFGH", stdout); > + for (i = 0; i < 8; i++) > + putchar(0); > + } else { > + for (i = 0; i < 256; i++) > + putchar(i); > + } > return (0); > } > > Added: head/bin/dd/ref.obs_zeroes > > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/bin/dd/ref.obs_zeroes Thu Feb 18 08:44:16 2016 (r295749) > @@ -0,0 +1,3 @@ > +$FreeBSD$ > +00000000 41 42 43 44 45 46 47 48 00 00 00 00 00 00 00 00 > |ABCDEFGH........| > +00000010 > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAH7qZft0YH0KLgkPydZVgppSsA=0c122j0w03eu0m_g0M1M_zA>