Clay Baenziger wrote:
> Hello all,
> I've got a webrev for some mighty delete-service changes. The bugs
> I'm hoping to address are:
> 4526 - delete-service is not deleting service as described in section
> 4.3.2 ai_design_doc
> 6587 - delete-service shouldn't remove the source image if there's other
> services actives 'linked' to the same source image
> 10740 - Need way to interact with SMF from Python for installadm
> components in Python
>
> Further, this webrev includes the already reviewed (but as yet unpushed
> and related libpyscf code). The files to ignore (unless you're
> interested) are anything under usr/src/lib/*
>
> Otherwise, if I could get initial comments as soon as possible, however,
> Eric has extended the expected timeline for these changes to be pushed
> to next week opposed to the initial hope of Friday.
>
> Webrev:
> http://cr.opensolaris.org/~clayb/delete_service/
>
> Thank you,
> Clay
Hi Clay,
My comments...
-evan
================================================
installadm.c
-----------------
line 181 and 182 - It may make more sense to have the comment
match "delete-device".
lines 898 - This comment needs a bit more clarification. Maybe
something like:
898 * If the -x argument is passed, it will remove the image.
899 * Other cleanup is handled and done in the SERVICE_DELETE_SCRIPT
ai_utils.c:
-----------------
In the function ai_delete_property() it might be helpful to add a
block comment which states that the property you're getting is just
used to temporarily hold the property and get it into the handle
before starting the transaction.
For lines 782, 785 and 788 shouldn't these if statements have {}'s
so that we're not setting handle->instance, handle->service and
handle->pg if they're already NULL?
libaiscf_backend.c:
-----------------
We're checking for failures from scf_scope_create,
scf_service_create, scf_pg_create and scf_instance_create all in
one if statement. When in the future we want to pass back errors
from these calls it would probably make it easier to have these
checks for each call and not in one if statement like we do for
the rest of the scf calls in this function.
libaiscf_service.c:
-----------------
It appears that the block comments for a majority of the functions
in this file have Parameters listed as none when they appear to
take an AIservice instance (see AIservice_get_prop_names_and_values()
and AIservice_set_subscript() for examples of these missing
parameters)
There also appears to be some formatting and indenting issues has
"cstyle -pvP" been run on the "c" files?
Remove the "Public Functions" comment at line 356 - 358 since there
are not any public functions after that just the method definitions
(or change the comment to reflect more accurately what these are).
libaiscf_instance.c:
-----------------
This has the same problems with missing parameter/argument comments
for the block comments above some of the functions.
Should we be calling "ai_scf_fini(self->scfHandle);" before lines
210 and 229?
