On 6/7/2021 2:08 PM, Steven Sistare wrote: > On 6/3/2021 3:36 PM, Dr. David Alan Gilbert wrote: >> * Steven Sistare (steven.sist...@oracle.com) wrote: >>> On 5/24/2021 6:39 AM, Dr. David Alan Gilbert wrote: >>>> * Steven Sistare (steven.sist...@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. >>> >>> [...] >> >> 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. >> >>> 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. > > In the attached file I annotated lines of code that have some overlap > with migration code actions. They include vm control, global_state_store, > and vmstate save, and cover 18 lines of 78 total. I did not include the > body of qf_file_open because it is also called by xen. > > The migration code adds capabilities, parameters, state, status, info, > precopy, postcopy, dirty bitmap, events, notifiers, 6 channel types, > blockers, pause+resume, return path, request-reply commands, throttling, colo, > blocks, phases, iteration, and threading, implemented by 20000+ lines of code. > To me it seems wrong to throw cpr into that mix to avoid adding tens of lines > of similar code.
Hi David, what is your decision, will you accept separate cpr commands? One last point is that Xen made a similar choice, adding the xen-save-devices-state command which calls qemu_save_device_state instead of migration_thread. - Steve
