Commit 1490791f614cfc0a80d47d5abd5d359d6b9256c6

Authored by aliguori
1 parent 492c30af

fix bdrv_aio_read API breakage in qcow2 (Andrea Arcangeli)

I noticed the qemu_aio_flush was doing nothing at all. And a flood of
cmd_writeb commands leading to a noop-invocation of qemu_aio_flush
were executed.

In short all 'memset;goto redo' places must be fixed to use the bh and
not to call the callback in the context of bdrv_aio_read or the
bdrv_aio_read model falls apart. Reading from qcow2 holes is possible
with phyisical readahead (kind of breada in linux buffer cache).

This is needed at least for scsi, ide is lucky (or it has been
band-aided against this API breakage by fixing the symptom and not the
real bug).

Same bug exists in qcow of course, can be fixed later as it's less
urgent.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>



git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@5574 c046a42c-6fe2-441c-8c8c-71466251a162
Showing 1 changed file with 40 additions and 4 deletions
block-qcow2.c
@@ -1165,8 +1165,18 @@ typedef struct QCowAIOCB { @@ -1165,8 +1165,18 @@ typedef struct QCowAIOCB {
1165 uint64_t cluster_offset; 1165 uint64_t cluster_offset;
1166 uint8_t *cluster_data; 1166 uint8_t *cluster_data;
1167 BlockDriverAIOCB *hd_aiocb; 1167 BlockDriverAIOCB *hd_aiocb;
  1168 + QEMUBH *bh;
1168 } QCowAIOCB; 1169 } QCowAIOCB;
1169 1170
  1171 +static void qcow_aio_read_cb(void *opaque, int ret);
  1172 +static void qcow_aio_read_bh(void *opaque)
  1173 +{
  1174 + QCowAIOCB *acb = opaque;
  1175 + qemu_bh_delete(acb->bh);
  1176 + acb->bh = NULL;
  1177 + qcow_aio_read_cb(opaque, 0);
  1178 +}
  1179 +
1170 static void qcow_aio_read_cb(void *opaque, int ret) 1180 static void qcow_aio_read_cb(void *opaque, int ret)
1171 { 1181 {
1172 QCowAIOCB *acb = opaque; 1182 QCowAIOCB *acb = opaque;
@@ -1182,7 +1192,6 @@ static void qcow_aio_read_cb(void *opaque, int ret) @@ -1182,7 +1192,6 @@ static void qcow_aio_read_cb(void *opaque, int ret)
1182 return; 1192 return;
1183 } 1193 }
1184 1194
1185 - redo:  
1186 /* post process the read buffer */ 1195 /* post process the read buffer */
1187 if (!acb->cluster_offset) { 1196 if (!acb->cluster_offset) {
1188 /* nothing to do */ 1197 /* nothing to do */
@@ -1223,12 +1232,30 @@ static void qcow_aio_read_cb(void *opaque, int ret) @@ -1223,12 +1232,30 @@ static void qcow_aio_read_cb(void *opaque, int ret)
1223 if (acb->hd_aiocb == NULL) 1232 if (acb->hd_aiocb == NULL)
1224 goto fail; 1233 goto fail;
1225 } else { 1234 } else {
1226 - goto redo; 1235 + if (acb->bh) {
  1236 + ret = -EIO;
  1237 + goto fail;
  1238 + }
  1239 + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb);
  1240 + if (!acb->bh) {
  1241 + ret = -EIO;
  1242 + goto fail;
  1243 + }
  1244 + qemu_bh_schedule(acb->bh);
1227 } 1245 }
1228 } else { 1246 } else {
1229 /* Note: in this case, no need to wait */ 1247 /* Note: in this case, no need to wait */
1230 memset(acb->buf, 0, 512 * acb->n); 1248 memset(acb->buf, 0, 512 * acb->n);
1231 - goto redo; 1249 + if (acb->bh) {
  1250 + ret = -EIO;
  1251 + goto fail;
  1252 + }
  1253 + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb);
  1254 + if (!acb->bh) {
  1255 + ret = -EIO;
  1256 + goto fail;
  1257 + }
  1258 + qemu_bh_schedule(acb->bh);
1232 } 1259 }
1233 } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) { 1260 } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
1234 /* add AIO support for compressed blocks ? */ 1261 /* add AIO support for compressed blocks ? */
@@ -1236,7 +1263,16 @@ static void qcow_aio_read_cb(void *opaque, int ret) @@ -1236,7 +1263,16 @@ static void qcow_aio_read_cb(void *opaque, int ret)
1236 goto fail; 1263 goto fail;
1237 memcpy(acb->buf, 1264 memcpy(acb->buf,
1238 s->cluster_cache + index_in_cluster * 512, 512 * acb->n); 1265 s->cluster_cache + index_in_cluster * 512, 512 * acb->n);
1239 - goto redo; 1266 + if (acb->bh) {
  1267 + ret = -EIO;
  1268 + goto fail;
  1269 + }
  1270 + acb->bh = qemu_bh_new(qcow_aio_read_bh, acb);
  1271 + if (!acb->bh) {
  1272 + ret = -EIO;
  1273 + goto fail;
  1274 + }
  1275 + qemu_bh_schedule(acb->bh);
1240 } else { 1276 } else {
1241 if ((acb->cluster_offset & 511) != 0) { 1277 if ((acb->cluster_offset & 511) != 0) {
1242 ret = -EIO; 1278 ret = -EIO;