On 12/14/10 1:34 PM, Karen Tung wrote:
Hi Drew,
--------------
usr/src/cmd/distro_const/__init__.py:
----------------
- line 92 and line 126: Nit: why add the extra blank line?
For /usr/bin/pep8 completeness.
- lines 337-345: In this block of code, you are trying to figure out
the mountpoint of datasets that have not been created based
on the mountpoint of the pool they will be created in. While it works
right now, I think you are making assumptions on how the zpool/zfs
commands interpret and use the zpool's mountpoint when new
ZFS datasets are created. I think it will be "safer" to not make this
assumption. Instead, you just let zfs do it's work. After the "ti"
checkpoint
is run, you retrieve the mountpoint that's assigned to the datasets
by the "zfs create" command.
We have discussed this offline and decided that we will allow the code
to take an educated guess at how ZFS wants to set the mountpoint for the
dataset but *after* TI is called, query ZFS directly and set the
mountpoint of the base dataset to whatever ZFS thinks it is.
--------------------
target_spec.py:
--------------------
lines 399-422: I think the algorithm doesn't take care of the case where
the child datasets are mounted differently than the parent dataset.
For example, if I have the following setup, and I
specified "space/karen/t1" as the build area.
"space/karen/t1" already exists.
kt...@rata {3577} zfs list -r space/karen
NAME USED AVAIL REFER MOUNTPOINT
space/karen 93K 1.78T 31K /space/karen
space/karen/t1 31K 1.78T 31K /t1
space/karen/t2 31K 1.78T 31K /t2
kt...@rata {3578}
The algorithm immediately strips off "t1" the first time it enters the
while loop.
So, it found that space/karen is mounted on /space/karen, and it will
conclude that space/karen/t1 is mounted on /space/karen/t1.
You're absolutely right. I re-ordered the logic as Alok had suggested
and it appears to be valid now. I tested it with explicitly setting
both a mountpoint for the zpool and a different mountpoint for the
dataset. Everything built as it should have.
---------------
ti.py:
--------
lines 92-93: are these 2 lines related to this bug fix?
This is a fix due to a changed target_spec.py. I think this code fix
got lost in the shuffle when did the interim putback of the TI code for
the engine / DC.
line 114: why is it necessary to explicitly specify "None" as the 3rd
argument?
If not specified, the 3rd argument defaults to None.
Fixed. I removed the call to the 3rd argument.
Thanks, Karen!
-Drew
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
