Ethan Quach wrote: > 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.
Agreed, which is why I included the BE's root dataset and subordinate datasets. However I can also envision the possibility of maybe only wanting to mount the root dataset for the BE. > >> 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. Why not both? A private one that just mounts the root dataset for the BE and the public one that mounts the BE and it's subordinate datasets (just skips all the shared FS's). > > > > 1056 - orig_mntpnt and zfs_get_name(zhp) need to be swapped. fixed. > > >>> 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. hmm it should be right there at line 1050. > > >>> 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(). If it has a line called defaultfoo grub won't know what to do with it either and will fail. However to be save I can put "default " as the string and check for that. > > > 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.) This ugly mess will be updated/fixed with the fixes for 9941 and not as part of this fix. Thanks! -evan
