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

Avra Sengupta asengupt at redhat.com
Thu Oct 13 06:45:44 UTC 2016


Hi Sriram,

The point I was trying to make is, that we want that each patch should 
compile by itself, and pass regression. So for that to happen, we need 
to consolidate these patches(the first three) into one patch, and have 
the necessary make file changes into that patch too.

http://review.gluster.org/#/c/15554/
http://review.gluster.org/#/c/15555/
http://review.gluster.org/#/c/15556/

That will give us one single patch, that contains the changes of having 
the current code moved into separate files, and it should get compiled 
on it's own, and should pass regression. Also, we use spaces, and not 
tabs in the code. So we will need to get those changed too. Thanks.

Regards,
Avra

On 10/12/2016 10:46 PM, sriram at marirs.net.in wrote:
> Hi Avra,
>
> Could you let me know on the below request?
>
> Sriram
>
>
> On Tue, Oct 4, 2016, at 11:16 AM, sriram at marirs.net.in 
> <mailto:sriram at marirs.net.in> wrote:
>> Hi Avra,
>>
>> I checked the comment, the series of patches, (There are nine 
>> patches) for which I've posted for a review below. They've all the 
>> necessary makefiles to compile.
>>
>> Would you want me to consolidate all'em and post them as a single 
>> patch? (I thought that would be a little confusing, since it'd 
>> changes with different intentions).
>>
>> Sriram
>>
>>
>> On Mon, Oct 3, 2016, at 03:54 PM, Avra Sengupta wrote:
>>> Hi Sriram,
>>>
>>> I posted a comment into the first patch. It doesn't compile by 
>>> itself. We need to update the respective makefiles to be able to 
>>> compile it. Then we can introduce the tabular structure in the same 
>>> patch to have the framework set for the zfs snapshots. Thanks.
>>>
>>> Regards,
>>> Avra
>>>
>>> On 09/30/2016 10:24 AM, sriram at marirs.net.in 
>>> <mailto:sriram at marirs.net.in> wrote:
>>>> Hi Avra,
>>>>
>>>> Could you have a look into the below request?
>>>>
>>>> Sriram
>>>>
>>>>
>>>> On Fri, Sep 23, 2016, at 04:10 PM, sriram at marirs.net.in 
>>>> <mailto:sriram at marirs.net.in> wrote:
>>>>> Hi Avra,
>>>>>
>>>>> Have submitted the patches for Modularizing snapshot,
>>>>>
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1377437
>>>>>
>>>>> This is the patch set:
>>>>>
>>>>> http://review.gluster.org/15554 This patch follows the discussion 
>>>>> from the gluster-devel mail chain of, ...
>>>>> http://review.gluster.org/15555 Referring to bugID:1377437, 
>>>>> Modularizing snapshot for plugin based modules.
>>>>> http://review.gluster.org/15556 - This is third patch in the 
>>>>> series for the bug=1377437
>>>>> http://review.gluster.org/15557 [BugId:1377437][Patch4]: Refering 
>>>>> to the bug ID,
>>>>> http://review.gluster.org/15558 [BugId:1377437][Patch5]: Refering 
>>>>> to the bug ID,
>>>>> http://review.gluster.org/15559 [BugId:1377437][Patch6]: Refering 
>>>>> to the bug ID,
>>>>> http://review.gluster.org/15560 [BugId:1377437][Patch7]: Refering 
>>>>> to the bug ID. * This patch has some minor ...
>>>>> http://review.gluster.org/15561 [BugId:1377437][Patch8]: Refering 
>>>>> to the bug ID, this commit has minor fixes ...
>>>>> http://review.gluster.org/15562 [BugId:1377437][Patch9]: Refering 
>>>>> to the bug ID, - Minor header file ...
>>>>>
>>>>> Primarily, focused on moving lvm based implementation into 
>>>>> plugins. Have spread the commits across nine patches, some of them 
>>>>> are minors, except a couple of ones which does the real work. 
>>>>> Others are minors. Followed this method since, it would be easy 
>>>>> for a review (accept/reject). Let me know if there is something 
>>>>> off the methods followed with gluster devel. Thanks
>>>>>
>>>>> Sriram
>>>>>
>>>>> On Mon, Sep 19, 2016, at 10:58 PM, Avra Sengupta wrote:
>>>>>> Hi Sriram,
>>>>>>
>>>>>> I have created a bug for this 
>>>>>> (https://bugzilla.redhat.com/show_bug.cgi?id=1377437). The plan 
>>>>>> is that for the first patch as mentioned below, let's not meddle 
>>>>>> with the zfs code at all. What we are looking at is segregating 
>>>>>> the lvm based code as is today, from the management 
>>>>>> infrastructure (which is addressed in your patch), and creating a 
>>>>>> table based pluggable infra(refer to gd_svc_cli_actors[] in 
>>>>>> xlators/mgmt/glusterd/src/glusterd-handler.c and other similar 
>>>>>> tables in gluster code base to get a understanding of what I am 
>>>>>> conveying), which can be used to call this code and still achieve 
>>>>>> the same results as we do today.
>>>>>>
>>>>>> Once this code is merged, we can use the same infra to start 
>>>>>> pushing in the zfs code (rest of your current patch). Please let 
>>>>>> me know if you have further queries regarding this. Thanks.
>>>>>>
>>>>>> Regards,
>>>>>> Avra
>>>>>>
>>>>>> On 09/19/2016 07:52 PM, sriram at marirs.net.in 
>>>>>> <mailto:sriram at marirs.net.in> wrote:
>>>>>>> Hi Avra,
>>>>>>>
>>>>>>> Do you have a bug id for this changes? Or may I raise a new one?
>>>>>>>
>>>>>>> Sriram
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Sep 16, 2016, at 11:37 AM, sriram at marirs.net.in 
>>>>>>> <mailto:sriram at marirs.net.in> wrote:
>>>>>>>> Thanks Avra,
>>>>>>>>
>>>>>>>> I'll send this patch to gluster master in a while.
>>>>>>>>
>>>>>>>> Sriram
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 14, 2016, at 03:08 PM, Avra Sengupta wrote:
>>>>>>>>> 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 
>>>>>>>>> <mailto: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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>     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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>>> _________________________________________________
>>>>>>>> Gluster-devel mailing list
>>>>>>>> Gluster-devel at gluster.org <mailto:Gluster-devel at gluster.org>
>>>>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>>>>
>>>>>
>>>>> _________________________________________________
>>>>> Gluster-devel mailing list
>>>>> Gluster-devel at gluster.org <mailto:Gluster-devel at gluster.org>
>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>
>>
>> _________________________________________________
>> Gluster-devel mailing list
>> Gluster-devel at gluster.org <mailto:Gluster-devel at gluster.org>
>> http://www.gluster.org/mailman/listinfo/gluster-devel
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20161013/7062af50/attachment-0001.html>


More information about the Gluster-devel mailing list