[PATCH 0/3] s390: add support for extended cmdline length

[PATCH 0/3] s390: add support for extended cmdline length

[Sven Schnelle]

Hi,

this patchset adds support for extended command line lengths on s390.
The former limit of 896 has been too short in a some configurations. The
linux kernel now provides an additional field in the parameter area
which contains the maximum allowed command line length. In older kernels
this field is zero. In that case the old limit of 896 bytes is used.
This was introduced with 5ecb2da660ab ("s390: support command lines
longer than 896 bytes") in linux.

And while at it, also add the --reuse-cmdline option and use
KEXEC_ALL_OPTIONS.

Sven Schnelle (3):
  s390: add variable command line size
  s390: use KEXEC_ALL_OPTIONS
  s390: add support for --reuse-cmdline

 kexec/arch/s390/include/arch/options.h | 10 ++--
 kexec/arch/s390/kexec-image.c          | 72 +++++++++++++++++---------
 kexec/arch/s390/kexec-s390.h           | 19 +++----
 3 files changed, 63 insertions(+), 38 deletions(-)

-- 
2.25.1


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

[PATCH 1/3] s390: add variable command line size

[Sven Schnelle]

Newer s390 kernels support a command line size longer than 896
bytes. Such kernels contain a new member in the parameter area,
which might be utilized by tools like kexec. Older kernels have
the location initialized to zero, so we check whether there's a
non-zero number present and use that. If there isn't, we fallback
to the legacy command line size of 896 bytes.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/kexec-image.c | 53 ++++++++++++++++++++++++-----------
 kexec/arch/s390/kexec-s390.h  | 19 +++++++------
 2 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index 3c24fdfe3c7c..7747d02399db 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -25,7 +25,7 @@
 #include <fcntl.h>
 
 static uint64_t crash_base, crash_end;
-static char command_line[COMMAND_LINESIZE];
+static char *command_line;
 
 static void add_segment_check(struct kexec_info *info, const void *buf,
 			      size_t bufsz, unsigned long base, size_t memsz)
@@ -38,11 +38,16 @@ static void add_segment_check(struct kexec_info *info, const void *buf,
 
 int command_line_add(const char *str)
 {
-	if (strlen(command_line) + strlen(str) + 1 > COMMAND_LINESIZE) {
-		fprintf(stderr, "Command line too long.\n");
+	char *tmp = NULL;
+
+	tmp = concat_cmdline(command_line, str);
+	if (!tmp) {
+		fprintf(stderr, "out of memory\n");
 		return -1;
 	}
-	strcat(command_line, str);
+
+	free(command_line);
+	command_line = tmp;
 	return 0;
 }
 
@@ -78,6 +83,8 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 		if (info->initrd_fd == -1) {
 			fprintf(stderr, "Could not open initrd file %s:%s\n",
 					ramdisk, strerror(errno));
+			free(command_line);
+			command_line = NULL;
 			return -1;
 		}
 	}
@@ -97,7 +104,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	const char *ramdisk;
 	off_t ramdisk_len;
 	unsigned int ramdisk_origin;
-	int opt;
+	int opt, ret = -1;
 
 	if (info->file_mode)
 		return image_s390_load_file(argc, argv, info);
@@ -112,7 +119,6 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
 
-	command_line[0] = 0;
 	ramdisk = NULL;
 	ramdisk_len = 0;
 	ramdisk_origin = 0;
@@ -132,7 +138,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (parse_iomem_single("Crash kernel\n", &crash_base,
 				       &crash_end))
-			return -1;
+			goto out;
 	}
 
 	/* Add kernel segment */
@@ -151,7 +157,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 		rd_buffer = slurp_file_mmap(ramdisk, &ramdisk_len);
 		if (rd_buffer == NULL) {
 			fprintf(stderr, "Could not read ramdisk.\n");
-			return -1;
+			goto out;
 		}
 		ramdisk_origin = MAX(RAMDISK_ORIGIN_ADDR, kernel_size);
 		ramdisk_origin = _ALIGN_UP(ramdisk_origin, 0x100000);
@@ -160,7 +166,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	}
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (load_crashdump_segments(info, crash_base, crash_end))
-			return -1;
+			goto out;
 	} else {
 		info->entry = (void *) IMAGE_READ_OFFSET;
 	}
@@ -183,15 +189,28 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 			*tmp = crash_end - crash_base + 1;
 		}
 	}
-	/*
-	 * We will write a probably given command line.
-	 * First, erase the old area, then setup the new parameters:
-	 */
-	if (strlen(command_line) != 0) {
-		memset(krnl_buffer + COMMAND_LINE_OFFS, 0, COMMAND_LINESIZE);
-		memcpy(krnl_buffer + COMMAND_LINE_OFFS, command_line, strlen(command_line));
+
+	if (command_line) {
+		unsigned long maxsize;
+		char *dest = krnl_buffer + COMMAND_LINE_OFFS;
+
+		maxsize = *(unsigned long *)(krnl_buffer + MAX_COMMAND_LINESIZE_OFFS);
+		if (!maxsize)
+			maxsize = LEGACY_COMMAND_LINESIZE;
+
+		if (strlen(command_line) > maxsize-1) {
+			fprintf(stderr, "command line too long, maximum allowed size %ld\n",
+				maxsize-1);
+			goto out;
+		}
+		strncpy(dest, command_line, maxsize-1);
+		dest[maxsize-1] = '\0';
 	}
-	return 0;
+	ret = 0;
+out:
+	free(command_line);
+	command_line = NULL;
+	return ret;
 }
 
 int 
diff --git a/kexec/arch/s390/kexec-s390.h b/kexec/arch/s390/kexec-s390.h
index ef53b111e167..1b0e04848460 100644
--- a/kexec/arch/s390/kexec-s390.h
+++ b/kexec/arch/s390/kexec-s390.h
@@ -10,16 +10,17 @@
 #ifndef KEXEC_S390_H
 #define KEXEC_S390_H
 
-#define IMAGE_READ_OFFSET     0x10000
+#define IMAGE_READ_OFFSET           0x10000
 
-#define RAMDISK_ORIGIN_ADDR   0x800000
-#define INITRD_START_OFFS     0x408
-#define INITRD_SIZE_OFFS      0x410
-#define OLDMEM_BASE_OFFS      0x418
-#define OLDMEM_SIZE_OFFS      0x420
-#define COMMAND_LINE_OFFS     0x480
-#define COMMAND_LINESIZE      896
-#define MAX_MEMORY_RANGES     1024
+#define RAMDISK_ORIGIN_ADDR         0x800000
+#define INITRD_START_OFFS           0x408
+#define INITRD_SIZE_OFFS            0x410
+#define OLDMEM_BASE_OFFS            0x418
+#define OLDMEM_SIZE_OFFS            0x420
+#define MAX_COMMAND_LINESIZE_OFFS   0x430
+#define COMMAND_LINE_OFFS           0x480
+#define LEGACY_COMMAND_LINESIZE     896
+#define MAX_MEMORY_RANGES           1024
 
 #define MAX(x, y) ((x) > (y) ? (x) : (y))
 #define MIN(x, y) ((x) < (y) ? (x) : (y))
-- 
2.25.1


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

[PATCH 2/3] s390: use KEXEC_ALL_OPTIONS

[Sven Schnelle]

KEXEC_ALL_OPTIONS could be used instead defining the same
array several times. This makes code easier to maintain when
new options are added.

Suggested-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/kexec-image.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index 7747d02399db..dbeb689b830a 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -58,10 +58,7 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 
 	static const struct option options[] =
 		{
-			KEXEC_OPTIONS
-			{"command-line",     1, 0, OPT_APPEND},
-			{"append",           1, 0, OPT_APPEND},
-			{"initrd",           1, 0, OPT_RAMDISK},
+			KEXEC_ALL_OPTIONS
 			{0,                  0, 0, 0},
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
@@ -111,10 +108,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 
 	static const struct option options[] =
 		{
-			KEXEC_OPTIONS
-			{"command-line",     1, 0, OPT_APPEND},
-			{"append",           1, 0, OPT_APPEND},
-			{"initrd",           1, 0, OPT_RAMDISK},
+			KEXEC_ALL_OPTIONS
 			{0,                  0, 0, 0},
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
-- 
2.25.1


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

[PATCH 3/3] s390: add support for --reuse-cmdline

[Sven Schnelle]

--reuse-cmdline reads the command line of the currently
running kernel from /proc/cmdline and uses that for the
kernel that should be kexec'd.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/include/arch/options.h | 10 ++++++----
 kexec/arch/s390/kexec-image.c          |  9 +++++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/s390/include/arch/options.h b/kexec/arch/s390/include/arch/options.h
index 76044a301ceb..b030b61d61be 100644
--- a/kexec/arch/s390/include/arch/options.h
+++ b/kexec/arch/s390/include/arch/options.h
@@ -1,9 +1,10 @@
 #ifndef KEXEC_ARCH_S390_OPTIONS_H
 #define KEXEC_ARCH_S390_OPTIONS_H
 
-#define OPT_ARCH_MAX	(OPT_MAX+0)
-#define OPT_APPEND	OPT_MAX+0
-#define OPT_RAMDISK	OPT_MAX+1
+#define OPT_ARCH_MAX		(OPT_MAX+0)
+#define OPT_APPEND		(OPT_MAX+0)
+#define OPT_RAMDISK		(OPT_MAX+1)
+#define OPT_REUSE_CMDLINE	(OPT_MAX+2)
 
 /* Options relevant to the architecture (excluding loader-specific ones),
  * in this case none:
@@ -31,7 +32,8 @@
 	KEXEC_ARCH_OPTIONS				\
 	{"command-line",     1, 0, OPT_APPEND},		\
 	{"append",           1, 0, OPT_APPEND},		\
-	{"initrd",	     1, 0, OPT_RAMDISK},
+	{"initrd",	     1, 0, OPT_RAMDISK},	\
+	{"reuse-cmdline",    0, 0, OPT_REUSE_CMDLINE },
 
 #define KEXEC_ALL_OPT_STR KEXEC_ARCH_OPT_STR
 
diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index dbeb689b830a..310d967ea331 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -72,6 +72,10 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 		case OPT_RAMDISK:
 			ramdisk = optarg;
 			break;
+		case OPT_REUSE_CMDLINE:
+			free(command_line);
+			command_line = get_command_line();
+			break;
 		}
 	}
 
@@ -123,6 +127,10 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 			if (command_line_add(optarg))
 				return -1;
 			break;
+		case OPT_REUSE_CMDLINE:
+			free(command_line);
+			command_line = get_command_line();
+			break;
 		case OPT_RAMDISK:
 			ramdisk = optarg;
 			break;
@@ -223,5 +231,6 @@ image_s390_usage(void)
 	printf("--command-line=STRING Set the kernel command line to STRING.\n"
 	       "--append=STRING       Set the kernel command line to STRING.\n"
 	       "--initrd=FILENAME     Use the file FILENAME as a ramdisk.\n"
+	       "--reuse-cmdline       Use kernel command line from running system.\n"
 		);
 }
-- 
2.25.1


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

Re: [PATCH 1/3] s390: add variable command line size

[Philipp Rudo]

Hi Sven,

On Mon, 22 Nov 2021 08:13:59 +0100
Sven Schnelle <svens@linux.ibm.com> wrote:

> Newer s390 kernels support a command line size longer than 896
> bytes. Such kernels contain a new member in the parameter area,
> which might be utilized by tools like kexec. Older kernels have
> the location initialized to zero, so we check whether there's a
> non-zero number present and use that. If there isn't, we fallback
> to the legacy command line size of 896 bytes.
> 
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> ---
>  kexec/arch/s390/kexec-image.c | 53 ++++++++++++++++++++++++-----------
>  kexec/arch/s390/kexec-s390.h  | 19 +++++++------
>  2 files changed, 46 insertions(+), 26 deletions(-)
> 
> diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
> index 3c24fdfe3c7c..7747d02399db 100644
> --- a/kexec/arch/s390/kexec-image.c
> +++ b/kexec/arch/s390/kexec-image.c
> @@ -25,7 +25,7 @@
>  #include <fcntl.h>
>  
>  static uint64_t crash_base, crash_end;
> -static char command_line[COMMAND_LINESIZE];
> +static char *command_line;

isn't this the perfect opportunity to get rid of this global variable
and...

>  static void add_segment_check(struct kexec_info *info, const void *buf,
>  			      size_t bufsz, unsigned long base, size_t memsz)
> @@ -38,11 +38,16 @@ static void add_segment_check(struct kexec_info *info, const void *buf,
>  
>  int command_line_add(const char *str)

... simply pass the pointer as an argument ;)

Thanks
Philipp

>  {
> -	if (strlen(command_line) + strlen(str) + 1 > COMMAND_LINESIZE) {
> -		fprintf(stderr, "Command line too long.\n");
> +	char *tmp = NULL;
> +
> +	tmp = concat_cmdline(command_line, str);
> +	if (!tmp) {
> +		fprintf(stderr, "out of memory\n");
>  		return -1;
>  	}
> -	strcat(command_line, str);
> +
> +	free(command_line);
> +	command_line = tmp;
>  	return 0;
>  }
>  
> @@ -78,6 +83,8 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
>  		if (info->initrd_fd == -1) {
>  			fprintf(stderr, "Could not open initrd file %s:%s\n",
>  					ramdisk, strerror(errno));
> +			free(command_line);
> +			command_line = NULL;
>  			return -1;
>  		}
>  	}
> @@ -97,7 +104,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  	const char *ramdisk;
>  	off_t ramdisk_len;
>  	unsigned int ramdisk_origin;
> -	int opt;
> +	int opt, ret = -1;
>  
>  	if (info->file_mode)
>  		return image_s390_load_file(argc, argv, info);
> @@ -112,7 +119,6 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  		};
>  	static const char short_options[] = KEXEC_OPT_STR "";
>  
> -	command_line[0] = 0;
>  	ramdisk = NULL;
>  	ramdisk_len = 0;
>  	ramdisk_origin = 0;
> @@ -132,7 +138,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  	if (info->kexec_flags & KEXEC_ON_CRASH) {
>  		if (parse_iomem_single("Crash kernel\n", &crash_base,
>  				       &crash_end))
> -			return -1;
> +			goto out;
>  	}
>  
>  	/* Add kernel segment */
> @@ -151,7 +157,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  		rd_buffer = slurp_file_mmap(ramdisk, &ramdisk_len);
>  		if (rd_buffer == NULL) {
>  			fprintf(stderr, "Could not read ramdisk.\n");
> -			return -1;
> +			goto out;
>  		}
>  		ramdisk_origin = MAX(RAMDISK_ORIGIN_ADDR, kernel_size);
>  		ramdisk_origin = _ALIGN_UP(ramdisk_origin, 0x100000);
> @@ -160,7 +166,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  	}
>  	if (info->kexec_flags & KEXEC_ON_CRASH) {
>  		if (load_crashdump_segments(info, crash_base, crash_end))
> -			return -1;
> +			goto out;
>  	} else {
>  		info->entry = (void *) IMAGE_READ_OFFSET;
>  	}
> @@ -183,15 +189,28 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  			*tmp = crash_end - crash_base + 1;
>  		}
>  	}
> -	/*
> -	 * We will write a probably given command line.
> -	 * First, erase the old area, then setup the new parameters:
> -	 */
> -	if (strlen(command_line) != 0) {
> -		memset(krnl_buffer + COMMAND_LINE_OFFS, 0, COMMAND_LINESIZE);
> -		memcpy(krnl_buffer + COMMAND_LINE_OFFS, command_line, strlen(command_line));
> +
> +	if (command_line) {
> +		unsigned long maxsize;
> +		char *dest = krnl_buffer + COMMAND_LINE_OFFS;
> +
> +		maxsize = *(unsigned long *)(krnl_buffer + MAX_COMMAND_LINESIZE_OFFS);
> +		if (!maxsize)
> +			maxsize = LEGACY_COMMAND_LINESIZE;
> +
> +		if (strlen(command_line) > maxsize-1) {
> +			fprintf(stderr, "command line too long, maximum allowed size %ld\n",
> +				maxsize-1);
> +			goto out;
> +		}
> +		strncpy(dest, command_line, maxsize-1);
> +		dest[maxsize-1] = '\0';
>  	}
> -	return 0;
> +	ret = 0;
> +out:
> +	free(command_line);
> +	command_line = NULL;
> +	return ret;
>  }
>  
>  int 
> diff --git a/kexec/arch/s390/kexec-s390.h b/kexec/arch/s390/kexec-s390.h
> index ef53b111e167..1b0e04848460 100644
> --- a/kexec/arch/s390/kexec-s390.h
> +++ b/kexec/arch/s390/kexec-s390.h
> @@ -10,16 +10,17 @@
>  #ifndef KEXEC_S390_H
>  #define KEXEC_S390_H
>  
> -#define IMAGE_READ_OFFSET     0x10000
> +#define IMAGE_READ_OFFSET           0x10000
>  
> -#define RAMDISK_ORIGIN_ADDR   0x800000
> -#define INITRD_START_OFFS     0x408
> -#define INITRD_SIZE_OFFS      0x410
> -#define OLDMEM_BASE_OFFS      0x418
> -#define OLDMEM_SIZE_OFFS      0x420
> -#define COMMAND_LINE_OFFS     0x480
> -#define COMMAND_LINESIZE      896
> -#define MAX_MEMORY_RANGES     1024
> +#define RAMDISK_ORIGIN_ADDR         0x800000
> +#define INITRD_START_OFFS           0x408
> +#define INITRD_SIZE_OFFS            0x410
> +#define OLDMEM_BASE_OFFS            0x418
> +#define OLDMEM_SIZE_OFFS            0x420
> +#define MAX_COMMAND_LINESIZE_OFFS   0x430
> +#define COMMAND_LINE_OFFS           0x480
> +#define LEGACY_COMMAND_LINESIZE     896
> +#define MAX_MEMORY_RANGES           1024
>  
>  #define MAX(x, y) ((x) > (y) ? (x) : (y))
>  #define MIN(x, y) ((x) < (y) ? (x) : (y))


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

Re: [PATCH 3/3] s390: add support for --reuse-cmdline

[Philipp Rudo]

Hi Sven,

makes absolutely sense to have this option. One problem though...

On Mon, 22 Nov 2021 08:14:01 +0100
Sven Schnelle <svens@linux.ibm.com> wrote:

> --reuse-cmdline reads the command line of the currently
> running kernel from /proc/cmdline and uses that for the
> kernel that should be kexec'd.
> 
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> ---
>  kexec/arch/s390/include/arch/options.h | 10 ++++++----
>  kexec/arch/s390/kexec-image.c          |  9 +++++++++
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/kexec/arch/s390/include/arch/options.h b/kexec/arch/s390/include/arch/options.h
> index 76044a301ceb..b030b61d61be 100644
[...]
> diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
> index dbeb689b830a..310d967ea331 100644
> --- a/kexec/arch/s390/kexec-image.c
> +++ b/kexec/arch/s390/kexec-image.c
> @@ -72,6 +72,10 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
>  		case OPT_RAMDISK:
>  			ramdisk = optarg;
>  			break;
> +		case OPT_REUSE_CMDLINE:
> +			free(command_line);
> +			command_line = get_command_line();

get_command_line reads a maximum of 2048 bytes from /prc/cmdline. With
the configurable size on s390 defaulting to 4096 bytes this will
ultimately cause problems. So you need to make get_command_line more
flexible first.

Thanks
Philipp

> +			break;
>  		}
>  	}
>  
> @@ -123,6 +127,10 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
>  			if (command_line_add(optarg))
>  				return -1;
>  			break;
> +		case OPT_REUSE_CMDLINE:
> +			free(command_line);
> +			command_line = get_command_line();
> +			break;
>  		case OPT_RAMDISK:
>  			ramdisk = optarg;
>  			break;
> @@ -223,5 +231,6 @@ image_s390_usage(void)
>  	printf("--command-line=STRING Set the kernel command line to STRING.\n"
>  	       "--append=STRING       Set the kernel command line to STRING.\n"
>  	       "--initrd=FILENAME     Use the file FILENAME as a ramdisk.\n"
> +	       "--reuse-cmdline       Use kernel command line from running system.\n"
>  		);
>  }


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

Re: [PATCH 1/3] s390: add variable command line size

[Sven Schnelle]

Hi Philipp,

Philipp Rudo <prudo@redhat.com> writes:

>> diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
>> index 3c24fdfe3c7c..7747d02399db 100644
>> --- a/kexec/arch/s390/kexec-image.c
>> +++ b/kexec/arch/s390/kexec-image.c
>> @@ -25,7 +25,7 @@
>>  #include <fcntl.h>
>>  
>>  static uint64_t crash_base, crash_end;
>> -static char command_line[COMMAND_LINESIZE];
>> +static char *command_line;
>
> isn't this the perfect opportunity to get rid of this global variable
> and...
>
>>  static void add_segment_check(struct kexec_info *info, const void *buf,
>>  			      size_t bufsz, unsigned long base, size_t memsz)
>> @@ -38,11 +38,16 @@ static void add_segment_check(struct kexec_info *info, const void *buf,
>>  
>>  int command_line_add(const char *str)
>
> ... simply pass the pointer as an argument ;)

The reason for it being global is that command_line_add() might get
called multiple times. But yes, we could move that variable scope into
the calling function.

Thanks
Sven

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

Re: [PATCH 3/3] s390: add support for --reuse-cmdline

[Sven Schnelle]

Hi Philipp,

Philipp Rudo <prudo@redhat.com> writes:

>> diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
>> index dbeb689b830a..310d967ea331 100644
>> --- a/kexec/arch/s390/kexec-image.c
>> +++ b/kexec/arch/s390/kexec-image.c
>> @@ -72,6 +72,10 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
>>  		case OPT_RAMDISK:
>>  			ramdisk = optarg;
>>  			break;
>> +		case OPT_REUSE_CMDLINE:
>> +			free(command_line);
>> +			command_line = get_command_line();
>
> get_command_line reads a maximum of 2048 bytes from /prc/cmdline. With
> the configurable size on s390 defaulting to 4096 bytes this will
> ultimately cause problems. So you need to make get_command_line more
> flexible first.

Thanks for pointing this out, i wasn't aware of that limitation. So we
likely want to change that to some dynamic allocation.

Sven

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