On Tue, Aug 06, 2024 at 10:32:54AM GMT, Daniel P. Berrangé wrote: > On Mon, Aug 05, 2024 at 09:21:36PM -0500, Eric Blake wrote: > > Since an NBD server may be long-living, serving clients that > > repeatedly connect and disconnect, it can be more efficient to clean > > up after each client disconnects, rather than storing a list of > > resources to clean up when the server exits. Rewrite the list of > > known clients to be double-linked so that we can get O(1) deletion to > > keep the list pruned to size as clients exit. This in turn requires > > each client to track an opaque pointer of owner information (although > > qemu-nbd doesn't need to refer to it). > > I tend to feel that this needs to be squashed into the previous > patch. The previous patch effectively creates unbounded memory > usage in the NBD server. ie consider a client that connects and > immediately disconnects, not sending any data, in a tight loop. > It will "leak" NBDConn & QIOChanelSocket pointers for each > iteration of the loop, only to be cleaned up when the NBD Server > is shutdown.
Hmm. Offline, we observed that qemu's default of unlimited clients may not be the smartest thing - if max-connections is not set, we may want to default it to something like 100 (even when MULTI_CONN is exploited, most clients that take advantage of it will probably only use 8 or 16 parallel sockets; more than that tends to run into other limits rather than providing continual improvements). I can add such a change in default in v4. In contrast, qemu-nbd defaults to max-connections 1 for historical reasons (among others, if you have a non-persistent server, it is really nice that qemu-nbd automatically exits after the first client disconnects, rather than needing to clean up the server yourself), but that's too small for MULTI_CONN to be of any use. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
