On 3 August 2011 09:49, Paolo Bonzini <pbonz...@redhat.com> wrote:
> @@ -157,8 +172,22 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag,
> uint32_t lun,
> uint8_t *buf, void *hba_private)
> {
> SCSIRequest *req;
> - req = d->info->alloc_req(d, tag, lun, hba_private);
> - memcpy(req->cmd.buf, buf, 16);
> + SCSICommand cmd;
> +
> + if (scsi_req_parse(&cmd, d, buf) != 0) {
> + trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
> + req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun,
> hba_private);
> + } else {
> + trace_scsi_req_parsed(d->id, lun, tag, buf[0],
> + cmd.mode, cmd.xfer);
> + if (req->cmd.lba != -1) {
> + trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0],
> + cmd.lba);
> + }
> + req = d->info->alloc_req(d, tag, lun, hba_private);
> + }
> +
> + req->cmd = cmd;
> return req;
> }
Does it still make sense to set req->cmd to cmd (and to look at cmd
at all) in the case where scsi_req_parse() failed and might not have
actually initialised all of cmd? For instance the tracing code (added
to scsi_req_new() after this patch) looks at cmd.buf[] based on the
value of buf[0], which seems kind of fragile to me.
-- PMM
