Project

General

Profile

Bug #52171

Merge in NFS updates from FreeBSD

Added by Alexander Motin 9 months ago. Updated 4 months ago.

Status:
Passed Testing
Priority:
No priority
Assignee:
Sean Fagan
Category:
OS
Target version:
Seen in:
Severity:
Med High
Reason for Closing:
Reason for Blocked:
Needs QA:
No
Needs Doc:
Yes
Needs Merging:
No
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:
ChangeLog Required:
No

Description

Some time ago John merged some Rick Macklem's pNFS patches from ones private repo to FreeNAS. It seems freenas/11-stable and freenas/11.2-stable are affected. But some of those patches were not committed to FreeBSD, that caused me some merge problems, not mentioning lack of trust. We need to sort out what actually happeed and probably remove the divergence, unless there is some reason why we need it.


Related issues

Related to FreeNAS - Bug #68581: Bring in NFS support for virtual hostnamesDone

Associated revisions

Revision 9d7b3953 (diff)
Added by Sean Fagan 8 months ago

As part of ticket 52171, I don't believe checking for WITH_PNFS
is useful any longer.

Ticket: #52171

History

#1 Updated by Sean Fagan 9 months ago

  • Status changed from Unscreened to Screened

#2 Updated by Sean Fagan 9 months ago

Going through the logs to collect the commits on our side.

#3 Updated by Sean Fagan 8 months ago

  • Status changed from Screened to In Progress

Since I was correctly chided for not updating this:

I've been looking at the pNFS code specifically. That mostly seems to be the same as in FreeBSD head. I've not yet compared that to 11-stable (or the new 12-stable). There are further some NFS changes which were added specific to our needs, and even john commented they weren't great. These are mostly related to HA and Kerberized NFS.

I have been looking at our os freenas/11-stable branch for comparison, which is not a great thing to compare to freebsd head, but I figured that's where most of the concern was.

I specifically looked at the freenas/11-stable-pNFS branch on our repo. There are some subsequent commits by John related to NFS, but most of them are in the FreeBSD side as well.

Apparently I need to look at freebsd stable/11. I don't know why I was comparing to head.

#4 Updated by Alexander Motin 8 months ago

Sean Fagan wrote:

I specifically looked at the freenas/11-stable-pNFS branch on our repo. There are some subsequent commits by John related to NFS, but most of them are in the FreeBSD side as well.

I've asked John some time ago why is the mention of that branch scratches my eyes in build scripts, and he told me that it nothing interesting and should be removed, to which I haven't got to do.

#5 Updated by Sean Fagan 8 months ago

So the differences between our 11-stable and freebsd stable/11 are pretty bothersome -- there's a bunch of them that diverge, and are no doubt the reason for merge conflicts.

As I said, most of them are in freebsd/head. So they could be tracked down and merged back in, I suppose, but the question is whether that would be allowed. That gets into releng and politics on the freebsd side.

So at this point, I think there are two areas of concern:
  1. The changes we have that aren't in freebsd at all;
  2. The changes we have in our 11 branches that are in freebsd, but not in freebsd stable/11

Is that reasonably accurate?

#6 Updated by Alexander Motin 8 months ago

Sean Fagan wrote:

So at this point, I think there are two areas of concern:
  1. The changes we have that aren't in freebsd at all;

If such ones exist, that is definitely a concern, since they may be not in FreeBSD for reason.

  1. The changes we have in our 11 branches that are in freebsd, but not in freebsd stable/11

That is somewhat less concern if our code state is consistent enough, and in FreeBSD head no new fixes for our code. And we should better know that we diverge in some areas in case some problem arise.

#7 Updated by Sean Fagan 8 months ago

Alexander Motin wrote:

  1. The changes we have that aren't in freebsd at all;

If such ones exist, that is definitely a concern, since they may be not in FreeBSD for reason.

They do exist, obviously. Most of them seem to be HA related, and I don't know that I would recommend merging them in. In particular, the one that I was looking at involves virtual hosts; specifically, to have a particular principal used for gssd -- John's solution was to have a file called /etc/nfs.virtualhost (or something like that, sorry), which if it exists, then to use that principal name instead of the hosts name. This allows for failover to happen, and for the principal to be what is expected, rather than whatever hostname the side running had.

This is acceptable for us, I think, but I think the better change would have been to have command-line options specifying that. But adding command-line options to nfsd is a slightly bigger step, in terms of social acceptance.

  1. The changes we have in our 11 branches that are in freebsd, but not in freebsd stable/11

That is somewhat less concern if our code state is consistent enough, and in FreeBSD head no new fixes for our code. And we should better know that we diverge in some areas in case some problem arise.

The NFS code does not seem to be in high churn, mostly. On either side. So there appears to be a reasonable amount of stability.

#8 Updated by Sean Fagan 8 months ago

Looking closely at the pnfs changes now. The stuff we have seems to be a slightly older version of what's in stable/12. I'm a bit hampered by having no experience with pNFS, so I'm not sure if the configuration file changed semantics.

The changes between the two versions aren't huge, but there's a set of small changes. Interspersed in these changes are some (fortunately small and localized) ix-only changes.

I'm going forward on the presumption that the goal is to minimize the changes with what's in stable/12, since we're not going to get pNFS pushed into stable/11 on the FreeBSD side :).

#9 Updated by Sean Fagan 7 months ago

I've got the first big set of changes reconciled (that is, r335012 aka git hash 77f312d46a380b908aca0ab0e3a7c4849bf79156, merged in with our code). I'll do some basic sanity checking, and then I'll start getting the subsequent changes merged in (or not, if for some reason I don't think they're good for us).

#10 Updated by Sean Fagan 7 months ago

Slogged through all but the last one, which looks like it's a vastly different base. I need to track down the history for nfs_clrpcops.c, and check the commits I'm sure I missed.

#11 Updated by Sean Fagan 7 months ago

I've pushed my WIP branch as FIX-52171-WIP.

This has everything except the changes related to nfs_clrpcops.c, which seems to involve changes I missed. I'm trying to track those down and figure out what to do with them.

#12 Updated by Sean Fagan 7 months ago

Oh, hm. A big thing happened between 11 and 12: migration to 64-bit inodes.

That's a cause of a lot of the merge conflicts with this file, and probably others.

I don't think we should move 11-STABLE to 64-bit inodes, which means I'll have to rewrite the last commits. This means we will have merge errors for a while with it, but it also means that when we move to FreeBSD 12, we will be as in-sync as possible with the NFS code, and can drop most of our changes.

#13 Updated by Sean Fagan 7 months ago

  • Status changed from In Progress to Blocked
  • Reason for Blocked set to Waiting for feedback

freenas/os pull request #163 based on 11-stable.

#14 Updated by Sean Fagan 7 months ago

  • Status changed from Blocked to In Progress
  • Reason for Blocked deleted (Waiting for feedback)

Alexander pointed out there's a better way to do this merge, which I hadn't realized was possible, so I am re-doing it. It won't take as long fortunately.

#15 Updated by Sean Fagan 7 months ago

Still in progress. Making the source look like the final result was a lot easier than sorting out commits that come from multiple svn sources.

#16 Updated by Sean Fagan 7 months ago

#17 Updated by Sean Fagan 6 months ago

  • Related to Bug #68581: Bring in NFS support for virtual hostnames added

#18 Updated by Sean Fagan 6 months ago

  • Status changed from In Progress to Blocked
  • Reason for Blocked set to Waiting for feedback

#19 Updated by Sean Fagan 6 months ago

This got merged into 11-stable; Alexander has asked about 11.2. I'm still fuzzy on the release schedule for that, so I don't know if it should go in. It's a large set of changes, so it should get tested lots, but if there's time to do that before 11.2 then I think it's ok to merge in.

#22 Updated by Dru Lavigne 6 months ago

  • Target version changed from Backlog to 11.3

#23 Updated by Dru Lavigne 5 months ago

  • Subject changed from Clean up NFS source tree to Merge in NFS updates from FreeBSD
  • Status changed from Blocked to Ready for Testing
  • Target version changed from 11.3 to 11.3-BETA1
  • Reason for Blocked deleted (Waiting for feedback)
  • Needs Merging changed from Yes to No

#24 Updated by Sean Fagan 4 months ago

I don't believe there are any specific tests for this. It was primarily a way to reduce merge conflicts going forward; it also probably fixed a few bugs, but nothing specific on our end.

#25 Updated by Dru Lavigne 4 months ago

  • Status changed from Ready for Testing to Passed Testing
  • Needs QA changed from Yes to No

Also available in: Atom PDF