From 6ce868a0da220f98a33f390a1292f01e63f5d973 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:02:19 +0100 Subject: [PATCH 01/11] event-util: add helper for adding pidref child to event loop --- src/libsystemd/sd-event/event-util.c | 17 +++++++++++++++++ src/libsystemd/sd-event/event-util.h | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/src/libsystemd/sd-event/event-util.c b/src/libsystemd/sd-event/event-util.c index a3101227af..2338df1d62 100644 --- a/src/libsystemd/sd-event/event-util.c +++ b/src/libsystemd/sd-event/event-util.c @@ -151,3 +151,20 @@ int event_add_time_change(sd_event *e, sd_event_source **ret, sd_event_io_handle return 0; } + +int event_add_child_pidref( + sd_event *e, + sd_event_source **s, + const PidRef *pid, + int options, + sd_event_child_handler_t callback, + void *userdata) { + + if (!pidref_is_set(pid)) + return -ESRCH; + + if (pid->fd >= 0) + return sd_event_add_child_pidfd(e, s, pid->fd, options, callback, userdata); + + return sd_event_add_child(e, s, pid->pid, options, callback, userdata); +} diff --git a/src/libsystemd/sd-event/event-util.h b/src/libsystemd/sd-event/event-util.h index c185584412..6259d5ae25 100644 --- a/src/libsystemd/sd-event/event-util.h +++ b/src/libsystemd/sd-event/event-util.h @@ -5,6 +5,8 @@ #include "sd-event.h" +#include "pidref.h" + int event_reset_time( sd_event *e, sd_event_source **s, @@ -32,3 +34,5 @@ static inline int event_source_disable(sd_event_source *s) { } int event_add_time_change(sd_event *e, sd_event_source **ret, sd_event_io_handler_t callback, void *userdata); + +int event_add_child_pidref(sd_event *e, sd_event_source **s, const PidRef *pid, int options, sd_event_child_handler_t callback, void *userdata); From 6cff1854f913f82e7356b8515b4da73f2bdf52cd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:03:49 +0100 Subject: [PATCH 02/11] vmspawn: rework how we spawn qemu off Let's swicth to PidRef for tracking the qemu child. let's also set a bunch of flags that ensure the child inherits less of of context. --- src/vmspawn/vmspawn.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index e6ab63a6cd..40ebe72c89 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -13,6 +13,7 @@ #include "copy.h" #include "creds-util.h" #include "escape.h" +#include "event-util.h" #include "fileio.h" #include "format-util.h" #include "fs-util.h" @@ -391,12 +392,16 @@ static int setup_notify_parent(sd_event *event, int fd, int *exit_status, sd_eve } static int on_orderly_shutdown(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { - pid_t pid; + PidRef *pidref = userdata; + int r; - pid = PTR_TO_PID(userdata); - if (pid > 0) { - /* TODO: actually talk to qemu and ask the guest to shutdown here */ - if (kill(pid, SIGKILL) >= 0) { + /* TODO: actually talk to qemu and ask the guest to shutdown here */ + + if (pidref) { + r = pidref_kill(pidref, SIGKILL); + if (r < 0) + log_warning_errno(r, "Failed to kill qemu, terminating: %m"); + else { log_info("Trying to halt qemu. Send SIGTERM again to trigger vmspawn to immediately terminate."); sd_event_source_set_userdata(s, NULL); return 0; @@ -663,15 +668,16 @@ static int run_virtual_machine(void) { (void) sd_event_set_watchdog(event, true); - pid_t child_pid; - r = safe_fork_full( + _cleanup_(pidref_done) PidRef child_pidref = PIDREF_NULL; + + r = pidref_safe_fork_full( qemu_binary, - NULL, + /* stdio_fds= */ NULL, &child_vsock_fd, 1, /* pass the vsock fd to qemu */ - FORK_CLOEXEC_OFF, - &child_pid); + FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_CLOEXEC_OFF|FORK_RLIMIT_NOFILE_SAFE, + &child_pidref); if (r < 0) - return log_error_errno(r, "Failed to fork off %s: %m", qemu_binary); + return r; if (r == 0) { /* set TERM and LANG if they are missing */ if (setenv("TERM", "vt220", 0) < 0) @@ -693,13 +699,13 @@ static int run_virtual_machine(void) { } /* shutdown qemu when we are shutdown */ - (void) sd_event_add_signal(event, NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_orderly_shutdown, PID_TO_PTR(child_pid)); - (void) sd_event_add_signal(event, NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_orderly_shutdown, PID_TO_PTR(child_pid)); + (void) sd_event_add_signal(event, NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_orderly_shutdown, &child_pidref); + (void) sd_event_add_signal(event, NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_orderly_shutdown, &child_pidref); (void) sd_event_add_signal(event, NULL, (SIGRTMIN+18) | SD_EVENT_SIGNAL_PROCMASK, sigrtmin18_handler, NULL); /* Exit when the child exits */ - (void) sd_event_add_child(event, NULL, child_pid, WEXITED, on_child_exit, NULL); + (void) event_add_child_pidref(event, NULL, &child_pidref, WEXITED, on_child_exit, NULL); r = sd_event_loop(event); if (r < 0) From 400da3e424bea7acdc25362c2f5b4e1c953aa24b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:04:56 +0100 Subject: [PATCH 03/11] vmspawn: modernize setup_notify_parent() a bit --- src/vmspawn/vmspawn.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index 40ebe72c89..d846a4a335 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -379,14 +379,19 @@ static int vmspawn_dispatch_vsock_connections(sd_event_source *source, int fd, u return 0; } -static int setup_notify_parent(sd_event *event, int fd, int *exit_status, sd_event_source **notify_event_source) { +static int setup_notify_parent(sd_event *event, int fd, int *exit_status, sd_event_source **ret_notify_event_source) { int r; - r = sd_event_add_io(event, notify_event_source, fd, EPOLLIN, vmspawn_dispatch_vsock_connections, exit_status); + assert(event); + assert(fd >= 0); + assert(exit_status); + assert(ret_notify_event_source); + + r = sd_event_add_io(event, ret_notify_event_source, fd, EPOLLIN, vmspawn_dispatch_vsock_connections, exit_status); if (r < 0) return log_error_errno(r, "Failed to allocate notify socket event source: %m"); - (void) sd_event_source_set_description(*notify_event_source, "vmspawn-notify-sock"); + (void) sd_event_source_set_description(*ret_notify_event_source, "vmspawn-notify-sock"); return 0; } From 02aacdcf9658b87eb6c43ca7bebac2448046c1c5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:05:28 +0100 Subject: [PATCH 04/11] vmspawn: use execv() rather than execve() if we just want to pass 'environ' as env block --- src/vmspawn/vmspawn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index d846a4a335..a136ae4c3c 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -691,7 +691,7 @@ static int run_virtual_machine(void) { if (setenv("LANG", "C.UTF-8", 0) < 0) return log_oom(); - execve(qemu_binary, cmdline, environ); + execv(qemu_binary, cmdline); log_error_errno(errno, "Failed to execve %s: %m", qemu_binary); _exit(EXIT_FAILURE); } From c7c6b6f28c2c006c059d40d5d6a7032f42ad8252 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:05:32 +0100 Subject: [PATCH 05/11] vmspawn: close host vsock fd once we passed it to the child Without this I qemu simply froze in a weird state for me if I kill it: it was supposedly a zombie, but we'd get the pidfd POLLIN event for it only once the fd is closed. Hence let's close it right-away. (Smells like a kernel issue actually, but too lazy to bother with this). --- src/vmspawn/vmspawn.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index a136ae4c3c..5341699b79 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -696,6 +696,9 @@ static int run_virtual_machine(void) { _exit(EXIT_FAILURE); } + /* Close the vsock fd we passed to qemu in the parent. We don't need it anymore. */ + child_vsock_fd = safe_close(child_vsock_fd); + int exit_status = INT_MAX; if (use_vsock) { r = setup_notify_parent(event, vsock_fd, &exit_status, ¬ify_event_source); From fdc7d50ba40a92348d1ea31dfeda4ee4cc804046 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:47:41 +0100 Subject: [PATCH 06/11] vmspawn: ENODEV is not the only error the kernel returns if a device is not there Let's use ERRNO_IS_DEVICE_ABSENT() to cover all cases. (And while we are at it also use ERRNO_IS_PRIVILEGE() where appropriate) --- src/vmspawn/vmspawn-util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vmspawn/vmspawn-util.c b/src/vmspawn/vmspawn-util.c index 16ece31809..c8e6759790 100644 --- a/src/vmspawn/vmspawn-util.c +++ b/src/vmspawn/vmspawn-util.c @@ -40,7 +40,7 @@ int qemu_check_kvm_support(void) { log_debug_errno(errno, "/dev/kvm not found. Not using KVM acceleration."); return false; } - if (errno == EPERM) { + if (ERRNO_IS_PRIVILEGE(errno)) { log_debug_errno(errno, "Permission denied to access /dev/kvm. Not using KVM acceleration."); return false; } @@ -62,11 +62,11 @@ int qemu_check_vsock_support(void) { fd = open("/dev/vhost-vsock", O_RDWR|O_CLOEXEC); if (fd >= 0) return true; - if (errno == ENODEV) { + if (ERRNO_IS_DEVICE_ABSENT(errno)) { log_debug_errno(errno, "/dev/vhost-vsock device doesn't exist. Not adding a vsock device to the virtual machine."); return false; } - if (errno == EPERM) { + if (ERRNO_IS_PRIVILEGE(errno)) { log_debug_errno(errno, "Permission denied to access /dev/vhost-vsock. Not adding a vsock device to the virtual machine."); return false; } From e8ce204d8659462be9ebafb553985ade910c0916 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 18:50:43 +0100 Subject: [PATCH 07/11] vmspawn: rework firmware selection logic Let's make the firmware file to choose configurable, and enumeratable. This adds --firmware= to select the formare, and in particular --firmware=list to show available options. --- man/systemd-vmspawn.xml | 13 ++- src/vmspawn/vmspawn-util.c | 194 ++++++++++++++++++++++++++----------- src/vmspawn/vmspawn-util.h | 4 +- src/vmspawn/vmspawn.c | 35 ++++++- 4 files changed, 188 insertions(+), 58 deletions(-) diff --git a/man/systemd-vmspawn.xml b/man/systemd-vmspawn.xml index cd46f823ad..351b7c0772 100644 --- a/man/systemd-vmspawn.xml +++ b/man/systemd-vmspawn.xml @@ -135,7 +135,18 @@ - + + + PATH + + Takes an absolute path, or a relative path beginning with + ./. Specifies a JSON firmware definition file, which allows selecting the + firmware to boot in the VM. If not specified a suitable firmware is automatically discovered. If the + special string list is specified lists all discovered firmwares. + + + + System Identity Options diff --git a/src/vmspawn/vmspawn-util.c b/src/vmspawn/vmspawn-util.c index c8e6759790..a66352c510 100644 --- a/src/vmspawn/vmspawn-util.c +++ b/src/vmspawn/vmspawn-util.c @@ -81,6 +81,12 @@ typedef struct FirmwareData { char *vars; } FirmwareData; +static bool firmware_data_supports_sb(const FirmwareData *fwd) { + assert(fwd); + + return strv_contains(fwd->features, "secure-boot"); +} + static FirmwareData* firmware_data_free(FirmwareData *fwd) { if (!fwd) return NULL; @@ -124,12 +130,134 @@ static int firmware_mapping(const char *name, JsonVariant *v, JsonDispatchFlags return json_dispatch(v, table, flags, userdata); } +static int get_firmware_search_dirs(char ***ret) { + int r; + + assert(ret); + + /* Search in: + * - $XDG_CONFIG_HOME/qemu/firmware + * - /etc/qemu/firmware + * - /usr/share/qemu/firmware + * + * Prioritising entries in "more specific" directories */ + + _cleanup_free_ char *user_firmware_dir = NULL; + r = xdg_user_config_dir(&user_firmware_dir, "/qemu/firmware"); + if (r < 0) + return r; + + _cleanup_strv_free_ char **l = NULL; + l = strv_new(user_firmware_dir, "/etc/qemu/firmware", "/usr/share/qemu/firmware"); + if (!l) + return log_oom_debug(); + + *ret = TAKE_PTR(l); + return 0; +} + +int list_ovmf_config(char ***ret) { + _cleanup_strv_free_ char **search_dirs = NULL; + int r; + + assert(ret); + + r = get_firmware_search_dirs(&search_dirs); + if (r < 0) + return r; + + r = conf_files_list_strv( + ret, + ".json", + /* root= */ NULL, + CONF_FILES_FILTER_MASKED|CONF_FILES_REGULAR, + (const char *const*) search_dirs); + if (r < 0) + return log_debug_errno(r, "Failed to list firmware files: %m"); + + return 0; +} + +static int load_firmware_data(const char *path, FirmwareData **ret) { + int r; + + assert(path); + assert(ret); + + _cleanup_(json_variant_unrefp) JsonVariant *json = NULL; + r = json_parse_file( + /* f= */ NULL, + path, + /* flags= */ 0, + &json, + /* ret_line= */ NULL, + /* ret_column= */ NULL); + if (r < 0) + return r; + + static const JsonDispatch table[] = { + { "description", JSON_VARIANT_STRING, NULL, 0, JSON_MANDATORY }, + { "interface-types", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, + { "mapping", JSON_VARIANT_OBJECT, firmware_mapping, 0, JSON_MANDATORY }, + { "targets", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, + { "features", JSON_VARIANT_ARRAY, json_dispatch_strv, offsetof(FirmwareData, features), JSON_MANDATORY }, + { "tags", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, + {} + }; + + _cleanup_(firmware_data_freep) FirmwareData *fwd = NULL; + fwd = new0(FirmwareData, 1); + if (!fwd) + return -ENOMEM; + + r = json_dispatch(json, table, JSON_ALLOW_EXTENSIONS, fwd); + if (r < 0) + return r; + + *ret = TAKE_PTR(fwd); + return 0; +} + +static int ovmf_config_make(FirmwareData *fwd, OvmfConfig **ret) { + assert(fwd); + assert(ret); + + _cleanup_free_ OvmfConfig *config = NULL; + config = new(OvmfConfig, 1); + if (!config) + return -ENOMEM; + + *config = (OvmfConfig) { + .path = TAKE_PTR(fwd->firmware), + .vars = TAKE_PTR(fwd->vars), + .supports_sb = firmware_data_supports_sb(fwd), + }; + + *ret = TAKE_PTR(config); + return 0; +} + +int load_ovmf_config(const char *path, OvmfConfig **ret) { + _cleanup_(firmware_data_freep) FirmwareData *fwd = NULL; + int r; + + assert(path); + assert(ret); + + r = load_firmware_data(path, &fwd); + if (r < 0) + return r; + + return ovmf_config_make(fwd, ret); +} + int find_ovmf_config(int search_sb, OvmfConfig **ret) { _cleanup_(ovmf_config_freep) OvmfConfig *config = NULL; - _cleanup_free_ char *user_firmware_dir = NULL; _cleanup_strv_free_ char **conf_files = NULL; int r; + assert(ret); + /* Search in: * - $XDG_CONFIG_HOME/qemu/firmware * - /etc/qemu/firmware @@ -138,79 +266,35 @@ int find_ovmf_config(int search_sb, OvmfConfig **ret) { * Prioritising entries in "more specific" directories */ - r = xdg_user_config_dir(&user_firmware_dir, "/qemu/firmware"); + r = list_ovmf_config(&conf_files); if (r < 0) return r; - r = conf_files_list_strv(&conf_files, ".json", NULL, CONF_FILES_FILTER_MASKED|CONF_FILES_REGULAR, - STRV_MAKE_CONST(user_firmware_dir, "/etc/qemu/firmware", "/usr/share/qemu/firmware")); - if (r < 0) - return log_debug_errno(r, "Failed to list config files: %m"); - STRV_FOREACH(file, conf_files) { _cleanup_(firmware_data_freep) FirmwareData *fwd = NULL; - _cleanup_(json_variant_unrefp) JsonVariant *config_json = NULL; - _cleanup_free_ char *contents = NULL; - size_t contents_sz = 0; - r = read_full_file(*file, &contents, &contents_sz); - if (r == -ENOMEM) - return r; + r = load_firmware_data(*file, &fwd); if (r < 0) { - log_debug_errno(r, "Failed to read contents of %s - ignoring: %m", *file); - continue; - } - - r = json_parse(contents, 0, &config_json, NULL, NULL); - if (r == -ENOMEM) - return r; - if (r < 0) { - log_debug_errno(r, "Failed to parse the JSON in %s - ignoring: %m", *file); - continue; - } - - static const JsonDispatch table[] = { - { "description", JSON_VARIANT_STRING, NULL, 0, JSON_MANDATORY }, - { "interface-types", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, - { "mapping", JSON_VARIANT_OBJECT, firmware_mapping, 0, JSON_MANDATORY }, - { "targets", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, - { "features", JSON_VARIANT_ARRAY, json_dispatch_strv, offsetof(FirmwareData, features), JSON_MANDATORY }, - { "tags", JSON_VARIANT_ARRAY, NULL, 0, JSON_MANDATORY }, - {} - }; - - fwd = new0(FirmwareData, 1); - if (!fwd) - return -ENOMEM; - - r = json_dispatch(config_json, table, JSON_ALLOW_EXTENSIONS, fwd); - if (r == -ENOMEM) - return r; - if (r < 0) { - log_debug_errno(r, "Failed to extract the required fields from the JSON in %s - ignoring: %m", *file); + log_debug_errno(r, "Failed to load JSON file '%s', skipping: %m", *file); continue; } if (strv_contains(fwd->features, "enrolled-keys")) { - log_debug("Skipping %s, firmware has enrolled keys which has been known to cause issues", *file); + log_debug("Skipping %s, firmware has enrolled keys which has been known to cause issues.", *file); continue; } - bool sb_present = strv_contains(fwd->features, "secure-boot"); - /* exclude firmware which doesn't match our Secure Boot requirements */ - if (search_sb >= 0 && search_sb != sb_present) { - log_debug("Skipping %s, firmware doesn't fit required Secure Boot configuration", *file); + if (search_sb >= 0 && !!search_sb != firmware_data_supports_sb(fwd)) { + log_debug("Skipping %s, firmware doesn't fit required Secure Boot configuration.", *file); continue; } - config = new0(OvmfConfig, 1); - if (!config) - return -ENOMEM; + r = ovmf_config_make(fwd, &config); + if (r < 0) + return r; - config->path = TAKE_PTR(fwd->firmware); - config->vars = TAKE_PTR(fwd->vars); - config->supports_sb = sb_present; + log_debug("Selected firmware definition %s.", *file); break; } diff --git a/src/vmspawn/vmspawn-util.h b/src/vmspawn/vmspawn-util.h index c931e66aac..3617bb07d6 100644 --- a/src/vmspawn/vmspawn-util.h +++ b/src/vmspawn/vmspawn-util.h @@ -31,6 +31,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(OvmfConfig*, ovmf_config_free); int qemu_check_kvm_support(void); int qemu_check_vsock_support(void); -int find_ovmf_config(int search_sb, OvmfConfig **ret_ovmf_config); +int list_ovmf_config(char ***ret); +int load_ovmf_config(const char *path, OvmfConfig **ret); +int find_ovmf_config(int search_sb, OvmfConfig **ret); int find_qemu_binary(char **ret_qemu_binary); int vsock_fix_child_cid(unsigned *machine_cid, const char *machine, int *ret_child_sock); diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index 5341699b79..bcc3e1662a 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -49,12 +49,14 @@ static int arg_secure_boot = -1; static MachineCredentialContext arg_credentials = {}; static SettingsMask arg_settings_mask = 0; static char **arg_parameters = NULL; +static char *arg_firmware = NULL; STATIC_DESTRUCTOR_REGISTER(arg_image, freep); STATIC_DESTRUCTOR_REGISTER(arg_machine, freep); STATIC_DESTRUCTOR_REGISTER(arg_qemu_smp, freep); STATIC_DESTRUCTOR_REGISTER(arg_parameters, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_credentials, machine_credential_context_done); +STATIC_DESTRUCTOR_REGISTER(arg_firmware, freep); static int help(void) { _cleanup_free_ char *link = NULL; @@ -83,6 +85,7 @@ static int help(void) { " --qemu-gui Start QEMU in graphical mode\n" " --secure-boot=BOOL Configure whether to search for firmware which\n" " supports Secure Boot\n" + " --firmware=PATH|list Select firmware definition file (or list available)\n" "\n%3$sSystem Identity:%4$s\n" " -M --machine=NAME Set the machine name for the container\n" "\n%3$sCredentials:%4$s\n" @@ -115,6 +118,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_SECURE_BOOT, ARG_SET_CREDENTIAL, ARG_LOAD_CREDENTIAL, + ARG_FIRMWARE, }; static const struct option options[] = { @@ -132,6 +136,7 @@ static int parse_argv(int argc, char *argv[]) { { "secure-boot", required_argument, NULL, ARG_SECURE_BOOT }, { "set-credential", required_argument, NULL, ARG_SET_CREDENTIAL }, { "load-credential", required_argument, NULL, ARG_LOAD_CREDENTIAL }, + { "firmware", required_argument, NULL, ARG_FIRMWARE }, {} }; @@ -242,6 +247,31 @@ static int parse_argv(int argc, char *argv[]) { break; } + case ARG_FIRMWARE: + if (streq(optarg, "list")) { + _cleanup_strv_free_ char **l = NULL; + + r = list_ovmf_config(&l); + if (r < 0) + return log_error_errno(r, "Failed to list firmwares: %m"); + + bool nl = false; + fputstrv(stdout, l, "\n", &nl); + if (nl) + putchar('\n'); + + return 0; + } + + if (!isempty(optarg) && !path_is_absolute(optarg) && !startswith(optarg, "./")) + return log_error_errno(SYNTHETIC_ERRNO(errno), "Absolute path or path starting with './' required."); + + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_firmware); + if (r < 0) + return r; + + break; + case '?': return -EINVAL; @@ -460,7 +490,10 @@ static int run_virtual_machine(void) { use_kvm = r; } - r = find_ovmf_config(arg_secure_boot, &ovmf_config); + if (arg_firmware) + r = load_ovmf_config(arg_firmware, &ovmf_config); + else + r = find_ovmf_config(arg_secure_boot, &ovmf_config); if (r < 0) return log_error_errno(r, "Failed to find OVMF config: %m"); From 71e42c36d0be3b9993481484406599c847f96706 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 19:14:09 +0100 Subject: [PATCH 08/11] vmspawn: make sure to pass the right firmware image type to qemu The JSON data reports whether firmware types are qcow or raw. Let's pass that into the qemu cmdline. --- src/vmspawn/vmspawn-util.c | 22 +++++++++++++++------- src/vmspawn/vmspawn-util.h | 10 ++++++++++ src/vmspawn/vmspawn.c | 4 ++-- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/vmspawn/vmspawn-util.c b/src/vmspawn/vmspawn-util.c index a66352c510..79e1e5399a 100644 --- a/src/vmspawn/vmspawn-util.c +++ b/src/vmspawn/vmspawn-util.c @@ -29,7 +29,9 @@ OvmfConfig* ovmf_config_free(OvmfConfig *config) { return NULL; free(config->path); + free(config->format); free(config->vars); + free(config->vars_format); return mfree(config); } @@ -78,7 +80,9 @@ int qemu_check_vsock_support(void) { typedef struct FirmwareData { char **features; char *firmware; + char *firmware_format; char *vars; + char *vars_format; } FirmwareData; static bool firmware_data_supports_sb(const FirmwareData *fwd) { @@ -91,9 +95,11 @@ static FirmwareData* firmware_data_free(FirmwareData *fwd) { if (!fwd) return NULL; - fwd->features = strv_free(fwd->features); - fwd->firmware = mfree(fwd->firmware); - fwd->vars = mfree(fwd->vars); + strv_free(fwd->features); + free(fwd->firmware); + free(fwd->firmware_format); + free(fwd->vars); + free(fwd->vars_format); return mfree(fwd); } @@ -101,8 +107,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(FirmwareData*, firmware_data_free); static int firmware_executable(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { static const JsonDispatch table[] = { - { "filename", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, firmware), JSON_MANDATORY }, - { "format", JSON_VARIANT_STRING, NULL, 0, JSON_MANDATORY }, + { "filename", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, firmware), JSON_MANDATORY }, + { "format", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, firmware_format), JSON_MANDATORY }, {} }; @@ -111,8 +117,8 @@ static int firmware_executable(const char *name, JsonVariant *v, JsonDispatchFla static int firmware_nvram_template(const char *name, JsonVariant *v, JsonDispatchFlags flags, void *userdata) { static const JsonDispatch table[] = { - { "filename", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, vars), JSON_MANDATORY }, - { "format", JSON_VARIANT_STRING, NULL, 0, JSON_MANDATORY }, + { "filename", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, vars), JSON_MANDATORY }, + { "format", JSON_VARIANT_STRING, json_dispatch_string, offsetof(FirmwareData, vars_format), JSON_MANDATORY }, {} }; @@ -229,7 +235,9 @@ static int ovmf_config_make(FirmwareData *fwd, OvmfConfig **ret) { *config = (OvmfConfig) { .path = TAKE_PTR(fwd->firmware), + .format = TAKE_PTR(fwd->firmware_format), .vars = TAKE_PTR(fwd->vars), + .vars_format = TAKE_PTR(fwd->vars_format), .supports_sb = firmware_data_supports_sb(fwd), }; diff --git a/src/vmspawn/vmspawn-util.h b/src/vmspawn/vmspawn-util.h index 3617bb07d6..f2d1b621e2 100644 --- a/src/vmspawn/vmspawn-util.h +++ b/src/vmspawn/vmspawn-util.h @@ -22,10 +22,20 @@ typedef struct OvmfConfig { char *path; + char *format; char *vars; + char *vars_format; bool supports_sb; } OvmfConfig; +static inline const char *ovmf_config_format(const OvmfConfig *c) { + return ASSERT_PTR(c)->format ?: "raw"; +} + +static inline const char *ovmf_config_vars_format(const OvmfConfig *c) { + return ASSERT_PTR(c)->vars_format ?: "raw"; +} + OvmfConfig* ovmf_config_free(OvmfConfig *ovmf_config); DEFINE_TRIVIAL_CLEANUP_FUNC(OvmfConfig*, ovmf_config_free); diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index bcc3e1662a..08d5a647ed 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -601,7 +601,7 @@ static int run_virtual_machine(void) { if (r < 0) return log_oom(); - r = strv_extendf(&cmdline, "if=pflash,format=raw,readonly=on,file=%s", ovmf_config->path); + r = strv_extendf(&cmdline, "if=pflash,format=%s,readonly=on,file=%s", ovmf_config_format(ovmf_config), ovmf_config->path); if (r < 0) return log_oom(); @@ -639,7 +639,7 @@ static int run_virtual_machine(void) { if (r < 0) return log_oom(); - r = strv_extendf(&cmdline, "file=%s,if=pflash,format=raw", ovmf_vars_to); + r = strv_extendf(&cmdline, "file=%s,if=pflash,format=%s", ovmf_vars_to, ovmf_config_format(ovmf_config)); if (r < 0) return log_oom(); } From dbb2718f927dfb9202043f6f57a6b37cafaa9fa9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 19:32:36 +0100 Subject: [PATCH 09/11] vmspawn: print a helpful message when we start the VM Let people know how they can exit the VM, it's not obvious after all. --- src/vmspawn/vmspawn.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index 08d5a647ed..b2c040b037 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -36,6 +36,7 @@ #include "vmspawn-settings.h" #include "vmspawn-util.h" +static bool arg_quiet = false; static PagerFlags arg_pager_flags = 0; static char *arg_image = NULL; static char *arg_machine = NULL; @@ -72,6 +73,7 @@ static int help(void) { "%5$sSpawn a command or OS in a virtual machine.%6$s\n\n" " -h --help Show this help\n" " --version Print version string\n" + " -q --quiet Do not show status information\n" " --no-pager Do not pipe output into a pager\n" "\n%3$sImage:%4$s\n" " -i --image=PATH Root file system disk image (or device node) for\n" @@ -124,6 +126,7 @@ static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, ARG_VERSION }, + { "quiet", no_argument, NULL, 'q' }, { "no-pager", no_argument, NULL, ARG_NO_PAGER }, { "image", required_argument, NULL, 'i' }, { "machine", required_argument, NULL, 'M' }, @@ -146,7 +149,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argv); optind = 0; - while ((c = getopt_long(argc, argv, "+hi:M", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "+hi:Mq", options, NULL)) >= 0) switch (c) { case 'h': return help(); @@ -154,6 +157,10 @@ static int parse_argv(int argc, char *argv[]) { case ARG_VERSION: return version(); + case 'q': + arg_quiet = true; + break; + case 'i': r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); if (r < 0) @@ -804,6 +811,11 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; + if (!arg_quiet) + log_info("Spawning VM %s on %s.\n" + "Press Ctrl-a x to kill VM.", + arg_machine, arg_image); + assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0); return run_virtual_machine(); From 3df61bf00cee67f93fe31dffde9e8df3a83a411d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 23:12:05 +0100 Subject: [PATCH 10/11] vmspawn: make "-m" value formatting independent of locale We cannot format the memory string via printf() %f format strings, since that's locale dependent and qemu doesn't like that. hence format this as an integer. We'll lose sub-MiB accuracy, but systems with less than 1 MiB memory don't really make much sense anyway. --- src/vmspawn/vmspawn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index b2c040b037..d93b6b29c9 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -41,7 +41,7 @@ static PagerFlags arg_pager_flags = 0; static char *arg_image = NULL; static char *arg_machine = NULL; static char *arg_qemu_smp = NULL; -static uint64_t arg_qemu_mem = 2ULL * 1024ULL * 1024ULL * 1024ULL; +static uint64_t arg_qemu_mem = UINT64_C(2) * U64_GB; static int arg_qemu_kvm = -1; static int arg_qemu_vsock = -1; static unsigned arg_vsock_cid = VMADDR_CID_ANY; @@ -523,7 +523,7 @@ static int run_virtual_machine(void) { if (r < 0) return log_error_errno(r, "Failed to find QEMU binary: %m"); - if (asprintf(&mem, "%.4fM", (double)arg_qemu_mem / (1024.0 * 1024.0)) < 0) + if (asprintf(&mem, "%" PRIu64, DIV_ROUND_UP(arg_qemu_mem, U64_MB)) < 0) return log_oom(); cmdline = strv_new( From 1807baa931e4afa6d0c61fb9582f6529b25189eb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 19 Jan 2024 23:23:48 +0100 Subject: [PATCH 11/11] nspawn,vmspawn: let's add some terminal magic to the welcome text Let's grey the text out, and prefix it with a vertical grey bar, to make clear this is output from the host, not the payload, and make it clearly distinguishable from what follows. Let's also make the image name clickable (with new enough shared-mime-info this should allow you to look into the image with gnome-disk-utility or a similar tool. --- src/nspawn/nspawn.c | 13 ++++++++++--- src/vmspawn/vmspawn.c | 13 +++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 445d85da6f..c48b51323b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5943,9 +5943,16 @@ static int run(int argc, char *argv[]) { if (arg_console_mode == CONSOLE_PIPE) /* if we pass STDERR on to the container, don't add our own logs into it too */ arg_quiet = true; - if (!arg_quiet) - log_info("Spawning container %s on %s.\nPress Ctrl-] three times within 1s to kill container.", - arg_machine, arg_image ?: arg_directory); + if (!arg_quiet) { + const char *t = arg_image ?: arg_directory; + _cleanup_free_ char *u = NULL; + (void) terminal_urlify_path(t, t, &u); + + log_info("%s %sSpawning container %s on %s.%s\n" + "%s %sPress %sCtrl-]%s three times within 1s to kill container.%s", + special_glyph(SPECIAL_GLYPH_LIGHT_SHADE), ansi_grey(), arg_machine, u ?: t, ansi_normal(), + special_glyph(SPECIAL_GLYPH_LIGHT_SHADE), ansi_grey(), ansi_highlight(), ansi_grey(), ansi_normal()); + } assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, SIGWINCH, SIGTERM, SIGINT, SIGRTMIN+18, -1) >= 0); diff --git a/src/vmspawn/vmspawn.c b/src/vmspawn/vmspawn.c index d93b6b29c9..a08bc4c679 100644 --- a/src/vmspawn/vmspawn.c +++ b/src/vmspawn/vmspawn.c @@ -811,10 +811,15 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; - if (!arg_quiet) - log_info("Spawning VM %s on %s.\n" - "Press Ctrl-a x to kill VM.", - arg_machine, arg_image); + if (!arg_quiet) { + _cleanup_free_ char *u = NULL; + (void) terminal_urlify_path(arg_image, arg_image, &u); + + log_info("%s %sSpawning VM %s on %s.%s\n" + "%s %sPress %sCtrl-a x%s to kill VM.%s", + special_glyph(SPECIAL_GLYPH_LIGHT_SHADE), ansi_grey(), arg_machine, u ?: arg_image, ansi_normal(), + special_glyph(SPECIAL_GLYPH_LIGHT_SHADE), ansi_grey(), ansi_highlight(), ansi_grey(), ansi_normal()); + } assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0);