Re: [master&rhel6-branch 2/3] Fix readNetInfo and writeEnabledNetInfo for s390 (#595388)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

These changes look fine from a technical standpoint, but in an effort to keep
git commits to a single functional change, could you break these changes in to
multiple commits?  Chris already requested this, but I'm requesting it as
well.  Even though they are for the same BZ, the actual changes included here
are for different issues.  The BZ filed is really a catch-all BZ, which is
fine, but we would prefer separate git commits.  Make sure you reference the
BZ number in each patch per the docs/commit-log.txt file.

Also, could you stop referencing many git commits via their full hash?  It
makes the commit log message difficult to read.

On Mon, 31 May 2010, Steffen Maier wrote:

Before f17d989bc2cb50ffaedcf97a332095180511a272, writeEnabledNetInfo
did not seem to be called on s390. Now that it is, it turned out that
readNetInfo and writeEnabledNetInfo are not complete.

Loader does not need to handle or understand layer2 and portno so
consolidate that into opaque options. This will prevent issues such as
ef100b6baa5324fb5f5060b21f804bddc16ebead or
9caaca40bcaffb502dc58659e828f97d78a2d0f8
and is also transparent to future extensions in linuxrc.

This change should be a single commit.  Referencing
ef100b6baa5324fb5f5060b21f804bddc16ebead and
9caaca40bcaffb502dc58659e828f97d78a2d0f8 is less desirable than referencing
the other bugs in a "Related: rhbz#XXXXXX" format.  Also, it's best to not
make promises we can't guarantee ("...and is also transparent to future
extensions in linuxrc").

Really ignore parm/conf file options that are deprecated,
otherwise fixed loader would leave them in the ifcfg file.

I don't know what change this sentence refers to in the patch.

Tell which stacks to configure in /etc/sysconfig/network

This should be a single commit.

GATEWAY in ifcfg is really IPv4 only.

This should be a single commit.

Please fixed loader to parse DNS and write DNS1,DNS2,... itself (again).

This should be a single commit.

Correctly parse OPTIONS whose value includes equal signs because
the values are attribute value pairs. See also bug 597205 and
8549a36d4b22171992951a272b82f0aa14234dc4.

My guess is this change should be part of the commit above to fix the handling
of layer2 and portno, but I could be wrong.

Do parse DOMAIN for DNS search suffixes which are very hand on s390
where it's sometimes hard to write more than 80 chars for
repo/stage2/updates etc. Also write out quoted since it is a whitespace
separated list.

This should be a single commit.

Don't write HWADDR on s390. See also 3411cda88be7ec7fe57ec5c9c73d15148354a414
and 2c1aab43a1010ec0e113fa86e4daea9cd99de7f6 (bug 546005) or bug 591533.

This should be a single commit.

---
isys/iface.c        |    3 +--
isys/iface.h        |    3 +--
loader/linuxrc.s390 |   17 ++++++++++++++++-
loader/loader.c     |   15 ++++++++-------
loader/loader.h     |    2 +-
loader/net.c        |   45 +++++++++++++--------------------------------
6 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/isys/iface.c b/isys/iface.c
index 0369104..afdc59f 100644
--- a/isys/iface.c
+++ b/isys/iface.c
@@ -383,8 +383,7 @@ void iface_init_iface_t(iface_t *iface) {
    iface->peerid = NULL;
    iface->nettype = NULL;
    iface->ctcprot = NULL;
-    iface->layer2 = NULL;
-    iface->portno = NULL;
+    iface->options = NULL;
    iface->flags = 0;
    iface->ipv4method = IPV4_UNUSED_METHOD;
    iface->ipv6method = IPV6_UNUSED_METHOD;
diff --git a/isys/iface.h b/isys/iface.h
index d7ecc56..b7eaa6f 100644
--- a/isys/iface.h
+++ b/isys/iface.h
@@ -95,8 +95,7 @@ typedef struct _iface_t {
    char *peerid;
    char *nettype;
    char *ctcprot;
-    char *layer2;
-    char *portno;
+    char *options;

    /* flags */
    uint64_t flags;
diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
index b421062..3716979 100644
--- a/loader/linuxrc.s390
+++ b/loader/linuxrc.s390
@@ -1992,6 +1992,7 @@ function do_network() {
    echo $"The NETWORK parameter isn't used anymore and will be ignored."
    echo $" It is sufficient to specify IPADDR and NETMASK."
    echo
+    unset NETWORK
}

### BROADCAST
@@ -2001,6 +2002,7 @@ function do_broadcast() {
    echo $"The BROADCAST parameter isn't used anymore and will be ignored."
    echo $" It is sufficient to specify IPADDR and NETMASK."
    echo
+    unset BROADCAST
}

### NETMASK (IPv6)
@@ -2994,12 +2996,16 @@ fi
cat > /etc/sysconfig/network << EOF
HOSTNAME=$HOSTNAME
EOF
+if [ "$ipv6" ]; then
+    echo "NETWORKING_IPV6=yes" >> /etc/sysconfig/network
+else
+    echo "NETWORKING=yes" >> /etc/sysconfig/network
+fi

cat > $IFCFGFILE << EOF
DEVICE=$DEVICE
ONBOOT=yes
BOOTPROTO=static
-GATEWAY=$GATEWAY
BROADCAST=$BROADCAST
MTU=$MTU
SUBCHANNELS=$SUBCHANNELS
@@ -3017,10 +3023,19 @@ else
    cat >> $IFCFGFILE << EOF
IPADDR=$IPADDR
NETMASK=$NETMASK
+GATEWAY=$GATEWAY
EOF
fi
+# real DNS config for NetworkManager to generate /etc/resolv.conf
[ "$DNS1" != "" ] && echo "DNS1=$DNS1" >> $IFCFGFILE
[ "$DNS2" != "" ] && echo "DNS2=$DNS2" >> $IFCFGFILE
+# just to please loader's readNetInfo && writeEnabledNetInfo
+# which eats DNS1,DNS2,... and generates it themselves based on DNS
+if [ "$ipv6" ]; then
+    [ "$DNS" != "" ] && echo "DNS=\"$DNS\"" >> $IFCFGFILE
+else
+    [ "$DNS" != "" ] && echo "DNS=\"$(echo $DNS|sed -e 's/:/,/g')\"" >> $IFCFGFILE
+fi
# colons in SEARCHDNS already replaced with spaces above for /etc/resolv.conf
[ "$SEARCHDNS" != "" ] && echo "DOMAIN=\"$SEARCHDNS\"" >> $IFCFGFILE
[ "$NETTYPE" != "" ] && echo "NETTYPE=$NETTYPE" >> $IFCFGFILE
diff --git a/loader/loader.c b/loader/loader.c
index 5f49cc0..049c26f 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -686,8 +686,7 @@ static void readNetInfo(struct loaderData_s ** ld) {
    loaderData->portname = NULL;
    loaderData->nettype = NULL;
    loaderData->ctcprot = NULL;
-    loaderData->layer2 = NULL;
-    loaderData->portno = NULL;
+    loaderData->options = NULL;
    loaderData->macaddr = NULL;
#ifdef ENABLE_IPV6
    loaderData->ipv6 = NULL;
@@ -722,7 +721,7 @@ static void readNetInfo(struct loaderData_s ** ld) {
        }

        tmp = g_strstrip(tmp);
-        pair = g_strsplit(tmp, "=", 0);
+        pair = g_strsplit(tmp, "=", 2);

        if (g_strv_length(pair) == 2) {
            gchar *val = g_shell_unquote(pair[1], &e);
@@ -735,12 +734,16 @@ 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")) {
                    loaderData->gateway = strdup(val);
                } else if (!g_strcmp0(pair[0], "DNS")) {
                    loaderData->dns = strdup(val);
+                } else if (!g_strcmp0(pair[0], "DOMAIN")) {
+                    loaderData->domain = strdup(val);
                } else if (!g_strcmp0(pair[0], "MTU")) {
                    errno = 0;
                    loaderData->mtu = strtol(val, NULL, 10);
@@ -761,10 +764,8 @@ static void readNetInfo(struct loaderData_s ** ld) {
                    loaderData->nettype = strdup(val);
                } else if (!g_strcmp0(pair[0], "CTCPROT")) {
                    loaderData->ctcprot = strdup(val);
-                } else if (!g_strcmp0(pair[0], "LAYER2")) {
-                    loaderData->layer2 = strdup(val);
-                } else if (!g_strcmp0(pair[0], "PORTNO")) {
-                    loaderData->portno = strdup(val);
+                } else if (!g_strcmp0(pair[0], "OPTIONS")) {
+                    loaderData->options = strdup(val);
                } else if (!g_strcmp0(pair[0], "MACADDR")) {
                    loaderData->macaddr = strdup(val);
                } else if (!g_strcmp0(pair[0], "HOSTNAME")) {
diff --git a/loader/loader.h b/loader/loader.h
index c64c475..f35e3ff 100644
--- a/loader/loader.h
+++ b/loader/loader.h
@@ -132,7 +132,7 @@ struct loaderData_s {
    int bootIf_set;
    char * netCls;
    int netCls_set;
-    char *ipv4, *netmask, *gateway, *dns, *hostname, *peerid, *ethtool, *subchannels, *portname, *essid, *wepkey, *nettype, *ctcprot, *layer2, *portno, *macaddr;
+    char *ipv4, *netmask, *gateway, *dns, *domain, *hostname, *peerid, *ethtool, *subchannels, *portname, *essid, *wepkey, *nettype, *ctcprot, *options, *macaddr;
#ifdef ENABLE_IPV6
    char *ipv6;
    int ipv6info_set;
diff --git a/loader/net.c b/loader/net.c
index 5a832de..769c1d4 100644
--- a/loader/net.c
+++ b/loader/net.c
@@ -366,6 +366,11 @@ void setupIfaceStruct(iface_t * iface, struct loaderData_s * loaderData) {
        logMessage(INFO, "dnsservers is %s", loaderData->dns);
    }

+    if (loaderData->domain) {
+        logMessage(INFO, "dnsdomains is %s", loaderData->domain);
+        iface->domain = strdup(loaderData->domain);
+    }
+
    if (loaderData->hostname) {
        logMessage(INFO, "setting specified hostname of %s",
                   loaderData->hostname);
@@ -400,12 +405,8 @@ void setupIfaceStruct(iface_t * iface, struct loaderData_s * loaderData) {
        parseEthtoolSettings(loaderData);
    }

-    if (loaderData->layer2) {
-        iface->layer2 = strdup(loaderData->layer2);
-    }
-
-    if (loaderData->portno) {
-        iface->portno = strdup(loaderData->portno);
+    if (loaderData->options) {
+        iface->options = strdup(loaderData->options);
    }

    if (loaderData->wepkey) {
@@ -1218,7 +1219,7 @@ int writeDisabledNetInfo(void) {
 *     /etc/sysconfig/network
 */
int writeEnabledNetInfo(iface_t *iface) {
-    int i = 0, osa_layer2 = 0, osa_portno = 0;
+    int i = 0;
    mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
    FILE *fp = NULL;
    char buf[INET6_ADDRSTRLEN+1];
@@ -1283,7 +1284,9 @@ int writeEnabledNetInfo(iface_t *iface) {
    }

    fprintf(fp, "DEVICE=%s\n", iface->device);
+#if !defined(__s390__) && !defined(__s390x__)
    fprintf(fp, "HWADDR=%s\n", iface_mac2str(iface->device));
+#endif
    fprintf(fp, "ONBOOT=yes\n");

    if (!FL_NOIPV4(flags)) {
@@ -1401,7 +1404,7 @@ int writeEnabledNetInfo(iface_t *iface) {
    }

    if (iface->domain) {
-        fprintf(fp, "DOMAIN=%s\n", iface->domain);
+        fprintf(fp, "DOMAIN=\"%s\"\n", iface->domain);
    }

    if (iface->mtu) {
@@ -1428,30 +1431,8 @@ 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) {
-        fprintf(fp, "OPTIONS=\"");
-
-        if (osa_layer2) {
-            fprintf(fp, "layer2=1");
-        }
-
-        if (osa_layer2 && osa_portno) {
-            fprintf(fp, " ");
-        }
-
-        if (osa_portno) {
-            fprintf(fp, "portno=1");
-        }
-
-        fprintf(fp, "\"\n");
+    if (iface->options) {
+        fprintf(fp, "OPTIONS=\'%s\'\n", iface->options);
    }

    if (iface->macaddr) {


- -- David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAkwGcaoACgkQ5hsjjIy1VkmuYQCfe6NPZHCIkfHyC2AEWty51h6q
id4AoNwEZces1xMcFCzIP0mikOWIHouA
=ktUM
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list


[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux