ubiupdatevol from pipe/compressed file

ubiupdatevol from pipe/compressed file

[Hamish Moffatt]

Hi,

We discussed this briefly a few months back but I want to investigate a
bit further.

I would like to be able to update a ubi volume from a compressed file.
My system has limited RAM so I can't always keep the whole file
uncompressed in RAM, and I'd prefer not to uncompress it to temporary
space on flash, just to copy it somewhere else.

I'm doing this on a block device with "zcat img.gz | dd of=..." but
ubiupdatevol doesn't accept a pipe.

Reading the source this looks quite difficult to fix - it really must
know how many bytes are to be programmed in advance because it
communicates that to the kernel.

One solution I see is to add decompression support directly into
ubiupdatevol. eg with zlib it can gzseek() to the end of the file to get
the length, then rewind. I modified flashcp once to do this. However I
prefer to use better compression (lzma or bzip2) and we don't want to
add all of these to mtd-utils I think.

Alternatively allow the uncompressed file size to be specified as a
ubiupdatevol parameter. In my application I can store the uncompressed
file size and provide it to ubiupdatevol ok.

Artem, do you have any other ideas?

Thanks
Hamish
-- 
Hamish Moffatt VK3SB <hamish@debian.org> <hamish@cloud.net.au>

Re: ubiupdatevol from pipe/compressed file

[Artem Bityutskiy]

On Mon, 2008-05-19 at 18:47 +1000, Hamish Moffatt wrote:
> Alternatively allow the uncompressed file size to be specified as a
> ubiupdatevol parameter. In my application I can store the uncompressed
> file size and provide it to ubiupdatevol ok.

Yeah, I think it is reasonable to add one more option and pass the size
there. Then it could be easy to teach ubiupdatevol to read the specified
amount of bytes from stdin.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: ubiupdatevol from pipe/compressed file

[Hamish Moffatt]

On Mon, May 19, 2008 at 11:54:01AM +0300, Artem Bityutskiy wrote:
> On Mon, 2008-05-19 at 18:47 +1000, Hamish Moffatt wrote:
> > Alternatively allow the uncompressed file size to be specified as a
> > ubiupdatevol parameter. In my application I can store the uncompressed
> > file size and provide it to ubiupdatevol ok.
> 
> Yeah, I think it is reasonable to add one more option and pass the size
> there. Then it could be easy to teach ubiupdatevol to read the specified
> amount of bytes from stdin.

Here's a patch for ubiupdatevol. It contains 3 changes;

1. Adds -s/--size switch;
2. Allows the image file to be specified as '-', meaning stdin; if used,
the size must be specified.

3. In ubi_update_volume(), the code assumed that a read() requesting a
whole LEB worth of data would always return the amount requested.
However read() is allowed to return less than that, and does in some
cases eg reading from a pipe. I modified it to call read() in a loop
until it had a whole LEB worth of data.

Actually, it looks like the kernel (drivers/mtd/ubi/upd.c) doesn't
require the data in whole LEB chunks anyway, so this code could be
revert with the check for a whole LEB worth of read() removed. I can
send an updated patch tomorrow.

My ubiupdatevol.c original may be out of date. If so I can rebase
tomorrow.

Hamish


Index: ubi-utils/new-utils/src/ubiupdatevol.c
===================================================================
--- ubi-utils/new-utils/src/ubiupdatevol.c	(revision 4575)
+++ ubi-utils/new-utils/src/ubiupdatevol.c	(working copy)
@@ -48,6 +48,8 @@
 	int devn;
 	char dev_name[256];
 	int broken_update;
+	size_t size;
+	int use_stdin;
 };
 
 static struct args args = {
@@ -62,6 +64,7 @@
 "-t, --truncate             truncate volume (wipe it out)\n"
 "-h, --help                 print help message\n"
 "-V, --version              print program version\n\n"
+"-s, --size=<bytes>         bytes in input, if not reading from file\n"
 "The following are compatibility options which are deprecated, do not use them\n"
 "-d, --devn=<devn>          UBI device number - may be used instead of the UBI\n"
 "                           device node name in which case the utility assumes\n"
@@ -69,7 +72,7 @@
 "-B, --broken-update        broken update, this is for testing";
 
 static const char *usage =
-"Usage: " PROGRAM_NAME " <UBI volume node file name> [-t] [-h] [-V] [--truncate] [--help]\n"
+"Usage: " PROGRAM_NAME " <UBI volume node file name> [-t] [-h] [-V] [--truncate] [--size=x] [--help]\n"
 "\t\t\t[--version] <image file>\n\n"
 "Example 1: " PROGRAM_NAME " /dev/ubi0_1 fs.img - write file \"fs.img\" to UBI volume /dev/ubi0_1\n"
 "Example 2: " PROGRAM_NAME " /dev/ubi0_1 -t - wipe out UBI volume /dev/ubi0_1";
@@ -78,6 +81,7 @@
 	{ .name = "truncate", .has_arg = 0, .flag = NULL, .val = 't' },
 	{ .name = "help",     .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",  .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "size",     .has_arg = 1, .flag = NULL, .val = 's' },
 	/* Deprecated -d and -B options */
 	{ .name = "devn",     .has_arg = 1, .flag = NULL, .val = 'd' },
 	{ .name = "broken-update", .has_arg = 1, .flag = NULL, .val = 'B' },
@@ -89,7 +93,7 @@
 	while (1) {
 		int key;
 
-		key = getopt_long(argc, argv, "n:th?Vd:", long_options, NULL);
+		key = getopt_long(argc, argv, "n:th?Vd:s:", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -98,6 +102,15 @@
 			args.truncate = 1;
 			break;
 
+		case 's':
+		{
+			char *endp;
+			args.size = strtoul(optarg, &endp, 0);
+			if (*endp != '\0' || endp == optarg || args.size < 0)
+				return errmsg("bad size: " "\"%s\"", optarg);
+			break;
+		}
+
 		case 'h':
 		case '?':
 			fprintf(stderr, "%s\n\n", doc);
@@ -151,6 +164,11 @@
 	args.node = argv[optind];
 	args.img  = argv[optind + 1];
 
+	if (strcmp(args.img, "-") == 0)
+		args.use_stdin = 1;
+	if (args.use_stdin && !args.size)
+		return errmsg("file size must be specified if input is stdin");
+
 	return 0;
 }
 
@@ -202,20 +220,24 @@
 {
 	int err, fd, ifd;
 	long long bytes;
-	struct stat st;
 	char *buf;
 
 	buf = malloc(vol_info->leb_size);
 	if (!buf)
 		return errmsg("cannot allocate %d bytes of memory", vol_info->leb_size);
 
-	err = stat(args.img, &st);
-	if (err < 0) {
-		errmsg("stat failed on \"%s\"", args.img);
-		goto out_free;
-	}
+	if (!args.size) {
+		struct stat st;
+		err = stat(args.img, &st);
+		if (err < 0) {
+			errmsg("stat failed on \"%s\"", args.img);
+			goto out_free;
+		}
 
-	bytes = st.st_size;
+		bytes = st.st_size;
+	} else
+		bytes = args.size;
+
 	if (bytes > vol_info->rsvd_bytes) {
 		errmsg("\"%s\" (size %lld) will not fit volume \"%s\" (size %lld)",
 		       args.img, bytes, args.node, vol_info->rsvd_bytes);
@@ -232,10 +254,14 @@
 		goto out_free;
 	}
 
-	ifd = open(args.img, O_RDONLY);
-	if (ifd == -1) {
-		sys_errmsg("cannot open \"%s\"", args.img);
-		goto out_close1;
+	if (args.use_stdin)
+		ifd = STDIN_FILENO;
+	else {
+		ifd = open(args.img, O_RDONLY);
+		if (ifd == -1) {
+			sys_errmsg("cannot open \"%s\"", args.img);
+			goto out_close1;
+		}
 	}
 
 	err = ubi_update_start(libubi, fd, bytes);
@@ -246,19 +272,26 @@
 
 	while (bytes) {
 		int tocopy = vol_info->leb_size;
+		int pos, ret;
 
 		if (tocopy > bytes)
 			tocopy = bytes;
 
-		err = read(ifd, buf, tocopy);
-		if (err != tocopy) {
-			if (errno == EINTR) {
-				warnmsg("do not interrupt me!");
-				continue;
+		pos = 0;
+
+		while (pos < tocopy) {
+			ret = read(ifd, &buf[pos], tocopy - pos);
+			if (ret <= 0) {
+				if (errno == EINTR) {
+					warnmsg("do not interrupt me!");
+					continue;
+				} else {
+					sys_errmsg("cannot read %d bytes from \"%s\"",
+							tocopy, args.img);
+					goto out_close;
+				}
 			} else {
-				sys_errmsg("cannot read %d bytes from \"%s\"",
-					   tocopy, args.img);
-				goto out_close;
+				pos += ret;
 			}
 		}
 
-- 
Hamish Moffatt VK3SB <hamish@debian.org> <hamish@cloud.net.au>

Re: ubiupdatevol from pipe/compressed file

[Artem Bityutskiy]

Hi,

the patch looks good.

On Tue, 2008-05-20 at 18:39 +1000, Hamish Moffatt wrote:
> Actually, it looks like the kernel (drivers/mtd/ubi/upd.c) doesn't
> require the data in whole LEB chunks anyway, so this code could be
> revert with the check for a whole LEB worth of read() removed. I can
> send an updated patch tomorrow.

Yes, UBI accepts any amount of bytes at a time.

> 
> My ubiupdatevol.c original may be out of date. If so I can rebase
> tomorrow.

OK, thanks.
 
-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: ubiupdatevol from pipe/compressed file

[Artem Bityutskiy]

On Tue, 2008-05-20 at 18:39 +1000, Hamish Moffatt wrote:
> Here's a patch for ubiupdatevol. It contains 3 changes;

Could you please also increase the program version since it is a new
feature.
 
-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: ubiupdatevol from pipe/compressed file

[Artem Bityutskiy]

On Tue, 2008-05-20 at 18:39 +1000, Hamish Moffatt wrote:
> On Mon, May 19, 2008 at 11:54:01AM +0300, Artem Bityutskiy wrote:
> > On Mon, 2008-05-19 at 18:47 +1000, Hamish Moffatt wrote:
> > > Alternatively allow the uncompressed file size to be specified as a
> > > ubiupdatevol parameter. In my application I can store the uncompressed
> > > file size and provide it to ubiupdatevol ok.
> > 
> > Yeah, I think it is reasonable to add one more option and pass the size
> > there. Then it could be easy to teach ubiupdatevol to read the specified
> > amount of bytes from stdin.
> 
> Here's a patch for ubiupdatevol. It contains 3 changes;
> 
> 1. Adds -s/--size switch;
> 2. Allows the image file to be specified as '-', meaning stdin; if used,
> the size must be specified.
> 
> 3. In ubi_update_volume(), the code assumed that a read() requesting a
> whole LEB worth of data would always return the amount requested.
> However read() is allowed to return less than that, and does in some
> cases eg reading from a pipe. I modified it to call read() in a loop
> until it had a whole LEB worth of data.

You didn't send this N3 change, so I did this (and few more minor
tewaks) and pushed your patch, thanks.

http://git.infradead.org/mtd-utils.git?a=commit;h=8ec6531393464519a9d2720c27be200d226c21fb

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)