jan damborsky wrote: > Hi Ethan, > > > Ethan Quach wrote: >> >> jan damborsky wrote: >>> Hi Ethan, >>> >>> the fix is fine taking into account the fact that TI supports for now >>> only subset >>> of features planned for Spring and that the installer is the only ZFS >>> TI consumer >>> as time of being. >>> >>> I have some more generic questions in order to clarify how these >>> changes would >>> fit into next version of TI, which will support also other consumers >>> (Snap Upgrade, Distribution Constructor). >>> >>> Please see below. >>> >>> Thank you, >>> Jan >>> >>> >>> >>> [1] I can see that you have introduced some new attributes describing >>> ZFS target: >>> >>> TI_ATTR_ZFS_INIT_BE_NAME >>> - I suppose this is name of BE to be created ? Since there is _INIT_ >>> part in attribute name, is there some intent to have also other BE >>> types ? If so, how they would differ ? >> >> I think you're right. With respect to TI, its just the BE name of the >> target being creating, so this could be more generic and be >> TI_ATTR_ZFS_BE_NAME. I'll change this. > > Thank you. > > With respect to how names of BE attributes are being created > and how this code will change for Snap Upgrade project (all > BE logic will be implemented in libbe instead of TI) I would suggest > to avoid using _ZFS_ part - then all attributes describing BE would be > in form of > > TI_ATTR_BE_<BE_attribute_name> > > and in this particular case the attribute describing BE name would be > simplified to > > TI_ATTR_BE_NAME > > Please see also below for more detailed comments. > > >> >>> TI_ATTR_ZFS_SHARED_FS_[NUM|NAMES] >>> - Originally, TI ZFS module didn't distinguish between shared and >>> non-shared filesystems. Looking the the code, it seems to me that >>> from TI point of view, there are two differences: >>> >>> * non-shared filesystem is created on >>> <root_pool>/ROOT/<be_name>/<fs_name>, >>> shared filesystem omits ROOT/<be_name> part. >>> >>> * since shared as well as non-shared filesystems needs to be mounted >>> on <alt_root>/<fs_name> mount point, different "zfs mount" command >>> needs to be used. >>> >>> Could you please let me know, if it this observation is correct ? >> >> That's correct. > > Since distinguishing between shared and non-shared file systems is BE > specific and thus this logic is to be implemented in libbe, not in ZFS > module, I think it might be better to use following set of attributes > for describing BE target for TI purposes in order to avoid the situation > when TI_ATTR_ZFS_FS_[NUM|NAMES] attributes are overloaded (used for both > ZFS as well as BE targets). Complete set of BE attributes is listed - > it is based on parameters passed to libbe be_create() function: > > > attribute type description > ------------------------------------------------------------------------- > TI_ATTR_BE_RPOOL_NAME string name of root pool > TI_ATTR_BE_NAME string BE name > TI_ATTR_BE_FS_NUM uint16 number of non-shared datasets > TI_ATTR_BE_FS_NAMES string array non-shared dataset names > TI_ATTR_BE_SHARED_FS_NUM uint16 number of shared datasets > TI_ATTR_BE_SHARED_FS_NAMES string array shared dataset names > TI_ATTR_BE_PERSISTENT boolean_t set to B_TRUE for persistent BE
These attributes seem fine, although I'm not sure what the PERSISTENT flag is. (We can discuss that off this thread though.) So did you intend for these to be used for my putback into the preview2 release or are you saying these are what we should change them to for the Spring release when we have libbe? > > >> >>> [2] Modification to TI ZFS module. >>> zfm_create_fs() function taking care of creating ZFS target now >>> requires that >>> >>> * TI_ATTR_ZFS_INIT_BE_NAME is provided and based on this attribute, >>> special >>> kind of actions is applied to ZFS target during "create" and >>> "mount" operations. >>> >>> >>> * both TI_ATTR_ZFS_SHARED_* and TI_ATTR_ZFS_FS_* has to be provided. >>> >>> Due to above restrictions, zfm_create_fs() is now specific to how >>> BE is designed and can't take care of creating other type of ZFS >>> dataset. >>> >>> Would you like to have this BE logic to be implemented in TI for >>> Snap Upgrade >>> project ? >> >> No. With libbe, all of this *specificness* to what a ZFS BE is >> defined to be >> should get removed from TI. Namely, the BE container dataset creation >> won't be >> in TI. Also, the understanding that a BE's children file systems live >> underneath >> its root dataset will also be encapsulated in libbe. > > Thank you for making this point clear - this was my assumption as well. > > Based on this new TI target type TI_TARGET_TYPE_BE is defined and > would be described by set of attributes listed in table above. > > Then it is assumed TI would only take care of non-BE actions, which > is creating of root pool for now, then it would convert TI BE attributes > to the form libbe understands and would call be_create() for doing the > real job. This sounds congruent to what we have in the design. > > >> >> However, the notion of separateness in a target's 'file systems' and >> 'shared file >> systems' may still be required in TI when creating a BE. > > To tell the truth, since all BE logic would go into libbe, could you please > making this point more clear - why it might be necessary that TI has notion > about how shared and non-shared BE filesystems are being created ? Because of the new BE design, shared and non-shared file systems get created in different locations in the dataset hierarchy, and so libbe needs to distinguish between them. It however, doesn't determine which file systems are shared its own, hence expects the caller to specify which ones are which. -ethan
