Re: wsconsctl.conf: mention mouse.tp.tapping in example
On Mon, Mar 22, 2021 at 08:18:45PM +0100, Klemens Nanni wrote: > I was too stupid to look at `wsconsctl' output (which needs root) and > only looked here. > > Mailing the diff for my lack of better wording, plus the knob atually > takes three values which I have yet to decode by reading wsconsctl(8) > code. The reason I had to look is because the (default) functionality does not work reliably at all: $ doas wsconsctl mouse.tp.tapping=1 mouse.tp.tapping -> 1,3,2 Single taps are always detected reliably and deliver left mouse button clicks, but tripple and double taps for middle and right mouse button clicks work are recognised so poorly that I first though multitouch support wasn't there at all. That's not the case though: two-finger srolling just works smoothly and pondering the touchpad with repeated double or tripple taps does deliver respective clicks eventually - so it's really just the tap support needing some love. > Anyone with a better text for the example so grepping "mouse" or "tap" > shows me what I'm looking for? Here's a better one. Feedback? OK? Index: etc/examples/wsconsctl.conf === RCS file: /cvs/src/etc/examples/wsconsctl.conf,v retrieving revision 1.1 diff -u -p -r1.1 wsconsctl.conf --- etc/examples/wsconsctl.conf 16 Jul 2014 13:21:33 - 1.1 +++ etc/examples/wsconsctl.conf 22 Mar 2021 21:18:35 - @@ -9,3 +9,5 @@ #display.vblank=on # enable vertical sync blank for screen burner #display.screen_off=6 # set screen burner timeout to 60 seconds #display.msact=off # disable screen unburn w/ mouse +#mouse.tp.tapping=0# 1,3,2=interpret one/three/two (simultaneous) + # tap(s) as left/middle/right mouse button click
Adding accessibility for blind and low vision individuals to OpenBSD?
So I've really wanted to try OpenBSD in a non-server configuration where I'm not installing over the internet on a remote server but on the local machine, but to my knowledge the OpenBSD installation media has no accessibility functionality whatsoever. (I'm not even sure if the installed system or any of the packages therein, such as in the ports collection, contains accessibility software.) Therefore, I'm wondering what it would take to add accessibility to the console portion of OpenBSD to begin with, as that as the simplest interface at the moment. The Orca screen reader may work on the desktop. There's a screen reader for the Linx console called Fenrir[1], but it may require functionality that is not available in OpenBSD, such as libevent. I've yet to try loading Fenrir on an installed OpenBSD system. Thoughts as to how this all could be achieved? I'm looking particular at screen readers; braille displays can be accomplished through something like brltty. [1]: https://github.com/chrys87/fenrir -- Signed, Ethin D. Probst
/dev/kmem address range check
Hi,
By specifying an invalid offset when reading /dev/kmem it is easy
to crash the kernel from userland. sysctl kern.allowkmem=0 prevents
this per default
kernel: protection fault trap, code=0
Stopped at copyout+0x53: repe movsq (%rsi),%es:(%rdi)
ddb> trace
copyout() at copyout+0x53
mmrw(201,80002145a5f8,0) at mmrw+0x199
spec_read(80002145a498) at spec_read+0xa6
VOP_READ(fd807c0d8068,80002145a5f8,0,fd807f7ba300) at VOP_READ+0x41
vn_read(fd80792bc780,80002145a5f8,1) at vn_read+0xa1
dofilereadv(8000d508,4,80002145a5f8,1,80002145a6d0) at dofilere
adv+0x14c
sys_pread(8000d508,80002145a678,80002145a6d0) at sys_pread+0x5c
syscall(80002145a740) at syscall+0x315
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7e4880, count: -9
ddb> show register
rdi 0x7f7e5100
rsi0x500bca4bc039b5b
The && when checking the direct map is wrong, it should be ||. This
bug exists since the check was added.
revision 1.3
date: 2004/07/22 00:48:41; author: art; state: Exp; lines: +4 -3;
Fix access to direct mapped memory through /dev/kmem.
When we check if a range is within the limits, we have to check the
end address including the size. So I think we should subtract the size
from the end limit.
ok?
bluhm
Index: arch/amd64/amd64/mem.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/mem.c,v
retrieving revision 1.34
diff -u -p -r1.34 mem.c
--- arch/amd64/amd64/mem.c 19 Feb 2018 08:59:52 - 1.34
+++ arch/amd64/amd64/mem.c 22 Mar 2021 20:10:16 -
@@ -148,13 +148,13 @@ mmrw(dev_t dev, struct uio *uio, int fla
case 1:
v = uio->uio_offset;
c = ulmin(iov->iov_len, MAXPHYS);
- if (v >= (vaddr_t) && v < kern_end) {
-if (v < (vaddr_t) &&
+ if (v >= (vaddr_t) && v < kern_end - c) {
+if (v < (vaddr_t) - c &&
uio->uio_rw == UIO_WRITE)
return EFAULT;
} else if ((!uvm_kernacc((caddr_t)v, c,
uio->uio_rw == UIO_READ ? B_READ : B_WRITE)) &&
- (v < PMAP_DIRECT_BASE && v > PMAP_DIRECT_END))
+ (v < PMAP_DIRECT_BASE || v > PMAP_DIRECT_END - c))
return (EFAULT);
error = uiomove((caddr_t)v, c, uio);
continue;
Re: wsconsctl.conf: mention mouse.tp.tapping in example
On Mon, Mar 22, 2021 at 08:18:45PM +0100, Klemens Nanni wrote: > I was too stupid to look at `wsconsctl' output (which needs root) and > only looked here. > > Mailing the diff for my lack of better wording, plus the knob atually > takes three values which I have yet to decode by reading wsconsctl(8) > code. Appears to be documented here: https://man.openbsd.org/wsmouse.4#mouse.tp.tapping > Anyone with a better text for the example so grepping "mouse" or "tap" > shows me what I'm looking for? > > > Index: examples/wsconsctl.conf > === > RCS file: /cvs/src/etc/examples/wsconsctl.conf,v > retrieving revision 1.1 > diff -u -p -r1.1 wsconsctl.conf > --- examples/wsconsctl.conf 16 Jul 2014 13:21:33 - 1.1 > +++ examples/wsconsctl.conf 22 Mar 2021 19:15:52 - > @@ -9,3 +9,4 @@ > #display.vblank=on # enable vertical sync blank for screen burner > #display.screen_off=6# set screen burner timeout to 60 seconds > #display.msact=off # disable screen unburn w/ mouse > +#mouse.tp.tapping=0 # 1=click on touch not physical push > >
Re: wsconsctl.conf: mention mouse.tp.tapping in example
On 3/22/21 10:19 PM, Klemens Nanni wrote: > On Mon, Mar 22, 2021 at 08:18:45PM +0100, Klemens Nanni wrote: >> I was too stupid to look at `wsconsctl' output (which needs root) and >> only looked here. >> >> Mailing the diff for my lack of better wording, plus the knob atually >> takes three values which I have yet to decode by reading wsconsctl(8) >> code. > The reason I had to look is because the (default) functionality does not > work reliably at all: > > $ doas wsconsctl mouse.tp.tapping=1 > mouse.tp.tapping -> 1,3,2 > > Single taps are always detected reliably and deliver left mouse button > clicks, but tripple and double taps for middle and right mouse button > clicks work are recognised so poorly that I first though multitouch > support wasn't there at all. Can you tell whether that's a hardware or a driver problem? wsmouse logging might be helpful here. You could enable it by $ doas wsconsctl mouse.param=256:1,257:1 , make a few two-finger taps, and deactivate it with $ doas wsconsctl mouse.param=256:0,257:0 For the output: $ grep 'wsmouse0-' /var/log/messages > > That's not the case though: two-finger srolling just works smoothly and > pondering the touchpad with repeated double or tripple taps does > deliver respective clicks eventually - so it's really just the tap > support needing some love. > >> Anyone with a better text for the example so grepping "mouse" or "tap" >> shows me what I'm looking for? > Here's a better one. > > Feedback? OK? > > > Index: etc/examples/wsconsctl.conf > === > RCS file: /cvs/src/etc/examples/wsconsctl.conf,v > retrieving revision 1.1 > diff -u -p -r1.1 wsconsctl.conf > --- etc/examples/wsconsctl.conf 16 Jul 2014 13:21:33 - 1.1 > +++ etc/examples/wsconsctl.conf 22 Mar 2021 21:18:35 - > @@ -9,3 +9,5 @@ > #display.vblank=on # enable vertical sync blank for screen burner > #display.screen_off=6# set screen burner timeout to 60 seconds > #display.msact=off # disable screen unburn w/ mouse > +#mouse.tp.tapping=0 # 1,3,2=interpret one/three/two (simultaneous) > + # tap(s) as left/middle/right mouse button click >
update explicit_bzero test to not assume SIGSTKSZ to be constant
In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
bring in asprintf/vasprintf, which causes the explicit_bzero test to
fail to compile since the size of SIGSTKSZ is no longer known at compile
time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
See http://patches-tcwg.linaro.org/patch/48127/ and
https://github.com/libressl-portable/portable/issues/653 for the
LibreSSL build failure report on Fedora Rawhide.
ok?
Index: explicit_bzero.c
===
RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 explicit_bzero.c
--- explicit_bzero.c11 Jul 2014 01:10:35 - 1.6
+++ explicit_bzero.c23 Mar 2021 01:32:21 -
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -36,16 +37,20 @@ enum {
SECRETBYTES = SECRETCOUNT * sizeof(secret)
};
-static char altstack[SIGSTKSZ + SECRETBYTES];
+static char *altstack;
+#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
static void
setup_stack(void)
{
+ altstack = malloc(ALTSTACK_SIZE);
+
const stack_t sigstk = {
.ss_sp = altstack,
- .ss_size = sizeof(altstack),
+ .ss_size = ALTSTACK_SIZE
};
+ ASSERT_NE(NULL, altstack);
ASSERT_EQ(0, sigaltstack(, NULL));
}
@@ -129,7 +134,7 @@ test_without_bzero()
char buf[SECRETBYTES];
assert_on_stack();
populate_secret(buf, sizeof(buf));
- char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
+ char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
ASSERT_NE(NULL, res);
return (res);
}
@@ -140,7 +145,7 @@ test_with_bzero()
char buf[SECRETBYTES];
assert_on_stack();
populate_secret(buf, sizeof(buf));
- char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
+ char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
ASSERT_NE(NULL, res);
explicit_bzero(buf, sizeof(buf));
return (res);
@@ -183,14 +188,14 @@ main()
* on the stack. This sanity checks that call_on_stack() and
* populate_secret() work as intended.
*/
- memset(altstack, 0, sizeof(altstack));
+ memset(altstack, 0, ALTSTACK_SIZE);
call_on_stack(do_test_without_bzero);
/*
* Now test with a call to explicit_bzero() and check that we
* *don't* find any instances of the secret data.
*/
- memset(altstack, 0, sizeof(altstack));
+ memset(altstack, 0, ALTSTACK_SIZE);
call_on_stack(do_test_with_bzero);
return (0);
Re: update explicit_bzero test to not assume SIGSTKSZ to be constant
Fine with me.
The malloc'd memory is not identical to bss memory, but this still matches
the spirit of the test.
Brent Cook wrote:
> In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
> source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
> bring in asprintf/vasprintf, which causes the explicit_bzero test to
> fail to compile since the size of SIGSTKSZ is no longer known at compile
> time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
>
> See http://patches-tcwg.linaro.org/patch/48127/ and
> https://github.com/libressl-portable/portable/issues/653 for the
> LibreSSL build failure report on Fedora Rawhide.
>
> ok?
>
> Index: explicit_bzero.c
> ===
> RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 explicit_bzero.c
> --- explicit_bzero.c 11 Jul 2014 01:10:35 - 1.6
> +++ explicit_bzero.c 23 Mar 2021 01:32:21 -
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -36,16 +37,20 @@ enum {
> SECRETBYTES = SECRETCOUNT * sizeof(secret)
> };
>
> -static char altstack[SIGSTKSZ + SECRETBYTES];
> +static char *altstack;
> +#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
>
> static void
> setup_stack(void)
> {
> + altstack = malloc(ALTSTACK_SIZE);
> +
> const stack_t sigstk = {
> .ss_sp = altstack,
> - .ss_size = sizeof(altstack),
> + .ss_size = ALTSTACK_SIZE
> };
>
> + ASSERT_NE(NULL, altstack);
> ASSERT_EQ(0, sigaltstack(, NULL));
> }
>
> @@ -129,7 +134,7 @@ test_without_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> return (res);
> }
> @@ -140,7 +145,7 @@ test_with_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> explicit_bzero(buf, sizeof(buf));
> return (res);
> @@ -183,14 +188,14 @@ main()
>* on the stack. This sanity checks that call_on_stack() and
>* populate_secret() work as intended.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_without_bzero);
>
> /*
>* Now test with a call to explicit_bzero() and check that we
>* *don't* find any instances of the secret data.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_with_bzero);
>
> return (0);
>
Re: vmctl status does not reflect the stopping state of a VM
On Fri, Mar 19, 2021 at 10:44:24AM +0100, Theo Buehler wrote: > On Thu, Mar 18, 2021 at 05:06:53PM -0400, Dave Voutila wrote: > > > > Preben Guldberg writes: > > > > > In "vmctl status", VMs that are being stopped but are still running > > > will simply show up as "running". > > > > > > The diff below gives preference to showing the stopping state akin to > > > how a paused VM is handled. > > > > I've tested this and it works as advertised. I guess I had been living > > in ignorance not knowing I should be seeing a "stopping" status! > > ok tb Committed, thanks.
wsconsctl.conf: mention mouse.tp.tapping in example
I was too stupid to look at `wsconsctl' output (which needs root) and only looked here. Mailing the diff for my lack of better wording, plus the knob atually takes three values which I have yet to decode by reading wsconsctl(8) code. Anyone with a better text for the example so grepping "mouse" or "tap" shows me what I'm looking for? Index: examples/wsconsctl.conf === RCS file: /cvs/src/etc/examples/wsconsctl.conf,v retrieving revision 1.1 diff -u -p -r1.1 wsconsctl.conf --- examples/wsconsctl.conf 16 Jul 2014 13:21:33 - 1.1 +++ examples/wsconsctl.conf 22 Mar 2021 19:15:52 - @@ -9,3 +9,4 @@ #display.vblank=on # enable vertical sync blank for screen burner #display.screen_off=6 # set screen burner timeout to 60 seconds #display.msact=off # disable screen unburn w/ mouse +#mouse.tp.tapping=0# 1=click on touch not physical push
Re: httpd.conf grammar
On Sun, Mar 21, 2021 at 10:58:02PM +, Raf Czlonka wrote: > > Hi Laurie, > > I'd simply use the existing wording, without getting into details. > > While there, "braces" dominate the manual page, with a single > occurrence of "brackets" so let's change that too, for consistency. > > Regards, > > Raf > hi. i could have sworn i recently discussed this documenting of multiple options with someone else, but cannot find the mails. anyway, a couple of points: - "a block of options that is enclosed in curly braces" can be shortened to "a block of options enclosed in curly braces". up to you. - if it is generally true that where multiple options can be given they are specified in the same way (curly braces) then it would be better just to say up front that this is the case, rather than repeating the text endlessly everywhere it's possible. in other respects i'm fine with the diff. jmc > Index: usr.sbin/httpd/httpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v > retrieving revision 1.114 > diff -u -p -r1.114 httpd.conf.5 > --- usr.sbin/httpd/httpd.conf.5 29 Oct 2020 12:30:52 - 1.114 > +++ usr.sbin/httpd/httpd.conf.5 21 Mar 2021 22:56:53 - > @@ -155,7 +155,7 @@ see > .Xr patterns 7 . > .El > .Pp > -Followed by a block of options that is enclosed in curly brackets: > +Followed by a block of options that is enclosed in curly braces: > .Bl -tag -width Ds > .It Ic alias Ar name > Specify an additional alias > @@ -282,6 +282,7 @@ will neither display nor generate a dire > .El > .It Oo Ic no Oc Ic fastcgi Oo Ar option Oc > Enable FastCGI instead of serving files. > +Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Ic socket Oo Cm tcp Oc Ar socket Oo Ar port Oc >
Re: OPENBSD-PF-MIB, use DisplayString not OCTET STRING
On 2021/03/22 00:20, Martijn van Duren wrote:
> There's two things I'd like to know before going further with this:
>
> 1) according to RFC2578 section 3.6:
> (1) registration: the definition of a particular item is registered as
> a particular OBJECT IDENTIFIER value, and associated with a
> particular descriptor. After such a registration, the semantics
> thereby associated with the value are not allowed to change, the
> OBJECT IDENTIFIER can not be used for any other registration, and
> the descriptor can not be changed nor associated with any other
> registration. The following macros result in a registration:
>
> OBJECT-TYPE, MODULE-IDENTITY, NOTIFICATION-TYPE, OBJECT-GROUP,
> OBJECT-IDENTITY, NOTIFICATION-GROUP, MODULE-COMPLIANCE,
> AGENT-CAPABILITIES.
>
> Unless I'm misreading this section this would require a registration of
> a new object to strictly comply with SMIv2. Is this change minor enough
> to allow it?
I suppose it depends what they mean by semantics. The type doesn't change,
it's still an OCTET STRING, the data don't change, the only difference is
the DISPLAY-HINT. I think in this case changing oid will cause more problems
(breaking config for anyone aleeady monitoring this) than it solves.
In terms of side-effects, the main one is that software indexing data
storage by the name in ifDescr after display-formatting may go from hex
bytes to readable strongs (the case with snmp_exporter->prometheus).
But I think most cases where people actually monitor OpenBSD MIBs are
more likely to be based on with net-snmp or possovly snmp(1) both of
which just interpret these as STRING anyway.
And in cases where this does change something it's easier to adapt to a
display format change than an oid change.
> 2) Afaik any byte sequence is technically allowed for these in pf.conf.
> (at least for pfLabelName and pfTableName I manage to make UTF-8 work),
> so changing this would technically mean that we wouldn't be able to
> export these tables and labels anymore. Note that RFC2579 section 3.1
> bullet 3 of octet-format doesn't specify how to handle invalid ascii or
> UTF-8. In snmp(1) we went with the replacement character, but how any
> other pieces of software would handle that, I don't know. Is this
> change safe enough in all situations?
>
> Pending the answers, from my limited knowledge of pf and the MIB I
> guess the impact on pfLogIfName and pfIfDescr is limited enough since
> they show the actual interface name, which is always ascii anyway.
> For pfTableName and pfLabelName it might be an idea to register a new
> object which returns something like a best effort UTF-8 presentation
> of the actual name and go with SnmpAdminString.
Good catch about UTF-8, so these should be SnmpAdminString, I think the
same applies about changing oid though.
> On Sat, 2021-03-20 at 14:29 +, Stuart Henderson wrote:
> > DisplayString seems a better type for pfIfDescr and some of the other
> > objects; this improves the display format in Prometheus snmp_exporter
> > which uses hex values for OCTET STRING.
> >
> > OK?
> >
> >
> > Index: OPENBSD-PF-MIB.txt
> > ===
> > RCS file: /cvs/src/share/snmp/OPENBSD-PF-MIB.txt,v
> > retrieving revision 1.6
> > diff -u -p -r1.6 OPENBSD-PF-MIB.txt
> > --- OPENBSD-PF-MIB.txt 19 Jun 2018 10:08:45 - 1.6
> > +++ OPENBSD-PF-MIB.txt 20 Mar 2021 14:24:11 -
> > @@ -23,7 +23,7 @@ IMPORTS
> > TimeTicks, enterprises
> > FROM SNMPv2-SMI
> >
> > - TruthValue
> > + TruthValue, DisplayString
> > FROM SNMPv2-TC
> >
> > openBSD
> > @@ -33,7 +33,7 @@ IMPORTS
> > FROM SNMPv2-CONF;
> >
> > pfMIBObjects MODULE-IDENTITY
> > - LAST-UPDATED "201506091728Z"
> > + LAST-UPDATED "202103201421Z"
> > ORGANIZATION "OpenBSD"
> > CONTACT-INFO "
> > Author: Joel Knight
> > @@ -43,6 +43,8 @@ pfMIBObjects MODULE-IDENTITY
> > DESCRIPTION "The MIB module for gathering information from
> > OpenBSD's packet filter.
> > "
> > + REVISION "202103201421Z"
> > + DESCRIPTION "Use DisplayString not OCTET STRING where appropriate"
> > REVISION "201506091728Z"
> > DESCRIPTION "Add separate counter for failed 'route-to' applications"
> > REVISION "201308310446Z"
> > @@ -300,7 +302,7 @@ pfStateRemovals OBJECT-TYPE
> > -- pfLogInterface
> >
> > pfLogIfName OBJECT-TYPE
> > - SYNTAX OCTET STRING
> > + SYNTAX DisplayString
> > MAX-ACCESS read-only
> > STATUS current
> > DESCRIPTION
> > @@ -683,7 +685,7 @@ pfIfEntry OBJECT-TYPE
> > PfIfEntry ::=
> > SEQUENCE {
> > pfIfIndex Integer32,
> > - pfIfDescr OCTET STRING,
> > + pfIfDescr DisplayString,
> >
uvm_page_physload: use km_alloc(9)
Convert the last MI uvm_km_zalloc(9) to km_alloc(9), ok?
Index: uvm/uvm_page.c
===
RCS file: /cvs/src/sys/uvm/uvm_page.c,v
retrieving revision 1.155
diff -u -p -r1.155 uvm_page.c
--- uvm/uvm_page.c 19 Jan 2021 13:21:36 - 1.155
+++ uvm/uvm_page.c 22 Mar 2021 10:23:39 -
@@ -542,8 +542,8 @@ uvm_page_physload(paddr_t start, paddr_t
npages = end - start; /* # of pages */
- pgs = (struct vm_page *)uvm_km_zalloc(kernel_map,
- npages * sizeof(*pgs));
+ pgs = km_alloc(npages * sizeof(*pgs), _any, _zero,
+ _waitok);
if (pgs == NULL) {
printf("uvm_page_physload: can not malloc vm_page "
"structs for segment\n");
Re: vmm crash on 6.9-beta
> On 21 Mar 2021, at 02:31, Theo de Raadt wrote: > Otto Moerbeek wrote: >> On Fri, Mar 19, 2021 at 04:15:31PM +, Stuart Henderson wrote: >> >>> On 2021/03/19 17:05, Jan Klemkow wrote: Hi, I had the same issue a few days ago a server hardware of mine. I just ran 'cvs up'. So, it looks like a generic bug in FFS and not related to vmm. >>> >>> This panic generally relates to filesystem corruption. If fsck doesn't >>> help then recreating which filesystem is triggering it is usually needed. >> >> Yeah, once in a while we see reports of it. It seems to be some nasty >> conspiracy between the generic filesystem code, ffs and fsck_ffs. >> Maybe even the device (driver) itself is involved. A possible >> underlying issue may be that some operation are re-ordered while they >> should not. > > Yes, it does hint at a reordering. > >> Now the strange thing is, fsck_ffs *should* be able to repair the >> inconsistency, but it appears in some cases it is not, and some bits >> on the disk remain to trigger it again. > > fsck_ffs can only repair one inconsistancy. There are a number of lockstep > operations, I suppose we can call them acid-in-lowercase, which allow fsck > to determine at which point the crashed system gave up the ghost. fsck then > removes the partial operations, leaving a viable filesystem. But if the disk > layer lands later writes but not earlier writes, fsck cannot handle it. I managed to re-create the issue. Created a fresh install qcow2 image and derived 35 new VMs from it. Then I started all the VMs in four cycles, 10 VMs per cycle and waiting 240 seconds after each cycle. Similar to the staggered start based on the amount of CPUs. This time it was “only” one VM that was affected by this. VM four that got started. ddb> show panic ffs_valloc: dup alloc ddb> trace db_enter() at db_enter+0x10 panic(81dc21b2) at panic+0x12a ffs_inode_alloc(fd803c94ef00,81a4,fd803f7bbf00,800014d728b8) at ffs _inode_alloc+0x442 ufs_makeinode(81a4,fd803c930908,800014d72bb0,800014d72c00) at ufs_m akeinode+0x7f ufs_create(800014d72960) at ufs_create+0x3c VOP_CREATE(fd803c930908,800014d72bb0,800014d72c00,800014d729c0) at VOP_CREATE+0x4a vn_open(800014d72b80,602,1a4) at vn_open+0x182 doopenat(8000c778,ff9c,f8fc28f00f4,601,1b6,800014d72d80) at doo penat+0x1d0 syscall(800014d72df0) at syscall+0x315 Xsyscall() at Xsyscall+0x128 end of kernel end trace frame: 0x7f7be450, count: -10 dmesg of the host below. Mischa OpenBSD 6.9-beta (GENERIC.MP) #421: Sun Mar 21 13:17:22 MDT 2021 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 137374924800 (131010MB) avail mem = 133196165120 (127025MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbf42c000 (99 entries) bios0: vendor Dell Inc. version "2.8.0" date 06/26/2019 bios0: Dell Inc. PowerEdge R620 acpi0 at bios0: ACPI 3.0 acpi0: sleep states S0 S4 S5 acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WD__ SLIC ERST HEST BERT EINJ TCPA PC__ SRAT SSDT acpi0: wakeup devices PCI0(S5) PCI1(S5) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.34 MHz, 06-2d-07 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE cpu1 at mainbus0: apid 32 (application processor) cpu1: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 1200.02 MHz, 06-2d-07 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 0, package 1 cpu2 at mainbus0: apid 2 (application processor) cpu2: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07 cpu2:
witness: skip first frame when saving stacktraces
The top frame is always `witness_checkorder', at least on amd64. Diff
below makes use of stacktrace_save_at() to skip it.
Previous output:
lock order ">lock"(rwlock) -> ">am_lock"(rwlock) first seen at:
#0 witness_checkorder+0x4d7 [/home/os/openbsd/sys/sys/stacktrace.h:0]
#1 rw_enter_write+0x43 [/home/os/openbsd/sys/kern/kern_rwlock.c:128]
#2 amap_ref+0x24 [/home/os/openbsd/sys/uvm/uvm_amap.c:1341]
#3 uvm_mapent_clone+0x129 [/home/os/openbsd/sys/uvm/uvm_map.c:3826]
#4 uvm_map_extract+0x324 [/home/os/openbsd/sys/uvm/uvm_map.c:4582]
#5 sys_kbind+0x2dd [/home/os/openbsd/sys/uvm/uvm_mmap.c:1174]
#6 syscall+0x389 [/home/os/openbsd/sys/sys/syscall_mi.h:102]
#7 Xsyscall+0x128
With this diff:
lock order ">lock"(rwlock) -> ">am_lock"(rwlock) first seen at:
#0 rw_enter_write+0x43 [/home/os/openbsd/sys/kern/kern_rwlock.c:128]
#1 amap_ref+0x24 [/home/os/openbsd/sys/uvm/uvm_amap.c:1341]
#2 uvm_mapent_clone+0x129 [/home/os/openbsd/sys/uvm/uvm_map.c:3826]
#3 uvm_map_extract+0x324 [/home/os/openbsd/sys/uvm/uvm_map.c:4582]
#4 sys_kbind+0x2dd [/home/os/openbsd/sys/uvm/uvm_mmap.c:1174]
#5 syscall+0x389 [/home/os/openbsd/sys/sys/syscall_mi.h:102]
#6 Xsyscall+0x128
ok?
Index: kern/subr_witness.c
===
RCS file: /cvs/src/sys/kern/subr_witness.c,v
retrieving revision 1.46
diff -u -p -r1.46 subr_witness.c
--- kern/subr_witness.c 10 Mar 2021 10:21:47 - 1.46
+++ kern/subr_witness.c 22 Mar 2021 10:00:15 -
@@ -764,7 +764,7 @@ witness_checkorder(struct lock_object *l
if (witness_cold || witness_watch < 1 || panicstr != NULL || db_active)
return;
-
+
if ((lock->lo_flags & LO_INITIALIZED) == 0) {
if (witness_uninitialized_report > 0) {
witness_uninitialized_report--;
@@ -2472,7 +2472,7 @@ witness_lock_order_add(struct witness *p
data->wlod_key = key;
w_lohash.wloh_array[hash] = data;
w_lohash.wloh_count++;
- stacktrace_save(>wlod_stack);
+ stacktrace_save_at(>wlod_stack, 1);
return (1);
}
namei/execve: entanglement's simplification
Hi,
The following diff tries to simplify a bit the entanglement of namei
data and execve(2) syscall.
Currently, when called, sys_execve() might doing recursive calls of
check_exec() with a shared `struct nameidata' (`ep_ndp' inside
`struct exec_package').
I would like to disassociate them, and make check_exec() to "own" the
`struct nameidata' data. execve(2) is complex enough, no needs to adds
namei() complexity inside.
check_exec() will initialize nameidata, call namei() (as now), extract
useful information for the caller (only the vnode and the
command-name, returned via exec_package struct), and free other namei
ressources.
To proceed, it only needs to know the wanted path (to properly init
`nd' with NDINIT).
As the call of check_exec() could be recursive (when scripts are
involved), the path could come from:
- directly from sys_execve() via SCARG(uap, path) (from userspace)
- from exec_script_makecmds() via the shellname (from systemspace)
In `struct exec_package', I opted to reuse `ep_name' for passing the
path: it is what sys_execve() is already doing at first call. Later it
is only used for construct the fake-args list in
exec_script_makecmds(), and it could being overrided after that (and
restored on error). I reordered them a bit to make it fit.
Eventually I could also introduce a new struct field for the wanted
path.
`ep_segflg' is introduced to mark if ep_name comes from UIO_USERSPACE
or UIO_SYSSPACE. it will be used by NDINIT() in check_exec().
`ep_comm' is the other information (with ep_vp) wanted in result of
check_exec() call. it will be copied to `ps_comm'.
Comments or OK ?
--
Sebastien Marie
diff 8de782433999755fc9356c8ed8dc7b327e532351 /home/semarie/repos/openbsd/src
blob - a31fa5a3269a3b568450066a1bd001317d710354
file + sys/kern/exec_script.c
--- sys/kern/exec_script.c
+++ sys/kern/exec_script.c
@@ -64,19 +64,24 @@ exec_script_makecmds(struct proc *p, struct exec_packa
{
int error, hdrlinelen, shellnamelen, shellarglen;
char *hdrstr = epp->ep_hdr;
- char *cp, *shellname, *shellarg, *oldpnbuf;
+ char *cp, *shellname, *shellarg;
char **shellargp = NULL, **tmpsap;
struct vnode *scriptvp;
+ char old_comm[MAXCOMLEN+1];
+ char *old_name;
+ enum uio_seg old_segflg;
uid_t script_uid = -1;
gid_t script_gid = -1;
u_short script_sbits;
/*
-* remember the old vp and pnbuf for later, so we can restore
+* remember the old epp values for later, so we can restore
* them if check_exec() fails.
*/
+ old_segflg = epp->ep_segflg;
+ old_name = epp->ep_name;
scriptvp = epp->ep_vp;
- oldpnbuf = epp->ep_ndp->ni_cnd.cn_pnbuf;
+ strlcpy(old_comm, epp->ep_comm, sizeof(old_comm));
/*
* if the magic isn't that of a shell script, or we've already
@@ -186,13 +191,7 @@ check_shell:
FRELE(fp, p);
}
- /* set up the parameters for the recursive check_exec() call */
- epp->ep_ndp->ni_dirfd = AT_FDCWD;
- epp->ep_ndp->ni_dirp = shellname;
- epp->ep_ndp->ni_segflg = UIO_SYSSPACE;
- epp->ep_flags |= EXEC_INDIR;
-
- /* and set up the fake args list, for later */
+ /* set up the fake args list, for later */
shellargp = mallocarray(4, sizeof(char *), M_EXEC, M_WAITOK);
tmpsap = shellargp;
*tmpsap = malloc(shellnamelen + 1, M_EXEC, M_WAITOK);
@@ -214,6 +213,11 @@ check_shell:
tmpsap++;
*tmpsap = NULL;
+ /* set up the parameters for the recursive check_exec() call */
+ epp->ep_segflg = UIO_SYSSPACE;
+ epp->ep_name = shellname;
+ epp->ep_flags |= EXEC_INDIR;
+
/*
* mark the header we have as invalid; check_exec will read
* the header from the new executable
@@ -233,9 +237,6 @@ check_shell:
if ((epp->ep_flags & EXEC_HASFD) == 0)
vn_close(scriptvp, FREAD, p->p_ucred, p);
- /* free the old pathname buffer */
- pool_put(_pool, oldpnbuf);
-
epp->ep_flags |= (EXEC_HASARGL | EXEC_SKIPARG);
epp->ep_fa = shellargp;
/*
@@ -250,8 +251,13 @@ check_shell:
return (0);
}
- /* XXX oldpnbuf not set for "goto fail" path */
- epp->ep_ndp->ni_cnd.cn_pnbuf = oldpnbuf;
+ /*
+* no need to restore these ep_* in 'fail' as there were not
+* changed at this point
+*/
+ epp->ep_segflg = old_segflg;
+ epp->ep_name = old_name;
+ strlcpy(epp->ep_comm, old_comm, sizeof(epp->ep_comm));
fail:
/* note that we've clobbered the header */
epp->ep_flags |= EXEC_DESTR;
@@ -265,8 +271,6 @@ fail:
} else
vn_close(scriptvp, FREAD, p->p_ucred, p);
- pool_put(_pool, epp->ep_ndp->ni_cnd.cn_pnbuf);
-
/* free the fake arg list, because we're not returning it */
Re: malloc: use km_alloc(9) for kmemusage
> Date: Mon, 22 Mar 2021 11:27:40 +0100
> From: Martin Pieuchot
>
> Diff below convert a use of uvm_km_zalloc(9) to km_alloc(9), this memory
> is never released, ok?
This will need some careful testing on multiple architectures.
> Index: kern/kern_malloc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_malloc.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 kern_malloc.c
> --- kern/kern_malloc.c23 Feb 2021 13:50:16 - 1.144
> +++ kern/kern_malloc.c22 Mar 2021 10:23:42 -
> @@ -580,8 +580,8 @@ kmeminit(void)
> FALSE, _map_store);
> kmembase = (char *)base;
> kmemlimit = (char *)limit;
> - kmemusage = (struct kmemusage *) uvm_km_zalloc(kernel_map,
> - (vsize_t)(nkmempages * sizeof(struct kmemusage)));
> + kmemusage = km_alloc(round_page(nkmempages * sizeof(struct kmemusage)),
> + _any, _zero, _waitok);
> for (indx = 0; indx < MINBUCKET + 16; indx++) {
> XSIMPLEQ_INIT([indx].kb_freelist);
> }
>
>
Re: uvm_page_physload: use km_alloc(9)
> Date: Mon, 22 Mar 2021 11:29:52 +0100
> From: Martin Pieuchot
>
> Convert the last MI uvm_km_zalloc(9) to km_alloc(9), ok?
Also needs some careful testing on multiple architectures.
> Index: uvm/uvm_page.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> retrieving revision 1.155
> diff -u -p -r1.155 uvm_page.c
> --- uvm/uvm_page.c19 Jan 2021 13:21:36 - 1.155
> +++ uvm/uvm_page.c22 Mar 2021 10:23:39 -
> @@ -542,8 +542,8 @@ uvm_page_physload(paddr_t start, paddr_t
>
> npages = end - start; /* # of pages */
>
> - pgs = (struct vm_page *)uvm_km_zalloc(kernel_map,
> - npages * sizeof(*pgs));
> + pgs = km_alloc(npages * sizeof(*pgs), _any, _zero,
> + _waitok);
> if (pgs == NULL) {
> printf("uvm_page_physload: can not malloc vm_page "
> "structs for segment\n");
>
>
malloc: use km_alloc(9) for kmemusage
Diff below convert a use of uvm_km_zalloc(9) to km_alloc(9), this memory
is never released, ok?
Index: kern/kern_malloc.c
===
RCS file: /cvs/src/sys/kern/kern_malloc.c,v
retrieving revision 1.144
diff -u -p -r1.144 kern_malloc.c
--- kern/kern_malloc.c 23 Feb 2021 13:50:16 - 1.144
+++ kern/kern_malloc.c 22 Mar 2021 10:23:42 -
@@ -580,8 +580,8 @@ kmeminit(void)
FALSE, _map_store);
kmembase = (char *)base;
kmemlimit = (char *)limit;
- kmemusage = (struct kmemusage *) uvm_km_zalloc(kernel_map,
- (vsize_t)(nkmempages * sizeof(struct kmemusage)));
+ kmemusage = km_alloc(round_page(nkmempages * sizeof(struct kmemusage)),
+ _any, _zero, _waitok);
for (indx = 0; indx < MINBUCKET + 16; indx++) {
XSIMPLEQ_INIT([indx].kb_freelist);
}
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 14:27, Bryan Steele wrote:
> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>>
>>> On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>>>
> Created a fresh install qcow2 image and derived 35 new VMs from it.
> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
> 240 seconds after each cycle.
> Similar to the staggered start based on the amount of CPUs.
>>>
For me this is not enough info to even try to reproduce, I know little
of vmm or vmd and have no idea what "derive" means in this context.
>>>
>>> This is a big bit of information that was missing from the original
>>
>> Well.. could have been better described indeed. :))
>> " I created 41 additional VMs based on a single qcow2 base image.”
>>
>>> report ;) qcow has a concept of a read-only base image (or 'backing
>>> file') which can be shared between VMs, with writes diverted to a
>>> separate image ('derived image').
>>>
>>> So e.g. you can create a base image, do a simple OS install for a
>>> particular OS version to that base image, then you stop using that
>>> for a VM and just use it as a base to create derived images from.
>>> You then run VMs using the derived image and make whatever config
>>> changes. If you have a bunch of VMs using the same OS release then
>>> you save some disk space for the common files.
>>>
>>> Mischa did you leave a VM running which is working on the base
>>> image directly? That would certainly cause problems.
>>
>> I did indeed. Let me try that again without keeping the base image running.
>>
>> Mischa
>
> I seemed to recall that the base image is not supposed to be modified,
> so this is a pretty big omission.
>
> Per original commit message:
>
> "A limitation of this format is that modifying the base image will
> corrupt the derived image."
>
> https://marc.info/?l=openbsd-cvs=153901633011716=2
Makes a lot of sense. I guess a man page patch is in order.
Mischa
Re: httpd.conf grammar
On Mon, Mar 22, 2021 at 07:08:32AM GMT, Jason McIntyre wrote: > On Sun, Mar 21, 2021 at 10:58:02PM +, Raf Czlonka wrote: > > > > Hi Laurie, > > > > I'd simply use the existing wording, without getting into details. > > > > While there, "braces" dominate the manual page, with a single > > occurrence of "brackets" so let's change that too, for consistency. > > > > Regards, > > > > Raf > > Hi Jason, > hi. i could have sworn i recently discussed this documenting of multiple > options with someone else, but cannot find the mails. anyway, a couple > of points: > > - "a block of options that is enclosed in curly braces" can be shortened to > "a block of options enclosed in curly braces". up to you. Yup. > - if it is generally true that where multiple options can be given they > are specified in the same way (curly braces) then it would be better > just to say up front that this is the case, rather than repeating the > text endlessly everywhere it's possible. I did think about it as there's a lot of repetition: $ grep '^Multiple options' /usr/share/man/man5/httpd.conf.5 Multiple options may be specified within curly braces. Multiple options may be specified within curly braces. Multiple options may be specified within curly braces. Multiple options may be specified within curly braces. Multiple options may be specified within curly braces. Multiple options may be specified within curly braces. but wanted the diff to be as small as possible ;^) > in other respects i'm fine with the diff. > jmc How about this? Cheers, Raf Index: usr.sbin/httpd/httpd.conf.5 === RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v retrieving revision 1.114 diff -u -p -r1.114 httpd.conf.5 --- usr.sbin/httpd/httpd.conf.5 29 Oct 2020 12:30:52 - 1.114 +++ usr.sbin/httpd/httpd.conf.5 22 Mar 2021 13:24:58 - @@ -78,6 +78,8 @@ the comment is effective until the end o Argument names not beginning with a letter, digit, or underscore must be quoted. .Pp +Multiple options may be specified within curly braces. +.Pp Additional configuration files can be included with the .Ic include keyword, for example: @@ -155,7 +157,7 @@ see .Xr patterns 7 . .El .Pp -Followed by a block of options that is enclosed in curly brackets: +Followed by a block of options enclosed in curly braces: .Bl -tag -width Ds .It Ic alias Ar name Specify an additional alias @@ -238,7 +240,6 @@ option. .El .It Ic connection Ar option Set the specified options and limits for HTTP connections. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Ic max request body Ar number @@ -265,7 +266,6 @@ Set the default media type for the speci overwriting the global setting. .It Ic directory Ar option Set the specified options when serving or accessing directories. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Oo Ic no Oc Ic auto index @@ -451,7 +451,6 @@ but can be changed per server or locatio Use the .Ic no log directive to disable logging of any requests. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Ic access Ar name @@ -509,7 +508,6 @@ Disable any previous in a location. .It Ic request Ar option Configure the options for the request path. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Oo Ic no Oc Ic rewrite Ar path @@ -547,7 +545,6 @@ Enable or disable the specified TCP/IP o and .Xr ip 4 for more information about the options. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Ic backlog Ar number @@ -577,7 +574,6 @@ This will affect the TCP window size. .It Ic tls Ar option Set the TLS configuration for the server. These options are only used if TLS has been enabled via the listen directive. -Multiple options may be specified within curly braces. Valid options are: .Bl -tag -width Ds .It Ic certificate Ar file
Re: vmm crash on 6.9-beta
> > I'm more familiar with the vmd(8) codebase than any ffs stuff, but I > > don't think the issue is the base image being r/w. > > AFAIKS, the issue is that if you start a vm modifying the base because it > uses it as a regular image, that r/o open for the other vms does not > matter a lot, vmd could possibly refuse to use an image as a base for a derived image if it already has the base open (or vice-versa), but then some other software (e.g. qemu, qemu-img) could modify the base image too, there are always ways to break things not matter what the safeguards.
Re: fork(2), PT_ATTACH & SIGTRAP
> Date: Sun, 21 Mar 2021 16:56:47 +0100
> From: Martin Pieuchot
>
> On 21/03/21(Sun) 13:42, Mark Kettenis wrote:
> > > Date: Sat, 20 Mar 2021 13:10:17 +0100
> > > From: Martin Pieuchot
> > >
> > > On SP systems, like bluhm@'s armv7 regression machine, the kern/ptrace2
> > > test is failing due to a subtle behavior. Diff below makes it pass.
> > >
> > > http://bluhm.genua.de/regress/results/2021-03-19T15%3A17%3A02Z/logs/sys/kern/ptrace2/make.log
> > >
> > > The failing test does a fork(2) and the parent issues a PT_ATTACH on the
> > > child before it has been scheduled for the first time. Then the parent
> > > goes to sleep in waitpid() and when the child starts executing the check
> > > below overwrites the ptrace(2)-received SIGSTOP by a SIGTRAP.
> > >
> > > This scenario doesn't seem to happen on MP machine because the child
> > > starts to execute itself on a different core right after sys_fork() is
> > > finished.
> > >
> > > What is the purpose of this check? Should it be relaxed or removed?
> >
> > This is part of PT_SET_EVENT_MASK/PTRACE_FORK support:
> >
> > https://github.com/openbsd/src/commit/f38bed7f869bd3503530c554b4860228ea4e8641
> >
> > When reporting of the PTRACE_FORK event is requested, the debugger
> > expects to see a SIGTRAP in both the parent and the child. The code
> > expects that the only way to have PS_TRACED set in the child from the
> > start is when PTRACE_FORK is requested. But the failing test shows
> > there is a race with PT_ATTACH.
>
> Thanks for the explanation.
>
> > I think the solution is to have fork1() only run fork_return() if the
> > FORK_PTRACE flag is set, and use run child_return() otherwise.
>
> Diff below does that and prevent the race, ok?
ok kettenis@
> Index: kern/kern_fork.c
> ===
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> retrieving revision 1.234
> diff -u -p -r1.234 kern_fork.c
> --- kern/kern_fork.c 15 Feb 2021 09:35:59 - 1.234
> +++ kern/kern_fork.c 21 Mar 2021 15:55:26 -
> @@ -95,12 +95,15 @@ fork_return(void *arg)
> int
> sys_fork(struct proc *p, void *v, register_t *retval)
> {
> + void (*func)(void *) = child_return;
> int flags;
>
> flags = FORK_FORK;
> - if (p->p_p->ps_ptmask & PTRACE_FORK)
> + if (p->p_p->ps_ptmask & PTRACE_FORK) {
> flags |= FORK_PTRACE;
> - return fork1(p, flags, fork_return, NULL, retval, NULL);
> + func = fork_return;
> + }
> + return fork1(p, flags, func, NULL, retval, NULL);
> }
>
> int
>
Re: vmm crash on 6.9-beta
> > Created a fresh install qcow2 image and derived 35 new VMs from it.
> > Then I started all the VMs in four cycles, 10 VMs per cycle and waiting 240
> > seconds after each cycle.
> > Similar to the staggered start based on the amount of CPUs.
> For me this is not enough info to even try to reproduce, I know little
> of vmm or vmd and have no idea what "derive" means in this context.
This is a big bit of information that was missing from the original
report ;) qcow has a concept of a read-only base image (or 'backing
file') which can be shared between VMs, with writes diverted to a
separate image ('derived image').
So e.g. you can create a base image, do a simple OS install for a
particular OS version to that base image, then you stop using that
for a VM and just use it as a base to create derived images from.
You then run VMs using the derived image and make whatever config
changes. If you have a bunch of VMs using the same OS release then
you save some disk space for the common files.
Mischa did you leave a VM running which is working on the base
image directly? That would certainly cause problems.
> Would it be possiblet for you to show the exact steps (preferably a
> script) to reproduce the issue?
>
> Though the specific hardware might play a role as well...
>
> -Otto
> >
> > Mischa
> >
> > OpenBSD 6.9-beta (GENERIC.MP) #421: Sun Mar 21 13:17:22 MDT 2021
> > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > real mem = 137374924800 (131010MB)
> > avail mem = 133196165120 (127025MB)
> > random: good seed from bootblocks
> > mpath0 at root
> > scsibus0 at mpath0: 256 targets
> > mainbus0 at root
> > bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbf42c000 (99 entries)
> > bios0: vendor Dell Inc. version "2.8.0" date 06/26/2019
> > bios0: Dell Inc. PowerEdge R620
> > acpi0 at bios0: ACPI 3.0
> > acpi0: sleep states S0 S4 S5
> > acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WD__ SLIC ERST HEST BERT
> > EINJ TCPA PC__ SRAT SSDT
> > acpi0: wakeup devices PCI0(S5) PCI1(S5)
> > acpitimer0 at acpi0: 3579545 Hz, 24 bits
> > acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> > cpu0 at mainbus0: apid 0 (boot processor)
> > cpu0: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.34 MHz, 06-2d-07
> > cpu0:
> > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> > cpu0: 256KB 64b/line 8-way L2 cache
> > cpu0: smt 0, core 0, package 0
> > mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
> > cpu0: apic clock running at 99MHz
> > cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
> > cpu1 at mainbus0: apid 32 (application processor)
> > cpu1: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 1200.02 MHz, 06-2d-07
> > cpu1:
> > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> > cpu1: 256KB 64b/line 8-way L2 cache
> > cpu1: smt 0, core 0, package 1
> > cpu2 at mainbus0: apid 2 (application processor)
> > cpu2: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
> > cpu2:
> > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> > cpu2: 256KB 64b/line 8-way L2 cache
> > cpu2: smt 0, core 1, package 0
> > cpu3 at mainbus0: apid 34 (application processor)
> > cpu3: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
> > cpu3:
> > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> > cpu3: 256KB 64b/line 8-way L2 cache
> > cpu3: smt 0, core 1, package 1
> > cpu4 at mainbus0: apid 4 (application processor)
> > cpu4: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
> > cpu4:
> >
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>
>
> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
> >
> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
> >>> 240 seconds after each cycle.
> >>> Similar to the staggered start based on the amount of CPUs.
> >
> >> For me this is not enough info to even try to reproduce, I know little
> >> of vmm or vmd and have no idea what "derive" means in this context.
> >
> > This is a big bit of information that was missing from the original
>
> Well.. could have been better described indeed. :))
> " I created 41 additional VMs based on a single qcow2 base image.”
>
> > report ;) qcow has a concept of a read-only base image (or 'backing
> > file') which can be shared between VMs, with writes diverted to a
> > separate image ('derived image').
> >
> > So e.g. you can create a base image, do a simple OS install for a
> > particular OS version to that base image, then you stop using that
> > for a VM and just use it as a base to create derived images from.
> > You then run VMs using the derived image and make whatever config
> > changes. If you have a bunch of VMs using the same OS release then
> > you save some disk space for the common files.
> >
> > Mischa did you leave a VM running which is working on the base
> > image directly? That would certainly cause problems.
>
> I did indeed. Let me try that again without keeping the base image running.
>
> Mischa
I seemed to recall that the base image is not supposed to be modified,
so this is a pretty big omission.
Per original commit message:
"A limitation of this format is that modifying the base image will
corrupt the derived image."
https://marc.info/?l=openbsd-cvs=153901633011716=2
-Bryan.
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 14:30, Otto Moerbeek wrote:
> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>>> On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>>>
> Created a fresh install qcow2 image and derived 35 new VMs from it.
> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
> 240 seconds after each cycle.
> Similar to the staggered start based on the amount of CPUs.
>>>
For me this is not enough info to even try to reproduce, I know little
of vmm or vmd and have no idea what "derive" means in this context.
>>>
>>> This is a big bit of information that was missing from the original
>>
>> Well.. could have been better described indeed. :))
>> " I created 41 additional VMs based on a single qcow2 base image.”
>>
>>> report ;) qcow has a concept of a read-only base image (or 'backing
>>> file') which can be shared between VMs, with writes diverted to a
>>> separate image ('derived image').
>>>
>>> So e.g. you can create a base image, do a simple OS install for a
>>> particular OS version to that base image, then you stop using that
>>> for a VM and just use it as a base to create derived images from.
>>> You then run VMs using the derived image and make whatever config
>>> changes. If you have a bunch of VMs using the same OS release then
>>> you save some disk space for the common files.
>>>
>>> Mischa did you leave a VM running which is working on the base
>>> image directly? That would certainly cause problems.
>>
>> I did indeed. Let me try that again without keeping the base image running.
>
> Right. As a safeguard, I would change the base image to be r/o.
>
> I was just looking at your script and scratching my head: why is Mischa
> starting vm01 ...
>
> -Otto
Normally I don’t use derived images, was a way to get a bunch of VMs running
quickly to put some more load on veb/vport.
I have moved vm01 out of the way to vm00 and redid the whole process.
Seems much better now.
Thank you all for showing me the way. :)
Mischa
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 11:34:25AM +0100, Mischa wrote: > > On 21 Mar 2021, at 02:31, Theo de Raadt wrote: > > Otto Moerbeek wrote: > >> On Fri, Mar 19, 2021 at 04:15:31PM +, Stuart Henderson wrote: > >> > >>> On 2021/03/19 17:05, Jan Klemkow wrote: > Hi, > > I had the same issue a few days ago a server hardware of mine. I just > ran 'cvs up'. So, it looks like a generic bug in FFS and not related to > vmm. > >>> > >>> This panic generally relates to filesystem corruption. If fsck doesn't > >>> help then recreating which filesystem is triggering it is usually needed. > >> > >> Yeah, once in a while we see reports of it. It seems to be some nasty > >> conspiracy between the generic filesystem code, ffs and fsck_ffs. > >> Maybe even the device (driver) itself is involved. A possible > >> underlying issue may be that some operation are re-ordered while they > >> should not. > > > > Yes, it does hint at a reordering. > > > >> Now the strange thing is, fsck_ffs *should* be able to repair the > >> inconsistency, but it appears in some cases it is not, and some bits > >> on the disk remain to trigger it again. > > > > fsck_ffs can only repair one inconsistancy. There are a number of lockstep > > operations, I suppose we can call them acid-in-lowercase, which allow fsck > > to determine at which point the crashed system gave up the ghost. fsck then > > removes the partial operations, leaving a viable filesystem. But if the > > disk > > layer lands later writes but not earlier writes, fsck cannot handle it. > > I managed to re-create the issue. > > Created a fresh install qcow2 image and derived 35 new VMs from it. > Then I started all the VMs in four cycles, 10 VMs per cycle and waiting 240 > seconds after each cycle. > Similar to the staggered start based on the amount of CPUs. > > This time it was “only” one VM that was affected by this. VM four that got > started. > > ddb> show panic > ffs_valloc: dup alloc > ddb> trace > db_enter() at db_enter+0x10 > panic(81dc21b2) at panic+0x12a > ffs_inode_alloc(fd803c94ef00,81a4,fd803f7bbf00,800014d728b8) at > ffs > _inode_alloc+0x442 > ufs_makeinode(81a4,fd803c930908,800014d72bb0,800014d72c00) at > ufs_m > akeinode+0x7f > ufs_create(800014d72960) at ufs_create+0x3c > VOP_CREATE(fd803c930908,800014d72bb0,800014d72c00,800014d729c0) > at VOP_CREATE+0x4a > vn_open(800014d72b80,602,1a4) at vn_open+0x182 > doopenat(8000c778,ff9c,f8fc28f00f4,601,1b6,800014d72d80) at > doo > penat+0x1d0 > syscall(800014d72df0) at syscall+0x315 > Xsyscall() at Xsyscall+0x128 > end of kernel > end trace frame: 0x7f7be450, count: -10 > > dmesg of the host below. For me this is not enough info to even try to reproduce, I know little of vmm or vmd and have no idea what "derive" means in this context. Would it be possiblet for you to show the exact steps (preferably a script) to reproduce the issue? Though the specific hardware might play a role as well... -Otto > > Mischa > > OpenBSD 6.9-beta (GENERIC.MP) #421: Sun Mar 21 13:17:22 MDT 2021 > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP > real mem = 137374924800 (131010MB) > avail mem = 133196165120 (127025MB) > random: good seed from bootblocks > mpath0 at root > scsibus0 at mpath0: 256 targets > mainbus0 at root > bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbf42c000 (99 entries) > bios0: vendor Dell Inc. version "2.8.0" date 06/26/2019 > bios0: Dell Inc. PowerEdge R620 > acpi0 at bios0: ACPI 3.0 > acpi0: sleep states S0 S4 S5 > acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WD__ SLIC ERST HEST BERT > EINJ TCPA PC__ SRAT SSDT > acpi0: wakeup devices PCI0(S5) PCI1(S5) > acpitimer0 at acpi0: 3579545 Hz, 24 bits > acpimadt0 at acpi0 addr 0xfee0: PC-AT compat > cpu0 at mainbus0: apid 0 (boot processor) > cpu0: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.34 MHz, 06-2d-07 > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN > cpu0: 256KB 64b/line 8-way L2 cache > cpu0: smt 0, core 0, package 0 > mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges > cpu0: apic clock running at 99MHz > cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE > cpu1 at mainbus0: apid 32 (application processor) > cpu1: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 1200.02 MHz, 06-2d-07 > cpu1: >
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 13:08, Otto Moerbeek wrote:
> On Mon, Mar 22, 2021 at 11:34:25AM +0100, Mischa wrote:
>
>>> On 21 Mar 2021, at 02:31, Theo de Raadt wrote:
>>> Otto Moerbeek wrote:
On Fri, Mar 19, 2021 at 04:15:31PM +, Stuart Henderson wrote:
> On 2021/03/19 17:05, Jan Klemkow wrote:
>> Hi,
>>
>> I had the same issue a few days ago a server hardware of mine. I just
>> ran 'cvs up'. So, it looks like a generic bug in FFS and not related to
>> vmm.
>
> This panic generally relates to filesystem corruption. If fsck doesn't
> help then recreating which filesystem is triggering it is usually needed.
Yeah, once in a while we see reports of it. It seems to be some nasty
conspiracy between the generic filesystem code, ffs and fsck_ffs.
Maybe even the device (driver) itself is involved. A possible
underlying issue may be that some operation are re-ordered while they
should not.
>>>
>>> Yes, it does hint at a reordering.
>>>
Now the strange thing is, fsck_ffs *should* be able to repair the
inconsistency, but it appears in some cases it is not, and some bits
on the disk remain to trigger it again.
>>>
>>> fsck_ffs can only repair one inconsistancy. There are a number of lockstep
>>> operations, I suppose we can call them acid-in-lowercase, which allow fsck
>>> to determine at which point the crashed system gave up the ghost. fsck then
>>> removes the partial operations, leaving a viable filesystem. But if the
>>> disk
>>> layer lands later writes but not earlier writes, fsck cannot handle it.
>>
>> I managed to re-create the issue.
>>
>> Created a fresh install qcow2 image and derived 35 new VMs from it.
>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting 240
>> seconds after each cycle.
>> Similar to the staggered start based on the amount of CPUs.
>>
>> This time it was “only” one VM that was affected by this. VM four that got
>> started.
>>
>> ddb> show panic
>> ffs_valloc: dup alloc
>> ddb> trace
>> db_enter() at db_enter+0x10
>> panic(81dc21b2) at panic+0x12a
>> ffs_inode_alloc(fd803c94ef00,81a4,fd803f7bbf00,800014d728b8) at
>> ffs
>> _inode_alloc+0x442
>> ufs_makeinode(81a4,fd803c930908,800014d72bb0,800014d72c00) at
>> ufs_m
>> akeinode+0x7f
>> ufs_create(800014d72960) at ufs_create+0x3c
>> VOP_CREATE(fd803c930908,800014d72bb0,800014d72c00,800014d729c0)
>> at VOP_CREATE+0x4a
>> vn_open(800014d72b80,602,1a4) at vn_open+0x182
>> doopenat(8000c778,ff9c,f8fc28f00f4,601,1b6,800014d72d80) at
>> doo
>> penat+0x1d0
>> syscall(800014d72df0) at syscall+0x315
>> Xsyscall() at Xsyscall+0x128
>> end of kernel
>> end trace frame: 0x7f7be450, count: -10
>>
>> dmesg of the host below.
>
> For me this is not enough info to even try to reproduce, I know little
> of vmm or vmd and have no idea what "derive" means in this context.
>
> Would it be possiblet for you to show the exact steps (preferably a
> script) to reproduce the issue?
Hopefully the below helps.
If you do have vmd running create a VM (qcow2 format) with the normal
installation process.
The base image I created with: vmctl create -s 50G /var/vmm/vm01.qcow2
I have dhcp setup so all the subsequent images will be able to pickup a
different IP address.
Once that is done replicate the vm.conf config for all the other VMs.
The config I used for the VMs is something like:
vm "vm01" {
disable
owner runbsd
memory 1G
disk "/var/vmm/vm01.qcow2" format qcow2
interface tap {
switch "uplink_veb911"
lladdr fe:e1:bb:d4:d4:01
}
}
I replicate them by running something like:
for i in $(jot 39 2); do vmctl create -b /var/vmm/vm01.qcow2
/var/vmm/vm${i}.qcow2; done
This will use vm01.qcow2 image as the base and create a derived image from it.
Which means only changes will be applied to the new image.
I start them with the following script:
#!/bin/sh
SLEEP=240
CPU=$(($(sysctl -n hw.ncpuonline)-2))
COUNTER=0
for i in $(vmctl show | sort | awk '/ - / {print $9}' | xargs); do
VMS[${COUNTER}]=${i}
COUNTER=$((${COUNTER}+1))
done
CYCLES=$((${#VMS[*]}/${CPU}+1))
echo "Starting ${#VMS[*]} VMs on ${CPU} CPUs in ${CYCLES} cycle(s), waiting
${SLEEP} seconds after each cycle."
COUNTER=0
for i in ${VMS[*]}; do
COUNTER=$((${COUNTER}+1))
vmctl start ${i}
if [ $COUNTER -eq $CPU ]; then
sleep ${SLEEP}
COUNTER=0
fi
done
This to make sure they are “settled” and all processes are properly started
before starting the next batch of VMs.
> Though the specific hardware might play a role as well…
I can also provide you access to the host itself.
Mischa
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>
>
> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
> >
> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
> >>> 240 seconds after each cycle.
> >>> Similar to the staggered start based on the amount of CPUs.
> >
> >> For me this is not enough info to even try to reproduce, I know little
> >> of vmm or vmd and have no idea what "derive" means in this context.
> >
> > This is a big bit of information that was missing from the original
>
> Well.. could have been better described indeed. :))
> " I created 41 additional VMs based on a single qcow2 base image.”
>
> > report ;) qcow has a concept of a read-only base image (or 'backing
> > file') which can be shared between VMs, with writes diverted to a
> > separate image ('derived image').
> >
> > So e.g. you can create a base image, do a simple OS install for a
> > particular OS version to that base image, then you stop using that
> > for a VM and just use it as a base to create derived images from.
> > You then run VMs using the derived image and make whatever config
> > changes. If you have a bunch of VMs using the same OS release then
> > you save some disk space for the common files.
> >
> > Mischa did you leave a VM running which is working on the base
> > image directly? That would certainly cause problems.
>
> I did indeed. Let me try that again without keeping the base image running.
Right. As a safeguard, I would change the base image to be r/o.
I was just looking at your script and scratching my head: why is Mischa
starting vm01 ...
-Otto
>
> Mischa
>
> >
> >
> >> Would it be possiblet for you to show the exact steps (preferably a
> >> script) to reproduce the issue?
> >>
> >> Though the specific hardware might play a role as well...
> >>
> >>-Otto
> >>>
> >>> Mischa
> >>>
> >>> OpenBSD 6.9-beta (GENERIC.MP) #421: Sun Mar 21 13:17:22 MDT 2021
> >>>dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> >>> real mem = 137374924800 (131010MB)
> >>> avail mem = 133196165120 (127025MB)
> >>> random: good seed from bootblocks
> >>> mpath0 at root
> >>> scsibus0 at mpath0: 256 targets
> >>> mainbus0 at root
> >>> bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbf42c000 (99 entries)
> >>> bios0: vendor Dell Inc. version "2.8.0" date 06/26/2019
> >>> bios0: Dell Inc. PowerEdge R620
> >>> acpi0 at bios0: ACPI 3.0
> >>> acpi0: sleep states S0 S4 S5
> >>> acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WD__ SLIC ERST HEST BERT
> >>> EINJ TCPA PC__ SRAT SSDT
> >>> acpi0: wakeup devices PCI0(S5) PCI1(S5)
> >>> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> >>> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> >>> cpu0 at mainbus0: apid 0 (boot processor)
> >>> cpu0: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.34 MHz, 06-2d-07
> >>> cpu0:
> >>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> >>> cpu0: 256KB 64b/line 8-way L2 cache
> >>> cpu0: smt 0, core 0, package 0
> >>> mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
> >>> cpu0: apic clock running at 99MHz
> >>> cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
> >>> cpu1 at mainbus0: apid 32 (application processor)
> >>> cpu1: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 1200.02 MHz, 06-2d-07
> >>> cpu1:
> >>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> >>> cpu1: 256KB 64b/line 8-way L2 cache
> >>> cpu1: smt 0, core 0, package 1
> >>> cpu2 at mainbus0: apid 2 (application processor)
> >>> cpu2: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
> >>> cpu2:
> >>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> >>> cpu2: 256KB 64b/line 8-way L2 cache
> >>> cpu2: smt 0, core 1, package 0
> >>> cpu3 at mainbus0: apid 34 (application processor)
> >>> cpu3: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
> >>> cpu3:
> >>>
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
>
> Otto Moerbeek writes:
>
> > On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
> >
> >>
> >>
> >> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
> >> >
> >> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
> >> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and
> >> >>> waiting 240 seconds after each cycle.
> >> >>> Similar to the staggered start based on the amount of CPUs.
> >> >
> >> >> For me this is not enough info to even try to reproduce, I know little
> >> >> of vmm or vmd and have no idea what "derive" means in this context.
> >> >
> >> > This is a big bit of information that was missing from the original
> >>
> >> Well.. could have been better described indeed. :))
> >> " I created 41 additional VMs based on a single qcow2 base image.”
> >>
> >> > report ;) qcow has a concept of a read-only base image (or 'backing
> >> > file') which can be shared between VMs, with writes diverted to a
> >> > separate image ('derived image').
> >> >
> >> > So e.g. you can create a base image, do a simple OS install for a
> >> > particular OS version to that base image, then you stop using that
> >> > for a VM and just use it as a base to create derived images from.
> >> > You then run VMs using the derived image and make whatever config
> >> > changes. If you have a bunch of VMs using the same OS release then
> >> > you save some disk space for the common files.
> >> >
> >> > Mischa did you leave a VM running which is working on the base
> >> > image directly? That would certainly cause problems.
> >>
> >> I did indeed. Let me try that again without keeping the base image running.
> >
> > Right. As a safeguard, I would change the base image to be r/o.
>
> vmd(8) should treating it r/o...the config process is responsible for
> opening the disk files and passing the fd's to the vm process. In
> config.c, the call to open(2) for the base images should be using the
> flags O_RDONLY | O_NONBLOCK.
>
> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
> disk image I based off the "alpine.qcow2" image:
>
> 20862 vmd CALL open(0x7f7d4370,0x26)
> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
> 20862 vmd RET open 10/0xa
> 20862 vmd CALL fstat(10,0x7f7d42b8)
> 20862 vmd STRU struct stat { dev=1051, ino=19531847, mode=-rw--- ,
> nlink=1, uid=1000<"dave">, gid=1000<"dave">, rdev=78096304,
> atime=1616420730<"Mar 22 09:45:30 2021">.509011764, mtime=1616420697<"Mar 22
> 09:44:57 2021">.189185158, ctime=1616420697<"Mar 22 09:44:57
> 2021">.189185158, size=262144, blocks=256, blksize=32768, flags=0x0,
> gen=0xb64d5d98 }
> 20862 vmd RET fstat 0
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
> 20862 vmd GIO fd 10 read 104 bytes
>"QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
> \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
> \0\0h"
> 20862 vmd RET pread 104/0x68
> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
> 20862 vmd GIO fd 10 read 12 bytes
>"alpine.qcow2"
> 20862 vmd RET pread 12/0xc
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd RET __realpath 0
> 20862 vmd CALL open(0x7f7d4370,0x4)
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd RET open 11/0xb
> 20862 vmd CALL fstat(11,0x7f7d42b8)
>
>
> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
> don't think the issue is the base image being r/w.
>
> -Dave
AFAIKS, the issue is that if you start a vm modifying the base because it
uses it as a regular image, that r/o open for the other vms does not
matter a lot,
-OPtto
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>
>>> Created a fresh install qcow2 image and derived 35 new VMs from it.
>>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting 240
>>> seconds after each cycle.
>>> Similar to the staggered start based on the amount of CPUs.
>
>> For me this is not enough info to even try to reproduce, I know little
>> of vmm or vmd and have no idea what "derive" means in this context.
>
> This is a big bit of information that was missing from the original
Well.. could have been better described indeed. :))
" I created 41 additional VMs based on a single qcow2 base image.”
> report ;) qcow has a concept of a read-only base image (or 'backing
> file') which can be shared between VMs, with writes diverted to a
> separate image ('derived image').
>
> So e.g. you can create a base image, do a simple OS install for a
> particular OS version to that base image, then you stop using that
> for a VM and just use it as a base to create derived images from.
> You then run VMs using the derived image and make whatever config
> changes. If you have a bunch of VMs using the same OS release then
> you save some disk space for the common files.
>
> Mischa did you leave a VM running which is working on the base
> image directly? That would certainly cause problems.
I did indeed. Let me try that again without keeping the base image running.
Mischa
>
>
>> Would it be possiblet for you to show the exact steps (preferably a
>> script) to reproduce the issue?
>>
>> Though the specific hardware might play a role as well...
>>
>> -Otto
>>>
>>> Mischa
>>>
>>> OpenBSD 6.9-beta (GENERIC.MP) #421: Sun Mar 21 13:17:22 MDT 2021
>>>dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>>> real mem = 137374924800 (131010MB)
>>> avail mem = 133196165120 (127025MB)
>>> random: good seed from bootblocks
>>> mpath0 at root
>>> scsibus0 at mpath0: 256 targets
>>> mainbus0 at root
>>> bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xbf42c000 (99 entries)
>>> bios0: vendor Dell Inc. version "2.8.0" date 06/26/2019
>>> bios0: Dell Inc. PowerEdge R620
>>> acpi0 at bios0: ACPI 3.0
>>> acpi0: sleep states S0 S4 S5
>>> acpi0: tables DSDT FACP APIC SPCR HPET DMAR MCFG WD__ SLIC ERST HEST BERT
>>> EINJ TCPA PC__ SRAT SSDT
>>> acpi0: wakeup devices PCI0(S5) PCI1(S5)
>>> acpitimer0 at acpi0: 3579545 Hz, 24 bits
>>> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
>>> cpu0 at mainbus0: apid 0 (boot processor)
>>> cpu0: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.34 MHz, 06-2d-07
>>> cpu0:
>>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
>>> cpu0: 256KB 64b/line 8-way L2 cache
>>> cpu0: smt 0, core 0, package 0
>>> mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
>>> cpu0: apic clock running at 99MHz
>>> cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
>>> cpu1 at mainbus0: apid 32 (application processor)
>>> cpu1: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 1200.02 MHz, 06-2d-07
>>> cpu1:
>>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
>>> cpu1: 256KB 64b/line 8-way L2 cache
>>> cpu1: smt 0, core 0, package 1
>>> cpu2 at mainbus0: apid 2 (application processor)
>>> cpu2: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
>>> cpu2:
>>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
>>> cpu2: 256KB 64b/line 8-way L2 cache
>>> cpu2: smt 0, core 1, package 0
>>> cpu3 at mainbus0: apid 34 (application processor)
>>> cpu3: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz, 2600.03 MHz, 06-2d-07
>>> cpu3:
>>> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,PAGE1GB,RDTSCP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
>>> cpu3: 256KB 64b/line 8-way L2 cache
>>> cpu3: smt 0, core 1, package 1
>>> cpu4 at mainbus0: apid 4 (application processor)
>>> cpu4: Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz,
Re: vmm crash on 6.9-beta
Otto Moerbeek writes:
> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>
>>
>>
>> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>> >
>> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
>> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and waiting
>> >>> 240 seconds after each cycle.
>> >>> Similar to the staggered start based on the amount of CPUs.
>> >
>> >> For me this is not enough info to even try to reproduce, I know little
>> >> of vmm or vmd and have no idea what "derive" means in this context.
>> >
>> > This is a big bit of information that was missing from the original
>>
>> Well.. could have been better described indeed. :))
>> " I created 41 additional VMs based on a single qcow2 base image.”
>>
>> > report ;) qcow has a concept of a read-only base image (or 'backing
>> > file') which can be shared between VMs, with writes diverted to a
>> > separate image ('derived image').
>> >
>> > So e.g. you can create a base image, do a simple OS install for a
>> > particular OS version to that base image, then you stop using that
>> > for a VM and just use it as a base to create derived images from.
>> > You then run VMs using the derived image and make whatever config
>> > changes. If you have a bunch of VMs using the same OS release then
>> > you save some disk space for the common files.
>> >
>> > Mischa did you leave a VM running which is working on the base
>> > image directly? That would certainly cause problems.
>>
>> I did indeed. Let me try that again without keeping the base image running.
>
> Right. As a safeguard, I would change the base image to be r/o.
vmd(8) should treating it r/o...the config process is responsible for
opening the disk files and passing the fd's to the vm process. In
config.c, the call to open(2) for the base images should be using the
flags O_RDONLY | O_NONBLOCK.
A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
disk image I based off the "alpine.qcow2" image:
20862 vmd CALL open(0x7f7d4370,0x26)
20862 vmd NAMI "/home/dave/vm/new.qcow2"
20862 vmd RET open 10/0xa
20862 vmd CALL fstat(10,0x7f7d42b8)
20862 vmd STRU struct stat { dev=1051, ino=19531847, mode=-rw--- ,
nlink=1, uid=1000<"dave">, gid=1000<"dave">, rdev=78096304,
atime=1616420730<"Mar 22 09:45:30 2021">.509011764, mtime=1616420697<"Mar 22
09:44:57 2021">.189185158, ctime=1616420697<"Mar 22 09:44:57 2021">.189185158,
size=262144, blocks=256, blksize=32768, flags=0x0, gen=0xb64d5d98 }
20862 vmd RET fstat 0
20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
20862 vmd RET kbind 0
20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
20862 vmd GIO fd 10 read 104 bytes
"QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
\0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
\0\0h"
20862 vmd RET pread 104/0x68
20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
20862 vmd GIO fd 10 read 12 bytes
"alpine.qcow2"
20862 vmd RET pread 12/0xc
20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
20862 vmd RET kbind 0
20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
20862 vmd RET kbind 0
20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
20862 vmd RET __realpath 0
20862 vmd CALL open(0x7f7d4370,0x4)
20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
20862 vmd RET open 11/0xb
20862 vmd CALL fstat(11,0x7f7d42b8)
I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
don't think the issue is the base image being r/w.
-Dave
Re: vmm crash on 6.9-beta
Otto Moerbeek writes:
> On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
>
>>
>> Otto Moerbeek writes:
>>
>> > On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>> >
>> >>
>> >>
>> >> > On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>> >> >
>> >> >>> Created a fresh install qcow2 image and derived 35 new VMs from it.
>> >> >>> Then I started all the VMs in four cycles, 10 VMs per cycle and
>> >> >>> waiting 240 seconds after each cycle.
>> >> >>> Similar to the staggered start based on the amount of CPUs.
>> >> >
>> >> >> For me this is not enough info to even try to reproduce, I know little
>> >> >> of vmm or vmd and have no idea what "derive" means in this context.
>> >> >
>> >> > This is a big bit of information that was missing from the original
>> >>
>> >> Well.. could have been better described indeed. :))
>> >> " I created 41 additional VMs based on a single qcow2 base image.”
>> >>
>> >> > report ;) qcow has a concept of a read-only base image (or 'backing
>> >> > file') which can be shared between VMs, with writes diverted to a
>> >> > separate image ('derived image').
>> >> >
>> >> > So e.g. you can create a base image, do a simple OS install for a
>> >> > particular OS version to that base image, then you stop using that
>> >> > for a VM and just use it as a base to create derived images from.
>> >> > You then run VMs using the derived image and make whatever config
>> >> > changes. If you have a bunch of VMs using the same OS release then
>> >> > you save some disk space for the common files.
>> >> >
>> >> > Mischa did you leave a VM running which is working on the base
>> >> > image directly? That would certainly cause problems.
>> >>
>> >> I did indeed. Let me try that again without keeping the base image
>> >> running.
>> >
>> > Right. As a safeguard, I would change the base image to be r/o.
>>
>> vmd(8) should treating it r/o...the config process is responsible for
>> opening the disk files and passing the fd's to the vm process. In
>> config.c, the call to open(2) for the base images should be using the
>> flags O_RDONLY | O_NONBLOCK.
>>
>> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
>> disk image I based off the "alpine.qcow2" image:
>>
>> 20862 vmd CALL open(0x7f7d4370,0x26)
>> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
>> 20862 vmd RET open 10/0xa
>> 20862 vmd CALL fstat(10,0x7f7d42b8)
>> 20862 vmd STRU struct stat { dev=1051, ino=19531847, mode=-rw---
>> , nlink=1, uid=1000<"dave">, gid=1000<"dave">, rdev=78096304,
>> atime=1616420730<"Mar 22 09:45:30 2021">.509011764, mtime=1616420697<"Mar 22
>> 09:44:57 2021">.189185158, ctime=1616420697<"Mar 22 09:44:57
>> 2021">.189185158, size=262144, blocks=256, blksize=32768, flags=0x0,
>> gen=0xb64d5d98 }
>> 20862 vmd RET fstat 0
>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>> 20862 vmd RET kbind 0
>> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
>> 20862 vmd GIO fd 10 read 104 bytes
>>
>> "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
>> \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
>>
>> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
>> \0\0h"
>> 20862 vmd RET pread 104/0x68
>> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
>> 20862 vmd GIO fd 10 read 12 bytes
>>"alpine.qcow2"
>> 20862 vmd RET pread 12/0xc
>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>> 20862 vmd RET kbind 0
>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>> 20862 vmd RET kbind 0
>> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>> 20862 vmd RET __realpath 0
>> 20862 vmd CALL open(0x7f7d4370,0x4)
>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>> 20862 vmd RET open 11/0xb
>> 20862 vmd CALL fstat(11,0x7f7d42b8)
>>
>>
>> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
>> don't think the issue is the base image being r/w.
>>
>> -Dave
>
> AFAIKS, the issue is that if you start a vm modifying the base because it
> uses it as a regular image, that r/o open for the other vms does not
> matter a lot,
>
> -OPtto
Good point. I'm going to look into the feasibility of having the
control[1] process track what disks it's opened and in what mode to see
if there's a way to build in some protection against this from
happening.
[1] I mistakenly called it the "config" process earlier.
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 03:06:40PM +0100, Mischa wrote:
> > On 22 Mar 2021, at 15:05, Dave Voutila wrote:
> > Otto Moerbeek writes:
> >> On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
> >>> Otto Moerbeek writes:
> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
> >> On 22 Mar 2021, at 13:43, Stuart Henderson
> >> wrote:
> >>
> Created a fresh install qcow2 image and derived 35 new VMs from it.
> Then I started all the VMs in four cycles, 10 VMs per cycle and
> waiting 240 seconds after each cycle.
> Similar to the staggered start based on the amount of CPUs.
> >>
> >>> For me this is not enough info to even try to reproduce, I know little
> >>> of vmm or vmd and have no idea what "derive" means in this context.
> >>
> >> This is a big bit of information that was missing from the original
> >
> > Well.. could have been better described indeed. :))
> > " I created 41 additional VMs based on a single qcow2 base image.”
> >
> >> report ;) qcow has a concept of a read-only base image (or 'backing
> >> file') which can be shared between VMs, with writes diverted to a
> >> separate image ('derived image').
> >>
> >> So e.g. you can create a base image, do a simple OS install for a
> >> particular OS version to that base image, then you stop using that
> >> for a VM and just use it as a base to create derived images from.
> >> You then run VMs using the derived image and make whatever config
> >> changes. If you have a bunch of VMs using the same OS release then
> >> you save some disk space for the common files.
> >>
> >> Mischa did you leave a VM running which is working on the base
> >> image directly? That would certainly cause problems.
> >
> > I did indeed. Let me try that again without keeping the base image
> > running.
>
> Right. As a safeguard, I would change the base image to be r/o.
> >>>
> >>> vmd(8) should treating it r/o...the config process is responsible for
> >>> opening the disk files and passing the fd's to the vm process. In
> >>> config.c, the call to open(2) for the base images should be using the
> >>> flags O_RDONLY | O_NONBLOCK.
> >>>
> >>> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
> >>> disk image I based off the "alpine.qcow2" image:
> >>>
> >>> 20862 vmd CALL open(0x7f7d4370,0x26)
> >>> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
> >>> 20862 vmd RET open 10/0xa
> >>> 20862 vmd CALL fstat(10,0x7f7d42b8)
> >>> 20862 vmd STRU struct stat { dev=1051, ino=19531847,
> >>> mode=-rw--- , nlink=1, uid=1000<"dave">, gid=1000<"dave">,
> >>> rdev=78096304, atime=1616420730<"Mar 22 09:45:30 2021">.509011764,
> >>> mtime=1616420697<"Mar 22 09:44:57 2021">.189185158, ctime=1616420697<"Mar
> >>> 22 09:44:57 2021">.189185158, size=262144, blocks=256, blksize=32768,
> >>> flags=0x0, gen=0xb64d5d98 }
> >>> 20862 vmd RET fstat 0
> >>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> >>> 20862 vmd RET kbind 0
> >>> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
> >>> 20862 vmd GIO fd 10 read 104 bytes
> >>>
> >>> "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
> >>>
> >>> \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
> >>>
> >>> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
> >>>\0\0h"
> >>> 20862 vmd RET pread 104/0x68
> >>> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
> >>> 20862 vmd GIO fd 10 read 12 bytes
> >>> "alpine.qcow2"
> >>> 20862 vmd RET pread 12/0xc
> >>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> >>> 20862 vmd RET kbind 0
> >>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> >>> 20862 vmd RET kbind 0
> >>> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
> >>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> >>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> >>> 20862 vmd RET __realpath 0
> >>> 20862 vmd CALL open(0x7f7d4370,0x4)
> >>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> >>> 20862 vmd RET open 11/0xb
> >>> 20862 vmd CALL fstat(11,0x7f7d42b8)
> >>>
> >>>
> >>> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
> >>> don't think the issue is the base image being r/w.
> >>>
> >>> -Dave
> >>
> >> AFAIKS, the issue is that if you start a vm modifying the base because it
> >> uses it as a regular image, that r/o open for the other vms does not
> >> matter a lot,
> >>
> >>-OPtto
> >
> > Good point. I'm going to look into the feasibility of having the
> > control[1] process track what disks it's opened and in what mode to see
> > if there's a way to build in some protection against this
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 03:20:37PM +0100, Mischa wrote:
>
>
> > On 22 Mar 2021, at 15:18, Otto Moerbeek wrote:
> >
> > On Mon, Mar 22, 2021 at 03:06:40PM +0100, Mischa wrote:
> >
> >>> On 22 Mar 2021, at 15:05, Dave Voutila wrote:
> >>> Otto Moerbeek writes:
> On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
> > Otto Moerbeek writes:
> >> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
> On 22 Mar 2021, at 13:43, Stuart Henderson
> wrote:
>
> >> Created a fresh install qcow2 image and derived 35 new VMs from it.
> >> Then I started all the VMs in four cycles, 10 VMs per cycle and
> >> waiting 240 seconds after each cycle.
> >> Similar to the staggered start based on the amount of CPUs.
>
> > For me this is not enough info to even try to reproduce, I know
> > little
> > of vmm or vmd and have no idea what "derive" means in this context.
>
> This is a big bit of information that was missing from the original
> >>>
> >>> Well.. could have been better described indeed. :))
> >>> " I created 41 additional VMs based on a single qcow2 base image.”
> >>>
> report ;) qcow has a concept of a read-only base image (or 'backing
> file') which can be shared between VMs, with writes diverted to a
> separate image ('derived image').
>
> So e.g. you can create a base image, do a simple OS install for a
> particular OS version to that base image, then you stop using that
> for a VM and just use it as a base to create derived images from.
> You then run VMs using the derived image and make whatever config
> changes. If you have a bunch of VMs using the same OS release then
> you save some disk space for the common files.
>
> Mischa did you leave a VM running which is working on the base
> image directly? That would certainly cause problems.
> >>>
> >>> I did indeed. Let me try that again without keeping the base image
> >>> running.
> >>
> >> Right. As a safeguard, I would change the base image to be r/o.
> >
> > vmd(8) should treating it r/o...the config process is responsible for
> > opening the disk files and passing the fd's to the vm process. In
> > config.c, the call to open(2) for the base images should be using the
> > flags O_RDONLY | O_NONBLOCK.
> >
> > A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
> > disk image I based off the "alpine.qcow2" image:
> >
> > 20862 vmd CALL
> > open(0x7f7d4370,0x26)
> > 20862 vmd NAMI "/home/dave/vm/new.qcow2"
> > 20862 vmd RET open 10/0xa
> > 20862 vmd CALL fstat(10,0x7f7d42b8)
> > 20862 vmd STRU struct stat { dev=1051, ino=19531847,
> > mode=-rw--- , nlink=1, uid=1000<"dave">, gid=1000<"dave">,
> > rdev=78096304, atime=1616420730<"Mar 22 09:45:30 2021">.509011764,
> > mtime=1616420697<"Mar 22 09:44:57 2021">.189185158,
> > ctime=1616420697<"Mar 22 09:44:57 2021">.189185158, size=262144,
> > blocks=256, blksize=32768, flags=0x0, gen=0xb64d5d98 }
> > 20862 vmd RET fstat 0
> > 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> > 20862 vmd RET kbind 0
> > 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
> > 20862 vmd GIO fd 10 read 104 bytes
> >
> > "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
> >
> > \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
> >
> > \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
> > \0\0h"
> > 20862 vmd RET pread 104/0x68
> > 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
> > 20862 vmd GIO fd 10 read 12 bytes
> > "alpine.qcow2"
> > 20862 vmd RET pread 12/0xc
> > 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> > 20862 vmd RET kbind 0
> > 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> > 20862 vmd RET kbind 0
> > 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
> > 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> > 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> > 20862 vmd RET __realpath 0
> > 20862 vmd CALL open(0x7f7d4370,0x4)
> > 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> > 20862 vmd RET open 11/0xb
> > 20862 vmd CALL fstat(11,0x7f7d42b8)
> >
> >
> > I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
> > don't think the issue is the base image being r/w.
> >
> > -Dave
>
> AFAIKS, the issue is that if you start a vm modifying the base
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 15:05, Dave Voutila wrote:
> Otto Moerbeek writes:
>> On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
>>> Otto Moerbeek writes:
On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>> On 22 Mar 2021, at 13:43, Stuart Henderson wrote:
>>
Created a fresh install qcow2 image and derived 35 new VMs from it.
Then I started all the VMs in four cycles, 10 VMs per cycle and
waiting 240 seconds after each cycle.
Similar to the staggered start based on the amount of CPUs.
>>
>>> For me this is not enough info to even try to reproduce, I know little
>>> of vmm or vmd and have no idea what "derive" means in this context.
>>
>> This is a big bit of information that was missing from the original
>
> Well.. could have been better described indeed. :))
> " I created 41 additional VMs based on a single qcow2 base image.”
>
>> report ;) qcow has a concept of a read-only base image (or 'backing
>> file') which can be shared between VMs, with writes diverted to a
>> separate image ('derived image').
>>
>> So e.g. you can create a base image, do a simple OS install for a
>> particular OS version to that base image, then you stop using that
>> for a VM and just use it as a base to create derived images from.
>> You then run VMs using the derived image and make whatever config
>> changes. If you have a bunch of VMs using the same OS release then
>> you save some disk space for the common files.
>>
>> Mischa did you leave a VM running which is working on the base
>> image directly? That would certainly cause problems.
>
> I did indeed. Let me try that again without keeping the base image
> running.
Right. As a safeguard, I would change the base image to be r/o.
>>>
>>> vmd(8) should treating it r/o...the config process is responsible for
>>> opening the disk files and passing the fd's to the vm process. In
>>> config.c, the call to open(2) for the base images should be using the
>>> flags O_RDONLY | O_NONBLOCK.
>>>
>>> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
>>> disk image I based off the "alpine.qcow2" image:
>>>
>>> 20862 vmd CALL open(0x7f7d4370,0x26)
>>> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
>>> 20862 vmd RET open 10/0xa
>>> 20862 vmd CALL fstat(10,0x7f7d42b8)
>>> 20862 vmd STRU struct stat { dev=1051, ino=19531847, mode=-rw---
>>> , nlink=1, uid=1000<"dave">, gid=1000<"dave">, rdev=78096304,
>>> atime=1616420730<"Mar 22 09:45:30 2021">.509011764, mtime=1616420697<"Mar
>>> 22 09:44:57 2021">.189185158, ctime=1616420697<"Mar 22 09:44:57
>>> 2021">.189185158, size=262144, blocks=256, blksize=32768, flags=0x0,
>>> gen=0xb64d5d98 }
>>> 20862 vmd RET fstat 0
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
>>> 20862 vmd GIO fd 10 read 104 bytes
>>>
>>> "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
>>>\0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
>>>
>>> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
>>>\0\0h"
>>> 20862 vmd RET pread 104/0x68
>>> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
>>> 20862 vmd GIO fd 10 read 12 bytes
>>> "alpine.qcow2"
>>> 20862 vmd RET pread 12/0xc
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd RET __realpath 0
>>> 20862 vmd CALL open(0x7f7d4370,0x4)
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd RET open 11/0xb
>>> 20862 vmd CALL fstat(11,0x7f7d42b8)
>>>
>>>
>>> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
>>> don't think the issue is the base image being r/w.
>>>
>>> -Dave
>>
>> AFAIKS, the issue is that if you start a vm modifying the base because it
>> uses it as a regular image, that r/o open for the other vms does not
>> matter a lot,
>>
>> -OPtto
>
> Good point. I'm going to look into the feasibility of having the
> control[1] process track what disks it's opened and in what mode to see
> if there's a way to build in some protection against this from
> happening.
>
> [1] I mistakenly called it the "config" process earlier.
I guess that would help a lot of poor souls like myself to not make that
mistake again. :)
Mischa
Re: vmm crash on 6.9-beta
On Mon, Mar 22, 2021 at 01:59:17PM +, Stuart Henderson wrote: > > > I'm more familiar with the vmd(8) codebase than any ffs stuff, but I > > > don't think the issue is the base image being r/w. > > > > AFAIKS, the issue is that if you start a vm modifying the base because it > > uses it as a regular image, that r/o open for the other vms does not > > matter a lot, > > vmd could possibly refuse to use an image as a base for a derived image > if it already has the base open (or vice-versa), but then some other > software (e.g. qemu, qemu-img) could modify the base image too, there > are always ways to break things not matter what the safeguards. > Hence my safeguard at the OS level: chmod it... -Otto
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 15:18, Otto Moerbeek wrote:
>
> On Mon, Mar 22, 2021 at 03:06:40PM +0100, Mischa wrote:
>
>>> On 22 Mar 2021, at 15:05, Dave Voutila wrote:
>>> Otto Moerbeek writes:
On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
> Otto Moerbeek writes:
>> On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
On 22 Mar 2021, at 13:43, Stuart Henderson
wrote:
>> Created a fresh install qcow2 image and derived 35 new VMs from it.
>> Then I started all the VMs in four cycles, 10 VMs per cycle and
>> waiting 240 seconds after each cycle.
>> Similar to the staggered start based on the amount of CPUs.
> For me this is not enough info to even try to reproduce, I know little
> of vmm or vmd and have no idea what "derive" means in this context.
This is a big bit of information that was missing from the original
>>>
>>> Well.. could have been better described indeed. :))
>>> " I created 41 additional VMs based on a single qcow2 base image.”
>>>
report ;) qcow has a concept of a read-only base image (or 'backing
file') which can be shared between VMs, with writes diverted to a
separate image ('derived image').
So e.g. you can create a base image, do a simple OS install for a
particular OS version to that base image, then you stop using that
for a VM and just use it as a base to create derived images from.
You then run VMs using the derived image and make whatever config
changes. If you have a bunch of VMs using the same OS release then
you save some disk space for the common files.
Mischa did you leave a VM running which is working on the base
image directly? That would certainly cause problems.
>>>
>>> I did indeed. Let me try that again without keeping the base image
>>> running.
>>
>> Right. As a safeguard, I would change the base image to be r/o.
>
> vmd(8) should treating it r/o...the config process is responsible for
> opening the disk files and passing the fd's to the vm process. In
> config.c, the call to open(2) for the base images should be using the
> flags O_RDONLY | O_NONBLOCK.
>
> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
> disk image I based off the "alpine.qcow2" image:
>
> 20862 vmd CALL open(0x7f7d4370,0x26)
> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
> 20862 vmd RET open 10/0xa
> 20862 vmd CALL fstat(10,0x7f7d42b8)
> 20862 vmd STRU struct stat { dev=1051, ino=19531847,
> mode=-rw--- , nlink=1, uid=1000<"dave">, gid=1000<"dave">,
> rdev=78096304, atime=1616420730<"Mar 22 09:45:30 2021">.509011764,
> mtime=1616420697<"Mar 22 09:44:57 2021">.189185158, ctime=1616420697<"Mar
> 22 09:44:57 2021">.189185158, size=262144, blocks=256, blksize=32768,
> flags=0x0, gen=0xb64d5d98 }
> 20862 vmd RET fstat 0
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
> 20862 vmd GIO fd 10 read 104 bytes
>
> "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
>
> \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
>
> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
> \0\0h"
> 20862 vmd RET pread 104/0x68
> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
> 20862 vmd GIO fd 10 read 12 bytes
> "alpine.qcow2"
> 20862 vmd RET pread 12/0xc
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
> 20862 vmd RET kbind 0
> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd RET __realpath 0
> 20862 vmd CALL open(0x7f7d4370,0x4)
> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
> 20862 vmd RET open 11/0xb
> 20862 vmd CALL fstat(11,0x7f7d42b8)
>
>
> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
> don't think the issue is the base image being r/w.
>
> -Dave
AFAIKS, the issue is that if you start a vm modifying the base because it
uses it as a regular image, that r/o open for the other vms does not
matter a lot,
-OPtto
>>>
>>> Good point. I'm going to look into the feasibility of having the
>>> control[1] process track what disks it's opened and in what mode to see
>>> if
Re: httpd.conf grammar
On Mon, Mar 22, 2021 at 01:33:34PM +, Raf Czlonka wrote: > On Mon, Mar 22, 2021 at 07:08:32AM GMT, Jason McIntyre wrote: > > On Sun, Mar 21, 2021 at 10:58:02PM +, Raf Czlonka wrote: > > > > > > Hi Laurie, > > > > > > I'd simply use the existing wording, without getting into details. > > > > > > While there, "braces" dominate the manual page, with a single > > > occurrence of "brackets" so let's change that too, for consistency. > > > > > > Regards, > > > > > > Raf > > > > > Hi Jason, > > > hi. i could have sworn i recently discussed this documenting of multiple > > options with someone else, but cannot find the mails. anyway, a couple > > of points: > > > > - "a block of options that is enclosed in curly braces" can be shortened to > > "a block of options enclosed in curly braces". up to you. > > Yup. > > > - if it is generally true that where multiple options can be given they > > are specified in the same way (curly braces) then it would be better > > just to say up front that this is the case, rather than repeating the > > text endlessly everywhere it's possible. > > I did think about it as there's a lot of repetition: > > $ grep '^Multiple options' /usr/share/man/man5/httpd.conf.5 > Multiple options may be specified within curly braces. > Multiple options may be specified within curly braces. > Multiple options may be specified within curly braces. > Multiple options may be specified within curly braces. > Multiple options may be specified within curly braces. > Multiple options may be specified within curly braces. > > but wanted the diff to be as small as possible ;^) > > > in other respects i'm fine with the diff. > > jmc > > How about this? > > Cheers, > > Raf > yes, exactly what i'd hoped. ok by me. jmc > Index: usr.sbin/httpd/httpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v > retrieving revision 1.114 > diff -u -p -r1.114 httpd.conf.5 > --- usr.sbin/httpd/httpd.conf.5 29 Oct 2020 12:30:52 - 1.114 > +++ usr.sbin/httpd/httpd.conf.5 22 Mar 2021 13:24:58 - > @@ -78,6 +78,8 @@ the comment is effective until the end o > Argument names not beginning with a letter, digit, or underscore > must be quoted. > .Pp > +Multiple options may be specified within curly braces. > +.Pp > Additional configuration files can be included with the > .Ic include > keyword, for example: > @@ -155,7 +157,7 @@ see > .Xr patterns 7 . > .El > .Pp > -Followed by a block of options that is enclosed in curly brackets: > +Followed by a block of options enclosed in curly braces: > .Bl -tag -width Ds > .It Ic alias Ar name > Specify an additional alias > @@ -238,7 +240,6 @@ option. > .El > .It Ic connection Ar option > Set the specified options and limits for HTTP connections. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Ic max request body Ar number > @@ -265,7 +266,6 @@ Set the default media type for the speci > overwriting the global setting. > .It Ic directory Ar option > Set the specified options when serving or accessing directories. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Oo Ic no Oc Ic auto index > @@ -451,7 +451,6 @@ but can be changed per server or locatio > Use the > .Ic no log > directive to disable logging of any requests. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Ic access Ar name > @@ -509,7 +508,6 @@ Disable any previous > in a location. > .It Ic request Ar option > Configure the options for the request path. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Oo Ic no Oc Ic rewrite Ar path > @@ -547,7 +545,6 @@ Enable or disable the specified TCP/IP o > and > .Xr ip 4 > for more information about the options. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Ic backlog Ar number > @@ -577,7 +574,6 @@ This will affect the TCP window size. > .It Ic tls Ar option > Set the TLS configuration for the server. > These options are only used if TLS has been enabled via the listen directive. > -Multiple options may be specified within curly braces. > Valid options are: > .Bl -tag -width Ds > .It Ic certificate Ar file >
Re: vmm crash on 6.9-beta
> On 22 Mar 2021, at 15:23, Otto Moerbeek wrote:
> On Mon, Mar 22, 2021 at 03:20:37PM +0100, Mischa wrote:
>>> On 22 Mar 2021, at 15:18, Otto Moerbeek wrote:
>>> On Mon, Mar 22, 2021 at 03:06:40PM +0100, Mischa wrote:
>>>
> On 22 Mar 2021, at 15:05, Dave Voutila wrote:
> Otto Moerbeek writes:
>> On Mon, Mar 22, 2021 at 09:51:19AM -0400, Dave Voutila wrote:
>>> Otto Moerbeek writes:
On Mon, Mar 22, 2021 at 01:47:18PM +0100, Mischa wrote:
>> On 22 Mar 2021, at 13:43, Stuart Henderson
>> wrote:
>>
Created a fresh install qcow2 image and derived 35 new VMs from it.
Then I started all the VMs in four cycles, 10 VMs per cycle and
waiting 240 seconds after each cycle.
Similar to the staggered start based on the amount of CPUs.
>>
>>> For me this is not enough info to even try to reproduce, I know
>>> little
>>> of vmm or vmd and have no idea what "derive" means in this context.
>>
>> This is a big bit of information that was missing from the original
>
> Well.. could have been better described indeed. :))
> " I created 41 additional VMs based on a single qcow2 base image.”
>
>> report ;) qcow has a concept of a read-only base image (or 'backing
>> file') which can be shared between VMs, with writes diverted to a
>> separate image ('derived image').
>>
>> So e.g. you can create a base image, do a simple OS install for a
>> particular OS version to that base image, then you stop using that
>> for a VM and just use it as a base to create derived images from.
>> You then run VMs using the derived image and make whatever config
>> changes. If you have a bunch of VMs using the same OS release then
>> you save some disk space for the common files.
>>
>> Mischa did you leave a VM running which is working on the base
>> image directly? That would certainly cause problems.
>
> I did indeed. Let me try that again without keeping the base image
> running.
Right. As a safeguard, I would change the base image to be r/o.
>>>
>>> vmd(8) should treating it r/o...the config process is responsible for
>>> opening the disk files and passing the fd's to the vm process. In
>>> config.c, the call to open(2) for the base images should be using the
>>> flags O_RDONLY | O_NONBLOCK.
>>>
>>> A ktrace on my system shows that's the case. Below, "new.qcow2" is a new
>>> disk image I based off the "alpine.qcow2" image:
>>>
>>> 20862 vmd CALL
>>> open(0x7f7d4370,0x26)
>>> 20862 vmd NAMI "/home/dave/vm/new.qcow2"
>>> 20862 vmd RET open 10/0xa
>>> 20862 vmd CALL fstat(10,0x7f7d42b8)
>>> 20862 vmd STRU struct stat { dev=1051, ino=19531847,
>>> mode=-rw--- , nlink=1, uid=1000<"dave">, gid=1000<"dave">,
>>> rdev=78096304, atime=1616420730<"Mar 22 09:45:30 2021">.509011764,
>>> mtime=1616420697<"Mar 22 09:44:57 2021">.189185158,
>>> ctime=1616420697<"Mar 22 09:44:57 2021">.189185158, size=262144,
>>> blocks=256, blksize=32768, flags=0x0, gen=0xb64d5d98 }
>>> 20862 vmd RET fstat 0
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL pread(10,0x7f7d42a8,0x68,0)
>>> 20862 vmd GIO fd 10 read 104 bytes
>>>
>>> "QFI\M-{\0\0\0\^C\0\0\0\0\0\0\0h\0\0\0\f\0\0\0\^P\0\0\0\^E\0\0\0\0\0\0\
>>>
>>> \0\0\0\0\0(\0\0\0\0\0\^A\0\0\0\0\0\0\0\^B\0\0\0\0\0\^A\0\0\0\0\0\0\0\
>>>
>>> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^D\0\
>>> \0\0h"
>>> 20862 vmd RET pread 104/0x68
>>> 20862 vmd CALL pread(10,0x7f7d4770,0xc,0x68)
>>> 20862 vmd GIO fd 10 read 12 bytes
>>> "alpine.qcow2"
>>> 20862 vmd RET pread 12/0xc
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL kbind(0x7f7d39d8,24,0x2a9349e63ae9950c)
>>> 20862 vmd RET kbind 0
>>> 20862 vmd CALL __realpath(0x7f7d3ea0,0x7f7d3680)
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd RET __realpath 0
>>> 20862 vmd CALL open(0x7f7d4370,0x4)
>>> 20862 vmd NAMI "/home/dave/vm/alpine.qcow2"
>>> 20862 vmd RET open 11/0xb
>>> 20862 vmd CALL fstat(11,0x7f7d42b8)
>>>
>>>
>>> I'm more familiar with the vmd(8) codebase than any ffs stuff, but I
>>> don't think the issue is the base image being r/w.
>>>
>>> -Dave
>>
>> AFAIKS, the issue is that if
Re: cwfg: update device-tree bindings
On Mon, Mar 22, 2021 at 06:19:14PM +0100, Mark Kettenis wrote:
> > @@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
> > free(batinfo, M_TEMP, len);
> >
> > sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
> > - "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
> > + "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
>
> I think the old property specified the interval in seconds. So you
> should adjust the default value as well. The minimum allowed value is
> 250 so the current 8 makes no sense. You could make it 8000, but
> maybe using 5000 is better as this is whatis used for the pinebook pro.
Right, missed that.
Feedback? OK?
Index: cwfg.c
===
RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
retrieving revision 1.1
diff -u -p -r1.1 cwfg.c
--- cwfg.c 10 Jun 2020 17:51:21 - 1.1
+++ cwfg.c 22 Mar 2021 18:05:35 -
@@ -96,7 +96,7 @@ struct cwfg_softc {
struct ksensordev sc_sensordev;
};
-#defineCWFG_MONITOR_INTERVAL_DEFAULT 8
+#defineCWFG_MONITOR_INTERVAL_DEFAULT 5000
#defineCWFG_DESIGN_CAPACITY_DEFAULT2000
#defineCWFG_ALERT_LEVEL_DEFAULT0
@@ -124,7 +124,7 @@ cwfg_match(struct device *parent, void *
{
struct i2c_attach_args *ia = aux;
- if (strcmp(ia->ia_name, "cellwise,cw201x") == 0)
+ if (strcmp(ia->ia_name, "cellwise,cw2015") == 0)
return 1;
return 0;
@@ -143,14 +143,14 @@ cwfg_attach(struct device *parent, struc
sc->sc_addr = ia->ia_addr;
sc->sc_node = *(int *)ia->ia_cookie;
- len = OF_getproplen(sc->sc_node, "cellwise,bat-config-info");
+ len = OF_getproplen(sc->sc_node, "cellwise,battery-profile");
if (len <= 0) {
printf(": missing or invalid battery info\n");
return;
}
batinfo = malloc(len, M_TEMP, M_WAITOK);
- OF_getprop(sc->sc_node, "cellwise,bat-config-info", batinfo, len);
+ OF_getprop(sc->sc_node, "cellwise,battery-profile", batinfo, len);
switch (len) {
case BATINFO_SIZE:
memcpy(sc->sc_batinfo, batinfo, BATINFO_SIZE);
@@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
free(batinfo, M_TEMP, len);
sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
- "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
+ "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
sc->sc_design_capacity = OF_getpropint(sc->sc_node,
"cellwise,design-capacity", CWFG_DESIGN_CAPACITY_DEFAULT);
sc->sc_alert_level = OF_getpropint(sc->sc_node,
Re: cwfg: update device-tree bindings
> Date: Mon, 22 Mar 2021 19:07:23 +0100
> From: Klemens Nanni
>
> On Mon, Mar 22, 2021 at 06:19:14PM +0100, Mark Kettenis wrote:
> > > @@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
> > > free(batinfo, M_TEMP, len);
> > >
> > > sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
> > > - "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
> > > + "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
> >
> > I think the old property specified the interval in seconds. So you
> > should adjust the default value as well. The minimum allowed value is
> > 250 so the current 8 makes no sense. You could make it 8000, but
> > maybe using 5000 is better as this is whatis used for the pinebook pro.
> Right, missed that.
>
> Feedback? OK?
ok kettenis@
> Index: cwfg.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 cwfg.c
> --- cwfg.c10 Jun 2020 17:51:21 - 1.1
> +++ cwfg.c22 Mar 2021 18:05:35 -
> @@ -96,7 +96,7 @@ struct cwfg_softc {
> struct ksensordev sc_sensordev;
> };
>
> -#define CWFG_MONITOR_INTERVAL_DEFAULT 8
> +#define CWFG_MONITOR_INTERVAL_DEFAULT 5000
> #define CWFG_DESIGN_CAPACITY_DEFAULT2000
> #define CWFG_ALERT_LEVEL_DEFAULT0
>
> @@ -124,7 +124,7 @@ cwfg_match(struct device *parent, void *
> {
> struct i2c_attach_args *ia = aux;
>
> - if (strcmp(ia->ia_name, "cellwise,cw201x") == 0)
> + if (strcmp(ia->ia_name, "cellwise,cw2015") == 0)
> return 1;
>
> return 0;
> @@ -143,14 +143,14 @@ cwfg_attach(struct device *parent, struc
> sc->sc_addr = ia->ia_addr;
> sc->sc_node = *(int *)ia->ia_cookie;
>
> - len = OF_getproplen(sc->sc_node, "cellwise,bat-config-info");
> + len = OF_getproplen(sc->sc_node, "cellwise,battery-profile");
> if (len <= 0) {
> printf(": missing or invalid battery info\n");
> return;
> }
>
> batinfo = malloc(len, M_TEMP, M_WAITOK);
> - OF_getprop(sc->sc_node, "cellwise,bat-config-info", batinfo, len);
> + OF_getprop(sc->sc_node, "cellwise,battery-profile", batinfo, len);
> switch (len) {
> case BATINFO_SIZE:
> memcpy(sc->sc_batinfo, batinfo, BATINFO_SIZE);
> @@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
> free(batinfo, M_TEMP, len);
>
> sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
> - "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
> + "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
> sc->sc_design_capacity = OF_getpropint(sc->sc_node,
> "cellwise,design-capacity", CWFG_DESIGN_CAPACITY_DEFAULT);
> sc->sc_alert_level = OF_getpropint(sc->sc_node,
>
Re: cwfg: update device-tree bindings
On Mon, Mar 22, 2021 at 05:48:56PM +0100, Klemens Nanni wrote: > Using our dtb package's blobs cwfg(4) won't attach on the Pinebook Pro > since linux changed it: > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml > > Thanks to kettenis and patrick for pointing this out. > > This diff makes the sensor attach again and both hw.sensors as well as > apm (with the other diff) show correct values. > > > NB: This used to work without this diff with whatever blob was on the > machine when it came to me; it broke after I updated/replaced it with > /usr/local/share/dtb/arm64/rockchip/rk3399-pinebook-pro.dtb . For the archive, you can look at the blobs like so: $ doas pkg_add dtc dtc -O dts ./rk3399-pinebook-pro.dtb 2>/dev/null | grep cellwise, compatible = "cellwise,cw2015"; cellwise,battery-profile = <0x17678073 ...>; cellwise,monitor-interval-ms = <0x1388>;
Re: cwfg: update device-tree bindings
> Date: Mon, 22 Mar 2021 17:48:56 +0100
> From: Klemens Nanni
>
> Using our dtb package's blobs cwfg(4) won't attach on the Pinebook Pro
> since linux changed it:
>
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
>
> Thanks to kettenis and patrick for pointing this out.
>
> This diff makes the sensor attach again and both hw.sensors as well as
> apm (with the other diff) show correct values.
>
>
> NB: This used to work without this diff with whatever blob was on the
> machine when it came to me; it broke after I updated/replaced it with
> /usr/local/share/dtb/arm64/rockchip/rk3399-pinebook-pro.dtb .
>
> Feedback? OK?
>
> Index: cwfg.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 cwfg.c
> --- cwfg.c10 Jun 2020 17:51:21 - 1.1
> +++ cwfg.c22 Mar 2021 16:37:17 -
> @@ -124,7 +124,7 @@ cwfg_match(struct device *parent, void *
> {
> struct i2c_attach_args *ia = aux;
>
> - if (strcmp(ia->ia_name, "cellwise,cw201x") == 0)
> + if (strcmp(ia->ia_name, "cellwise,cw2015") == 0)
> return 1;
>
> return 0;
> @@ -143,14 +143,14 @@ cwfg_attach(struct device *parent, struc
> sc->sc_addr = ia->ia_addr;
> sc->sc_node = *(int *)ia->ia_cookie;
>
> - len = OF_getproplen(sc->sc_node, "cellwise,bat-config-info");
> + len = OF_getproplen(sc->sc_node, "cellwise,battery-profile");
> if (len <= 0) {
> printf(": missing or invalid battery info\n");
> return;
> }
>
> batinfo = malloc(len, M_TEMP, M_WAITOK);
> - OF_getprop(sc->sc_node, "cellwise,bat-config-info", batinfo, len);
> + OF_getprop(sc->sc_node, "cellwise,battery-profile", batinfo, len);
> switch (len) {
> case BATINFO_SIZE:
> memcpy(sc->sc_batinfo, batinfo, BATINFO_SIZE)
> @@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
> free(batinfo, M_TEMP, len);
>
> sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
> - "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
> + "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
I think the old property specified the interval in seconds. So you
should adjust the default value as well. The minimum allowed value is
250 so the current 8 makes no sense. You could make it 8000, but
maybe using 5000 is better as this is whatis used for the pinebook pro.
> sc->sc_design_capacity = OF_getpropint(sc->sc_node,
> "cellwise,design-capacity", CWFG_DESIGN_CAPACITY_DEFAULT);
> sc->sc_alert_level = OF_getpropint(sc->sc_node,
>
>
cwfg: update device-tree bindings
Using our dtb package's blobs cwfg(4) won't attach on the Pinebook Pro
since linux changed it:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/power/supply/cw2015_battery.yaml
Thanks to kettenis and patrick for pointing this out.
This diff makes the sensor attach again and both hw.sensors as well as
apm (with the other diff) show correct values.
NB: This used to work without this diff with whatever blob was on the
machine when it came to me; it broke after I updated/replaced it with
/usr/local/share/dtb/arm64/rockchip/rk3399-pinebook-pro.dtb .
Feedback? OK?
Index: cwfg.c
===
RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
retrieving revision 1.1
diff -u -p -r1.1 cwfg.c
--- cwfg.c 10 Jun 2020 17:51:21 - 1.1
+++ cwfg.c 22 Mar 2021 16:37:17 -
@@ -124,7 +124,7 @@ cwfg_match(struct device *parent, void *
{
struct i2c_attach_args *ia = aux;
- if (strcmp(ia->ia_name, "cellwise,cw201x") == 0)
+ if (strcmp(ia->ia_name, "cellwise,cw2015") == 0)
return 1;
return 0;
@@ -143,14 +143,14 @@ cwfg_attach(struct device *parent, struc
sc->sc_addr = ia->ia_addr;
sc->sc_node = *(int *)ia->ia_cookie;
- len = OF_getproplen(sc->sc_node, "cellwise,bat-config-info");
+ len = OF_getproplen(sc->sc_node, "cellwise,battery-profile");
if (len <= 0) {
printf(": missing or invalid battery info\n");
return;
}
batinfo = malloc(len, M_TEMP, M_WAITOK);
- OF_getprop(sc->sc_node, "cellwise,bat-config-info", batinfo, len);
+ OF_getprop(sc->sc_node, "cellwise,battery-profile", batinfo, len);
switch (len) {
case BATINFO_SIZE:
memcpy(sc->sc_batinfo, batinfo, BATINFO_SIZE);
@@ -167,7 +167,7 @@ cwfg_attach(struct device *parent, struc
free(batinfo, M_TEMP, len);
sc->sc_monitor_interval = OF_getpropint(sc->sc_node,
- "cellwise,monitor-interval", CWFG_MONITOR_INTERVAL_DEFAULT);
+ "cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
sc->sc_design_capacity = OF_getpropint(sc->sc_node,
"cellwise,design-capacity", CWFG_DESIGN_CAPACITY_DEFAULT);
sc->sc_alert_level = OF_getpropint(sc->sc_node,
Re: arm64: make cwfg(4) report battery information to apm(4)
Better diff at the end thanks to jca's eyeballing, see comments inline.
kettenis: I see room for improvement in our subsystems and their
interactions, but I don't think the current situation is bad enough to
leave those bits out for now.
Feedback? OK?
On Mon, Mar 22, 2021 at 01:21:02AM +0100, Jeremie Courreges-Anglas wrote:
> > Index: dev/fdt/cwfg.c
> > ===
> > RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 cwfg.c
> > --- dev/fdt/cwfg.c 10 Jun 2020 17:51:21 - 1.1
> > +++ dev/fdt/cwfg.c 20 Mar 2021 23:43:13 -
> > @@ -32,12 +32,15 @@
> > #include
> > #include
> >
> > +#include
> > #include
> >
> > #include
> >
> > #include
> >
> > +#include "apm.h"
> > +
> > #defineVERSION_REG 0x00
> > #defineVCELL_HI_REG0x02
> > #define VCELL_HI_MASK 0x3f
> > @@ -119,6 +122,18 @@ struct cfdriver cwfg_cd = {
> > NULL, "cwfg", DV_DULL
> > };
> >
> > +int cwfg_apminfo(struct apm_power_info *info);
>
> Why out of the #if NAPM > 0 check?
Copied over from the loongson driver; the declaration doesn't hurt
outside but isn't needed either. I've removed it.
> > +#if NAPM > 0
> > +struct apm_power_info cwfg_power;
> > +
> > +int
> > +cwfg_apminfo(struct apm_power_info *info)
> > +{
> > + bcopy(_power, info, sizeof(*info));
>
> There's no overlap between source and destination, better use memcpy.
> (Else, better use memmove.)
Right, I also copied this over from loongson (we can dust that one off
as well later on).
>
> > + return 0;
> > +}
> > +#endif
> > +
> > int
> > cwfg_match(struct device *parent, void *match, void *aux)
> > {
> > @@ -202,6 +217,12 @@ cwfg_attach(struct device *parent, struc
> >
> > sensor_task_register(sc, cwfg_update_sensors, 5);
> >
> > +#if NAPM > 0
> > + /* make sure we have the apm state initialized before apm attaches */
> > + cwfg_update_sensors(sc);
> > + apm_setinfohook(cwfg_apminfo);
> > +#endif
> > +
> > printf("\n");
> > }
> >
> > @@ -324,6 +345,7 @@ cwfg_update_sensors(void *arg)
> > uint32_t vcell, rtt, tmp;
> > uint8_t val;
> > int error, n;
> > + u_char bat_percent;
> >
> > if ((error = cwfg_lock(sc)) != 0)
> > return;
> > @@ -348,6 +370,7 @@ cwfg_update_sensors(void *arg)
> > if ((error = cwfg_read(sc, SOC_HI_REG, )) != 0)
> > goto done;
> > if (val != 0xff) {
> > + bat_percent = val;
> > sc->sc_sensor[CWFG_SENSOR_SOC].value = val * 1000;
> > sc->sc_sensor[CWFG_SENSOR_SOC].flags &= ~SENSOR_FINVALID;
> > }
>
> If val == 0xff bat_percent is unitialized. Note that in this error
> condition the sensors code leaves sc->sc_sensor[CWFG_SENSOR_SOC].value
> alone.
Oops. Both `val' and `rtt' can be "useless" and we could end up with
a partially updated `cwfg_power'.
If we always reset all values up front and then update whatever is
possible, we can avoid the intermediate variable completely and end up
with `cwfg_power' providing consistent readings.
>
> > @@ -363,6 +386,14 @@ cwfg_update_sensors(void *arg)
> > sc->sc_sensor[CWFG_SENSOR_RTT].value = rtt;
> > sc->sc_sensor[CWFG_SENSOR_RTT].flags &= ~SENSOR_FINVALID;
> > }
> > +
> > +#if NAPM > 0
> > + cwfg_power.battery_state = bat_percent > sc->sc_alert_level ?
> > + APM_BATT_HIGH : APM_BATT_LOW;
>
> It's cool that this driver keeps track of the "alert level". acpibat(4)
> also does that on amd64, but the apm(4)-through-acpi(4) userland
> interface just hardcodes levels:
>
> /* running on battery */
> pi->battery_life = remaining * 100 / capacity;
> if (pi->battery_life > 50)
> pi->battery_state = APM_BATT_HIGH;
> else if (pi->battery_life > 25)
> pi->battery_state = APM_BATT_LOW;
> else
> pi->battery_state = APM_BATT_CRITICAL;
>
> Maybe the levels reported by hardware couldn't be trusted? Or maybe
> those hardcoded values were deemed good enough, back then. Anyway, on
> this new hardware I think it makes sense to just trust the exported
> values and later act if we get reports.
Yes, I explicitly want to use what hardware provides and we can still
tweak it to more sensible values if need be.
>
> > + cwfg_power.ac_state = APM_AC_UNKNOWN;
>
> This kinda points out the need for an AC driver on this platform.
> If done in another driver, the code will need to be changed. But this
> looks acceptable to me for now.
Yup.
>
> > + cwfg_power.battery_life = bat_percent;
>
> So to keep apm and sensors in sync I think it would be better to reuse
> the cached sc->sc_sensor[CWFG_SENSOR_SOC].value.
I did `sc->sc_sensor[CWFG_SENSOR_SOC].value / 1000' first but ended up
with bogus values, hence the `bat_percent' buffer to avoid arithmetics.
> I don't know
