Skip to content

Commit

Permalink
cgroup systemd: ignore unsupported properties
Browse files Browse the repository at this point in the history 
introduce a mechanism to detect and register the properties systemd doesn't
support so that we don't attempt to set them next time.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
@giuseppe
giuseppe committed Oct 15, 2024
1 parent 4712dff commit dcaf7e0
Showing 1 changed file with 147 additions and 27 deletions.
174 changes: 147 additions & 27 deletions src/libcrun/cgroup-systemd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,65 @@
# define CGROUP_WEIGHT_DEFAULT ((uint64_t) 100)
# define CGROUP_WEIGHT_MAX ((uint64_t) 10000)

# define SYSTEMD_MISSING_FEATURES_DIR ".cache/systemd-missing-properties"

static int
register_missing_property_from_message (const char *state_dir, const char *message, libcrun_error_t *err)
{
cleanup_free char *file_path = NULL;
cleanup_free char *dir_path = NULL;
cleanup_free char *property = NULL;
char *p;
int ret;

if (! has_prefix (message, "Cannot set property "))
return 0;

property = xstrdup (message + sizeof ("Cannot set property ") - 1);
p = strchr (property, ',');
if (! p)
return 0;
*p = '\0';

ret = append_paths (&dir_path, err, state_dir, SYSTEMD_MISSING_FEATURES_DIR, NULL);
if (UNLIKELY (ret < 0))
return ret;

ret = crun_ensure_directory (dir_path, 0755, true, err);
if (UNLIKELY (ret < 0))
return ret;

ret = append_paths (&file_path, err, dir_path, property, NULL);
if (UNLIKELY (ret < 0))
return ret;

ret = write_file (file_path, NULL, 0, err);
if (UNLIKELY (ret < 0))
return ret;

return 1;
}

static int
check_missing_property (const char *state_dir, const char *property, libcrun_error_t *err)
{
cleanup_free char *file_path = NULL;
int ret;

ret = append_paths (&file_path, err, state_dir, SYSTEMD_MISSING_FEATURES_DIR, property, NULL);
if (UNLIKELY (ret < 0))
return ret;

ret = access (file_path, F_OK);
if (ret < 0)
{
if (errno == ENOENT)
return 0;
return crun_make_error (err, errno, "access `%s`", file_path);
}
return 1;
}

int
cpuset_string_to_bitmask (const char *str, char **out, size_t *out_size, libcrun_error_t *err)
{
Expand Down Expand Up @@ -937,6 +996,7 @@ bus_append_byte_array (sd_bus_message *m, const char *field, const void *buf, si

static int
append_uint64_from_unified_map (sd_bus_message *m,
const char *state_dir,
const char *attr,
const char *key,
runtime_spec_schema_config_linux_resources *resources,
Expand All @@ -945,6 +1005,14 @@ append_uint64_from_unified_map (sd_bus_message *m,
uint64_t value;
int ret;

ret = check_missing_property (state_dir, attr, err);
if (UNLIKELY (ret < 0))
return ret;

/* The feature is missing, so skip setting it. */
if (ret == 1)
return 0;

ret = get_value_from_unified_map (resources, key, &value, err);
if (UNLIKELY (ret < 0))
return ret;
Expand Down Expand Up @@ -1068,6 +1136,7 @@ append_devices (sd_bus_message *m,

static int
append_resources (sd_bus_message *m,
const char *state_dir,
runtime_spec_schema_config_linux_resources *resources,
int cgroup_mode,
libcrun_error_t *err)
Expand All @@ -1076,10 +1145,10 @@ append_resources (sd_bus_message *m,
int sd_err;
int ret;

# define APPEND_UINT64(name, fn) \
# define APPEND_UINT64_VALUE(name, value) \
do \
{ \
ret = fn (resources, &value, err); \
ret = check_missing_property (state_dir, name, err); \
if (UNLIKELY (ret < 0)) \
return ret; \
if (ret) \
Expand All @@ -1090,6 +1159,18 @@ append_resources (sd_bus_message *m,
} \
} while (0)

# define APPEND_UINT64(name, fn) \
do \
{ \
ret = fn (resources, &value, err); \
if (UNLIKELY (ret < 0)) \
return ret; \
if (ret) \
{ \
APPEND_UINT64_VALUE (name, value); \
} \
} while (0)

if (resources == NULL)
return 0;

Expand All @@ -1110,13 +1191,8 @@ append_resources (sd_bus_message *m,
if (quota % 10000)
quota = ((quota / 10000) + 1) * 10000;

sd_err = sd_bus_message_append (m, "(sv)", "CPUQuotaPerSecUSec", "t", quota);
if (UNLIKELY (sd_err < 0))
return crun_make_error (err, -sd_err, "sd-bus message append CPUQuotaPerSecUSec");

sd_err = sd_bus_message_append (m, "(sv)", "CPUQuotaPeriodUSec", "t", resources->cpu->period);
if (UNLIKELY (sd_err < 0))
return crun_make_error (err, -sd_err, "sd-bus message append CPUQuotaPeriodUSec");
APPEND_UINT64_VALUE ("CPUQuotaPerSecUSec", quota);
APPEND_UINT64_VALUE ("CPUQuotaPeriodUSec", resources->cpu->period);
}
}

Expand All @@ -1138,40 +1214,53 @@ append_resources (sd_bus_message *m,
return crun_make_error (err, -sd_err, "sd-bus message append CPUWeight idle");
}

ret = append_uint64_from_unified_map (m, "MemoryMin", "memory.min", resources, err);
ret = append_uint64_from_unified_map (m, state_dir, "MemoryMin", "memory.min", resources, err);
if (UNLIKELY (ret < 0))
return ret;
ret = append_uint64_from_unified_map (m, "MemoryHigh", "memory.high", resources, err);
ret = append_uint64_from_unified_map (m, state_dir, "MemoryHigh", "memory.high", resources, err);
if (UNLIKELY (ret < 0))
return ret;
ret = append_uint64_from_unified_map (m, "MemoryZSwapMax", "memory.zswap.max", resources, err);
ret = append_uint64_from_unified_map (m, state_dir, "MemoryZSwapMax", "memory.zswap.max", resources, err);
if (UNLIKELY (ret < 0))
return ret;

if (resources->cpu && resources->cpu->cpus)
{
size_t allowed_cpus_len = 0;
const char *property_name = "AllowedCPUs";
cleanup_free char *allowed_cpus = NULL;
size_t allowed_cpus_len = 0;

ret = cpuset_string_to_bitmask (resources->cpu->cpus, &allowed_cpus, &allowed_cpus_len, err);
if (UNLIKELY (ret < 0))
return ret;

ret = bus_append_byte_array (m, "AllowedCPUs", allowed_cpus, allowed_cpus_len, err);
ret = check_missing_property (state_dir, property_name, err);
if (UNLIKELY (ret < 0))
return ret;

if (ret == 0)
{
ret = bus_append_byte_array (m, property_name, allowed_cpus, allowed_cpus_len, err);
if (UNLIKELY (ret < 0))
return ret;
}
}

if (resources->cpu && resources->cpu->mems)
{
size_t allowed_mems_len = 0;
const char *property_name = "AllowedMemoryNodes";
cleanup_free char *allowed_mems = NULL;
size_t allowed_mems_len = 0;

ret = cpuset_string_to_bitmask (resources->cpu->mems, &allowed_mems, &allowed_mems_len, err);
if (UNLIKELY (ret < 0))
return ret;

ret = bus_append_byte_array (m, "AllowedMemoryNodes", allowed_mems, allowed_mems_len, err);
ret = check_missing_property (state_dir, property_name, err);
if (UNLIKELY (ret < 0))
return ret;

ret = bus_append_byte_array (m, property_name, allowed_mems, allowed_mems_len, err);
if (UNLIKELY (ret < 0))
return ret;
}
Expand All @@ -1181,18 +1270,15 @@ append_resources (sd_bus_message *m,
case CGROUP_MODE_LEGACY:
case CGROUP_MODE_HYBRID:
if (resources->cpu && resources->cpu->shares > 0)
{
sd_err = sd_bus_message_append (m, "(sv)", "CPUShares", "t", resources->cpu->shares);
if (UNLIKELY (sd_err < 0))
return crun_make_error (err, -sd_err, "sd-bus message append CPUShares");
}
APPEND_UINT64_VALUE ("CPUShares", resources->cpu->shares);
break;

default:
return crun_make_error (err, 0, "invalid cgroup mode `%d`", cgroup_mode);
}

# undef APPEND_UINT64
# undef APPEND_UINT64_VALUE

return append_devices (m, resources, err);
}
Expand Down Expand Up @@ -1233,8 +1319,11 @@ static int
enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resources,
int cgroup_mode,
json_map_string_string *annotations,
const char *state_root,
const char *scope, const char *slice,
pid_t pid, libcrun_error_t *err)
pid_t pid,
bool *can_retry,
libcrun_error_t *err)
{
sd_bus *bus = NULL;
sd_bus_message *m = NULL;
Expand All @@ -1245,6 +1334,11 @@ enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resource
struct systemd_job_removed_s job_data = {};
int i;
const char *boolean_opts[10];
cleanup_free char *state_dir = NULL;

*can_retry = false;

state_dir = libcrun_get_state_directory (state_root, NULL);

i = 0;
boolean_opts[i++] = "Delegate";
Expand Down Expand Up @@ -1358,7 +1452,7 @@ enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resource
}
}

ret = append_resources (m, resources, cgroup_mode, err);
ret = append_resources (m, state_dir, resources, cgroup_mode, err);
if (UNLIKELY (ret < 0))
goto exit;

Expand Down Expand Up @@ -1392,6 +1486,14 @@ enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resource
}
if (sd_err < 0)
{
if (sd_err == -EROFS)
{
ret = register_missing_property_from_message (state_dir, error.message, err);
if (UNLIKELY (ret < 0))
goto exit;
if (ret > 0)
*can_retry = true;
}
ret = crun_make_error (err, sd_bus_error_get_errno (&error), "sd-bus call: %s", error.message ?: error.name);
goto exit;
}
Expand Down Expand Up @@ -1509,6 +1611,7 @@ libcrun_cgroup_enter_systemd (struct libcrun_cgroup_args *args,
cleanup_free char *scope = NULL;
cleanup_free char *path = NULL;
cleanup_free char *slice = NULL;
int retries_left = 32;
const char *suffix;
const char *id = args->id;
pid_t pid = args->pid;
Expand All @@ -1521,9 +1624,23 @@ libcrun_cgroup_enter_systemd (struct libcrun_cgroup_args *args,

get_systemd_scope_and_slice (id, cgroup_path, &scope, &slice);

ret = enter_systemd_cgroup_scope (resources, cgroup_mode, args->annotations, scope, slice, pid, err);
if (UNLIKELY (ret < 0))
return ret;
for (;;)
{
bool can_retry = false;

ret = enter_systemd_cgroup_scope (resources, cgroup_mode, args->annotations, args->state_root,
scope, slice, pid, &can_retry, err);
if (LIKELY (ret >= 0))
break;

if (can_retry && retries_left-- > 0)
{
crun_error_release (err);
continue;
}

return ret;
}

suffix = find_systemd_subgroup (args->annotations);

Expand Down Expand Up @@ -1600,12 +1717,15 @@ libcrun_update_resources_systemd (struct libcrun_cgroup_status *cgroup_status,
{
struct systemd_job_removed_s job_data = {};
sd_bus_error error = SD_BUS_ERROR_NULL;
cleanup_free char *state_dir = NULL;
sd_bus_message *reply = NULL;
sd_bus_message *m = NULL;
sd_bus *bus = NULL;
int sd_err, ret;
int cgroup_mode;

state_dir = libcrun_get_state_directory (state_root, NULL);

cgroup_mode = libcrun_get_cgroup_mode (err);
if (UNLIKELY (cgroup_mode < 0))
return cgroup_mode;
Expand Down Expand Up @@ -1642,7 +1762,7 @@ libcrun_update_resources_systemd (struct libcrun_cgroup_status *cgroup_status,
goto exit;
}

ret = append_resources (m, resources, cgroup_mode, err);
ret = append_resources (m, state_dir, resources, cgroup_mode, err);
if (UNLIKELY (ret < 0))
goto exit;

Expand Down

0 comments on commit dcaf7e0

Please sign in to comment.