Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No dataset created when path contains spaces #122

Closed
Jip-Hop opened this issue Apr 22, 2024 · 3 comments
Closed

No dataset created when path contains spaces #122

Jip-Hop opened this issue Apr 22, 2024 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Jip-Hop
Copy link
Owner

Jip-Hop commented Apr 22, 2024

In #118 we introduced a bug in this line:

if fields[1] == path and fields[2] == "zfs":

It does not compare as intended in case one of the parent datasets (and possible the pool name) contain spaces:
The contents of /proc/mounts contain the paths with spaces encoded as discussed in openzfs/zfs#11182.

Which results in us doing this comparison:

/mnt/tank/name\040with\040blanks/jailmaker == /mnt/tank/name with blanks/jailmaker/jails and zfs == "zfs"

The comparison fails and we end up not creating datasets, but plain directories for new jails.

@templehasfallen FYI

Reproduction steps

  1. Prepare dataset with spaces
zfs create -o compression=lz4 -o normalization=formD -o utf8only=on -o dnodesize=auto -o relatime=on -o acltype=posixacl "tank/name with blanks" 
zfs create -o compression=lz4 -o normalization=formD -o utf8only=on -o dnodesize=auto -o relatime=on -o acltype=posixacl "tank/name with blanks/jailmaker"
  1. Copy the jlmkr.py file in the jailmaker dir and run jlmkr.py create test.
  2. Check in the TrueNAS GUI under Datasets that there's no child dataset created under the jailmaker dataset.

Impact

The impact is relatively low as TrueNAS GUI doesn't allow creating datasets with spaces in their name. However datasets created with spaces by using the cli will show up fine in the TrueNAS GUI. So jailmaker needs to support this too.

@Jip-Hop Jip-Hop added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Apr 22, 2024
@templehasfallen
Copy link
Contributor

templehasfallen commented Apr 22, 2024

While I see the issue, I cannot reproduce the bug as the jlmkr name validation fails me before I can get to jail creation.
Tried both interactive and via jlmkr.py create test jail, jlmkr.py create "test jail" while I have a dataset jailmaker/jails/test jail.

I've written a small patch, just need to figure out how to reproduce the issue first to test it.

@Jip-Hop
Copy link
Owner Author

Jip-Hop commented Apr 23, 2024

Thanks for looking into it. It's not the jail name which contains spaces but a parent dataset which contains the jailmaker dataset. The reproduction steps are in the issue.

@templehasfallen
Copy link
Contributor

You're right, totally didn't think of that.
Update the code in the PR with tested working code, test on 23.10.2 and 24.04-RC1.
Should be fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants