From 8ea641c1063b6983c1d62071e722c252216be26b Mon Sep 17 00:00:00 2001 From: Mike Hibler Date: Fri, 1 Dec 2006 20:34:24 +0000 Subject: [PATCH] Bug fixes from Annette DeSchon and Keith Sklower for the following, related to the -z (zero) option in imageunzip/frisbee: 1. For the case where a full-disk image is smaller than the disk the image is being unzipped onto, we added code to zero the area between the end of the image and the end of the disk. 2. During the unzipping process, when zeros are being written at the end of a chunk, a write() that returned a length different from the expected value previously caused an infinite loop. We noticed this problem at ISI, on a number of pc733s, which we suspect may have (relatively minor) hardware disk problems. The latter addressed a Mike-o that has existed for 4 years. Call it failure resilient computing or just plain denial, but because of a botched conditional, I was ignoring failed writes to the disk. This lead to one of those infinite loop thingees if you actually had a bad disk. --- os/frisbee.redux/GNUmakefile.in | 7 ++- os/imagezip/GNUmakefile.in | 4 +- os/imagezip/disksize.c | 11 +++- os/imagezip/imageunzip.c | 97 ++++++++++++++++++++++++++++++--- 4 files changed, 107 insertions(+), 12 deletions(-) diff --git a/os/frisbee.redux/GNUmakefile.in b/os/frisbee.redux/GNUmakefile.in index 71b8296f8..ce4eebd0a 100644 --- a/os/frisbee.redux/GNUmakefile.in +++ b/os/frisbee.redux/GNUmakefile.in @@ -1,6 +1,6 @@ # # EMULAB-COPYRIGHT -# Copyright (c) 2000-2004 University of Utah and the Flux Group. +# Copyright (c) 2000-2004, 2006 University of Utah and the Flux Group. # All rights reserved. # @@ -48,7 +48,7 @@ endif CLIENTFLAGS = $(CFLAGS) CLIENTLIBS = -lz $(PTHREADLIBS) -CLIENTOBJS = client.o frisbee.o $(SHAREDOBJS) +CLIENTOBJS = client.o frisbee.o disksize.o $(SHAREDOBJS) SERVERFLAGS = $(CFLAGS) SERVERLIBS = $(PTHREADLIBS) @@ -92,6 +92,9 @@ event.o: $(SRCDIR)/event.c decls.h log.h event.h $(FRISBEEDIR)/imageunzip.c: $(FRISBEEDIR)/imagehdr.h $(FRISBEEDIR)/queue.h +disksize.o: $(FRISBEEDIR)/disksize.c + $(CC) -c $(CFLAGS) -I$(FRISBEEDIR) -o disksize.o $< + frisbee.o: $(FRISBEEDIR)/imageunzip.c $(CC) -c $(CFLAGS) -DFRISBEE -I$(FRISBEEDIR) -o frisbee.o $< diff --git a/os/imagezip/GNUmakefile.in b/os/imagezip/GNUmakefile.in index 63628a1ab..eeb485c36 100644 --- a/os/imagezip/GNUmakefile.in +++ b/os/imagezip/GNUmakefile.in @@ -156,8 +156,8 @@ imagezip: imagezip.o disksize.o version.o $(ZIPLIBS) imagezip.o: imagezip.c $(CC) -c $(ZIPCFLAGS) -o imagezip.o $< -imageunzip: imageunzip.o crc.o version.o - $(CC) $(CFLAGS) imageunzip.o crc.o version.o $(UNZIPLIBS) -o imageunzip +imageunzip: imageunzip.o disksize.o crc.o version.o + $(CC) $(CFLAGS) imageunzip.o disksize.o crc.o version.o $(UNZIPLIBS) -o imageunzip imageunzip.o: imageunzip.c $(CC) -c $(UNZIPCFLAGS) -o imageunzip.o $< diff --git a/os/imagezip/disksize.c b/os/imagezip/disksize.c index c049f1281..5793ba275 100644 --- a/os/imagezip/disksize.c +++ b/os/imagezip/disksize.c @@ -1,6 +1,6 @@ /* * EMULAB-COPYRIGHT - * Copyright (c) 2000-2005 University of Utah and the Flux Group. + * Copyright (c) 2000-2006 University of Utah and the Flux Group. * All rights reserved. */ @@ -29,6 +29,7 @@ getdisksize(int fd) { unsigned long disksize = 0; unsigned int ssize = 512; + off_t whuzat; #ifdef linux if (disksize == 0) { @@ -63,6 +64,8 @@ getdisksize(int fd) #endif #endif + whuzat = lseek(fd, (off_t)0, SEEK_CUR); + /* * OS wouldn't tell us anything directly, try a seek to the * end of the device. @@ -83,5 +86,11 @@ getdisksize(int fd) "could not seek to final sector (%lu) of disk\n", disksize - 1); + if (whuzat >= 0) { + if (lseek(fd, whuzat, SEEK_SET) < 0) + fprintf(stderr, "WARNING: " + "could not seek to previous offset on disk\n"); + } + return disksize; } diff --git a/os/imagezip/imageunzip.c b/os/imagezip/imageunzip.c index 9f2462094..035c86f6c 100644 --- a/os/imagezip/imageunzip.c +++ b/os/imagezip/imageunzip.c @@ -1,6 +1,6 @@ /* * EMULAB-COPYRIGHT - * Copyright (c) 2000-2005 University of Utah and the Flux Group. + * Copyright (c) 2000-2006 University of Utah and the Flux Group. * All rights reserved. */ @@ -95,11 +95,13 @@ static int inflate_subblock(const char *); void writezeros(off_t offset, off_t zcount); void writedata(off_t offset, size_t count, void *buf); +static void zero_remainder(void); static void getrelocinfo(const blockhdr_t *hdr); static void applyrelocs(off_t offset, size_t cc, void *buf); static int seekable; static off_t nextwriteoffset; +static off_t maxwrittenoffset; static int imagetoobigwarned; @@ -255,8 +257,8 @@ void dodots(int dottype, off_t cc) if ((count = newgb - lastgb) <= 0) return; lastgb = newgb; - chr = (dottype == DODOTS_DATA) ? '*' : - (dottype == DODOTS_ZERO) ? 'o' : 's'; + chr = (dottype == DODOTS_DATA) ? '.' : + (dottype == DODOTS_ZERO) ? 'z' : 's'; break; } @@ -687,6 +689,21 @@ main(int argc, char *argv[]) perror("Setting seek pointer to slice"); exit(1); } + + /* + * XXX zeroing and slice mode don't mix. + * + * To do so properly, we would have to zero all space before + * and after the slice in question. But zeroing before would + * clobber the MBR and boot info, probably not what was + * intended. So we just don't do it til we figure out what + * the "right" behavior is. + */ + if (slice) { + fprintf(stderr, + "WARNING: requested zeroing in slice mode, " + "will NOT zero outside of slice!\n"); + } } threadinit(); @@ -766,6 +783,9 @@ main(int argc, char *argv[]) done: close(infd); + /* When zeroing, may need to zero the rest of the disk */ + zero_remainder(); + /* This causes the output queue to drain */ threadquit(); @@ -874,6 +894,9 @@ ImageUnzipChunk(char *chunkdata) void ImageUnzipFlush(void) { + /* When zeroing, may need to zero the rest of the disk */ + zero_remainder(); + threadwait(); } @@ -1323,7 +1346,7 @@ inflate_subblock(const char *chunkbufp) void writezeros(off_t offset, off_t zcount) { - size_t zcc; + size_t zcc, wcc; off_t ozcount; assert((offset & (SECSIZE-1)) == 0); @@ -1361,17 +1384,30 @@ writezeros(off_t offset, off_t zcount) compute_crc((u_char *)zeros, zcc, &crc); else #endif - if ((zcc = write(outfd, zeros, zcc)) != zcc) { - if (zcc < 0) { + if ((wcc = write(outfd, zeros, zcc)) != zcc) { + if (wcc < 0) { perror("Writing Zeros"); + } else if ((wcc & (SECSIZE-1)) != 0) { + fprintf(stderr, "Writing Zeros: " + "partial sector write (%d bytes)\n", + wcc); + wcc = -1; + } else if (wcc == 0) { + fprintf(stderr, "Writing Zeros: " + "unexpected EOF\n"); + wcc = -1; } - exit(1); + if (wcc < 0) + exit(1); + zcc = wcc; } zcount -= zcc; totalrdata += zcc; nextwriteoffset += zcc; dodots(DODOTS_ZERO, ozcount-zcount); } + if (nextwriteoffset > maxwrittenoffset) + maxwrittenoffset = nextwriteoffset; } void @@ -1405,10 +1441,57 @@ writedata(off_t offset, size_t size, void *buf) exit(1); } nextwriteoffset = offset + cc; + if (nextwriteoffset > maxwrittenoffset) + maxwrittenoffset = nextwriteoffset; totalrdata += cc; dodots(DODOTS_DATA, cc); } +/* + * If the target disk is larger than the disk on which the image was made, + * there will be some remaining space on the disk that needs to be zeroed. + */ +static void +zero_remainder() +{ + extern unsigned long getdisksize(int fd); + off_t disksize; + + if (!dofill) + return; + + /* XXX zeroing and slice mode don't mix; see earlier comment. */ + if (slice) + return; + + if (outputmaxsec == 0) + outputmaxsec = getdisksize(outfd); + disksize = sectobytes(outputmaxsec); + if (debug) + fprintf(stderr, "\ndisksize = %lld\n", disksize); + + /* XXX must wait for writer thread to finish to get maxwrittenoffset value */ + threadwait(); + + if (disksize > maxwrittenoffset) { + off_t remaining = disksize - maxwrittenoffset; + writebuf_t *wbuf; + + if (debug) + fprintf(stderr, "zeroing %lld bytes at offset %lld " + "(%u sectors at %u)\n", + remaining, maxwrittenoffset, + bytestosec(remaining), bytestosec(maxwrittenoffset)); + wbuf = alloc_writebuf(maxwrittenoffset, remaining, 0, 1); + dowrite_request(wbuf); + } else { + if (debug) + fprintf(stderr, "not zeroing: disksize = %lld, " + "maxwritten = %lld\n", + disksize, maxwrittenoffset); + } +} + #include "sliceinfo.h" static long long outputmaxsize = 0; -- GitLab