[libvirt] [PATCH] network: fix virtual network bridge delay setting
libvirt's network config documents that a bridge's STP "forward delay" (called "delay" in the XML) should be specified in seconds, but virNetDevBridgeSetSTPDelay() assumes that it is given a delay in milliseconds (although the comment at the top of the function incorrectly says "seconds". This fixes the comment, and converts the delay to milliseconds before calling virNetDevBridgeSetSTPDelay(). --- (Fortunately the default delay (and what most people set) is 0) src/network/bridge_driver.c | 6 +++++- src/util/virnetdevbridge.c | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 59b2c10..53eebed 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1826,8 +1826,12 @@ networkStartNetworkVirtual(struct network_driver *driver, } /* Set bridge options */ + + /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay + * expects milliseconds + */ if (virNetDevBridgeSetSTPDelay(network->def->bridge, - network->def->delay) < 0) + network->def->delay * 1000) < 0) goto err1; if (virNetDevBridgeSetSTP(network->def->bridge, diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index a616d8e..7b11bee 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2011 Red Hat, Inc. + * Copyright (C) 2007-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -378,7 +378,7 @@ int virNetDevBridgeRemovePort(const char *brname, /** * virNetDevBridgeSetSTPDelay: * @brname: the bridge name - * @delay: delay in seconds + * @delay: delay in milliseconds * * Set the bridge forward delay * -- 1.7.11.4
On 08/23/2012 09:26 AM, Laine Stump wrote:
libvirt's network config documents that a bridge's STP "forward delay" (called "delay" in the XML) should be specified in seconds, but virNetDevBridgeSetSTPDelay() assumes that it is given a delay in milliseconds (although the comment at the top of the function incorrectly says "seconds".
This fixes the comment, and converts the delay to milliseconds before calling virNetDevBridgeSetSTPDelay(). ---
ACK.
+ /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay + * expects milliseconds + */ if (virNetDevBridgeSetSTPDelay(network->def->bridge, - network->def->delay) < 0) + network->def->delay * 1000) < 0)
Do we need to worry about integer overflow, or are the chances of someone configuring network->def->delay > INT_MAX/1000 unlikely? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/23/2012 12:06 PM, Eric Blake wrote:
On 08/23/2012 09:26 AM, Laine Stump wrote:
libvirt's network config documents that a bridge's STP "forward delay" (called "delay" in the XML) should be specified in seconds, but virNetDevBridgeSetSTPDelay() assumes that it is given a delay in milliseconds (although the comment at the top of the function incorrectly says "seconds".
This fixes the comment, and converts the delay to milliseconds before calling virNetDevBridgeSetSTPDelay(). --- ACK.
Thanks. I pushed it.
+ /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay + * expects milliseconds + */ if (virNetDevBridgeSetSTPDelay(network->def->bridge, - network->def->delay) < 0) + network->def->delay * 1000) < 0) Do we need to worry about integer overflow, or are the chances of someone configuring network->def->delay > INT_MAX/1000 unlikely?
If they set a value so large that it would overflow, they'd have bigger problems than the overflow :-) (as in their guest's would need to be up for the magical 49.7 days before they began to pass network traffic). There's really no point in setting anything much bigger than 30 seconds; if anything, we might want to consider limiting it to some maximum in the RNG and parser. I just don't know what is the highest "reasonable" value, though.
participants (2)
-
Eric Blake -
Laine Stump