qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 3/3] block: gluster as block backend


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [RFC PATCH 3/3] block: gluster as block backend
Date: Tue, 19 Jun 2012 12:05:44 +0100

On Tue, Jun 19, 2012 at 10:30 AM, Bharata B Rao
<address@hidden> wrote:
> On Mon, Jun 18, 2012 at 06:35:28PM +0100, Stefan Hajnoczi wrote:
>> On Mon, Jun 11, 2012 at 3:21 PM, Bharata B Rao
>> <address@hidden> wrote:
>> > + á á/* 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.
>
> I picked up this logic from block/raw-posix.c:raw_open_common(). Don't see
> anything related to bs->enable_write_cache there. Will find out more about
> bs->enable_write_cache.

If you fetch the latest qemu.git and check bdrv_open_common() there is
new code that stashes BDRV_O_CACHE_WB in bs->enable_write_cache and
then opens the actual block driver with BDRV_O_CACHE_WB set.  You can
use bdrv_enable_write_cache() to test the original flag.

>> > +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?
>
> Again, this comes from rbd. gluster_finish_aiocb() is the callback
> that we have registered with gluster. I am not doing any error handling when
> we even fail to write to the pipe. An even reader would be waiting to read
> from the other end of the pipe. Typically error handling and decrementing
> the in-flight IO request count is done by that event reader. But in this
> case, we even failed to kick (via pipe write) the even reader.

It sounds like you're saying the request is not properly cleaned up
and completed on failure.  Please fix :).

>> > +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.
>
> Ok. May be I could just return value from gluster_fstat().

The gluster_fstat() code also does not return negative errnos (at
least in the first case I checked, when CALLOC() fails).

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]