On 11/15/2011 06:14 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrangeberra...@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?
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index 68bdcfc..9a51fc2 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -252,21 +252,23 @@ virNWFilterTerminateLearnReq(const char *ifname) {
int ifindex;
virNWFilterIPAddrLearnReqPtr req;
-if (ifaceGetIndex(false, ifname,ifindex) == 0) {
-
-IFINDEX2STR(ifindex_str, ifindex);
+if (virNetDevGetIndex(ifname,ifindex) 0) {
+virResetLastError();
+return rc;
+}
-virMutexLock(pendingLearnReqLock);
+IFINDEX2STR(ifindex_str, ifindex);
-req = virHashLookup(pendingLearnReq, ifindex_str);
-if (req) {
-rc = 0;
-req-terminate = true;
-}
+virMutexLock(pendingLearnReqLock);
-virMutexUnlock(pendingLearnReqLock);
+req = virHashLookup(pendingLearnReq, ifindex_str);
+if (req) {
+rc = 0;
+req-terminate = true;
}
+virMutexUnlock(pendingLearnReqLock);
+
return rc;
git/diff didn't go to any pains to make *that* hunk easy to follow :-/
original:
if (ifaceGetIndex(false, ifname,ifindex) == 0) {
IFINDEX2STR(ifindex_str, ifindex);
virMutexLock(pendingLearnReqLock);
req = virHashLookup(pendingLearnReq, ifindex_str);
if (req) {
rc = 0;
req-terminate = true;
}
virMutexUnlock(pendingLearnReqLock);
}
return rc;
vs new:
if (virNetDevGetIndex(ifname,ifindex) 0) {
virResetLastError();
return rc;
}
IFINDEX2STR(ifindex_str, ifindex);
virMutexLock(pendingLearnReqLock);
req = virHashLookup(pendingLearnReq, ifindex_str);
if (req) {
rc = 0;
req-terminate = true;
}
virMutexUnlock(pendingLearnReqLock);
return rc;
so that looks okay (modulo the issue with virNetDevGetIndex errors being
logged rather than ignored).
I'm uncomfortable enough with the change in error behavior that I don't
want to ACK this as-is.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
