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?
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
