[Gluster-devel] Question on merging zfs snapshot support into the mainline glusterfs
sriram at marirs.net.in
sriram at marirs.net.in
Fri Sep 23 10:40:13 UTC 2016
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
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20160923/ec8b35aa/attachment-0001.html>
More information about the Gluster-devel
mailing list