[Qemu-devel] [PATCH -V3 20/32] virtio-9p: Remove BUG_ON and add proper error handling

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <address@hidden>
---
 hw/virtio-9p.c |  106 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 84 insertions(+), 22 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 1237bac..3ce26ca 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -244,7 +244,6 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
         return NULL;
 
     f = qemu_mallocz(sizeof(V9fsFidState));
-    BUG_ON(f == NULL);
 
     f->fid = fid;
     f->fd = -1;
@@ -320,15 +319,18 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID 
*qidp)
         qidp->type |= P9_QID_TYPE_SYMLINK;
 }
 
-static void fid_to_qid(V9fsState *s, V9fsFidState *fidp, V9fsQID *qidp)
+static int fid_to_qid(V9fsState *s, V9fsFidState *fidp, V9fsQID *qidp)
 {
     struct stat stbuf;
     int err;
 
     err = posix_lstat(s, &fidp->path, &stbuf);
-    BUG_ON(err == -1);
+    if (err) {
+        return err;
+    }
 
     stat_to_qid(&stbuf, qidp);
+    return 0;
 }
 
 static V9fsPDU *alloc_pdu(V9fsState *s)
@@ -653,7 +655,7 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf)
     return mode;
 }
 
-static void stat_to_v9stat(V9fsState *s, V9fsString *name,
+static int stat_to_v9stat(V9fsState *s, V9fsString *name,
                             const struct stat *stbuf,
                             V9fsStat *v9stat)
 {
@@ -681,7 +683,10 @@ static void stat_to_v9stat(V9fsState *s, V9fsString *name,
 
         if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
             err = posix_readlink(s, name, &v9stat->extension);
-            BUG_ON(err == -1);
+            if (err == -1) {
+                err = -errno;
+                return err;
+            }
             v9stat->extension.data[err] = 0;
             v9stat->extension.size = err;
         } else if (v9stat->mode & P9_STAT_MODE_DEVICE) {
@@ -708,6 +713,7 @@ static void stat_to_v9stat(V9fsState *s, V9fsString *name,
         v9fs_string_size(&v9stat->gid) +
         v9fs_string_size(&v9stat->muid) +
         v9fs_string_size(&v9stat->extension);
+    return 0;
 }
 
 static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
@@ -745,7 +751,12 @@ static void v9fs_attach(V9fsState *s, V9fsPDU *pdu)
     fidp->uid = n_uname;
 
     v9fs_string_sprintf(&fidp->path, "%s", "/");
-    fid_to_qid(s, fidp, &qid);
+    err = fid_to_qid(s, fidp, &qid);
+    if (err) {
+        err = -EINVAL;
+        free_fid(s, fid);
+        goto out;
+    }
 
     offset += pdu_marshal(pdu, offset, "Q", &qid);
 
@@ -772,7 +783,10 @@ static void v9fs_stat_post_lstat(V9fsState *s, 
V9fsStatState *vs, int err)
         goto out;
     }
 
-    stat_to_v9stat(s, &vs->fidp->path, &vs->stbuf, &vs->v9stat);
+    err = stat_to_v9stat(s, &vs->fidp->path, &vs->stbuf, &vs->v9stat);
+    if (err) {
+        goto out;
+    }
     vs->offset += pdu_marshal(vs->pdu, vs->offset, "wS", 0, &vs->v9stat);
     err = vs->offset;
 
@@ -925,10 +939,8 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
 
     if(vs->nwnames) {
         vs->wnames = qemu_mallocz(sizeof(vs->wnames[0]) * vs->nwnames);
-        BUG_ON(vs->wnames == NULL);
 
         vs->qids = qemu_mallocz(sizeof(vs->qids[0]) * vs->nwnames);
-        BUG_ON(vs->qids == NULL);
 
         for (i = 0; i < vs->nwnames; i++) {
             vs->offset += pdu_unmarshal(vs->pdu, vs->offset, "s",
@@ -1070,7 +1082,10 @@ out:
 
 static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)
 {
-    BUG_ON(err == -1);
+    if (err) {
+        err = -errno;
+        goto out;
+    }
 
     stat_to_qid(&vs->stbuf, &vs->qid);
 
@@ -1082,7 +1097,10 @@ static void v9fs_open_post_lstat(V9fsState *s, 
V9fsOpenState *vs, int err)
                                     omode_to_uflags(vs->mode));
         v9fs_open_post_open(s, vs, err);
     }
-
+    return;
+out:
+    complete_pdu(s, vs->pdu, err);
+    qemu_free(vs);
 }
 
 static void v9fs_open(V9fsState *s, V9fsPDU *pdu)
@@ -1186,11 +1204,15 @@ static void v9fs_read_post_readdir(V9fsState *, 
V9fsReadState *, ssize_t );
 
 static void v9fs_read_post_seekdir(V9fsState *s, V9fsReadState *vs, ssize_t 
err)
 {
+    if (err) {
+        goto out;
+    }
     v9fs_stat_free(&vs->v9stat);
     v9fs_string_free(&vs->name);
     vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->count);
     vs->offset += vs->count;
     err = vs->offset;
+out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
     return;
@@ -1199,8 +1221,14 @@ static void v9fs_read_post_seekdir(V9fsState *s, 
V9fsReadState *vs, ssize_t err)
 static void v9fs_read_post_dir_lstat(V9fsState *s, V9fsReadState *vs,
                                     ssize_t err)
 {
-    BUG_ON(err == -1);
-    stat_to_v9stat(s, &vs->name, &vs->stbuf, &vs->v9stat);
+    if (err) {
+           err = -errno;
+           goto out;
+    }
+    err = stat_to_v9stat(s, &vs->name, &vs->stbuf, &vs->v9stat);
+    if (err) {
+        goto out;
+    }
 
     vs->len = pdu_marshal(vs->pdu, vs->offset + 4 + vs->count, "S",
                             &vs->v9stat);
@@ -1217,6 +1245,11 @@ static void v9fs_read_post_dir_lstat(V9fsState *s, 
V9fsReadState *vs,
     vs->dent = posix_readdir(s, vs->fidp->dir);
     v9fs_read_post_readdir(s, vs, err);
     return;
+out:
+    posix_seekdir(s, vs->fidp->dir, vs->dir_pos);
+    v9fs_read_post_seekdir(s, vs, err);
+    return;
+
 }
 
 static void v9fs_read_post_readdir(V9fsState *s, V9fsReadState *vs, ssize_t 
err)
@@ -1256,7 +1289,11 @@ static void v9fs_read_post_rewinddir(V9fsState *s, 
V9fsReadState *vs,
 
 static void v9fs_read_post_readv(V9fsState *s, V9fsReadState *vs, ssize_t err)
 {
-    BUG_ON(vs->len < 0);
+    if (err  < 0) {
+        /* IO error return the error */
+        err = -errno;
+        goto out;
+    }
     vs->total += vs->len;
     vs->sg = adjust_sg(vs->sg, vs->len, &vs->cnt);
     if (vs->total < vs->count && vs->len > 0) {
@@ -1266,19 +1303,27 @@ static void v9fs_read_post_readv(V9fsState *s, 
V9fsReadState *vs, ssize_t err)
             }
             vs->len = posix_readv(s, vs->fidp->fd, vs->sg, vs->cnt);
         } while (vs->len == -1 && errno == EINTR);
+        if (vs->len == -1) {
+            err  = -errno;
+        }
         v9fs_read_post_readv(s, vs, err);
         return;
     }
     vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->total);
     vs->offset += vs->count;
     err = vs->offset;
+
+out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
 }
 
 static void v9fs_read_post_lseek(V9fsState *s, V9fsReadState *vs, ssize_t err)
 {
-    BUG_ON(err == -1);
+    if (err == -1) {
+        err = -errno;
+        goto out;
+    }
     vs->sg = cap_sg(vs->sg, vs->count, &vs->cnt);
 
     if (vs->total < vs->count) {
@@ -1288,9 +1333,15 @@ static void v9fs_read_post_lseek(V9fsState *s, 
V9fsReadState *vs, ssize_t err)
             }
             vs->len = posix_readv(s, vs->fidp->fd, vs->sg, vs->cnt);
         } while (vs->len == -1 && errno == EINTR);
+       if (vs->len == -1) {
+            err  = -errno;
+        }
         v9fs_read_post_readv(s, vs, err);
         return;
     }
+out:
+    complete_pdu(s, vs->pdu, err);
+    qemu_free(vs);
 }
 
 static void v9fs_read(V9fsState *s, V9fsPDU *pdu)
@@ -1370,7 +1421,11 @@ typedef struct V9fsWriteState {
 static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs,
                                    ssize_t err)
 {
-    BUG_ON(vs->len < 0);
+    if (err  < 0) {
+        /* IO error return the error */
+        err = -errno;
+        goto out;
+    }
     vs->total += vs->len;
     vs->sg = adjust_sg(vs->sg, vs->len, &vs->cnt);
     if (vs->total < vs->count && vs->len > 0) {
@@ -1379,19 +1434,26 @@ static void v9fs_write_post_writev(V9fsState *s, 
V9fsWriteState *vs,
                 print_sg(vs->sg, vs->cnt);
             vs->len =  posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt);
         } while (vs->len == -1 && errno == EINTR);
+        if (vs->len == -1) {
+            err  = -errno;
+        }
         v9fs_write_post_writev(s, vs, err);
+       return;
     }
     vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->total);
 
     err = vs->offset;
+out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
 }
 
 static void v9fs_write_post_lseek(V9fsState *s, V9fsWriteState *vs, ssize_t 
err)
 {
-    BUG_ON(err == -1);
-
+    if (err == -1) {
+        err = -errno;
+        goto out;
+    }
     vs->sg = cap_sg(vs->sg, vs->count, &vs->cnt);
 
     if (vs->total < vs->count) {
@@ -1400,11 +1462,14 @@ static void v9fs_write_post_lseek(V9fsState *s, 
V9fsWriteState *vs, ssize_t err)
                 print_sg(vs->sg, vs->cnt);
             vs->len = posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt);
         } while (vs->len == -1 && errno == EINTR);
-
+       if (vs->len == -1) {
+            err  = -errno;
+        }
         v9fs_write_post_writev(s, vs, err);
         return;
     }
 
+out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
 }
@@ -1834,7 +1899,6 @@ static void v9fs_wstat_post_chown(V9fsState *s, 
V9fsWstatState *vs, int err)
             end = old_name;
 
         new_name = qemu_malloc(end - old_name + vs->v9stat.name.size + 1);
-        BUG_ON(new_name == NULL);
 
         memset(new_name, 0, end - old_name + vs->v9stat.name.size + 1);
         memcpy(new_name, old_name, end - old_name);
@@ -2104,7 +2168,6 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)
     s->pdus[i].next = NULL;
 
     s->vq = virtio_add_queue(&s->vdev, MAX_REQ, handle_9p_output);
-    BUG_ON(s->vq == NULL);
 
     if (!conf->share_path || !conf->tag) {
            /* we haven't specified a mount_tag */
@@ -2127,7 +2190,6 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)
     s->tag = qemu_malloc(len);
     memcpy(s->tag, conf->tag, len);
     s->tag_len = len;
-    BUG_ON(s->fs_root == NULL);
     s->uid = -1;
 
     s->ops = virtio_9p_init_local(conf->share_path);
-- 
1.7.0.2.323.g0d092