2015-11-13 0:30 GMT+02:00 Jiri Denemark <jdene...@redhat.com>: > On Thu, Nov 12, 2015 at 23:47:54 +0200, noxdafox wrote: > > Greetings, > > > > I was investigating on an issue in which QEMU's dynamic ownership was > > not properly working when calling qemuDomainCoreDumpWithFormat(). > > Could describe this issue you are investigating? >
When calling qemuDomainCoreDumpWithFormat() the file is create as root:root even when the dynamic ownership is specified in the qemu.conf configuration file. > > > The core of the issue seems to be the qemuOpenFileAs() function which > > does not handle the dynamic ownership. This might affect other libvirt's > > features within as well. > > Because you are most likely looking at a wrong place; qemuOpenFileAs is > a quite low level function which is just supposed to open/create a file > accessible to a given user. It's up to the caller to decide what the > user should be. > 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. 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. 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; > ... > > The issue is that all the functions within the qemu_driver.c module are > > static. I could indeed include the module itself in my tests but I'm not > > sure whether this is acceptable. > > We solve this kind of issues by removing "static" from the functions and > adding a new header file (if it doesn't exist yet) called *priv.h > (qemu_driverpriv.h in this case) with the prototypes for such functions. > Only tests are allowed to include such header files. > This is a way more elegant solution but I didn't know if it was acceptable to modify the headers. > > Furthermore I'd like to have some clarification about the NFS related > > code. It seems that some effort has been put in order to tackle > > something I'm not aware of. Could someone briefly explain how to > > reproduce NFS failing scenarios? > > The main problem with NFS which this ugly function is trying to handle > is called "root-squash". This feature maps all access from UID 0 to an > unprivileged UID. That is, libvirtd (even though it is running as root) > will not be able to access the desired file. > This is very helpful thanks! 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. > > Jirka >
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list