On Sun, Jul 24, 2022 at 10:50:36AM +0530, Amneesh Singh wrote:
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.
The main thing I was worried about was libvirtd crashing on possibly expoited qemu process. Whether we fall back or report nothing in this case is not that big of a deal, since there's something wrong already anyway.
Then can we just "continue;" in case the value fails to parse as well?
That's another option 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.