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 > >> [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. > > 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 ? Thank you very much, Jan