[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate
- From: Bart Smaalders <
- Subject: [pkg-discuss] Re: code review request: pkg dehydrate/rehydrate
- Date: Thu, 05 Jun 2014 13:18:39 -0700
On 05/29/14 16:23, Yiteng Zhang wrote:
> Hi folks,
> Can I ask you to review on this?
> Some notes:
> 1) pkg dehydrate/rehydrate go through the api planning code path. After
> in the meeting and some experiments, instead of using manifest
> comparison to get
> the files and hardlinks that are needed to remove in dehydrate or to add in
> rehydrate, I generate these actions to a dict and pass it explicitly to
> pkg plans to
> propose an repair. Details in imageplan.py.
> 2) Confirmed with Jesse that pkg dehydrate/rehydrate only needs to work on
> alternate root.
> 3) The last change of pkg man pages was pushed to s11.2 only but not to
> which is weird. So there are a lot of change in src/man/pkg.1.
> 4) Test suite of pkg dehydrate and pkg rehydrate are combined in one
> file, client
> test in src/tests/cli/t_pkg_hydrate.py, and api test in
> Test case might not be sufficient, let me know your idea.
> 5) Manual test on full, partial and user image.
> For example, it takes about 44s to dehydrate an user image only
> installed with
> package core-os. The size of the image goes from 1.2GB to 308MB. And it
> about 3m20s to rehydrate the image.
> 6) Fix "bug 15521086 pkg fix should be moved into the api".
> 7) Probably I need to file a bug about this project.
> webrev: https://ips.java.net/webrev/yitezhan/pkg_hydrate/
> Please let me know your opinion.
This looks like a good start to me.
How does it perform on an image containing solaris-large-server, both in
time taken and space saved?
Some areas of concern:
In __gen_hydrate_actions, you hard-code in exceptions for etc/zones/* files.
This would be
much better done by adding 'dehydrate=false' to those files in their own
way if we find other files that need to be retained, we can add the same
attribute - or
other developers of those packages can. This would need to be done in ON
-but it is
file-specific metadata, and belongs in the packages.
In __gen_hydrate_actions, you call get_manifest with ignore_excludes=True.
It is possible to add a publisher and install packages from it _after_
I think this will break your use of 'all' in the dehydrated_pubs list; you're
off not using 'all' but just dealing with the list of publishers.
Pkg operations performed after dehydration may malfunction in interesting
Do we need to prevent this or test it? What happens if I add a package from
the solaris publisher after doing dehydration? What happens if I rehydrate
doing that? It may just work, but we either need to test it or prevent users
from doing silly things.
The pkg fix stuff needs some thought given about how we want to output to
Should we be using the progress tracker?
Bart Smaalders Solaris Core OS
"You will contribute more with Mercurial than with Thunderbird."
"Civilization advances by extending the number of important
operations which we can perform without thinking about them."