Re: [Qemu-devel] [PATCH for-2.6 v2 0/3] Bug fixes for gluster

[Raghavendra Gowdappa <rgowdapp@redhat.com>]

----- Original Message -----
> From: "Jeff Cody" <jcody@redhat.com>
> To: "Ric Wheeler" <rwheeler@redhat.com>
> Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, kwolf@redhat.com, pkarampu@redhat.com, rgowdapp@redhat.com,
> ndevos@redhat.com, "Rik van Riel" <riel@redhat.com>
> Sent: Tuesday, April 19, 2016 7:39:17 PM
> Subject: Re: [PATCH for-2.6 v2 0/3] Bug fixes for gluster
> 
> On Tue, Apr 19, 2016 at 08:18:39AM -0400, Ric Wheeler wrote:
> > On 04/19/2016 08:07 AM, Jeff Cody wrote:
> > >Bug fixes for gluster; third patch is to prevent
> > >a potential data loss when trying to recover from
> > >a recoverable error (such as ENOSPC).
> > 
> > Hi Jeff,
> > 
> > Just a note, I have been talking to some of the disk drive people
> > here at LSF (the kernel summit for file and storage people) and got
> > a non-public confirmation that individual storage devices (s-ata
> > drives or scsi) can also dump cache state when a synchronize cache
> > command fails.  Also followed up with Rik van Riel - in the page
> > cache in general, when we fail to write back dirty pages, they are
> > simply marked "clean" (which means effectively that they get
> > dropped).

Yes. Thanks for confirming that. This was another source of confusion for us while deciding on what should be Gluster's "reaction" on failure of write-backs, as Linux kernel page-cache itself doesn't do retries and was one of the questions raised by us on how QEMU handles the same scenario on different platforms. Nevertheless, it doesn't hurt glusterfs to do retries till an fsync or flush.

> > 
> > Long winded way of saying that I think that this scenario is not
> > unique to gluster - any failed fsync() to a file (or block device)
> > might be an indication of permanent data loss.
> >
> 
> Ric,
> 
> Thanks.
> 
> I think you are right, we likely do need to address how QEMU handles fsync
> failures across the board in QEMU at some point (2.7?).  Another point to
> consider is that QEMU is cross-platform - so not only do we have different
> protocols, and filesystems, but also different underlying host OSes as well.
> It is likely, like you said, that there are other non-gluster scenarios where
> we have non-recoverable data loss on fsync failure.
> 
> With Gluster specifically, if we look at just ENOSPC, does this mean that
> even if Gluster retains its cache after fsync failure, we still won't know
> that there was no permanent data loss?  If we hit ENOSPC during an fsync, I
> presume that means Gluster itself may have encountered ENOSPC from a fsync to
> the underlying storage.  In that case, does Gluster just pass the error up
> the stack?

Yes. It passes errno up the stack. (But,) If the option "resync-failed-syncs-after-fsync" is set, Gluster retains the cache after failed fsync to backend irrespective of the errno (including ENOSPC) till a flush. So, there is no permanent data-loss as long as the fd is not closed or backend store recovers from the error before fd is closed.

To summarize consequences of the scenario you explained:

1. Application/kernel sees a failed fsync with same errno as the one backend-storage returned.
2. (Nevertheless) Glusterfs retains the writes cached before fsync (even after fsync failure) and does retry, if performance.resync-failed-syncs-after-fsync option is set.

regards,
Raghavendra

> 
> Jeff
> 
> > 
> > >
> > >The final patch closes the gluster fd and sets the
> > >protocol drv to NULL on fsync failure in gluster;
> > >we have no way of knowing what gluster versions
> > >support retaining fysnc cache on error, so until
> > >we do the safest thing to do is invalidate the
> > >drive.
> > >
> > >Jeff Cody (3):
> > >   block/gluster: return correct error value
> > >   block/gluster: code movement of qemu_gluster_close()
> > >   block/gluster: prevent data loss after i/o error
> > >
> > >  block/gluster.c | 66
> > >  ++++++++++++++++++++++++++++++++++++++++++++++-----------
> > >  configure       |  8 +++++++
> > >  2 files changed, 62 insertions(+), 12 deletions(-)
> > >
> > 
>