Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit

[Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2483 bytes --]

Le lundi 07 janvier 2008 à 11:27 +0200, Avi Kivity a écrit :
> Carlo Marcelo Arenas Belon wrote:
> > revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
> > redefined sector as an array of pointers to char, instead of a statically
> > allocated buffer of chars, that was triggering the following warnings :
> >
> > block.c: In function `bdrv_commit':
> > block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
> > block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
> >
> > Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
> >   
> 
> Doesn't the cache=off option warrant an allocation here to ensure 
> alignment (or perhaps a 1K stack buffer with runtime adjustment)?

You're right, a good patch should be something like this (it is not
tested or even compiled) :

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
diff --git a/qemu/block.c b/qemu/block.c
index 519be24..c198659 100644
--- a/qemu/block.c
+++ b/qemu/block.c
@@ -460,7 +460,7 @@ int bdrv_commit(BlockDriverState *bs)
     BlockDriver *drv = bs->drv;
     int64_t i, total_sectors;
     int n, j;
-    unsigned char *sector[512];
+    unsigned char *sector;
 
     if (!drv)
         return -ENOMEDIUM;
@@ -473,15 +473,21 @@ int bdrv_commit(BlockDriverState *bs)
        return -ENOTSUP;
     }
 
+    sector = qemu_memalign(512,512);
+    if (sector == NULL)
+        return -ENOMEM;
+
     total_sectors = bdrv_getlength(bs) >> SECTOR_BITS;
     for (i = 0; i < total_sectors;) {
         if (drv->bdrv_is_allocated(bs, i, 65536, &n)) {
             for(j = 0; j < n; j++) {
                 if (bdrv_read(bs, i, sector, 1) != 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
 
                 if (bdrv_write(bs->backing_hd, i, sector, 1) != 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
                 i++;
@@ -491,6 +497,7 @@ int bdrv_commit(BlockDriverState *bs)
         }
     }
 
+    qemu_free(sector);
     if (drv->bdrv_make_empty)
        return drv->bdrv_make_empty(bs);

-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel