On Fri, Jul 22, 2022 at 07:09:57PM +0200, Paolo Bonzini wrote: > On 7/22/22 17:43, Martin Kletzander wrote: > > As mentioned before, all these failures do not have to exit the > > function, but rather fallback to the old way. You can even create > > two new functions for the new and old implementations and then call > > them from here to make the fallback easier to spot (and code). > > More precisely, they should just "continue;" to the next iteration of > the for loop, like > > > if (!success_obj || !fail_obj) > continue; > > found = true; > > and then go fall back if found is false at the end of the loop. > > On the other hand, here: > > if (virJSONValueGetNumberUlong(success_obj, &curHaltPollSuccess) < 0) > return 0; > > if (virJSONValueGetNumberUlong(fail_obj, &curHaltPollFail) < 0) > return 0; > > I am not sure about falling back, because it is really an unexpected > situation where the statistic exist but somehow the value cannot be > parsed. Then can we just "continue;" in case the value fails to parse as well? > > Paolo > > > I wanted to change this before pushing as well, but I feel like I'm > > changing too much of your code. And I also had to rebase this > > (although trivially). Would you mind just changing these few last > > things so that we can get it in before the rc0 freeze? Alright, as soon as there is a viable check decided for the virJSONValueGet* statements above, I will push a v4 with the changes you mentioned in your reviews. Thank you both for taking the time to review. >
Attachment:
signature.asc
Description: PGP signature
