Re: [libvirt] [PATCH 09/10] qemu: use virDomainNetGetActual*() functions where appropriate
On 07/05/2011 01:45 AM, Laine Stump wrote:
The qemu driver accesses fields in the virDomainNetDef directly, but
with the advent of the virDomainActualNetDef, some pieces of
information may be found in a different place (the ActualNetDef) if
the network connection is of type='network' and that network is of
forward type='bridge|private|vepa|passthrough'. The previous patch
added functions to mask this difference from callers - they hide the
decision making process and just pick the value from the proper place.
This patch uses those functions in the qemu driver as a first step in
making qemu work with the new network types. At this point, it's
assumed that the virDomainActualNetDef is already properly initialized
(it isn't yet).
Is this going to bite anyone during bisection of this patch series?
Hopefully not, so I'm not sure how much you want to rework this while
rebasing, which means you can probably keep the code as-is. But my
approach would have been:
patch 1 - introduce wrapper functions that make no semantic change,
while updating all callers to use wrapper functions. Something like:
int
virDomainNetGetActualType(virDomainNetDefPtr iface)
{
return iface->type;
}
and replace all uses of iface->type with virDomainNetGetActualType().
patch 2 - enhance wrapper functions to actually look into
virDomainActualNetDef, preferably while guaranteeing that it is
initialized correctly
at this stage, you fix the body of virDomainNetGetActualType to:
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK ||
!iface->data.network.actual)
return iface->type;
return iface->data.network.actual->type;
+++ b/src/qemu/qemu_driver.c
@@ -3935,9 +3935,35 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
for (i = 0 ; i < def->nnets ; i++) {
virDomainNetDefPtr net = def->nets[i];
int bootIndex = net->bootIndex;
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
- net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ int actualType = virDomainNetGetActualType(net);
VIR_FREE(net->data.network.name);
+ VIR_FREE(net->data.network.portgroup);
+ if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
And here is an instance where you are refactoring existing code
(converting net->type to virDomainNetGetActualType(net)) as well as
adding new code (taking action if actualType is TYPE_BRIDGE).
Separating the refactoring from the introduction of new features can
make review a bit easier.
+ char *brname =
strdup(virDomainNetGetActualBridgeName(net));
+ virDomainActualNetDefFree(net->data.network.actual);
+
+ memset(net, 0, sizeof *net);
+
+ net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
+ net->data.ethernet.dev = brname;
Need to check for strdup failure, rather than setting dev to NULL.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)