wip-addr-features

wip-addr-features

[Sage Weil]

Hi Zhao,

We identified wip-addr as a desireable first step to fixing up the 
messenger protocol.  It has a bunch of other nice benefits as well, like 
being able to support both IPv4 and IPv6 in the same cluster.

I started rebasing the branch this morning ran into a bunch of non-trivial 
questions about how we should structure the entity_addr_t to support both 
new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire 
protocol.  But this is actually step 2...

Step 1 is to just get the feature bits plumbed down to every bit of code 
that encodes an entity_addr_t to send over the wire so that we will 
know whether to encode using the new or legacy format.  To do that, I 
pushed a simplified branch, wip-addr-features, to

	https://github.com/liewegas/ceph/commits/wip-addr-features

The are some prelim encoding patches, then a patch that makes 
entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's 
call it optional-features-patch.  This compiles cleanly, and allows either 
an existing featureless encode

  entity_addr_t foo;
  ::encode(foo, bl);             // old way we need to eliminate

or a featured encode

  entity_addr_t foo;
  ::encode(foo, bl, features);   // what we want

compile.

Then there's a patch that makes the old way not compile.  Let's call it 
required-features-patch.  This will make the compiler spit out a million 
errors for all the call sites that still need to be fixed.

Finally, there is a 'wip' commit that fixes some but not all call sites.  
This commit needs to be broken down into simple, individual changes that 
can be tested and reviewed separately.

The idea is to work with the require-features-patch applied to identify 
remaining call sites that need features to encode entity_addr_t.  Create 
small, contained patches that fix individual modules, call sites, or add 
supporting infrastructure (like the changes that expose features to OSD 
cls ops in objclass/*).

Once there are several of those, remove the require-features-patch, and 
make sure the changes are safe and clean and everything still works 
properly.  We can merge these as we go to make incremental progress.

Finally, once *everything* is fixed to pass features to addr encode sites, 
the last commit will be the require-features-patch, and we can get it all 
in master.

That will lay the foundation so that we can change the entity_addr_t 
encoding to support the new protocols and encoding, the entity_addrvec_t 
type, and so on.

(The plan is to replace most instnaces of entity_addr_t with 
entity_addrvec_t--a list of addresses instead of a single one.  And make 
entity_addrvec_t encoded with old features spit out the legacy addr with 
the legacy entity_addr_t encoding so that old clients can still function.)

We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there 
or on this email thread.  Or we can do another video conf session to go 
through it.

Thanks!
sage

Re: wip-addr-features

[Junwang Zhao]

Hi Sage,

This weekend, I read the wip-addr-features patches in your repo and
the related wip-addr patches in ceph repo, I understand that my task
is to make the required-features-patch work by breaking done the 'wip'
into small patches to make the required-features-patch finally work.
I got one question, when I applied a small individual patch, how can I
know it works, will 'make' tell(from the errors)?

btw, I set up an branch, and my work will be pushed there:
https://github.com/zhjwpku/ceph/commits/wip-addr-zhjw

Cheers!
Zhao

On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
> Hi Zhao,
>
> We identified wip-addr as a desireable first step to fixing up the
> messenger protocol.  It has a bunch of other nice benefits as well, like
> being able to support both IPv4 and IPv6 in the same cluster.
>
> I started rebasing the branch this morning ran into a bunch of non-trivial
> questions about how we should structure the entity_addr_t to support both
> new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
> protocol.  But this is actually step 2...
>
> Step 1 is to just get the feature bits plumbed down to every bit of code
> that encodes an entity_addr_t to send over the wire so that we will
> know whether to encode using the new or legacy format.  To do that, I
> pushed a simplified branch, wip-addr-features, to
>
>         https://github.com/liewegas/ceph/commits/wip-addr-features
>
> The are some prelim encoding patches, then a patch that makes
> entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
> call it optional-features-patch.  This compiles cleanly, and allows either
> an existing featureless encode
>
>   entity_addr_t foo;
>   ::encode(foo, bl);             // old way we need to eliminate
>
> or a featured encode
>
>   entity_addr_t foo;
>   ::encode(foo, bl, features);   // what we want
>
> compile.
>
> Then there's a patch that makes the old way not compile.  Let's call it
> required-features-patch.  This will make the compiler spit out a million
> errors for all the call sites that still need to be fixed.
>
> Finally, there is a 'wip' commit that fixes some but not all call sites.
> This commit needs to be broken down into simple, individual changes that
> can be tested and reviewed separately.
>
> The idea is to work with the require-features-patch applied to identify
> remaining call sites that need features to encode entity_addr_t.  Create
> small, contained patches that fix individual modules, call sites, or add
> supporting infrastructure (like the changes that expose features to OSD
> cls ops in objclass/*).
>
> Once there are several of those, remove the require-features-patch, and
> make sure the changes are safe and clean and everything still works
> properly.  We can merge these as we go to make incremental progress.
>
> Finally, once *everything* is fixed to pass features to addr encode sites,
> the last commit will be the require-features-patch, and we can get it all
> in master.
>
> That will lay the foundation so that we can change the entity_addr_t
> encoding to support the new protocols and encoding, the entity_addrvec_t
> type, and so on.
>
> (The plan is to replace most instnaces of entity_addr_t with
> entity_addrvec_t--a list of addresses instead of a single one.  And make
> entity_addrvec_t encoded with old features spit out the legacy addr with
> the legacy entity_addr_t encoding so that old clients can still function.)
>
> We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
> or on this email thread.  Or we can do another video conf session to go
> through it.
>
> Thanks!
> sage

Re: wip-addr-features

[Sage Weil]

Hi Junwang,

On Sun, 8 May 2016, Junwang Zhao wrote:
> This weekend, I read the wip-addr-features patches in your repo and
> the related wip-addr patches in ceph repo, I understand that my task
> is to make the required-features-patch work by breaking done the 'wip'
> into small patches to make the required-features-patch finally work.
> I got one question, when I applied a small individual patch, how can I
> know it works, will 'make' tell(from the errors)?

'make' is the first barrier.  Making sure 'make check' passes would be 
the next step.  You might want to make sure this passes on your machine 
before making any changes, as it can be somewhat sensitive to the 
environment.

Breaking up the current changes into a patch series is a start.  I would 
start with that and check in (ping us in #ceph-devel).  There are also 
still a lot of remaining places where we don't have features yet and the 
code still doesn't compile.  Usually it's a matter of adding arguments to 
a chain of calling functions.  If you're not sure how to proceed check 
with us on IRC.

Thanks!
sage

> 
> Cheers!
> Zhao
> 
> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
> > Hi Zhao,
> >
> > We identified wip-addr as a desireable first step to fixing up the
> > messenger protocol.  It has a bunch of other nice benefits as well, like
> > being able to support both IPv4 and IPv6 in the same cluster.
> >
> > I started rebasing the branch this morning ran into a bunch of non-trivial
> > questions about how we should structure the entity_addr_t to support both
> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
> > protocol.  But this is actually step 2...
> >
> > Step 1 is to just get the feature bits plumbed down to every bit of code
> > that encodes an entity_addr_t to send over the wire so that we will
> > know whether to encode using the new or legacy format.  To do that, I
> > pushed a simplified branch, wip-addr-features, to
> >
> >         https://github.com/liewegas/ceph/commits/wip-addr-features
> >
> > The are some prelim encoding patches, then a patch that makes
> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
> > call it optional-features-patch.  This compiles cleanly, and allows either
> > an existing featureless encode
> >
> >   entity_addr_t foo;
> >   ::encode(foo, bl);             // old way we need to eliminate
> >
> > or a featured encode
> >
> >   entity_addr_t foo;
> >   ::encode(foo, bl, features);   // what we want
> >
> > compile.
> >
> > Then there's a patch that makes the old way not compile.  Let's call it
> > required-features-patch.  This will make the compiler spit out a million
> > errors for all the call sites that still need to be fixed.
> >
> > Finally, there is a 'wip' commit that fixes some but not all call sites.
> > This commit needs to be broken down into simple, individual changes that
> > can be tested and reviewed separately.
> >
> > The idea is to work with the require-features-patch applied to identify
> > remaining call sites that need features to encode entity_addr_t.  Create
> > small, contained patches that fix individual modules, call sites, or add
> > supporting infrastructure (like the changes that expose features to OSD
> > cls ops in objclass/*).
> >
> > Once there are several of those, remove the require-features-patch, and
> > make sure the changes are safe and clean and everything still works
> > properly.  We can merge these as we go to make incremental progress.
> >
> > Finally, once *everything* is fixed to pass features to addr encode sites,
> > the last commit will be the require-features-patch, and we can get it all
> > in master.
> >
> > That will lay the foundation so that we can change the entity_addr_t
> > encoding to support the new protocols and encoding, the entity_addrvec_t
> > type, and so on.
> >
> > (The plan is to replace most instnaces of entity_addr_t with
> > entity_addrvec_t--a list of addresses instead of a single one.  And make
> > entity_addrvec_t encoded with old features spit out the legacy addr with
> > the legacy entity_addr_t encoding so that old clients can still function.)
> >
> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
> > or on this email thread.  Or we can do another video conf session to go
> > through it.
> >
> > Thanks!
> > sage
> 
> 

Re: wip-addr-features

[Junwang Zhao]

I am working on it, the complie is quite slow, I will ping you tomorrow.

BR,
Zhao

On Mon, May 9, 2016 at 8:20 PM, Sage Weil <sweil@redhat.com> wrote:
> Hi Junwang,
>
> On Sun, 8 May 2016, Junwang Zhao wrote:
>> This weekend, I read the wip-addr-features patches in your repo and
>> the related wip-addr patches in ceph repo, I understand that my task
>> is to make the required-features-patch work by breaking done the 'wip'
>> into small patches to make the required-features-patch finally work.
>> I got one question, when I applied a small individual patch, how can I
>> know it works, will 'make' tell(from the errors)?
>
> 'make' is the first barrier.  Making sure 'make check' passes would be
> the next step.  You might want to make sure this passes on your machine
> before making any changes, as it can be somewhat sensitive to the
> environment.
>
> Breaking up the current changes into a patch series is a start.  I would
> start with that and check in (ping us in #ceph-devel).  There are also
> still a lot of remaining places where we don't have features yet and the
> code still doesn't compile.  Usually it's a matter of adding arguments to
> a chain of calling functions.  If you're not sure how to proceed check
> with us on IRC.
>
> Thanks!
> sage
>
>>
>> Cheers!
>> Zhao
>>
>> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
>> > Hi Zhao,
>> >
>> > We identified wip-addr as a desireable first step to fixing up the
>> > messenger protocol.  It has a bunch of other nice benefits as well, like
>> > being able to support both IPv4 and IPv6 in the same cluster.
>> >
>> > I started rebasing the branch this morning ran into a bunch of non-trivial
>> > questions about how we should structure the entity_addr_t to support both
>> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
>> > protocol.  But this is actually step 2...
>> >
>> > Step 1 is to just get the feature bits plumbed down to every bit of code
>> > that encodes an entity_addr_t to send over the wire so that we will
>> > know whether to encode using the new or legacy format.  To do that, I
>> > pushed a simplified branch, wip-addr-features, to
>> >
>> >         https://github.com/liewegas/ceph/commits/wip-addr-features
>> >
>> > The are some prelim encoding patches, then a patch that makes
>> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
>> > call it optional-features-patch.  This compiles cleanly, and allows either
>> > an existing featureless encode
>> >
>> >   entity_addr_t foo;
>> >   ::encode(foo, bl);             // old way we need to eliminate
>> >
>> > or a featured encode
>> >
>> >   entity_addr_t foo;
>> >   ::encode(foo, bl, features);   // what we want
>> >
>> > compile.
>> >
>> > Then there's a patch that makes the old way not compile.  Let's call it
>> > required-features-patch.  This will make the compiler spit out a million
>> > errors for all the call sites that still need to be fixed.
>> >
>> > Finally, there is a 'wip' commit that fixes some but not all call sites.
>> > This commit needs to be broken down into simple, individual changes that
>> > can be tested and reviewed separately.
>> >
>> > The idea is to work with the require-features-patch applied to identify
>> > remaining call sites that need features to encode entity_addr_t.  Create
>> > small, contained patches that fix individual modules, call sites, or add
>> > supporting infrastructure (like the changes that expose features to OSD
>> > cls ops in objclass/*).
>> >
>> > Once there are several of those, remove the require-features-patch, and
>> > make sure the changes are safe and clean and everything still works
>> > properly.  We can merge these as we go to make incremental progress.
>> >
>> > Finally, once *everything* is fixed to pass features to addr encode sites,
>> > the last commit will be the require-features-patch, and we can get it all
>> > in master.
>> >
>> > That will lay the foundation so that we can change the entity_addr_t
>> > encoding to support the new protocols and encoding, the entity_addrvec_t
>> > type, and so on.
>> >
>> > (The plan is to replace most instnaces of entity_addr_t with
>> > entity_addrvec_t--a list of addresses instead of a single one.  And make
>> > entity_addrvec_t encoded with old features spit out the legacy addr with
>> > the legacy entity_addr_t encoding so that old clients can still function.)
>> >
>> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
>> > or on this email thread.  Or we can do another video conf session to go
>> > through it.
>> >
>> > Thanks!
>> > sage
>>
>>

Re: wip-addr-features

[Junwang Zhao]

Hi Sage,

Check this:
https://github.com/zhjwpku/ceph/commit/2dddc32eb18c2a065963ae33674cb2c8a9e64dc5

The original patch doesn't compile, I add dump and
generate_test_instances, now it passed
the 'make', I am running the 'make check'.

Not sure did I do it right, please give some feedback.

Thanks!

On Mon, May 9, 2016 at 10:14 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
> I am working on it, the complie is quite slow, I will ping you tomorrow.
>
> BR,
> Zhao
>
> On Mon, May 9, 2016 at 8:20 PM, Sage Weil <sweil@redhat.com> wrote:
>> Hi Junwang,
>>
>> On Sun, 8 May 2016, Junwang Zhao wrote:
>>> This weekend, I read the wip-addr-features patches in your repo and
>>> the related wip-addr patches in ceph repo, I understand that my task
>>> is to make the required-features-patch work by breaking done the 'wip'
>>> into small patches to make the required-features-patch finally work.
>>> I got one question, when I applied a small individual patch, how can I
>>> know it works, will 'make' tell(from the errors)?
>>
>> 'make' is the first barrier.  Making sure 'make check' passes would be
>> the next step.  You might want to make sure this passes on your machine
>> before making any changes, as it can be somewhat sensitive to the
>> environment.
>>
>> Breaking up the current changes into a patch series is a start.  I would
>> start with that and check in (ping us in #ceph-devel).  There are also
>> still a lot of remaining places where we don't have features yet and the
>> code still doesn't compile.  Usually it's a matter of adding arguments to
>> a chain of calling functions.  If you're not sure how to proceed check
>> with us on IRC.
>>
>> Thanks!
>> sage
>>
>>>
>>> Cheers!
>>> Zhao
>>>
>>> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
>>> > Hi Zhao,
>>> >
>>> > We identified wip-addr as a desireable first step to fixing up the
>>> > messenger protocol.  It has a bunch of other nice benefits as well, like
>>> > being able to support both IPv4 and IPv6 in the same cluster.
>>> >
>>> > I started rebasing the branch this morning ran into a bunch of non-trivial
>>> > questions about how we should structure the entity_addr_t to support both
>>> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
>>> > protocol.  But this is actually step 2...
>>> >
>>> > Step 1 is to just get the feature bits plumbed down to every bit of code
>>> > that encodes an entity_addr_t to send over the wire so that we will
>>> > know whether to encode using the new or legacy format.  To do that, I
>>> > pushed a simplified branch, wip-addr-features, to
>>> >
>>> >         https://github.com/liewegas/ceph/commits/wip-addr-features
>>> >
>>> > The are some prelim encoding patches, then a patch that makes
>>> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
>>> > call it optional-features-patch.  This compiles cleanly, and allows either
>>> > an existing featureless encode
>>> >
>>> >   entity_addr_t foo;
>>> >   ::encode(foo, bl);             // old way we need to eliminate
>>> >
>>> > or a featured encode
>>> >
>>> >   entity_addr_t foo;
>>> >   ::encode(foo, bl, features);   // what we want
>>> >
>>> > compile.
>>> >
>>> > Then there's a patch that makes the old way not compile.  Let's call it
>>> > required-features-patch.  This will make the compiler spit out a million
>>> > errors for all the call sites that still need to be fixed.
>>> >
>>> > Finally, there is a 'wip' commit that fixes some but not all call sites.
>>> > This commit needs to be broken down into simple, individual changes that
>>> > can be tested and reviewed separately.
>>> >
>>> > The idea is to work with the require-features-patch applied to identify
>>> > remaining call sites that need features to encode entity_addr_t.  Create
>>> > small, contained patches that fix individual modules, call sites, or add
>>> > supporting infrastructure (like the changes that expose features to OSD
>>> > cls ops in objclass/*).
>>> >
>>> > Once there are several of those, remove the require-features-patch, and
>>> > make sure the changes are safe and clean and everything still works
>>> > properly.  We can merge these as we go to make incremental progress.
>>> >
>>> > Finally, once *everything* is fixed to pass features to addr encode sites,
>>> > the last commit will be the require-features-patch, and we can get it all
>>> > in master.
>>> >
>>> > That will lay the foundation so that we can change the entity_addr_t
>>> > encoding to support the new protocols and encoding, the entity_addrvec_t
>>> > type, and so on.
>>> >
>>> > (The plan is to replace most instnaces of entity_addr_t with
>>> > entity_addrvec_t--a list of addresses instead of a single one.  And make
>>> > entity_addrvec_t encoded with old features spit out the legacy addr with
>>> > the legacy entity_addr_t encoding so that old clients can still function.)
>>> >
>>> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
>>> > or on this email thread.  Or we can do another video conf session to go
>>> > through it.
>>> >
>>> > Thanks!
>>> > sage
>>>
>>>

Re: wip-addr-features

[Junwang Zhao]

Hi Sage, all,

I run "make check" and got two FAIL, one is unittest_chain_xattr, which I once
came across, I remeber haomai said this was because the selinux, and the
other fail is test/test_pidfile.sh, I checkout [1] and run "make && make check",
got the same result, so I think that's not because my applied patch, maybe
the master branch got the same problem?

Hope somebody could share some tips.

[1] https://github.com/liewegas/ceph/commit/35f1077c3afd016d54707f9fe4de8c312eb47c67

Cheers,
Zhao

On Tue, May 10, 2016 at 9:07 AM, Junwang Zhao <zhjwpku@gmail.com> wrote:
> Hi Sage,
>
> Check this:
> https://github.com/zhjwpku/ceph/commit/2dddc32eb18c2a065963ae33674cb2c8a9e64dc5
>
> The original patch doesn't compile, I add dump and
> generate_test_instances, now it passed
> the 'make', I am running the 'make check'.
>
> Not sure did I do it right, please give some feedback.
>
> Thanks!
>
> On Mon, May 9, 2016 at 10:14 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>> I am working on it, the complie is quite slow, I will ping you tomorrow.
>>
>> BR,
>> Zhao
>>
>> On Mon, May 9, 2016 at 8:20 PM, Sage Weil <sweil@redhat.com> wrote:
>>> Hi Junwang,
>>>
>>> On Sun, 8 May 2016, Junwang Zhao wrote:
>>>> This weekend, I read the wip-addr-features patches in your repo and
>>>> the related wip-addr patches in ceph repo, I understand that my task
>>>> is to make the required-features-patch work by breaking done the 'wip'
>>>> into small patches to make the required-features-patch finally work.
>>>> I got one question, when I applied a small individual patch, how can I
>>>> know it works, will 'make' tell(from the errors)?
>>>
>>> 'make' is the first barrier.  Making sure 'make check' passes would be
>>> the next step.  You might want to make sure this passes on your machine
>>> before making any changes, as it can be somewhat sensitive to the
>>> environment.
>>>
>>> Breaking up the current changes into a patch series is a start.  I would
>>> start with that and check in (ping us in #ceph-devel).  There are also
>>> still a lot of remaining places where we don't have features yet and the
>>> code still doesn't compile.  Usually it's a matter of adding arguments to
>>> a chain of calling functions.  If you're not sure how to proceed check
>>> with us on IRC.
>>>
>>> Thanks!
>>> sage
>>>
>>>>
>>>> Cheers!
>>>> Zhao
>>>>
>>>> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
>>>> > Hi Zhao,
>>>> >
>>>> > We identified wip-addr as a desireable first step to fixing up the
>>>> > messenger protocol.  It has a bunch of other nice benefits as well, like
>>>> > being able to support both IPv4 and IPv6 in the same cluster.
>>>> >
>>>> > I started rebasing the branch this morning ran into a bunch of non-trivial
>>>> > questions about how we should structure the entity_addr_t to support both
>>>> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
>>>> > protocol.  But this is actually step 2...
>>>> >
>>>> > Step 1 is to just get the feature bits plumbed down to every bit of code
>>>> > that encodes an entity_addr_t to send over the wire so that we will
>>>> > know whether to encode using the new or legacy format.  To do that, I
>>>> > pushed a simplified branch, wip-addr-features, to
>>>> >
>>>> >         https://github.com/liewegas/ceph/commits/wip-addr-features
>>>> >
>>>> > The are some prelim encoding patches, then a patch that makes
>>>> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
>>>> > call it optional-features-patch.  This compiles cleanly, and allows either
>>>> > an existing featureless encode
>>>> >
>>>> >   entity_addr_t foo;
>>>> >   ::encode(foo, bl);             // old way we need to eliminate
>>>> >
>>>> > or a featured encode
>>>> >
>>>> >   entity_addr_t foo;
>>>> >   ::encode(foo, bl, features);   // what we want
>>>> >
>>>> > compile.
>>>> >
>>>> > Then there's a patch that makes the old way not compile.  Let's call it
>>>> > required-features-patch.  This will make the compiler spit out a million
>>>> > errors for all the call sites that still need to be fixed.
>>>> >
>>>> > Finally, there is a 'wip' commit that fixes some but not all call sites.
>>>> > This commit needs to be broken down into simple, individual changes that
>>>> > can be tested and reviewed separately.
>>>> >
>>>> > The idea is to work with the require-features-patch applied to identify
>>>> > remaining call sites that need features to encode entity_addr_t.  Create
>>>> > small, contained patches that fix individual modules, call sites, or add
>>>> > supporting infrastructure (like the changes that expose features to OSD
>>>> > cls ops in objclass/*).
>>>> >
>>>> > Once there are several of those, remove the require-features-patch, and
>>>> > make sure the changes are safe and clean and everything still works
>>>> > properly.  We can merge these as we go to make incremental progress.
>>>> >
>>>> > Finally, once *everything* is fixed to pass features to addr encode sites,
>>>> > the last commit will be the require-features-patch, and we can get it all
>>>> > in master.
>>>> >
>>>> > That will lay the foundation so that we can change the entity_addr_t
>>>> > encoding to support the new protocols and encoding, the entity_addrvec_t
>>>> > type, and so on.
>>>> >
>>>> > (The plan is to replace most instnaces of entity_addr_t with
>>>> > entity_addrvec_t--a list of addresses instead of a single one.  And make
>>>> > entity_addrvec_t encoded with old features spit out the legacy addr with
>>>> > the legacy entity_addr_t encoding so that old clients can still function.)
>>>> >
>>>> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
>>>> > or on this email thread.  Or we can do another video conf session to go
>>>> > through it.
>>>> >
>>>> > Thanks!
>>>> > sage
>>>>
>>>>

Re: wip-addr-features

[Haomai Wang]

On Tue, May 10, 2016 at 7:34 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
> Hi Sage, all,
>
> I run "make check" and got two FAIL, one is unittest_chain_xattr, which I once
> came across, I remeber haomai said this was because the selinux, and the
> other fail is test/test_pidfile.sh, I checkout [1] and run "make && make check",
> got the same result, so I think that's not because my applied patch, maybe
> the master branch got the same problem?

Do you mean backport this PR(https://github.com/ceph/ceph/pull/8290)?

>
> Hope somebody could share some tips.
>
> [1] https://github.com/liewegas/ceph/commit/35f1077c3afd016d54707f9fe4de8c312eb47c67
>
> Cheers,
> Zhao
>
> On Tue, May 10, 2016 at 9:07 AM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>> Hi Sage,
>>
>> Check this:
>> https://github.com/zhjwpku/ceph/commit/2dddc32eb18c2a065963ae33674cb2c8a9e64dc5
>>
>> The original patch doesn't compile, I add dump and
>> generate_test_instances, now it passed
>> the 'make', I am running the 'make check'.
>>
>> Not sure did I do it right, please give some feedback.
>>
>> Thanks!
>>
>> On Mon, May 9, 2016 at 10:14 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>>> I am working on it, the complie is quite slow, I will ping you tomorrow.
>>>
>>> BR,
>>> Zhao
>>>
>>> On Mon, May 9, 2016 at 8:20 PM, Sage Weil <sweil@redhat.com> wrote:
>>>> Hi Junwang,
>>>>
>>>> On Sun, 8 May 2016, Junwang Zhao wrote:
>>>>> This weekend, I read the wip-addr-features patches in your repo and
>>>>> the related wip-addr patches in ceph repo, I understand that my task
>>>>> is to make the required-features-patch work by breaking done the 'wip'
>>>>> into small patches to make the required-features-patch finally work.
>>>>> I got one question, when I applied a small individual patch, how can I
>>>>> know it works, will 'make' tell(from the errors)?
>>>>
>>>> 'make' is the first barrier.  Making sure 'make check' passes would be
>>>> the next step.  You might want to make sure this passes on your machine
>>>> before making any changes, as it can be somewhat sensitive to the
>>>> environment.
>>>>
>>>> Breaking up the current changes into a patch series is a start.  I would
>>>> start with that and check in (ping us in #ceph-devel).  There are also
>>>> still a lot of remaining places where we don't have features yet and the
>>>> code still doesn't compile.  Usually it's a matter of adding arguments to
>>>> a chain of calling functions.  If you're not sure how to proceed check
>>>> with us on IRC.
>>>>
>>>> Thanks!
>>>> sage
>>>>
>>>>>
>>>>> Cheers!
>>>>> Zhao
>>>>>
>>>>> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
>>>>> > Hi Zhao,
>>>>> >
>>>>> > We identified wip-addr as a desireable first step to fixing up the
>>>>> > messenger protocol.  It has a bunch of other nice benefits as well, like
>>>>> > being able to support both IPv4 and IPv6 in the same cluster.
>>>>> >
>>>>> > I started rebasing the branch this morning ran into a bunch of non-trivial
>>>>> > questions about how we should structure the entity_addr_t to support both
>>>>> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
>>>>> > protocol.  But this is actually step 2...
>>>>> >
>>>>> > Step 1 is to just get the feature bits plumbed down to every bit of code
>>>>> > that encodes an entity_addr_t to send over the wire so that we will
>>>>> > know whether to encode using the new or legacy format.  To do that, I
>>>>> > pushed a simplified branch, wip-addr-features, to
>>>>> >
>>>>> >         https://github.com/liewegas/ceph/commits/wip-addr-features
>>>>> >
>>>>> > The are some prelim encoding patches, then a patch that makes
>>>>> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
>>>>> > call it optional-features-patch.  This compiles cleanly, and allows either
>>>>> > an existing featureless encode
>>>>> >
>>>>> >   entity_addr_t foo;
>>>>> >   ::encode(foo, bl);             // old way we need to eliminate
>>>>> >
>>>>> > or a featured encode
>>>>> >
>>>>> >   entity_addr_t foo;
>>>>> >   ::encode(foo, bl, features);   // what we want
>>>>> >
>>>>> > compile.
>>>>> >
>>>>> > Then there's a patch that makes the old way not compile.  Let's call it
>>>>> > required-features-patch.  This will make the compiler spit out a million
>>>>> > errors for all the call sites that still need to be fixed.
>>>>> >
>>>>> > Finally, there is a 'wip' commit that fixes some but not all call sites.
>>>>> > This commit needs to be broken down into simple, individual changes that
>>>>> > can be tested and reviewed separately.
>>>>> >
>>>>> > The idea is to work with the require-features-patch applied to identify
>>>>> > remaining call sites that need features to encode entity_addr_t.  Create
>>>>> > small, contained patches that fix individual modules, call sites, or add
>>>>> > supporting infrastructure (like the changes that expose features to OSD
>>>>> > cls ops in objclass/*).
>>>>> >
>>>>> > Once there are several of those, remove the require-features-patch, and
>>>>> > make sure the changes are safe and clean and everything still works
>>>>> > properly.  We can merge these as we go to make incremental progress.
>>>>> >
>>>>> > Finally, once *everything* is fixed to pass features to addr encode sites,
>>>>> > the last commit will be the require-features-patch, and we can get it all
>>>>> > in master.
>>>>> >
>>>>> > That will lay the foundation so that we can change the entity_addr_t
>>>>> > encoding to support the new protocols and encoding, the entity_addrvec_t
>>>>> > type, and so on.
>>>>> >
>>>>> > (The plan is to replace most instnaces of entity_addr_t with
>>>>> > entity_addrvec_t--a list of addresses instead of a single one.  And make
>>>>> > entity_addrvec_t encoded with old features spit out the legacy addr with
>>>>> > the legacy entity_addr_t encoding so that old clients can still function.)
>>>>> >
>>>>> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
>>>>> > or on this email thread.  Or we can do another video conf session to go
>>>>> > through it.
>>>>> >
>>>>> > Thanks!
>>>>> > sage
>>>>>
>>>>>

Re: wip-addr-features

[Junwang Zhao]

Hi Haomai,
I will try that and report the result later.

On Tue, May 10, 2016 at 7:41 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
> Sorry for not applied the attachment :)
>
> On Tue, May 10, 2016 at 7:34 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>> Hi Sage, all,
>>
>> I run "make check" and got two FAIL, one is unittest_chain_xattr, which I once
>> came across, I remeber haomai said this was because the selinux, and the
>> other fail is test/test_pidfile.sh, I checkout [1] and run "make && make check",
>> got the same result, so I think that's not because my applied patch, maybe
>> the master branch got the same problem?
>>
>> Hope somebody could share some tips.
>>
>> [1] https://github.com/liewegas/ceph/commit/35f1077c3afd016d54707f9fe4de8c312eb47c67
>>
>> Cheers,
>> Zhao
>>
>> On Tue, May 10, 2016 at 9:07 AM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>>> Hi Sage,
>>>
>>> Check this:
>>> https://github.com/zhjwpku/ceph/commit/2dddc32eb18c2a065963ae33674cb2c8a9e64dc5
>>>
>>> The original patch doesn't compile, I add dump and
>>> generate_test_instances, now it passed
>>> the 'make', I am running the 'make check'.
>>>
>>> Not sure did I do it right, please give some feedback.
>>>
>>> Thanks!
>>>
>>> On Mon, May 9, 2016 at 10:14 PM, Junwang Zhao <zhjwpku@gmail.com> wrote:
>>>> I am working on it, the complie is quite slow, I will ping you tomorrow.
>>>>
>>>> BR,
>>>> Zhao
>>>>
>>>> On Mon, May 9, 2016 at 8:20 PM, Sage Weil <sweil@redhat.com> wrote:
>>>>> Hi Junwang,
>>>>>
>>>>> On Sun, 8 May 2016, Junwang Zhao wrote:
>>>>>> This weekend, I read the wip-addr-features patches in your repo and
>>>>>> the related wip-addr patches in ceph repo, I understand that my task
>>>>>> is to make the required-features-patch work by breaking done the 'wip'
>>>>>> into small patches to make the required-features-patch finally work.
>>>>>> I got one question, when I applied a small individual patch, how can I
>>>>>> know it works, will 'make' tell(from the errors)?
>>>>>
>>>>> 'make' is the first barrier.  Making sure 'make check' passes would be
>>>>> the next step.  You might want to make sure this passes on your machine
>>>>> before making any changes, as it can be somewhat sensitive to the
>>>>> environment.
>>>>>
>>>>> Breaking up the current changes into a patch series is a start.  I would
>>>>> start with that and check in (ping us in #ceph-devel).  There are also
>>>>> still a lot of remaining places where we don't have features yet and the
>>>>> code still doesn't compile.  Usually it's a matter of adding arguments to
>>>>> a chain of calling functions.  If you're not sure how to proceed check
>>>>> with us on IRC.
>>>>>
>>>>> Thanks!
>>>>> sage
>>>>>
>>>>>>
>>>>>> Cheers!
>>>>>> Zhao
>>>>>>
>>>>>> On Thu, May 5, 2016 at 11:41 PM, Sage Weil <sweil@redhat.com> wrote:
>>>>>> > Hi Zhao,
>>>>>> >
>>>>>> > We identified wip-addr as a desireable first step to fixing up the
>>>>>> > messenger protocol.  It has a bunch of other nice benefits as well, like
>>>>>> > being able to support both IPv4 and IPv6 in the same cluster.
>>>>>> >
>>>>>> > I started rebasing the branch this morning ran into a bunch of non-trivial
>>>>>> > questions about how we should structure the entity_addr_t to support both
>>>>>> > new address types (ipv4, ipv6, xio, etc.) *and* the a new on-wire
>>>>>> > protocol.  But this is actually step 2...
>>>>>> >
>>>>>> > Step 1 is to just get the feature bits plumbed down to every bit of code
>>>>>> > that encodes an entity_addr_t to send over the wire so that we will
>>>>>> > know whether to encode using the new or legacy format.  To do that, I
>>>>>> > pushed a simplified branch, wip-addr-features, to
>>>>>> >
>>>>>> >         https://github.com/liewegas/ceph/commits/wip-addr-features
>>>>>> >
>>>>>> > The are some prelim encoding patches, then a patch that makes
>>>>>> > entity_addr_t and entity_inst_t *optionally* accept feature bits.  Let's
>>>>>> > call it optional-features-patch.  This compiles cleanly, and allows either
>>>>>> > an existing featureless encode
>>>>>> >
>>>>>> >   entity_addr_t foo;
>>>>>> >   ::encode(foo, bl);             // old way we need to eliminate
>>>>>> >
>>>>>> > or a featured encode
>>>>>> >
>>>>>> >   entity_addr_t foo;
>>>>>> >   ::encode(foo, bl, features);   // what we want
>>>>>> >
>>>>>> > compile.
>>>>>> >
>>>>>> > Then there's a patch that makes the old way not compile.  Let's call it
>>>>>> > required-features-patch.  This will make the compiler spit out a million
>>>>>> > errors for all the call sites that still need to be fixed.
>>>>>> >
>>>>>> > Finally, there is a 'wip' commit that fixes some but not all call sites.
>>>>>> > This commit needs to be broken down into simple, individual changes that
>>>>>> > can be tested and reviewed separately.
>>>>>> >
>>>>>> > The idea is to work with the require-features-patch applied to identify
>>>>>> > remaining call sites that need features to encode entity_addr_t.  Create
>>>>>> > small, contained patches that fix individual modules, call sites, or add
>>>>>> > supporting infrastructure (like the changes that expose features to OSD
>>>>>> > cls ops in objclass/*).
>>>>>> >
>>>>>> > Once there are several of those, remove the require-features-patch, and
>>>>>> > make sure the changes are safe and clean and everything still works
>>>>>> > properly.  We can merge these as we go to make incremental progress.
>>>>>> >
>>>>>> > Finally, once *everything* is fixed to pass features to addr encode sites,
>>>>>> > the last commit will be the require-features-patch, and we can get it all
>>>>>> > in master.
>>>>>> >
>>>>>> > That will lay the foundation so that we can change the entity_addr_t
>>>>>> > encoding to support the new protocols and encoding, the entity_addrvec_t
>>>>>> > type, and so on.
>>>>>> >
>>>>>> > (The plan is to replace most instnaces of entity_addr_t with
>>>>>> > entity_addrvec_t--a list of addresses instead of a single one.  And make
>>>>>> > entity_addrvec_t encoded with old features spit out the legacy addr with
>>>>>> > the legacy entity_addr_t encoding so that old clients can still function.)
>>>>>> >
>>>>>> > We're all in #ceph-devel on irc.oftc.net--feel free to ask questions there
>>>>>> > or on this email thread.  Or we can do another video conf session to go
>>>>>> > through it.
>>>>>> >
>>>>>> > Thanks!
>>>>>> > sage
>>>>>>
>>>>>>