coreos/rpm-ostree

Parse Spaces in `kargs` Input

Closed this issue · 7 comments

czhang03 commented

Describe the bug

When send in multiple kargs input in a single string, the kargs seems to treat it as a single key value pair. For example run rpm-ostree karg --delete "init_on_alloc=1 init_on_free=1" will delete neither parameter, and rpm-ostree karg --append-if-missing "init_on_alloc=1 init_on_free=1" will duplicate these two kargs even when they exists

Reproduction steps

  1. run rpm-ostree karg --append "init_on_alloc=1 init_on_free=1", notice that init_on_alloc=1 init_on_free=1 gets added to kargs
  2. run rpm-ostree karg --append-if-missing "init_on_alloc=1 init_on_free=1", will duplicate these two kargs even when they are already present
  3. run rpm-ostree karg --delete "init_on_alloc=1 init_on_free=1" will give an error stating the argument is not found

Expected behavior

rpm-ostree karg --append-if-missing "init_on_alloc=1 init_on_free=1" should not duplicate the kargs, and rpm-ostree karg --delete "init_on_alloc=1 init_on_free=1" should delete the kargs when they are present.

Or at least raise an error/warning to the user about invalid input.

Actual behavior

Described in "Reproduction steps" section

System details

$ rpm-ostree version

rpm-ostree:
 Version: '2024.2'
 Git: 3d9a8755ddd96395a5c1d02b42243eb54ea01193
 Features:
  - rust
  - compose
  - container
  - fedora-integration

Additional information

Related:

HuijingHei commented

Normally we do this via rpm-ostree kargs, with --append multiple times, also --append-if-missing and delete.

[root@cosa-devsh ~]# rpm-ostree kargs --append init_on_alloc=1 --append init_on_free=1
Staging deployment... done
Changes queued for next boot. Run "systemctl reboot" to start a reboot
[root@cosa-devsh ~]# rpm-ostree kargs
... init_on_alloc=1 init_on_free=1

[root@cosa-devsh ~]# rpm-ostree kargs --append-if-missing init_on_alloc=1 --append-if-missing init_on_free=1
No changes.

[root@cosa-devsh ~]# rpm-ostree kargs --delete init_on_alloc=1 --delete init_on_free=1
czhang03 commented

Thank you for your response! I understand your point, but I think the user should at least get a warning about this, otherwise the semantics of rpm-ostree will be misaligned from the semantics of system.

For example, when user do rpm-ostree karg --append "init_on_alloc=1 init_on_free=1", the semantics for rpm-ostree will be "setting the init_on_alloc variable to 1 init_on_free=1", yet once the karg is added, the system will interprete it as "init_on_alloc is set to 1 and init_on_free is set to 1".

This is not terrible, but to me, it feels like inelegant design in the software. And from the link https://discussion.fedoraproject.org/t/how-i-do-to-put-several-kernel-arguments-in-rpm-ostree-in-a-single-command/74113 , people indeed misinterprets the use cases of these commands.

jlebon commented

I think it'd be fine to handle this by splitting on whitespace. However, I'd suggest rather than trying to handle quoting, we just detect if any quotes are present and pass through as is in that case.

Edit: so basically, something like this: if there's a literal " in the argument, pass through as is. Otherwise, split on space.

travier commented

Note that there are the " from the Bash command and the ones in the passed arguments:

$ rpm-ostree kargs --append-if-missing "foo=bar" # "Bash" quotes
$ rpm-ostree kargs --append-if-missing "foo=\"bar foo\"" # Quotes in the argument

According to https://www.kernel.org/doc/html/v4.14/admin-guide/kernel-parameters.html, we can have quoted spaces in kernel command line arguments so we need to support that.

Thus this makes this tricky, as we need to correctly detect spaces outside of quotes to split the arguments to process them.

$ rpm-ostree kargs --append-if-missing "foo=\"bar foo\" bar=foo"
  • 👍1
HuijingHei commented

Seems can append as whole, but failed when delete (if there is space), IMU, we should fix this.

# rpm-ostree kargs --append='foo="bar foo"'
Or # rpm-ostree kargs --append="foo=\"bar foo\""
rpm-ostree kargs
... foo="bar foo"

# rpm-ostree kargs --delete 'foo="bar foo"'
error: No karg 'foo="bar foo"' found
# rpm-ostree kargs --delete "foo=\"bar foo\""
error: No karg 'foo="bar foo"' found

We should split with space as the issue description.

# rpm-ostree kargs --append "foo=1 bar=1"
# rpm-ostree kargs
... foo=1 bar=1
# rpm-ostree kargs --delete "foo=1 bar=1"
error: No karg 'foo=1 bar=1' found

Xerf to Colin's comment openshift/machine-config-operator#3856 (comment).

  • 👍1
jlebon commented

According to kernel.org/doc/html/v4.14/admin-guide/kernel-parameters.html, we can have quoted spaces in kernel command line arguments so we need to support that.

Right, that's what I'm referring to in #4821 (comment). My suggestion is to not try to handle it and just pass through as is if there are literal quotes. OTOH, it wouldn't be very hard to make a quoting-aware splitter so cool with that too. (IIRC we have similar code already for handling this with package requests in manifests too.)

  • 👍1