On 06/21/11 07:06 PM, Edward Pilatowicz wrote:
hey all,
i've got another wad of misc zones/li bugfixes:
http://mcescher.us.oracle.com/export/ws/pkg.bugfix2/webrev
18349 linked image code needs to be more resilient in the face of zoneadm
failure
18550 zone attach should reject entire if necessary
18578 p2v - cannot install s11 images with embedded zones
thanks
ed
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
General comment:
I find this code fairly hard to review because parameter names without
obvious interpretations don't have any comments or docstrings explaining
them (for example li_md_only). Adding docstrings for the parameters
would be a nice addition.
There seems to be a lot of renaming going on for reasons I couldn't
really infer (f -> ferrout, pkg_errout -> errout).
I had a hard time lining up the changes I was seeing with the bugs
listed above, but here's what I came up with:
18349: zone.py lines 162-177 and the change from SubprocessError to
LinkedImageException seem related to this, but I'm left wondering what
happens if /etc/zones exists, and is populated, but populated with
garbage? From what I'm seeing, it looks like in that situation, we'd
print a nice error message, but it's not clear to me how these changes
would allow a 'pkg fix' to be run on the GZ when it couldn't before.
18550: this seems to be covered by the change to attach where --reject
is optionally provided plus the addition of the reject_list parameters
in common.py.
18578: I think this is the -I change in p2v.sh...
The rest of the changes I have a hard time understanding how they're
fixing these bugs.
linkedimage/common.py:
I wasn't sure why lin had been removed from a bunch of functions. I'm
assuming it's because it's only needed in __detach_child, and it can be
inferred from lic. Is there ever a situation where we'd want to use a
different name than a child thought it had?
t_linked_image:
Having a test that the -I option (if it's needed) allows fix and other
commands to work in the face of a broken /etc/zones seems useful.
I looked over the shell files but I have no feeling that I'd notice
anything wrong w/them, someone else needs to review those bits.
p2v.sh:
A comment explaining why the -I is necessary would be nice. At first
glance, since we know we're in a ngz, I'm surprised a -I is needed (and
by extension, if we ever did support images which link to the zone root
image, like user images might, I would've thought we'd want to propagate
the change in variant setting.)
Just a random thought that popped into my head but is there any worry
about making sure to change the variant in all the boot environments
that might have come along from the physical system, or is only one
specific BE moved during p2v?
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
