<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2015-11-13 0:30 GMT+02:00 Jiri Denemark <span dir="ltr"><<a href="mailto:jdenemar@redhat.com" target="_blank">jdenemar@redhat.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">On Thu, Nov 12, 2015 at 23:47:54 +0200, noxdafox wrote:<br>
> Greetings,<br>
><br>
> I was investigating on an issue in which QEMU's dynamic ownership was<br>
> not properly working when calling qemuDomainCoreDumpWithFormat().<br>
<br>
</span>Could describe this issue you are investigating?<br></blockquote><div> </div><div>When calling qemuDomainCoreDumpWithFormat() the file is create as root:root even when the dynamic ownership is specified in the qemu.conf configuration file.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> The core of the issue seems to be the qemuOpenFileAs() function which<br>
> does not handle the dynamic ownership. This might affect other libvirt's<br>
> features within as well.<br>
<br>
</span>Because you are most likely looking at a wrong place; qemuOpenFileAs is<br>
a quite low level function which is just supposed to open/create a file<br>
accessible to a given user. It's up to the caller to decide what the<br>
user should be. <br></blockquote><div> </div><div>When debugging the call, the parameters are correctly forwarded: dynamicOwnership is true and fallback_uid and fallback_gid are correct. The function itself seems to ignore them when a new file is created.<br><br></div><div>The following patch I provided on libvirt-users list seems to fix the issue but I'm not aware of possible side effects. This is why I'd like to provide some tests for these core functions.<br><br>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c<br>
index a2cc002..1b47dc6 100644<br>
--- a/src/qemu/qemu_driver.c<br>
+++ b/src/qemu/qemu_driver.c<br>
@@ -2932,6 +2932,11 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t
fallback_gid,<br>
if (path_shared <= 0 || dynamicOwnership)<br>
vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;<br>
<br>
+ if (dynamicOwnership) {<br>
+ uid = fallback_uid;<br>
+ gid = fallback_gid;<br>
+ }<br>
+<br>
if (stat(path, &sb) == 0) {<br>
/* It already exists, we don't want to delete it on
error */<br>
need_unlink = false;<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
...<br>
<span class="">> The issue is that all the functions within the qemu_driver.c module are<br>
> static. I could indeed include the module itself in my tests but I'm not<br>
> sure whether this is acceptable.<br>
<br>
</span>We solve this kind of issues by removing "static" from the functions and<br>
adding a new header file (if it doesn't exist yet) called *priv.h<br>
(qemu_driverpriv.h in this case) with the prototypes for such functions.<br>
Only tests are allowed to include such header files.<br></blockquote><div> </div><div>This is a way more elegant solution but I didn't know if it was acceptable to modify the headers. <br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> Furthermore I'd like to have some clarification about the NFS related<br>
> code. It seems that some effort has been put in order to tackle<br>
> something I'm not aware of. Could someone briefly explain how to<br>
> reproduce NFS failing scenarios?<br>
<br>
</span>The main problem with NFS which this ugly function is trying to handle<br>
is called "root-squash". This feature maps all access from UID 0 to an<br>
unprivileged UID. That is, libvirtd (even though it is running as root)<br>
will not be able to access the desired file.<br></blockquote><div><br></div><div>This is very helpful thanks!<br><br></div><div>I'll try to set some tests and provide patches later on. I guess it would be beneficial for libvirt anyway. About the refactoring we can discuss further later.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Jirka<br>
</blockquote></div><br></div></div>