Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
"Ted Unangst" writes: > On 2022-02-25, Robert Nagy wrote: >> Maybe we need a default vmd class? What do you guys think? > > Regardless of what the limit is, this seems like a daemon where people > will bump into the limit. Perhaps a reminder is in order too? > The reminder is good, but we still need to fix the problem that the vmm process can abort given the child dies so quickly. On my machine, the call to read(2) results in a zero byte read, tripping the existing fatal path. diff ff838b72f50de699ee43d3dac58ff7e8435669ee /usr/src blob - 4c6c99f1133cec7cb1e38dfd22e595e4d2023842 file + usr.sbin/vmd/vm.c --- usr.sbin/vmd/vm.c +++ usr.sbin/vmd/vm.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -292,8 +293,12 @@ start_vm(struct vmd_vm *vm, int fd) ret = alloc_guest_mem(vcp); if (ret) { + struct rlimit lim; + const char *msg = "could not allocate guest memory - exiting"; + if (getrlimit(RLIMIT_DATA, &lim) == 0) + msg = "could not allocate guest memory (data limit is %llu) - exiting"; errno = ret; - fatal("could not allocate guest memory - exiting"); + fatal(msg, lim.rlim_cur); } ret = vmm_create_vm(vcp); blob - eb75b4c587884ec43704420ef4172386a5b39bd9 file + usr.sbin/vmd/vmm.c --- usr.sbin/vmd/vmm.c +++ usr.sbin/vmd/vmm.c @@ -616,6 +616,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p int ret = EINVAL; int fds[2]; size_t i, j; + ssize_t sz; if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) { log_warnx("%s: can't find vm", __func__); @@ -674,9 +675,13 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *p } /* Read back the kernel-generated vm id from the child */ - if (read(fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id)) != - sizeof(vcp->vcp_id)) + sz = read(fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id)); + if (sz < 0) fatal("read vcp id"); + else if (sz != sizeof(vcp->vcp_id)) { + log_warn("failed to read vcp id"); + goto err; + } if (vcp->vcp_id == 0) goto err;
Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
On Fri, Feb 25, 2022 at 06:36:35PM -, Stuart Henderson wrote: > On 2022-02-25, Robert Nagy wrote: > > Maybe we need a default vmd class? What do you guys think? > > I definitely think it makes sense. Not sure whether it should just go in > etc.amd64 or the others too (vmd only exists on amd64 so far, but if it's > ever added e.g. to arm64 then adding it in the other files would avoid > having to remember to add it later..) > > Maybe the same 16G limit as bgpd by default.. > > I am in support of this change as well. Thanks everyone.
Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
On Fri, Feb 25, 2022 at 01:46:00PM -0500, Ted Unangst wrote: > On 2022-02-25, Robert Nagy wrote: > > Maybe we need a default vmd class? What do you guys think? > > Regardless of what the limit is, this seems like a daemon where people > will bump into the limit. Perhaps a reminder is in order too? > ok mlarkin on this one. thanks > > Index: vm.c > === > RCS file: /cvs/src/usr.sbin/vmd/vm.c,v > retrieving revision 1.67 > diff -u -p -r1.67 vm.c > --- vm.c 30 Dec 2021 08:12:23 - 1.67 > +++ vm.c 25 Feb 2022 18:42:39 - > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -292,8 +293,12 @@ start_vm(struct vmd_vm *vm, int fd) > ret = alloc_guest_mem(vcp); > > if (ret) { > + struct rlimit lim; > + const char *msg = "could not allocate guest memory - exiting"; > + if (getrlimit(RLIMIT_DATA, &lim) == 0) > + msg = "could not allocate guest memory (data limit is > %llu) - exiting"; > errno = ret; > - fatal("could not allocate guest memory - exiting"); > + fatal(msg, lim.rlim_cur); > } > > ret = vmm_create_vm(vcp); >
Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
On 2022-02-25, Robert Nagy wrote: > Maybe we need a default vmd class? What do you guys think? Regardless of what the limit is, this seems like a daemon where people will bump into the limit. Perhaps a reminder is in order too? Index: vm.c === RCS file: /cvs/src/usr.sbin/vmd/vm.c,v retrieving revision 1.67 diff -u -p -r1.67 vm.c --- vm.c30 Dec 2021 08:12:23 - 1.67 +++ vm.c25 Feb 2022 18:42:39 - @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -292,8 +293,12 @@ start_vm(struct vmd_vm *vm, int fd) ret = alloc_guest_mem(vcp); if (ret) { + struct rlimit lim; + const char *msg = "could not allocate guest memory - exiting"; + if (getrlimit(RLIMIT_DATA, &lim) == 0) + msg = "could not allocate guest memory (data limit is %llu) - exiting"; errno = ret; - fatal("could not allocate guest memory - exiting"); + fatal(msg, lim.rlim_cur); } ret = vmm_create_vm(vcp);
Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
On 2022-02-25, Robert Nagy wrote: > Maybe we need a default vmd class? What do you guys think? I definitely think it makes sense. Not sure whether it should just go in etc.amd64 or the others too (vmd only exists on amd64 so far, but if it's ever added e.g. to arm64 then adding it in the other files would avoid having to remember to add it later..) Maybe the same 16G limit as bgpd by default..
Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM
Maybe we need a default vmd class? What do you guys think? On 25/02/22 15:34 +0100, Paul de Weerd wrote: > Hi all, > > In commit Eg1WuG7hzCoCPdcz, robert@ changed the ulimit for the daemon > class in /etc/login.conf for amd64 from 'infinity' to 4096M (see [0] > and [1]). > > This change broke my vmd setup, and I had to dig around to understand > what happened. Sharing here in hopes of preventing others from > wasting their time like I did. > > > I have a VM that is configured with 4GB of RAM: > > [weerd@pom] $ grep -B2 4G /etc/vm.conf > vm "builder" { > owner weerd > memory 4G > > After upgrading to a newer snapshot (and sysmerge'ing login.conf), vmd > crashes when this VM gets started: > > pom vmd[98555]: builder: could not allocate guest memory - exiting: Cannot > allocate memory > pom vmd[71874]: vmm: read vcp id > pom vmd[10670]: priv exiting, pid 10670 > pom vmd[73889]: control exiting, pid 73889 > > (resource limits doing exactly what they're supposed to do here!) > > It took me longer than I care to admit to realize that this would be > caused by the newly reduced datasize limit imposed by Robert's change. > I fixed this by adding a dedicated login.conf stanza for vmd: > > [weerd@pom] $ tail -n7 /etc/login.conf > ## > # Local changes > # > # vmd runs VMs with 4GB, so it needs an increased datasize limit: > vmd:\ > :datasize=5120M:\ > :tc=daemon: > > Alternatively, I could've stuck that bit in /etc/login.conf.d/vmd > which would've had the same effect. But with that change everything > is working just fine again. When you run into a similar issue, make > sure not to just revert back to "infinite" - find a suitable limit for > whatever piece of software you have and adjust accordingly. > > Cheers, > > Paul > > [0]: https://marc.info/?l=openbsd-cvs&m=164542553811748&w=2 > [1]: > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/etc/etc.amd64/login.conf.diff?r1=1.21&r2=1.22 > > -- > >[<++>-]<+++.>+++[<-->-]<.>+++[<+ > +++>-]<.>++[<>-]<+.--.[-] > http://www.weirdnet.nl/ > -- Regards, Robert Nagy