Re: [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

["Roger Pau Monné" <roger.pau@citrix.com>]

On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:
> Copy data operated on during request from/to local buffers to/from
> the grant references.
> 
> Before grant copy operation local buffers must be allocated what is
> done by calling ioreq_init_copy_buffers. For the 'read' operation,
> first, the qemu device invokes the read operation on local buffers
> and on the completion grant copy is called and buffers are freed.
> For the 'write' operation grant copy is performed before invoking
> write by qemu device.
> 
> A new value 'feature_grant_copy' is added to recognize when the
> grant copy operation is supported by a guest.
> The body of the function 'ioreq_runio_qemu_aio' is moved to
> 'ioreq_runio_qemu_aio_blk' and in the 'ioreq_runio_qemu_aio' depending
> on the support for grant copy according checks, initialization, grant
> operation are made, then the 'ioreq_runio_qemu_aio_blk' function is
> called.
> 
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
> ---
> Changes since v2:
> - to use the xengnttab_* function directly added -lxengnttab to configure
>   and include <xengnttab.h> in include/hw/xen/xen_common.h
> - in ioreq_copy removed an out path, changed a log level, made explicit 
>   assignement to 'xengnttab_copy_grant_segment'
> * I did not change the way of testing if grant_copy operation is implemented.
>   As far as I understand if the code from gnttab_unimp.c is used then the gnttab 
>   device is unavailable and the handler to gntdev would be invalid. But 
>   if the handler is valid then the ioctl should return operation unimplemented 
>   if the gntdev does not implement the operation.
> 
>  configure                   |   2 +-
>  hw/block/xen_disk.c         | 171 ++++++++++++++++++++++++++++++++++++++++----
>  include/hw/xen/xen_common.h |   2 +
>  3 files changed, 162 insertions(+), 13 deletions(-)

[...]
 
> @@ -1020,10 +1160,17 @@ static int blk_connect(struct XenDevice *xendev)
>  
>      xen_be_bind_evtchn(&blkdev->xendev);
>  
> +    blkdev->feature_grant_copy =
> +                (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);

Isn't this going to trigger an abort on OSes that don't implement 
xengnttab_grant_copy? AFAICT the 'unimplemented' handler in libgnttab for 
this is just an abort.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel