12:02 p.m.
On 03/19/2018 09:56 AM, Tony Asleson wrote:
I was wondering if this discussion (and ones like it) should be done
on
the public mailing list? To encourage community involvement.
Yes please, in the future. For now, I'm CCing the list here, so our
throngs of theoretical list subscribers :-) could catch up by reading
the quoted conversation below.
Thanks -- Andy
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.
>
> Regards,
> -Tony
>
>
> 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
>>
>
12:18 p.m.
New subject: Options for dealing with devicemapper context
On 03/19/2018 10:02 AM, Andy Grover wrote:
> 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.
I also like II, but I'm not convinced of the benefits of implementing
this in dm-rs rather than just in stratisd. If stratisd uses DM so much
we want to make it a singleton that's cool, but is it useful for other
users of dm? (Hard to say, since there are currently no other users.)
How about we move ahead with II, but take a wait-and-see approach
towards pushing it down into dm-rs?
Regards -- Andy
>>
>> 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
12:28 p.m.
New subject: Options for dealing with devicemapper context
On 03/19/2018 12:18 PM, Andy Grover wrote:
On 03/19/2018 10:02 AM, Andy Grover wrote:
>> 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.
I also like II, but I'm not convinced of the benefits of implementing
this in dm-rs rather than just in stratisd. If stratisd uses DM so much
we want to make it a singleton that's cool, but is it useful for other
users of dm? (Hard to say, since there are currently no other users.)
Adding singleton functionality to devicemapper.rs doesn't require end
library users to utilize, at least from what I was thinking WRT
implementation. I see this as an addition to the API, not a replacement
or API feature removal. Nothing should prevent a user from utilizing
the API as it is today and instantiating and using a dm context as
before, but I haven't looked closely at any proposed changes to dm to
support this yet to see if this is indeed true.
Regards,
Tony
2231
days inactive
2231
days old
stratis-devel@lists.fedorahosted.org
2 comments
2 participants
participants (2)
-
Andy Grover -
Tony Asleson