[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Only issue the rtc-reset-reinjection QMP command on x86.



On Tue, May 26, 2015 at 10:53:53 +0200, Martin Kletzander wrote:
> On Tue, May 26, 2015 at 10:08:58AM +0200, Peter Krempa wrote:
> >On Mon, May 25, 2015 at 17:43:05 +0200, Martin Kletzander wrote:
> >> On Mon, May 25, 2015 at 05:26:01PM +0200, Peter Krempa wrote:
> >> >On Mon, May 25, 2015 at 16:59:08 +0200, Andrea Bolognani wrote:

...

> >> >And just conditionally call this when the
> >> >QEMU_CAPS_RTC_RESET_REINJECTION is present and not in an architecture
> >> >specific way.
> >> >
> >> >By this you get rid of the arch specific hackery.
> >> >
> >>
> >> But on x86 we don't even want to call the SetTime command when we
> >> cannot reset the rtc reinjection.  On ppc there is no reinjection
> >> being done, hence nothing to reset.
> >
> >Well I don't think that the missing reinjection request should be a hard
> >requirement on x86 since the guest can modify time independently and in
> >that case the reinjection command won't be even issued. The guest agent
> >command is technically a guest issued time change too, so there's only
> >the added bonus that we'd reset the interrupt backlog. One slight issue
> >is that libvirt does not allow to reset the interrupt backlog while not
> >setting the time though.
> >
> 
> The implementer of the check might remember more reasons for arguing
> on this, let him shed the light into this discussion.  Michal?
> 
> Anyway, this is broken all the way because the command also makes
> sense only with some particular timers and their settins, not with all
> of them.

In that case it would make more sense to have the interrupt reinjection
reset as a separate option or it to be enabled with a flag.

> 
> >Even if we decide that the check for the existance of the command should
> >be arch-specific and should report error, the actual place where the
> >command is called should be made dependant on the presence of the
> >command rather than the ARCH check, so that it does not need to be
> >modified once it's implemented somewhere else.
> >
> 
> I don't quite catch your drift here.  If you mean that the second
> condition added should be based on virQEMUCapsGet(), then I agree.

Yes that was my intent. The actual call should be based on the
capability. Sanity checks that are arch specific should be present
before.

Peter

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]