Can you plase unsubrcribe me from your community?
Thanks ---------- Initial Header ----------- >From : caiman-discuss-bounces at opensolaris.org To : caiman-discuss at opensolaris.org Cc : Date : Thu, 02 Oct 2008 09:32:40 -0700 Subject : caiman-discuss Digest, Vol 21, Issue 5 > Send caiman-discuss mailing list submissions to > caiman-discuss at opensolaris.org > > To subscribe or unsubscribe via the World Wide Web, visit > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > or, via email, send a message with subject or body 'help' to > caiman-discuss-request at opensolaris.org > > You can reach the person managing the list at > caiman-discuss-owner at opensolaris.org > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of caiman-discuss digest..." > > > Today's Topics: > > 1. Re: Please review: DC bootroot autosize (Karen Tung) > 2. Code review request 3678 libict.so.1 is failing to build > (William Schumann) > 3. Re: Code review request 3678 libict.so.1 is failing to build > (jan damborsky) > 4. Re: Code review request 3678 libict.so.1 is failing to build > (Joseph J VLcek) > 5. Re: Code review request for AI installadm tools (jan damborsky) > 6. Distro-Constructor meeting Thursday 10/2: Notes (Jack Schwartz) > 7. Re: DC code review (Jean McCormack) > > > ---------------------------------------------------------------------- > > Message: 1 > Date: Wed, 01 Oct 2008 16:06:25 -0700 > From: Karen Tung <karen.tung at sun.com> > Subject: Re: [caiman-discuss] Please review: DC bootroot autosize > To: Jack Schwartz <Jack.A.Schwartz at sun.com> > Cc: caiman-discuss at opensolaris.org > Message-ID: <48E40271.5090107 at sun.com> > Content-Type: text/plain; charset=us-ascii; format=flowed > > Jack Schwartz wrote: > > Hi everyone. > > > > Here are changes for bootroot autosizing plus a nit bug: > > > > 3340 bootroot auto size > > 3632 pkgrm of SUNWdistro-const doesn't clean up auto_install dir > > > > Webrev: http://cr.opensolaris.org/~schwartz/080928.1/webrev/ > > > > Some ideas / issues came up during development which I made assumptions > > about; please let this email open the forum for discussion as needed. > > > > 1) I added a fourth argument to all finalizer scripts: this arg is the > > root of the bootroot build area. I did this because without it, shell > > scripts (as opposed to python scripts) won't know where it is unless > > they are hardwired with the value. Hardwiring is a bad idea, as it > > introduces duplication. With the fourth arg, the location is defined in > > one place (DC_defs.py) and is automatically disseminated to all scripts > > called by the finalizer. It is much more automatic, thus more easy to > > use and more maintainable. > > > OK. I have no problem with this approach. > > 2) How much extra room to allocate in the bootroot? I added an item to > > the manifest so users can specify. If not specified, it defaults to 10 > > Mb. Seems a number is better than a percentage; why set aside extra > > additional space if the bootroot is already large? Please let me know > > if you have an opinion on this. > > > I agree that a number is better than a percentage. It's OK for this to > be optional. > However, I think the sample slim_cd.xml file we include in the source > tree should > have this value listed, and probably with a comment to explain that it > is 10mb. > Otherwise, without reading the bootroot_archive.py code, > it is harder for ppl to find where/how to make the adjustment, and what > 10 means. > > 3) I also tunefs the bootroot, to get back the 10% UFS sets aside; this > > 10 % isn't needed as the bootroot ends up in RAM anyway. > > > OK. Currently, we are probably unknowingly utilizing this 10% as padding > as well. > I guess we will just have to adjust the padding accordingly. > > 4) For this putback I temporarily created the bootroot build area at > > PKG_IMG_MNT_PT/../br_build_area. I know Jean's working on changes which > > will define a better location for the build area; I will adjust after > > she puts back. > > > OK > > 5) ... about the nit bug (3632): the auto_install directory under > > usr/share/distro_const doesn't get cleaned up because I forgot to list > > in the SUNWdistro-const package's prototype_com file. I am bundling it > > with this code review, but will put it back as a separate fix, since I > > don't want prototype_com to inherit the bootroot autosizing RFE in it's > > history/changeset. > > > OK. > > I've tested my changes by building and booting an AI iso image. I'll be > > sure to test with a slim_cd image before putting back. > > > > > I only have 1 comment about the code in the webrev. Everything else look > good to me. > > bootroot_initialize.py: > > - line 84: Check if the bootroot already exists? If so, no need to > create it. > Also, if it exists and have content, should probably completely remove > all content > before starting to fill it with what you really want. > > Thanks, > > --Karen > > > ------------------------------ > > Message: 2 > Date: Wed, 01 Oct 2008 23:17:35 -0700 > From: William Schumann <William.Schumann at Sun.COM> > Subject: [caiman-discuss] Code review request 3678 libict.so.1 is > failing to build > To: caiman-discuss <caiman-discuss at opensolaris.org> > Message-ID: <48E4677F.9000101 at sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > After looking at lct.c, I noticed that the dependency on libtd is only > to use a function to execute shell commands. This is not a good reason > to have a dependency, and it is misleading, since the log messages are > tagged with TD (Target Discovery). > > So I changed the approach and put the functionality in ict.c and removed > the dependency on TD. > > Tested build with dmake parallel level=16. Tested new libict.so.1. > Updated webrev. > > http://defect.opensolaris.org/bz/show_bug.cgi?id=3678 > http://cr.opensolaris.org/~wmsch/bug-3678/ > > > > ------------------------------ > > Message: 3 > Date: Thu, 02 Oct 2008 10:32:28 +0200 > From: jan damborsky <Jan.Damborsky at Sun.COM> > Subject: Re: [caiman-discuss] Code review request 3678 libict.so.1 is > failing to build > To: William Schumann <William.Schumann at Sun.COM> > Cc: caiman-discuss <caiman-discuss at opensolaris.org> > Message-ID: <48E4871C.8090103 at sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > Hi William, > > the changes look good to me. > Thank you, > Jan > > > William Schumann wrote: > > After looking at lct.c, I noticed that the dependency on libtd is only > > to use a function to execute shell commands. This is not a good reason > > to have a dependency, and it is misleading, since the log messages are > > tagged with TD (Target Discovery). > > > > So I changed the approach and put the functionality in ict.c and removed > > the dependency on TD. > > > > Tested build with dmake parallel level=16. Tested new libict.so.1. > > Updated webrev. > > > > http://defect.opensolaris.org/bz/show_bug.cgi?id=3678 > > http://cr.opensolaris.org/~wmsch/bug-3678/ > > > > _______________________________________________ > > caiman-discuss mailing list > > caiman-discuss at opensolaris.org > > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > > > > > ------------------------------ > > Message: 4 > Date: Thu, 02 Oct 2008 06:32:11 -0400 > From: Joseph J VLcek <Joseph.Vlcek at Sun.COM> > Subject: Re: [caiman-discuss] Code review request 3678 libict.so.1 is > failing to build > To: William Schumann <William.Schumann at Sun.COM> > Cc: caiman-discuss <caiman-discuss at opensolaris.org> > Message-ID: <48E4A32B.6000203 at Sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > William Schumann wrote: > > After looking at lct.c, I noticed that the dependency on libtd is only > > to use a function to execute shell commands. This is not a good reason > > to have a dependency, and it is misleading, since the log messages are > > tagged with TD (Target Discovery). > > > > So I changed the approach and put the functionality in ict.c and removed > > the dependency on TD. > > > > Tested build with dmake parallel level=16. Tested new libict.so.1. > > Updated webrev. > > > > http://defect.opensolaris.org/bz/show_bug.cgi?id=3678 > > http://cr.opensolaris.org/~wmsch/bug-3678/ > > > > _______________________________________________ > > caiman-discuss mailing list > > caiman-discuss at opensolaris.org > > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > This change looks good to me. > > Thank you very much for taking care of this William! > > Joe > > > ------------------------------ > > Message: 5 > Date: Thu, 02 Oct 2008 16:59:56 +0200 > From: jan damborsky <Jan.Damborsky at Sun.COM> > Subject: Re: [caiman-discuss] Code review request for AI installadm > tools > To: Sundar Yamunachari <sundar.yamunachari at Sun.COM> > Cc: caiman-discuss at opensolaris.org > Message-ID: <48E4E1EC.6090702 at sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > Hi Sundar, > > I have gone through all the files - those which > are not referred below look good to me. > > Thank you, > Jan > > > cmd/installadm/installadm.h > --------------------------- > line 55 - According to the AI design specification, > I think it should be: > > 55 #define INSTALL_TYPE "_OSInstall._tcp" > > > cmd/installadm/installadm.c > --------------------------- > 158 - casting to (char *) is redundant > > 161 dirname = fullpath > -> > 161 dirname = strdup(fullpath) > > 164-169 - This might be moved to the beginning > of the function > > 174-175 - 0 is always returned, even if subcommand fails > (for instance because of unknown option). If it is > possible to use installadm in non-interactive mode as well, > I think it might be better if non-zero is returned in case > of failure, so that caller is aware of this. > > 242- is 128 bytes sufficient for DHCP macro ? > > 301-302, 313-314 might be simplified and then 235 could > be removed (it also applies to 555-556, 614-615, 659-660): > > 301 cmdp = &cmds[0]; > 302 (void) fprintf(stderr, "%s\n", cmdp->c_usage); > -> > 301 (void) fprintf(stderr, "%s\n", cmds[0].c_usage); > > 425-426, 621-622 - Is it necessary to define two different > ports wsport and sdport ? I think that they should be the same, > since they both provide the same kind of information - port on > which AI service (AI webserver) can be reached. > > 430 - Is "ai_webserver" name of TXT record correct ? > I think it should be "aiwebserver" instead. > > 859 - just a nit: comment is not correct > > 1008 - I think this is not necessary, since > fgets(3C) reads '\n' into 'buf' > > > cmd/installadm/create-client.sh > ------------------------------- > 241,269 - it seems these checks are redundant, they were > already done on lines 160,192 particularly > > > cmd/installadm/installadm-common.sh > ----------------------------------- > 69 - check for 'multiboot' is done, but on 71-92 no > entry containing 'multiboot' is added - is the check > on 69 correct ? > > 83,86 - ':' separator is added between web server address > and net image path. As 'http://' prefix is used, I think > ':' separator should be omitted, as it might be assumed > that standard URL format is to be provided. > > > cmd/installadm/setup-dhcp.sh > ---------------------------- > 57,87 - TMP_DHCP is populated here - since it seems > it is only temporary variable, I think it should be > removed when it is no longer needed. > > 68 macro+$1 > -> > 68 macro=$1 > > 109,132,158: > net=`echo $n1.0` > -> > net="$n1.0" > > 180 - just a nit (typo): > deson't -> doesn't > > > cmd/installadm/setup-image.sh > ----------------------------- > 115 mkdir -p -- $1 > -> > 115 mkdir -p $1 > > 143 & 150 can be combined in following way: > 143 lofi_dev=`lofiadm -a $file` > /dev/null 2>&1 > 150 mount -F hsfs -o ro $lofi_dev $MOUNT_DIR > /dev/null 2>&1 > -> > mount -F hsfs $file $MOUNT_DIR > /dev/null 2>&1 > > Then 167 & 168 can be combined as well: > 167 umount $MOUNT_DIR > /dev/null 2>&1 > 168 lofiadm -d $lofi_dev > /dev/null 2>&1 > -> > 167 umount $MOUNT_DIR > /dev/null 2>&1 > > 205-220 - Just a thought - at this point it seems we will > be copying bunch of stuff we actually don't need - but I > feel that the right thing to do might be to take care of > this in Distro Constructor and delete all unnecessary files > there - some finalizer script could probably do the job. > > > cmd/installadm/setup-service.sh > ------------------------------- > 46,79: > # $2 - Service Type (_install._tcp) > -> > # $2 - Service Type (_OSInstall._tcp) > > 57-59 - Is it assured that enough time is given > for the look up process ? It seems it might > fail on fast machines just because 'grep' > is called to early before dns-sd is given > a chance to finish its job. > > 114-115 - If dns-sd(1M) fails to register the > service, I think it should be killed. > > 146 - It seems this is fine for now, but > when we start using unicast DNS, check for > domain should be done as well. > > > pkgdefs/SUNWinstalladm-tools/prototype_com > ------------------------------------------ > '/usr/sbin/installadm/webserver.py' > is missing > > > pkgdefs/SUNWinstalladm-tools/depend > ----------------------------------- > 51-52 - Is apache2 needed ? > > I think that SUNWtftp package should be added > > > > Sundar Yamunachari wrote: > > Hi all, > > > > Please review the code for installadm > > > > 3640 Installadm tools to manage services > > http://defect.opensolaris.org/bz/show_bug.cgi?id=3640 > > > > > > 3641 installadm tools to manage clients > > http://defect.opensolaris.org/bz/show_bug.cgi?id=3641 > > > > Webrev: > > http://cr.opensolaris.org/~ysundar/installadm > > > > The code covers the following functionality: > > > > 1. create-service: > > - Setup the netimage directory (from iso or another netimage) > > - Create a DNS service > > - Start a web server associated with the DNS service > > - Create DHCP server if needed > > - Create DHCP network and add addresses to the DHCP table (if needed) > > - Create the DHCP macro for this netimage (and service) > > - Create bootfile in the /tftpboot directory > > - All the services (DHCP, TFTP, and mDNS) are setup and enabled if > > they are not running > > > > 2. delete-service > > - Remove the service > > - Remove the image if needed > > > > 3. start-service > > - Create a service with given name > > > > 4. stop-service > > - remove the given service > > > > 5. create-client > > - Create bootfile in the /tftpboot directory with the name derived > > from MACaddress > > - Create a DHCP macro for this client > > - Enable tftp service if not running > > > > 6. delete-client > > - Remove the tftp bootfile > > > > Additional features: > > > > 1. Apache webserver will be configured at a port 5555 to host images > > (zlibs). When the image is setup, a link will be created under Apache > > docroot to point to the net image directory. The client will access the > > zlib through the link. An apache configured file will be shipped with > > this package to start a webserver at port 5555. > > > > 2. For each service, an AI webserver will be started. The code for the > > AI webserver will be reviewed separately but the webserver > > (webserver.py) is used in this code. > > > > Code organization: > > > > - The main code is in installadm.c. It parses the user input and checks > > the options. Based on the options, call one more scripts. Each service > > or task is in a separate script. The scripts are: > > > > create-client.sh; Create a client > > delete-client.sh; Remove a client > > setup-image.sh: Add/Remove netimages and creates link to webserver > > setup-dhcp.sh: DHCP table creation, Add network, add ip addresses, > > create DHCP macro etc. > > setup-service.sh: Register/lookup/remove service, start/stop aI web > > server. Uses DNS-SD > > setup-tftp-links.sh: creates bootfile under /tftpboot > > installadm-common: Common functions between create-client.sh and > > setup-tftp-links.sh > > > > Thank you, > > Sundar & Sue > > _______________________________________________ > > caiman-discuss mailing list > > caiman-discuss at opensolaris.org > > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > > > ------------------------------ > > Message: 6 > Date: Thu, 02 Oct 2008 08:43:21 -0700 > From: Jack Schwartz <Jack.A.Schwartz at Sun.COM> > Subject: [caiman-discuss] Distro-Constructor meeting Thursday 10/2: > Notes > To: caiman-discuss at opensolaris.org > Message-ID: <48E4EC19.5030001 at sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > * Attendees* > Jean, Karen, Barbara, Dave, Jack > > *AIs * > > Jack: Early next week, Jack to go through buglist and send status. > Since all are busy with putbacks in progress, there seemed to be little > point to going through all the bugs today... > > All: After code freeze next week, we are to review documentation and > send comments to Barbara. > > Jack: close message router bug; bug is no longer necessary due to > Python logging. > > * Notes * > > > > Agenda: > > > >> Checkins: > > > > - Barbara: Documentation > Another batch of edits to article draft next week. As soon as we have > code freeze, will do formal review. > Will do review after code freeze. > > Jean will putback her stuff, then Karen will do logging, then webpage > will be edited by Karen. Karen to let Barbara know when webpage has > been updated. > > - Erik: Testing > Not present. > > - Glenn: Virtual Box images > Not present. > > - Jack: Progress on bugs > - AI stuff went back on Friday. > - Bootroot autosize is out for code review. > - Needs to coordinate with Jean. She's merged with Jack's changes. > Jack to do review of her post-merge > stuff before she puts back. Then Jack to post new code review of > bootroot autosize changes after Jean > puts back. > > - Jean: Progress on bugs > - code review is out for smaller bugs. > - Has a separate workspace for a new set of changes, purge history. > - Wants to work next on bug for default authority for post-install > > - Karen: Progress on bugs > - Got logging to work using python logging. Will have same info which > goes to simple log also go to screen. > - Later today or tomorrow, will send out output for > comments/opinions on format. Can have > timestamps, severity levels, other things supported by Python > logging. > - Out for CR early next week. > - Jack to close bug he filed for writing message router; it is no > longer needed. > > > >> New business: > > > > - Bugcourt > > > > Bugs have been reorganized. We will be looking at new bugs (target > > milestone = "---") and those with target milestone of 2008.11 to mark > > our progress. > > > > Bugs we'll look at are at: > > http://defect.opensolaris.org/bz/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ACCEPTED&bug_status=CAUSEKNOWN&bug_status=FIXUNDERSTOOD&bug_status=FIXINPROGRESS&bug_status=REOPENED&bugidtype=include&chfieldto=Now&columnlist=target_milestone%2Cbug_severity%2Cpriority%2Cassigned_to%2Cbug_status%2Cshort_desc&product=distro-constructor&query_format=advanced&target_milestone=2008.11&target_milestone=---&order=bugs.target_milestone%2Cbugs.priority > > > > > > > >> Old business (carry-overs from last week): > > > >> Roundtable > > > > ------------------------------ > > Message: 7 > Date: Thu, 02 Oct 2008 10:32:15 -0600 > From: Jean McCormack <Jean.McCormack at Sun.COM> > Subject: Re: [caiman-discuss] DC code review > To: Karen Tung <Karen.Tung at Sun.COM> > Cc: caiman-discuss <caiman-discuss at opensolaris.org> > Message-ID: <48E4F78F.5030505 at sun.com> > Content-Type: text/plain; format=flowed; charset=ISO-8859-1 > > Karen, > > Thanks for the review. Comments inline. > > Karen Tung wrote: > > Hi Jean, > > > > Your webrev list doesn't include any change to ai_x86_image.xml. I > > believe that file > > also need to be updated with changes similar to the slim_cd.xml file. > You're correct. Done. > > > > > Here are my comments about files that's on the webrev: > > > > usr/src/cmd/distro_const/DC-manifest.defval.xml > > > > -lines 223-225: You didn't specify a default value here. Doesn't > > everything need a default value > > to be specified in the default section? > The problem I ran into was that since it's an optional attribute, if I > didn't list this in the > defaults file I received an error. (Validation?) Since I'd like the > default to be > Executing <script> which you can't really put into the defaults file I > put nothing. It worked. > So I'll have to say it's not required. Jack may have more to say on this. > > > > usr/src/cmd/distro_const/DC-manifest.rng > > > > -General: the build area is now something associated with the whole DC > > app, not just > > the image. Should we move it to the "distro_constr_params" section > > instead of > > leaving it in the "img_params" section? > Not sure. To me the line is very thin as to the difference between these > two sections. > > > > -line 440: Nite: I know you are not going to fix the problem with the > > mirror in this putback, so, > > perhaps you shouldn't change this comment yet. > Ack you're correct. The comment is back. > > > > usr/src/cmd/distro_const/DC_checkpoint.py > > > > -lines 688-692: The checkpoint_name for each finalizer scripts is > > required. You already > > specified this requirement in the DC-manifest.rng. The manifest > > checking module would > > have already enforced that requirement, you don't need to check it here. > Not true. If you do this in the manifest file: > > <checkpoint name=""> Then it validates OK but the read code will return > None. > > > > > usr/src/cmd/distro_const/DC_ti.py > > > > -The function DC_create_subdirs() is called in multiple places in the > > DC_create_build_area() > > function. Since the DC_create_subdirs() function can handle both ZFS > > and UFS mount points, > > I think it would make the code much easier to read if you were to have > > DC_create_build_area() be focused on determining using ZFS or UFS, and > > enable checkpoint or not.. > > Set up all the mountpoints and stuff. Then, at the end of > > DC_create_build_area(), call > > DC_create_subdirs() instead of calling it all over the place. > I'd like to leave this for now since another bug I'm working on (the zfs > datasets created before execution > starts bug) addresses this and changes it along with other changes. > > > > usr/src/cmd/distro_const/DC_tm.py > > > > -Not quiet sure which bug the change in line 179 is fixing. > > Would "alt_url" be None, if it is part of the "add_repo_url_list"? > > I guess I am confused about what condition this check is trying to > > detect. > Like the checkpoint name. The user leaves the value as "" which > validates but then > becomes none and is unusable. > > > > > > usr/src/cmd/distro_const/DefaultsModule.py > > > > -I think the out_img_path() function is also not used after your > > putback of the work area stuff. > > So, it can be deleted. > Nice catch. Deleted. > > > > usr/src/cmd/distro_const/distro_const.py > > > > -Lines 55-64: This is code is for verifying the command line. Would it > > be better to have > > this preliminary check of the command line options as a separate > > function, > > and call it before DC_get_manifest_server_obj(). That way, it will be > > very > > clear as to where commandline args are checked initially. Then, > > in DC_parse_command_line(), we can skip this initial check, and just > > check > > for the resume/pause step stuff. I think this makes the code much > > easier to read. > Can't. There's a complicated series of interdependencies here. I'm > actually making this cleaner with > the zfs dataset bug too. > > Basically, in order to do the -r -p checks you need to know the > build_area and be able to > check if you can checkpoint or not. To do that, you need the manifest > server obj. > > > > - So, the "chart" that displays what steps are available to be resumed is > > only displayed when people specify "-l"? What if they specify a step to > > resume and it is not valid. What will get displayed? > You must specify an earlier step to resume at. > Valid steps to resume from are: > <list of steps you can resume from> > > So basically a more specific version of the -l > > > > > - Instead of having "Build completed....." in different places, why > > not put it at the end > > of the file? Then, it is just one line. > Good idea. Done. > > > > usr/src/cmd/distro_const/finalizer_checkpoint.py > > usr/src/cmd/distro_const/finalizer_rollback.py > > > > -For both finalizer_checkpoint.py and finalizer_rollback.py, I think > > it would > > be a good idea to check for a minimum required length for the arguments. > OK. I can do minimum, just not exact. > > jean > > > > Thanks, > > > > --Karen > > > > Jean McCormack wrote: > >> I'd like Jack, when he gets back, and Karen to look at this: > >> > >> Webrev: > >> http://cr.opensolaris.org/~jeanm/DC_bugs/ > >> > >> > >> bugs fixed: > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3357 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3359 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3467 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3555 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3556 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3557 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3560 > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3647 > >> > >> Jean > >> > >> > >> > >> _______________________________________________ > >> caiman-discuss mailing list > >> caiman-discuss at opensolaris.org > >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > >> > > > > > > ------------------------------ > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > > End of caiman-discuss Digest, Vol 21, Issue 5 > ********************************************* >
