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
_______________________________________________
libvirt-users mailing list
libvirt-users@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvirt-users
