Re: adding tests to qemu_driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>:

On Fri, Nov 13, 2015 at 10:44:01 +0200, NoxDaFox wrote:
2015-11-13 0:30 GMT+02:00 Jiri Denemark <jdenemar@xxxxxxxxxx>:

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]