External email: Use caution opening links or attachments
On 29.11.2024 15:08, Cédric Le Goater wrote:
On 11/17/24 20:20, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
It's possible for load_cleanup SaveVMHandler to get called without
load_setup handler being called first.
Since we'll be soon running cleanup operations there that access
objects
that need earlier initialization in load_setup let's make sure these
cleanups only run when load_setup handler had indeed been called
earlier.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
tbh, that's a bit ugly. I agree it's similar to those 'bool initialized'
attributes we have in some structs, so nothing new or really wrong.
But it does look like a workaound for a problem or cleanups missing
that would need time to untangle.
I would prefer to avoid this change and address the issue from the
migration subsystem if possible.
While it would be pretty simple to only call {load,save}_cleanup
SaveVMHandlers when the relevant {load,save}_setup handler was
successfully called first this would amount to a change of these
handler semantics.
This would risk introducing regressions - for example vfio_save_setup()
doesn't clean up (free) newly allocated migration->data_buffer
if vfio_migration_set_state() were to fail later in this handler
and relies on an unconstitutional call to vfio_save_cleanup() in
order to clean it up.
There might be similar issues in other drivers too.