Skip to content

Commit

Permalink
lib/repo: Allow min-free-space-size and -percent to co-exist
Browse files Browse the repository at this point in the history
Previously, we would error out if both of the options were mentioned
in the config file (even if one of them is disabled with 0). There
were few suggestions that this behavior was not quite right.

Therefore, instead of throwing error and exiting, it's preferred to
warn the user. Hence, the solution that worked out is:
* Allow both options to exist simulateneously
* Check each config's value and decide:
  * If both are present and are non-zero, warn the user. Also, prefer
    to use min-free-space-size over the another.
  * If both are absent, then use -percent=3% as fallback
  * Every other case is valid hence, no warning

https://phabricator.endlessm.com/T13698
(cherry picked from commit be68991cf80f0aa1da7d36ab6e1d2c4d6c7cd3fb)
Signed-off-by: Robert McQueen <rob@endlessm.com>
  • Loading branch information
Umang Jain committed Jul 20, 2018
1 parent 7306577 commit 66a8d44
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
5 changes: 4 additions & 1 deletion man/ostree.repo-config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ Boston, MA 02111-1307, USA.
<term><varname>min-free-space-percent</varname></term>
<listitem><para>Integer percentage value (0-99) that specifies a minimum
percentage of total space (in blocks) in the underlying filesystem to
keep free. The default value is 3.</para></listitem>
keep free. The default value is 3.</para>
<para>In case this option is co-existing with min-free-space-size (see below),
it would be ignored and min-free-space-size check would be enforced instead.
</para></listitem>
</varlistentry>

<varlistentry>
Expand Down
30 changes: 19 additions & 11 deletions src/libostree/ostree-repo.c
Original file line number Diff line number Diff line change
Expand Up @@ -2818,12 +2818,10 @@ reload_core_config (OstreeRepo *self,
}

{
if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL) &&
g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL))
{
return glnx_throw (error, "min-free-space-percent and min-free-space-size are mutually exclusive");
}
else if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
/* Try to parse both min-free-space-* config options first. If both are absent, fallback on 3% free space.
* If both are present and are non-zero, use min-free-space-size unconditionally and display a warning.
*/
if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
{
g_autofree char *min_free_space_size_str = NULL;

Expand All @@ -2835,21 +2833,31 @@ reload_core_config (OstreeRepo *self,
if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error))
return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str);
}
else

if (g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL))
{
g_autofree char *min_free_space_percent_str = NULL;
/* If changing this, be sure to change the man page too */
const char *default_min_free_space = "3";

if (!ot_keyfile_get_value_with_default (self->config, "core", "min-free-space-percent",
default_min_free_space,
&min_free_space_percent_str, error))
NULL, &min_free_space_percent_str, error))
return FALSE;

self->min_free_space_percent = g_ascii_strtoull (min_free_space_percent_str, NULL, 10);
if (self->min_free_space_percent > 99)
return glnx_throw (error, "Invalid min-free-space-percent '%s'", min_free_space_percent_str);
}
else if (!g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
{
/* Default fallback of 3% free space. If changing this, be sure to change the man page too */
self->min_free_space_percent = 3;
self->min_free_space_mb = 0;
}

if (self->min_free_space_percent != 0 && self->min_free_space_mb != 0)
{
self->min_free_space_percent = 0;
g_debug ("Both min-free-space-percent and -size are mentioned in config. Enforcing min-free-space-size check only.");
}
}

{
Expand Down

0 comments on commit 66a8d44

Please sign in to comment.