Right, this was just a first unfinished request for comments,
which was replaced with the other clean patches you applied.
On 06/11/2010 10:36 PM, David Cantrell wrote:
> You can probably disregard the previous message as I see these changes made
> their way in to other patches.
>
> On Fri, 11 Jun 2010, David Cantrell wrote:
>
>> 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.
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
