On 18/11/15 12:11, Michal Privoznik wrote:
On 08.11.2015 12:43, noxdafox wrote:
I've been spending a bit of time looking into libvirt's code and I
believe this is not implemented as Daniel first said.
The issue is in the qemuOpenFileAs function in src/qemu/qemu_driver.c
which ignores the dynamic ownership flag and does not set correct
ownership on the file.
The qemuOpenFileAs function is used by other ones as well, so I wonder
if this affects other QEMU features.
I tried to fix and test it in different use cases and I didn't notice
any side effect, yet I'd like to provide tests as this function is quite
a core one and it's implementation is a bit confusing (maybe a
refactoring would simplify its logic).
Unfortunately the function is not exposed, therefore unittests are a bit
challenging. Higher level tests seem to mock the driver
(src/test/test_driver.c) as well.
Here's the patch fixing the issue. I set the correct uid and gid only if
the file is being created and dynamic ownership is set.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a2cc002..1b47dc6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2932,6 +2932,11 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t
fallback_gid,
if (path_shared <= 0 || dynamicOwnership)
vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
+ if (dynamicOwnership) {
+ uid = fallback_uid;
+ gid = fallback_gid;
+ }
+
if (stat(path, &sb) == 0) {
/* It already exists, we don't want to delete it on error */
need_unlink = false;
Please let me know what do you think about it and if we can somehow
include it in the sources. I'd be glad to offer further help if necessary.
Can you send it as an actual patch? Looks reasonable to me.
Michal
Done,
please let me know if I did something wrong as it's the first patch I'm
sending.
BR.
_______________________________________________
libvirt-users mailing list
libvirt-users@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvirt-users
