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.
Attachment:
signature.asc
Description: PGP signature
