[PATCH v2 makedumpfile 0/3] Add option to limit file size

[PATCH v2 makedumpfile 0/3] Add option to limit file size

[Benjamin Poirier]

See feature description in patch "Add -L option to limit output file
size"

v1:
	http://lists.infradead.org/pipermail/kexec/2021-June/022728.html
v2:
	Add patches 1, 2 to support dmesg limit with different kernel
	versions.

	Stricter parsing of -L option value.

	Instead of using RLIMIT_FSIZE, use write_bytes or SEEK_CUR to
	enforce limit. This better integrates with the -F option for
	stdout output.

Benjamin Poirier (3):
  Fix off by one error when checking cache_size
  Dump dmesg using write_and_check_space()
  Add -L option to limit output file size

 makedumpfile.8 |  12 ++++--
 makedumpfile.c | 115 +++++++++++++++++++++++++++++++++++--------------
 makedumpfile.h |   6 ++-
 print_info.c   |   3 ++
 printk.c       |  24 +++++++----
 sadump_info.c  |   2 +-
 6 files changed, 116 insertions(+), 46 deletions(-)

-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 makedumpfile 1/3] Fix off by one error when checking cache_size

[Benjamin Poirier]

Given the test in write_cache():
	if (cd->buf_size < cd->cache_size)
		return TRUE;
a write is done if buf_size == cache_size.
The test at the beginning of write_kdump_page() intends to detect cases
when write_cache() will do a write, however it was missing this boundary
condition. This would lead write_kdump_page() to ignore errors from
write_cache().

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 makedumpfile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index fcb571f..aa05be7 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7993,8 +7993,8 @@ write_kdump_page(struct cache_data *cd_header, struct cache_data *cd_page,
 	 * write the buffer cd_header into dumpfile and then write the cd_page.
 	 * With that, when enospc occurs, we can save more useful information.
 	 */
-	if (cd_header->buf_size + sizeof(*pd) > cd_header->cache_size
-		|| cd_page->buf_size + pd->size > cd_page->cache_size){
+	if (cd_header->buf_size + sizeof(*pd) >= cd_header->cache_size ||
+	    cd_page->buf_size + pd->size >= cd_page->cache_size) {
 		if( !write_cd_buf(cd_header) ) {
 			memset(cd_header->buf, 0, cd_header->cache_size);
 			write_cd_buf(cd_header);
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 makedumpfile 2/3] Dump dmesg using write_and_check_space()

[Benjamin Poirier]

Detect incomplete writes which may happen due to insufficient space on the
output filesystem.

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 makedumpfile.c | 36 ++++++++++++++++++++----------------
 makedumpfile.h |  3 ++-
 printk.c       | 24 +++++++++++++++---------
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index aa05be7..9175a6a 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -4713,7 +4713,8 @@ is_bigendian(void)
 }
 
 int
-write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
+write_and_check_space(int fd, void *buf, size_t buf_size, const char* desc,
+		      const char *file_name)
 {
 	int status, written_size = 0;
 
@@ -4731,8 +4732,8 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
 		}
 		if (errno == ENOSPC)
 			info->flag_nospace = TRUE;
-		MSG("\nCan't write the dump file(%s). %s\n",
-		    file_name, strerror(errno));
+		MSG("\nCan't write the %s file(%s). %s\n", desc, file_name,
+		    strerror(errno));
 		return FALSE;
 	}
 	return TRUE;
@@ -4757,7 +4758,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
 			fdh.offset   = bswap_64(offset);
 			fdh.buf_size = bswap_64(buf_size);
 		}
-		if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
+		if (!write_and_check_space(fd, &fdh, sizeof(fdh), "dump",
+					   file_name))
 			return FALSE;
 	} else if (!info->flag_dry_run &&
 		    lseek(fd, offset, SEEK_SET) == failed) {
@@ -4765,7 +4767,7 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
 		return FALSE;
 	}
 
-	if (!write_and_check_space(fd, buf, buf_size, file_name))
+	if (!write_and_check_space(fd, buf, buf_size, "dump", file_name))
 		return FALSE;
 
 	return TRUE;
@@ -5281,7 +5283,7 @@ reset_bitmap_of_free_pages(unsigned long node_zones, struct cycle *cycle)
 }
 
 static int
-dump_log_entry(char *logptr, int fp)
+dump_log_entry(char *logptr, int fp, const char *file_name)
 {
 	char *msg, *p, *bufp;
 	unsigned int i, text_len, indent_len, buf_need;
@@ -5307,7 +5309,8 @@ dump_log_entry(char *logptr, int fp)
 
 	for (i = 0, p = msg; i < text_len; i++, p++) {
 		if (bufp - buf >= sizeof(buf) - buf_need) {
-			if (write(info->fd_dumpfile, buf, bufp - buf) < 0)
+			if (!write_and_check_space(fp, buf, bufp - buf, "log",
+						   file_name))
 				return FALSE;
 			bufp = buf;
 		}
@@ -5322,10 +5325,7 @@ dump_log_entry(char *logptr, int fp)
 
 	*bufp++ = '\n';
 
-	if (write(info->fd_dumpfile, buf, bufp - buf) < 0)
-		return FALSE;
-	else
-		return TRUE;
+	return write_and_check_space(fp, buf, bufp - buf, "log", file_name);
 }
 
 /*
@@ -5507,7 +5507,9 @@ dump_dmesg()
 			ERRMSG("Can't open output file.\n");
 			goto out;
 		}
-		if (write(info->fd_dumpfile, log_buffer, length_log) < 0)
+		if (!write_and_check_space(info->fd_dumpfile, log_buffer,
+					   length_log, "log",
+					   info->name_dumpfile))
 			goto out;
 
 		if (!close_files_for_creating_dumpfile())
@@ -5532,7 +5534,8 @@ dump_dmesg()
 		idx = log_first_idx;
 		while (idx != log_next_idx) {
 			log_ptr = log_from_idx(idx, log_buffer);
-			if (!dump_log_entry(log_ptr, info->fd_dumpfile))
+			if (!dump_log_entry(log_ptr, info->fd_dumpfile,
+					    info->name_dumpfile))
 				goto out;
 			idx = log_next(idx, log_buffer);
 		}
@@ -6947,8 +6950,9 @@ write_start_flat_header()
 	memset(buf, 0, sizeof(buf));
 	memcpy(buf, &fh, sizeof(fh));
 
-	if (!write_and_check_space(info->fd_dumpfile, buf, MAX_SIZE_MDF_HEADER,
-	    info->name_dumpfile))
+	if (!write_and_check_space(info->fd_dumpfile, buf,
+				   MAX_SIZE_MDF_HEADER, "dump",
+				   info->name_dumpfile))
 		return FALSE;
 
 	return TRUE;
@@ -6966,7 +6970,7 @@ write_end_flat_header(void)
 	fdh.buf_size = END_FLAG_FLAT_HEADER;
 
 	if (!write_and_check_space(info->fd_dumpfile, &fdh, sizeof(fdh),
-	    info->name_dumpfile))
+				   "dump", info->name_dumpfile))
 		return FALSE;
 
 	return TRUE;
diff --git a/makedumpfile.h b/makedumpfile.h
index 79046f2..fb23efd 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -2510,7 +2510,8 @@ ulong htol(char *s, int flags);
 int hexadecimal(char *s, int count);
 int decimal(char *s, int count);
 int file_exists(char *file);
-
+int write_and_check_space(int fd, void *buf, size_t buf_size,
+			  const char* desc, const char *file_name);
 int open_dump_file(void);
 int dump_lockless_dmesg(void);
 
diff --git a/printk.c b/printk.c
index 2af8562..e8501c7 100644
--- a/printk.c
+++ b/printk.c
@@ -53,7 +53,7 @@ static enum desc_state get_desc_state(unsigned long id,
 	return DESC_STATE(state_val);
 }
 
-static void
+static int
 dump_record(struct prb_map *m, unsigned long id)
 {
 	unsigned long long ts_nsec;
@@ -80,7 +80,7 @@ dump_record(struct prb_map *m, unsigned long id)
 	state_var = ULONG(desc + OFFSET(prb_desc.state_var) + OFFSET(atomic_long_t.counter));
 	state = get_desc_state(id, state_var);
 	if (state != desc_committed && state != desc_finalized)
-		return;
+		return TRUE;
 
 	begin = ULONG(desc + OFFSET(prb_desc.text_blk_lpos) + OFFSET(prb_data_blk_lpos.begin)) %
 			m->text_data_ring_size;
@@ -89,7 +89,7 @@ dump_record(struct prb_map *m, unsigned long id)
 
 	/* skip data-less text blocks */
 	if (begin == next)
-		return;
+		return TRUE;
 
 	inf = m->infos + ((id % m->desc_ring_count) * SIZE(printk_info));
 
@@ -121,8 +121,10 @@ dump_record(struct prb_map *m, unsigned long id)
 
 	for (i = 0, p = text; i < text_len; i++, p++) {
 		if (bufp - buf >= sizeof(buf) - buf_need) {
-			if (write(info->fd_dumpfile, buf, bufp - buf) < 0)
-				return;
+			if (!write_and_check_space(info->fd_dumpfile, buf,
+						   bufp - buf, "log",
+						   info->name_dumpfile))
+				return FALSE;
 			bufp = buf;
 		}
 
@@ -136,7 +138,8 @@ dump_record(struct prb_map *m, unsigned long id)
 
 	*bufp++ = '\n';
 
-	write(info->fd_dumpfile, buf, bufp - buf);
+	return write_and_check_space(info->fd_dumpfile, buf, bufp - buf,
+				     "log", info->name_dumpfile);
 }
 
 int
@@ -219,11 +222,14 @@ dump_lockless_dmesg(void)
 		goto out_text_data;
 	}
 
-	for (id = tail_id; id != head_id; id = (id + 1) & DESC_ID_MASK)
-		dump_record(&m, id);
+	for (id = tail_id; id != head_id; id = (id + 1) & DESC_ID_MASK) {
+		if (!dump_record(&m, id))
+			goto out_text_data;
+	}
 
 	/* dump head record */
-	dump_record(&m, id);
+	if (!dump_record(&m, id))
+		goto out_text_data;
 
 	if (!close_files_for_creating_dumpfile())
 		goto out_text_data;
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 makedumpfile 3/3] Add -L option to limit output file size

[Benjamin Poirier]

This option can be used to ensure that a certain amount of free space is
preserved. It is useful when the output of makedumpfile is on the root
filesystem and some services fail to start at boot if there is no space
left.

Note that in some cases the limit can be triggered before the output file
reaches the specified size because makedumpfile seeks and tries to write
beyond the end of the file.

Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 makedumpfile.8 | 12 ++++++--
 makedumpfile.c | 79 ++++++++++++++++++++++++++++++++++++++++----------
 makedumpfile.h |  3 ++
 print_info.c   |  3 ++
 sadump_info.c  |  2 +-
 5 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/makedumpfile.8 b/makedumpfile.8
index 313a41c..9a90f0e 100644
--- a/makedumpfile.8
+++ b/makedumpfile.8
@@ -157,9 +157,10 @@ will be effective if you specify domain-0's \fIvmlinux\fR with \-x option.
 Then the pages are excluded only from domain-0.
 .br
 If specifying multiple dump_levels with the delimiter ',', makedumpfile retries
-to create a \fIDUMPFILE\fR by other dump_level when "No space on device" error
-happens. For example, if dump_level is "11,31" and makedumpfile fails
-by dump_level 11, makedumpfile retries it by dump_level 31.
+to create \fIDUMPFILE\fR using the next dump_level when the size of a dumpfile
+exceeds the limit specified with '-L' or when when a "No space on device" error
+happens. For example, if dump_level is "11,31" and makedumpfile fails with
+dump_level 11, makedumpfile retries with dump_level 31.
 .br
 .B Example:
 .br
@@ -221,6 +222,11 @@ Here is the all combinations of the bits.
     30 |      |   X   |   X   |  X   |  X
     31 |  X   |   X   |   X   |  X   |  X
 
+.TP
+\fB\-L\fR \fISIZE\fR
+Limit the size of the output file to \fISIZE\fR bytes. An incomplete
+\fIDUMPFILE\fR or \fILOGFILE\fR is written if the size would otherwise exceed
+\fISIZE\fR.
 
 .TP
 \fB\-E\fR
diff --git a/makedumpfile.c b/makedumpfile.c
index 9175a6a..4518ef4 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -4716,27 +4716,62 @@ int
 write_and_check_space(int fd, void *buf, size_t buf_size, const char* desc,
 		      const char *file_name)
 {
-	int status, written_size = 0;
+	size_t limit, done = 0;
+	int retval = 0;
+	off_t pos;
 
-	write_bytes += buf_size;
+	if (fd == STDOUT_FILENO) {
+		pos = write_bytes;
+	} else {
+		pos = lseek(fd, 0, SEEK_CUR);
+		if (pos == -1) {
+			ERRMSG("Can't seek the dump file(%s). %s\n",
+			       file_name, strerror(errno));
+			return FALSE;
+		}
+	}
+
+	if (info->size_limit != -1 && pos + buf_size > info->size_limit) {
+		if (pos > info->size_limit)
+			limit = 0;
+		else
+			limit = info->size_limit - pos;
+		info->flag_nospace = TRUE;
+	} else {
+		limit = buf_size;
+	}
 
 	if (info->flag_dry_run)
-		return TRUE;
+		done = limit;
 
-	while (written_size < buf_size) {
-		status = write(fd, buf + written_size,
-				   buf_size - written_size);
-		if (0 < status) {
-			written_size += status;
-			continue;
+	while (done < limit) {
+		retval = write(fd, buf, limit - done);
+		if (retval > 0) {
+			done += retval;
+			buf += retval;
+		} else {
+			if (retval == -1) {
+				if (errno == EINTR)
+					continue;
+				if (errno == ENOSPC)
+					info->flag_nospace = TRUE;
+				else
+					info->flag_nospace = FALSE;
+				MSG("\nCan't write the %s file(%s). %s\n",
+				    desc, file_name, strerror(errno));
+			}
+			break;
 		}
-		if (errno == ENOSPC)
-			info->flag_nospace = TRUE;
-		MSG("\nCan't write the %s file(%s). %s\n", desc, file_name,
-		    strerror(errno));
-		return FALSE;
 	}
-	return TRUE;
+
+	write_bytes += done;
+
+	if (retval != -1 && done < buf_size) {
+		MSG("\nCan't write the %s file(%s). Size limit(%llu) reached.\n",
+		    desc, file_name, (unsigned long long) info->size_limit);
+	}
+
+	return done == buf_size;
 }
 
 int
@@ -11589,6 +11624,7 @@ main(int argc, char *argv[])
 	}
 	info->file_vmcoreinfo = NULL;
 	info->fd_vmlinux = -1;
+	info->size_limit = -1;
 	info->fd_xen_syms = -1;
 	info->fd_memory = -1;
 	info->fd_dumpfile = -1;
@@ -11609,9 +11645,12 @@ main(int argc, char *argv[])
 
 	info->block_order = DEFAULT_ORDER;
 	message_level = DEFAULT_MSG_LEVEL;
-	while ((opt = getopt_long(argc, argv, "b:cDd:eEFfg:hi:lpRvXx:", longopts,
+	while ((opt = getopt_long(argc, argv, "b:cDd:eEFfg:hi:lL:pRvXx:", longopts,
 	    NULL)) != -1) {
 		switch (opt) {
+			unsigned long long val;
+			char *endptr;
+
 		case OPT_BLOCK_ORDER:
 			info->block_order = atoi(optarg);
 			break;
@@ -11628,6 +11667,14 @@ main(int argc, char *argv[])
 			if (!parse_dump_level(optarg))
 				goto out;
 			break;
+		case OPT_SIZE_LIMIT:
+			val = memparse(optarg, &endptr);
+			if (*endptr || val == 0) {
+				MSG("Limit size(%s) is invalid.\n", optarg);
+				goto out;
+			}
+			info->size_limit = val;
+			break;
 		case OPT_ELF_DUMPFILE:
 			info->flag_elf_dumpfile = 1;
 			break;
diff --git a/makedumpfile.h b/makedumpfile.h
index fb23efd..ca50a89 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1344,6 +1344,7 @@ struct DumpInfo {
 	int		max_dump_level;      /* maximum dump level */
 	int		num_dump_level;      /* number of dump level */
 	int		array_dump_level[NUM_ARRAY_DUMP_LEVEL];
+	off_t		size_limit;          /* dump file size limit */
 	int		flag_compress;       /* flag of compression */
 	int		flag_lzo_support;    /* flag of LZO compression support */
 	int		flag_elf_dumpfile;   /* flag of creating ELF dumpfile */
@@ -2458,6 +2459,7 @@ struct elf_prstatus {
 #define OPT_HELP                'h'
 #define OPT_READ_VMCOREINFO     'i'
 #define OPT_COMPRESS_LZO        'l'
+#define OPT_SIZE_LIMIT          'L'
 #define OPT_COMPRESS_SNAPPY     'p'
 #define OPT_REARRANGE           'R'
 #define OPT_VERSION             'v'
@@ -2514,5 +2516,6 @@ int write_and_check_space(int fd, void *buf, size_t buf_size,
 			  const char* desc, const char *file_name);
 int open_dump_file(void);
 int dump_lockless_dmesg(void);
+unsigned long long memparse(char *ptr, char **retptr);
 
 #endif /* MAKEDUMPFILE_H */
diff --git a/print_info.c b/print_info.c
index ad4184e..8b28554 100644
--- a/print_info.c
+++ b/print_info.c
@@ -144,6 +144,9 @@ print_usage(void)
 	MSG("        16  |                                    X\n");
 	MSG("        31  |   X       X        X       X       X\n");
 	MSG("\n");
+	MSG("  [-L SIZE]:\n");
+	MSG("      Limit the size of the output file to SIZE bytes.\n");
+	MSG("\n");
 	MSG("  [-E]:\n");
 	MSG("      Create DUMPFILE in the ELF format.\n");
 	MSG("      This option cannot be specified with the -c, -l or -p options,\n");
diff --git a/sadump_info.c b/sadump_info.c
index 9f5f568..b91eae5 100644
--- a/sadump_info.c
+++ b/sadump_info.c
@@ -1062,7 +1062,7 @@ get_vec0_addr(ulong idtr)
  * Parse a string of [size[KMG]@]offset[KMG]
  * Import from Linux kernel(lib/cmdline.c)
  */
-static ulong memparse(char *ptr, char **retptr)
+unsigned long long memparse(char *ptr, char **retptr)
 {
 	char *endptr;
 
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH v2 makedumpfile 0/3] Add option to limit file size

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> See feature description in patch "Add -L option to limit output file
> size"
> 
> v1:
> 	http://lists.infradead.org/pipermail/kexec/2021-June/022728.html
> v2:
> 	Add patches 1, 2 to support dmesg limit with different kernel
> 	versions.
> 
> 	Stricter parsing of -L option value.
> 
> 	Instead of using RLIMIT_FSIZE, use write_bytes or SEEK_CUR to
> 	enforce limit. This better integrates with the -F option for
> 	stdout output.

sorry for the delay, and thank you for the update and fix.

Note that the v2 patchset cannot be built except for x86_64, so I moved
the memparse() from sadump_info.c to tools.c.  Otherwise, looks good.

https://github.com/makedumpfile/makedumpfile/compare/9df519d...f0cfa86

Thanks,
Kazu


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 makedumpfile 0/3] Add option to limit file size

[Benjamin Poirier]

On 2021-07-30 02:13 +0000, HAGIO KAZUHITO(萩尾 一仁) wrote:
> -----Original Message-----
> > See feature description in patch "Add -L option to limit output file
> > size"
> > 
> > v1:
> > 	http://lists.infradead.org/pipermail/kexec/2021-June/022728.html
> > v2:
> > 	Add patches 1, 2 to support dmesg limit with different kernel
> > 	versions.
> > 
> > 	Stricter parsing of -L option value.
> > 
> > 	Instead of using RLIMIT_FSIZE, use write_bytes or SEEK_CUR to
> > 	enforce limit. This better integrates with the -F option for
> > 	stdout output.
> 
> sorry for the delay, and thank you for the update and fix.
> 
> Note that the v2 patchset cannot be built except for x86_64, so I moved
> the memparse() from sadump_info.c to tools.c.  Otherwise, looks good.
> 
> https://github.com/makedumpfile/makedumpfile/compare/9df519d...f0cfa86
> 

Thank you for checking and correcting that.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec