On 20/11/15 19:18, Jiri Denemark wrote:
On Fri, Nov 20, 2015 at 14:10:49 +0200, NoxDaFox wrote:
2015-11-19 14:25 GMT+02:00 Jiri Denemark <jdenemar@redhat.com>:
On Fri, Nov 13, 2015 at 10:44:01 +0200, NoxDaFox wrote:
2015-11-13 0:30 GMT+02:00 Jiri Denemark <jdenemar@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. And what is the result? Does dumping fail because of this or does it work anyway? The file gets dumped correctly, nevertheless it's ownership is still root:root and not the one set in the configuration. OK, this is actually the correct behavior. If so, I'm definitely lost here :)
From qemu.conf
# Whether libvirt should dynamically change file ownership # to match the configured user/group above. Defaults to 1. # Set to 0 to disable file ownership changes. dynamic_ownership = 1
Now, given a user and a group, when I run qemuDomainCoreDumpWithFormat() I get the file still belonging to root:root. Right, but (even though the comment doesn't say so) this applies only to files used by a running QEMU and only when they are used. They are chowned to root:root (they'd ideally be changed back to the original user/group, but we are no there yet) once they are no longer used. E.g., a disk image file will only be owned by the user/group while it is attached to a running domain. When the domain is not running, they will be owned by root:root.
Now I'm confused. The qemuDomainCoreDumpWithFormat should return a "picture" of the memory state of a running VM. It's a separate file which does not belong to the running VM itself. Why permissions should be transient? Moreover for what I've observed, the file gets root:root and remains so no matter whether the owner process is running or not.
I tested the patch locally and it was working. I could create the file anywhere and the ownership was right. Hmm, you are right, it looks like I didn't read the code carefully enough. Anyway, the file should be owne by root:root so that other QEMU processes cannot access it.
May I ask why such a policy? Wouldn't be better to leave the decision to the API user? In my case is making the logic quite complicated.
What I'd like to do though, is to refactor the function as its logic is a bit cumbersome. Before that, I'll try to provide a set of tests to help me during the refactoring. Oh sure, the code is a mess and if you have any idea for making saner and more readable. Creating a test suite first is definitely a good idea, because we don't want to accidentally change the behavior of this ugly code.
Jirka
The code is not *that* bad but it's pretty core logic. The lack of tests and the fragile logic suggests me this is the right place where to refactor a bit. As I said, my free time is quite limited and I need to get accustomed to libvirt's tests and build process. I'll start exposing the functions to a private header and to add basic tests for them.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list