Evan, Evan Layton wrote: > > I've made the following changes. A new webrev is available at > http://cr.opensolaris.org/~evanl/9594v2 > >> >> >> be_mount.c >> ------------------ >> 332-343 - If we're really mounting root only, this chunk >> needs to be at line 320 instead of here. > > That would cause us to skip mounting any of the BE's subordinate > datasets. Isn't it possible that we could need those for getting > things like zones information?
Yes, if we mounted root only, we would have made an assumption that what we're looking for must exist within the root dataset and not in some subordinate dataset of the BE; though, the things we're looking for are all in /etc, like /etc/vfstab and /etc/zones/ These things likely won't be in separate subordinate datasets, but I'm not sure we can say that these are the only things we'll ever look for going forward. > I can easily move this but I'm not > sure this is the correct thing here. Maybe what we need is a flag > for ROOT_ONLY as well as one for DS_ONLY (or NO_SHARED) that just > mounts the BE and it's datasets? Given the above, it doesn't seem like a ROOT_ONLY flag is what we need, but rather a NO_ZONES flag. And if we switch it to this, it would be useful to expose it in the public libbe and get rid of the private flag altogether. 1056 - orig_mntpnt and zfs_get_name(zhp) need to be swapped. >> 1060 - For cases where orig_mntpnt is non NULL, we >> also need to cleanup and remove the temp directory >> that the pool dataset was mounted on. > > fixed for this instance of the use of be_make_tmp_mountpoint()... I don't see the changes for this. >> be_utils.c >> --------------- >> 1301 - What is the reason why this is changed from >> strcmp to strstr? > > changed back to strcmp You've got a strncmp there now instead, and this doesn't look right either. If the line we've read in has a token named "defaultfoo", this would match and it shouldn't. I think it needs to be strcmp(). Can you please also update usr/src/cmd/beadm/messages.py with the new error codes, and the previous two that we forgot to update from earlier fixes. (yeah, we've still got this mess going on.) thanks, -ethan
