Hi Thorsten...
Thorsten Frueauf wrote:
> Hi Ed et al,
>
> see comments inline:
>
> Ed McKnight wrote:
>> Thanks Thorsten, see inline.
>>
>> Thorsten Frueauf wrote:
>>> Hi Ed et al,
>>>
>>> here are my comments:
>>>
>>> - usr/src/cmd/initpkg/manifest/Makefile
>>> usr/src/cmd/initpkg/svcmethod/Makefile
>>>
>>> * copyright 2008 -> 2009
>>
>> Oops.
>>
>>>
>>> - usr/src/ipsdefs/ha-cluster/*
>>> usr/src/ipsdefs/Makefile
>>>
>>> * I am not sure why we need that. My vote is to not deliver this.
>>
>> This was Nick's request. Lucia also expressed some...slight confusion
>> (?) If Nick doesn't chime in (doubtful for today) to explain it more
>> I'll defer it for now, and maybe we'll drop it altogether.
>
> OK.
>
>>>
>>> - usr/src/cmd/initpkg/svcmethod/svc_boot_check_enabler
>>>
>>> * I see that line 64 says this service should be never enabled.
>>
>> Well, should be disabled after being run once.
>
> Right. What would happen if the user would by some reason enable it
> again? Is it kind of idempotent?
Yes, as you noticed below.
>
>>> Wouldn't it be better to have this service deleted once it was run?
>>> I guess the service can't delete itself, but can't this be done
>>> within
>>> scinstall?
>>
>> Umm...probably. The target task used to be done in
>> <pkg>_ips_processing under scinstall without the use of a service at
>> all. The scenario I'm working around is that of installing bits, then
>> rebooting before running scinstall. scinstall itself didn't know
>> about the task and I don't think it's important enough to introduce
>> that knowledge into scinstall itself for cleanup. So, I'd have to put
>> the service deletion into the package ips_processing script, which
>> would work, but might be more confusing than just leaving it visible
>> as having done its task. More remarks below.
>>
>> In wondering how deletion at install time would play/conflict with
>> deletion at remove time I came to realize that I hadn't set up
>> service cleanup in preremove as we do for all services we deliver.
>> Updated diffs:
>
> Note that the diff below modifies smf_sync_enable - thought you meant
> disable here (assuming it is in the ips_preremove() section, while
> below looks it is ips_postinstall()).
In postinstall I no longer do the smf_synch_enable: that all gets moved
to the new service.
In preremove I do the deletes and such on a (new) service delivered in
this package.
>
>> *** /tmp/geta25746 Mon Mar 16 10:10:57 2009
>> --- SUNWscz.ips-processing.ksh Mon Mar 16 10:04:05 2009
>> ***************
>> *** 110,117 ****
>> rbac_add SUNWscz/exec_attr || errs=$(expr ${errs} + 1)
>> rbac_add SUNWscz/prof_attr || errs=$(expr ${errs} + 1)
>>
>> - smf_synch_enable svc:/system/cluster/cl_boot_check:default
>> -
>> is_exclusive_zone && (update_etc_services || errs=$(expr
>> ${errs} + 1))
>>
>> if [ ${errs} -ne 0 ]; then
>> --- 110,115 ----
>> ***************
>> *** 203,208 ****
>> --- 201,210 ----
>> svc:/system/cluster/cl_boot_check:default \
>> /var/svc/manifest/system/cluster/cl_boot_check.xml
>>
>> + smf_manifest_rm \
>> + svc:/system/cluster/cl_boot_check_enabler:default \
>> + /var/svc/manifest/system/cluster/cl_boot_check_enabler.xml
>> +
>> if [ ${errs} -ne 0 ]; then
>> cleanup 1
>> else
>>
>>>
>>> If this is too complicated to impelemt then I won't like the
>>> service
>>> being there, but won't object for the change.
>>
>> The entire concept of using smf services as "assemblers" requires
>> run-once-then-never-again behavior. A slight twist would be to leave
>> the service enabled and within it check to see if it should do
>> anything, but the only effective difference then would be in svcs
>> displays. IMHO, the use of smf services for assemby and tear-down is
>> flawed, and this case demonstrates one of the reasons. You can see
>> why I didn't want to use smf services for postinstall of the
>> framework. This one case, however, is most reliably solved using this
>> technique. It does generate the desired result: run once at first
>> boot time after bits laid down, then never again. Minimal clutter.
>
> I agree - but we should make sure that if the user (for whatever
> reason) enables this "run-once-and-never-again" service, it should
> behave like that - ie. only do thinks once, afterwards just return 0.
>
> If in this specific case it will not cause any harm that
>
> echo "/usr/sbin/svcadm enable -s $1" >> ${upgrade_file}
>
> is run again, I will be happy with that as well, but should be
> explained in a comment.
Very good, thx, --emk
>
> Greets
> Thorsten
>
>
>>> Ed McKnight wrote:
>>>> I need one more reviewer for this... thx, --emk
>>>>
>>>> Ed McKnight wrote:
>>>>> Sorry for the rush--hoping to putback these changes for build 7.
>>>>>
>>>>> http://galileo.sfbay.sun.com/workspace/emk/sc/ws/webreviews/CO-IPS-10/
>>>>>
>>>>>
>>>>> Please review the changes for:
>>>>>
>>>>> - 6802680 Disabled service cl_boot_check:default prevents node
>>>>> from boot up
>>>>> Under SVR4, postinstall schedules this service for synch enable
>>>>> at next boot; if it's not enabled at boot then console-login can't
>>>>> be enabled. Under IPS there is no postinstall and presently, if
>>>>> this is synch enabled via scinstall then there is a window between
>>>>> pkg install and scinstall where if the user reboots, the login
>>>>> service will not be enabled.
>>>>> The fix is to deliver (only under IPS) a new service called
>>>>> cl_boot_check_enabler which is delivered enabled so it runs at
>>>>> package installation time, which does the necessary scheduling.
>>>>> Now the bad-boot window no longer exists.
>>>>> Note that this fix requires the application of the fix for
>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7264 which
>>>>> unfortunately didn't make it into 109. For people on-SWAN I made a
>>>>> little installer that applies the fix. It can also be done
>>>>> manually very easily.
>>>>>
>>>>> - create default group package: ha-cluster
>>>>> As a placeholder I've set this group package to do
>>>>> ha-cluster-full. Those who believe that default install should be
>>>>> ha-cluster-minimal, sing out now.
>>>>>
>>>>> - 6805657 manual for vxvm in clsetup needs to be removed from
>>>>> colorado
>>>>> Similar to the fix for disabling unavailable data service
>>>>> wizards in clsetup, this fix tests to see if we have clustering
>>>>> support for vxvm, and if not, disables all vxvm-specific options
>>>>>
>>>>> thx, --emk
