9:55 a.m.
On 30.11.2012 19:43, Laine Stump wrote:
On 11/19/2012 11:51 AM, Michal Privoznik wrote:
> Network should be notified if we plug in or unplug an
> interface, so it can perform some action, e.g. set/unset
> network part of QoS.
> ---
> src/Makefile.am | 7 ++-
> src/conf/domain_conf.h | 1 +
> src/conf/network_conf.c | 1 +
> src/conf/network_conf.h | 3 +
> src/libvirt_network.syms | 8 ++
> src/network/bridge_driver.c | 165 +++++++++++++++++++++++++++++++++++++++++++
> src/network/bridge_driver.h | 10 ++-
> src/qemu/qemu_command.c | 32 ++++++---
> src/qemu/qemu_process.c | 12 +++
> 9 files changed, 225 insertions(+), 14 deletions(-)
> create mode 100644 src/libvirt_network.syms
>
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 3e46304..8a8d1e4 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -221,6 +221,9 @@ struct _virNetworkObj {
>
> virNetworkDefPtr def; /* The current definition */
> virNetworkDefPtr newDef; /* New definition to activate at shutdown */
> +
> + unsigned int class_id; /* next class ID for QoS */
Okay, so this is just a monotonically increasing counter so that each
interface gets a new and different class_id. Maybe you should call it
"nextFreeClassID" or something like that, so that everyone understands
it's not a class id used by the network. Or you might want to do this
with a bitmap so you can re-use id's of interfaces that are
disconnected. (can virBitmaps being dynamically expanded?)
> + unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs
*/
And you keep track of the sum of all floors here.
So, what happens if libvirtd is restarted? It looks like they both go
back to their initial values.
We think alike :) In the next patches I am fixing this. class_id becomes
a virBitmap and among with floor_sum gets stored into network status
XML. I've split it into several patches because changes are big (in
number of lines at least) and I think this way it's easier to review.
And what about hotplug?
This is a similar problem to the pool of interfaces used by
macvtap/hostdev modes - a network with an interface pool needs to have
the "inuse" counters of each interface refreshed whenever libvirtd
restarts. So the necessary hooks are already in place:
networkAllocateActualDevice - called any time an interface with
type='network' is initially
allocated and connected.
networkNotifyActualDevice - called for each type='network' interface
of each active domain
any time libvirtd restarts
networkReleaseActualDevice - called for every type='network'
interface as it is being
disconnected and destroyed.
These are called for *all* type='network' interfaces, not just those
that need to allocate a physical netdev from a pool.
Rather than adding your own new hooks (and figuring out all the places
they need to be called), I think it would be better to use the existing
hooks (perhaps calling a reduced/renamed version of your functions,
which can possibly be moved over to ). That will have two advantages:
1) It will assure that the bandwidth functions are called at all the
right times, including hotplug/unplug, and libvirtd restart
2) It will continue the process of consolidating all network-related
functionality need for these three events into 3 functions which may
some day be moved into their own daemom with a public (within libvirt
anyway) API accessible via RPC (thus allowing non-privileged libvirts to
have full networking functionality).
Okay, renamed to networkPlugBandwidth and networkUnplugBandwidth and
inserted into networkAllocateActualDevice and networkReleaseActualDevice
respectively.
Note that you will probably want to save the interface class_id in
the
iface->actual (as a matter of fact, in
iface->data.network.actual->bandwidth, which can be retrieved with
virDomainNetGetActualBandwidth()) so that it's saved properly in the
interface's state without polluting the interface's config. Then it will
be read back from the state file during the restart and available during
networkNotifyActualDevice() of each interface. I guess you can then
re-set the network->class_id by checking interface->class_id and setting
network->class_id = MAX(network->class_id, iface->class_id + 1);
for each encountered interface (or add it to a bitmap, if a) bitmaps can
be enlarged dynamically or b) we can determine a reasonable maximum
limit on number of active domains). At the same time, you'll recompute
the network->floor_sum iteratively as the network is called with a
notify for each interface.
> ...
To summarize: I think this needs to be refactored to be integrated into
the network*ActualDevice() functions so that the bookkeeping is properly
handled in all cases, including hotplug and libvirtd restart.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list