<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><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" cite="mid:1475211290.4156626.741550609.6D58EF37@webmail.messagingengine.com"><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">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">sriram@marirs.net.in</a> wrote:<br></div>
</div>
<blockquote type="cite" cite="mid:1474294942.1405059.730207729.7F842F25@webmail.messagingengine.com"><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" cite="mid:1473709497.1969417.723495857.69EC675F@webmail.messagingengine.com"><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" cite="mid:1473320374.1840238.719279817.61F4BD92@webmail.messagingengine.com"><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"></a><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 style="color:rgb(136, 136, 136)" class="colour"></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>
</body>
</html>