On 06/17/10 04:58 PM, Keith Mitchell wrote:
If this is the case, there are places that call get_smf_prop() that
may seem to result in error. For example, at line 438-439, if return
code is 0, but shell="", the command option snippet that gets built
is broken isn't it? Same goes for subsequent calls go to
get_smf_prop()
in this function.
You are right, thank you for catching this. I have added explicit check
for those cases (login, shell, roles, home_zfs_fs, home_mntpoint).
OK, just a couple more that I missed ...
452 - I think its a bit dangerous to hardcode "rpool" as the
pool name here (even though currently in AI we always expect
to install into 'rpool') It would be nice if you just wrote a small
bit of code to figure out the name of the pool on which the
dirname of $home_mntpoint file system lives, and fill that in
as the poolname. That way it'll just work when we use this with
text-installer, and when AI supports pool naming and multiple
pools. I would prefer that you do this now but if you choose
not to, a bug to track this would be ok I suppose.
(Sorry for jumping in in the middle, but this caught my eye)
Looks like this is just a fallback for if the SMF prop doesn't exist?
Yep.
Nevertheless, this default would cause conflicts in the text installer
if an existing "rpool" were preserved - this code could accidentally
modify an existing dataset, as opposed to the newly created
OpenSolaris root pool
As I mentioned in my response to Ethan, I believe that in case of text
installer we might just go with all parameters filled in SC profile,
since all that information is determined during installation and could be
stored in DOC, thus later put into generated profile.
I agree we will have to address this when AI supports multiple pools,
but I believe we do not need to resolve it right now.
On a related note - if the mountpoint is not explicitly set, it
shouldn't be set. Instead, let that ZFS property be inherited from the
parent dataset. For example, if I were to create
"rpool/export/home/newuser" on my laptop right now, I wouldn't need to
explicitly set the mountpoint - ZFS would set it as
${mountpoint_of_rpool/export/home}/newuser
Yep. I agree. I have changed the code accordingly.
If not explicitly provided, default mountpoint is only
determined for non-global zones. In that scenarios,
delegated ZFS datasets are created in global
zone with legacy mountpoint.
For global zone case, if not explicitly provided,
mountpoint is inherited from parent ZFS dataset.
I have updated webrevs:
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/
http://cr.opensolaris.org/~dambi/bug-15723-cr/
Thank you,
Jan
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
