[Gluster-devel] Question on merging zfs snapshot support into the mainline glusterfs
sriram at marirs.net.in
sriram at marirs.net.in
Tue Oct 4 05:46:43 UTC 2016
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
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20161004/8f633db6/attachment-0001.html>
More information about the Gluster-devel
mailing list