From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junwang Zhao Subject: Re: wip-addr-features Date: Tue, 10 May 2016 19:46:55 +0800 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-vk0-f67.google.com ([209.85.213.67]:34066 "EHLO mail-vk0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394AbcEJLq5 (ORCPT ); Tue, 10 May 2016 07:46:57 -0400 Received: by mail-vk0-f67.google.com with SMTP id v68so955190vka.1 for ; Tue, 10 May 2016 04:46:56 -0700 (PDT) In-Reply-To: Sender: ceph-devel-owner@vger.kernel.org List-ID: To: Sage Weil , Haomai Wang Cc: Ceph Development Hi Haomai, I will try that and report the result later. On Tue, May 10, 2016 at 7:41 PM, Junwang Zhao wrote: > Sorry for not applied the attachment :) > > On Tue, May 10, 2016 at 7:34 PM, Junwang Zhao 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 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 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 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 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 >>>>>> >>>>>>