On Wed, Aug 01, 2012 at 06:35:22PM +0000, Blue Swirl wrote:
> > +
> > + if (!transport) {
> > + uri->transport = strdup("socket");
>
> g_strdup
Sorry about that, pitfalls of developing the parsing code out of line :(
> > +static int qemu_gluster_parseuri(GlusterURI *uri, const char *filename)
> > +{
> > + char *token, *saveptr;
> > + char *p, *r;
> > + int ret = -EINVAL;
> > +
> > + p = r = g_strdup(filename);
>
> Why?
- Are you asking why use 2 variables ? I need them because I loose p and
need r to free the string.
- Or are you asking why strdup ? That's because filename is const char *
and I need to modify the filename when parsing.
> > +static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void
> > *arg)
> > +{
> > + GlusterAIOCB *acb = (GlusterAIOCB *)arg;
> > + BDRVGlusterState *s = acb->common.bs->opaque;
> > +
> > + acb->ret = ret;
> > + if (qemu_gluster_send_pipe(s, acb) < 0) {
> > + error_report("Could not complete read/write/flush from gluster");
> > + abort();
>
> Aborting is a bit drastic, it would be nice to save and exit gracefully.
I am not sure if there is an easy way to recover sanely and exit from this
kind of error.
Here the non-QEMU thread (gluster thread) failed to notify the QEMU thread
on the read side of the pipe about the IO completion. So essentially
bdrv_read or bdrv_write will never complete if this error happens.
Do you have any suggestions on how to exit gracefully here ?
> > +static QEMUOptionParameter qemu_gluster_create_options[] = {
>
> 'const'?
Hmm no precedence of const usage for identical scenario in other block
drivers in QEMU.
>
> > + {
> > + .name = BLOCK_OPT_SIZE,
> > + .type = OPT_SIZE,
> > + .help = "Virtual disk size"
> > + },
> > + { NULL }
> > +};
> > +
> > +static BlockDriver bdrv_gluster = {
>
> 'const'?
Again dodn't see the precedence for this.
Thanks for your review.
Regards,
Bharata.
