From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39185)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <prvs=017ac68b9=wei.liu2@citrix.com>)
	id 1bVF6b-0002af-Iz
	for qemu-devel@nongnu.org; Thu, 04 Aug 2016 05:38:22 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <prvs=017ac68b9=wei.liu2@citrix.com>)
	id 1bVF6X-0003zB-Cu
	for qemu-devel@nongnu.org; Thu, 04 Aug 2016 05:38:20 -0400
Received: from smtp02.citrix.com ([66.165.176.63]:59484)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <prvs=017ac68b9=wei.liu2@citrix.com>)
	id 1bVF6X-0003z3-84
	for qemu-devel@nongnu.org; Thu, 04 Aug 2016 05:38:17 -0400
Date: Thu, 4 Aug 2016 10:38:13 +0100
From: Wei Liu <wei.liu2@citrix.com>
Message-ID: <20160804093813.GB32096@citrix.com>
References: <1470146790-6168-1-git-send-email-paulinaszubarczyk@gmail.com>
	<1470146790-6168-2-git-send-email-paulinaszubarczyk@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1470146790-6168-2-git-send-email-paulinaszubarczyk@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v4 1/2] Interface for grant copy operation
 in libs.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
Cc: xen-devel@lists.xenproject.org, roger.pau@citrix.com, wei.liu2@citrix.com, ian.jackson@eu.citrix.com, david.vrabel@citrix.com, sstabellini@kernel.org, anthony.perard@citrix.com, qemu-devel@nongnu.org

The code looks ok. I have two minor suggestions below.

I would suggest changing the subject line to:

  libs/gnttab: introduce grant copy interface

On Tue, Aug 02, 2016 at 04:06:29PM +0200, Paulina Szubarczyk wrote:
> In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
> system call is invoked. In mini-os the operation is yet not
> implemented. For the OSs that does not implement gnttab the
> call of the grant copy operation causes abort.
> 
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
> ---
> Changes since v3:
> - revert to cast from xengnttab_grant_copy_segment_t
>   to ioctl_gntdev_grant_copy.
> - added compile-time check to compare the libs
>   xengnttab_grant_copy_segment_t with the ioctl structure.
>   The patch relies on Wei patch introducing XENGNTTAB_BUILD_BUG_ON  
>   in libs/gnttab.

I should resubmit that one soon.

> ---
[...]
> +    rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, &copy);
> +    if (rc) 
> +    {
> +        GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
> +    }

Normally for a single statement you don't need {} around it.

No need to resubmit just because of this patch. I can handle the subject
line change,  fix up the style issue and change the comment according to
David's suggestion while committing if you don't object to any of them.

I won't commit this patch right away though. I will wait until the QEMU
patch is acked because I would avoid committing things that have no
users.

If you end up submitting another version you can make those changes
yourself.


Wei.