[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/15] scsi: Implement alloc_req_iov callback
From: |
Christoph Hellwig |
Subject: |
Re: [Qemu-devel] [PATCH 13/15] scsi: Implement alloc_req_iov callback |
Date: |
Thu, 25 Nov 2010 16:29:22 +0100 |
User-agent: |
Mutt/1.3.28i |
On Thu, Nov 25, 2010 at 09:53:25AM +0100, Hannes Reinecke wrote:
> Looked into it.
> Sure I could be doing it for scsi-disk; for scsi-generic it won't
> work, though. And it's not much of a code-share to have from it;
> you'll end up with something like:
Yes, and that is a good start to completely get rid of the non-iovec
version. Keeping two parallel APIs around that have slighly mismatching
semantics is a bad idea. And for scsi-generic in the version in your
the difference is even worse and more subtile.
I think the only way to get this interface future proof is to unify
it. That is always pass an iovec from the HBA driver, and make the
length chunking an explicitly flag passed to ->alloc_req instead of
an implicit one when using the old interface. Then refactor the code
currently resetting.
Talking about scsi-generic, how is the auto request-sense in
scsi_read_data and the mode select snooping in scsi_write_complete
supposed to for for the iovec interface?
> And for the naming:
> The SCSI stack is using 'req' for every function accepting
> SCSIRequest as an argument:
>
> hw/scsi.h:
> SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d,
> uint32_t tag, uint32_t lun);
> void scsi_req_free(SCSIRequest *req);
>
> int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
> void scsi_req_print(SCSIRequest *req);
> void scsi_req_complete(SCSIRequest *req);
>
> So using 'alloc_req' and 'free_req' is in line with this naming
> scheme. Using 'alloc_request' and 'free_request' really looked odd
> in the light of the general usage.
Keeping the method names is fine, but please name the implementations
matching it, e.g.
scsi_alloc_req and co.
And yes, using the scsi_ prefix is a bit confusing with the generic
code also using it, eventually the disk driver should use scsi_disk
and the generic driver scsi_generic prefixes.
And in general it would be nice if you could simplify answer to reviews.
If something that the reviewer requests doesn't make sense state so in
reply instead of silently ignoring it.
- [Qemu-devel] [PATCH 07/15] lsi53c895a: Rename 'sense' to 'status', (continued)
- [Qemu-devel] [PATCH 07/15] lsi53c895a: Rename 'sense' to 'status', Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 05/15] scsi-disk: Remove duplicate cdb parsing, Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 02/15] scsi: Return SAM status codes, Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 01/15] scsi: Increase the number of possible devices, Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 08/15] scsi-disk: Allocate iovec dynamically, Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 12/15] scsi: Implement 'get_sense' callback, Hannes Reinecke, 2010/11/24
- [Qemu-devel] [PATCH 13/15] scsi: Implement alloc_req_iov callback, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 04/15] scsi: Move sense handling into the driver, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 15/15] Make SCSI HBA configurable, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 10/15] scsi-disk: add data direction checking, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 09/15] scsi: Use 'SCSIRequest' directly, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 06/15] scsi: Update sense code handling, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 11/15] Remove 'bus' argument from SCSI command completion callbacks, Hannes Reinecke, 2010/11/24
[Qemu-devel] [PATCH 14/15] megasas: LSI Megaraid SAS emulation, Hannes Reinecke, 2010/11/24