On Wed, Feb 13, 2019 at 01:04:07PM +0100, Andrea Bolognani wrote: > Logging the error is fine and all, but getting the information > to the user directly is even better. > > https://bugzilla.redhat.com/show_bug.cgi?id=1578741 > Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx> > --- > src/util/virfile.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 271bf5e796..30cad87df9 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -351,8 +351,12 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd) > if (!wfd) > return; > > + /* If the command used to process IO has produced errors, it's fair > + * to assume those will be more relevant to the user than whatever > + * eg. QEMU can figure out on its own, so it's okay if we end up > + * discarding an existing error */ > if (wfd->err_msg && *wfd->err_msg) > - VIR_WARN("iohelper reports: %s", wfd->err_msg); > + virReportError(VIR_ERR_OPERATION_FAILED, "%s", wfd->err_msg); I don't think this is a good place to report errors. We normally write xxxxFree() functions such that they always succeed & don't report errors. Essentially they should just be cleaning up memory and other allocated resources. This is especially true now that we rely on GCC cleanup functions which automagically call virFileWrapperFdFree when the variable goes out of scope. IOW, if we need to report an error from the io helper, then it needs to be done earlier, pehrpas in virFileWrapperFdClose ? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
