On Mon, Feb 07, 2022 at 11:49:38PM -0300, Leonardo Bras Soares Passos wrote: > Hello Peter, thanks for reviewing! > > On Mon, Feb 7, 2022 at 11:22 PM Peter Xu <pet...@redhat.com> wrote: > > > > On Tue, Feb 01, 2022 at 03:29:03AM -0300, Leonardo Bras wrote: > > > -void multifd_send_sync_main(QEMUFile *f) > > > +int multifd_send_sync_main(QEMUFile *f) > > > { > > > int i; > > > + bool flush_zero_copy; > > > > > > if (!migrate_use_multifd()) { > > > - return; > > > + return 0; > > > } > > > if (multifd_send_state->pages->num) { > > > if (multifd_send_pages(f) < 0) { > > > error_report("%s: multifd_send_pages fail", __func__); > > > - return; > > > + return 0; > > > > I've not checked how it used to do if multifd_send_pages() failed, but.. > > should > > it returns -1 rather than 0 when there will be a return code? > > Yeah, that makes sense. > The point here is that I was trying not to modify much of the current > behavior. > > I mean, multifd_send_sync_main() would previously return void, so any > other errors would not matter to the caller of this function, which > will continue to run as if nothing happened. > > Now, if it fails with flush_zero_copy, the operation needs to be aborted.
Right, so how I understand is we'll fail anyway, and this allows us to fail (probably) sooner. > > Maybe, I should make it different: > - In any error, return -1. > - Create/use a specific error code in the case of a failing > flush_zero_copy, so I can test the return value for it on the caller > function and return early. > > Or alternatively, the other errors could also return early, but since > this will change how the code currently works, I would probably need > another patch for that change. (so it can be easily reverted if > needed) Yeah, should work too to add a patch before this one. > > What do you think is better? I just don't see how it could continue if e.g. multifd_send_pages() failed. The other thing is returning zero looks weird itself when there's obviously an error. Normally we could allow that but better with a comment showing why. For this case it's more natural to me if we could just fail early. Juan? -- Peter Xu