Hi Sriram,
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.
http://review.gluster.org/#/c/15554/
http://review.gluster.org/#/c/15555/
http://review.gluster.org/#/c/15556/
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.
Regards,
Avra
On 10/12/2016 10:46 PM, sriram@xxxxxxxxxxxxx wrote:
Hi Avra,
Could you let me know on the below request?
Sriram
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
Hi Avra,
Could you have a look into the below request?
Sriram
Hi Avra,
Have submitted the patches for Modularizing snapshot,
This is the patch set:
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
Hi Avra,
Do you have a bug id for this changes? Or may I
raise a new one?
Sriram
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:
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
Hi Avra,
You'd time to look into the below request?
Sriram
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
Hello Rajesh,
Sorry to bother. Could you have a
look at the below request?
Sriram
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:
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
Hello
Rajesh,
Could
you've a look
at the below
request?
Sriram
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?
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
07/19/2016
11:01 AM, Atin
Mukherjee
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 mailing list
_______________________________________________
Gluster-devel mailing list
_______________________________________________
Gluster-devel mailing list
|
