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

[Nikunj A Dadhania]

Alexander Graf <address@hidden> writes:

> On 26.08.2013, at 13:46, Nikunj A Dadhania wrote:
>
>> Alexander Graf <address@hidden> writes:
>> 
>>> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote:
>>> 
>>>> 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 <address@hidden>
>>>> ---
>>>> +    rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
>>>> +    if (rc)  {
>>>> +        fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
>>> 
>>> At this point cap contains random host data, no?
>> 
>> Yes, and we zero it out in this case.
>
> Then please make this obvious to the reader. Either memset(0) it or do
> cap = { };. But do something that doesn't make me look up the header
> file to check whether you really did catch all the fields there are in
> the struct.

Here is the patch incorporating the comments, moreover goto is not there
anymore, we are being more accomodative and copying only the size(cap)
structure even if guest is requesting for more.

From: Nikunj A Dadhania <address@hidden>

This implements capabilities exchange between vscsi host and client.  As
at the moment no capability is supported, put zero flags everywhere and
return.

Signed-off-by: Nikunj A Dadhania <address@hidden>
---
 hw/scsi/spapr_vscsi.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index e9090e5..030b775 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -858,6 +858,57 @@ 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, req_len;
+    uint64_t buffer;
+    int rc;
+
+    vcap = &req->iu.mad.capabilities;
+    req_len = len = be16_to_cpu(vcap->common.length);
+    buffer = be64_to_cpu(vcap->buffer);
+    if (len > sizeof(cap)) {
+        fprintf(stderr, "vscsi_send_capabilities: capabilities size mismatch 
!\n");
+
+        /*
+         * Just read and populate the structure that is known.
+         * Zero rest of the structure.
+         */
+        len = sizeof(cap);
+    }
+    rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
+    if (rc)  {
+        fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
+    }
+
+    /*
+     * Current implementation does not suppport any migration or
+     * reservation capabilities. Construct the response telling the
+     * guest not to use them.
+     */
+    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, buffer, &cap, len);
+    if (rc)  {
+        fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
+    }
+    if (req_len > len) {
+        /*
+         * Being paranoid and lets not worry about the error code
+         * here. Actual write of the cap is done above.
+         */
+        spapr_vio_dma_set(&s->vdev, (buffer + len), 0, (req_len - len));
+    }
+    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 +929,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));
-- 
1.8.3.1