All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Amit Shah <amit.shah@redhat.com>
Cc: "qemu list" <qemu-devel@nongnu.org>,
	"kvm list" <kvm@vger.kernel.org>,
	"Eric B Munson" <emunson@mgebm.net>,
	"Raghavendra K T" <raghavendra.kt@linux.vnet.ibm.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Laszlo Ersek" <lersek@redhat.com>
Subject: Re: [PATCH 1/1] kvmclock: fix guest stop notification
Date: Sun, 30 Sep 2012 20:05:16 -0300	[thread overview]
Message-ID: <20120930230516.GA3923@amt.cnet> (raw)
In-Reply-To: <20120920124641.GA15092@amt.cnet>

On Thu, Sep 20, 2012 at 09:46:41AM -0300, Marcelo Tosatti wrote:
> On Thu, Sep 20, 2012 at 01:55:20PM +0530, Amit Shah wrote:
> > Commit f349c12c0434e29c79ecde89029320c4002f7253 added the guest stop
> > notification, but it did it in a way that the stop notification would
> > never reach the kernel.  The kvm_vm_state_changed() function gets a
> > value of 0 for the 'running' parameter when the VM is stopped, making
> > all the code added previously dead code.
> > 
> > This patch reworks the code so that it's called when 'running' is 0,
> > which indicates the VM was stopped.
> > 
> > CC: Eric B Munson <emunson@mgebm.net>
> > CC: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> > CC: Andreas Färber <afaerber@suse.de>
> > CC: Marcelo Tosatti <mtosatti@redhat.com>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > CC: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/kvm/clock.c |   21 +++++++++++----------
> >  1 files changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c
> > index 824b978..f3427eb 100644
> > --- a/hw/kvm/clock.c
> > +++ b/hw/kvm/clock.c
> > @@ -71,18 +71,19 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> >  
> >      if (running) {
> >          s->clock_valid = false;
> > +        return;
> > +    }
> >  
> > -        if (!cap_clock_ctrl) {
> > -            return;
> > -        }
> > -        for (penv = first_cpu; penv != NULL; penv = penv->next_cpu) {
> > -            ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > -            if (ret) {
> > -                if (ret != -EINVAL) {
> > -                    fprintf(stderr, "%s: %s\n", __func__, strerror(-ret));
> > -                }
> > -                return;
> > +    if (!cap_clock_ctrl) {
> > +        return;
> > +    }
> > +    for (penv = first_cpu; penv != NULL; penv = penv->next_cpu) {
> > +        ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > +        if (ret) {
> > +            if (ret != -EINVAL) {
> > +                fprintf(stderr, "%s: %s\n", __func__, strerror(-ret));
> >              }
> > +            return;
> >          }
> >      }
> >  }
> > -- 
> > 1.7.7.6
> 
> ACK
> 
> Avi, please merge through uq/master.

NACK, guest should be notified when the VM is starting, not
when stopping.


WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Amit Shah <amit.shah@redhat.com>
Cc: "kvm list" <kvm@vger.kernel.org>,
	"Raghavendra K T" <raghavendra.kt@linux.vnet.ibm.com>,
	"qemu list" <qemu-devel@nongnu.org>,
	"Eric B Munson" <emunson@mgebm.net>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH 1/1] kvmclock: fix guest stop notification
Date: Sun, 30 Sep 2012 20:05:16 -0300	[thread overview]
Message-ID: <20120930230516.GA3923@amt.cnet> (raw)
In-Reply-To: <20120920124641.GA15092@amt.cnet>

On Thu, Sep 20, 2012 at 09:46:41AM -0300, Marcelo Tosatti wrote:
> On Thu, Sep 20, 2012 at 01:55:20PM +0530, Amit Shah wrote:
> > Commit f349c12c0434e29c79ecde89029320c4002f7253 added the guest stop
> > notification, but it did it in a way that the stop notification would
> > never reach the kernel.  The kvm_vm_state_changed() function gets a
> > value of 0 for the 'running' parameter when the VM is stopped, making
> > all the code added previously dead code.
> > 
> > This patch reworks the code so that it's called when 'running' is 0,
> > which indicates the VM was stopped.
> > 
> > CC: Eric B Munson <emunson@mgebm.net>
> > CC: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> > CC: Andreas Färber <afaerber@suse.de>
> > CC: Marcelo Tosatti <mtosatti@redhat.com>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > CC: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/kvm/clock.c |   21 +++++++++++----------
> >  1 files changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c
> > index 824b978..f3427eb 100644
> > --- a/hw/kvm/clock.c
> > +++ b/hw/kvm/clock.c
> > @@ -71,18 +71,19 @@ static void kvmclock_vm_state_change(void *opaque, int running,
> >  
> >      if (running) {
> >          s->clock_valid = false;
> > +        return;
> > +    }
> >  
> > -        if (!cap_clock_ctrl) {
> > -            return;
> > -        }
> > -        for (penv = first_cpu; penv != NULL; penv = penv->next_cpu) {
> > -            ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > -            if (ret) {
> > -                if (ret != -EINVAL) {
> > -                    fprintf(stderr, "%s: %s\n", __func__, strerror(-ret));
> > -                }
> > -                return;
> > +    if (!cap_clock_ctrl) {
> > +        return;
> > +    }
> > +    for (penv = first_cpu; penv != NULL; penv = penv->next_cpu) {
> > +        ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > +        if (ret) {
> > +            if (ret != -EINVAL) {
> > +                fprintf(stderr, "%s: %s\n", __func__, strerror(-ret));
> >              }
> > +            return;
> >          }
> >      }
> >  }
> > -- 
> > 1.7.7.6
> 
> ACK
> 
> Avi, please merge through uq/master.

NACK, guest should be notified when the VM is starting, not
when stopping.

  reply	other threads:[~2012-09-30 23:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-20  8:25 [PATCH 1/1] kvmclock: fix guest stop notification Amit Shah
2012-09-20  8:25 ` [Qemu-devel] " Amit Shah
2012-09-20 12:46 ` Marcelo Tosatti
2012-09-20 12:46   ` [Qemu-devel] " Marcelo Tosatti
2012-09-30 23:05   ` Marcelo Tosatti [this message]
2012-09-30 23:05     ` Marcelo Tosatti
2012-10-01  1:50     ` Amos Kong
2012-10-01  1:50       ` [Qemu-devel] " Amos Kong
2012-10-03 13:28       ` Marcelo Tosatti
2012-10-03 13:28         ` [Qemu-devel] " Marcelo Tosatti
2012-10-08 10:43       ` Amit Shah
2012-10-08 10:43         ` [Qemu-devel] " Amit Shah
2012-10-08 10:40     ` Amit Shah
2012-10-08 10:40       ` [Qemu-devel] " Amit Shah
2012-09-20 12:47 ` Marcelo Tosatti
2012-09-20 12:47   ` [Qemu-devel] " Marcelo Tosatti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120930230516.GA3923@amt.cnet \
    --to=mtosatti@redhat.com \
    --cc=afaerber@suse.de \
    --cc=amit.shah@redhat.com \
    --cc=emunson@mgebm.net \
    --cc=kvm@vger.kernel.org \
    --cc=lersek@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.