* [Qemu-devel] [PATCH] scsi: Implement alloc_req_iov callback
@ 2010-11-22 10:15 Hannes Reinecke
2010-11-22 21:48 ` [Qemu-devel] " Stefan Hajnoczi
0 siblings, 1 reply; 4+ messages in thread
From: Hannes Reinecke @ 2010-11-22 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, nab, kraxel
Add callback to create a request with a predefined iovec.
This is required for drivers which can use the iovec
of a command directly.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
hw/scsi-disk.c | 25 +++++++++++++++++++++----
hw/scsi-generic.c | 44 ++++++++++++++++++++++++++++++++++----------
hw/scsi.h | 2 ++
3 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d1b7f74..88a2f74 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -96,14 +96,30 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
return req;
}
+static SCSIRequest *scsi_new_request_iovec(SCSIDevice *d, uint32_t tag,
+ uint32_t lun, struct iovec *iov, int iov_num)
+{
+ SCSIRequest *req;
+ SCSIDiskReq *r;
+
+ req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun);
+ r = DO_UPCAST(SCSIDiskReq, req, req);
+ r->iov = iov;
+ r->iov_num = iov_num;
+ r->iov_buf = NULL;
+ return req;
+}
+
static void scsi_remove_request(SCSIRequest *req)
{
SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
- qemu_vfree(r->iov);
- r->iov = NULL;
- qemu_vfree(r->iov_buf);
- r->iov_buf = NULL;
+ if (r->iov_buf) {
+ qemu_vfree(r->iov);
+ r->iov = NULL;
+ qemu_vfree(r->iov_buf);
+ r->iov_buf = NULL;
+ }
scsi_req_free(&r->req);
}
@@ -1207,6 +1223,7 @@ static SCSIDeviceInfo scsi_disk_info = {
.init = scsi_disk_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
+ .alloc_req_iov = scsi_new_request_iovec,
.free_req = scsi_remove_request,
.send_command = scsi_send_command,
.read_data = scsi_read_data,
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 5c0f6ab..7d30115 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -107,6 +107,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
return req;
}
+static SCSIRequest *scsi_new_request_iovec(SCSIDevice *d, uint32_t tag,
+ uint32_t lun, struct iovec *iov, int iov_num)
+{
+ SCSIRequest *req;
+ SCSIGenericReq *r;
+ int i;
+
+ req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
+ r = DO_UPCAST(SCSIGenericReq, req, req);
+ r->io_header.dxferp = iov;
+ r->io_header.iovec_count = iov_num;
+ r->io_header.dxfer_len = 0;
+ for (i = 0; i < iov_num; i++)
+ r->io_header.dxfer_len += iov[i].iov_len;
+ r->buf = NULL;
+ r->buflen = 0;
+ return req;
+}
+
static void scsi_remove_request(SCSIRequest *req)
{
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -179,8 +198,10 @@ static int execute_command(BlockDriverState *bdrv,
r->io_header.interface_id = 'S';
r->io_header.dxfer_direction = direction;
- r->io_header.dxferp = r->buf;
- r->io_header.dxfer_len = r->buflen;
+ if (r->buf) {
+ r->io_header.dxferp = r->buf;
+ r->io_header.dxfer_len = r->buflen;
+ }
r->io_header.cmdp = r->req.cmd.buf;
r->io_header.cmd_len = r->req.cmd.len;
r->io_header.mx_sb_len = sizeof(s->sensebuf);
@@ -286,7 +307,7 @@ static int scsi_write_data(SCSIRequest *req)
DPRINTF("scsi_write_data 0x%x\n", req->tag);
- if (r->len == 0) {
+ if (r->len == 0 && r->io_header.dxfer_len == 0) {
r->len = r->buflen;
r->req.bus->complete(&r->req, SCSI_REASON_DATA, r->len);
return 0;
@@ -380,14 +401,16 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
return 0;
}
- if (r->buflen != r->req.cmd.xfer) {
- if (r->buf != NULL)
- qemu_free(r->buf);
- r->buf = qemu_malloc(r->req.cmd.xfer);
- r->buflen = r->req.cmd.xfer;
- }
+ if (!r->io_header.iovec_count) {
+ if (r->buflen != r->req.cmd.xfer) {
+ if (r->buf != NULL)
+ qemu_free(r->buf);
+ r->buf = qemu_malloc(r->req.cmd.xfer);
+ r->buflen = r->req.cmd.xfer;
+ }
- memset(r->buf, 0, r->buflen);
+ memset(r->buf, 0, r->buflen);
+ }
r->len = r->req.cmd.xfer;
if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
r->len = 0;
@@ -560,6 +583,7 @@ static SCSIDeviceInfo scsi_generic_info = {
.init = scsi_generic_initfn,
.destroy = scsi_destroy,
.alloc_req = scsi_new_request,
+ .alloc_req_iov = scsi_new_request_iovec,
.free_req = scsi_remove_request,
.send_command = scsi_send_command,
.read_data = scsi_read_data,
diff --git a/hw/scsi.h b/hw/scsi.h
index 0c467d1..538ae54 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -72,6 +72,8 @@ struct SCSIDeviceInfo {
scsi_qdev_initfn init;
void (*destroy)(SCSIDevice *s);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
+ SCSIRequest *(*alloc_req_iov)(SCSIDevice *s, uint32_t tag, uint32_t lun,
+ struct iovec *iov, int iov_num);
void (*free_req)(SCSIRequest *req);
int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
void (*read_data)(SCSIRequest *req);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] Re: [PATCH] scsi: Implement alloc_req_iov callback
2010-11-22 10:15 [Qemu-devel] [PATCH] scsi: Implement alloc_req_iov callback Hannes Reinecke
@ 2010-11-22 21:48 ` Stefan Hajnoczi
2010-11-23 8:12 ` Hannes Reinecke
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2010-11-22 21:48 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: qemu-devel, nab, kraxel
On Mon, Nov 22, 2010 at 10:15 AM, Hannes Reinecke <hare@suse.de> wrote:
Looks good. If you send out another version of the patchset you might
like to fix this nitpick:
> + if (!r->io_header.iovec_count) {
> + if (r->buflen != r->req.cmd.xfer) {
> + if (r->buf != NULL)
> + qemu_free(r->buf);
qemu_free(NULL) is a nop so it's safe to drop the if (r->buf != NULL)
check and just use qemu_free(r->buf) unconditionally. That's nice
since it also fixes the if statement without curly braces.
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] scsi: Implement alloc_req_iov callback
2010-11-22 21:48 ` [Qemu-devel] " Stefan Hajnoczi
@ 2010-11-23 8:12 ` Hannes Reinecke
2010-11-23 8:31 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Hannes Reinecke @ 2010-11-23 8:12 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, nab, kraxel
On 11/22/2010 10:48 PM, Stefan Hajnoczi wrote:
> On Mon, Nov 22, 2010 at 10:15 AM, Hannes Reinecke <hare@suse.de> wrote:
> Looks good. If you send out another version of the patchset you might
> like to fix this nitpick:
>
>> + if (!r->io_header.iovec_count) {
>> + if (r->buflen != r->req.cmd.xfer) {
>> + if (r->buf != NULL)
>> + qemu_free(r->buf);
>
> qemu_free(NULL) is a nop so it's safe to drop the if (r->buf != NULL)
> check and just use qemu_free(r->buf) unconditionally. That's nice
> since it also fixes the if statement without curly braces.
>
Really?
qemu-malloc.c has:
void qemu_free(void *ptr)
{
trace_qemu_free(ptr);
free(ptr);
}
and 'free' doesn't normally do an error checking on the argument.
Am I missing something?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] Re: [PATCH] scsi: Implement alloc_req_iov callback
2010-11-23 8:12 ` Hannes Reinecke
@ 2010-11-23 8:31 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2010-11-23 8:31 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: Stefan Hajnoczi, qemu-devel, nab, kraxel
On 11/23/2010 09:12 AM, Hannes Reinecke wrote:
> qemu-malloc.c has:
>
> void qemu_free(void *ptr)
> {
> trace_qemu_free(ptr);
> free(ptr);
> }
>
>
> and 'free' doesn't normally do an error checking on the argument.
> Am I missing something?
It's not error checking: from free(3),
free() frees the memory space pointed to by ptr, which must have been
returned by a previous call to malloc(), calloc() or realloc().
Otherwise, or if free(ptr) has already been called before, undefined
behavior occurs. If ptr is NULL, no operation is performed.
Which means, that unless ptr is so often NULL that there is a measurable
overhead from the call (unlikely in any case, not just this one) the
"if" is actually going to be done by "free", and thus causing actually
worse performance.
Not that man pages are always right, but in this case they agree with
POSIX. :)
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-23 8:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-22 10:15 [Qemu-devel] [PATCH] scsi: Implement alloc_req_iov callback Hannes Reinecke
2010-11-22 21:48 ` [Qemu-devel] " Stefan Hajnoczi
2010-11-23 8:12 ` Hannes Reinecke
2010-11-23 8:31 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.