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

[Steven Sistare <steven.sistare@oracle.com>]

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