Report more statistics#827
Conversation
51457ba to
98ecb1d
Compare
81389d2 to
505a779
Compare
marmarek
left a comment
There was a problem hiding this comment.
I don't like the breaking change regarding CPU usage. Is that really that useful to risk breaking many other things (you found one, but there may be more, including 3rd-party you have no way of knowing)?
If you really want to do that, better include new field with higher precision, but I'd prefer you don't.
|
|
||
| - memory_kb - current memory assigned, in kb | ||
| - memory_used_kb - current memory that can be used, in kb | ||
| - maxmem_kb - maximum memory assigned, in kb |
There was a problem hiding this comment.
What you probably intend to have here is just maxmem property (so, no need to include it in the stats). But what you actually get here will be closer to the current assignment, and will actually change as qmemman does its work.
There was a problem hiding this comment.
The domain cannot exceed the memory qmemman assigned to it, so for a moment (or a really long time when there is no available memory to redistribute), that is really the memory the domain can try to use at maximum. Is there a problem of reporting both?
Edit:
Note it may be bigger than actually assigned memory (in which case some swap is likely in use).
Hum... yes, then I think we can't rely on the method I proposed.
There was a problem hiding this comment.
just
maxmemproperty (so, no need to include it in the stats)
That can change while the domain is running and the client might be too late to initialize the cache. Stubdomains don't have that property. I think that I should set maxmem_kb the same as mem_kb of app.vmm.xc.domain_getinfo() and the same as the qube.libvirt_domain.info()[2], which doesn't change while qube is running.
Yes, this means that QubesVM.get_mem_static_max with self.libvirt_domain.maxMemory() is wrong, cause it should have been self.libvirt_domain.info()[2] or self.libvirt_domain.memoryStats()["actual"], but only for non-memory balanced domains.
ahhhhhhhh. maybe I just need to cached maxmem on the server when the domain starts.
There was a problem hiding this comment.
How can I know which keys a domain can write to xenstore? When I am reading xenstore of a qube, should I assume that a domain can change any key? It can change memory/meminfo, but can it change memory/static-max?
There was a problem hiding this comment.
import qubes
app = qubes.Qubes()
domains = app.domains
def gs(name):
qube = domains[name]
print("libvirt:")
print(" info:", qube.libvirt_domain._vm.info())
print(" maxMemory:", qube.libvirt_domain._vm.maxMemory())
print(" memoryStats:", qube.libvirt_domain._vm.memoryStats())
print("xen:")
print(" domain_getinfo:", qube.app.vmm.xc.domain_getinfo(qube.xid, 1))
print(" xenstore:", {i: qube.app.vmm.xs.read("", "/local/domain/{}/memory/{}".format(qube.xid, i)) for i in qube.app.vmm.xs.ls("", "/local/domain/{}/memory".format(qube.xid))})
while True:
try:
gs(input("Write qube name: "))
except KeyError:
print("Qube doesn't exist")summary markdown
-
dom0 (balanced):
- xen - domain_getinfo:
- mem_kb: 4177920 kB = 4080 MiB
- maxmem_kb: 4195328 kB = 4097 MiB
- xen - xs.read:
- static-max: 4194304 kB = 4096 MiB
- meminfo: 2144108 kB = 2093 MiB
- target: 4177920 kB = 4080 MiB
- libvirt - info:
- [1]: 33182058 kB = 32404 MiB
- [2]: 4177920 kB = 4080 MiB
- libvirt - memoryStats:
- actual: 4177920 kB = 4080 MiB
- available: 33182058 kB = 32404 MiB
- libvirt - maxMemory:
- result: 33182058 kB = 32404 MiB
- xen - domain_getinfo:
-
sys-net (not balanced):
- qube:
- memory: 512000 kB = 500 MiB
- maxmem: 0 kB = 0 MiB
- xen - domain_getinfo:
- mem_kb: 495660 kB = 484 MiB
- maxmem_kb: 513024 kB = 501 MiB
- xen - xs.read:
- statix-max: 512000 kB = 500 MiB
- target: 495616 kB = 484 MiB
- videoram: 16384 kB = 16 MiB
- libvirt - info:
- [1]: 512000 kB = 500 MiB
- [2]: 495660 kB = 484 MiB
- libvirt - memoryStats:
- actual: 495660 kB = 484 MiB
- available: 512000 kB = 500 MiB
- libvirt - maxMemory:
- result: 512000 kB = 500 MiB
- qube:
-
test (balanced, using swap, mod memory prop to 600MiB when running):
- qube:
- memory: 409600 kB = 400 MiB
- maxmem: 512000 kB = 500 MiB
- xen - domain_getinfo:
- mem_kb: 495672 kB = 484 MiB
- maxmem_kb: 513024 kB = 501 MiB
- xen - xs.read:
- statix-max: 512000 kB = 500 MiB
- target: 495616 kB = 484 MiB
- videoram: 0 kB = 0 MiB
- meminfo: 847892 kB = 828 MiB
- hotplug-max: 512000 kB = 500 MiB
- libvirt - info:
- [1]: 409600 kB = 400 MiB
- [2]: 495672 kB = 484 MiB
- libvirt - memoryStats:
- actual: 495672 kB = 484 MiB
- available: 409600 kB = 400 MiB
- libvirt - maxMemory:
- result: 409600 kB = 400 MiB
- qube:
-
dispX (balanced, preloaded disposable, qmemman just restricted memory for pause:
- qube:
- memory: 409600 kB = 400 MiB
- maxmem: 4096000 kB = 4000 MiB
- xen - domain_getinfo:
- mem_kb: 434444 kB = 424 MiB
- maxmem_kb: 423860 kB = 413 MiB
- xen - xs.read:
- static-max: 422838 kB = 412 MiB
- target: 406454 kB = 396 MiB
- videoram: 0 kB = 0 MiB
- meminfo: 325260 kB = 317 MiB
- hotplug-max: 4096000 kB = 4000 MiB
- libvirt - info:
- [1]: 409600 kB = 400 MiB
- [2]: 434444 kB = 424 MiB
- libvirt - memoryStats:
- actual: 434444 kB = 424 MiB
- available: 409600 kB = 400 MiB
- libvirt - maxMemory:
- result: 409600 kB = 400 MiB
- qube:
summary with indented fields
- dom0 (balanced):
- xen - domain_getinfo:
- mem_kb: 4177920 kB = 4080 MiB
- maxmem_kb: 4195328 kB = 4097 MiB
- xen - xs.read:
- static-max: 4194304 kB = 4096 MiB
- meminfo: 2144108 kB = 2093 MiB
- target: 4177920 kB = 4080 MiB
- libvirt - info:
- [1]: 33182058 kB = 32404 MiB
- [2]: 4177920 kB = 4080 MiB
- libvirt - memoryStats:
- actual: 4177920 kB = 4080 MiB
- available: 33182058 kB = 32404 MiB
- libvirt - maxMemory:
- result: 33182058 kB = 32404 MiB
- sys-net (not balanced):
- qube:
- memory: 512000 kB = 500 MiB
- maxmem: 0 kB = 0 MiB
- xen - domain_getinfo:
- mem_kb: 495660 kB = 484 MiB
- maxmem_kb: 513024 kB = 501 MiB
- xen - xs.read:
- statix-max: 512000 kB = 500 MiB
- target: 495616 kB = 484 MiB
- videoram: 16384 kB = 16 MiB
- libvirt - info:
- [1]: 512000 kB = 500 MiB
- [2]: 495660 kB = 484 MiB
- libvirt - memoryStats:
- actual: 495660 kB = 484 MiB
- available: 512000 kB = 500 MiB
- libvirt - maxMemory:
- result: 512000 kB = 500 MiB
- test (balanced, using swap, mod memory prop to 600MiB when running):
- qube:
- memory: 409600 kB = 400 MiB
- maxmem: 512000 kB = 500 MiB
- xen - domain_getinfo:
- mem_kb: 495672 kB = 484 MiB
- maxmem_kb: 513024 kB = 501 MiB
- xen - xs.read:
- statix-max: 512000 kB = 500 MiB
- target: 495616 kB = 484 MiB
- videoram: 0 kB = 0 MiB
- meminfo: 847892 kB = 828 MiB
- hotplug-max: 512000 kB = 500 MiB
- libvirt - info:
- [1]: 409600 kB = 400 MiB
- [2]: 495672 kB = 484 MiB
- libvirt - memoryStats:
- actual: 495672 kB = 484 MiB
- available: 409600 kB = 400 MiB
- libvirt - maxMemory:
- result: 409600 kB = 400 MiB
- dispX (balanced, preloaded disposable, qmemman just restricted memory for pause:
- qube:
- memory: 409600 kB = 400 MiB
- maxmem: 4096000 kB = 4000 MiB
- xen - domain_getinfo:
- mem_kb: 434444 kB = 424 MiB
- maxmem_kb: 423860 kB = 413 MiB
- xen - xs.read:
- static-max: 422838 kB = 412 MiB
- target: 406454 kB = 396 MiB
- videoram: 0 kB = 0 MiB
- meminfo: 325260 kB = 317 MiB
- hotplug-max: 4096000 kB = 4000 MiB
- libvirt - info:
- [1]: 409600 kB = 400 MiB
- [2]: 434444 kB = 424 MiB
- libvirt - memoryStats:
- actual: 434444 kB = 424 MiB
- available: 409600 kB = 400 MiB
- libvirt - maxMemory:
- result: 409600 kB = 400 MiB
There was a problem hiding this comment.
So, uhm, if you want get_mem to be memory usage, it has to read xenstore meminfo, else, I change the docstring to be "memory assignment" and calculate the correct value.
There was a problem hiding this comment.
From the system perspective it makes more sense to have what is allocated to that qube (how much host memory it actually uses), not what the qube would want. So, libvirt
memoryStats()['actual'].
Github didn't load this reply for me until I reloaded the page.
There was a problem hiding this comment.
Currently, the server broadcast how much memory is assigned to the domain, for app.vmm.xc.domain_get_info(), this is mem_kb and maxmem_kb. I believe that mem_kb is memory qmemman has assigned to the domain while is maxmem_kb is mem_kb+videoram+EPT-or-whatever, because videoram is 16MiB and the EPT is ~1MiB. I think I will broadcast both values.
>>> {m: app.vmm.xs.read("", "/local/domain/0/memory/" + m) for m in app.vmm.xs.ls("", "/local/domain/0/memory")}
{'static-max': b'4194304', 'meminfo': b'1864900', 'target': b'4177920'}
>>> d0 = app.vmm.xc.domain_getinfo(0,1)[0]; {'maxmem_kb': d0['maxmem_kb'], 'mem_kb': d0['mem_kb']}
{'maxmem_kb': 4195328, 'mem_kb': 4177920}
>>> i = dom0.libvirt_domain.info(); {'maxmem_kb': i[1], 'mem_kb': i[1+1]}
{'maxmem_kb': 33182508, 'mem_kb': 4177920}
>>>
>>> {m: app.vmm.xs.read("", "/local/domain/38/memory/" + m) for m in app.vmm.xs.ls("", "/local/domain/38/memory")}
{'static-max': b'4096000', 'target': b'2556453', 'videoram': b'16384', 'meminfo': b'482496', 'swapinfo': b''}
>>> da = app.vmm.xc.domain_getinfo(38,1)[0]; {'maxmem_kb': da['maxmem_kb'], 'mem_kb': da['mem_kb']}
{'maxmem_kb': 2573860, 'mem_kb': 2556496}
>>> i = doma.libvirt_domain.info(); {'maxmem_kb': i[1], 'mem_kb': i[1+1]}
{'maxmem_kb': 4096000, 'mem_kb': 2556496}Libvirt is very discrepant on maxmem_kb compared to Xen, most likely because it doesn't update when memory is redistributed. So I am here again, asking to broadcast both maxmem_kib and mem_kb. Use case:
- how much memory the qube can claim at any time: up to
mem_kib - how much memory the qube is consuming from the system (including everything known, videoram, EPT and whatever may come:
maxmem_kib.
Example.
With pressure:
No pressure:
- MS: set/assigned
- MM: maxmem
Notice that for dom0, gray, no-guivm, orange, sys-firewall, MS never reaches MM, because it is using mem_kib. For aaaa I allowed to show maxmem_kib, and it is one MiB over because Xen wants, as discussed elsewhere.
So, giving real examples of what I think is important to show
- A summary of how much memory the system has and how much it is using (sum of all
maxmem_kib). - How much memory the qube is using divided by how much it has asigned:
meminfo / mem_kib.
Because if I don't broadcast both values, things will be wrong. And how wrong? As wrong as videoram+EPT * memory-balanced-qubes.
I cannot use mamxmem property to get how much memory the qubes are using from the system, it will be wrong, as that value doesn't represent how much memory is really available to a qube.
There was a problem hiding this comment.
Lets focus on what would be useful and easy to understand. For an average user, not somebody who spent a month studying the subject.
I would say, a current total memory usage of a qube is an interesting value, because it shows where your overall system RAM got allocated (ideally, those values should sum up to your RAM size).
A second interesting value would be something answering "is a qube is running out of memory?". That would be likely meminfo / mem_kb as you said above.
Those two will not match the numbers you see in top inside a qube, but I would say that's okay, as that doesn't match on bare meta either (for example, a bare metal system with 96GiB shows total of 96440 MiB total memory, which is just above 94GiB).
Maybe some advanced view that shows overhead size of each qube (either as separate overhead size, or as memory usable for the qube itself) would be useful for somebody, but most of the time IMO it would be more confusing.
There was a problem hiding this comment.
The question was "is it okay to broadcast?". Per your response, it seems to be fine, you are just worried of pumping too much information in the viewer. By default, qvm-top doesn't show that many columns. I will take your comments into consideration in the documentation of each column, and to consider which ones should be enabled by default and how they relate to each other.
| where measurements is a dictionary with key: domid, value: dict: | ||
|
|
||
| - memory_kb - current memory assigned, in kb | ||
| - memory_used_kb - current memory that can be used, in kb |
There was a problem hiding this comment.
What do you mean by "can be used"? This is what qube itself reports as usage. Note it may be bigger than actually assigned memory (in which case some swap is likely in use).
There was a problem hiding this comment.
I assiged memory=400, maxmem=500 to a qube, started it and opened firefox, memoy_usage=140.0. Do you think I should cap it to 100.0? I don't think any number above 100.0 is informative, at that is already critical, as swap is slower and dom0 is not aware of how much swap the qube has available, so can't calculate.
There was a problem hiding this comment.
So, the above means it wants to use 700MB? Or 560MB? Percentage of what? IMO better report a size here, and let the frontend calculate whatever percentage it wants. For example, if it's above memory_kb but below maxmem it may show different info than when it's above maxmem - in the latter case, user may want to increase maxmem, but in the former just close some other qube to free memory.
There was a problem hiding this comment.
So, the above means it wants to use 700MB? Or 560MB? Percentage of what?
Percentage is meminfo / maxmem. The value exceeded because qube was using most of its memory and swap, so while maxmem was 500MiB, swap was 1G and by using swap, meminfo was way over maxmem.
| if self.app.vmm.is_xen and ( | ||
| stubdom_id := self.app.vmm.xs.read( | ||
| "", | ||
| "/local/domain/{}/image/device-model-domid".format(domid), |
There was a problem hiding this comment.
This doesn't change while a VM is running. Maybe cache it, by including in the returned dict, so the next iteration will give it back as previous argument?
There was a problem hiding this comment.
I added cache in #819. I am testing QubesOS/qubes-core-admin-client#333 with a merge of #827 and #819.
| new_vm_info = vm_info.copy() | ||
| new_vm_info["cpu_time"] = int(new_vm_info["cpu_time"] / 1000000) |
There was a problem hiding this comment.
While it's shorter this way, it hides what is actually being sent, making it easy to accidentally change the output format.
There was a problem hiding this comment.
Dropped this commit.
| info = {v["domid"]: v for v in info} | ||
| for domid in info.keys(): | ||
| stubdom_id = None | ||
| if domid in stubdoms: |
There was a problem hiding this comment.
This check will work only if target domain is processed before it's stubdomain. Currently stubdomain has its ID one bigger than its target, but I'm not sure if that's really guaranteed, and not just a coincidence (for example, it might not be the case if two HVMs are started in parallel in the exact same time).
Plus, this is working only because nowadays dicts in python preserve order on iteration and the domain_getinfo() API happens to return info sorted. But, it's rather fragile, easy to break when doing some changes here. At the very least, add a comment about assumptions of this line. But maybe, at the very least add also explicit sorting of info.keys() (should be cheap on already sorted input)?
There was a problem hiding this comment.
I am having to query the domain name to check the domain property maxmem (I need to access the name because we have domid only, which is xid, which is different than qid, so can't check presence in collection using domid/xid if not looping through every domain anyway. Therefore now I am using the VMCollection and will use stubdom_xid for this, instead of assuming upstream stays the same.
There was a problem hiding this comment.
No, please don't include maxmem in the stats output. Application if needed can access it itself, and if it needs updates can also listen for events (so no need for polling yet another thing). Keep only really dynamic info in the stats output.
There was a problem hiding this comment.
there is no more assumption about order. It may come at any order, and although it may not enter that check, there are extra checks later when more information is available to determine if qube is stubdom.
|
BTW, currently returned fields are documented at https://doc.qubes-os.org/en/r4.3/developer/services/admin-api.html, that will need an update if you add more. |
|
We have a problem with units, some are kB and others are MiB: QubesOS/qubes-issues#1737. Unfortunately, a memory stat is already being reported as |
IMO all usage stats should be in KiB, to be accurate. It might not matter that much for bigger qubes, but it does matter for smaller ones. The technical assignment unit is memory page (4k). If one day memory ballooning would support operation on super pages (2MiB), I would be very happy (and then usage could be reported in MiB too), but that's not going to happen in practice ever... For configuration values we have MiB, as that's more convenient for the user. |
bf0547b to
6ee911b
Compare
aa4e10d to
5c63d74
Compare
|
For HVMs, there is videoram: >>> net.name
'sys-net'
>>> domid = net.xid; {i: app.vmm.xs.read("", "/local/domain/{}/memory/{}".format(domid, i)) for i in app.vmm.xs.ls("", "/local/domain/{}/memory".format(domid))}
{'static-max': b'512000', 'target': b'495616', 'videoram': b'16384'}
>>> # For libvirt, I unfortunately only found it in XMLDesc()
>>> net_xml.find(".//video/model[@type='vga']").get("vram")
'16384'
>>> 512000 - 495616
16384
>>> net.libvirt_domain._vm.info()
[1, 512000, 495660, 2, 319197464138]
>>> net.libvirt_domain._vm.memoryStats()
{'actual': 495660, 'available': 512000}So videoram is not considered for redistribution, cool. But if we want to really know how much memory is reserved for that qube, we want to know the |
bed7c65 to
9ed126d
Compare
|
Code is working, as I have been using it. I just need to fix the tests. Marked as draft. |
|
Aggregating value of HVM+stubdom is fine when they are not memory balanced, but when they are, all memory values are a bit misleading forum discussion. So, I am rethinking how to best report the values. I think that when it is an HVM, reporting extra fields for stubdom might be better, then the client becomes responsible for showing the value how they see fit. Fields don't need to mention stubdom, as it's implementation specific, can be just |
|
Also, could the in-qube qmemman agent, meminfo-writer, continue reporting memory+swap as one value, plus write to a new key with a value of just the memory being used (no swap)? So the stats can report memory being used from what is assigned and if qube is swapping. Reporting separate values here: https://github.com/QubesOS/qubes-linux-utils/blob/0dc52abe8ad3325f5e982feaac1010d2d340e957/qmemman/meminfo-writer.c#L69. The objective is for clients to not show allocated memory over used memory or maxmem, and show the swap. |
Yes, that should be okay. |
3399878 to
06f9c73
Compare
a14272a to
8ed76fb
Compare
Only meminfo reports the correct memory used for each balanced domain. For: QubesOS/qubes-issues#8368
It's not completely Xen agnostic because then it would not be possible to query stubdomains, as libvirt is not aware of them. On the other hand, non-Xen has one more API methods working. When not using Xen, construct the dictionary as "xc.domain_getinfo()" would, so the info loop can consume from both inputs. Stubdomains are returned as separate keys instead of aggregated to allow clients to distinguish if it's HVM using the resources or it's stubdomain. This is specially relevant for HVMs that have memory balancing enabled.
Changing maxmem property of a running domain allows qmemman to read new information and balance appropriately. For the maxmem property to accurately reflect current state for clients to query, it is forbidden to change it for a running non memory balanced domain.
Add "maxmem" property to "Qubes" (app) and "AdminVM", so clients can now
system/host memory and dom0 memory.
Fix "get_mem" and "get_mem_static_max" mess:
- AdminVM:
- "get_mem": supposedly, should report usage in KiB, but it's reading
/proc/meminfo and returning kB
- "get_mem_static_max": returning total host memory
- QubesVM:
- "get_mem": supposedly, should report usage, but it's
reporting initial memory
- "get_mem_static_max": same as above
Now all of these methods returns memory in KiB. "get_mem" returns
allocated/assigned memory, while "get_mem_static_max" returns maxmem in
KiB.
get_vm_stats()is called is fromadmin.vm.Stats.Requires: