Il 16/09/2014 09:16, Fam Zheng ha scritto: > On Mon, 09/15 12:17, Paolo Bonzini wrote: >> Il 15/09/2014 07:23, Fam Zheng ha scritto: >>> SCSIRequest *sreq; >>> size_t resp_size; >>> enum SCSIXferMode mode; >>> - QEMUIOVector resp_iov; >>> union { >>> VirtIOSCSICmdResp cmd; >>> VirtIOSCSICtrlTMFResp tmf; >>> @@ -68,23 +75,27 @@ static inline SCSIDevice >>> *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) >>> static VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq) >>> { >>> VirtIOSCSIReq *req; >>> - VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); >>> - >>> - req = g_malloc0(sizeof(*req) + vs->cdb_size); >>> + VirtIOSCSICommon *vs = (VirtIOSCSICommon *)s; >>> + const size_t zero_skip = offsetof(VirtIOSCSIReq, elem) >>> + + sizeof(VirtQueueElement); >>> >>> + req = g_slice_alloc(sizeof(*req) + vs->cdb_size); >> >> Looks good, but why do you need to zero the union? You only need to >> zero sreq, resp_size and mode, don't you (and at this point, memset >> becomes superfluous)? >> > > The structures in unions are not zeroed by caller, also leaving them breaks > virtio-scsi in my test. > > FWIW, I will remove the "req->sreq = NULL;" two lines below in v3. At this > point tuning these small fields are subtle optimization compared to the > arrays, > I say let's just simply keep the memset so that adding more fields in the > future are also safe.
Perhaps the response fields have to be zeroed? The request shouldn't need it. It can be done separately though---the VirtQueueElement is the big one that we have to fix. Paolo