[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 08/16] Rename ifaceGetIndex and ifaceGetVLAN



On Thu, Nov 17, 2011 at 06:55:34AM -0500, Laine Stump wrote:
> On 11/15/2011 06:14 AM, Daniel P. Berrange wrote:
> >From: "Daniel P. Berrange"<berrange redhat com>
> >
> >Rename the ifaceGetIndex method to virNetDevGetIndex and
> >ifaceGetVlanID to virNetDevGetVLanID. Also change the error
> >reporting behaviour to always raise errors and return -1 on
> >failure
> >
> >* util/interface.c, util/interface.h: Rename ifaceGetIndex
> >   and ifaceGetVLAN
> >* nwfilter/nwfilter_gentech_driver.c, nwfilter/nwfilter_learnipaddr.c,
> >   nwfilter/nwfilter_learnipaddr.c, util/virnetdevvportprofile.c: Update
> >   for API renames and error handling changes
> >---
> >  src/libvirt_private.syms               |    4 +-
> >  src/nwfilter/nwfilter_gentech_driver.c |   13 +++--
> >  src/nwfilter/nwfilter_learnipaddr.c    |   22 ++++---
> >  src/util/interface.c                   |  103 ++++++++++++++++----------------
> >  src/util/interface.h                   |    6 +-
> >  src/util/virnetdevmacvlan.c            |   17 ++++--
> >  src/util/virnetdevvportprofile.c       |    6 +-
> >  7 files changed, 92 insertions(+), 79 deletions(-)
> >
> >diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> >index d71186b..e621f79 100644
> >--- a/src/libvirt_private.syms
> >+++ b/src/libvirt_private.syms
> >@@ -576,12 +576,12 @@ virHookPresent;
> >
> >  # interface.h
> >  ifaceCheck;
> >-ifaceGetIndex;
> >+virNetDevGetIndex;
> >  ifaceGetIPAddress;
> >  ifaceGetNthParent;
> >  ifaceGetPhysicalFunction;
> >  ifaceGetVirtualFunctionIndex;
> >-ifaceGetVlanID;
> >+virNetDevGetVLanID;
> >  ifaceIsVirtualFunction;
> >  virNetDevMacVLanCreate;
> >  virNetDevMacVLanDelete;
> >diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> >index 899bd32..9f44aef 100644
> >--- a/src/nwfilter/nwfilter_gentech_driver.c
> >+++ b/src/nwfilter/nwfilter_gentech_driver.c
> >@@ -903,9 +903,10 @@ _virNWFilterInstantiateFilter(virConnectPtr conn,
> >      /* after grabbing the filter update lock check for the interface; if
> >         it's not there anymore its filters will be or are being removed
> >         (while holding the lock) and we don't want to build new ones */
> >-    if (ifaceGetIndex(false, net->ifname,&ifindex)<  0) {
> >+    if (virNetDevGetIndex(net->ifname,&ifindex)<  0) {
> >          /* interfaces / VMs can disappear during filter instantiation;
> >             don't mark it as an error */
> >+        virResetLastError();
> 
> 
> But since the error has already been logged, it isn't really being
> ignored. Based on past experience with other "errors that aren't
> really errors", I'm betting this will lead to bogus bug reports
> (unless it's *extremely* rare, and requires doing something way out
> of the ordinary such that the admin might expect to get spurious
> error messages). Maybe virNetDevGetIndex could have some sort of
> "allow/ignore/dontreport failure" flag added?

The 'allow/ignore/dontreport' parameters are exactly the kind of thing
I killed off in this patch server, because they end up polluting huge
calls of functions. To avoid the log message in the ENODEV case it is
easy enough to just add in a virNetDevExists() check to this block.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]