[PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()

Akihiko Odaki posted 7 patches 4 months, 1 week ago
Maintainers: Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefan Weil <sw@weilnetz.de>
[PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()
Posted by Akihiko Odaki 4 months, 1 week ago
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.odaki@daynix.com>
Message-Id: <20230921075425.16738-2-akihiko.odaki@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");
+}
+
 void qemu_set_tty_echo(int fd, bool echo)
 {
     struct termios tty;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index b623830d624f..8c5a02ee881d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -27,6 +27,8 @@
  */
 
 #include "qemu/osdep.h"
+#include <shlobj.h>
+#include <wchar.h>
 #include <windows.h>
 #include "qapi/error.h"
 #include "qemu/main-loop.h"
@@ -237,6 +239,30 @@ qemu_get_local_state_dir(void)
     return g_strdup(data_dirs[0]);
 }
 
+char *
+qemu_get_runtime_dir(void)
+{
+    size_t size = GetEnvironmentVariableA("XDG_RUNTIME_DIR", NULL, 0);
+    if (size) {
+        char *env = g_malloc(size);
+        GetEnvironmentVariableA("XDG_RUNTIME_DIR", env, size);
+        return env;
+    }
+
+    PWSTR wpath;
+    const wchar_t *cwpath;
+    if (!SHGetKnownFolderPath(&FOLDERID_LocalAppData, KF_FLAG_DEFAULT, NULL, &wpath)) {
+        cwpath = wpath;
+        size = wcsrtombs(NULL, &cwpath, 0, &(mbstate_t){0}) + 1;
+        char *path = g_malloc(size);
+        wcsrtombs(path, &cwpath, size, &(mbstate_t){0});
+        CoTaskMemFree(wpath);
+        return path;
+    }
+
+    return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
+}
+
 void qemu_set_tty_echo(int fd, bool echo)
 {
     HANDLE handle = (HANDLE)_get_osfhandle(fd);

-- 
2.45.2
Re: [PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()
Posted by Daniel P. Berrangé 4 months, 1 week ago
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.odaki@daynix.com>
> Message-Id: <20230921075425.16738-2-akihiko.odaki@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.

IMHO we should be doing something more like this

#ifndef WIN32
  if (geteuid() == 0) {
     return  get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
  } else {
#endif
     return g_get_user_runtime_dir();
#ifndef WIN32
  }
#endif

> +
>  void qemu_set_tty_echo(int fd, bool echo)
>  {
>      struct termios tty;
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b623830d624f..8c5a02ee881d 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -27,6 +27,8 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include <shlobj.h>
> +#include <wchar.h>
>  #include <windows.h>
>  #include "qapi/error.h"
>  #include "qemu/main-loop.h"
> @@ -237,6 +239,30 @@ qemu_get_local_state_dir(void)
>      return g_strdup(data_dirs[0]);
>  }
>  
> +char *
> +qemu_get_runtime_dir(void)
> +{
> +    size_t size = GetEnvironmentVariableA("XDG_RUNTIME_DIR", NULL, 0);
> +    if (size) {
> +        char *env = g_malloc(size);
> +        GetEnvironmentVariableA("XDG_RUNTIME_DIR", env, size);
> +        return env;
> +    }
> +
> +    PWSTR wpath;
> +    const wchar_t *cwpath;
> +    if (!SHGetKnownFolderPath(&FOLDERID_LocalAppData, KF_FLAG_DEFAULT, NULL, &wpath)) {
> +        cwpath = wpath;
> +        size = wcsrtombs(NULL, &cwpath, 0, &(mbstate_t){0}) + 1;
> +        char *path = g_malloc(size);
> +        wcsrtombs(path, &cwpath, size, &(mbstate_t){0});
> +        CoTaskMemFree(wpath);
> +        return path;
> +    }
> +
> +    return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
> +}
> +
>  void qemu_set_tty_echo(int fd, bool echo)
>  {
>      HANDLE handle = (HANDLE)_get_osfhandle(fd);
> 
> -- 
> 2.45.2
> 
> 

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 :|
Re: [PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()
Posted by Akihiko Odaki 4 months, 1 week ago
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.odaki@daynix.com>
>> Message-Id: <20230921075425.16738-2-akihiko.odaki@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.

Regards,
Akihiko Odaki

Re: [PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()
Posted by Daniel P. Berrangé 4 months, 1 week ago
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.odaki@daynix.com>
> > > Message-Id: <20230921075425.16738-2-akihiko.odaki@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 :|


Re: [PATCH v4 1/7] util: Introduce qemu_get_runtime_dir()
Posted by Akihiko Odaki 4 months, 1 week ago
On 2024/07/16 19:54, Daniel P. Berrangé wrote:
> 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.odaki@daynix.com>
>>>> Message-Id: <20230921075425.16738-2-akihiko.odaki@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.

The fallback location is FOLDERID_InternetCache, which is not better 
than looking at LocalAppData.

Regards,
Akihiko Odaki