[libvirt] [PATCH 2/3] Debug: Remove unnecessary errors reported while parsing non-existent sysfs files.

[Prerna]

I agree. Thanks for pointing out that this is a behavioural change, which
should not happen.

I should be doing something like this:

if (virFileReadAllQuiet(path, 1024, &buf) < 0 ) {
    if (errno != ENOENT) {
        virReportSystemError(errno,
                                   _("unable to read: %s"), path);

    }
   goto cleanup;
}

On Wed, Mar 22, 2017 at 1:56 PM, Peter Krempa <pkrempa at redhat.com> wrote:

> On Wed, Mar 22, 2017 at 09:14:41 +0100, Peter Krempa wrote:
> > On Wed, Mar 22, 2017 at 01:02:18 -0700, Prerna Saxena wrote:
> > > Sample from current logs:
> > > error : virFileReadAll:1290 : Failed to open file '/sys/class/net/tap3/operstate':
> No such file or directory
> > > error : virNetDevGetLinkInfo:1895 : unable to read:
> /sys/class/net/tap3/operstate: No such file or directory
> > >
> > > These have no useful data point and are redundant.
> > >
> > > Signed-off-by: Prerna Saxena <saxenap.ltc at gmail.com>
> > > ---
> > >  src/util/virnetdev.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> > > index d123248..3e2f962 100644
> > > --- a/src/util/virnetdev.c
> > > +++ b/src/util/virnetdev.c
> > > @@ -1874,7 +1874,7 @@ virNetDevGetLinkInfo(const char *ifname,
> > >      if (virNetDevSysfsFile(&path, ifname, "operstate") < 0)
> > >          goto cleanup;
> > >
> > > -    if (virFileReadAll(path, 1024, &buf) < 0) {
> > > +    if (virFileReadAllQuiet(path, 1024, &buf) < 0 && errno != ENOENT)
> {
> > >          virReportSystemError(errno,
> > >                               _("unable to read: %s"),
> > >                               path);
> >
> > Remove this message here instead of switching to virFileReadAllQuiet.
> > virFileReadAll reports messages according to the failure itself.
>
> Hmm, So you want to avoid the error message altogether. So in that case
> you should rather call virFileAccess and also note what happens in an
> comment.
>
> Additionally since that would be a semantic change you need to mention
> it in the commit message and also make sure that all callers would
> ignore such failure. Otherwise you may cause a regression where we'd
> report an "unknown error" in cases where it was actually necessary.
>
> Since I found at least one code path that propagates the error
> (nodeDeviceGetXMLDesc) you should re-evaluate your approach.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170322/7ce6f9a6/attachment-0001.htm>