On Tue, Jul 16, 2024 at 07:52:00PM +0900, Akihiko Odaki wrote: > On 2024/07/16 18:53, Daniel P. Berrangé wrote: > > On Tue, Jul 16, 2024 at 04:27:31PM +0900, Akihiko Odaki wrote: > > > qemu_get_runtime_dir() returns a dynamically allocated directory path > > > that is appropriate for storing runtime files. It corresponds to "run" > > > directory in Unix. > > > > > > With a tree-wide search, it was found that there are several cases > > > where such a functionality is implemented so let's have one as a common > > > utlity function. > > > > > > A notable feature of qemu_get_runtime_dir() is that it uses > > > $XDG_RUNTIME_DIR if available. While the function is often called by > > > executables which requires root privileges, it is still possible that > > > they are called from a user without privilege to write the system > > > runtime directory. In fact, I decided to write this patch when I ran > > > virtiofsd in a Linux namespace created by a normal user and realized > > > it tries to write the system runtime directory, not writable in this > > > case. $XDG_RUNTIME_DIR should provide a writable directory in such > > > cases. > > > > > > This function does not use qemu_get_local_state_dir() or its logic > > > for Windows. Actually the implementation of qemu_get_local_state_dir() > > > for Windows seems not right as it calls g_get_system_data_dirs(), > > > which refers to $XDG_DATA_DIRS. In Unix terminology, it is basically > > > "/usr/share", not "/var", which qemu_get_local_state_dir() is intended > > > to provide. Instead, this function try to use the following in order: > > > - $XDG_RUNTIME_DIR > > > - LocalAppData folder > > > - get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run") > > > > > > This function does not use g_get_user_runtime_dir() either as it > > > falls back to g_get_user_cache_dir() when $XDG_DATA_DIRS is not > > > available. In the case, we rather use: > > > get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run") > > > > > > Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com> > > > Message-Id: <20230921075425.16738-2-akihiko.od...@daynix.com> > > > --- > > > include/qemu/osdep.h | 12 ++++++++++++ > > > util/oslib-posix.c | 11 +++++++++++ > > > util/oslib-win32.c | 26 ++++++++++++++++++++++++++ > > > 3 files changed, 49 insertions(+) > > > > > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > > > index 191916f38e6d..fe8609fc1375 100644 > > > --- a/include/qemu/osdep.h > > > +++ b/include/qemu/osdep.h > > > @@ -670,6 +670,18 @@ void qemu_set_cloexec(int fd); > > > */ > > > char *qemu_get_local_state_dir(void); > > > +/** > > > + * qemu_get_runtime_dir: > > > + * > > > + * Return a dynamically allocated directory path that is appropriate for > > > storing > > > + * runtime files. It corresponds to "run" directory in Unix, and uses > > > + * $XDG_RUNTIME_DIR if available. > > > + * > > > + * The caller is responsible for releasing the value returned with > > > g_free() > > > + * after use. > > > + */ > > > +char *qemu_get_runtime_dir(void); > > > + > > > /** > > > * qemu_getauxval: > > > * @type: the auxiliary vector key to lookup > > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > > > index e76441695bdc..9599509a9aa7 100644 > > > --- a/util/oslib-posix.c > > > +++ b/util/oslib-posix.c > > > @@ -278,6 +278,17 @@ qemu_get_local_state_dir(void) > > > return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR); > > > } > > > +char * > > > +qemu_get_runtime_dir(void) > > > +{ > > > + char *env = getenv("XDG_RUNTIME_DIR"); > > > + if (env) { > > > + return g_strdup(env); > > > + } > > > + > > > + return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run"); > > > +} > > > > I'm not convinced this is the correct logic to be following. > > > > In the cover letter you mention not using g_get_user_runtime_dir() > > because it falls back to XDG_CACHE_HOME, and we need to fallback > > to LOCALSTATEDIR/run. This is not right for normal users though, > > where falling back to LOCALSTATEDIR/run is always wrong, as it > > won't be writable - the g_get_user_runtime_dir() fallback is > > desirable for non-root users. > > It also checks LocalAppData, which should be usually available. > > g_get_user_runtime_dir() is not a proper fallback in case neither of > XDG_RUNTIME_DIR and LocalAppData are available. g_get_user_cache_dir(), > which gets called by g_get_user_runtime_dir(), internally uses: > - XDG_CACHE_HOME or > - FOLDERID_InternetCache > > g_get_user_cache_dir() just returns NULL if neither of them is available. > > We can't expect XDG_CACHE_HOME is present when XDG_RUNTIME_DIR is missing. > FOLDERID_InternetCache points to %LOCALAPPDATA%\Microsoft\Windows\Temporary > Internet Files, according to: > https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid > > So we can't expect FOLDERID_InternetCache is available when LocalAppData is > missing.
XDG_CACHE_HOME isn't required to be present. Glib will use a fallback location if XDG_CACHE_HOME isn't set, and it will mkdir() the location if it doesn't exist. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|