Hi Avra, Could you have a look into the below request?
Sriram On Fri, Sep 23, 2016, at 04:10 PM, sri...@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, sri...@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, sri...@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, sri...@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, sri...@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, sri...@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, sri...@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, <sri...@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, sri...@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 >>>>>>>>>>>>>> <vbel...@redhat.com> wrote: >>>>>>>>>>>>>>> On 07/19/2016 11:01 AM, Atin Mukherjee wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Tue, Jul 19, 2016 at 7:29 PM, Rajesh Joseph >>>>>>>>>>>>>>>> <rjos...@redhat.com <mailto:rjos...@redhat.com>> >>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Tue, Jul 19, 2016 at 11:23 AM, >>>>>>>>>>>>>>>> <sri...@marirs.net.in <mailto:sri...@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@gluster.org >>>>>>>>>>>>>> http://www.gluster.org/mailman/listinfo/gluster-devel >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>> >>>> >>>> _________________________________________________ >>>> Gluster-devel mailing list >>>> Gluster-devel@gluster.org >>>> http://www.gluster.org/mailman/listinfo/gluster-devel >>> > > _________________________________________________ > Gluster-devel mailing list > Gluster-devel@gluster.org > http://www.gluster.org/mailman/listinfo/gluster-devel
_______________________________________________ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel