> On 03/19/2018 11:02 AM, Tony Asleson wrote: >> My preference is what you refer to as class II, with placing much of the >> base functionality into devicemapper.rs and limiting or removing dm >> reference passing to functions and methods.

>> >> On 03/16/2018 04:10 PM, Anne Mulhern wrote: >>> Hi! >>> >>> I have identified several options. Do you have a preference among them, >>> or have you another option in mind? Why? >>> >>> The options fall into two classes: >>> >>> I. Pass the DM context around as a parameter. >>> II. Instantiate it once and use a method to get a reference to the >>> parameter where necessary. >>> >>> For (I) the question is where to instantiate the context. The two >>> consistent options are >>>    A. At the outermost level >>>    B. Exactly where needed. >>> >>>    A drawback of (A) is that it requires a bit of maintenance; a >>> merit is that it is >>>    informative, since the caller knows if the context is needed >>> somewhere by any of the >>>    method's callees. (A) is implemented here: >>> https://github.com/stratis-storage/stratisd/pull/835. >>>    It was mostly already done, this is the last little bit. >>> >>>    A drawback of (B) is that it makes failure getting the context >>> need to be handled wherever >>>    the context is obtained. Mostly, this is fairly benign, since a >>> method that needs a DM >>>    context probably has a lot of other ways to fail, too, so usually >>> the code should be as >>>    simple as a "?". Another drawback is that it is an action that is >>> repeated unnecessarily. >>>    A merit is that fewer arguments are passed in many methods. >>> >>> For (II) there are three tasks: >>>    1. Write the necessary code that memoizes the DM after the first >>> call and returns a reference to the DM. >>>       This method is called get_dm_context. This is a bit tricky, >>> there is an unsafe block. >>>    2. Write the code that calls get_dm_context() and correctly >>> handles the possible error that may occur the first >>>       time get_dm_context() is called (using catch_unwind, which is >>> probably a bit risky, kind of like unsafe). >>>    3. Alter the other code accordingly (this is rather broad) >>> >>> and three possible options: >>>    A. Do (1), (2), and (3) in stratisd. In this case (3) consists of >>> calling get_dm_context() whenever it >>>       is needed to pass to a devicemapper-rs method, e.g., >>> LinearDev::setup() and eliminating where >>>       the DM context is instantiated and passed down to callees. See >>> https://github.com/stratis-storage/stratisd/pull/841 >>>       for a partial implementation of this approach. >>>    B. Do (1) in devicemapper-rs and (2) and (3) in stratisd. See >>> https://github.com/stratis-storage/devicemapper-rs/pull/286 >>>       for (1) and >>> https://github.com/stratis-storage/stratisd/pull/843 for (2) and >>> partial implementation of (3). >>>    C. Do (1) in devicemapper-rs, (2) in stratisd, and (3) in both. >>> For (3) in devicemapper-rs, eliminate all &DM parameters >>>       in all methods. See >>> https://github.com/stratis-storage/devicemapper-rs/pull/288 for >>> rough devicemapper-rs implementation >>>       of this part. In stratisd, call all devicemapper-rs methods >>> w/out passing any DM parameter, since the methods no longer >>>       require any such. Do not pass DM parameter anywhere. This part >>> has not been implemented, but you get the idea. >>> >>> I tend to prefer (II), because it changes the places to fail from >>> many to one, at least in theory. >>> >>> - mulhern >>> >> > _______________________________________________ stratis-devel mailing list -- stratis-devel(a)lists.fedorahosted.org To unsubscribe send an email to stratis-devel-leave(a)lists.fedorahosted.org