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

[Laurent V. <Lau...@bu...>]

Le lundi 07 janvier 2008 =C3=A0 11:27 +0200, Avi Kivity a =C3=A9crit :
> Carlo Marcelo Arenas Belon wrote:
> > revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 th=
at
> > redefined sector as an array of pointers to char, instead of a statical=
ly
> > 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 po=
inter type
> > block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible p=
ointer type
> >
> > Signed-off-by: Carlo Marcelo Arenas Belon <ca...@sa...>
> >  =20
>=20
> Doesn't the cache=3Doff option warrant an allocation here to ensure=20
> 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 <Lau...@bu...>
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 =3D bs->drv;
     int64_t i, total_sectors;
     int n, j;
-    unsigned char *sector[512];
+    unsigned char *sector;
=20
     if (!drv)
         return -ENOMEDIUM;
@@ -473,15 +473,21 @@ int bdrv_commit(BlockDriverState *bs)
        return -ENOTSUP;
     }
=20
+    sector =3D qemu_memalign(512,512);
+    if (sector =3D=3D NULL)
+        return -ENOMEM;
+
     total_sectors =3D bdrv_getlength(bs) >> SECTOR_BITS;
     for (i =3D 0; i < total_sectors;) {
         if (drv->bdrv_is_allocated(bs, i, 65536, &n)) {
             for(j =3D 0; j < n; j++) {
                 if (bdrv_read(bs, i, sector, 1) !=3D 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
=20
                 if (bdrv_write(bs->backing_hd, i, sector, 1) !=3D 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
                 i++;
@@ -491,6 +497,7 @@ int bdrv_commit(BlockDriverState *bs)
         }
     }
=20
+    qemu_free(sector);
     if (drv->bdrv_make_empty)
        return drv->bdrv_make_empty(bs);

--=20
----------------- Lau...@bu...  ------------------
  "La perfection est atteinte non quand il ne reste rien =C3=A0
ajouter mais quand il ne reste rien =C3=A0 enlever." Saint Exup=C3=A9ry