Skip to content

Commit

Permalink
kargs: run delete-if-present and append-if-missing failed
Browse files Browse the repository at this point in the history
when there is existing `key`

Revert part of 1a1ef13

`ostree_kernel_args_delete_if_present()` calls
`ostree_kernel_args_contains()` which only looks at the `key`
part, which makes `delete-if-present` failed when there is
existing `key`. `append-if-missing` will hit the same problem.
```
]# rpm-ostree kargs --append=crashkernel=1G
]# rpm-ostree kargs
... crashkernel=1G
]# rpm-ostree kargs --unchanged-exit-77 --delete-if-present=crashkernel=auto
error: No karg 'crashkernel=auto' found
]# rpm-ostree kargs --unchanged-exit-77 --append-if-missing=crashkernel=auto
No changes.
```

And fix corner case when `append` and `append-if-missing` with
the same value which will append twice.

Fixes: #4718
  • Loading branch information
HuijingHei committed Dec 21, 2023
1 parent 6a61574 commit cdd56d6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/daemon/rpmostreed-transaction-types.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -2679,6 +2679,9 @@ kernel_arg_apply_patching (KernelArgTransaction *self, RpmOstreeSysrootUpgrader
= static_cast<char **> (vardict_lookup_strv_canonical (self->options, "append-if-missing"));
g_autofree char **delete_if_present
= static_cast<char **> (vardict_lookup_strv_canonical (self->options, "delete-if-present"));
g_autoptr (OstreeKernelArgs) existing_kargs
= ostree_kernel_args_from_string (self->existing_kernel_args);
g_auto (GStrv) existing_kargs_strv = ostree_kernel_args_to_strv (existing_kargs);
gboolean changed = FALSE;

/* Delete all the entries included in the kernel args */
Expand Down Expand Up @@ -2707,19 +2710,20 @@ kernel_arg_apply_patching (KernelArgTransaction *self, RpmOstreeSysrootUpgrader
for (char **iter = append_if_missing; iter && *iter; iter++)
{
const char *arg = *iter;
if (!ostree_kernel_args_contains (kargs, arg))
g_auto (GStrv) kargs_strv = ostree_kernel_args_to_strv (kargs);
if (!g_strv_contains (kargs_strv, arg))
{
ostree_kernel_args_append_if_missing (kargs, arg);
ostree_kernel_args_append (kargs, arg);
changed = TRUE;
}
}

for (char **iter = delete_if_present; iter && *iter; iter++)
{
const char *arg = *iter;
if (ostree_kernel_args_contains (kargs, arg))
if (g_strv_contains (existing_kargs_strv, arg))
{
if (!ostree_kernel_args_delete_if_present (kargs, arg, error))
if (!ostree_kernel_args_delete (kargs, arg, error))
return FALSE;
changed = TRUE;
}
Expand Down
22 changes: 22 additions & 0 deletions tests/vmcheck/test-kernel-args.sh
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,28 @@ vm_rpmostree kargs --delete-if-present=PACKAGE3=TEST3 --unchanged-exit-77 || rc=
assert_streq $rc 77
echo "ok exit 77 when unchanged kargs with unchanged-exit-77"

# Test append-if-missing and delete-if-present for existing key
vm_rpmostree kargs --append-if-missing=PACKAGE4=TEST4
vm_rpmostree kargs > kargs.txt
assert_file_has_content_literal kargs.txt 'PACKAGE4=TEST4'
vm_rpmostree kargs --append-if-missing=PACKAGE4=NEWTEST
vm_rpmostree kargs > kargs.txt
assert_file_has_content_literal kargs.txt 'PACKAGE4=NEWTEST'
vm_rpmostree kargs --delete-if-present=PACKAGE4=TEST --unchanged-exit-77 || rc=$?
assert_streq $rc 77
echo "ok for append-if-missing and delete-if-present with existing key"

# Test corner case for append and append-if-missing with the same value
vm_rpmostree kargs --append=foo --append-if-missing=foo
vm_rpmostree kargs > kargs.txt
assert_not_file_has_content_literal kargs.txt 'foo foo'
assert_file_has_content_literal kargs.txt 'foo'
vm_rpmostree kargs --append-if-missing=bar=foo --append-if-missing=bar=foo
vm_rpmostree kargs > kargs.txt
assert_not_file_has_content_literal kargs.txt 'bar=foo bar=foo'
assert_file_has_content_literal kargs.txt 'bar=foo'
echo "ok for append and append-if-missing with the same value"

# Test for 'rpm-ostree kargs --editor'.
vm_rpmostree kargs > kargs.txt
assert_not_file_has_content_literal kargs.txt 'nonexisting'
Expand Down

0 comments on commit cdd56d6

Please sign in to comment.