Re: [PATCH 4/8] KVM: Add a route layer to convert MSI message to GSI

[Marcelo Tosatti <mtosatti@redhat.com>]

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?