-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Wed, 5 May 2010, Steffen Maier wrote:
Sorry, but you screwed it up completely.
There seem to be some inconsistences with your review vs. what's in the
Bugzilla comments. Please follow along.
On 05/04/2010 11:40 PM, David Cantrell wrote:
The geninitrdsz.c program was originally provided by IBM. An old bug
number is referenced in the source. Apparently this program is not
generating a useful file any more, so IBM has reworked it and it now
generates a load address patch file that can be used when booting an
LPAR directly.
geninitrdsz would still generate a working patch file for the initrd
size. However, we now also need to patch the initrd load address.
So IBM provides a new tool geninitrdpatch, which generates a patch
for *both load address and size of initrd* and thus happens to replace
geninitrdsz.
See below for more issues.
---
utils/geninitrdsz.c | 51 +++++++++++++++++++++++++++++++++++----------------
1 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/utils/geninitrdsz.c b/utils/geninitrdsz.c
index 6dfd976..b8c824a 100644
--- a/utils/geninitrdsz.c
+++ b/utils/geninitrdsz.c
@@ -1,11 +1,12 @@
/*
- * geninitrdsz.c
- * Generate initrd.size file for zSeries platforms.
+ * gen-initrd-addr.c
* gen-initrd-patch.c
+ * Generate initrd.addr file for s390x platforms.
* Generate initrd.patch file for s390x platforms.
Does it really matter if it's called initrd.addr or initrd.patch? Or even
initrd.size for that matter? I used initrd.addr thinking that it might be a
good idea for people to not mistake this file for the output of diff(1).
* Takes an integer argument and writes out the binary representation of
- * that value to the initrd.size file.
+ * that value to the initrd.addr file.
* Takes an integer argument as initrd load address,
* a filename of an initrd to determine its size, and
* a filename of the output patch file.
* Writes out the binary representation of load address and size
* as two 64 bit big endian integers.
* https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197773
+ * https://bugzilla.redhat.com/show_bug.cgi?id=546422
*
- * Copyright (C) 2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2007-2010 Red Hat, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -33,27 +34,45 @@
#include <string.h>
int main(int argc,char **argv) {
- unsigned int zero = 0;
- int fd;
- unsigned int size;
- mode_t mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH;
+ struct stat initrd_stat;
+ unsigned int addr = 0, size = 0, zero = 0;
+ int fd, rc;
+ char *tmp = NULL;
if (argc != 3) {
if (argc != 4) {
The create_initrd_patch.c file attached to comment #31 says:
if (argc != 3) {
- printf("Usage: %s [integer size] [output file]\n", basename(argv[0]));
- printf("Example: %s 12288475 initrd.size\n", basename(argv[0]));
- return 0;
+ printf("Generate initrd.addr file used by the .ins LPAR load mechanism\n");
+ printf("Usage: %s [address] [output file]\n", basename(argv[0]));
+ printf("Example: %s 0x2000000 initrd.size\n", basename(argv[0]));
printf("Generate initrd.patch file used by the .ins LPAR load
mechanism\n");
printf("Usage: %s [load address] [input initrd file] [output
patch file]\n", basename(argv[0]));
printf("Example: %s 0x2000000 initrd.img initrd.patch\n",
basename(argv[0]));
Fine, but this is what was in create_initrd_patch.c from comment #31:
printf("\n\nUtility which creates an initrd patch file \n");
printf("to be used for the .ins LPAR load mechanism.\n\n");
printf("Usage: %s <addr> <file>\n\n", argv[0]);
printf(" addr\twhere to load initrd\n");
printf(" file\tfilename of initrd\n");
+ return EXIT_SUCCESS;
I know our tool had this with zero, but how can calling the tool
incorrectly exit with a successful error level?
Because it's just printing the usage information?
}
- size = htonl(atoi(argv[1]));
- fd = open(argv[2], O_CREAT | O_RDWR, mode);
+ rc = stat(argv[2], &initrd_stat);
+ if (rc) {
+ perror("Error getting initrd stats ");
+ return rc;
return 1;
+ }
+
+ addr = htonl(strtoul(argv[1], &tmp, 0));
+ size = initrd_stat.st_size;
size = htonl(initrd_stat.st_size);
https://bugzilla.redhat.com/show_bug.cgi?id=546422#c31
says there is a htonl missing since all those values must be big endian
and a new tool source had been attached.
Legitimate problem. I neglected to copy over that line.
+ fd = open(argv[2], O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
fd = open(argv[3], O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
In order to be able to specify all those funny absolute paths on
mk-images, we want to write to the outfile in the 3rd command line
argument and not overwrite our precious input file initrd.img.
That's fine, but how about getting it correct in the version of
create_initrd_patch.c that you attach to the BZ?
+
+ if (write(fd, &zero, sizeof(int)) == -1) {
+ perror("writing initrd.addr (zero) ");
+ return errno;
perror("writing output patch file (first zero) ");
return 2;
+ }
+
+ if (write(fd, &addr, sizeof(int)) == -1) {
+ perror("writing initrd.addr (zero) ");
+ return errno;
perror("writing output patch file (address) ");
return 3;
+ }
if (write(fd, &zero, sizeof(int)) == -1) {
- perror("writing initrd.size (zero)");
+ perror("writing initrd.addr (zero) ");
return errno;
perror("writing output patch file (second zero) ");
return 4;
}
- if (write(fd, &size, sizeof(int)) == -1) {
- perror("writing initrd.size (size)");
+ if (write(fd, &addr, sizeof(int)) == -1) {
+ perror("writing initrd.addr (zero) ");
return errno;
if (write(fd, &size, sizeof(int)) == -1) {
perror("writing output patch file (size) ");
return 5;
This really must be the initrd size as in our tool source and not again
the address.
}
The return code changes, fine, but again, those were not in
create_initrd_patch.c. In fact, the version attached to comment #31 did not
check the return value of write().
It appears the version of create_initrd_patch.c you know and love is not the
same as what was attached to the BZ. There were minimal changes between
geninitrdsz.c and create_initrd_patch.c, which is why I merged in the changes
and renamed the file in anaconda's source as opposed to just taking
create_initrd_patch.c as-is.
Before running at me with pitchforks in the future, could you make sure the
story in the BZ matches what you're going by? Better yet, providing a patch
would greatly simplify and speed up this entire process.
- --
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAkvgwR4ACgkQ5hsjjIy1VkmRQACfUynZXmrmg+h/NVHGXgm8iE37
09sAniXIK4ymG3XhcCHkOfWsdxZ5O3MG
=6/63
-----END PGP SIGNATURE-----
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
