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. Fam