Re: [PATCH 2/5] reuse kvm_vm_ioctl

Glauber Costa
2009-07-17T16:41:20Z

On Fri, Jul 17, 2009 at 12:57:38PM -0300, Marcelo Tosatti wrote:
> On Fri, Jul 17, 2009 at 12:46:23PM -0300, Marcelo Tosatti wrote:
> > On Fri, Jul 17, 2009 at 12:49:04PM -0300, Glauber Costa wrote:
> > > On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote:
> > > > On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > > > > Start using kvm_vm_ioctl's code.
> > > > > For type safety, delete vm_fd from kvm_context entirely, so the
> > > > > compiler can play along with us helping to detect errors I might
> > > > > have made.
> > > > >
> > > > > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > > > > ---
> > > > > kvm-all.c | 2 ++
> > > > > qemu-kvm-x86.c | 18 +++++++++---------
> > > > > qemu-kvm.c | 52 ++++++++++++++++++++++++++--------------------------
> > > > > qemu-kvm.h | 6 +++---
> > > > > 4 files changed, 40 insertions(+), 38 deletions(-)
> > > >
> > > > <snip>
> > > >
> > > > > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> > > > > vcpu_ctx->kvm = kvm;
> > > > > vcpu_ctx->id = id;
> > > > >
> > > > > - r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > > > > + r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> > > > > if (r == -1) {
> > > > > fprintf(stderr, "kvm_create_vcpu: %m\n");
> > > >
> > > >
> > > > int kvm_vm_ioctl(KVMState *s, int type, ...)
> > > > {
> > > > ...
> > > > if (ret == -1)
> > > > ret = -errno;
> > > > }
> > > >
> > > > Is that fine?
> > > I don't see a problem with that.
> >
> > Problem is the sites in qemu-kvm.c kvm_vm_ioctl to return -1
> > on error, but kvm_vm_ioctl converts -1 to -errno, no?
>
> BTW kvm-all.c:
>
> if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
> dprintf("ioctl failed %d\n", errno);
> ret = -1;
> break;
> }
gee, that's a real bug.

Maybe we should in fact change kvm-all to stop doing this altogether.
Anthony?