Discussion:
[libvirt] [PATCH 0/2] syncNicRxFilterMultiMode: Trivial fixes
Michal Privoznik
2018-12-04 08:07:57 UTC
Permalink
Pushed under trivial rule.

Michal Prívozník (2):
syncNicRxFilterMultiMode: Check for helper's retval properly
syncNicRxFilterMultiMode: Fix indentation

src/qemu/qemu_driver.c | 80 +++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 40 deletions(-)
--
2.19.2
Michal Privoznik
2018-12-04 08:07:58 UTC
Permalink
There are two functions called from syncNicRxFilterMultiMode:
virNetDevSetRcvAllMulti() and virNetDevSetRcvMulti(). Both of
them return 0 on success and -1 on error. However, currently
their return value is checked for != 0 which conflicts with our
assumptions on retvals: a positive value is still considered
success but with current check it would lead to failure.

Signed-off-by: Michal Privoznik <***@redhat.com>
---
src/qemu/qemu_driver.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8403492ec1..7bac10506b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4448,7 +4448,7 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
guestFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_NORMAL)) {
switch (guestFilter->multicast.mode) {
case VIR_NETDEV_RX_FILTER_MODE_ALL:
- if (virNetDevSetRcvAllMulti(ifname, true)) {
+ if (virNetDevSetRcvAllMulti(ifname, true) < 0) {
VIR_WARN("Couldn't set allmulticast flag to 'on' for "
"device %s while responding to "
"NIC_RX_FILTER_CHANGED", ifname);
@@ -4461,7 +4461,7 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
break;
}

- if (virNetDevSetRcvMulti(ifname, true)) {
+ if (virNetDevSetRcvMulti(ifname, true) < 0) {
VIR_WARN("Couldn't set multicast flag to 'on' for "
"device %s while responding to "
"NIC_RX_FILTER_CHANGED", ifname);
@@ -4478,13 +4478,13 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
break;

case VIR_NETDEV_RX_FILTER_MODE_NONE:
- if (virNetDevSetRcvAllMulti(ifname, false)) {
+ if (virNetDevSetRcvAllMulti(ifname, false) < 0) {
VIR_WARN("Couldn't set allmulticast flag to 'off' for "
"device %s while responding to "
"NIC_RX_FILTER_CHANGED", ifname);
}

- if (virNetDevSetRcvMulti(ifname, false)) {
+ if (virNetDevSetRcvMulti(ifname, false) < 0) {
VIR_WARN("Couldn't set multicast flag to 'off' for "
"device %s while responding to "
"NIC_RX_FILTER_CHANGED",
--
2.19.2
Michal Privoznik
2018-12-04 08:07:59 UTC
Permalink
The indentation of the code in this function is a bit off.

Signed-off-by: Michal Privoznik <***@redhat.com>
---
src/qemu/qemu_driver.c | 80 +++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7bac10506b..83de42f041 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4445,52 +4445,52 @@ syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
{
if (hostFilter->multicast.mode != guestFilter->multicast.mode ||
(guestFilter->multicast.overflow &&
- guestFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_NORMAL)) {
+ guestFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_NORMAL)) {
switch (guestFilter->multicast.mode) {
- case VIR_NETDEV_RX_FILTER_MODE_ALL:
- if (virNetDevSetRcvAllMulti(ifname, true) < 0) {
- VIR_WARN("Couldn't set allmulticast flag to 'on' for "
- "device %s while responding to "
- "NIC_RX_FILTER_CHANGED", ifname);
- }
- break;
+ case VIR_NETDEV_RX_FILTER_MODE_ALL:
+ if (virNetDevSetRcvAllMulti(ifname, true) < 0) {
+ VIR_WARN("Couldn't set allmulticast flag to 'on' for "
+ "device %s while responding to "
+ "NIC_RX_FILTER_CHANGED", ifname);
+ }
+ break;

- case VIR_NETDEV_RX_FILTER_MODE_NORMAL:
- if (guestFilter->multicast.overflow &&
- (hostFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_ALL)) {
- break;
- }
+ case VIR_NETDEV_RX_FILTER_MODE_NORMAL:
+ if (guestFilter->multicast.overflow &&
+ (hostFilter->multicast.mode == VIR_NETDEV_RX_FILTER_MODE_ALL)) {
+ break;
+ }

- if (virNetDevSetRcvMulti(ifname, true) < 0) {
- VIR_WARN("Couldn't set multicast flag to 'on' for "
- "device %s while responding to "
- "NIC_RX_FILTER_CHANGED", ifname);
- }
+ if (virNetDevSetRcvMulti(ifname, true) < 0) {
+ VIR_WARN("Couldn't set multicast flag to 'on' for "
+ "device %s while responding to "
+ "NIC_RX_FILTER_CHANGED", ifname);
+ }

- if (virNetDevSetRcvAllMulti(ifname,
- guestFilter->multicast.overflow) < 0) {
- VIR_WARN("Couldn't set allmulticast flag to '%s' for "
- "device %s while responding to "
- "NIC_RX_FILTER_CHANGED",
- virTristateSwitchTypeToString(virTristateSwitchFromBool(guestFilter->multicast.overflow)),
- ifname);
- }
- break;
+ if (virNetDevSetRcvAllMulti(ifname,
+ guestFilter->multicast.overflow) < 0) {
+ VIR_WARN("Couldn't set allmulticast flag to '%s' for "
+ "device %s while responding to "
+ "NIC_RX_FILTER_CHANGED",
+ virTristateSwitchTypeToString(virTristateSwitchFromBool(guestFilter->multicast.overflow)),
+ ifname);
+ }
+ break;

- case VIR_NETDEV_RX_FILTER_MODE_NONE:
- if (virNetDevSetRcvAllMulti(ifname, false) < 0) {
- VIR_WARN("Couldn't set allmulticast flag to 'off' for "
- "device %s while responding to "
- "NIC_RX_FILTER_CHANGED", ifname);
- }
+ case VIR_NETDEV_RX_FILTER_MODE_NONE:
+ if (virNetDevSetRcvAllMulti(ifname, false) < 0) {
+ VIR_WARN("Couldn't set allmulticast flag to 'off' for "
+ "device %s while responding to "
+ "NIC_RX_FILTER_CHANGED", ifname);
+ }

- if (virNetDevSetRcvMulti(ifname, false) < 0) {
- VIR_WARN("Couldn't set multicast flag to 'off' for "
- "device %s while responding to "
- "NIC_RX_FILTER_CHANGED",
- ifname);
- }
- break;
+ if (virNetDevSetRcvMulti(ifname, false) < 0) {
+ VIR_WARN("Couldn't set multicast flag to 'off' for "
+ "device %s while responding to "
+ "NIC_RX_FILTER_CHANGED",
+ ifname);
+ }
+ break;
}
}
}
--
2.19.2
Loading...