Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenBSD support #1466

Merged
merged 82 commits into from
Jan 17, 2021
Merged

OpenBSD support #1466

merged 82 commits into from
Jan 17, 2021

Conversation

mprins
Copy link
Member

@mprins mprins commented Dec 28, 2020

OpenBSD support

resolve #1465

@dbwiddis
Copy link
Member

I went ahead and did a mass copy of all the other files. Let me know what doesn't work and I can help you sort it out.

@coveralls
Copy link

coveralls commented Dec 28, 2020

Coverage Status

Coverage increased (+0.1%) to 87.966% when pulling e953502 on mprins:openbsd into c5fd098 on oshi:master.

@dbwiddis
Copy link
Member

OK, so I did manage to get a minimal openbsd VM set up and see what you mean with JNA -- I can't for the life of me figure out how to load the C library.

@dbwiddis
Copy link
Member

OK, at this point SystemInfoTest runs without crashing. All those nulls and empty arrays you added were causing headaches. :)

I will call that a win for now. Let me know which data is most important for you.

@dbwiddis
Copy link
Member

OK, I've got the C library loading. It was just confusion trying to open the "FreeBSD" library with poorly linked structures. OpenBSD does have sysctl() but not sysctlbyname() which means we can make most of the native calls we need, we just need to fetch the constants. Dropping the link here for later reference: http://fxr.watson.org/fxr/source/sys/sysctl.h?v=OPENBSD

@mprins
Copy link
Member Author

mprins commented Dec 29, 2020

Cool!
Away from computer atm but https://github.com/openbsd/src/blob/master/sys/sys/sysctl.h is the current version

Will give it a shot in the morning

@dbwiddis
Copy link
Member

Current status:

  • Native sysctl should work for most KERN and HW queries. Not sure what MIB values are for the MACHDEP and VM queries yet as they are not in the header file.
  • Using native sysctl for CPU ticks and frequency. For some reason hw.ncpu fails in the native version so I fell back to your commandline workaround there.
  • Parsed the CPUID to get family, model, stepping.
  • CentralProcessor looks done except for context switches and interrupts.

Next up is memory, available from sysctl CTL_VM , VM_UVMEXP, returning a uvmexp structure, which looks like it's just a pointer and manual offsets are needed. See https://github.com/openbsd/src/blob/master/usr.bin/systat/uvm.c and https://github.com/openbsd/src/blob/master/usr.bin/vmstat/vmstat.c#L321 for all sorts of porting fun.

@dbwiddis
Copy link
Member

I think I'm mostly done with processor and memory. Next big thing to tackle is the process listing. There may be a way to do it natively, but I keep running into problems with those (possibly due to OpenBSD's famed security measures) so parsing ps is probably the way to go. The existing code produces something but it's clearly out of sync on the columns, so you might take a look at where the FreeBSD and OpenBSD commands differ and tweak the process (and possibly thread) code.

@dbwiddis
Copy link
Member

dbwiddis commented Jan 14, 2021

OK, not quite done. For non-root HW disk store I think we can parse dmesg for disk label and size:

wd0 at pciide0 channel 0 drive 0: <OpenBSD-0 SSD>
wd0: 128-sector PIO, LBA48, 8192MB, 16777216 sectors

or

wd0 at pciide0 channel 0 drive 0: <HTS424040M9AT00>
wd0: 16-sector PIO, LBA48, 38154MB, 78140160 sectors

or

wd0 at pciide0 channel 0 drive 0: <ST9160821A>
wd0: 16-sector PIO, LBA48, 152627MB, 312581808 sectors

@dbwiddis
Copy link
Member

I think this is ready to merge. Anything else you think needs doing?

@dbwiddis
Copy link
Member

Heh, I just fixed those spaces and tried to push and it yelled at me. Thanks. :)

@mprins
Copy link
Member Author

mprins commented Jan 15, 2021

attached results of running mvn clean test -pl :oshi-core -Dtest=SystemInfoTest (regular user)
oshi.SystemInfoTest-output.txt
and doas mvn clean test -pl :oshi-core -Dtest=SystemInfoTest (elevated priviledges)
doas-oshi.SystemInfoTest-output.txt

So I think all the functionality is there, I still see a test failure while running install:

[ERROR] Tests run: 10, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 101.161 s <<< FAILURE! - in oshi.software.os.OperatingSystemTest
[ERROR] oshi.software.os.OperatingSystemTest.testProcessStats  Time elapsed: 0.185 s  <<< FAILURE!
java.lang.AssertionError: 
Current running process thread count should be greater than 0
Expected: is a value greater than <0>
     but: <0> was equal to <0>
	at oshi.software.os.OperatingSystemTest.testProcessStats(OperatingSystemTest.java:119)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   OperatingSystemTest.testProcessStats:119 Current running process thread count should be greater than 0
Expected: is a value greater than <0>
     but: <0> was equal to <0>

And on a vbox vm the cpu stuff was odd yesterday, but may also be caused by vbox.
Off to work now

@dbwiddis
Copy link
Member

dbwiddis commented Jan 15, 2021

Hmm, interesting with non-root you still don't get the disk model and size. Can you post your dmesg output for lines beginning with sd0?

As for the test error, apparently thread count is 0. This may be a race condition for a process which ended before updateThreadCount() was called. Should probably have the default set to 1 on OpenBsdOSProcess.java line 349, rather than 0.

@mprins
Copy link
Member Author

mprins commented Jan 15, 2021

I'll look up my laptop dmesg tonight.

here's the vbox oddity...

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   CentralProcessorTest.testFrequencies:83 Central Processor's logical processor frequency array length should be the same as its logical processor count
Expected: is <3>
     but: was <1>
[INFO] 
[ERROR] Tests run: 116, Failures: 1, Errors: 0, Skipped: 0

while sysctl hw says we have both 1 and 3 processors ;-)

hw.machine=amd64
hw.model=AMD Ryzen 7 3700X 8-Core Processor
hw.ncpu=1
hw.byteorder=1234
hw.pagesize=4096
hw.disknames=wd0:485ae173f5008c6c,cd0:
hw.diskcount=2
hw.sensors.acpiac0.indicator0=On (power supply)
hw.cpuspeed=3600
hw.vendor=innotek GmbH
hw.product=VirtualBox
hw.version=1.2
hw.serialno=0
hw.uuid=526d3985-a79f-fc41-b3d2-3687111ac93c
hw.physmem=4278124544
hw.usermem=4278108160
hw.ncpufound=3
hw.allowpowerdown=1
hw.smt=0
hw.ncpuonline=1

while dmesg reports just 1 cpu:

cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 3700X 8-Core Processor, 3600.55 MHz, 17-71-00
cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,RDRAND,NXE,MMXX,FFXSR,RDTSCP,LONG,LAHF,CMPLEG,SVM,AMCR8,ABM,SSE4A,MASSE,3DNOWP,ITSC,FSGSBASE,AVX2,RDSEED,CLFLUSHOPT
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 8-way L2 cache, 32MB 64b/line disabled L3 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: CPU supports MTRRs but not enabled by BIOS
cpu0: apic clock running at 1000MHz
cpu at mainbus0: not configured
cpu at mainbus0: not configured

perhaps we should go with the number reported by hw.ncpuonline?

@dbwiddis
Copy link
Member

perhaps we should go with the number reported by hw.ncpuonline

Does that change during operation? Why do we keep fetching the sysctl value when getting cpu frequency? (And per earlier comment rather than using modulo 2 we should line up with smt threads from dmesg if we continue to do it that way, which I'm not sure we should.)

Whichever we choose we should pick the same one for counting CPUs as for cpu frequency... we can just fill an array with getLogicalProcessorCount() entries. The question becomes which is a more accurate statement of the number of processors.

In this case, it looks like the vm is running the single-threaded kernel and not bsd.mp which might be able to see the other processors. We really don't have any sort of variable anywhere that says "this hardware exists but the OS can't see it".

@dbwiddis
Copy link
Member

Also I found another format for disk size in another dmesg so I'll try to add that.

sd0 at scsibus1 targ 0 lun 0: <ATA, P3-2TB, T180> SCSI3 0/direct fixed naa.5000000000000000
sd0: 1953514MB, 512 bytes/sector, 4000797360 sectors, thin
sd1 at scsibus1 targ 2 lun 0: <ATA, MT-1TB, T180> SCSI3 0/direct fixed naa.5000000000000000
sd1: 976762MB, 512 bytes/sector, 2000409264 sectors, thin

@dbwiddis
Copy link
Member

So I think we're all done now except for determining the array size of the cpu frequency array, and whether to fill it all with the same number or to be complex and figure out which might be smt threads and set them to 0.

My preference is to just go simple. Create an array of size getLogicalProcessorCount() and fill it with the same value. That's what we do on macOS.

# Conflicts:
#	oshi-core/src/main/java/oshi/SystemInfo.java
#	oshi-core/src/test/java/oshi/util/LsofUtilTest.java
@mprins
Copy link
Member Author

mprins commented Jan 16, 2021

perhaps we should go with the number reported by hw.ncpuonline

Does that change during operation? Why do we keep fetching the sysctl value when getting cpu frequency? (And per earlier comment rather than using modulo 2 we should line up with smt threads from dmesg if we continue to do it that way, which I'm not sure we should.)

On consumer hardware probably no way to turn off processors, on enterprise level hardware like sparc64 maybe you can

@dbwiddis
Copy link
Member

Any update on whether my disk name and size non-root backup worked for you?

I think this is ready to merge. If you do, click "Ready for review" above! Or click squash and merge yourself! :)

@mprins
Copy link
Member Author

mprins commented Jan 17, 2021

LGTM

Disks:
 sd0: (model: ATA, SAMSUNG MZ7TD256, DXT0 - S/N: unknown) size: 256.1 GB, reads: 191321 (6.6 GiB), writes: 267191 (3.9 GiB), xfer: 139200
 |-- /dev/sd0a: sd0a (unknown) Maj:Min=4:0, size: 1.1 GB @ /
 |-- /dev/sd0l: sd0l (unknown) Maj:Min=4:11, size: 196.7 GB @ /home
 |-- /dev/sd0d: sd0d (unknown) Maj:Min=4:3, size: 4.2 GB @ /tmp
 |-- /dev/sd0f: sd0f (unknown) Maj:Min=4:5, size: 2.1 GB @ /usr
 |-- /dev/sd0g: sd0g (unknown) Maj:Min=4:6, size: 1.1 GB @ /usr/X11R6
 |-- /dev/sd0h: sd0h (unknown) Maj:Min=4:7, size: 10.6 GB @ /usr/local
 |-- /dev/sd0k: sd0k (unknown) Maj:Min=4:10, size: 6.3 GB @ /usr/obj
 |-- /dev/sd0j: sd0j (unknown) Maj:Min=4:9, size: 2.1 GB @ /usr/src
 |-- /dev/sd0e: sd0e (unknown) Maj:Min=4:4, size: 20.9 GB @ /var
 cd0: (model: MATSHITA, DVD-RAM UJ8E2, SB01 - S/N: unknown) size: ?, reads: ? (?), writes: ? (?), xfer: ?
 sd1: (model: KingSpec, Z3-128, 1214 - S/N: unknown) size: 128.0 GB, reads: 33 (42.6 KiB), writes: 0 (0 bytes), xfer: 0

@mprins mprins marked this pull request as ready for review January 17, 2021 11:17
@mprins mprins merged commit e858b1e into oshi:master Jan 17, 2021
@mprins mprins deleted the openbsd branch January 17, 2021 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Operating system not supported: 5" on OpenBSD
3 participants