Hi Dave,

Thanks for having a look at this.

* Dave Miner ([email protected]) wrote:
> On 07/13/10 06:50 PM, Glenn Lagasse wrote:
> >[I apologize for anyone that recieves this in duplicate, it was pointed
> >out to me that my mailer had munged the CC list and so people that were
> >meant to be copied, weren't]
> >
> >The ON SNAP Integration project is ready for a code review.  Since we're
> >moving existing code in slim_source into ON I've tried to make things a
> >little easier to review.
> >
> >I have two webrevs/workspaces for review.  The first workspace is based
> >on slim_source and is sync'ed with Jack's latest changes that he's
> >already got a review out for.  This workspace has the file differences
> >for the existing files that are moving from slim_source into ON.  This
> >workspace should be used to review only files that aren't "new"
> >as part of the integration into ON*.  That list of files is:
> >
> >     usr/src/cmd/beadm/BootEnvironment.py
> >     usr/src/cmd/beadm/__init__.py
> >     usr/src/cmd/beadm/beadm.py
> >     usr/src/cmd/beadm/messages.py
> >     usr/src/lib/libbe/be_activate.c
> >     usr/src/lib/libbe/be_create.c
> >     usr/src/lib/libbe/be_list.c
> >     usr/src/lib/libbe/be_mount.c
> >     usr/src/lib/libbe/be_rename.c
> >     usr/src/lib/libbe/be_snapshot.c
> >     usr/src/lib/libbe/be_utils.c
> >     usr/src/lib/libbe/be_zones.c
> >     usr/src/lib/libbe/libbe.h
> >     usr/src/lib/libbe/libbe_priv.h
> >     usr/src/lib/libbe_pymod/libbe_py.c
> >     usr/src/pkg/manifests/install-beadm.mf
> >
> >The webrev for this is:
> >
> >http://cr.opensolaris.org/~glagasse/hg_beadm_1_noinstallgrub/
> >
> >* Note: This workspace/webrev does *not* contain all of the changes to
> >actually remove beadm/libbe from slim_source.  That will be done
> >seperately.
> >
> 
> No comments on this webrev.
> 
> >The second workspace is based on the ON gate and is the 'final' workspace
> >which includes all of Jack's changes as well as my changes for the project.
> >This workspace should be used to review only files that are "new" as part
> >of the integration (although you can also look at it for a feel of what is
> >going where once everything is transferred).  That list of files is:
> >
> >added:
> >     usr/src/cmd/beadm/Makefile
> >     usr/src/cmd/beadm/__init__.stub.py
> >     usr/src/lib/libbe/Makefile
> >     usr/src/lib/libbe/Makefile.com
> >     usr/src/lib/libbe/common/llib-lbe
> >     usr/src/lib/libbe/common/mapfile-vers
> >     usr/src/lib/libbe/i386/Makefile
> >     usr/src/lib/libbe/sparc/Makefile
> >     usr/src/lib/pylibbe/Makefile
> >     usr/src/lib/pylibbe/Makefile.com
> >     usr/src/lib/pylibbe/common/mapfile-vers
> >     usr/src/lib/pylibbe/i386/Makefile
> >     usr/src/lib/pylibbe/sparc/Makefile
> >     usr/src/pkg/manifests/SUNWbeadm.mf
> >modified:
> >     exception_lists/interface_check
> >     usr/src/Makefile.lint
> >     usr/src/Targetdirs
> >     usr/src/cmd/Makefile
> >     usr/src/lib/Makefile
> >     usr/src/lib/libsecdb/exec_attr.txt
> >     usr/src/lib/libsecdb/prof_attr.txt
> >
> >The webrev for this is:
> >
> >http://cr.opensolaris.org/~glagasse/onnv-clone
> >
> 
> usr/src/cmd/Makefile:
> 
> you've not preserved alphabetical order here; beadm needs to be two
> lines further down

Fixed.

> usr/src/lib/Makefile:
> 
> Should there be a .WAIT somewhere between libzfs and libbe?

I wondered about that actually.  My tree builds successfully on the few
machines I've done it on (several times) and so wasn't sure it was
really needed.  But since you've pointed it out, I think it probably
makes sense if for nothing else to protect us against potential races.
How does something like this look:

libzfs .WAIT \
libbe \
pylibbe \
libzfs_jni \
pyzfs \
pysolaris \

> usr/src/lib/pylibbe/Makefile:

> Should copyright for this and/or Makefile.com be based from
> libbe_pymod/Makefile?

I didn't do that since I ended up writing these pretty much from scratch
with some copy/paste from the original libbe_pymod/Makefile.  If I
should still retain that copyright, that's easy enough to change.

> install-beadm.mf:
> 
> Do we really want a compilation link for libbe in /usr/lib?

Probably not.  This was part of the original manifest and has been
delivered to the system all this time so I didn't think to change it.
I'll double check, but I can't think of any reason we need to deliver it
and so if that's true I'll remove it (or follow up with what I find if
it's not true).

Thanks a lot Dave.

Glenn
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to