Bug 1477192

Summary:	xfs_info suddenly doesn't work with a device path
Product:	[Fedora] Fedora	Reporter:	Vratislav Podzimek <vpodzime>
Component:	xfsprogs	Assignee:	Eric Sandeen <esandeen>
Status:	CLOSED NOTABUG	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	27	CC:	dustymabe, esandeen, walters
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-05-08 16:00:07 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Vratislav Podzimek 2017-08-01 12:44:29 UTC

Description of problem:
With xfsprogs-4.11 and older 'xfs_info /dev/sda1' works just fine, but with 4.12 it doesn't.

Version-Release number of selected component (if applicable):
xfsprogs-4.12.0-1.fc27.i686

How reproducible:
100%

Steps to Reproduce:
1. run 'xfs_info' with some device path
2. run 'xfs_info' with the mount point

Actual results:
The first case doesn't work, the second one does.

Expected results:
Both cases should work as they used to with older versions.

Comment 1 Eric Sandeen 2017-08-01 13:27:38 UTC

Yes, this was largely intentional.  The behavior before was a bug:

SYNOPSIS

...

       xfs_info [ -t mtab ] mount-point
                            ^^^^^^^^^^^

There were problems with people pointing at unmounted mount points, and filesystem images, etc, and getting unexpected information.  It's documented as to be used on a mount point, but that had not been enforced.  To be honest, I was not aware that it had worked on a device node before; that might be something we could revisit, but it's never been a documented feature.  If it was working before, it was by accident.

Comment 2 Vratislav Podzimek 2017-08-03 10:02:00 UTC

How hard would it be to turn the old bug into a new feature? It's not particularly easy to find the device's mount point in places where only the device path is known and information about the particular FS is required.

Comment 3 Eric Sandeen 2017-08-03 15:06:50 UTC

So that's the flip side of it, the command will not work on a block device unless it's mounted, so thinking that it works against block devices may also confuse people...

In what situations is it difficult to find a mount point from a device?  /proc/mounts contains the mapping, right?  Or "mount" output?

It may not be too difficult to add block device lookup as a documented, supported feature, but I didn't know that would be such a desired or necessary usecase.

Thanks,
-Eric

Comment 4 Vratislav Podzimek 2017-08-07 07:35:03 UTC

(In reply to Eric Sandeen from comment #3)
> So that's the flip side of it, the command will not work on a block device
> unless it's mounted, so thinking that it works against block devices may
> also confuse people...

Sure, but that can be quite easily documented.

> 
> In what situations is it difficult to find a mount point from a device? 
> /proc/mounts contains the mapping, right?  Or "mount" output?

Yes, but parsing those in a C code line by line is cumbersome, devices can have multiple names/paths etc. I'm not saying it's impossible, but it's complicated enough to be worth having just in one place (xfs_info).

> 
> It may not be too difficult to add block device lookup as a documented,
> supported feature, but I didn't know that would be such a desired or
> necessary usecase.

Well, if some code relied on the old behavior, they would have to adapt now. So it is desired to not remove functionality. But I understand that it wasn't documented and probably not intentionally working either.

Comment 5 Jan Kurik 2017-08-15 09:36:09 UTC

This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Comment 6 Dusty Mabe 2017-09-11 19:38:20 UTC

FYI - cloud-init uses this functionality so we need to resolve between the cloud-init maintainers and xfs maintainers how this needs to be handled. If not then we'll break every cloud instance we boot with an XFS root filesystem. 

see https://bugzilla.redhat.com/show_bug.cgi?id=1490505

Comment 7 Eric Sandeen 2017-09-11 20:59:41 UTC

nb: cloud-init just needs a one-line change to fix this and invoke xfs_growfs properly, I added comments to the bug #1490505

Comment 8 Colin Walters 2017-09-12 13:51:59 UTC

This bug is a great example of where if we actually had the ability to revert, that should have been done first.  Fix the consumers, then try again.

Comment 9 Eric Sandeen 2017-09-12 18:44:16 UTC

(In reply to Colin Walters from comment #8)
> This bug is a great example of where if we actually had the ability to
> revert, that should have been done first.  Fix the consumers, then try again.

I'm not so sure.  This was an undocumented/unsupported interface.  The "consumer" in this bug's case was an admin, and we can't really "fix" admins.  ;)

The consumer in bug #1490505, cloud-init, can be fixed with a trivial 1-line patch.  It's not clear to me that reverting the change in xfsprogs has any advantage over simply fixing the consumer properly, does it?

Comment 10 Vratislav Podzimek 2017-09-14 09:52:42 UTC

(In reply to Eric Sandeen from comment #9)
> (In reply to Colin Walters from comment #8)
> > This bug is a great example of where if we actually had the ability to
> > revert, that should have been done first.  Fix the consumers, then try again.
> 
> I'm not so sure.  This was an undocumented/unsupported interface.  The
> "consumer" in this bug's case was an admin, and we can't really "fix"
> admins.  ;)

Which brings us back/again to the discussion about the lack of APIs for file systems. Since CLI is the only option, APIs are actually built on top of it.

Comment 11 Eric Sandeen 2018-05-08 16:00:07 UTC

While recognizing that this was an unexpected and for some undesirable change, it was made intentionally.  It won't be changing back at this point, so closing.