Add on bhyvectl the capability to list the running vms.
AbandonedPublic
Actions

Authored by araujo on Jun 16 2015, 6:56 AM.

Details

Reviewers

allanjude
tychon
anish
jhb
rgrimes

Group Reviewers

bhyve

Summary

Nowadays the only way to check how many VMs we are running on bhyve is
performe an ls(1) on '/dev/vmm/'. This patch adds a new command option
on bhyvectl.

root@pipoca:/ # bhyvectl --list
NAME             MEM                     VCPU
fbsd             2147483648              4
fbsd2            2147483648              4
fbsd1            536870912               1

Also it is possible to search by name:
root@pipoca:/ # bhyvectl --list --vm=fbsd1
NAME             MEM                     VCPU
fbsd1            536870912               1

NOTE: I have another patch that converts this output to libxo(3), however I would like to do it in two steps. First add this feature and then convert the entirely bhyvectl to libxo(3).

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16753
Build 16642: arc lint + arc unit

Event Timeline

araujo updated this revision to Diff 6231.Jun 16 2015, 6:56 AM

araujo retitled this revision from to Add on bhyvectl the capability to list the running vms..

araujo updated this object.

araujo edited the test plan for this revision. (Show Details)

araujo added reviewers: grehan, neel, • rodrigc, allanjude.

Herald added a subscriber: imp. · View Herald TranscriptJun 16 2015, 6:56 AM

Fix a core dump in case you have no permission to open /dev/vmm/*.
Just a cosmetic fix of the column size that was over 80.

Looks good to me, but I recommend you get review from someone with stronger C skills

This revision is now accepted and ready to land.Jun 18 2015, 5:06 AM

This patch will need to be updated after r284539
https://svnweb.freebsd.org/base?view=revision&revision=284539

The API vm_get_memory_seg() is no longer available and in any case its use wasn't right in the original patch. The VM memory size should be obtained by adding up vm_get_lowmem_size() and vm_get_highmem_size() instead.

Also, /dev/vmm now includes not just /dev/vmm/vmname but also device memory nodes like /dev/vmm/vmname.bootrom. The listing code needs to handle this properly.

Regarding libxo:

bhyvectl is primarily a developer tool to inspect low-level details about a virtual machine (except for --destroy which is used in VM lifecycle management). It does not even have a man page (it is nothing to be proud of) but even if it did most of its options would make no sense unless you were knee deep in vmm.ko development.

bhyvectl is not a general purpose VM management tool so converting it to libxo makes no sense IMHO.

This may be controversial, but I'll just throw it out there as an idea for you to think about.
It would be cool if the output you generate from bhyvectl such as listing vm's could
use libxo to output things in JSON or XML format. This would make it easier for
people writing scripts or web apps to incorporate and analyze this information.

In D2838#55146, @rodrigc wrote:

This may be controversial, but I'll just throw it out there as an idea for you to think about.
It would be cool if the output you generate from bhyvectl such as listing vm's could
use libxo to output things in JSON or XML format. This would make it easier for
people writing scripts or web apps to incorporate and analyze this information.

I think what neel is trying to say, is that the 'list vms' functionality, and other stuff that is designed to be 'user facing', and get libxo'd, maybe should be a separate utility, since bhyvectl is a debugging utility.

maybe bhyvemanage or something, that has the few user facing options like:

[--inject-nmi]
[--force-reset]
[--force-poweroff]
[--destroy]

acpi shutdown
control for memory ballooning
list vms
etc

Use strstr(3) to check the case of a devname as bootrom.
Redo the patch and cover the changes on revision r284539.

Instead to use vm_get_lowmem_size() and vm_get_highmem_size() as suggested
by @neel, I would prefer use vm_mmap_getnext() and use the function
humanize_number() like in the function show_memmap().

This revision now requires review to proceed.Jun 20 2015, 12:23 AM

I forgot to mention, remove a duplicated header.

I'm not interested on it any more.

Sorry about that - the delay wasn't intentional :-(

I'm revive this patch after see the OpenBSD presentation yesterday at bhyveconf, they have something pretty much similar.

araujo edited the summary of this revision. (Show Details)Mar 10 2017, 4:35 AM

araujo added a reviewer: rgrimes.Mar 11 2017, 4:21 AM

In D2838#55182, @allanjude wrote:
In D2838#55146, @rodrigc wrote:

This may be controversial, but I'll just throw it out there as an idea for you to think about.
It would be cool if the output you generate from bhyvectl such as listing vm's could
use libxo to output things in JSON or XML format. This would make it easier for
people writing scripts or web apps to incorporate and analyze this information.

I think what neel is trying to say, is that the 'list vms' functionality, and other stuff that is designed to be 'user facing', and get libxo'd, maybe should be a separate utility, since bhyvectl is a debugging utility.

maybe bhyvemanage or something, that has the few user facing options like:
[--inject-nmi]
[--force-reset]
[--force-poweroff]
[--destroy]

I have to agree with Neel, and I think Allan is also saying he things this may best
go some place else. Bhyvectl is not meant to be a user interface, and though
--destroy has been used externally that should, and I am pretty sure, be going
away in the near future.

Save this code though please, we do need a vm list tool that is better than
ls /dev/vmm, and the day for that tool should be sooner rather than later.

You also said that OpenBSD has this vm list functioning some place, could you
please give me a direct pointer if you have one? Thanks!

rgrimes added inline comments.Mar 11 2017, 3:18 PM

usr.sbin/bhyvectl/bhyvectl.c
1648	You could collapse that to a simpler dir->d_type != DT_DIR and save yourself from errors if at some point a subdirectory is created by vmm.ko in this name space.
1649	I need to look at this, but this would not list a vm I named bootrom, I am not sure that there is any protection from creating such a vm name.
1962	It looks like this is not reached if (list) so !list would always be true, so I believe you can remove the && !list

In D2838#205748, @rgrimes wrote:
In D2838#55182, @allanjude wrote:
In D2838#55146, @rodrigc wrote:

This may be controversial, but I'll just throw it out there as an idea for you to think about.
It would be cool if the output you generate from bhyvectl such as listing vm's could
use libxo to output things in JSON or XML format. This would make it easier for
people writing scripts or web apps to incorporate and analyze this information.

I think what neel is trying to say, is that the 'list vms' functionality, and other stuff that is designed to be 'user facing', and get libxo'd, maybe should be a separate utility, since bhyvectl is a debugging utility.

maybe bhyvemanage or something, that has the few user facing options like:
[--inject-nmi]
[--force-reset]
[--force-poweroff]
[--destroy]
I have to agree with Neel, and I think Allan is also saying he things this may best
go some place else. Bhyvectl is not meant to be a user interface, and though
--destroy has been used externally that should, and I am pretty sure, be going
away in the near future.

Save this code though please, we do need a vm list tool that is better than
ls /dev/vmm, and the day for that tool should be sooner rather than later.

We could create a new tool to deal with these VM stuff.

You also said that OpenBSD has this vm list functioning some place, could you
please give me a direct pointer if you have one? Thanks!

http://man.openbsd.org/vmctl.8

Update this patch and fix the latest suggestions.

Harbormaster completed remote builds in B16753: Diff 42829.May 22 2018, 3:43 AM

araujo edited reviewers, added: tychon, anish, jhb; removed: • rodrigc, rgrimes, neel.May 22 2018, 3:44 AM

Herald added a reviewer: rgrimes. · View Herald TranscriptMay 22 2018, 3:44 AM

araujo added a reviewer: bhyve.Oct 26 2018, 12:14 PM

I shall re assert my earlier concern that we want to keep bhyvectl purely as a debugging tool, and not start to clutter it with stuff that is not part of that. Yes, there is the use of --destroy by others, but that is and should be the exception rather than the norm.

In D2838#378512, @rgrimes wrote:

I shall re assert my earlier concern that we want to keep bhyvectl purely as a debugging tool, and not start to clutter it with stuff that is not part of that. Yes, there is the use of --destroy by others, but that is and should be the exception rather than the norm.

Based on our talk yesterday on bhyve group I just brought it to the attention of the group, as I stated yesterday this must be in another tool together with the destroy option! So you are right too!

Proposal: Rename the current bhyvectl code to bhyvedbg, and create a new bhyvectl that has --create, --destory and --list. This could also be the tool to do the bhyvectl --addcpu, --addmemory, --adddisk, etc, etc. This is just a random braincell firing of an idea, anyone and everyone can ignore as they see fit.

In D2838#378519, @rgrimes wrote:

Proposal: Rename the current bhyvectl code to bhyvedbg, and create a new bhyvectl that has --create, --destory and --list. This could also be the tool to do the bhyvectl --addcpu, --addmemory, --adddisk, etc, etc. This is just a random braincell firing of an idea, anyone and everyone can ignore as they see fit.

I had a very similar thought too about rename bhyvectl to bhyvedbg, we shall take it to one of our bhyve meetings.
To split the command line would be very easy, I can do that in couple days.