[Gluster-devel] Question on merging zfs snapshot support into the mainline glusterfs
sriram at marirs.net.in
sriram at marirs.net.in
Wed Oct 12 17:16:27 UTC 2016
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/20161012/9c25d485/attachment-0001.html>
More information about the Gluster-devel
mailing list