Re: [Qemu-devel] [v9 Patch 2/6]Qemu: Error classes for file reopen and data sync failure

[Luiz Capitulino]

On Fri, 18 Nov 2011 14:59:40 +0530
supriya kannery <address@hidden> wrote:

> Luiz Capitulino wrote:
> > On Fri, 11 Nov 2011 12:17:35 +0530
> > Supriya Kannery <address@hidden> wrote:
> >
> >   
> >> New error classes defined for file reopen failure and data
> >> sync error
> >>
> >> Signed-off-by: Supriya Kannery <address@hidden>
> >>
> >> ---
> >>  qerror.c |    8 ++++++++
> >>  qerror.h |    6 ++++++
> >>  2 files changed, 14 insertions(+)
> >>
> >> Index: qemu/qerror.c
> >> ===================================================================
> >> --- qemu.orig/qerror.c
> >> +++ qemu/qerror.c
> >> @@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
> >>          .desc      = "Device '%(device)' is not removable",
> >>      },
> >>      {
> >> +        .error_fmt = QERR_DATA_SYNC_FAILED,
> >> +        .desc      = "Syncing of data failed for device '%(device)'",
> >> +    },
> >> +    {
> >> +        .error_fmt = QERR_REOPEN_FILE_FAILED,
> >> +        .desc      = "Could not reopen '%(filename)'",
> >> +    },
> >>     
> >
> > Is this really needed? I think you could use QERR_OPEN_FILE_FAILED.
> >
> >   
> 
> This could confuse the user, as to what was tried on an
> already opened file by qemu. So I feel, QERR_REOPEN_FILE_FAILED can
> bring in better clarity to the message passed to the user.

I think it's the contrary.

First, the fact that we reopen the image is an implementation detail. If we
do it differently in the future than the error won't fit anymore.

Second, what did really fail when REOPEN_FILE_FAILED is triggered? Flushing
the buffers? Closing the image? Opening it with different flags? All these
operations are part of the "reopen" algorithm.

On the other hand, if you see a OPEN_FILE_FAILED then there's only one
single operation that might have failed. Also, OPEN_FILE_FAILED is widely
well known as some version of it exists in any operating system.