On Mon, Jun 11, 2012 at 3:21 PM, Bharata B Rao
<bhar...@linux.vnet.ibm.com> wrote:
> +#include "block_int.h"
> +#include "gluster-helpers.h"
> +
> +typedef void *gluster_file_t;
This typedef is already in gluster-helpers.h. It's ugly BTW, "typedef
struct gluster_file gluster_file_t" is nicer since it won't cast to
other pointer types automatically.
> +
> +typedef struct glusterConf {
> + char volfile[PATH_MAX];
> + char image[PATH_MAX];
> +} glusterConf;
QEMU coding style always uses UpperCase for struct names.
> +static void qemu_gluster_aio_event_reader(void *opaque)
> +{
> + BDRVGlusterState *s = opaque;
> + ssize_t ret;
> +
> + do {
> + char *p = (char *)&s->event_gaiocb;
Why make this a BDRVGlusterState field? It could be a local, I think.
> + /* Use O_DSYNC for write-through caching, no flags for write-back
> caching,
> + * and O_DIRECT for no caching. */
> + if ((bdrv_flags & BDRV_O_NOCACHE))
> + s->open_flags |= O_DIRECT;
> + if (!(bdrv_flags & BDRV_O_CACHE_WB))
> + s->open_flags |= O_DSYNC;
Paolo has changed this recently, you might need to use
bs->enable_write_cache instead.
> +out:
> + if (c) {
> + g_free(c);
> + }
g_free(NULL) is a nop, you never need to test that the pointer is non-NULL.
> +static void gluster_finish_aiocb(void *arg)
> +{
> + int ret;
> + gluster_aiocb_t *gaiocb = (gluster_aiocb_t *)arg;
> + BDRVGlusterState *s = ((glusterAIOCB *)gaiocb->opaque)->s;
> +
> + ret = qemu_gluster_send_pipe(s, gaiocb);
> + if (ret < 0) {
> + g_free(gaiocb);
What about the glusterAIOCB? You need to invoke the callback with an
error value.
What about decrementing the in-flight I/O request count?
> +static BlockDriverAIOCB *qemu_gluster_aio_rw(BlockDriverState *bs,
> + int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> + BlockDriverCompletionFunc *cb, void *opaque, int write)
> +{
> + int ret;
> + glusterAIOCB *acb;
> + gluster_aiocb_t *gaiocb;
> + BDRVGlusterState *s = bs->opaque;
> + char *buf;
> + size_t size;
> + off_t offset;
> +
> + acb = qemu_aio_get(&gluster_aio_pool, bs, cb, opaque);
> + acb->write = write;
> + acb->qiov = qiov;
> + acb->bounce = qemu_blockalign(bs, qiov->size);
> + acb->ret = 0;
> + acb->bh = NULL;
> + acb->s = s;
> +
> + if (write) {
> + qemu_iovec_to_buffer(acb->qiov, acb->bounce);
> + }
> +
> + buf = acb->bounce;
> + offset = sector_num * BDRV_SECTOR_SIZE;
> + size = nb_sectors * BDRV_SECTOR_SIZE;
> + s->qemu_aio_count++;
> +
> + gaiocb = g_malloc(sizeof(gluster_aiocb_t));
Can you make this a field of glusterAIOCB? Then you don't need to
worry about freeing gaiocb later.
> +static int64_t qemu_gluster_getlength(BlockDriverState *bs)
> +{
> + BDRVGlusterState *s = bs->opaque;
> + gluster_file_t fd = s->fd;
> + struct stat st;
> + int ret;
> +
> + ret = gluster_fstat(fd, &st);
> + if (ret < 0) {
> + return -1;
Please return a negative errno instead of -1.
Stefan
