Comments below.
On Fri, 28 May 2010, Steffen Maier wrote:
---
loader/linuxrc.s390 | 44 +-------------------------------------------
loader/loader.c | 16 ++++++++++++++++
loader/net.c | 21 +++++++--------------
3 files changed, 24 insertions(+), 57 deletions(-)
diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
index 99e5192..a5cedaf 100644
--- a/loader/linuxrc.s390
+++ b/loader/linuxrc.s390
@@ -2971,49 +2971,7 @@ done
# [ "$ipv6" ] && echo "IPV6=yes"
# transfer options into install environment
-cat > /tmp/install.cfg << EOF
-LANG="$LANG"
-S390ARCH="$S390ARCH"
-TEXTDOMAIN="$TEXTDOMAIN"
-TEXTDOMAINDIR="$TEXTDOMAINDIR"
-PORTNAME="$PORTNAME"
-HOSTNAME="$HOSTNAME"
-DEVICE="$DEVICE"
-NETTYPE="$NETTYPE"
-IPADDR="$IPADDR"
-GATEWAY="$GATEWAY"
-MTU="$MTU"
-NETWORK="$NETWORK"
-NETMASK="$NETMASK"
-BROADCAST="$BROADCAST"
-SEARCHDNS="$SEARCHDNS"
-PEERID="$PEERID"
-SUBCHANNELS="$SUBCHANNELS"
-ONBOOT="yes"
-CTCPROT="$CTCPROT"
-EOF
-if [ "$ipv6" ]; then
- DNS1=$(echo $DNS | cut -d ',' -f 1)
- echo DNS=\"$DNS1\" >> /tmp/install.cfg
- echo DNS1=\"$DNS1\" >> /tmp/install.cfg
- echo DNS2=\"$(echo $DNS | cut -d ',' -f 2)\" >> /tmp/install.cfg
-else
- DNS1=$(echo $DNS | cut -d ':' -f 1)
- echo DNS=\"$DNS1\" >> /tmp/install.cfg
- echo DNS1=\"$DNS1\" >> /tmp/install.cfg
- echo DNS2=\"$(echo $DNS | cut -d ':' -f 2)\" >> /tmp/install.cfg
-fi
-cat >> /tmp/install.cfg << EOF
-export LANG PORTNAME S390ARCH TEXTDOMAIN TEXTDOMAINDIR
-export HOSTNAME DEVICE NETTYPE IPADDR GATEWAY MTU
-export NETWORK NETMASK BROADCAST DNS DNS1 DNS2 SEARCHDNS
-export PEERID ONBOOT SUBCHANNELS CTCPROT
-EOF
-# immediately read it in again to export these into the shell below
-. /tmp/install.cfg
-if [ -z "$testing" ]; then
- cat /tmp/install.cfg >> /etc/profile
-fi # testing
+# loader now uses ifcfg instead of install.cfg to receive our network
config
NETSCRIPTS="/etc/sysconfig/network-scripts"
IFCFGFILE="$NETSCRIPTS/ifcfg-$DEVICE"
diff --git a/loader/loader.c b/loader/loader.c
index 63594d8..19149d6 100644
This hunk was already removed in 3c3b439ec81da495f4bd5a1a0ca0fc5c13ccd234.
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -692,6 +692,8 @@ static void readNetInfo(struct loaderData_s ** ld) {
} else {
if (!g_strcmp0(pair[0], "IPADDR")) {
loaderData->ipv4 = strdup(val);
+ loaderData->ipinfo_set = 1;
+ flags |= LOADER_FLAGS_IP_PARAM;
} else if (!g_strcmp0(pair[0], "NETMASK")) {
loaderData->netmask = strdup(val);
} else if (!g_strcmp0(pair[0], "GATEWAY")) {
@@ -718,15 +720,23 @@ static void readNetInfo(struct loaderData_s ** ld) {
loaderData->nettype = strdup(val);
} else if (!g_strcmp0(pair[0], "CTCPROT")) {
loaderData->ctcprot = strdup(val);
+/* FIXME: That's deprecated install.cfg syntax.
+ ifcfg syntax is OPTIONS="layer2=X portno=Y" and we need to parse this.
+ Optionally just take the value of OPTIONS as string without parsing
+ and get rid of the layer2 and portno fields of loaderData_S and
iface_t.
+ After all, OPTIONS can contain any qeth sysfs attribute not just layer2
or
+ portno.
} else if (!g_strcmp0(pair[0], "LAYER2")) {
loaderData->layer2 = strdup(val);
} else if (!g_strcmp0(pair[0], "PORTNO")) {
loaderData->portno = strdup(val);
+*/
Just remove these lines then.
} else if (!g_strcmp0(pair[0], "MACADDR")) {
loaderData->macaddr = strdup(val);
} else if (!g_strcmp0(pair[0], "HOSTNAME")) {
loaderData->hostname = strdup(val);
}
+ /* FIXME: IPv6 support still missing */
No FIXMEs, please add IPv6 support if it's necessary.
}
g_free(val);
@@ -737,9 +747,11 @@ static void readNetInfo(struct loaderData_s ** ld) {
line++;
}
+ /* bogus and not compatible with ipv6
if (loaderData->ipv4 && loaderData->netmask) {
flags |= LOADER_FLAGS_HAVE_CMSCONF;
}
+ */
If this hunk can be removed, remove it.
free(cfgfile);
g_strfreev(lines);
@@ -1470,6 +1482,10 @@ static char *doLoaderMain(struct loaderData_s
*loaderData,
logMessage(INFO, "going to do getNetConfig");
/* s390 provides all config info by way of the CMS conf
file */
+ /* This is broken. Info can also be given by parm file or
even
+ interactively without any cms conf file.
+ Also it could be ipv4, ipv6, or both, so we cannot
assume
+ it's always both just because of that bogus flag. */
Please fix it if broken rather than explaining it is broken.
if (FL_HAVE_CMSCONF(flags)) {
loaderData->ipinfo_set = 1;
#ifdef ENABLE_IPV6
diff --git a/loader/net.c b/loader/net.c
index dd27916..a302e02 100644
--- a/loader/net.c
+++ b/loader/net.c
@@ -1412,27 +1412,20 @@ int writeEnabledNetInfo(iface_t *iface) {
fprintf(fp, "CTCPROT=%s\n", iface->ctcprot);
}
- if (iface->layer2 && !strcmp(iface->layer2, "1")) {
- osa_layer2 = 1;
- }
-
- if (iface->portno && !strcmp(iface->portno, "1")) {
- osa_portno = 1;
- }
-
- if (osa_layer2 || osa_portno) {
+ /* don't (mis)understand layer2 and portno values, just pass them */
+ if (iface->layer2 || iface->portno) {
fprintf(fp, "OPTIONS=\"");
- if (osa_layer2) {
- fprintf(fp, "layer2=1");
+ if (iface->layer2) {
+ fprintf(fp, "layer2=%s", iface->layer2);
}
- if (osa_layer2 && osa_portno) {
+ if (iface->layer2 && iface->portno) {
fprintf(fp, " ");
}
- if (osa_portno) {
- fprintf(fp, "portno=1");
+ if (iface->portno) {
+ fprintf(fp, "portno=%s", iface->portno);
}
fprintf(fp, "\"\n");
This last changeset for loader/net.c seems like the only valid part of this
entire patch. Please update the patch if other parts are relevant. The
layer2 and portno handling in net.c showed up in
9caaca40bcaffb502dc58659e828f97d78a2d0f8, so please make sure we don't
regress
on that bug. It appears as though how we handle those values has changed,
which is fine.
