[Gluster-devel] Question on merging zfs snapshot support into the mainline glusterfs

Avra Sengupta asengupt at redhat.com
Wed Sep 14 09:38:59 UTC 2016


Hi Sriram,

Sorry for the delay in response. I started going through the commits in 
the github repo. I finished going through the first commit, where you 
create a plugin structure and move code. Following is the commit link:

https://github.com/sriramster/glusterfs/commit/7bf157525539541ebf0aa36a380bbedb2cae5440

FIrst of all, the overall approach of using plugins, and maintaining 
plugins that is used in the patch is in sync with what we had discussed. 
There are some gaps though, like in the zfs functions the snap brick is 
mounted without updating labels, and in restore you perform a zfs 
rollback, which significantly changes the behavior between how a lvm 
based snapshot and a zfs based snapshot.

But before we get into these details, I would request you to kindly send 
this particular patch to the gluster master branch, as that is how we 
formally review patches, and I would say this particular patch in itself 
is ready for a formal review. Once we straighten out the quirks in this 
patch, we can significantly start moving the other dependent patches to 
master and reviewing them. Thanks.

Regards,
Avra

P.S : Adding gluster-devel

On 09/13/2016 01:14 AM, sriram at marirs.net.in wrote:
> Hi Avra,
>
> You'd time to look into the below request?
>
> Sriram
>
>
> On Thu, Sep 8, 2016, at 01:20 PM, sriram at marirs.net.in 
> <mailto:sriram at marirs.net.in> wrote:
>> Hi Avra,
>>
>> Thank you. Please, let me know your feedback. It would be helpful on 
>> continuing from then.
>>
>> Sriram
>>
>>
>> On Thu, Sep 8, 2016, at 01:18 PM, Avra Sengupta wrote:
>>> Hi Sriram,
>>>
>>> Rajesh is on a vacation, and will be available towards the end of 
>>> next week. He will be sharing his feedback once he is back. 
>>> Meanwhile I will have a look at the patch and share my feedback with 
>>> you. But it will take me some time to go through it. Thanks.
>>>
>>> Regards,
>>> Avra
>>>
>>> On 09/08/2016 01:09 PM, sriram at marirs.net.in 
>>> <mailto:sriram at marirs.net.in> wrote:
>>>> Hello Rajesh,
>>>>
>>>> Sorry to bother. Could you have a look at the below request?
>>>>
>>>> Sriram
>>>>
>>>>
>>>> On Tue, Sep 6, 2016, at 11:27 AM, sriram at marirs.net.in 
>>>> <mailto:sriram at marirs.net.in> wrote:
>>>>> Hello Rajesh,
>>>>>
>>>>> Sorry for the delayed mail, was on leave. Could you let me know 
>>>>> the feedback?
>>>>>
>>>>> Sriram
>>>>>
>>>>>
>>>>> On Fri, Sep 2, 2016, at 10:08 AM, Rajesh Joseph wrote:
>>>>>> + Avra
>>>>>> Hi Srirram,
>>>>>>
>>>>>> Sorry, I was on leave therefore could not reply.
>>>>>> Added Avra who is also working on the snapshot component for review.
>>>>>> Will take a look at your changes today.
>>>>>> Thanks & Regards,
>>>>>> Rajesh
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 1, 2016 at 1:22 PM, <sriram at marirs.net.in 
>>>>>> <mailto:sriram at marirs.net.in>> wrote:
>>>>>>
>>>>>>
>>>>>>     Hello Rajesh,
>>>>>>
>>>>>>     Could you've a look at the below request?
>>>>>>
>>>>>>     Sriram
>>>>>>
>>>>>>     On Tue, Aug 30, 2016, at 01:03 PM, sriram at marirs.net.in
>>>>>>     <mailto:sriram at marirs.net.in> wrote:
>>>>>>>     Hi Rajesh,
>>>>>>>
>>>>>>>     Continuing from the discussion we've had below and
>>>>>>>     suggestions made by you, had created a plugin like structure
>>>>>>>     (A generic plugin model) and added snapshot to be the first
>>>>>>>     plugin implementation. Could you've a look if the approach
>>>>>>>     is fine? I've not raised a official review request yet.
>>>>>>>     Could you give an initial review of the model?
>>>>>>>
>>>>>>>     https://github.com/sriramster/glusterfs/tree/sriram_dev
>>>>>>>     <https://github.com/sriramster/glusterfs/tree/sriram_dev>
>>>>>>>
>>>>>>>     Things done,
>>>>>>>
>>>>>>>     - Created a new folder for glusterd plugins and added
>>>>>>>     snapshot as a plugin. Like this,
>>>>>>>
>>>>>>>     $ROOT/xlators/mgmt/glusterd/plugins +
>>>>>>>     |
>>>>>>>     + __ snapshot/src
>>>>>>>
>>>>>>>     Moved LVM related snapshot implementation to
>>>>>>>     xlators/mgmt/glusterd/plugins/snapshot/src/lvm-snapshot.c
>>>>>>>
>>>>>>>     - Mostly isolated, glusterd code from snapshot
>>>>>>>     implementation by using logging, error codes and messages
>>>>>>>     from glusterd and libglusterfs.
>>>>>>>     - This way, i though we could get complete isolation of
>>>>>>>     snapshot plugin implementation which avoids most of compiler
>>>>>>>     and linking dependency issues.
>>>>>>>     - Created a library of the above like libgsnapshot.so and
>>>>>>>     linking it with glusterd.so to get this working.
>>>>>>>     - The complete isolation also makes us to avoid reverse
>>>>>>>     dependency like some api's inside plugin/snapshot being
>>>>>>>     dependent on glusterd.so
>>>>>>>
>>>>>>>     TODO's :
>>>>>>>
>>>>>>>     - Need to create glusterd_snapshot_ops structure which would
>>>>>>>     be used to register snapshot related API's with glusterd.so.
>>>>>>>     - Add command line snapshot plugin option, so that it picks
>>>>>>>     up on compilation.
>>>>>>>     - If any missed implementation for plugin.
>>>>>>>     - Cleanup and get a review ready branch.
>>>>>>>
>>>>>>>     Let me know if this looks ok? Or need to any more into the
>>>>>>>     list.
>>>>>>>
>>>>>>>     Sriram
>>>>>>>
>>>>>>>     On Fri, Jul 22, 2016, at 02:43 PM, Rajesh Joseph wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>     On Thu, Jul 21, 2016 at 3:07 AM, Vijay Bellur
>>>>>>>>     <vbellur at redhat.com <mailto:vbellur at redhat.com>> wrote:
>>>>>>>>
>>>>>>>>         On 07/19/2016 11:01 AM, Atin Mukherjee wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>             On Tue, Jul 19, 2016 at 7:29 PM, Rajesh Joseph
>>>>>>>>             <rjoseph at redhat.com <mailto:rjoseph at redhat.com>
>>>>>>>>             <mailto:rjoseph at redhat.com
>>>>>>>>             <mailto:rjoseph at redhat.com>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>             On Tue, Jul 19, 2016 at 11:23 AM,
>>>>>>>>             <sriram at marirs.net.in <mailto:sriram at marirs.net.in>
>>>>>>>>             <mailto:sriram at marirs.net.in
>>>>>>>>             <mailto:sriram at marirs.net.in>>> wrote:
>>>>>>>>
>>>>>>>>             __
>>>>>>>>             Hi Rajesh,
>>>>>>>>
>>>>>>>>             I'd thought about moving the zfs specific
>>>>>>>>             implementation to
>>>>>>>>             something like
>>>>>>>>
>>>>>>>>             xlators/mgmt/glusterd/src/plugins/zfs-specifs-stuffs for
>>>>>>>>             the
>>>>>>>>             inital go. Could you let me know if this works or
>>>>>>>>             in sync with
>>>>>>>>             what you'd thought about?
>>>>>>>>
>>>>>>>>             Sriram
>>>>>>>>
>>>>>>>>
>>>>>>>>             Hi Sriram,
>>>>>>>>
>>>>>>>>             Sorry, I was not able to send much time on this. I
>>>>>>>>             would prefer you
>>>>>>>>             move the code to
>>>>>>>>
>>>>>>>>             xlators/mgmt/glusterd/plugins/src/zfs-specifs-stuffs
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>             How about having it under
>>>>>>>>             xlators/mgmt/glusterd/plugins/snapshot/src/zfs-specifs-stuffs
>>>>>>>>             such that
>>>>>>>>             in future if we have to write plugins for other
>>>>>>>>             features they can be
>>>>>>>>             segregated?
>>>>>>>>
>>>>>>>>
>>>>>>>>         It would be nicer to avoid "specific-stuff" or similar
>>>>>>>>         from the naming. We can probably leave it at
>>>>>>>>         xlators/mgmt/glusterd/plugins/snapshot/src/zfs. The
>>>>>>>>         naming would be sufficient to indicate that code is
>>>>>>>>         specific to zfs snapshots.
>>>>>>>>
>>>>>>>>
>>>>>>>>     I don't think the directory would be named
>>>>>>>>     "zfs-specific_stuffs, instead zfs specific source file will
>>>>>>>>     come directly under
>>>>>>>>     "xlators/mgmt/glusterd/plugins/snapshot/src/". I think I
>>>>>>>>     should have been more clear, my bad.
>>>>>>>>     -Rajesh
>>>>>>>>     _________________________________________________
>>>>>>>>     Gluster-devel mailing list
>>>>>>>>     Gluster-devel at gluster.org <mailto:Gluster-devel at gluster.org>
>>>>>>>>     http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>>>>>     <http://www.gluster.org/mailman/listinfo/gluster-devel>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20160914/41ccfe86/attachment-0001.html>


More information about the Gluster-devel mailing list