[ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Dave Chinner]

[-- Attachment #1.1: Type: text/plain, Size: 3370 bytes --]

Hi folks,

I' ve just added a branch to the xfsprogs repository on kernel.org
that is up to date with the current 4.1-rc2 kernel code. It's smoke
tested, so I don't think there's any major problems with it. Can
everyone with development patches (i.e. things that change on disk
formats) for xfsprogs please rebase their work against this branch,
retest and repost? This branch will end up being the 3.3 release
tree...

FWIW, now that I've done this update, I'm considered how we keep
this up to date in future. Keeping the userspace code up to date and
working with all the internal kernel API changes takes quite a bit
of effort and that currently is all being done by the Maintainer.
i.e. me. This is the reason that userspace is not updated
frequently.

With this update, libxfs in the kernel and userspace are mostly
identical, so I'm thinking that a new rule for XFS developers needs
to be put in place: if you modify a libxfs kernel API, then you need
to make the same change to userspace. This means all the changes
needed for xfs_repair, xfs_db, etc, need to be done as well, because
sometimes these things are non-trivial.

A good example of this is Christoph's xfs_trans_commit() patchset.
There's 42 xfs_trans_commit() calls in userspace and it has a
completely different implementation in userspace. To put that in
context, there are 61 xfs_trans_commit() in the kernel code. If I
merge that changeset into the kernel code, then Christoph walks away
and it has become the Maintainer's problem to make userspace work
with the API change.

This *sucks* from a maintainer's POV - - it simply does not scale.
This is one of the reasons why my productivity as an XFS developer
has cratered over the last year. This userspace update work needs to
be distributed over the developers making the API and functionality
changes, so I'm throwing this out there to see what people think
about solving this problem.

I'm thinking that we need a userspace dev branch similar to the
for-next branch for the kernel code, where we aggregate topic
branches that contain each set of userspace patches that add/change
functionality that is shared with the kernel. If the developer
hasn't done the work to update userspace and test the changes there,
then the change is incomplete.

Note that I'm only suggesting this for the main XFS developers
making internal API or on-disk format changes to XFS - the typical
drive-by patches and bug fixes from random people we commit to the
kernel code will still need the maintainer to push them across to
userspace.

This will make my life easier from both an update and maintenance
point of view, and will make it easier for us as developers to get
the latest dev code faster. It will also make porting kernel changes
over to userspace faster, as the patches will mostly apply cleanly
to the userspace libxfs code base. And if it gets merged quicker,
tehn we'll get better test coverage of the dev tree because we'll be
testing it sooner. And as developers, we won't have to be working
out what changed and needs forward porting from the kernel to
userspace to make stuff work....

So, what do the people I'm asking to do more work so I don't have
to spend so much time on time consuming maintenance tasks think?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Christoph Hellwig]

On Mon, May 11, 2015 at 10:05:08AM +1000, Dave Chinner wrote:
> A good example of this is Christoph's xfs_trans_commit() patchset.
> There's 42 xfs_trans_commit() calls in userspace and it has a
> completely different implementation in userspace. To put that in
> context, there are 61 xfs_trans_commit() in the kernel code. If I
> merge that changeset into the kernel code, then Christoph walks away
> and it has become the Maintainer's problem to make userspace work
> with the API change.

I'm perfectly happy to do this work for userspace as well. 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Brian Foster]

On Mon, May 11, 2015 at 10:05:08AM +1000, Dave Chinner wrote:
> Hi folks,
> 
> I' ve just added a branch to the xfsprogs repository on kernel.org
> that is up to date with the current 4.1-rc2 kernel code. It's smoke
> tested, so I don't think there's any major problems with it. Can
> everyone with development patches (i.e. things that change on disk
> formats) for xfsprogs please rebase their work against this branch,
> retest and repost? This branch will end up being the 3.3 release
> tree...
> 
> FWIW, now that I've done this update, I'm considered how we keep
> this up to date in future. Keeping the userspace code up to date and
> working with all the internal kernel API changes takes quite a bit
> of effort and that currently is all being done by the Maintainer.
> i.e. me. This is the reason that userspace is not updated
> frequently.
> 
> With this update, libxfs in the kernel and userspace are mostly
> identical, so I'm thinking that a new rule for XFS developers needs
> to be put in place: if you modify a libxfs kernel API, then you need
> to make the same change to userspace. This means all the changes
> needed for xfs_repair, xfs_db, etc, need to be done as well, because
> sometimes these things are non-trivial.
> 
> A good example of this is Christoph's xfs_trans_commit() patchset.
> There's 42 xfs_trans_commit() calls in userspace and it has a
> completely different implementation in userspace. To put that in
> context, there are 61 xfs_trans_commit() in the kernel code. If I
> merge that changeset into the kernel code, then Christoph walks away
> and it has become the Maintainer's problem to make userspace work
> with the API change.
> 
> This *sucks* from a maintainer's POV - - it simply does not scale.
> This is one of the reasons why my productivity as an XFS developer
> has cratered over the last year. This userspace update work needs to
> be distributed over the developers making the API and functionality
> changes, so I'm throwing this out there to see what people think
> about solving this problem.
> 
> I'm thinking that we need a userspace dev branch similar to the
> for-next branch for the kernel code, where we aggregate topic
> branches that contain each set of userspace patches that add/change
> functionality that is shared with the kernel. If the developer
> hasn't done the work to update userspace and test the changes there,
> then the change is incomplete.
> 
> Note that I'm only suggesting this for the main XFS developers
> making internal API or on-disk format changes to XFS - the typical
> drive-by patches and bug fixes from random people we commit to the
> kernel code will still need the maintainer to push them across to
> userspace.
> 
> This will make my life easier from both an update and maintenance
> point of view, and will make it easier for us as developers to get
> the latest dev code faster. It will also make porting kernel changes
> over to userspace faster, as the patches will mostly apply cleanly
> to the userspace libxfs code base. And if it gets merged quicker,
> tehn we'll get better test coverage of the dev tree because we'll be
> testing it sooner. And as developers, we won't have to be working
> out what changed and needs forward porting from the kernel to
> userspace to make stuff work....
> 
> So, what do the people I'm asking to do more work so I don't have
> to spend so much time on time consuming maintenance tasks think?
> 

This seems reasonable to me. As it is, I'd expect to do this anyways for
anything that changes on-disk format. Such a change isn't really
complete if mkfs/xfs_repair/xfs_db can't handle it appropriately.
Provided we keep the development branch up to date, I don't see much
harm in extending that to libxfs API level changes as well.

What's the proposition with regard to submission/review process? I don't
think we necessarily need the userspace bits until there is some review
feedback on the kernel bits because that just increases development and
review overhead (though nothing precludes posting both, of course). Also
(and I think we discussed this briefly at LSF), I assume it is
reasonable to condense a kernel patch series to a single userspace "sync
XYZ feature to xfsprogs" patch for the bits that port directly over,
since we have the kernel git log for finer grained history..? Case in
point: I could squash the sparse inode kernel patches into a single
xfsprogs patch. The functional xfsprogs bits on top of that (e.g., mkfs,
repair, etc.) would of course remain as independent patches that require
indepenent review.

Something to note here is that I think this might warrant a bit more
positive feedback with regard to when things are considered reviewed on
the kernel side (i.e., merge is imminent). Perhaps we could create the
kernel topic branch but not merge it until the userspace bits are
available..? Just a thought, whatever works best for you is probably
fine provided the developer is made clear where things stand.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com



> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Dave Chinner]

On Mon, May 11, 2015 at 08:39:18AM -0400, Brian Foster wrote:
> On Mon, May 11, 2015 at 10:05:08AM +1000, Dave Chinner wrote:
> > Hi folks,
> > 
> > I' ve just added a branch to the xfsprogs repository on kernel.org
> > that is up to date with the current 4.1-rc2 kernel code. It's smoke
> > tested, so I don't think there's any major problems with it. Can
> > everyone with development patches (i.e. things that change on disk
> > formats) for xfsprogs please rebase their work against this branch,
> > retest and repost? This branch will end up being the 3.3 release
> > tree...
> > 
> > FWIW, now that I've done this update, I'm considered how we keep
> > this up to date in future. Keeping the userspace code up to date and
> > working with all the internal kernel API changes takes quite a bit
> > of effort and that currently is all being done by the Maintainer.
> > i.e. me. This is the reason that userspace is not updated
> > frequently.
> > 
> > With this update, libxfs in the kernel and userspace are mostly
> > identical, so I'm thinking that a new rule for XFS developers needs
> > to be put in place: if you modify a libxfs kernel API, then you need
> > to make the same change to userspace. This means all the changes
> > needed for xfs_repair, xfs_db, etc, need to be done as well, because
> > sometimes these things are non-trivial.
> > 
> > A good example of this is Christoph's xfs_trans_commit() patchset.
> > There's 42 xfs_trans_commit() calls in userspace and it has a
> > completely different implementation in userspace. To put that in
> > context, there are 61 xfs_trans_commit() in the kernel code. If I
> > merge that changeset into the kernel code, then Christoph walks away
> > and it has become the Maintainer's problem to make userspace work
> > with the API change.
> > 
> > This *sucks* from a maintainer's POV - - it simply does not scale.
> > This is one of the reasons why my productivity as an XFS developer
> > has cratered over the last year. This userspace update work needs to
> > be distributed over the developers making the API and functionality
> > changes, so I'm throwing this out there to see what people think
> > about solving this problem.
> > 
> > I'm thinking that we need a userspace dev branch similar to the
> > for-next branch for the kernel code, where we aggregate topic
> > branches that contain each set of userspace patches that add/change
> > functionality that is shared with the kernel. If the developer
> > hasn't done the work to update userspace and test the changes there,
> > then the change is incomplete.
> > 
> > Note that I'm only suggesting this for the main XFS developers
> > making internal API or on-disk format changes to XFS - the typical
> > drive-by patches and bug fixes from random people we commit to the
> > kernel code will still need the maintainer to push them across to
> > userspace.
> > 
> > This will make my life easier from both an update and maintenance
> > point of view, and will make it easier for us as developers to get
> > the latest dev code faster. It will also make porting kernel changes
> > over to userspace faster, as the patches will mostly apply cleanly
> > to the userspace libxfs code base. And if it gets merged quicker,
> > tehn we'll get better test coverage of the dev tree because we'll be
> > testing it sooner. And as developers, we won't have to be working
> > out what changed and needs forward porting from the kernel to
> > userspace to make stuff work....
> > 
> > So, what do the people I'm asking to do more work so I don't have
> > to spend so much time on time consuming maintenance tasks think?
> > 
> 
> This seems reasonable to me. As it is, I'd expect to do this anyways for
> anything that changes on-disk format. Such a change isn't really
> complete if mkfs/xfs_repair/xfs_db can't handle it appropriately.
> Provided we keep the development branch up to date, I don't see much
> harm in extending that to libxfs API level changes as well.

Right - the point of this is making sure we keep userspace up to
date, rather than doing an update once every blue moon and it being
a lot of work to do.

> What's the proposition with regard to submission/review process? I don't
> think we necessarily need the userspace bits until there is some review
> feedback on the kernel bits because that just increases development and
> review overhead (though nothing precludes posting both, of course). Also

Yes, that's fine - the userspace libxfs bits should be a straight
conversion of the kernel patches with a simple bit of path
stripping. e.g. the kernel patch path will be a/fs/xfs/libxfs/....
and the xfsprogs patch path is a/libxfs/...

Hence the same patch should apply, simply with a -p 5 (?) path strip
level rather than -p 1. i.e. `patch -p 5 < kernel.diff` should
simply apply the kernel patches to the xfsprogs libxfs.

> (and I think we discussed this briefly at LSF), I assume it is
> reasonable to condense a kernel patch series to a single userspace "sync
> XYZ feature to xfsprogs" patch for the bits that port directly over,
> since we have the kernel git log for finer grained history..?

Haven't really thought about it. Fine grained history is more
important for the kernel, but for userspace I can still see it being
useful for things like API changes where the same set of steps made
to the kernel code to make review easy should be made to make the
xfs_repair/db/... changes easy to review.

> Case in
> point: I could squash the sparse inode kernel patches into a single
> xfsprogs patch. The functional xfsprogs bits on top of that (e.g., mkfs,
> repair, etc.) would of course remain as independent patches that require
> indepenent review.

Right, for something that is all new code this can be done. It does
mean we'll have to be vigilant about merging bug fixes into the
userspace code, though. Those are really only picked up during
the large merges right now...

> Something to note here is that I think this might warrant a bit more
> positive feedback with regard to when things are considered reviewed on
> the kernel side (i.e., merge is imminent). Perhaps we could create the
> kernel topic branch but not merge it until the userspace bits are
> available..? Just a thought, whatever works best for you is probably
> fine provided the developer is made clear where things stand.

Well, for on-disk format changes we'll need the userspace code at
tha same time so that reviewers can test the changes, but for
everything else a port from the topic branch that gets merged into
the for-next tree is probably fine from a timing POV.

What I'd really like to is for all the changes merged in a kernel
-rc1 window are already merged into the userspace tree, or are at
least well on their way to being merged. That way we can even start
to think about a simple rolling release of xfsprogs that mirrors
the timing of the kernel release cycle. That way for new features
we get userspace support released at the same time that kernel
support is released....

The other benefit of doing this is that we get the changes committed
while it is still fresh in our minds - we don't have to look back to
code written 6-9 months ago and scratch our heads wondering why the
obvious merge changes aren't working because we've forgotten some
little detail about the change....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Brian Foster]

On Mon, May 11, 2015 at 11:20:09PM +1000, Dave Chinner wrote:
> On Mon, May 11, 2015 at 08:39:18AM -0400, Brian Foster wrote:
> > On Mon, May 11, 2015 at 10:05:08AM +1000, Dave Chinner wrote:
> > > Hi folks,
> > > 
> > > I' ve just added a branch to the xfsprogs repository on kernel.org
> > > that is up to date with the current 4.1-rc2 kernel code. It's smoke
> > > tested, so I don't think there's any major problems with it. Can
> > > everyone with development patches (i.e. things that change on disk
> > > formats) for xfsprogs please rebase their work against this branch,
> > > retest and repost? This branch will end up being the 3.3 release
> > > tree...
> > > 
> > > FWIW, now that I've done this update, I'm considered how we keep
> > > this up to date in future. Keeping the userspace code up to date and
> > > working with all the internal kernel API changes takes quite a bit
> > > of effort and that currently is all being done by the Maintainer.
> > > i.e. me. This is the reason that userspace is not updated
> > > frequently.
> > > 
> > > With this update, libxfs in the kernel and userspace are mostly
> > > identical, so I'm thinking that a new rule for XFS developers needs
> > > to be put in place: if you modify a libxfs kernel API, then you need
> > > to make the same change to userspace. This means all the changes
> > > needed for xfs_repair, xfs_db, etc, need to be done as well, because
> > > sometimes these things are non-trivial.
> > > 
> > > A good example of this is Christoph's xfs_trans_commit() patchset.
> > > There's 42 xfs_trans_commit() calls in userspace and it has a
> > > completely different implementation in userspace. To put that in
> > > context, there are 61 xfs_trans_commit() in the kernel code. If I
> > > merge that changeset into the kernel code, then Christoph walks away
> > > and it has become the Maintainer's problem to make userspace work
> > > with the API change.
> > > 
> > > This *sucks* from a maintainer's POV - - it simply does not scale.
> > > This is one of the reasons why my productivity as an XFS developer
> > > has cratered over the last year. This userspace update work needs to
> > > be distributed over the developers making the API and functionality
> > > changes, so I'm throwing this out there to see what people think
> > > about solving this problem.
> > > 
> > > I'm thinking that we need a userspace dev branch similar to the
> > > for-next branch for the kernel code, where we aggregate topic
> > > branches that contain each set of userspace patches that add/change
> > > functionality that is shared with the kernel. If the developer
> > > hasn't done the work to update userspace and test the changes there,
> > > then the change is incomplete.
> > > 
> > > Note that I'm only suggesting this for the main XFS developers
> > > making internal API or on-disk format changes to XFS - the typical
> > > drive-by patches and bug fixes from random people we commit to the
> > > kernel code will still need the maintainer to push them across to
> > > userspace.
> > > 
> > > This will make my life easier from both an update and maintenance
> > > point of view, and will make it easier for us as developers to get
> > > the latest dev code faster. It will also make porting kernel changes
> > > over to userspace faster, as the patches will mostly apply cleanly
> > > to the userspace libxfs code base. And if it gets merged quicker,
> > > tehn we'll get better test coverage of the dev tree because we'll be
> > > testing it sooner. And as developers, we won't have to be working
> > > out what changed and needs forward porting from the kernel to
> > > userspace to make stuff work....
> > > 
> > > So, what do the people I'm asking to do more work so I don't have
> > > to spend so much time on time consuming maintenance tasks think?
> > > 
> > 
> > This seems reasonable to me. As it is, I'd expect to do this anyways for
> > anything that changes on-disk format. Such a change isn't really
> > complete if mkfs/xfs_repair/xfs_db can't handle it appropriately.
> > Provided we keep the development branch up to date, I don't see much
> > harm in extending that to libxfs API level changes as well.
> 
> Right - the point of this is making sure we keep userspace up to
> date, rather than doing an update once every blue moon and it being
> a lot of work to do.
> 
> > What's the proposition with regard to submission/review process? I don't
> > think we necessarily need the userspace bits until there is some review
> > feedback on the kernel bits because that just increases development and
> > review overhead (though nothing precludes posting both, of course). Also
> 
> Yes, that's fine - the userspace libxfs bits should be a straight
> conversion of the kernel patches with a simple bit of path
> stripping. e.g. the kernel patch path will be a/fs/xfs/libxfs/....
> and the xfsprogs patch path is a/libxfs/...
> 
> Hence the same patch should apply, simply with a -p 5 (?) path strip
> level rather than -p 1. i.e. `patch -p 5 < kernel.diff` should
> simply apply the kernel patches to the xfsprogs libxfs.
> 
> > (and I think we discussed this briefly at LSF), I assume it is
> > reasonable to condense a kernel patch series to a single userspace "sync
> > XYZ feature to xfsprogs" patch for the bits that port directly over,
> > since we have the kernel git log for finer grained history..?
> 
> Haven't really thought about it. Fine grained history is more
> important for the kernel, but for userspace I can still see it being
> useful for things like API changes where the same set of steps made
> to the kernel code to make review easy should be made to make the
> xfs_repair/db/... changes easy to review.
> 

I'd always expect non-libxfs stuff to consist of independent patches as
normally done...

> > Case in
> > point: I could squash the sparse inode kernel patches into a single
> > xfsprogs patch. The functional xfsprogs bits on top of that (e.g., mkfs,
> > repair, etc.) would of course remain as independent patches that require
> > indepenent review.
> 
> Right, for something that is all new code this can be done. It does
> mean we'll have to be vigilant about merging bug fixes into the
> userspace code, though. Those are really only picked up during
> the large merges right now...
> 
> > Something to note here is that I think this might warrant a bit more
> > positive feedback with regard to when things are considered reviewed on
> > the kernel side (i.e., merge is imminent). Perhaps we could create the
> > kernel topic branch but not merge it until the userspace bits are
> > available..? Just a thought, whatever works best for you is probably
> > fine provided the developer is made clear where things stand.
> 
> Well, for on-disk format changes we'll need the userspace code at
> tha same time so that reviewers can test the changes, but for
> everything else a port from the topic branch that gets merged into
> the for-next tree is probably fine from a timing POV.
> 

Sure, we at least need a subset of the kernel code sufficient to
facilitate testing. The tarballs for the sparse inode code, for example,
might not precisely match all the little fixups/reworks in the kernel
code.

My assumption has been that things that are reviewed and pending merge
aren't necessarily merged immediately. The merges may be more timed with
the kernel release process or whatever your process happens to be. My
point is just that if things are not merged synchronous to review
so-to-speak, positive feedback when merge is imminent is useful to know
when is the optimal time to backport changes. That could manifest as
making a non-merged branch available, posting more reviewed-by's, etc.

> What I'd really like to is for all the changes merged in a kernel
> -rc1 window are already merged into the userspace tree, or are at
> least well on their way to being merged. That way we can even start
> to think about a simple rolling release of xfsprogs that mirrors
> the timing of the kernel release cycle. That way for new features
> we get userspace support released at the same time that kernel
> support is released....
> 
> The other benefit of doing this is that we get the changes committed
> while it is still fresh in our minds - we don't have to look back to
> code written 6-9 months ago and scratch our heads wondering why the
> obvious merge changes aren't working because we've forgotten some
> little detail about the change....
> 

Agreed.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 5/10/15 7:05 PM, Dave Chinner wrote:
> Hi folks,

...

> This userspace update work needs to
> be distributed over the developers making the API and functionality
> changes, so I'm throwing this out there to see what people think
> about solving this problem.

...

> So, what do the people I'm asking to do more work so I don't have
> to spend so much time on time consuming maintenance tasks think?

I think it makes perfect sense, and I'm happy to do it.  The giant
sync-ups after the fact have never been ideal from any perspective.

I'm glad you brought this up; I don't think anyone ever meant to simply
leave this to the maintainer - it just ended up that way, because the
rules or best practices weren't necessarily clear.

So; do you envision a 0/8 patch series, 4 of which touch libxfs, to
now be a 0/12 patch series, with the userspace bits i.e. at the end?
I'd be perfectly happy with this; once libxfs stays up to date in
userspace, it should be trivial.

- -Eric
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJVUMEiAAoJECCuFpLhPd7gwsEP/24cd6X9CQAAiCQGmB5rfTON
jj2QcvyANY0AFIxZGiJWlaulSGYsnzcN55G1W0JkoIDSfCwQq+I1UgCVtcns9nUU
LXaPgWQh0LgJSfYs+LezkwlulOAYWW2LOolDPPIyEyoVxkAHmm6M9kajj2NaHd5S
OmvoqBxRM/nl3DfRw3H/b5ARH2EDWQvgkbBoPwzD74oYMrxBK0bDV7dHrBh9I4yu
HE3R8zI+eIZxMy6hAV2WGRQg00GPW9bWJfuDp2HvekcdsbGc8JJN1fF/j5nlJX/r
h8dz0HbkB2ioZ2ErgcB7v4xZjpvcqg/sD6Zo7ehSyW0TE1Nxr9I4L7x1k048fh4P
JCZWViR4ddo5h4Wiheq7wFmNqSu0VL6lPzYpty/M8wIt2nLZcgjKq1agtDMHKcvJ
PeDanT2u6PkgrJIlZORnhCaGLfu1bNBqtnUGJXtPUMKGwl+CdZBqAsEe3pg5LH4f
3007zGBHhPg93/ZTdmgkjjbZlGt3qeT+tpFgnYJYSNTzj3Pom1ZvxXvXl3TnATTu
3phKrSpuWvHETmm9vz1xj/a2KXeyLOMpdgZYkDULfrwJnERbyE5wcNFNwSmUH0lI
69Ul/KvSY1TdrhvuAPA121TsuhSPP9YTZaeZnkITDn841P4wnaKb1TtP6NKldje0
f/4574r8AUVM5RoLuW9K
=PWIs
-----END PGP SIGNATURE-----

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

On 5/11/15 7:39 AM, Brian Foster wrote:

...

> What's the proposition with regard to submission/review process? I don't
> think we necessarily need the userspace bits until there is some review
> feedback on the kernel bits because that just increases development and
> review overhead (though nothing precludes posting both, of course). Also
> (and I think we discussed this briefly at LSF), I assume it is
> reasonable to condense a kernel patch series to a single userspace "sync
> XYZ feature to xfsprogs" patch for the bits that port directly over,
> since we have the kernel git log for finer grained history..? Case in
> point: I could squash the sparse inode kernel patches into a single
> xfsprogs patch. The functional xfsprogs bits on top of that (e.g., mkfs,
> repair, etc.) would of course remain as independent patches that require
> indepenent review.

I'd prefer fine-grained, myself; that way, going forward, we can have a
more or less 1:1 commit history.  With libxfs in userspace up to date,
it should be pretty easy, if not even scriptable, and I think the minimal
extra time needed to keep the fine-grained history around would be useful.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Dave Chinner]

On Mon, May 11, 2015 at 09:48:03AM -0500, Eric Sandeen wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 5/10/15 7:05 PM, Dave Chinner wrote:
> > Hi folks,
> 
> ...
> 
> > This userspace update work needs to
> > be distributed over the developers making the API and functionality
> > changes, so I'm throwing this out there to see what people think
> > about solving this problem.
> 
> ...
> 
> > So, what do the people I'm asking to do more work so I don't have
> > to spend so much time on time consuming maintenance tasks think?
> 
> I think it makes perfect sense, and I'm happy to do it.  The giant
> sync-ups after the fact have never been ideal from any perspective.
> 
> I'm glad you brought this up; I don't think anyone ever meant to simply
> leave this to the maintainer - it just ended up that way, because the
> rules or best practices weren't necessarily clear.

*nod*

> So; do you envision a 0/8 patch series, 4 of which touch libxfs, to
> now be a 0/12 patch series, with the userspace bits i.e. at the end?
> I'd be perfectly happy with this; once libxfs stays up to date in
> userspace, it should be trivial.

I'd expect separate patch series, because it's kinda hard to do a
git send-email or guilt patchbomb that spans mutltiple
repositories.... :P

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

On 5/11/15 4:28 PM, Dave Chinner wrote:
>> So; do you envision a 0/8 patch series, 4 of which touch libxfs, to
>> > now be a 0/12 patch series, with the userspace bits i.e. at the end?
>> > I'd be perfectly happy with this; once libxfs stays up to date in
>> > userspace, it should be trivial.
> I'd expect separate patch series, because it's kinda hard to do a
> git send-email or guilt patchbomb that spans mutltiple
> repositories.... :P

Yeah, uh, right.  :)  I knew that.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

One interesting problem that arises from this is in xfs_format.h:

/* On-disk XFS extended attribute names */
#define SGI_ACL_FILE            (char *)"SGI_ACL_FILE"
#define SGI_ACL_DEFAULT         (char *)"SGI_ACL_DEFAULT"
#define SGI_ACL_FILE_SIZE       (sizeof(SGI_ACL_FILE)-1)
#define SGI_ACL_DEFAULT_SIZE    (sizeof(SGI_ACL_DEFAULT)-1)

(aside: why is it unsigned in the kernel but not here?)

Anyway: that "sizeof" gives us 7, because it's getting the 
size of the pointer, not the string literal.  Cool eh!
Never mattered in the kernel, because the _SIZE macros aren't
used.

I'm not actually sure what the cleanest way to fix this is;
casting string literals gets way past my comfort level with 
C pedantry; technically if we want an array of unsigned chars,
we probably need:

unsigned char *SGI_ACL_FILE[] = "SGI_ACL_FILE";
unsigned char *SGI_ACL_DEFAULT[] = "SGI_ACL_DEFAULT";

but then it can't live in the format header; in kernelspace we need that
symbol in xfs_acl.c, in userspace we need it in repair/attr_repair.c 

- -Eric
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJVU8zRAAoJECCuFpLhPd7gzmIQAJPuO9ZRCMEjswGNVssux0Jm
+++pRvgPVl3qhdplzgukWcPTnZeq9NzfrU2rzwNNiA5nolgyKdo+Iu5rsPm3ZYHR
d80WU3mmkFPjoKbtwHP70bp71mxfSw7gb/k+MBOks/8PdkKHhbzI81n9q5WKGWxv
RQ+tUSVDxy/n25j4nLNp5nUwLPCycglxk8BD+rA3m4yJ1oDXZVWq7hXV+rYPxrQR
RV+GrwN+2thETlFSRexnzyTPO7n3cysUVaDVWC1RvqcMY2Ku3yb4EiJJdiCgLLq9
sBX4OWiy/A6020fhbaSnNV3tX4OQ7DmZe8JQ82GmtfwlzHWiQtFogJFgH4kLaXNm
oRwIsgX8BCzwDIYdbu+kbY5krtMaO8I4Fi1hM7nxVZKodo+hbPzTQjWEt9dmx4up
ForoWXsX3eibKhVEei7kuM0GIXK8C0G40VH2eFFGhJBTOOHj5NglLiZxMx0pl/HE
tl/l/SXhYK025ADFxpiwOBGkIoEo/bZI8REJblPIpb1PZNVEc05jTT3aTXpGmx+p
1AAeDnI3/urseJjhtQdvD8O7q47NrQNC4MLStA9wxQkn3d+XAoeQAzSCbxGS2fHO
CClPrV8tmAjI6Q3XvtdauRMXcO9npNroIndXjK4fuiSn0jtcHDJJts0wjPJkAI/x
6dnAQ7VcMg4V/Cxrt5XK
=T5wD
-----END PGP SIGNATURE-----

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

On 5/13/15 5:14 PM, Eric Sandeen wrote:
> One interesting problem that arises from this is in xfs_format.h:
> 
> /* On-disk XFS extended attribute names */
> #define SGI_ACL_FILE            (char *)"SGI_ACL_FILE"
> #define SGI_ACL_DEFAULT         (char *)"SGI_ACL_DEFAULT"
> #define SGI_ACL_FILE_SIZE       (sizeof(SGI_ACL_FILE)-1)
> #define SGI_ACL_DEFAULT_SIZE    (sizeof(SGI_ACL_DEFAULT)-1)
> 
> (aside: why is it unsigned in the kernel but not here?)
> 
> Anyway: that "sizeof" gives us 7, because it's getting the
> size of the pointer, not the string literal.  Cool eh!
> Never mattered in the kernel, because the _SIZE macros aren't
> used.
> 
> I'm not actually sure what the cleanest way to fix this is;
> casting string literals gets way past my comfort level with
> C pedantry; technically if we want an array of unsigned chars,
> we probably need:
> 
> unsigned char *SGI_ACL_FILE[] = "SGI_ACL_FILE";
> unsigned char *SGI_ACL_DEFAULT[] = "SGI_ACL_DEFAULT";

Er, drop the * of course ...

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Eric Sandeen]

On 5/13/15 5:25 PM, Eric Sandeen wrote:
> On 5/13/15 5:14 PM, Eric Sandeen wrote:
>> One interesting problem that arises from this is in xfs_format.h:
>>
>> /* On-disk XFS extended attribute names */
>> #define SGI_ACL_FILE            (char *)"SGI_ACL_FILE"
>> #define SGI_ACL_DEFAULT         (char *)"SGI_ACL_DEFAULT"
>> #define SGI_ACL_FILE_SIZE       (sizeof(SGI_ACL_FILE)-1)
>> #define SGI_ACL_DEFAULT_SIZE    (sizeof(SGI_ACL_DEFAULT)-1)
>>
>> (aside: why is it unsigned in the kernel but not here?)
>>
>> Anyway: that "sizeof" gives us 7, because it's getting the
>> size of the pointer, not the string literal.  Cool eh!
>> Never mattered in the kernel, because the _SIZE macros aren't
>> used.
>>
>> I'm not actually sure what the cleanest way to fix this is;
>> casting string literals gets way past my comfort level with
>> C pedantry; technically if we want an array of unsigned chars,
>> we probably need:
>>
>> unsigned char *SGI_ACL_FILE[] = "SGI_ACL_FILE";
>> unsigned char *SGI_ACL_DEFAULT[] = "SGI_ACL_DEFAULT";
> 
> Er, drop the * of course ...

I think we can just drop the ([unsigned] char *) bit and move on,
assigning the string literal to an unsigned char * should be fine.
Right?

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [ANNOUNCE, DISCUSS] xfsprogs: libxfs-4.1-update branch created

[Dave Chinner]

On Wed, May 13, 2015 at 05:14:42PM -0500, Eric Sandeen wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> One interesting problem that arises from this is in xfs_format.h:
> 
> /* On-disk XFS extended attribute names */
> #define SGI_ACL_FILE            (char *)"SGI_ACL_FILE"
> #define SGI_ACL_DEFAULT         (char *)"SGI_ACL_DEFAULT"
> #define SGI_ACL_FILE_SIZE       (sizeof(SGI_ACL_FILE)-1)
> #define SGI_ACL_DEFAULT_SIZE    (sizeof(SGI_ACL_DEFAULT)-1)
> 
> (aside: why is it unsigned in the kernel but not here?)

Because otherwise xfsprogs throws compile warnings all over the
place about pasing unsigned char * to strn*() functions.

And, IIRC, if we make the char * in the kernel, then they throw
compile warnings for passing char * to const unsigned char *
function parameters....

The kernel fs code (all of it, not just XFS) is an utter mess of
unsigned/signed char used for names and strings - don't ever try
turning on warn-unsigned when you build the kernel....

> Anyway: that "sizeof" gives us 7, because it's getting the 
> size of the pointer, not the string literal.  Cool eh!
> Never mattered in the kernel, because the _SIZE macros aren't
> used.

Yeah, that's a bug ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs