From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 4/8] KVM: Add a route layer to convert MSI message to GSI Date: Sun, 28 Dec 2008 19:14:51 +0800 Message-ID: <20081228111451.GA2610@syang10-desktop> 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> <20081227192725.GB3715@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mga09.intel.com ([134.134.136.24]:11660 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753568AbYL1LO5 (ORCPT ); Sun, 28 Dec 2008 06:14:57 -0500 Content-Disposition: inline In-Reply-To: <20081227192725.GB3715@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, Dec 27, 2008 at 05:27:25PM -0200, Marcelo Tosatti wrote: > > > > > +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? > Oh, Marcelo, finally I got your meaning. I misunderstand you at first time. And now I think you just means: Why "if (r == 0)" is there? It's unnecessary! Yes your are right. It can be called a little habit, and preseved value is useless at all, and maybe I just want to indicate what's the legal return value here. And from the other side, "if (r != 0)", the value should be meaningless and assigning it to another variable may not proper. -- regards Yang, Sheng