include/qemu/osdep.h | 10 +++++++--- contrib/ivshmem-server/main.c | 20 ++++++++++++++++---- qga/main.c | 9 ++++----- scsi/qemu-pr-helper.c | 6 +++--- ui/spice-app.c | 4 ++-- util/module.c | 3 ++- util/oslib-posix.c | 9 +++++++-- util/oslib-win32.c | 24 ++++++++++++++++++++---- 8 files changed, 61 insertions(+), 24 deletions(-)
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")
V2 -> V3:
Rebase to the current master.
Dropped patch "qga: Remove platform GUID definitions" since it is
irrelevant.
V1 -> V2:
Rebased to the current master since Patchew complains.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v4:
- Rebased.
- Link to v3: https://lore.kernel.org/r/20230921075425.16738-1-akihiko.odaki@daynix.com
---
Akihiko Odaki (7):
util: Introduce qemu_get_runtime_dir()
ivshmem-server: Use qemu_get_runtime_dir()
qga: Use qemu_get_runtime_dir()
scsi: Use qemu_get_runtime_dir()
module: Use qemu_get_runtime_dir()
util: Remove qemu_get_local_state_dir()
spice-app: Use qemu_get_runtime_dir()
include/qemu/osdep.h | 10 +++++++---
contrib/ivshmem-server/main.c | 20 ++++++++++++++++----
qga/main.c | 9 ++++-----
scsi/qemu-pr-helper.c | 6 +++---
ui/spice-app.c | 4 ++--
util/module.c | 3 ++-
util/oslib-posix.c | 9 +++++++--
util/oslib-win32.c | 24 ++++++++++++++++++++----
8 files changed, 61 insertions(+), 24 deletions(-)
---
base-commit: f2cb4026fccfe073f84a4b440e41d3ed0c3134f6
change-id: 20240218-run-6f0d91ec7439
Best regards,
--
Akihiko Odaki <akihiko.odaki@daynix.com>
Queued, thanks. Paolo
16.07.2024 10:27, 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. Since runtime dir is always used with a filename within, how about char *qemu_get_runtime_path(const char *filename) which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? Thanks, /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
On Tue, Jul 16, 2024 at 11:06:57AM +0300, Michael Tokarev wrote: > 16.07.2024 10:27, 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. > > Since runtime dir is always used with a filename within, how about > > char *qemu_get_runtime_path(const char *filename) > > which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? Yeah, I agree, every single caller of the function goes on to call g_build_filename with the result. The helper should just be building the filename itself. 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 :|
On Tue, Jul 16, 2024 at 11:56 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Jul 16, 2024 at 11:06:57AM +0300, Michael Tokarev wrote: > > 16.07.2024 10:27, 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. > > > > Since runtime dir is always used with a filename within, how about > > > > char *qemu_get_runtime_path(const char *filename) > > > > which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? > > Yeah, I agree, every single caller of the function goes on to call > g_build_filename with the result. The helper should just be building > the filename itself. That would mean using variable arguments and g_build_filename_valist(). Paolo
On 2024/07/16 19:43, Paolo Bonzini wrote:
> On Tue, Jul 16, 2024 at 11:56 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Tue, Jul 16, 2024 at 11:06:57AM +0300, Michael Tokarev wrote:
>>> 16.07.2024 10:27, 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.
>>>
>>> Since runtime dir is always used with a filename within, how about
>>>
>>> char *qemu_get_runtime_path(const char *filename)
>>>
>>> which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ?
>>
>> Yeah, I agree, every single caller of the function goes on to call
>> g_build_filename with the result. The helper should just be building
>> the filename itself.
>
> That would mean using variable arguments and g_build_filename_valist().
We can't prepend an element to va_list. The best thing I came up with is
a macro as follows:
#define get_runtime_path(first_element, ...) ({ \
g_autofree char *_s = qemu_get_runtime_dir(); \
g_build_filename(_s, first_element, ...); \
})
But it makes me wonder if we need such a macro.
Regards,
Akihiko Odaki
On Tue, Jul 16, 2024 at 2:46 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > On 2024/07/16 19:43, Paolo Bonzini wrote: > > On Tue, Jul 16, 2024 at 11:56 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > >> > >> On Tue, Jul 16, 2024 at 11:06:57AM +0300, Michael Tokarev wrote: > >>> 16.07.2024 10:27, 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. > >>> > >>> Since runtime dir is always used with a filename within, how about > >>> > >>> char *qemu_get_runtime_path(const char *filename) > >>> > >>> which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? > >> > >> Yeah, I agree, every single caller of the function goes on to call > >> g_build_filename with the result. The helper should just be building > >> the filename itself. > > > > That would mean using variable arguments and g_build_filename_valist(). > > We can't prepend an element to va_list. You could do it in two steps, with g_build_filename(runtime_dir, first) followed by g_build_filename_valist(result, ap); doing these steps only if if first != NULL of course. But I agree that leaving the concatenation in the caller is not particularly worse, and makes qemu_get_runtime_dir() more readable. Paolo Paolo
On 2024/07/16 22:29, Paolo Bonzini wrote: > On Tue, Jul 16, 2024 at 2:46 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: >> >> On 2024/07/16 19:43, Paolo Bonzini wrote: >>> On Tue, Jul 16, 2024 at 11:56 AM Daniel P. Berrangé <berrange@redhat.com> wrote: >>>> >>>> On Tue, Jul 16, 2024 at 11:06:57AM +0300, Michael Tokarev wrote: >>>>> 16.07.2024 10:27, 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. >>>>> >>>>> Since runtime dir is always used with a filename within, how about >>>>> >>>>> char *qemu_get_runtime_path(const char *filename) >>>>> >>>>> which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? >>>> >>>> Yeah, I agree, every single caller of the function goes on to call >>>> g_build_filename with the result. The helper should just be building >>>> the filename itself. >>> >>> That would mean using variable arguments and g_build_filename_valist(). >> >> We can't prepend an element to va_list. > > You could do it in two steps, with g_build_filename(runtime_dir, > first) followed by g_build_filename_valist(result, ap); doing these > steps only if if first != NULL of course. It unfortunately involves an extra effort to free the result of the first call of g_build_filename(). Regards, Akihiko Odaki
On 2024/07/16 17:06, Michael Tokarev wrote: > 16.07.2024 10:27, 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. > > Since runtime dir is always used with a filename within, how about > > char *qemu_get_runtime_path(const char *filename) > > which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? I'm not sure. Such a function would be certainly useful, but I slightly feel such a function concerns with too many responsibilities. Getting a runtime directory is one responsibility, and how to use is another. They are clearly distinguished; it does not matter how the path to the runtime directory is used after acquiring it. For example, you can keep the path to the runtime directory, and derive the paths to two files in the directory. I don't object to such a change, but I rather keep this series as is unless there is anything wrong else. Regards, Akihiko Odaki
16.07.2024 12:32, Akihiko Odaki wrote: > On 2024/07/16 17:06, Michael Tokarev wrote: >> Since runtime dir is always used with a filename within, how about >> >> char *qemu_get_runtime_path(const char *filename) >> >> which return RUNTIME_DIR/filename instead of just RUNTIME_DIR ? > > I'm not sure. Such a function would be certainly useful, but I slightly feel such a function concerns with too many responsibilities. Getting a > runtime directory is one responsibility, and how to use is another. They are clearly distinguished; it does not matter how the path to the runtime > directory is used after acquiring it. For example, you can keep the path to the runtime directory, and derive the paths to two files in the directory. You can pass NULL as filename and get the directory itself. /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
© 2016 - 2024 Red Hat, Inc.