[libvirt] [PATCH v2] vz: allow to start vz driver without host cache info

Mikhail Feoktistov posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1499763545-504317-1-git-send-email-mfeoktistov@virtuozzo.com
src/vz/vz_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Mikhail Feoktistov 6 years, 8 months ago
Show warning message instead of fail operation.
It happens if kernel or cpu doesn't support reporting cpu cache info.
In case of Virtuozzo file "id" doesn't exist.
---
 src/vz/vz_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 6f4aee3..eb97e54 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -119,7 +119,7 @@ vzBuildCapabilities(void)
         goto error;
 
     if (virCapabilitiesInitCaches(caps) < 0)
-        goto error;
+        VIR_WARN("Failed to get host CPU cache info");
 
     verify(ARRAY_CARDINALITY(archs) == ARRAY_CARDINALITY(emulators));
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Martin Kletzander 6 years, 8 months ago
On Tue, Jul 11, 2017 at 04:59:05AM -0400, Mikhail Feoktistov wrote:
>Show warning message instead of fail operation.
>It happens if kernel or cpu doesn't support reporting cpu cache info.
>In case of Virtuozzo file "id" doesn't exist.

What is your kernel version?

Is there another way of getting the information about the cache ID?
Maybe we need to parse the name of the cache directory 'index2' would be
id 2 maybe?  If there is no other way, then this fix is fine (as most
drivers do the same thing), but I would rather fix it if that's
possible.  Unfortunately the cache information is structured stupidly
compared to other kernel-provided topology-related information.

>---
> src/vz/vz_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
>index 6f4aee3..eb97e54 100644
>--- a/src/vz/vz_driver.c
>+++ b/src/vz/vz_driver.c
>@@ -119,7 +119,7 @@ vzBuildCapabilities(void)
>         goto error;
>
>     if (virCapabilitiesInitCaches(caps) < 0)
>-        goto error;
>+        VIR_WARN("Failed to get host CPU cache info");
>
>     verify(ARRAY_CARDINALITY(archs) == ARRAY_CARDINALITY(emulators));
>
>--
>1.8.3.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Mikhail Feoktistov 6 years, 8 months ago
On 11.07.2017 12:22, Martin Kletzander wrote:
> On Tue, Jul 11, 2017 at 04:59:05AM -0400, Mikhail Feoktistov wrote:
>> Show warning message instead of fail operation.
>> It happens if kernel or cpu doesn't support reporting cpu cache info.
>> In case of Virtuozzo file "id" doesn't exist.
>
> What is your kernel version?
>
  3.10.0-514.16.1.vz7.30.10
> Is there another way of getting the information about the cache ID?
> Maybe we need to parse the name of the cache directory 'index2' would be
> id 2 maybe?  If there is no other way, then this fix is fine (as most
> drivers do the same thing), but I would rather fix it if that's
> possible.  Unfortunately the cache information is structured stupidly
> compared to other kernel-provided topology-related information.
Only kernel 4.11 reports cache ID (maybe 4.10, but not checked)
4.9 doesn't report ID
ID doesn't match to indexN

[liveuser@localhost-live ~]$ uname -a
Linux localhost-live 4.11.0-2.fc26.x86_64 #1 SMP Tue May 9 15:24:49 UTC 
2017 x86_64 x86_64 x86_64 GNU/Linux
[liveuser@localhost-live ~]$ cat 
/sys/devices/system/cpu/cpu1/cache/index0/id
0
[liveuser@localhost-live ~]$ cat 
/sys/devices/system/cpu/cpu1/cache/index1/id
0
[liveuser@localhost-live ~]$ cat 
/sys/devices/system/cpu/cpu1/cache/index2/id
0
[liveuser@localhost-live ~]$ cat 
/sys/devices/system/cpu/cpu0/cache/index2/id
0
[liveuser@localhost-live ~]$ cat 
/sys/devices/system/cpu/cpu0/cache/index0/id
0

>> ---
>> src/vz/vz_driver.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
>> index 6f4aee3..eb97e54 100644
>> --- a/src/vz/vz_driver.c
>> +++ b/src/vz/vz_driver.c
>> @@ -119,7 +119,7 @@ vzBuildCapabilities(void)
>>         goto error;
>>
>>     if (virCapabilitiesInitCaches(caps) < 0)
>> -        goto error;
>> +        VIR_WARN("Failed to get host CPU cache info");
>>
>>     verify(ARRAY_CARDINALITY(archs) == ARRAY_CARDINALITY(emulators));
>>
>> -- 
>> 1.8.3.1
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Martin Kletzander 6 years, 8 months ago
On Tue, Jul 11, 2017 at 03:20:38PM +0300, Mikhail Feoktistov wrote:
>
>On 11.07.2017 12:22, Martin Kletzander wrote:
>> On Tue, Jul 11, 2017 at 04:59:05AM -0400, Mikhail Feoktistov wrote:
>>> Show warning message instead of fail operation.
>>> It happens if kernel or cpu doesn't support reporting cpu cache info.
>>> In case of Virtuozzo file "id" doesn't exist.
>>
>> What is your kernel version?
>>
>  3.10.0-514.16.1.vz7.30.10
>> Is there another way of getting the information about the cache ID?
>> Maybe we need to parse the name of the cache directory 'index2' would be
>> id 2 maybe?  If there is no other way, then this fix is fine (as most
>> drivers do the same thing), but I would rather fix it if that's
>> possible.  Unfortunately the cache information is structured stupidly
>> compared to other kernel-provided topology-related information.
>Only kernel 4.11 reports cache ID (maybe 4.10, but not checked)
>4.9 doesn't report ID

Oh, so that is just older kernel.  OK, then.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

>ID doesn't match to indexN
>

Yeah, in vanilla it is number per level and easily deductible, but not
really something we want to add into the code.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Mikhail Feoktistov 6 years, 4 months ago
On 11.07.2017 15:55, Martin Kletzander wrote:
> On Tue, Jul 11, 2017 at 03:20:38PM +0300, Mikhail Feoktistov wrote:
>>
>> On 11.07.2017 12:22, Martin Kletzander wrote:
>>> On Tue, Jul 11, 2017 at 04:59:05AM -0400, Mikhail Feoktistov wrote:
>>>> Show warning message instead of fail operation.
>>>> It happens if kernel or cpu doesn't support reporting cpu cache info.
>>>> In case of Virtuozzo file "id" doesn't exist.
>>>
>>> What is your kernel version?
>>>
>>  3.10.0-514.16.1.vz7.30.10
>>> Is there another way of getting the information about the cache ID?
>>> Maybe we need to parse the name of the cache directory 'index2' 
>>> would be
>>> id 2 maybe?  If there is no other way, then this fix is fine (as most
>>> drivers do the same thing), but I would rather fix it if that's
>>> possible.  Unfortunately the cache information is structured stupidly
>>> compared to other kernel-provided topology-related information.
>> Only kernel 4.11 reports cache ID (maybe 4.10, but not checked)
>> 4.9 doesn't report ID
>
> Oh, so that is just older kernel.  OK, then.
>
> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
>
>> ID doesn't match to indexN
>>
>
> Yeah, in vanilla it is number per level and easily deductible, but not
> really something we want to add into the code.
Martin, do you agree with this patch?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: allow to start vz driver without host cache info
Posted by Martin Kletzander 6 years, 4 months ago
On Tue, Nov 07, 2017 at 12:02:05PM +0300, Mikhail Feoktistov wrote:
>
>On 11.07.2017 15:55, Martin Kletzander wrote:
>> On Tue, Jul 11, 2017 at 03:20:38PM +0300, Mikhail Feoktistov wrote:
>>>
>>> On 11.07.2017 12:22, Martin Kletzander wrote:
>>>> On Tue, Jul 11, 2017 at 04:59:05AM -0400, Mikhail Feoktistov wrote:
>>>>> Show warning message instead of fail operation.
>>>>> It happens if kernel or cpu doesn't support reporting cpu cache info.
>>>>> In case of Virtuozzo file "id" doesn't exist.
>>>>
>>>> What is your kernel version?
>>>>
>>>  3.10.0-514.16.1.vz7.30.10
>>>> Is there another way of getting the information about the cache ID?
>>>> Maybe we need to parse the name of the cache directory 'index2'
>>>> would be
>>>> id 2 maybe?  If there is no other way, then this fix is fine (as most
>>>> drivers do the same thing), but I would rather fix it if that's
>>>> possible.  Unfortunately the cache information is structured stupidly
>>>> compared to other kernel-provided topology-related information.
>>> Only kernel 4.11 reports cache ID (maybe 4.10, but not checked)
>>> 4.9 doesn't report ID
>>
>> Oh, so that is just older kernel.  OK, then.
>>
>> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
>>
>>> ID doesn't match to indexN
>>>
>>
>> Yeah, in vanilla it is number per level and easily deductible, but not
>> really something we want to add into the code.
>Martin, do you agree with this patch?
>

As I said, if there is no other way, then just not populating the info is
perfectly fine, we don't need to error out.

Have a nice day,
Martin--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list