-
Notifications
You must be signed in to change notification settings - Fork 11
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
add config with no exists config #671
base: master
Are you sure you want to change the base?
add config with no exists config #671
Conversation
No worries, your set of changes is great to spot the differences to other variants. Adding the values to the defaults has the effect that ioc does no longer complain about unknown JailConfig properties, but they will just be ignored when processing a jail. There are the cases:
As one with the ideas from your other branch, @himrock922, I'd like to derive more Issues from each individual change you suggest to the Globals. Doing so, while following the workflow described above, will lead to a user friendly solution with minimal breakage for iocage users. (iocage_legacy should not be affected). |
1e65386
to
22d2592
Compare
allow_mlock propetry changes from 0 to None.
ref2 https://www.freebsd.org/cgi/man.cgi?jail(8) enforce_staffs property equal 2 So, set allow_mount_fusefs values False(0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @himrock922, the comments I left are a start to implement your suggested changes. Some changes require new features to be implemented, which you can find linked in my comments. If you would create one PR per feature, we can individually discuss and test the implications for libioc.
@@ -67,7 +67,7 @@ | |||
"allow_mlock": None, | |||
"allow_mount": 0, | |||
"allow_mount_devfs": 0, | |||
"allow_mount_fusefs": 0, | |||
"allow_mount_fusefs": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting idea to actually turn the allow_mount_*
properties into booleans. The sysctl value is an integer type, but there is a translation of boolean values happening. We should just be consistent and change similar properties among this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I think should keep a consistency for code(For example, 0 or 1 integer only used, or boolean type only used).
If possible, To refactoring for future, I think should only use boolean type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, I do fix such allow_mount.*
parameters from 0,1 to True, False.
"bpf": None, | ||
"comment": None, | ||
"compression": None, | ||
"compressratio": None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a property that can be set. It should not be in the jail configuration at all. Is that the case in python-iocage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, such parameters using with python-iocage.
https://www.freebsd.org/cgi/man.cgi?query=iocage&sektion=8
available Available space in the jail's dataset. The amount of space
available to the dataset and all its children, assuming
that there is no other activity in the pool. Because space
is shared within a pool, availability can be limited by any
number of factors, including physical pool size, quotas,
reservations, or other datasets within the pool.
Source: zfs(8)
- bpf
https://github.com/iocage/iocage/blob/528f007e92f6e081614d8a09186c4831fc9f3e32/iocage.8#L1019
Toggle starting the jail with Berkely Packet Filter devices enabled.
- compression
https://github.com/iocage/iocage/blob/528f007e92f6e081614d8a09186c4831fc9f3e32/iocage.8#L1724
Controls the compression algorithm used for this dataset.
The lzjb compression algorithm is optimized for performance while
providing decent data compression.
Setting compression to on uses the lzjb compression algorithm.
The gzip algorithm uses the same compression as the
- compressratio
https://github.com/iocage/iocage/blob/528f007e92f6e081614d8a09186c4831fc9f3e32/iocage.8#L1789
Compression ratio.
Read-only.
For non-snapshots, the compression ratio achieved for the used space
of this dataset, expressed as a multiplier.
The used property includes descendant datasets, and, for clones, does
not include the space shared with the origin snapshot.
Also, Also, comment
property setting a lower layer for user
property.
ref #630 (comment)
For example
user {
comment: None,
notes: None
}
"compression": None, | ||
"compressratio": None, | ||
"count": None, | ||
"cpuset": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a new feature suggested in #636 - this change should be handled in a separate PR that addresses the GitHub Issue.
"provision": { | ||
"method": None, | ||
"source": None, | ||
"rev": "master" | ||
} | ||
}, | ||
"last_started": None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing feature tracked in #632 - this change belongs in a separate PR.
Also duplicate parameter does delete.
ref #630
Hi All,
I'm sorry, after rebase execute, confflict code(like as <<<HEAD, ======) mixed PR(Also, reflog failed). So, recreate branch...
Make sure to follow and check these boxes before submitting a PR! Thank you.