Re: [libvirt] [PATCH 1/3] network: consolidate connection count updates for device pool

2016-02-12 Thread John Ferlan


On 02/11/2016 04:37 PM, Laine Stump wrote:
> networkReleaseActualDevice() and networkNotifyActualDevice() both were
> updating the individual devices' connections count in two separate
> places (unlike networkAllocateActualDevice() which does it in a single
> unified place after success:). The code is correct, but prone to
> confusion / later breakage. All of these updates are anyway located at
> the end of if/else clauses that are (with the exception of a single
> VIR_DEBUG() in each case) immediately followed by the success: label
> anyway, so this patch replaces the duplicated ++/-- instructions with
> a single ++/-- inside a qualifying "if (dev)" down below success:.
> (NB: if dev != NULL, by definition we are using a device (either pci
> or netdev, doesn't matter for these purposes) from the network's pool)
> 
> The VIR_DEBUG args (which will be replaced in a followup patch anyway)
> were all adjusted to account for the connection count being out of
> date at the time.
> ---
>  src/network/bridge_driver.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 

ACK

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/3] network: consolidate connection count updates for device pool

2016-02-11 Thread Laine Stump
networkReleaseActualDevice() and networkNotifyActualDevice() both were
updating the individual devices' connections count in two separate
places (unlike networkAllocateActualDevice() which does it in a single
unified place after success:). The code is correct, but prone to
confusion / later breakage. All of these updates are anyway located at
the end of if/else clauses that are (with the exception of a single
VIR_DEBUG() in each case) immediately followed by the success: label
anyway, so this patch replaces the duplicated ++/-- instructions with
a single ++/-- inside a qualifying "if (dev)" down below success:.
(NB: if dev != NULL, by definition we are using a device (either pci
or netdev, doesn't matter for these purposes) from the network's pool)

The VIR_DEBUG args (which will be replaced in a followup patch anyway)
were all adjusted to account for the connection count being out of
date at the time.
---
 src/network/bridge_driver.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c19696c..dcd38ed 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1,7 +1,7 @@
 /*
  * bridge_driver.c: core driver methods for managing network
  *
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -4389,10 +4389,8 @@ networkNotifyActualDevice(virDomainDefPtr dom,
 goto error;
 }
 
-/* we are now assured of success, so mark the allocation */
-dev->connections++;
 VIR_DEBUG("Using physical device %s, connections %d",
-  dev->device.dev, dev->connections);
+  dev->device.dev, dev->connections + 1);
 
 }  else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ {
 virDomainHostdevDefPtr hostdev;
@@ -,8 +4442,6 @@ networkNotifyActualDevice(virDomainDefPtr dom,
 goto error;
 }
 
-/* we are now assured of success, so mark the allocation */
-dev->connections++;
 VIR_DEBUG("Using physical device %04x:%02x:%02x.%x, connections %d",
   dev->device.pci.domain, dev->device.pci.bus,
   dev->device.pci.slot, dev->device.pci.function,
@@ -4454,6 +4450,8 @@ networkNotifyActualDevice(virDomainDefPtr dom,
 
  success:
 netdef->connections++;
+if (dev)
+dev->connections++;
 VIR_DEBUG("Using network %s, %d connections",
   netdef->name, netdef->connections);
 
@@ -4562,9 +4560,8 @@ networkReleaseActualDevice(virDomainDefPtr dom,
 goto error;
 }
 
-dev->connections--;
 VIR_DEBUG("Releasing physical device %s, connections %d",
-  dev->device.dev, dev->connections);
+  dev->device.dev, dev->connections - 1);
 
 } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ {
 virDomainHostdevDefPtr hostdev;
@@ -4598,16 +4595,18 @@ networkReleaseActualDevice(virDomainDefPtr dom,
 goto error;
 }
 
-dev->connections--;
 VIR_DEBUG("Releasing physical device %04x:%02x:%02x.%x, connections 
%d",
   dev->device.pci.domain, dev->device.pci.bus,
   dev->device.pci.slot, dev->device.pci.function,
-  dev->connections);
+  dev->connections - 1);
 }
 
  success:
 if (iface->data.network.actual) {
 netdef->connections--;
+if (dev)
+dev->connections--;
+
 VIR_DEBUG("Releasing network %s, %d connections",
   netdef->name, netdef->connections);
 
-- 
2.5.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list