On Thu, Aug 2, 2012 at 3:55 AM, Bharata B Rao
<bhar...@linux.vnet.ibm.com> wrote:
> 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.
OK.
>
>> > +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 ?
Ignore but set the callback return to -EIO, see for example curl.c:249.
>
>> > +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.
OK.
>
> Thanks for your review.
>
> Regards,
> Bharata.
>
