[libvirt] Potential problem with /proc/mounts items that don't exist inside Kubernetes
Juan Hernández
jhernand at redhat.com
Tue Jul 11 06:59:18 UTC 2017
On 07/06/2017 01:07 PM, Juan Hernández wrote:
> On 07/06/2017 12:11 PM, Daniel P. Berrange wrote:
>> On Thu, Jul 06, 2017 at 11:48:43AM +0200, Juan Hernández wrote:
>>> On 07/06/2017 11:33 AM, Daniel P. Berrange wrote:
>>>> On Thu, Jul 06, 2017 at 11:26:58AM +0200, Juan Hernández wrote:
>>>>> On 07/06/2017 11:18 AM, Daniel P. Berrange wrote:
>>>>>> On Thu, Jul 06, 2017 at 11:11:03AM +0200, Juan Hernández wrote:
>>>>>>> Hello all,
>>>>>>>
>>>>>>> This is my first mail to this list, so let me introduce myself.
>>>>>>> My name is
>>>>>>> Juan Hernandez, and I work in the oVirt team. Currently I am
>>>>>>> experimenting
>>>>>>> with the integration between ManageIQ and KubeVirt.
>>>>>>>
>>>>>>> I recently detected a potential issue when running libvirt inside
>>>>>>> Kubernetes, as part of KubeVirt. There are entries in
>>>>>>> /proc/mounts that
>>>>>>> don't exist, and libvirt can't start virtual machines because of
>>>>>>> that. This
>>>>>>> is specific to this enviroment, but I think it may be worth
>>>>>>> addressing it in
>>>>>>> libvirt itself. See the following issue for details:
>>>>>>>
>>>>>>> Libvirt fails when there are hidden cgroup mount points in
>>>>>>> `/proc/mounts`
>>>>>>> https://github.com/kubevirt/libvirt/issues/4
>>>>>>>
>>>>>>> I suggested a possible fix there, which seems simple, but it
>>>>>>> makes all tests
>>>>>>> fail. I'd be happy to fix the tests as well, but I would need
>>>>>>> some guidance
>>>>>>> on how to do so. Any suggestion is welcome.
>>>>>>
>>>>>> The root cause problem will be the code that parse /proc/mounts.
>>>>>> It needs
>>>>>> to pick the last entry in the mounts file, since the earlier ones
>>>>>> can be
>>>>>> hidden. For some reason virCgroupDetectMountsFromFile instead
>>>>>> picks the
>>>>>> first entry, so that function needs updating todo the reverse.
>>>>>>
>>>>>
>>>>> Is the order of /proc/mounts guaranteed? It may be, but I'd suggest
>>>>> to not
>>>>> rely on that. Instead of that libvirt could check if the mount
>>>>> point does
>>>>> actually exist, and skip if it it doesn't. That is the fix I proposed:
>>>>
>>>> The order of /proc/mounts reflects the order in which the mounts were
>>>> performed. IOW, later entries will override earlier entries if there
>>>> is path overlap.
>>>>
>>>>>
>>>>> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
>>>>> index 5aa1db5..021a3f2 100644
>>>>> --- a/src/util/vircgroup.c
>>>>> +++ b/src/util/vircgroup.c
>>>>> @@ -393,6 +393,14 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
>>>>> if (STRNEQ(entry.mnt_type, "cgroup"))
>>>>> continue;
>>>>>
>>>>> + /* Some mount points in the /proc/mounts file may be
>>>>> + * hidden by others, and may not actually exist from
>>>>> + * the point of the view of the process, so we need
>>>>> + * to skip them.
>>>>> + */
>>>>> + if (!virFileExists(entry.mnt_dir))
>>>>> + continue;
>>>>
>>>> This is fragile because it is possible for the mount point to
>>>> still exist, but for its contents to have been replaced or
>>>> hidden. So we really do want to explicitly take only the last
>>>> entry, instead of doing this check.
>>>>
>>>
>>> That makes sense to me. Should I open a BZ to request that change?
>>
>> Yes, its worth tracking this in BZ.
>>
>> If you wanted to try to write a patch too, that'd be awesome :-)
>>
>
> I created the following BZ:
>
> Don't use cgroup mount points from /proc/mounts that are hidden
> https://bugzilla.redhat.com/1468214
>
> I will assign it to myself, and I will try to create a fix. Should I
> fail, I will ask for help.
>
The patch is here:
https://www.redhat.com/archives/libvir-list/2017-July/msg00141.html
I'd appreciate any review.
>>>
>>>>> +
>>>>> for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
>>>>> const char *typestr =
>>>>> virCgroupControllerTypeToString(i);
>>>>> int typelen = strlen(typestr);
>>>>>
>>>>> That fix makes things work, for it doesn't pass the tests.
>>>>
>>
>> Regards,
>> Daniel
>>
>
More information about the libvir-list
mailing list