@@ -332,7 +332,39 @@ const char *libxl__domain_device_model(libxl__gc *gc,
dm = libxl__abs_path(gc, "qemu-dm", libxl__private_bindir_path());
break;
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- dm = qemu_xen_path(gc);
+ const char *configured_dm = qemu_xen_path(gc);
+ if (strchr(configured_dm, '/'))
+ dm = libxl__strdup(gc, configured_dm);
+ else
+ {
+ const char *path_env = getenv("PATH");
+ if (!path_env)
+ dm = libxl__strdup(gc, configured_dm);
+ else
+ {
+ char *path_dup = libxl__strdup(gc, path_env);
+ char *saveptr;
+
+ char *path = strtok_r(path_dup, ":", &saveptr);
+ char fullpath[PATH_MAX];
+ bool dm_found = false;
+ while (path)
+ {
+ snprintf(fullpath, sizeof(fullpath), "%s/%s", path,
+ configured_dm);
+ if (access(fullpath, X_OK) == 0)
+ {
+ dm = libxl__strdup(gc, fullpath);
+ dm_found = true;
+ break;
+ }
+ path = strtok_r(NULL, ":", &saveptr);
+ }
+
+ if (!dm_found)
+ dm = libxl__strdup(gc, configured_dm);
+ }
+ }
break;
default:
LOG(ERROR, "invalid device model version %d",
`QEMU_XEN_PATH` will be configured as `qemu-system-i386` with no clue where, if `--with-system-qemu` is set without giving a path (as matched in the case `yes` but not `*`). However, the existence of the executable is checked by `access()`, that will not look for anywhere in $PATH but the current directory. And since it is possible for `qemu-system-i386` (or any other configured values) to be executed from PATH later, we'd better find that in PATH and return the full path for the caller to check against. Signed-off-by: Hongbo <hehongbo@mail.com> --- This patch is from the maintenance team of the Xen Project Hypervisor at NixOS. We encountered this and thought it was an edge case, and came up with this while maintaining the package and module of the Hypervisor at NixOS. According to `xen.git/tools/configure.ac`, `QEMU_XEN_PATH` will be configured as `qemu-system-i386` (relative) if `--with-system-qemu` is set (as `yes`) but without an absolute path. However, it will execute `qemu-system-i386` from the `PATH` only if a file is called `qemu-system-i386` in the *current directory*. That is because the existence of the device model executable file, in this case `qemu-system-i386`, is checked via `access()` without concatenating it with current PATHs. And `access()` is not tailored for executables, it will not search for the PATHs for us. See `libxl__domain_build_info_setdefault()` at `xen.git/tools/libs/light/libxl_create.c`. It reads `dm` from `libxl__domain_device_model()` and then uses `access()` on it. If that fails, it will modify the `device_model_version` to qemu-traditional. Then, in `libxl__spawn_local_dm()` at `xen.git/tools/lib/light/libxl_dm.c`, it reads from `libxl__domain_device_model()` again, and `access()` is used again to detect the file's existence. In my investigations, if I comment out these 2 existence checks then it will run `qemu-system-i386` from the current PATH without issues. I guess if it's not blocked by those 2 checks, it will finally reach `libxl__exec()`. Then, inside the `libxl__exec()` the device model executable will be executed by `execvp()`, which can certainly call the executable from both absolute paths and current PATHs. Since the device model executable will be checked twice, both of which will call `libxl__domain_device_model()` to get its location, I think the preferred solution would be patching the `libxl__domain_device_model()` function itself to tell if we're referring to an executable in PATHs, and resolve to the full path of it for the caller to check against. It's indeed an edge case. But why would we need this? Because in Nix (the package manager) and NixOS, we use Nix expressions to declare dependencies on the dependents, and we ran into the issue of circular dependency - to build QEMU with Xen support, we should give the Xen header and libraries into the building process of QEMU, that makes Xen (`pkgs.xen`) a dependency of QEMU (`pkgs.qemu_xen`), which prevents us from using `pkgs.qemu_xen` in the building process of Xen, and in `--with-system-qemu=` argument in particular. It is very different compared to those distros and package managers that follow the Filesystem Hierarchy Standard (FHS), in which Xen can be built with `--with-system-qemu=` points to a non-existent FHS location of `qemu-system-i386`, and then use these Xen libraries from the artifacts to build QEMU afterward. So we decide to build Xen with `--with-system-qemu` but not including an executable path, taking advantage of the fact that `QEMU_XEN_PATH` can be configured as a relative `qemu-system-i386` when omitted, as declared as the `yes` case in `xen.git/tools/configure.ac`, and that results in we finding the aforementioned "current directory" issue, and submitting this patch. In the patch, I'm using the existence of slash (`/`) to tell if `QEMU_XEN_PATH` is relative, and begin to search in PATH if it is. I'm sort of iffy on this, would it make more sense if we do this on inputs starting with a slash instead? And should we notify the user if it's not found anywhere in the PATH thus proceeding with the value configured in `QEMU_XEN_PATH` as-is? Let me know if it's appropriate and if further changes are needed. Best regards, Hongbo --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Juergen Gross <jgross@suse.com> --- tools/libs/light/libxl_dm.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) -- 2.39.5 (Apple Git-154)