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

From: Steven Sistare <steven.sistare@oracle.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "Jason Zeng" <jason.zeng@linux.intel.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	QEMU <qemu-devel@nongnu.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrange" <berrange@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH V5 10/25] util: env var helpers
Date: Tue, 13 Jul 2021 12:15:30 -0400	[thread overview]
Message-ID: <895c7a47-2c9a-3a8f-835d-b2798a92c890@oracle.com> (raw)
In-Reply-To: <CAJ+F1C+OHw5=GOwSEtoEbpujVtbA80bBPmq3DFJKv5d9T1xxPQ@mail.gmail.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