From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 4/8] KVM: Add a route layer to convert MSI message to
	GSI
Date: Sat, 27 Dec 2008 17:27:25 -0200
Message-ID: <20081227192725.GB3715@amt.cnet>
References: <1230019231-16543-1-git-send-email-sheng@linux.intel.com> <20081223175542.GB5449@amt.cnet> <200812241031.01195.sheng@linux.intel.com> <200812250959.46479.sheng@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org
To: Sheng Yang <sheng@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:41493 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753509AbYL0T1i (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 27 Dec 2008 14:27:38 -0500
Content-Disposition: inline
In-Reply-To: <200812250959.46479.sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, Dec 25, 2008 at 09:59:45AM +0800, Sheng Yang wrote:
> > > > +	found_msg = kvm_find_gsi_msg(kvm, gsi_msg->gsi);
> > > > +	if (found_msg)
> > > > +		*found_msg = *gsi_msg;
> > > > +	else {
> > > > +		gsi = find_first_zero_bit(kvm->gsi_msg_bitmap, KVM_NR_GSI_MSG);
> > > > +		if (gsi >= KVM_NR_GSI_MSG) {
> > > > +			r = -EFAULT;
> > >
> > > ENOSPC?
> >
> > OK. (Though I am confusing with all kinds of ERR all the time, ENOSPC show
> > "No space left in the device"... And last time somebody told me "ENOTTY"
> > means something is not available...)

Not sure what the most appropriate error is here.

> >
> > > > +static int kvm_vm_ioctl_request_gsi_msg(struct kvm *kvm,
> > > > +					struct kvm_assigned_gsi_msg *agsi_msg)
> > > > +{
> > > > +	struct kvm_gsi_msg gsi_msg;
> > > > +	int r;
> > > > +
> > > > +	gsi_msg.gsi = agsi_msg->gsi;
> > > > +	gsi_msg.msg.address_lo = agsi_msg->msg.addr_lo;
> > > > +	gsi_msg.msg.address_hi = agsi_msg->msg.addr_hi;
> > > > +	gsi_msg.msg.data = agsi_msg->msg.data;
> > > > +
> > > > +	r = kvm_update_gsi_msg(kvm, &gsi_msg);
> > > > +	if (r == 0)
> > > > +		agsi_msg->gsi = gsi_msg.gsi;
> > > > +	return r;
> > > > +}
> > >
> > > Can't see the purpose of this function. Why preserve the user-passed GSI
> > > value in case of failure? It will return an error anyway...
> >
> Oh, we preserved the user-passed GSI in case of userspace want to update one 
> entry. :)

The structure is not copied back to userspace in case of failure anyway:

+           r = kvm_vm_ioctl_request_gsi_msg(kvm, &agsi_msg);
+               if (r)
+                       goto out;
+               r = -EFAULT;
+           if (copy_to_user(argp, &agsi_msg, sizeof agsi_msg))
+                   goto out;

So I don't see the point of doing this?