Extend the VMM stats interface to support a dynamic count of statistics.

[jhb]

Summary

• Add a starting index to 'struct vmstats' and change the VM_STATS ioctl to fetch the 64 stats starting at that index. A compat shim for 12 continues to fetch only the first 64 stats.

• Extend vm_get_stats() in libvmmapi to use a loop and a static buffer which grows to hold the stats needed. Currently this function is not thread-safe, but it's only user in the tree is single-threaded. It could be made thread-safe by converting the static variables to be per-thread variables.

Test Plan

• this has been compiled, but it has not been run-tested

[jhb]

sys/amd64/vmm/vmm_stat.c

109

Possibly the stats should only be refreshed for requests with an index of 0.

[markj]

sys/amd64/vmm/vmm_stat.c

95

Should we guard against a negative index? Or perhaps change index and count to be unsigned.

[jhb]

• Rebase.
• COMPAT_FREEBSD12 -> COMPAT_FREEBSD13.
• Reject negative 'index' and 'count'.

[jhb]

Rebased and tested.

[markj]

Seems ok, this is just nitpicking.

lib/libvmmapi/vmmapi.c

1069

It seems kind of weird for a libvmmapi function to not be thread-safe. I see that this is only used by bhyvectl for now, so I guess that's ok.

1081

Perhaps check explicitly for errno == ENOENT to decide whether to return an error to the caller. Or perhaps the ioctl should just return success and num_entries == 0 if the index is equal to the number of stats.

sys/amd64/vmm/vmm_stat.c

85–86

Long line.

[jhb]

lib/libvmmapi/vmmapi.c

1069

It indeed might be a bit weird, though it was already non-safe before. I don't know how often this will really be used anyway. The main point of this change is to make it easier to raise the maximum number of supported virtual CPUs without breaking the ABI of this ioctl.

1081

You can also get EINVAL for an invalid vcpu passed in by the caller. Hmm, but yes I guess if the total stat count % vmstats.statbuf == 0 you can get a bogus ENOENT here and the ioctl should return 0 in that case with num_entries == 0 instead.

sys/amd64/vmm/vmm_stat.c

98

So here we would just need to return (0) and set *num_stats to 0 for the 'index == vst_num_elems' case?

[markj]

lib/libvmmapi/vmmapi.c

1069

Yeah, I think it's ok, I was just surprised.

sys/amd64/vmm/vmm_stat.c

98

Yeah, that was my thinking. If the index > vst_num_elems case continues to return an error, then vm_get_stats() needs to be adjusted to increment index by the number of entries returned rather than nitems(vmstats.statbuf). I'm not sure if it's worth the hassle, just a suggestion.

[jhb]

Return success with a count of 0 for a copy starting at the end.

[markj]

lib/libvmmapi/vmmapi.c

1090

The realloc() will free stats_buf, so I don't think this free() call is right.

[jhb]

lib/libvmmapi/vmmapi.c

1087

I need to move the free() up here then I think (or use reallocf()).

[markj]

lib/libvmmapi/vmmapi.c

1087

stats_buf is defined as static, so it's technically ok to leave it allocated so long as it stays in harmony with stats_count.

[jhb]

• Remove extraneous free().
• Add _Thread_local.

[markj]

sys/amd64/vmm/vmm_stat.c

85–86

Still unwrapped.

[jhb]

sys/amd64/vmm/vmm_stat.c

85–86

Oops, fixed before commit.