<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div>Hi Avra,<br></div>
<div><br></div>
<div>Thanks for the reply, <br></div>
<div><br></div>
<div>But the problem I see here is the previous patch set sent would'nt compile individually. So, I merged the changes into a single patch , which i'd posted today. Is it ok to drop all the previous posted patches and consider from the new one? Please suggest. <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Thu, Dec 15, 2016, at 12:45 PM, Avra Sengupta wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div> <br></div>
<div> I have already provided comments on the new patch. It seems this
new patch while addressing merge cloflicts, has undone some
previous patches. I suggest you send this patch on top of the
previous patchset(<a href="http://review.gluster.org/#/c/15554/1">http://review.gluster.org/#/c/15554/1</a>) instead
of creating a new one. This will allow you to view the diff
between the new version and the previous version, and will give u
an idea if the diff is something that you added in the patch or
got added as part of merge conflict.<br></div>
<div> <br></div>
<div> Regards,<br></div>
<div> Avra<br></div>
<div> <br></div>
<div> On 12/15/2016 12:09 PM, <a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>I've update the patch according to the comments below. And
created a single patch which does the initial modularization.
Fixed the tab->space issue as well. I've raised a new review
request for the same bug ID here: <br></div>
<div><a href="http://review.gluster.org/#/c/16138/">http://review.gluster.org/#/c/16138/</a><br></div>
<div><br></div>
<div>Added, Rajesh and You as the reviewers, let me know if I need
to do anything else. <br></div>
<div><br></div>
<div>Could you have a look and let me know? <br></div>
<div><br></div>
<div>(Sorry for the delay in creating this) <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div>On Thu, Oct 13, 2016, at 12:15 PM, Avra Sengupta wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div><br></div>
<div>The point I was trying to make is, that we want that
each patch should compile by itself, and pass regression. So
for that to happen, we need to consolidate these patches(the
first three) into one patch, and have the necessary make
file changes into that patch too.<br></div>
<div><br></div>
<div><a href="http://review.gluster.org/#/c/15554/">http://review.gluster.org/#/c/15554/</a><br></div>
<div><a href="http://review.gluster.org/#/c/15555/">http://review.gluster.org/#/c/15555/</a><br></div>
<div><a href="http://review.gluster.org/#/c/15556/">http://review.gluster.org/#/c/15556/</a><br></div>
<div><br></div>
<div>That will give us one single patch, that contains the
changes of having the current code moved into separate
files, and it should get compiled on it's own, and should
pass regression. Also, we use spaces, and not tabs in the
code. So we will need to get those changed too. Thanks.<br></div>
<div><br></div>
<div>Regards,<br></div>
<div>Avra<br></div>
<div><br></div>
<div>On 10/12/2016 10:46 PM, <a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>Could you let me know on the below request? <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Tue, Oct 4, 2016, at 11:16 AM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Hi Avra,<br></div>
<div><br></div>
<div>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. <br></div>
<div><br></div>
<div>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). <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Mon, Oct 3, 2016, at 03:54 PM, Avra Sengupta wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>Regards,<br></div>
<div>Avra<br></div>
<div><br></div>
<div>On 09/30/2016 10:24 AM, <a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>Could you have a look into the below request? <br></div>
<div><br></div>
<div>Sriram <br></div>
<div><br></div>
<div><br></div>
<div>On Fri, Sep 23, 2016, at 04:10 PM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>Have submitted the patches for Modularizing
snapshot, <br></div>
<div><br></div>
<div><a href="https://bugzilla.redhat.com/show_bug.cgi?id=1377437">https://bugzilla.redhat.com/show_bug.cgi?id=1377437</a><br></div>
<div><br></div>
<div>This is the patch set: <br></div>
<div><br></div>
<div> <a href="http://review.gluster.org/15554">http://review.gluster.org/15554</a> This patch follows the discussion from the
gluster-devel mail chain of, ...<br></div>
<div> <a href="http://review.gluster.org/15555">http://review.gluster.org/15555</a> Referring to bugID:1377437, Modularizing snapshot
for plugin based modules.<br></div>
<div> <a href="http://review.gluster.org/15556">http://review.gluster.org/15556</a> - This is third patch in the series for the
bug=1377437<br></div>
<div> <a href="http://review.gluster.org/15557">http://review.gluster.org/15557</a> [BugId:1377437][Patch4]: Refering to the bug ID,<br></div>
<div> <a href="http://review.gluster.org/15558">http://review.gluster.org/15558</a> [BugId:1377437][Patch5]: Refering to the bug ID,<br></div>
<div> <a href="http://review.gluster.org/15559">http://review.gluster.org/15559</a> [BugId:1377437][Patch6]: Refering to the bug ID,<br></div>
<div> <a href="http://review.gluster.org/15560">http://review.gluster.org/15560</a> [BugId:1377437][Patch7]: Refering to the bug ID. *
This patch has some minor ...<br></div>
<div> <a href="http://review.gluster.org/15561">http://review.gluster.org/15561</a> [BugId:1377437][Patch8]: Refering to the bug ID,
this commit has minor fixes ...<br></div>
<div> <a href="http://review.gluster.org/15562">http://review.gluster.org/15562</a> [BugId:1377437][Patch9]: Refering to the bug ID, -
Minor header file ...<br></div>
<div><br></div>
<div>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<br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div>On Mon, Sep 19, 2016, at 10:58 PM, Avra Sengupta
wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div><br></div>
<div>I have created a bug for this (<a href="https://bugzilla.redhat.com/show_bug.cgi?id=1377437"></a><a href="https://bugzilla.redhat.com/show_bug.cgi?id=1377437">https://bugzilla.redhat.com/show_bug.cgi?id=1377437</a>).
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. <br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>Regards,<br></div>
<div>Avra<br></div>
<div><br></div>
<div>On 09/19/2016 07:52 PM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>Do you have a bug id for this changes? Or may
I raise a new one? <br></div>
<div><br></div>
<div>Sriram <br></div>
<div><br></div>
<div><br></div>
<div>On Fri, Sep 16, 2016, at 11:37 AM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Thanks Avra, <br></div>
<div><br></div>
<div>I'll send this patch to gluster master in a
while. <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Wed, Sep 14, 2016, at 03:08 PM, Avra
Sengupta wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div><br></div>
<div>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:<br></div>
<div><br></div>
<div><a href="https://github.com/sriramster/glusterfs/commit/7bf157525539541ebf0aa36a380bbedb2cae5440">https://github.com/sriramster/glusterfs/commit/7bf157525539541ebf0aa36a380bbedb2cae5440</a><br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>Regards,<br></div>
<div>Avra<br></div>
<div><br></div>
<div>P.S : Adding gluster-devel<br></div>
<div><br></div>
<div>On 09/13/2016 01:14 AM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>You'd time to look into the below
request?<br></div>
<div><br></div>
<div>Sriram <br></div>
<div><br></div>
<div><br></div>
<div>On Thu, Sep 8, 2016, at 01:20 PM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Hi Avra, <br></div>
<div><br></div>
<div>Thank you. Please, let me know your
feedback. It would be helpful on
continuing from then. <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Thu, Sep 8, 2016, at 01:18 PM,
Avra Sengupta wrote:<br></div>
<blockquote type="cite"><div><div>Hi Sriram,<br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>Regards,<br></div>
<div>Avra<br></div>
<div><br></div>
<div>On 09/08/2016 01:09 PM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite"><div>Hello Rajesh, <br></div>
<div><br></div>
<div>Sorry to bother. Could you have a
look at the below request? <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Tue, Sep 6, 2016, at 11:27 AM, <a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Hello Rajesh, <br></div>
<div><br></div>
<div>Sorry for the delayed mail, was
on leave. Could you let me know
the feedback? <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div><br></div>
<div>On Fri, Sep 2, 2016, at 10:08
AM, Rajesh Joseph wrote:<br></div>
<blockquote type="cite"><div dir="ltr"><div><div><div><div>+ Avra<br></div>
</div>
<div>Hi Srirram,<br></div>
<div><br></div>
</div>
<div>Sorry, I was on leave
therefore could not reply. <br></div>
</div>
<div><div>Added Avra who is also
working on the snapshot
component for review. <br></div>
</div>
<div><div>Will take a look at your
changes today.<br></div>
</div>
<div>Thanks & Regards,<br></div>
<div>Rajesh<br></div>
<div><br></div>
<div><div><div><div><div><div><div><div><br></div>
<div><div>On Thu, Sep
1, 2016 at
1:22 PM, <span dir="ltr"><<a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a>></span> wrote:<br></div>
<blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204, 204, 204);border-left-style:solid;padding-left:1ex;"><div><br></div>
<div><div>Hello
Rajesh, <br></div>
<div><br></div>
<div>Could
you've a look
at the below
request? <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><div><div><br></div>
<div>On Tue,
Aug 30, 2016,
at 01:03 PM, <a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a> wrote:<br></div>
<blockquote type="cite"><div>Hi
Rajesh, <br></div>
<div><br></div>
<div>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? <br></div>
<div><br></div>
<div><a href="https://github.com/sriramster/"></a><a href="https://github.com/sriramster/">https://github.com/sriramster/</a><wbr>glusterfs/tree/sriram_dev<br></div>
<div><br></div>
<div>Things
done, <br></div>
<div><br></div>
<div>- Created
a new folder
for glusterd
plugins and
added snapshot
as a plugin.
Like this, <br></div>
<div><br></div>
<div>$ROOT/xlators/mgmt/glusterd/<wbr>plugins
+<br></div>
<div> <wbr> <wbr>
|<br></div>
<div> <wbr> <wbr>
+ __
snapshot/src<br></div>
<div><br></div>
<div>Moved LVM
related
snapshot
implementation
to <br></div>
<div>xlators/mgmt/glusterd/plugins/<wbr>snapshot/src/lvm-snapshot.c <br></div>
<div><br></div>
<div>- Mostly
isolated,
glusterd code
from snapshot
implementation
by using
logging, error
codes and
messages from
glusterd and
libglusterfs. <br></div>
<div>- This
way, i though
we could get
complete
isolation of
snapshot
plugin
implementation
which avoids
most of
compiler and
linking
dependency
issues. <br></div>
<div>- Created
a library of
the above like
libgsnapshot.so
and linking it
with
glusterd.so to
get this
working. <br></div>
<div>- The
complete
isolation also
makes us to
avoid reverse
dependency
like some
api's inside
plugin/snapshot
being
dependent on
glusterd.so <br></div>
<div><br></div>
<div>TODO's : <br></div>
<div><br></div>
<div>- Need to
create
glusterd_snapshot_ops
structure
which would be
used to
register
snapshot
related API's
with
glusterd.so. <br></div>
<div>- Add
command line
snapshot
plugin option,
so that it
picks up on
compilation.<br></div>
<div>- If any
missed
implementation
for plugin. <br></div>
<div>- Cleanup
and get a
review ready
branch. <br></div>
<div><br></div>
<div>Let me
know if this
looks ok? Or
need to any
more into the
list. <br></div>
<div><br></div>
<div>Sriram<br></div>
<div><br></div>
<div>On Fri,
Jul 22, 2016,
at 02:43 PM,
Rajesh Joseph
wrote:<br></div>
<blockquote type="cite"><div dir="ltr"><div><br></div>
<div><div><br></div>
<div><div>On Thu,
Jul 21, 2016
at 3:07 AM,
Vijay Bellur <span dir="ltr"><<a href="mailto:vbellur@redhat.com"></a><a href="mailto:vbellur@redhat.com">vbellur@redhat.com</a>></span> wrote:<br></div>
<blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div><span>On
07/19/2016
11:01 AM, Atin
Mukherjee
wrote:</span><br></div>
<blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div><span><br><br>On Tue, Jul
19, 2016 at
7:29 PM,
Rajesh Joseph
<<a href="mailto:rjoseph@redhat.com"></a><a href="mailto:rjoseph@redhat.com">rjoseph@redhat.com</a><br> </span><span> <mailto:<a href="mailto:rjoseph@redhat.com"></a><a href="mailto:rjoseph@redhat.com">rjoseph@redhat.com</a>>>
wrote:<br> <br> <br> <br> On Tue, Jul
19, 2016 at
11:23 AM, <<a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a><br> </span> <mailto:<a href="mailto:sriram@marirs.net.in"></a><a href="mailto:sriram@marirs.net.in">sriram@marirs.net.in</a>>>
wrote:</div>
<div><br></div>
<div>__<span><br> Hi Rajesh,<br> <br> I'd thought
about moving
the zfs
specific
implementation
to<br> something like<br> <br> xlators/mgmt/glusterd/src/<wbr>plugins/zfs-specifs-stuffs for the<br> inital go.
Could you let
me know if
this works or
in sync with<br> what you'd
thought about?<br> <br> Sriram<br> <br> <br> Hi Sriram,<br> <br> Sorry, I was
not able to
send much time
on this. I
would prefer
you<br> move the code
to<br> <br> xlators/mgmt/glusterd/plugins/<wbr>src/zfs-specifs-stuffs<br> <br> <br> <br> How about
having it
under<br> xlators/mgmt/glusterd/plugins/<wbr>snapshot/src/zfs-specifs-<wbr>stuffs
such that<br> in future if
we have to
write plugins
for other
features they
can be<br> segregated?</span></div>
</blockquote><div><br></div>
<div>It would
be nicer to
avoid
"specific-stuff"
or similar
from the
naming. We can
probably leave
it at
xlators/mgmt/glusterd/plugins/<wbr>snapshot/src/zfs.
The naming
would be
sufficient to
indicate that
code is
specific to
zfs snapshots.<span><span class="colour" style="color:rgb(136, 136, 136)"></span></span><br></div>
</blockquote><div><br></div>
<div>I don't
think the
directory
would be named
"zfs-specific_stuffs,
instead zfs
specific
source file
will come
directly under
"xlato<span>rs/mgmt/glusterd/<wbr>plugins/snapshot/src/".
I think I
should have
been more
clear, my bad.</span><br></div>
<div><span>-Rajesh</span><br></div>
</div>
</div>
</div>
<div><u>______________________________<wbr>_________________</u><br></div>
<div>Gluster-devel
mailing list<br></div>
<div><a href="mailto:Gluster-devel@gluster.org"></a><a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br></div>
<div><a href="http://www.gluster.org/"></a><a href="http://www.gluster.org/">http://www.gluster.org/</a><wbr>mailman/listinfo/gluster-devel<br></div>
</blockquote><div><br></div>
</blockquote><div><br></div>
</div>
</div>
</div>
</blockquote></div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote><div><br></div>
</blockquote><div><br></div>
</blockquote></blockquote><div><br></div>
</blockquote></blockquote></blockquote><div><br></div>
<div><u>_______________________________________________</u><br></div>
<div>Gluster-devel mailing list<br></div>
<div><a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br></div>
<div><a href="http://www.gluster.org/mailman/listinfo/gluster-devel">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br></div>
</blockquote><div><br></div>
</blockquote></blockquote><div><br></div>
<div><u>_______________________________________________</u><br></div>
<div>Gluster-devel mailing list<br></div>
<div><a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br></div>
<div><a href="http://www.gluster.org/mailman/listinfo/gluster-devel">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br></div>
</blockquote><div><br></div>
</blockquote></blockquote><div><br></div>
<div><u>_______________________________________________</u><br></div>
<div>Gluster-devel mailing list<br></div>
<div><a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a><br></div>
<div><a href="http://www.gluster.org/mailman/listinfo/gluster-devel">http://www.gluster.org/mailman/listinfo/gluster-devel</a><br></div>
</blockquote><div><br></div>
</blockquote></blockquote><div><br></div>
</blockquote></blockquote><div><br></div>
</body>
</html>