Re: [Qemu-devel] [PATCH v4 1/2] Interface for grant copy operation in libs.

[Wei Liu]

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 <address@hidden>
> ---
> 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.