Re: [PATCH V5 10/25] util: env var helpers

[Steven Sistare]

On 7/12/2021 3:36 PM, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 12, 2021 at 11:19 PM Steven Sistare <steven.sistare@oracle.com 
> <mailto:steven.sistare@oracle.com>> wrote:
> 
>     On 7/8/2021 11:10 AM, Marc-André Lureau wrote:
>     > Hi
>     >
>     > On Wed, Jul 7, 2021 at 9:30 PM Steve Sistare <steven.sistare@oracle.com 
> <mailto:steven.sistare@oracle.com> <mailto:steven.sistare@oracle.com 
> <mailto:steven.sistare@oracle.com>>> wrote:
>     >
>     >     Add functions for saving fd's and other values in the environment 
> via
>     >     setenv, and for reading them back via getenv.
>     >
>     >
>     > I understand that the rest of the series will rely on environment 
> variables to associate and recover the child-passed FDs, but I am not really 
> convinced that it is a good idea.
>     >
>     > Environment variables have a number of issues that we may encounter 
> down the road: namespace, limits, concurrency, observability etc.. I wonder 
> if the VMState couldn't have a section about the FD to recover. Or maybe just 
> another shared memory region?
> 
>     They also have some advantages.  Their post-exec value can be observed 
> via /proc/$pid/environ,
>     and modified values can be observed by calling printenv() in a debugger.  
> They are naturally carried
>     across exec, with no external file to create and potentially lose.  
> Lastly, libcs already defines
>     put and get methods, so the additional layered code is small and simple.  
> The number of variables
>     is small, and I would rather not over-engineer an alternate solution 
> until the env proves
>     inadequate.  The limits on env size are huge on Linux.  The limits are 
> smaller on Windows, but
>     that is just one of multiple issues to be addressed to support live 
> update on windows.
> 
>     For the alternatives, shared memory is no more observable (maybe less) 
> and also has no concurrency
>     protection.  VMstate does not help because the descriptors are needed 
> before the vmstate file
>     is opened.
>  
> Why does it need to be "observable" from outside the process?
> 
> I meant memory to be shared between the qemu instances (without concurrency 
> etc).
> 
> You would only need that memory fd to be passed as argument to the next qemu 
> instance, to restore the rest of the contexts/fds I suppose.
> 
> I think we need to do this right, as it may have consequences for future 
> updates. It's effectively a kind of protocol. We have better chances to 
> handle different versions correctly by reusing VMState imho.

OK, I yield.  David also does not like using env vars here. I'll define 
accessors that manipulate a QLIST of struct {int fd, char *name}, create a 
vmstate struct to describe it using VMSTATE_QLIST_V, 
and serialize to a memfd.  

Sound OK?  

- Steve