Skip to content

Commit

Permalink
util.c: add fsync_close() helper, use where appropriate
Browse files Browse the repository at this point in the history
Using genimage directly on an eMMC on target, I'm seeing that the
BLKRRPART ioctl fails with EBUSY, presumably because there's still
lots of data in flight, and then subsequent parts of my bootstrap
procedure fail because the expected partitions can't be found.

Whenever we've written some data, we really should ensure that all
data has hit the disk before proceeding, and close() itself is not
synchronous.

Add a fsync_close() helper that does exactly what it says on the
tin. Use that wherever we close an fd that has been open for writing
and actually written to (i.e., no point in doing that in the
reload_partitions() function).

Currently, the return value of these close() calls are ignored, so at
least for now continue to do that, but at least we do see an error
message in case something went wrong.

A test case is updated to reflect the new, and more accurate, disk
usage. It turns out that the lack of this fsync'ing has really created
images which later (long after genimage is done and the test framework
has accepted them) would increase in disk usage, i.e. once the kernel
gets around to write out any buffered data. With current master, one
can observe this:

  $ mkdir images input tmp
  $ dd if=/dev/zero of=input/part1.img bs=512 count=7 && dd if=/dev/zero of=input/part2.img bs=512 count=11 && touch input/part3.img
  $ ./genimage --outputpath=images --inputpath=input --rootpath=/no/such/dir --tmppath=tmp --config test/hdimage.config
  $ date ; du -B 1 images/test.hdimage-2
  Fri Nov  1 08:20:39 PM CET 2024
  61440   images/test.hdimage-2
  # Let time pass...
  $ date ; du -B 1 images/test.hdimage-2
  Fri Nov  1 08:21:10 PM CET 2024
  65536   images/test.hdimage-2

Signed-off-by: Rasmus Villemoes <[email protected]>
  • Loading branch information
ravi-prevas committed Nov 1, 2024
1 parent 92cc7c5 commit ec5fb97
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
2 changes: 1 addition & 1 deletion test/hdimage.test
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test_expect_success fdisk,sfdisk "hdimage" "
test_cmp '${testdir}/hdimage.fdisk' hdimage.fdisk &&
check_size images/test.hdimage-2 11539968 &&
sfdisk_validate images/test.hdimage-2 &&
check_disk_usage_range images/test.hdimage-2 61290 65376 &&
check_disk_usage_range images/test.hdimage-2 61290 65536 &&
sanitized_fdisk_sfdisk images/test.hdimage-2 > hdimage.fdisk-2 &&
test_cmp '${testdir}/hdimage.fdisk-2' hdimage.fdisk-2
"
Expand Down
21 changes: 17 additions & 4 deletions util.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,19 @@ void debug(const char *fmt, ...)
va_end(args);
}

static int fsync_close(struct image *image, int fd)
{
int a, b;

a = fsync(fd);
if (a)
image_error(image, "fsync() failed: %s\n", strerror(errno));
b = close(fd);
if (b)
image_error(image, "close() failed: %s\n", strerror(errno));
return (a || b) ? -1 : 0;
}

/*
* printf wrapper around 'system'
*/
Expand Down Expand Up @@ -519,7 +532,7 @@ int prepare_image(struct image *image, unsigned long long size)
* than necessary.
*/
ret = ftruncate(fd, size);
close(fd);
fsync_close(image, fd);
if (ret < 0) {
ret = -errno;
image_error(image, "failed to truncate %s to %lld: %s\n",
Expand Down Expand Up @@ -634,7 +647,7 @@ int insert_image(struct image *image, struct image *sub,

out:
if (fd >= 0)
close(fd);
fsync_close(image, fd);
if (in_fd >= 0)
close(in_fd);
free(extents);
Expand Down Expand Up @@ -671,7 +684,7 @@ int insert_data(struct image *image, const void *_data, const char *outfile,
data += now;
}
err_out:
close(outf);
fsync_close(image, outf);

return ret;
}
Expand Down Expand Up @@ -709,7 +722,7 @@ int extend_file(struct image *image, size_t size)
}
ret = 0;
out:
close(f);
fsync_close(image, f);
return ret;
}

Expand Down

0 comments on commit ec5fb97

Please sign in to comment.