Hello Dmitry, +-- On Tue, 13 Sep 2016, Dmitry Fleytman wrote --+ | Better put here: | | if(elemcnt++ >= PVSCSI_MAX_SG_ELEM) { | return; | } | | And ditch additional conditions from while clauses. | This way you’ll have less compare operations and avoid | adding the same element twice when leaving because of too long SG list.
Okay. | Also this is not a good idea to send truncated requests | when SG list becomes too long. I’d prefer to return error | code from pvscsi_convert_sglist() and handle it as appropriate. This could be a separate patch. Should pvscsi_build_sglist() destroy the sglist on error? @@ -656,7 +660,9 @@ pvscsi_build_sglist(PVSCSIState *s, PVSCSIRequest *r) pci_dma_sglist_init(&r->sgl, d, 1); if (r->req.flags & PVSCSI_FLAG_CMD_WITH_SG_LIST) { - pvscsi_convert_sglist(r); + if (pvscsi_convert_sglist(r) < 0) { + qemu_sglist_destroy(&r->sgl); + } } else { qemu_sglist_add(&r->sgl, r->req.dataAddr, r->req.dataLen); } I'll send both patches together once you confirm. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F