Patchwork [1/5] use qemu_blockalign consistently

login
register
mail settings
Submitter Christoph Hellwig
Date Sept. 12, 2010, 9:42 p.m.
Message ID <20100912214256.GA4854@lst.de>
Download mbox | patch
Permalink /patch/64563/
State New
Headers show

Comments

Christoph Hellwig - Sept. 12, 2010, 9:42 p.m.
Use qemu_blockalign for all allocations in the block layer.  This allows
increasing the required alignment, which is need to support O_DIRECT on
devices with large block sizes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Stefan Hajnoczi - Sept. 13, 2010, 7:02 p.m.
On Sun, Sep 12, 2010 at 10:42 PM, Christoph Hellwig <hch@lst.de> wrote:
> Use qemu_blockalign for all allocations in the block layer.  This allows
> increasing the required alignment, which is need to support O_DIRECT on
> devices with large block sizes.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I noticed that the backing file may have different alignment
requirements than the image file but qemu_blockalign() currently only
takes into account the image file's requirements.  Not necessarily
something for this patch, but since you're in the area I wanted to
mention it.

It seems safe to take the max alignment (they should both be powers of
two I think) of the image and backing files.

Stefan
Christoph Hellwig - Sept. 14, 2010, 4:09 p.m.
On Mon, Sep 13, 2010 at 08:02:28PM +0100, Stefan Hajnoczi wrote:
> On Sun, Sep 12, 2010 at 10:42 PM, Christoph Hellwig <hch@lst.de> wrote:
> > Use qemu_blockalign for all allocations in the block layer. ?This allows
> > increasing the required alignment, which is need to support O_DIRECT on
> > devices with large block sizes.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I noticed that the backing file may have different alignment
> requirements than the image file but qemu_blockalign() currently only
> takes into account the image file's requirements.  Not necessarily
> something for this patch, but since you're in the area I wanted to
> mention it.
> 
> It seems safe to take the max alignment (they should both be powers of
> two I think) of the image and backing files.

Currently we check the required alignment based on the blocksize we
present to the guest, which is set by the driver.  That's a bit upside
down, but the best we can do.  The problem is that the blocksize of the
device the image (and it's backing file reside on) are the lower bound
of what we can support in the guest without massive read/modify/write
cycles.  If we use the page cache the kernel does the cycles for us,
but for cache=none we would have to do it ourselves in a rather
inefficient way.

Patch

Index: qemu/hw/scsi-disk.c
===================================================================
--- qemu.orig/hw/scsi-disk.c	2010-09-12 14:42:40.942759377 -0300
+++ qemu/hw/scsi-disk.c	2010-09-12 14:43:04.694759377 -0300
@@ -70,14 +70,15 @@  struct SCSIDiskState
     char *serial;
 };
 
-static SCSIDiskReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
+static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag,
+        uint32_t lun)
 {
     SCSIRequest *req;
     SCSIDiskReq *r;
 
-    req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun);
     r = DO_UPCAST(SCSIDiskReq, req, req);
-    r->iov.iov_base = qemu_memalign(512, SCSI_DMA_BUF_SIZE);
+    r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
     return r;
 }
 
@@ -939,7 +940,7 @@  static int32_t scsi_send_command(SCSIDev
     }
     /* ??? Tags are not unique for different luns.  We only implement a
        single lun, so this should not matter.  */
-    r = scsi_new_request(d, tag, lun);
+    r = scsi_new_request(s, tag, lun);
     outbuf = (uint8_t *)r->iov.iov_base;
     is_write = 0;
     DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
Index: qemu/hw/sd.c
===================================================================
--- qemu.orig/hw/sd.c	2010-09-12 14:31:04.387759376 -0300
+++ qemu/hw/sd.c	2010-09-12 14:43:04.695759377 -0300
@@ -440,7 +440,7 @@  SDState *sd_init(BlockDriverState *bs, i
     SDState *sd;
 
     sd = (SDState *) qemu_mallocz(sizeof(SDState));
-    sd->buf = qemu_memalign(512, 512);
+    sd->buf = qemu_blockalign(bs, 512);
     sd->spi = is_spi;
     sd->enable = 1;
     sd_reset(sd, bs);
Index: qemu/qemu-io.c
===================================================================
--- qemu.orig/qemu-io.c	2010-09-12 14:31:04.394759376 -0300
+++ qemu/qemu-io.c	2010-09-12 14:43:04.699759377 -0300
@@ -61,7 +61,7 @@  static void *qemu_io_alloc(size_t len, i
 
 	if (misalign)
 		len += MISALIGN_OFFSET;
-	buf = qemu_memalign(512, len);
+	buf = qemu_blockalign(bs, len);
 	memset(buf, pattern, len);
 	if (misalign)
 		buf += MISALIGN_OFFSET;
Index: qemu/qemu-nbd.c
===================================================================
--- qemu.orig/qemu-nbd.c	2010-09-12 14:31:04.401759376 -0300
+++ qemu/qemu-nbd.c	2010-09-12 14:43:04.706759377 -0300
@@ -446,7 +446,7 @@  int main(int argc, char **argv)
     max_fd = sharing_fds[0];
     nb_fds++;
 
-    data = qemu_memalign(512, NBD_BUFFER_SIZE);
+    data = qemu_blockalign(bs, NBD_BUFFER_SIZE);
     if (data == NULL)
         errx(EXIT_FAILURE, "Cannot allocate data buffer");
 
Index: qemu/posix-aio-compat.c
===================================================================
--- qemu.orig/posix-aio-compat.c	2010-09-12 14:42:46.725759377 -0300
+++ qemu/posix-aio-compat.c	2010-09-12 14:43:04.711759377 -0300
@@ -270,7 +270,7 @@  static ssize_t handle_aiocb_rw(struct qe
      * Ok, we have to do it the hard way, copy all segments into
      * a single aligned buffer.
      */
-    buf = qemu_memalign(512, aiocb->aio_nbytes);
+    buf = qemu_blockalign(aiocb->common.bs, aiocb->aio_nbytes);
     if (aiocb->aio_type & QEMU_AIO_WRITE) {
         char *p = buf;
         int i;