On Thu, 2009-01-01 at 01:27 +0100, Steffen Maier wrote: > On 12/07/2008 04:56 PM, Jeremy Katz wrote: > > On Dec 5, 2008, at 4:00 PM, David Cantrell wrote: > > So, some comments from trying to go through and at least look for > > obvious problems/things to fix. Some of these may only be rawhide > > applicable and given that the patch was against the RHEL5 codebase, they > > may not be (easily) doable there > > > > * A lot of checking for things like a 2.6 kernel. There's _always_ > > going to be at least a 2.6 kernel and such checks just needlessly > > complicate the script, making it that much harder to see what's really > > going on > > I suspect you mean the two places in linuxrc where we check for at least > Linux 2.6.26 which is when IPv6 support for HiperSockets in layer 3 mode > went upstream. In other words this is not about being a 2.6 kernel (we > already assume that anyway), it is about a certain sub-version. The > reason why I put this check in was to make my code base as independent > as possible from a specific RHEL release. In fact I put the checks in > hoping that (almost) no changes would be necessary for running under > RHEL6 as opposed to my development environment under RHEL5.2. But a kernel version doesn't tell you those things -- distros backport kernel changes all the time. You have to instead check based on functionality being provided rather than assuming anything off of the kernel version. > Lsznet raw was developed as a distro-independent tool which should work > on a multitude of releases. Therefore, we check for some basic > requirements. Does that make sense? It makes the code harder to maintain and if it's going to be in anaconda, then we really don't want those sorts of things. > > * Are udev and friends getting installed into the s390 initrd right now? > > Just a minimum base consisting of udevd and udevsettle just for calling > the latter as we potentially dynamically "add" (potentially lots of, > i.e. thousands of) devices by dynamically freeing them from the device > ignore list cio_ignore. Since this may take quite some time we do not > know upfront, we decided to use udevsettle instead of a bogus constant > sleep hoping all devices would have been sensed until we wake up and > continue and try to use them. There is really not more than udevd and > udevsettle, there are no config file (at least no content) and no rule > files. My point was that I don't think that the scripts which generate the s390 initramfs (mk-images.s390) are pulling in the udev bits at the moment. Which means this won't work as-is. > > * Some of the stuff in lsznet makes me think that modules should be > > getting auto-loaded by udev given the existence of modaliases and > > matching against things in /sys/bus/ccw > > Lsznet does not load any kernel modules at all. It relies on the caller > to setup the environment and does auto-sensing based on the given > environment. In case you were referring to the arrays related to CU > type/model: They are there for mapping the values in sysfs to > user-friendly strings that appear in the generated table of possible > network hardware configurations. Since they contain more information > than what is in modalias, alas, they cannot all be automatically > extracted from the kernel modules. E.g. cardtype, devname, and > groupchannels cannot be deduced automatically. This information *really* needs to be provided from the kernel. Otherwise, we're going to be carrying duplicates of it in anaconda. > With regard to linuxrc.s390, the way kernel modules are treated -- > namely explicitly loaded including dependencies -- has been taken from > the existing linxurc.s390 in order to introduce as few as possible > changes. It's already a complete rewrite to the point that diff is useless. So ditch the cruft. Reviewing in between the cruft being kept to try to "minimize" changes just makes life more painful > Also, trying to switch to auto-loading using udev would at > least for the network types LCS and CTC which are based on cu3088 not be > straight forward. The cu3088 module contains the modaliases but the > corresponding driver modules (lcs/ctc) need to be loaded as well, see, > e.g. RHIT 116546. Then the drivers need to be fixed. Period. As of rawhide, *ANYTHING* calling modprobe for something which would be considered a "driver" is a bug. And as we move towards an initramfs for the installed system which also depends on udev, etc it will become even more explicit. > > * Manual mknods shouldn't ever need to be present; udev should be > > creating device nodes > > I would fully agree, if we used a full-fledged udev environment. But as > stated above it is only there to wait for device sensing. Since this is going to rawhide, we should have a more fully-fledged udev environment. So the cruft needs to go. > I even start > udevd very late, when (according to a running udevmonitor in my test > environment) all udev events concerning non-s390-devices have already > vanished. As for now, this code is just what has been in linuxrc.s390 > before we touched it. Introducing full udev just for linuxrc.s390 just > did not seem worth it. What do you think? rawhide has udev everywhere other than s390 right now I think ;-) > > * modprobe instead of insmod if modules have to be manually loaded so > > that at least dependent modules can be auto-loaded. This also lets us > > kill the $LO hack > > I don't know why this $LO thing is there, I just kept it from the > existing linuxrc.s390. In fact, insmod and friends come from busybox > which is why it finds, extracts, and loads the modules even though they > reside in a modules.cgz. Since I did not understand the moddep magic of > the busybox kernel module tools, I did not touch the explicit loading of > dependencies. (And again there would be the LCS/CTC speciality mentioned > above.) $LO comes from the switch from .o files to .ko files with kernel modules going from 2.4 -> 2.6. It's mindless noise that should really be dropped. Also, we really don't want to use busybox insmod, etc at this point -- we should just use the pieces from module-init-tools. > > * There is code duplicated between the linuxrc and lsznet (see, eg, the > > declaration of $CU) and there's already divergence between the two > > Wow, I'm impressed by the detail of your findings. > The divergence is intentional. Lsznet was developed as a generic tool > sensing all types of network adaptors independent of a use case such as > in an installer environment. However, linuxrc.s390 is only interested in > the supported ones, i.e. qeth, lcs (and the deprecated ctc and iucv) all > with TCP/IP. With some magic we could put them in a common code base, > but I refrained from doing so because it would have generated yet > another (small) file both lsznet and linuxrc depend on. If both pieces are in anaconda, then we can depend on it. If something's not supported, then modules just shouldn't be built and detection should carry-on from there. It shouldn't be based on arbitrarily which pieces of the shell script are left out. > > * cardtype2cleartext() is the sort of thing which gets out of date and > > then becomes critical must fix bugs... if this information is > > important, it probably should be something we can query either with > > modinfo on a module or having the kernel tell it to us directly > > The output of cardtype2cleartext is for information presented to the > user on the screen. Nothing more, nothing less. It is not used for any > program logic. I don't think we can query it automatically such as using > modinfo since the sysfs values only become meaningful after establishing > a ccwgroup and setting the group device online, since only then the > device driver gets this information from the adaptor; in other words, > the values are dynamic. Sure, the device driver could provide > user-friendly strings in the sysfs attribute. However, this would > require changing the existing value other software may already depend on > and would break on changing the strings. That's why I'm convinced that > the values won't be changed often if at all and we should be safe with a > static mapping table. If a new sysfs value should appear in the future, > our code will inform the user and we may easily fix it. Then add a new sysfs value. You guys are the upstream of the kernel module -- I'd rather have the new bits depend on newer kernels than carrying around display information that's out of date. Because inevitably, things that are used for "information presentation" become the last minute, fire drill, oh my god the world is falling apart bugs. Every. Single. Time. > > * Aren't we now doing udev rules for persistent device naming rather > > than modprobe.conf aliases? > > Sorry, I don't know about that. Would that be a Rawhide/RHEL6 thing? If > so, could you please point me to some more information so I can make the > script more Rawhide compliant? It is rawhide, yes. See the various udev rule generation in anaconda's network.py as well as the stock persistent rules in udev upstream. > > * Why are they disabling ipv6 autoconf? > > Disclaimer: IPv6 support is not (fully) functional. The part in linuxrc > should be complete but support in loader is missing for transferring the > configuration over to stage2. > > As far as I understood linuxrc, it only supports manual configuration of > IPv4, i.e. no DHCP or other fancy stuff. So I coded IPv6 support the > same way. Am I mistaken that we either ask the user for IP address and > the like or user autoconfiguration? We ask because the impression that has always been given is that there isn't support for dhcp, etc on the s390 net adapters. If there now is, then it'd be great to use it instead :) > > * Some of the helptext (helptext_nettype() is the one I'm seeing right > > now) is concerning. If something isn't supported, then we shouldn't > > support it. If the code is there, it's going to be considered supported > > At least for RHEL5 I was told, that the code including drivers are still > there since there might be customers needing those things even though > they might not be officially supported by RHEL. Since we on System z are > very careful not breaking any potentially old environment I made sure > the code for CTC and IUCV remains there and also benefits from the > improved user interface. In fact it was in the existing linuxrc and I > just transferred it over. But again, if we're pushing something to rawhide (which is what David said he was doing), then we _shouldn't put the code in_. > > * Translators are going to hate a lot of the text. And given that this > > is being shown on the line mode terminal, how many translations can > > really be displayed? > > AFAIK, the texts in linuxrc haven't had any translations so far even > though they were already previously coded with the deprecated $"..." > construct of the bash. From the top of my head, I wouldn't even come up > with a solution when and where to select a language before linuxrc > starts running. In a nutshell, I assumed there would only be English > texts and as a non-native speaker I welcome any suggestions for > improvement (hoping they still fit into one line ;)). If it's not going to be translated/shown translated, then it shouldn't be marked with $"...". Then it's not a problem :-) > > * Checking bash versions is also silly > > As mentioned above, lsznet was designed independently as a generic > distro-independent tool and I found many peculiarities with different > bash versions. I found it more user-friendly to bail out early than to > keep on running and producing errors all over since the necessary bash > functionality is not there. Yeah, but if it's not being packaged separately and maintained entirely separately, then we can make assumptions. Note: if you want to maintain lsznet as an entirely separate thing that anaconda just sucks in, much like we do lots of other utilities, that's fine too. And then check bash versions all you want :-) > > The > > mix of new functionality, reformatting and changing the way things work > > makes this essentially unreviewable for correctness so I wasn't really > > able to address anything there :/ > > IMHO, you addressed a lot of valid points which I am thankful for. I > tried to retain as much of the old code as possible but the new user > interface and all the new checking which was missing before plus some > rearranging for better maintenance made it virtually undiffable. :( > For what it's worth, I spent a lot of effort making sure it behaves like > the old linuxrc with regard to configuration outcome. Yeah, I think that given the rearranging, etc, there's an effort which could definitely be made to remove some of the "cruft" rather than just trying to keep the old to keep the old. Exhaustive testing is the only way correctness is really going to be able to be determined with that much change. > Sorry for the long reply. On System z things are sometimes quite special > and I felt that thorough explanations are in order. Yep, s390 is a strange beast. I hope that as we move forward in rawhide, we can get it to be a little bit less strange and special as we use all the "normal system stuff" in anaconda across all arches (eg, udev/hal/NetworkManager). And if those don't work for s390, we should focus on fixing them rather than working around that fact in anaconda code (+ then the real system). Jeremy _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
