Re: login.conf daemon datasize limit effects on VMs with 4GB+ RAM

2022-02-26 Thread Dave Voutila


"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, ) == 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_id, sizeof(vcp->vcp_id)) !=
-   sizeof(vcp->vcp_id))
+   sz = read(fds[0], >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

2022-02-25 Thread Mike Larkin
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

2022-02-25 Thread Mike Larkin
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, ) == 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

2022-02-25 Thread Ted Unangst
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, ) == 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

2022-02-25 Thread Stuart Henderson
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

2022-02-25 Thread Robert Nagy
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=164542553811748=2
> [1]: 
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/etc/etc.amd64/login.conf.diff?r1=1.21=1.22
> 
> -- 
> >[<++>-]<+++.>+++[<-->-]<.>+++[<+
> +++>-]<.>++[<>-]<+.--.[-]
>  http://www.weirdnet.nl/ 
> 

-- 
Regards,
Robert Nagy



login.conf daemon datasize limit effects on VMs with 4GB+ RAM

2022-02-25 Thread Paul de Weerd
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=164542553811748=2
[1]: 
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/etc/etc.amd64/login.conf.diff?r1=1.21=1.22

-- 
>[<++>-]<+++.>+++[<-->-]<.>+++[<+
+++>-]<.>++[<>-]<+.--.[-]
 http://www.weirdnet.nl/