Am 23.10.2012 13:08, schrieb Nicholas Thomas: > On Tue, 2012-10-23 at 12:33 +0200, Kevin Wolf wrote: >> Am 22.10.2012 13:09, schrieb n...@bytemark.co.uk: >>> >>> This is unlikely to come up now, but is a necessary prerequisite for >>> reconnection >>> behaviour. >>> >>> Signed-off-by: Nick Thomas <n...@bytemark.co.uk> >>> --- >>> block/nbd.c | 13 +++++++++++-- >>> 1 files changed, 11 insertions(+), 2 deletions(-) >> >> What's the real requirement here? Silently ignoring a flush and >> returning success for it feels wrong. Why is it correct? >> >> Kevin > > I just needed to avoid socket operations while s->sock == -1, and > extending the existing case of "can't do the command, so pretend I did > it" to "can't do the command right now, so pretend..." seemed like an > easy way out.
The difference is that NBD_FLAG_SEND_FLUSH is only clear if the server is configured like that, i.e. it's completely in the control of the user, and it ignores flushes consistently (for example because it already uses a writethrough mode so that flushes aren't required). Network outage usually isn't in the control of the user and ignoring a flush request when the server actually asked for flushes is completely surprising and dangerous. > In the Bytemark case, the NBD server always opens the file O_SYNC, so > nbd_co_flush could check in_flight == 0 and return 0/1 based on that; > but I'd be surprised if that's true for all NBD servers. Should we be > returning 1 here for both "not supported" and "can't do it right now", > instead? The best return value is probably -ENOTCONN or -EIO if you must return an error. But maybe using the same logic as for writes would be appropriate, i.e. try to reestablish the connection before you return an error. Kevin
