[PATCH 3/3] qemu_driver: use qemuMonitorQueryStats to extract halt poll time

Martin Kletzander mkletzan at redhat.com
Tue Jul 26 10:44:29 UTC 2022


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.
>>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220726/877d2f8c/attachment-0001.sig>


More information about the libvir-list mailing list