Changeset View
Standalone View
lib/libvmmapi/vmmapi.c
Show First 20 Lines • Show All 1,060 Lines • ▼ Show 20 Lines | vm_disable_pptdev_msix(struct vmctx *ctx, int bus, int slot, int func) | ||||
return ioctl(ctx->fd, VM_PPTDEV_DISABLE_MSIX, &ppt); | return ioctl(ctx->fd, VM_PPTDEV_DISABLE_MSIX, &ppt); | ||||
} | } | ||||
uint64_t * | uint64_t * | ||||
vm_get_stats(struct vmctx *ctx, int vcpu, struct timeval *ret_tv, | vm_get_stats(struct vmctx *ctx, int vcpu, struct timeval *ret_tv, | ||||
int *ret_entries) | int *ret_entries) | ||||
{ | { | ||||
int error; | static _Thread_local uint64_t *stats_buf; | ||||
markj: It seems kind of weird for a libvmmapi function to not be thread-safe. I see that this is only… | |||||
Done Inline ActionsIt 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. jhb: It indeed might be a bit weird, though it was already non-safe before. I don't know how often… | |||||
Done Inline ActionsYeah, I think it's ok, I was just surprised. markj: Yeah, I think it's ok, I was just surprised. | |||||
static _Thread_local u_int stats_count; | |||||
uint64_t *new_stats; | |||||
struct vm_stats vmstats; | |||||
u_int count, index; | |||||
bool have_stats; | |||||
static struct vm_stats vmstats; | have_stats = false; | ||||
vmstats.cpuid = vcpu; | vmstats.cpuid = vcpu; | ||||
count = 0; | |||||
for (index = 0;; index += nitems(vmstats.statbuf)) { | |||||
vmstats.index = index; | |||||
if (ioctl(ctx->fd, VM_STATS, &vmstats) != 0) | |||||
Done Inline ActionsPerhaps 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. markj: Perhaps check explicitly for `errno == ENOENT` to decide whether to return an error to the… | |||||
Done Inline ActionsYou 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. jhb: You can also get `EINVAL` for an invalid `vcpu` passed in by the caller. Hmm, but yes I guess… | |||||
break; | |||||
if (stats_count < index + vmstats.num_entries) { | |||||
new_stats = realloc(stats_buf, | |||||
(index + vmstats.num_entries) * sizeof(uint64_t)); | |||||
if (new_stats == NULL) { | |||||
errno = ENOMEM; | |||||
Done Inline ActionsI need to move the free() up here then I think (or use reallocf()). jhb: I need to move the free() up here then I think (or use reallocf()). | |||||
Done Inline Actionsstats_buf is defined as static, so it's technically ok to leave it allocated so long as it stays in harmony with stats_count. markj: `stats_buf` is defined as static, so it's technically ok to leave it allocated so long as it… | |||||
return (NULL); | |||||
} | |||||
stats_count = index + vmstats.num_entries; | |||||
Done Inline ActionsThe realloc() will free stats_buf, so I don't think this free() call is right. markj: The realloc() will free `stats_buf`, so I don't think this free() call is right. | |||||
stats_buf = new_stats; | |||||
} | |||||
memcpy(stats_buf + index, vmstats.statbuf, | |||||
vmstats.num_entries * sizeof(uint64_t)); | |||||
count += vmstats.num_entries; | |||||
have_stats = true; | |||||
error = ioctl(ctx->fd, VM_STATS, &vmstats); | if (vmstats.num_entries != nitems(vmstats.statbuf)) | ||||
if (error == 0) { | break; | ||||
} | |||||
if (have_stats) { | |||||
if (ret_entries) | if (ret_entries) | ||||
*ret_entries = vmstats.num_entries; | *ret_entries = count; | ||||
if (ret_tv) | if (ret_tv) | ||||
*ret_tv = vmstats.tv; | *ret_tv = vmstats.tv; | ||||
return (vmstats.statbuf); | return (stats_buf); | ||||
} else | } else | ||||
return (NULL); | return (NULL); | ||||
} | } | ||||
const char * | const char * | ||||
vm_get_stat_desc(struct vmctx *ctx, int index) | vm_get_stat_desc(struct vmctx *ctx, int index) | ||||
{ | { | ||||
static struct vm_stat_desc statdesc; | static struct vm_stat_desc statdesc; | ||||
▲ Show 20 Lines • Show All 620 Lines • Show Last 20 Lines |
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.