From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEFZJ-0005B0-5E for qemu-devel@nongnu.org; Tue, 27 Aug 2013 05:28:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEFZA-0007FN-B2 for qemu-devel@nongnu.org; Tue, 27 Aug 2013 05:28:09 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:60170) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEFZ9-0007EA-On for qemu-devel@nongnu.org; Tue, 27 Aug 2013 05:28:00 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 27 Aug 2013 19:24:37 +1000 From: Nikunj A Dadhania In-Reply-To: <1D179285-9F24-4E62-AED0-E5A181F6AED6@suse.de> References: <1377249737-12570-1-git-send-email-aik@ozlabs.ru> <24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de> <1377468637.3819.27.camel@pasglop> <87vc2tysur.fsf@linux.vnet.ibm.com> <874naczpk5.fsf@linux.vnet.ibm.com> <011735F8-0ED7-40CF-A23A-CE34C21F1D3C@suse.de> <87wqn8y8ra.fsf@linux.vnet.ibm.com> <421F6761-C72A-4BFD-9164-BFFB86E852A0@suse.de> <87y57nadua.fsf@abhimanyu.in.ibm.com> <1D179285-9F24-4E62-AED0-E5A181F6AED6@suse.de> Date: Tue, 27 Aug 2013 14:57:38 +0530 Message-ID: <87vc2ra3g5.fsf@abhimanyu.in.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Alexey Kardashevskiy , Paolo Bonzini , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" Alexander Graf writes: >> +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"); > > ... if we fail reading > >> + } >> + >> + /* >> + * 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); > > ... but then succeed writing back the empty structure we will report > "success". Is this correct? It's fine with me if it's intended, just > want to make sure we're making a conscious decision about it. Yes, that should be fine if it succeeds writing it. If DMA read/write is an issue, it should fail writing as well and we report back failure. > > The rest looks good to me. But please post new patch versions as new > top level emails, not as inline replies. My scripts use patchwork and > that can't really handle this case overly well (it works, but is > additional work). It also makes it hard for people who just follow the > thread loosely to realize that a new version is there. Sure, will take care next time. Regards Nikunj