On 02/23/12 02:54 PM, Karen Tung wrote:
Hi Drew and Glenn,
Thanks for the previous code review.
I have updated the code with your comments and suggestions.
All the ISO were rebuilt with the updated code, and all the testing
specified below was redone.
updated webrev:
https://cr.opensolaris.org/action/browse/caiman/ktung/umount-bug-fix3/webrev/
Thanks,
--Karen
Hi Karen,
Couple of nits:
305 If the the processor -> If the processor
317 suggest:
libc is mounted. -> libc is mounted, unmounting...
Sue
On 02/23/12 12:38, Karen Tung wrote:
On 02/23/12 12:36, Drew Fisher wrote:
On 2/23/12 1:31 PM, Karen Tung wrote:
Hi Drew,
Thanks for the review. Please see my response inline.
On 02/23/12 12:20, Drew Fisher wrote:
Karen,
I was looking at this and what you have will work, it just seems like there are
a few
unnecessary things that we could clean up:
Why not just do something very simple like:
p = run([MOUNT], logger=logger)
if "/lib/libc.so.1" in p.stdout:
logger.debug("Optimized libc is mounted.")
run([UMOUNT, "/lib/libc.so.1"], logger=logger)
There's no need to keep a LIBC global here. /lib/libc.so.1 isn't going anywhere
so there's no
need to worry about changing it later down the road. No need to make
unnecessary variables /
lookups.
Well, I can remove the LIBC global. I defined it like that because I don't like
hard coding multiple instances of the same string in the code.
Also, walking each line of the output isn't really necessary is it? If
libc.so.1 is anywhere in
the mount output you know it's mounted, right?
I walk each line because I want to find the one that *begins* with
/lib/libc.so.1.
Is there a way to find that without walking each line?
If we just check the output like you suggested, will we pick up false instances
where
/lib/libc.so.1 is just part of the output for something else?
I don't think anything is going to mount /lib/libc.so.1 and you can only mount
one path once so
it's not going to show up multiple times..
What about changing the if-statement to:
if "/lib/libc.so.1 on" in p.stdout (add the 'on' part)
so that we match this:
[mox:src] > mount | grep libc
/lib/libc.so.1 on /usr/lib/libc/libc_hwcap1.so.1
read/write/setuid/devices/rstchown/dev=3250002
on Tue Feb 14 10:17:54 2012
I think that would work. I am going to change the code like you suggested,
and publish a new webrev later today.
Thanks,
--Karen
Thanks,
--Karen
What do you think?
-Drew
On 2/23/12 12:11 PM, Karen Tung wrote:
Thanks everyone for reviewing my initial changes for bug 7108163.
Sanjay pointed out to me privately that the fix only takes care of
unmounting optimized libc overlays for x86. It does not take care
of libc overlays for SPARC. For now, there's no libc overlay for sparc,
so, the fix would still work, but the code will have to be changed again if we
have
a libc overlay for sparc in the future.
I completely re-implemented my changes to take care of the possibility
of having optimized libc for sparc in the future.
The updated webrev is below:
https://cr.opensolaris.org/action/browse/caiman/ktung/umount-bug-fix2/webrev/
For testing, I have performed the same testing outlined below. In addition, I
also built a SPARC text installer image and AI x86 images.
Mary helped installed a T4 machine using the SPARC text installer image to make
sure the changes does not affect SPARC systems. She also helped perform a
a net-boot text install using the AI x86 image to make sure everything works
well
there too.
Thanks,
--Karen
On 02/21/12 17:01, Karen Tung wrote:
I would like to get a review of my changes for fixing the following bug:
7108163 <http://monaco.us.oracle.com/detail.jsf?cr=7108163> Solaris 11 image
fails to boot
after switching between x86 CPU models
http://monaco.us.oracle.com/detail.jsf?cr=7108163
webrev:
https://cr.opensolaris.org/action/browse/caiman/ktung/umount-bug/webrev/
Testing:
- built a x86 text installer image that includes the updated file.
- Install the ISO in a virtual box running on an intel based x86 system.
Exported the ova.
Then, imported
the ova in a virtual box running on an AMD based system. Make sure the imported
virtual machine instance can boot fully to login prompt, and I can log in.
- Also install the ISO in a virtual box running on an AMD based system.
Exported the ova. Then,
imported the ova into a virtual box running on an intel based system. Make sure
the imported
virtual machine instance can boot fully to login prompt, and I can log in.
Thanks,
--Karen
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
