Hi Drew,
--------------
usr/src/cmd/distro_const/__init__.py:
----------------
- line 92 and line 126: Nit: why add the extra blank line?
- 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.
--------------------
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.
---------------
ti.py:
--------
lines 92-93: are these 2 lines related to this bug fix?
line 114: why is it necessary to explicitly specify "None" as the 3rd
argument?
If not specified, the 3rd argument defaults to None.
Thanks,
--Karen
On 12/14/10 10:01, Drew Fisher wrote:
Could I please get a code review for:
7006443 transfer-ips-install checkpoint created image in wrong location
Bug:
http://monaco.sfbay.sun.com/detail.jsf?cr=7006443
Webrev:
http://cr.opensolaris.org/~drewfish/dc_7006443/
The way DC handled mountpoints of ZFS datasets and zpools was very
wrong. It made an assumption that everything would mount at "/".
Tests run:
- Create a new zpool and a new zfs dataset both with no mountpoint
information (zpool mounts at /, dataset mounts at
/zpool_name/dataset_name)
- Create a new zpool with an explicit mountpoint defined and a new zfs
dataset with no mountpoint information (zpool mounts at /mountpoint,
dataset mounts at /mountpoint/dataset_name *with the name of the zpool
stripped from the dataset_name* - this is how ZFS currently works)
- Use an existing zpool, but explicitly set the mountpoint of the zfs
dataset to something outside of the mountpoint of the pool.
Thanks!
-Drew
_______________________________________________
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
