Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Mon, 2013-08-26 at 10:02 +0530, Nikunj A Dadhania wrote:

> 
> From: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> 
> This implements capabilities exchange between host and client.
> As at the moment no capability is supported, put zero flags everywhere
> and return.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hw/scsi/spapr_vscsi.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index e9090e5..fae3644 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -858,6 +858,40 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req *req)
>      return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT);
>  }
>  
> +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
> +{
> +    struct viosrp_capabilities *vcap;
> +    struct capabilities cap;
> +    uint16_t len = 0;

The above initialization isn't useful

> +    int rc = true;
> +
> +    vcap = &req->iu.mad.capabilities;
> +    len = be16_to_cpu(vcap->common.length);
> +    if (len > sizeof(&cap)) {
                        ^ Ugh ? Why the & here ?

> +        fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n");
> +        goto error_out;
> +    }

I am not 100% familiar with the protocol, could it be that we should
just read sizeof(cap) instead of erroring out or is there no way it
can be correct and have a len too long ?

> +    rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer),
> +                            &cap, len);
> +    if (rc)  {
> +        fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
> +    }
> +
> +    cap.flags = 0;
> +    cap.migration.ecl = 0;
> +    cap.reserve.type = 0;
> +    cap.migration.common.server_support = 0;
> +    cap.reserve.common.server_support = 0;
> +    rc = spapr_vio_dma_write(&s->vdev, be64_to_cpu(vcap->buffer),
> +                             &cap, len);
> +    if (rc)  {
> +        fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
> +    }
> +error_out:
> +    vcap->common.status = rc ? cpu_to_be32(1) : 0;
> +    return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT);
> +}
> +
>  static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
>  {
>      union mad_iu *mad = &req->iu.mad;
> @@ -878,6 +912,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
>          mad->host_config.common.status = cpu_to_be16(1);
>          vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT);
>          break;
> +    case VIOSRP_CAPABILITIES_TYPE:
> +        vscsi_send_capabilities(s, req);
> +        break;
>      default:
>          fprintf(stderr, "VSCSI: Unknown MAD type %02x\n",
>                  be32_to_cpu(mad->empty_iu.common.type));