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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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
    <mailto: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 <mailto: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>
            <mailto: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>
            <mailto: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 <mailto:Gluster-devel@gluster.org>
    http://www.gluster.org/mailman/listinfo/gluster-devel






_________________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org <mailto: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

Reply via email to