Moinak Ghosh wrote: > Hi, > > From a series of previous discussions it was deemed better > to re-write the Transfer module in Python. There are several > reasons to do this. The biggest being ease of implementation > and maintenance. This module needs to do a bunch of work that > is more appropriate for a shell script which at the same time > doing stuff better handled by a high-level language. Python > lends itself well to either task. It needs some ugly, voluminous > C code to do these things (copy repository.db, create a few > directories etc.) in the present Transfer module. In addition > Transfer module is also being extended to handle setting up and > installing packages in the proto directory for Distribution > Constructor. That is also easier done via Python. > > I have re-written the module in Python. The original C interfaces > to the Transfer module remain unchanged. However the these > interfaces are now wrappers that load up an embedded Python > interpreter. An argument list is built up based on the nvlist > entries passed in and the python module is invoked. The C wrapper > extends the embedded interpreter by providing a couple of callbacks > that the python code can invoke. The memory overhead of loading an > embedded python interpreter is approximately 3MB. There is no > performance degradation as Transfer module spends most of it's > time in cpio. > > The webrev is at: > http://cr.opensolaris.org/~moinakg/tm_python/ > > This code replaces the functionality provided by the existing > Transfer module in C. I have successfully installed a DP2 image > using this module and got a working installed setup. >
transfer_mod.py 55 (and others): seems stylistically wrong for the indent level of the continuation line to increment to the same as the contained block; I would have expected a four-space indent similar to what we do in other languages. Is there a reason you did it this way? 63: s/spio/cpio/ 113 (and others): now you went very deep with the continuation indent; let's be consistent somehow. 112-122: it seems like we will want to make the slim post-processing in the DC either provide these patterns or the complete file lists. Has that been discussed with those on the slim team who are supposed to do the post-processing? 129: should be under the if 176: please file that rfe in solaris/kernel/keyboard 333: a comment on this block seems advisable 355: why flush these at different levels? 400,445: i'm a little concerned that we completely dismiss any cpio errors, but seem to expect to see them. what are the errors we're typically seeing, and can we eliminate them? Users who look at logs are liable to generate extra support traffic that we don't need. 425: can this actually happen? seems like we'll end up with mis-owned directories if we do, so maybe this should be an error? 455: the skeleton.cpio isn't on the slim CD, so this can be removed 470-: There's a very fuzzy line between what's here and what happens in install-finish and in other parts of the orchestrator. I'd like to see us have a discussion about where these things actually belong, because right now it's a big mish-mash of things (especially things like removing menu.lst here, then install-finish creates a different one). A few things look like they belong here, such as some of the unlnk_list and the cleanup of /var/tmp, but others may not. 482-484: please leave the ssh keys alone, they're now generated on each boot and should just be transferred and used 487: I don't believe we're creating /.mounted any more, that was in anil's fix for bug 700. transfermod.c numerous cstyle errors here... 1: Missing our CDDL and copyright 41: I presume we're going to switch this over to using liblogsvc 237,255,261: needs to be a log message Makefile.master, Makefile.lib, Makefile.targ, Makefile update copyright and remove SCCS idents Dave
