[libvirt] adding tests to qemu_driver

[noxdafox]

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