(t) Rockstor 4.5.7 Cannot create share with quotas disabled on pool

[Hooverdan96]

Issue created based on forum post a few days ago:
https://forum.rockstor.com/t/rockstor-4-5-7-cannot-create-share-with-quotas-disabled-on-pool/8706
Not sure, whether this was discussed already in the past (my search didn't yield any relevant results), but on Rockstor 4.5.7 during share creation on a pool for which the quotas are disabled, the UI comes back with an error that prevents it from doing so:

Traceback (most recent call last):
File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception yield
File "/opt/rockstor/src/rockstor/storageadmin/views/share.py", line 206, in post add_share(pool, sname, pqid)
File "/opt/rockstor/src/rockstor/fs/btrfs.py", line 652, in add_share return run_command(sub_vol_cmd)
File "/opt/rockstor/src/rockstor/system/osi.py", line 227, in run_command raise CommandException(cmd, out, err, rc)
CommandException: Error running a command. cmd = /usr/sbin/btrfs subvolume create -i -1/-1 /mnt2/rockleap/config-test2. rc = 255. stdout = ['']. stderr = ['ERROR: invalid qgroupid or subvolume path: -1/-1', '']`

On the flip side, when I have quotas enabled on the pool, the share is created without a problem.

On my current 4.1 "production" instance (with kernel backport installed), the same behavior seems to occur, I just did not notice because I had not created any new shares, since I disabled the quotas on there some time ago.

[FroggyFlox]

Thanks @Hooverdan96, nice find!
I can confirm I see the same behavior on my end, running the latest btrfsprogs on Leap 15.4:

# zypper info btrfsprogs
Information for package btrfsprogs:
-----------------------------------
Repository     : Leap_15_4
Name           : btrfsprogs
Version        : 5.14-150400.3.6
Arch           : x86_64
Vendor         : SUSE LLC <https://www.suse.com/>
Installed Size : 3.4 MiB
Installed      : Yes
Status         : up-to-date
Source package : btrfsprogs-5.14-150400.3.6.src
Upstream URL   : https://btrfs.wiki.kernel.org/index.php/Main_Page
Summary        : Utilities for the Btrfs filesystem
Description    : 
    Utilities needed to create and maintain btrfs file systems under Linux.

After a quick look at our fs/btrfs.py, it seems we return -1/-1 for the qgroup when quotas are disabled so that part is behaving as expected. It thus seem that btrfsprogs now won't accept that value as qid anymore... This requires @phillxnet's expertise, unfortunately.

[Hooverdan96]

Thanks @FroggyFlox for confirming. I checked, in the 15.4 flavor I tested this, I have the same version as you listed.

In my production instance (version 4.1.0-0) with kernel backport that shows the same behavior it's a slightly new version:

# zypper info btrfsprogs
Information for package btrfsprogs:
-----------------------------------
Repository     : filesystems
Name           : btrfsprogs
Version        : 5.16.2-150300.356.1
Arch           : x86_64
Vendor         : obs://build.opensuse.org/filesystems
Installed Size : 3.6 MiB
Installed      : Yes
Status         : up-to-date
Source package : btrfsprogs-5.16.2-150300.356.1.src
Upstream URL   : https://btrfs.wiki.kernel.org/
Summary        : Utilities for the Btrfs filesystem
Description    :
    Utilities needed to create and maintain btrfs file systems under Linux.

For completeness, on my Tumbleweed-installer based installation of Rockstor, the btrfsprogs are a bit newer (obviously):

# zypper info btrfsprogs
Information for package btrfsprogs: 
----------------------------------- 
Repository     : Tumbleweed
Name           : btrfsprogs 
Version        : 6.1.3-1.1 
Arch           : x86_64 
Vendor         : openSUSE 
Installed Size : 3.8 MiB 
Installed      : Yes 
Status         : up-to-date
Source package : btrfsprogs-6.1.3-1.1.src 
Upstream URL   : https://btrfs.wiki.kernel.org/ 
Summary        : Utilities for the Btrfs filesystem 
Description    : 
    Utilities needed to create and maintain btrfs file systems under Linux.

There it's the same behavior as well (share creation failure when quotas are turned off on underlying pool)

[Hooverdan96]

if quotas are disabled it seems that the option -i -1/-1 needs to be dropped from the subvolume creation process. I created the subvolume successfully manually that way /usr/sbin/btrfs subvolume create /mnt2/rockleap/config-test2). It also shows up in the WebUI upon a page refresh (which makes sense). Of course, I have no idea whether that different way of creating the share has any other repercussions.

When subsequently re-enabling quotas in the WebUI and checking at the command line with btrfs qgroup show /mnt2/<poolname> then it seems to show a consistent qgroupID across the previously (via WebUI created) shares, all with 0/<xxx>.
Curiously I am also seeing a qgroup 2015 with entries 1 through 9 but each line also has the `<0 member qgroups> (I have 9 shares defined + root share on that instance if that means anything.

All for @phillxnet to explain the magic ...

[phillxnet]

@Hooverdan96 @FroggyFlox

if quotas are disabled it seems that the option -i -1/-1 needs to be dropped from the subvolume creation process.

Agreed. From memory I thought the the -1 values were flag values. I.e. if it's -1 bypass quotas. I'll have to have a look again to be sure. From the report and confirmation it looks like we passed this through to the command and it was then nullified as a quotas request. It's been a while since I last looked at that code!

Curiously I am also seeing a qgroup 2015 with entries 1 through 9

That is normal, 'our' chosen qgroup of 2015 is what we 'tally' our use under. It's the Rockstor quoata group chosen. Other programs may establish their own qgroup and assign as they see fit. But we account use under the 2015 qgroup.

In the early days we basically failed across the board if quotas were disabled. But starting around here:

improve quotas not enabled behaviour. Fixes #1869 #1874

and followed-up via:
Add option to disable BTRFS quota-qgroups. Fixes #1592 #1903
improve pool import with disabled or indeterminate quota state. Fixes #1987 #1988
repeated log errors on disabled quotas (#2236) #2271

where the above help for context of what we attempt to do to accommodate quotas disabled.

From the initial report this does sound just like we requried to not pass -i -1/-1 when quotas are disabled, so yes, hopefully we can test for -1/-1 and drop this part of the command.

This report, and it's late-comer status, does rather indicate that many or our users may now no longer disable quotas so that's good to know. I think this should be on our current Milestone however so I'll pop it on now.
Do feel free to remove is you disagree.

[phillxnet]

Preparing a simple pr for this issue now:
When debug mode is on we already have the first of the following lines logged already:

[06/Mar/2023 10:48:28] INFO [fs.btrfs:1216] Mount Point: /mnt2/test-pool has Quotas disabled, skipping qgroup show.
[06/Mar/2023 10:48:28] DEBUG [fs.btrfs:692] ADD_SHARE called with share_name=test-share, qid=-1/-1

generated by qgroup_max() which is called by qgroup_create() where the -1/-1 is assigned in this case.
The second is my double checking what we are passing along there.

Not sure when this rejection of -1/-1 came into effect but yes, lets just drop the '-i qgourp' option when encountering this flag value as it's intended use was to indicate a quotas disabled so we seem to have a straight line here:

SUBCOMMAND
       create [-i <qgroupid>] [<dest>/]<name>
           Create a subvolume <name> in <dest>.

           If <dest> is not given, subvolume <name> will be created in the current directory.

           Options

           -i <qgroupid>
               Add the newly created subvolume to a qgroup. This option can be given multiple times.

Likely in the past the -1/-1 qgroup designation was simply silently ignored.
We normally use a 2015/n designation which we generate/derive (via qgroup_create() referenced above) but alas we should now just skip the entire option all-together as @Hooverdan96 has confirmed to work. Same here by the way.

[phillxnet]

@Hooverdan96 Thanks for doing the leg work on this one. The referenced pr is basically what you suggested.
@FroggyFlox Thanks for the confirmation. I'm a little surprised this hasn't been reported sooner.
Closing as:
Fixed by #2511

[FroggyFlox]

I'm a little surprised this hasn't been reported sooner.

Me too; I've had a quick look at the btrfsprogs changelogs when @Hooverdan96 first reported the issue but I couldn't see anything obviously related to that. There may have been a more "subtle" change/fix that implemented that.

On a side note, I wonder if this hasn't come to light earlier due to the fact that most users create Pools and shares and then disable quotas? In this case, this should this work post #2511 , correct?

[phillxnet]

@FroggyFlox Yes, I had a little look also and thought I saw a changelog entry regarding parsing of qgroups but not certain. Also, as you say, it may just have been a sanity check under some other heading. Or we have just not had it reported for years !!

In this case, this should this work post #2511 , correct?

Yes. I tested enabled and disabled and re-enabled function and all looks to be OK.