Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
src/conf/capabilities.c | 45 ++++++++++++++--------
tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +-
.../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 4 +-
.../vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml | 4 +-
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 4 +-
5 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 095ef51c424a..5bf8ac2019f9 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -883,7 +883,8 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
for (i = 0; i < ncaches; i++) {
virCapsHostCacheBankPtr bank = caches[i];
char *cpus_str = virBitmapFormat(bank->cpus);
- bool kilos = !(bank->size % 1024);
+ const char *unit = NULL;
+ unsigned long long short_size = virPrettySize(bank->size, &unit);
if (!cpus_str)
return -1;
@@ -897,32 +898,44 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
"size='%llu' unit='%s' cpus='%s'",
bank->id, bank->level,
virCacheTypeToString(bank->type),
- bank->size >> (kilos * 10),
- kilos ? "KiB" : "B",
- cpus_str);
+ short_size, unit, cpus_str);
VIR_FREE(cpus_str);
virBufferSetChildIndent(&controlBuf, buf);
for (j = 0; j < bank->ncontrols; j++) {
- bool min_kilos = !(bank->controls[j]->granularity % 1024);
-
- /* Only use KiB if both values are divisible */
- if (bank->controls[j]->min)
- min_kilos = min_kilos && !(bank->controls[j]->min % 1024);
+ const char *min_unit;
+ unsigned long long gran_short_size = bank->controls[j]->granularity;
+ unsigned long long min_short_size = bank->controls[j]->min;
+
+ gran_short_size = virPrettySize(gran_short_size, &unit);
+ min_short_size = virPrettySize(min_short_size, &min_unit);
+
+ /* Only use the smaller unit if they are different */
+ if (min_short_size) {
+ unsigned long long gran_div;
+ unsigned long long min_div;
+
+ gran_div = bank->controls[j]->granularity / gran_short_size;
+ min_div = bank->controls[j]->min / min_short_size;
+
+ if (min_div > gran_div) {
+ min_short_size *= min_div / gran_div;
+ } else if (min_div < gran_div) {
+ unit = min_unit;
+ gran_short_size *= gran_div / min_div;
+ }
+ }
virBufferAsprintf(&controlBuf,
"<control granularity='%llu'",
- bank->controls[j]->granularity >> (min_kilos * 10));
+ gran_short_size);
- if (bank->controls[j]->min) {
- virBufferAsprintf(&controlBuf,
- " min='%llu'",
- bank->controls[j]->min >> (min_kilos * 10));
- }
+ if (min_short_size)
+ virBufferAsprintf(&controlBuf, " min='%llu'", min_short_size);
virBufferAsprintf(&controlBuf,
" unit='%s' type='%s' maxAllocs='%u'/>\n",
- min_kilos ? "KiB" : "B",
+ unit,
virCacheTypeToString(bank->controls[j]->scope),
bank->controls[j]->max_allocation);
}
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-caches.xml b/tests/vircaps2xmldata/vircaps-x86_64-caches.xml
index fe0be6d08fa7..0c6f3769a2a7 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-caches.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-caches.xml
@@ -29,7 +29,7 @@
</cells>
</topology>
<cache>
- <bank id='0' level='3' type='both' size='8192' unit='KiB' cpus='0-7'/>
+ <bank id='0' level='3' type='both' size='8' unit='MiB' cpus='0-7'/>
</cache>
</host>
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
index 7361537bfb56..443917c62d69 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
@@ -41,11 +41,11 @@
</cells>
</topology>
<cache>
- <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
+ <bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'>
<control granularity='768' unit='KiB' type='code' maxAllocs='8'/>
<control granularity='768' unit='KiB' type='data' maxAllocs='8'/>
</bank>
- <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
+ <bank id='1' level='3' type='both' size='15' unit='MiB' cpus='6-11'>
<control granularity='768' unit='KiB' type='code' maxAllocs='8'/>
<control granularity='768' unit='KiB' type='data' maxAllocs='8'/>
</bank>
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml
index 4e91c87de3b1..0cd25e59a9e0 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml
@@ -22,8 +22,8 @@
</cells>
</topology>
<cache>
- <bank id='0' level='3' type='both' size='33792' unit='KiB' cpus='0'>
- <control granularity='3072' unit='KiB' type='both' maxAllocs='16'/>
+ <bank id='0' level='3' type='both' size='33' unit='MiB' cpus='0'>
+ <control granularity='3' unit='MiB' type='both' maxAllocs='16'/>
</bank>
</cache>
</host>
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
index eb02ad3322a2..7629259294d6 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
@@ -41,10 +41,10 @@
</cells>
</topology>
<cache>
- <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
+ <bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'>
<control granularity='768' min='1536' unit='KiB' type='both' maxAllocs='4'/>
</bank>
- <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
+ <bank id='1' level='3' type='both' size='15' unit='MiB' cpus='6-11'>
<control granularity='768' min='1536' unit='KiB' type='both' maxAllocs='4'/>
</bank>
</cache>
--
2.15.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/13/2017 03:50 AM, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> src/conf/capabilities.c | 45 ++++++++++++++--------
> tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +-
> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 4 +-
> .../vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml | 4 +-
> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 4 +-
> 5 files changed, 36 insertions(+), 23 deletions(-)
>
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
couple of noisy review thoughts below...
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 095ef51c424a..5bf8ac2019f9 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -883,7 +883,8 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> for (i = 0; i < ncaches; i++) {
> virCapsHostCacheBankPtr bank = caches[i];
> char *cpus_str = virBitmapFormat(bank->cpus);
> - bool kilos = !(bank->size % 1024);
> + const char *unit = NULL;
> + unsigned long long short_size = virPrettySize(bank->size, &unit);
>
> if (!cpus_str)
> return -1;
> @@ -897,32 +898,44 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> "size='%llu' unit='%s' cpus='%s'",
> bank->id, bank->level,
> virCacheTypeToString(bank->type),
> - bank->size >> (kilos * 10),
> - kilos ? "KiB" : "B",
> - cpus_str);
> + short_size, unit, cpus_str);
> VIR_FREE(cpus_str);
>
> virBufferSetChildIndent(&controlBuf, buf);
> for (j = 0; j < bank->ncontrols; j++) {
You could have "virResctrlInfoPtr controls = bank->controls[j];"
> - bool min_kilos = !(bank->controls[j]->granularity % 1024);
> -
> - /* Only use KiB if both values are divisible */
> - if (bank->controls[j]->min)
> - min_kilos = min_kilos && !(bank->controls[j]->min % 1024);
> + const char *min_unit;
> + unsigned long long gran_short_size = bank->controls[j]->granularity;
> + unsigned long long min_short_size = bank->controls[j]->min;
> +
> + gran_short_size = virPrettySize(gran_short_size, &unit);
> + min_short_size = virPrettySize(min_short_size, &min_unit);
> +
> + /* Only use the smaller unit if they are different */
Or "if (STRNEQ(unit, min_unit))" - to be more faithful to the comment! I
read this as - if min_short_size is there, then we check the unit by
knowing the math to get the value.
To some degree if the pretty format function allowed one to "choose" a
specific format size that'd perhaps work too, but that's perhaps more
work than it's worth.
> + if (min_short_size) {
> + unsigned long long gran_div;
> + unsigned long long min_div;
> +
> + gran_div = bank->controls[j]->granularity / gran_short_size;
> + min_div = bank->controls[j]->min / min_short_size;
> +
> + if (min_div > gran_div) {
> + min_short_size *= min_div / gran_div;
> + } else if (min_div < gran_div) {
> + unit = min_unit;
> + gran_short_size *= gran_div / min_div;
> + }
> + }
>
> virBufferAsprintf(&controlBuf,
> "<control granularity='%llu'",
> - bank->controls[j]->granularity >> (min_kilos * 10));
> + gran_short_size);
>
> - if (bank->controls[j]->min) {
> - virBufferAsprintf(&controlBuf,
> - " min='%llu'",
> - bank->controls[j]->min >> (min_kilos * 10));
> - }
> + if (min_short_size)
> + virBufferAsprintf(&controlBuf, " min='%llu'", min_short_size);
>
> virBufferAsprintf(&controlBuf,
> " unit='%s' type='%s' maxAllocs='%u'/>\n",
> - min_kilos ? "KiB" : "B",
> + unit,
> virCacheTypeToString(bank->controls[j]->scope),
> bank->controls[j]->max_allocation);
> }
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 14, 2017 at 07:38:50AM -0500, John Ferlan wrote:
>
>
>On 11/13/2017 03:50 AM, Martin Kletzander wrote:
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>> src/conf/capabilities.c | 45 ++++++++++++++--------
>> tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +-
>> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 4 +-
>> .../vircaps2xmldata/vircaps-x86_64-resctrl-skx.xml | 4 +-
>> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 4 +-
>> 5 files changed, 36 insertions(+), 23 deletions(-)
>>
>
>Reviewed-by: John Ferlan <jferlan@redhat.com>
>
>John
>
>couple of noisy review thoughts below...
>
>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index 095ef51c424a..5bf8ac2019f9 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -883,7 +883,8 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> for (i = 0; i < ncaches; i++) {
>> virCapsHostCacheBankPtr bank = caches[i];
>> char *cpus_str = virBitmapFormat(bank->cpus);
>> - bool kilos = !(bank->size % 1024);
>> + const char *unit = NULL;
>> + unsigned long long short_size = virPrettySize(bank->size, &unit);
>>
>> if (!cpus_str)
>> return -1;
>> @@ -897,32 +898,44 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> "size='%llu' unit='%s' cpus='%s'",
>> bank->id, bank->level,
>> virCacheTypeToString(bank->type),
>> - bank->size >> (kilos * 10),
>> - kilos ? "KiB" : "B",
>> - cpus_str);
>> + short_size, unit, cpus_str);
>> VIR_FREE(cpus_str);
>>
>> virBufferSetChildIndent(&controlBuf, buf);
>> for (j = 0; j < bank->ncontrols; j++) {
>
>You could have "virResctrlInfoPtr controls = bank->controls[j];"
>
done
>> - bool min_kilos = !(bank->controls[j]->granularity % 1024);
>> -
>> - /* Only use KiB if both values are divisible */
>> - if (bank->controls[j]->min)
>> - min_kilos = min_kilos && !(bank->controls[j]->min % 1024);
>> + const char *min_unit;
>> + unsigned long long gran_short_size = bank->controls[j]->granularity;
>> + unsigned long long min_short_size = bank->controls[j]->min;
>> +
>> + gran_short_size = virPrettySize(gran_short_size, &unit);
>> + min_short_size = virPrettySize(min_short_size, &min_unit);
>> +
>> + /* Only use the smaller unit if they are different */
>
>Or "if (STRNEQ(unit, min_unit))" - to be more faithful to the comment! I
>read this as - if min_short_size is there, then we check the unit by
>knowing the math to get the value.
>
>To some degree if the pretty format function allowed one to "choose" a
>specific format size that'd perhaps work too, but that's perhaps more
>work than it's worth.
>
if I add STRNEQ there it doesn't allow me to remove any code, it would just add
a line. I need the math after that anyway.
>> + if (min_short_size) {
>> + unsigned long long gran_div;
>> + unsigned long long min_div;
>> +
>> + gran_div = bank->controls[j]->granularity / gran_short_size;
>> + min_div = bank->controls[j]->min / min_short_size;
>> +
>> + if (min_div > gran_div) {
>> + min_short_size *= min_div / gran_div;
>> + } else if (min_div < gran_div) {
>> + unit = min_unit;
>> + gran_short_size *= gran_div / min_div;
>> + }
>> + }
>>
>> virBufferAsprintf(&controlBuf,
>> "<control granularity='%llu'",
>> - bank->controls[j]->granularity >> (min_kilos * 10));
>> + gran_short_size);
>>
>> - if (bank->controls[j]->min) {
>> - virBufferAsprintf(&controlBuf,
>> - " min='%llu'",
>> - bank->controls[j]->min >> (min_kilos * 10));
>> - }
>> + if (min_short_size)
>> + virBufferAsprintf(&controlBuf, " min='%llu'", min_short_size);
>>
>> virBufferAsprintf(&controlBuf,
>> " unit='%s' type='%s' maxAllocs='%u'/>\n",
>> - min_kilos ? "KiB" : "B",
>> + unit,
>> virCacheTypeToString(bank->controls[j]->scope),
>> bank->controls[j]->max_allocation);
>> }
>[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.