Skip to content

krun: allow configuring the virtiofs device #2076

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.

Sign up for GitHub

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

Merged
giuseppe merged 1 commit into from
Apr 24, 2026
+28 −6
Merged

krun: allow configuring the virtiofs device #2076

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions src/libcrun/handlers/krun.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ libkrun_exec (void *cookie, libcrun_container_t *container, const char *pathname
runtime_spec_schema_config_schema *def = container->container_def;
int32_t (*krun_set_log_level) (uint32_t level);
int (*krun_start_enter) (uint32_t ctx_id);
int32_t (*krun_set_root) (uint32_t ctx_id, const char *root_path);
int32_t (*krun_add_virtiofs2) (uint32_t ctx_id, const char *c_tag, const char *c_path, uint64_t shm_size);
int32_t (*krun_set_root_disk) (uint32_t ctx_id, const char *disk_path);
int32_t (*krun_set_tee_config_file) (uint32_t ctx_id, const char *file_path);
int32_t (*krun_set_console_output) (uint32_t ctx_id, const char *c_filepath);
Expand Down Expand Up @@ -471,14 +471,36 @@ libkrun_exec (void *cookie, libcrun_container_t *container, const char *pathname
}
else
{
krun_set_root = dlsym (handle, "krun_set_root");
const char *path_virtiofs_tag[] = { "virtiofs_tag", (const char *) 0 };
const char *path_virtiofs_shm_size[] = { "virtiofs_shm_size", (const char *) 0 };
yajl_val val_virtiofs_tag = NULL;
yajl_val val_virtiofs_shm_size = NULL;
const char *virtiofs_tag = NULL;
// Default to a conservative DAX size of 512MB, just like krun_set_root() does.
uint64_t virtiofs_shm_size = 512 * 1024 * 1024ULL;
Comment on lines +478 to +480
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other krun configuration parameters (such as cpus, ram_mib, and gpu_flags), it would be beneficial to allow configuring virtiofs_tag and virtiofs_shm_size via OCI annotations (e.g., krun.virtiofs_tag and krun.virtiofs_shm_size). This would allow users to override these settings directly from the container engine (like Podman) without needing to provide a .krun_vm.json file inside the container image.

Copy link
Copy Markdown
Contributor Author

@slp slp Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is in a static property of the workload, I don't think it'd make sense overriding it via an annotation. That said, I want to consolidate the whole way in which configuration parameters are parsed to make it more consistent. Possibly after the yyjson transition?

Copy link
Copy Markdown
Member

@giuseppe giuseppe Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, we can do that later


if (krun_set_root == NULL)
error (EXIT_FAILURE, 0, "could not find symbol in `libkrun.so`");
if (kconf->config_tree != NULL)
{
val_virtiofs_tag = yajl_tree_get (kconf->config_tree, path_virtiofs_tag, yajl_t_string);
if (val_virtiofs_tag != NULL && YAJL_IS_STRING (val_virtiofs_tag))
virtiofs_tag = YAJL_GET_STRING (val_virtiofs_tag);

val_virtiofs_shm_size = yajl_tree_get (kconf->config_tree, path_virtiofs_shm_size, yajl_t_number);
if (val_virtiofs_shm_size != NULL && YAJL_IS_INTEGER (val_virtiofs_shm_size))
virtiofs_shm_size = (uint64_t) YAJL_GET_INTEGER (val_virtiofs_shm_size);
Comment on lines +488 to +490
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The virtiofs_shm_size value from JSON is cast to uint64_t without checking if it is negative. If a negative value is provided in the configuration, it will result in an extremely large value due to the unsigned cast, which could lead to unexpected behavior or failures when calling krun_add_virtiofs2. Consider validating that the value is non-negative.

          val_virtiofs_shm_size = yajl_tree_get (kconf->config_tree, path_virtiofs_shm_size, yajl_t_number);
          if (val_virtiofs_shm_size != NULL && YAJL_IS_INTEGER (val_virtiofs_shm_size))
            {
              long long val = YAJL_GET_INTEGER (val_virtiofs_shm_size);
              if (UNLIKELY (val < 0))
                error (EXIT_FAILURE, 0, "virtiofs_shm_size must be a non-negative integer");
              virtiofs_shm_size = (uint64_t) val;
            }

Copy link
Copy Markdown
Member

@giuseppe giuseppe Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's ignore json stuff for now, I am working on a port to yyjson

}

if (virtiofs_tag == NULL)
virtiofs_tag = "/dev/root";
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Changing the default virtiofs tag from krunroot (the default used by libkrun's krun_set_root) to "/dev/root" is a breaking change for existing containers that rely on the default kernel command line provided by libkrun. Unless there is a specific reason to change the default for all users, it is safer to stick with "krunroot" as the default and allow users who need "/dev/root" to set it via the new configuration field.

        virtiofs_tag = "krunroot";

Copy link
Copy Markdown
Contributor Author

@slp slp Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear AI overlord. The default virtiofs tag in libkrun has never, ever been krunroot. It's always been /dev/root. Do you know how I know it? Because I wrote it.

Comment on lines +493 to +494
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The default tag is changed to "/dev/root". Note that the previous krun_set_root implementation in libkrun used "krunroot" as the default tag. If the kernel command line (either the default one in libkrunfw or a user-provided one) expects the old tag, this change might break the boot process unless the command line is also updated to match. Consider if "krunroot" should remain the default for better backward compatibility.


krun_add_virtiofs2 = dlsym (handle, "krun_add_virtiofs2");

if (krun_add_virtiofs2 == NULL)
error (EXIT_FAILURE, 0, "could not find symbol `krun_add_virtiofs2` in `libkrun.so`");
Comment on lines +496 to +499
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The code now has a hard dependency on the krun_add_virtiofs2 symbol, which was introduced in recent versions of libkrun. If libcrun is linked against an older version of libkrun.so, it will fail to start the container. Consider adding a fallback to the older krun_set_root symbol if krun_add_virtiofs2 is not found to maintain compatibility with older library versions.

Comment on lines +496 to +499
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This change replaces the use of krun_set_root with krun_add_virtiofs2, which introduces a hard dependency on the new symbol. This will cause a fatal error for users running against older versions of libkrun.so that do not export krun_add_virtiofs2. To maintain backward compatibility, consider falling back to krun_set_root if krun_add_virtiofs2 is not found, particularly when default values are being used.


ret = krun_set_root (ctx_id, "/");
ret = krun_add_virtiofs2 (ctx_id, virtiofs_tag, "/", virtiofs_shm_size);
if (UNLIKELY (ret < 0))
error (EXIT_FAILURE, -ret, "could not set krun root");
error (EXIT_FAILURE, -ret, "could not add virtiofs root with tag `%s`", virtiofs_tag);
}

if (kconf->awsnitro)
Expand Down
Loading