[Gluster-devel] Question on merging zfs snapshot support into the mainline glusterfs
sriram at marirs.net.in
sriram at marirs.net.in
Thu Dec 15 06:39:32 UTC 2016
Hi Avra,
I've update the patch according to the comments below. And created a
single patch which does the initial modularization. Fixed the tab-
>space issue as well. I've raised a new review request for the same
bug ID here:
http://review.gluster.org/#/c/16138/
Added, Rajesh and You as the reviewers, let me know if I need to do
anything else.
Could you have a look and let me know?
(Sorry for the delay in creating this)
Sriram
On Thu, Oct 13, 2016, at 12:15 PM, Avra Sengupta wrote:
> 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 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 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 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 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 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 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
>>>>>>>>>>> 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 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
>>>>>>>>>>>>>> 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>
>>>>>>>>>>>>>>>> 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 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> 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>>
>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On Tue, Jul 19, 2016 at 11:23 AM,
>>>>>>>>>>>>>>>>>>>>> <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
>>>>>>>>>>>>>>>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _________________________________________________
>>>>>>>>> Gluster-devel mailing list
>>>>>>>>> Gluster-devel at gluster.org
>>>>>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>>>>>
>>>>>>
>>>>>> _________________________________________________
>>>>>> Gluster-devel mailing list
>>>>>> Gluster-devel at gluster.org
>>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>>>>
>>>
>>> _________________________________________________
>>> Gluster-devel mailing list
>>> 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/20161215/f66fcfd9/attachment-0001.html>
More information about the Gluster-devel
mailing list