Re: [PATCH V3 00/22] Live Update [restart] : exec

[Steven Sistare]

On 6/3/2021 4:44 PM, Daniel P. Berrangé wrote:
> On Thu, Jun 03, 2021 at 08:36:42PM +0100, Dr. David Alan Gilbert wrote:
>> * Steven Sistare (steven.sistare@oracle.com) wrote:
>>> On 5/24/2021 6:39 AM, Dr. David Alan Gilbert wrote:
>>>> * Steven Sistare (steven.sistare@oracle.com) wrote:
>>>>> On 5/20/2021 9:13 AM, Dr. David Alan Gilbert wrote:
>>>>>> On the 'restart' branch of questions; can you explain,
>>>>>> other than the passing of the fd's, why the outgoing side of
>>>>>> qemu's 'migrate exec:' doesn't work for you?
>>>>>
>>>>> I'm not sure what I should describe.  Can you be more specific?
>>>>> Do you mean: can we add the cpr specific bits to the migrate exec code?
>>>>
>>>> Yes; if possible I'd prefer to just keep the one exec mechanism.
>>>> It has an advantage of letting you specify the new command line; that
>>>> avoids the problems I'd pointed out with needing to change the command
>>>> line if a hotplug had happened.  It also means we only need one chunk of
>>>> exec code.
>>>
>>> How/where would you specify a new command line?  Are you picturing the 
>>> usual migration
>>> setup where you start a second qemu with its own arguments, plus a 
>>> migrate_incoming
>>> option or command?  That does not work for live update restart; the old 
>>> qemu must exec
>>> the new qemu.  Or something else?
>>
>> The existing migration path allows an exec - originally intended to exec
>> something like a compressor or a store to file rather than a real
>> migration; i.e. you can do:
>>
>>   migrate "exec:gzip > mig"
>>
>> and that will save the migration stream to a compressed file called mig.
>> Now, I *think* we can already do:
>>
>>   migrate "exec:path-to-qemu command line parameters -incoming 'hmmmmm'"
>> (That's probably cleaner via the QMP interface).
>>
>> I'm not quite sure what I want in the incoming there, but that is
>> already the source execing the destination qemu - although I think we'd
>> probably need to check if that's actually via an intermediary.
> 
> I don't think you can dirctly exec  qemu in that way, because the
> source QEMU migration code is going to wait for completion of the
> QEMU you exec'd and that'll never come on success. So you'll end
> up with both QEMU's running forever. If you pass the daemonize
> option to the new QEMU then it will immediately detach itself,
> and the source QEMU will think the migration command has finished
> or failed.
> 
> I think you can probably do it if you use a wrapper script though.
> The wrapper would have to fork QEMU in the backend, and then the
> wrapper would have to monitor the new QEMU to see when the incoming
> migration has finished/aborted, at which point the wrapper can
> exit, so the source QEMU sees a successful cleanup of the exec'd
> command. </hand waving>

cpr restart does not work for any scheme that involves the old qemu process 
co-existing with
the new qemu process.  To preserve descriptors and anonymous memory, cpr 
restart requires 
that old qemu directly execs new qemu.  Not fork-exec.  Same pid.

So responding to Dave's comment, "keep the one exec mechanism", that is not 
possible.
We still need the qemu_exec_requested mechanism to cause a direct exec after 
state is
saved.

>>> We could shoehorn cpr restart into the migrate exec path by defining a new 
>>> migration 
>>> capability that the client would set before issuing the migrate command.  
>>> However, the
>>> implementation code would be sprinkled with conditionals to suppress 
>>> migrate-only bits
>>> and call cpr-only bits.  IMO that would be less maintainable than having a 
>>> separate
>>> cprsave function.  Currently cprsave does not duplicate any migration 
>>> functionality.
>>> cprsave calls qemu_save_device_state() which is used by xen.
>>
>> To me it feels like cprsave in particular is replicating more code.
>>
>> It's also jumping through hoops in places to avoid changing the
>> commandline;  that's going to cause more pain for a lot of people - not
>> just because it's hacks all over for that, but because a lot of people
>> are actually going to need to change the commandline even in a cpr like
>> case (e.g. due to hotplug or changing something else about the
>> environment, like auth data or route to storage or networking that
>> changed).
> 
> Management apps that already support migration, will almost certainly
> know how to start up a new QEMU with a different command line that
> takes account of hotplugged/unplugged devices. IOW avoiding changing
> the command line only really addresses the simple case, and the hard
> case is likely already solved for purposes of handling regular live
> migration. 

Agreed, with the caveat that for cpr, the management app must communicate the 
new arguments
to the qemu-exec trampoline, rather than passing the args on the command line 
to a new 
qemu process.

>> There are hooks for early parameter parsing, so if we need to add extra
>> commandline args we can; but for example the case of QEMU_START_FREEZE
>> to add -S just isn't needed as soon as you let go of the idea of needing
>> an identical commandline.

I'll delete QEMU_START_FREEZE.  

I still need to preserve argv_main and pass it to the qemu-exec trampoline, 
though, as 
the args contain identifying information that the management app needs to 
modify the 
arguments based the the instances's hot plug history.

Or, here is another possibility.  We could redefine cprsave to leave the VM in a
stopped state, and add a cprstart command to be called subsequently that 
performs 
the exec.  It takes a single string argument: a command plus arguments to exec. 
 
The command may be qemu or a trampoline like qemu-exec.  I like that the 
trampoline
name is no longer hardcoded.  The management app can derive new qemu args for 
the
instances as it would with migration, and pass them to the command, instead of 
passing
them to qemu-exec via some side channel.  cprload finishes the job and does not 
change.
I already like this scheme better.

- Steve