[libvirt] [PATCH 00/11] RFC: refactor internal driver architecture
Currently we have a bit of a craz setup for activating drivers for virConnectPtr. We iterate over the primary hypervisor drivers until one matches and activates. This bit is fine. Then we do the same for the network, storage, interface, etc secondary drivers. This is where the fun starts. In the case of the stateless drivers that run outside of libvirtd, they always want the secondary drivers to match the primary driver. Unfortunately if they don't register any dummy secondary driver they'll get given the reomte driver instead. The probing of secondary drivers is really unhelpful here. In the case of stateful drivers that run inside of libvirtd, there is (almost) always only one driver instance that they want to work with. So at best the probing of secondary drivers is a waste of time. With the increasing feature set though we're getting tight dependancies from the hypervisor driver to the secondary driver. eg QEMU depends on the specific network driver we have because it calls various internal APIs it exposes. We also have circular dependancy problems during libvirtd startup: https://www.redhat.com/archives/libvir-list/2014-January/msg01143.html There the storage driver needs a virConnectPtr instance in order to talk to the secret driver. At the same time the storage driver must run before the hypervisor driver because the hypervisor driver needs to use it for autostart of VMs. This is a horrible mess. The solution I think is to remove the concept of probing for secondary drivers entirely. Instead I suggest creating a struct virDriver { ... } which contains a pointer to virHypervisorDriver, virNetworkDriver, virInterfaceDriver, etc. now virConnectPtr will only reference the single virDriverPtr and when we open a hypervisor driver, we immediately get a full set of APIs for everything. This way all the hypervisor drivers will always know exactly what secondary driver they are working with. We'll be able to remove all the xxxPrivateData elements from the virConnectPtr. The stateless drivers can just access the main privateData. The stateful drivers can avoid any use of privateData at all - just access their global static variable with the driverState. This won't quite solve the circular dependancy problem in that mail thread above. By removing the directly reliance on the virConnectPtr driver table though, we'll be able to update the code more easily so it can fetch secrets directly and thus the storage driver wont need to do virConnectOpen("qemu:///system"). The patches that follow illustrate the first part of cleaning up use of the xxxPrivateData fields. This isn't complete but I wanted to show the direction it would go before I spend the time todo the full job. Incidentally our code is buggy as hell when it comes to using the xxxPrivateData fields. The PHyp driver actually abuses the networkPrivateData field for all its APIs. Alot of code wil directly access the global state driver state already ignoring the privateData fields already. So they are really more pain than they are worth and leading to buggy code. Daniel P. Berrange (11): Clean up remote driver connection open code Update remote driver to always use privateData Update ESX driver to always use privateData Update Hyper-V driver to always use privateData Move phyp internal info out of the header file Remove abuse of networkPrivateData in phyp driver Update Parallels driver to always use privateData Update Test driver to always use privateData Remove use of storagePrivateData from storage driver Remove use of networkPrivateData from network driver Remove use of networkPrivateData from netcf driver src/esx/esx_device_monitor.c | 6 +- src/esx/esx_interface_driver.c | 16 +- src/esx/esx_network_driver.c | 20 +- src/esx/esx_nwfilter_driver.c | 6 +- src/esx/esx_secret_driver.c | 6 +- src/esx/esx_storage_backend_iscsi.c | 26 +- src/esx/esx_storage_backend_vmfs.c | 36 +-- src/esx/esx_storage_driver.c | 44 ++-- src/hyperv/hyperv_device_monitor.c | 6 +- src/hyperv/hyperv_interface_driver.c | 6 +- src/hyperv/hyperv_network_driver.c | 6 +- src/hyperv/hyperv_nwfilter_driver.c | 6 +- src/hyperv/hyperv_secret_driver.c | 6 +- src/hyperv/hyperv_storage_driver.c | 6 +- src/interface/interface_backend_netcf.c | 71 ++---- src/network/bridge_driver.c | 435 ++++++++++++++------------------ src/parallels/parallels_network.c | 2 - src/parallels/parallels_storage.c | 12 +- src/parallels/parallels_utils.h | 1 + src/phyp/phyp_driver.c | 225 ++++++++--------- src/phyp/phyp_driver.h | 53 ---- src/remote/remote_driver.c | 153 +++-------- src/rpc/gendispatch.pl | 28 +- src/storage/storage_driver.c | 194 +++++++------- src/test/test_driver.c | 24 +- 25 files changed, 537 insertions(+), 857 deletions(-) -- 2.1.0
The remote driver has had a long term hack to deal with the fact that the old Xen driver worked outside libvirtd, but the rest of the drivers worked inside. So you could have a local hypervisor driver but everything else go via the remote driver. The Xen driver long ago moved inside libvirtd, so this hack is no longer needed. Thus we should open use the remote driver for secondary drivers if the primary driver is already the remote driver. --- src/remote/remote_driver.c | 85 ++++++++++++---------------------------------- 1 file changed, 21 insertions(+), 64 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 067f2d0..19a70a1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1124,33 +1124,6 @@ remoteAllocPrivateData(void) return priv; } -static int -remoteOpenSecondaryDriver(virConnectPtr conn, - virConnectAuthPtr auth, - unsigned int flags, - struct private_data **priv) -{ - int ret; - int rflags = 0; - - if (!((*priv) = remoteAllocPrivateData())) - return VIR_DRV_OPEN_ERROR; - - if (flags & VIR_CONNECT_RO) - rflags |= VIR_DRV_OPEN_REMOTE_RO; - - ret = doRemoteOpen(conn, *priv, auth, rflags); - if (ret != VIR_DRV_OPEN_SUCCESS) { - remoteDriverUnlock(*priv); - VIR_FREE(*priv); - } else { - (*priv)->localUses = 1; - remoteDriverUnlock(*priv); - } - - return ret; -} - static virDrvOpenStatus remoteConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, @@ -3554,8 +3527,7 @@ remoteConnectListAllSecrets(virConnectPtr conn, /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteGenericOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags, void **genericPrivateData) +remoteGenericOpen(virConnectPtr conn, void **genericPrivateData) { if (inside_daemon) return VIR_DRV_OPEN_DECLINED; @@ -3573,24 +3545,9 @@ remoteGenericOpen(virConnectPtr conn, virConnectAuthPtr auth, *genericPrivateData = priv; remoteDriverUnlock(priv); return VIR_DRV_OPEN_SUCCESS; - } else if (conn->networkDriver && - STREQ(conn->networkDriver->name, "remote")) { - struct private_data *priv = conn->networkPrivateData; - remoteDriverLock(priv); - *genericPrivateData = priv; - priv->localUses++; - remoteDriverUnlock(priv); - return VIR_DRV_OPEN_SUCCESS; - } else { - /* Using a non-remote driver, so we need to open a - * new connection for network APIs, forcing it to - * use the UNIX transport. This handles Xen driver - * which doesn't have its own impl of the network APIs. */ - struct private_data *priv; - int ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); - *genericPrivateData = priv; - return ret; } + + return VIR_DRV_OPEN_DECLINED; } static int @@ -3614,10 +3571,10 @@ remoteGenericClose(virConnectPtr conn, void **genericPrivateData) } static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteNetworkOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteNetworkOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->networkPrivateData); + return remoteGenericOpen(conn, &conn->networkPrivateData); } static int @@ -3629,10 +3586,10 @@ remoteNetworkClose(virConnectPtr conn) /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteInterfaceOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteInterfaceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->interfacePrivateData); + return remoteGenericOpen(conn, &conn->interfacePrivateData); } static int @@ -3644,10 +3601,10 @@ remoteInterfaceClose(virConnectPtr conn) /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteStorageOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteStorageOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->storagePrivateData); + return remoteGenericOpen(conn, &conn->storagePrivateData); } static int @@ -3826,10 +3783,10 @@ remoteStoragePoolListAllVolumes(virStoragePoolPtr pool, /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteNodeDeviceOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteNodeDeviceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->nodeDevicePrivateData); + return remoteGenericOpen(conn, &conn->nodeDevicePrivateData); } static int @@ -3947,10 +3904,10 @@ remoteNodeDeviceReset(virNodeDevicePtr dev) /* ------------------------------------------------------------- */ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteNWFilterOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteNWFilterOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->nwfilterPrivateData); + return remoteGenericOpen(conn, &conn->nwfilterPrivateData); } static int @@ -5603,10 +5560,10 @@ remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth, - unsigned int flags) +remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, auth, flags, &conn->secretPrivateData); + return remoteGenericOpen(conn, &conn->secretPrivateData); } static int -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
The remote driver has had a long term hack to deal with the fact that the old Xen driver worked outside libvirtd, but the rest of the drivers worked inside. So you could have a local hypervisor driver but everything else go via the remote driver. The Xen driver long ago moved inside libvirtd, so this hack is no longer needed. Thus we should open use the remote driver for secondary drivers if the primary driver is already the remote driver. --- src/remote/remote_driver.c | 85 ++++++++++++---------------------------------- 1 file changed, 21 insertions(+), 64 deletions(-)
Nice simplification. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Since the secondary drivers are only active when the primary driver is also the remote driver, there is no need to use the different type specific privateData fields. --- src/remote/remote_driver.c | 84 +++++++++++++--------------------------------- src/rpc/gendispatch.pl | 28 +--------------- 2 files changed, 24 insertions(+), 88 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 19a70a1..fa4556b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3527,60 +3527,30 @@ remoteConnectListAllSecrets(virConnectPtr conn, /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) -remoteGenericOpen(virConnectPtr conn, void **genericPrivateData) +remoteGenericOpen(virConnectPtr conn) { if (inside_daemon) return VIR_DRV_OPEN_DECLINED; if (conn->driver && STREQ(conn->driver->name, "remote")) { - struct private_data *priv; - - /* If we're here, the remote driver is already - * in use due to a) a QEMU uri, or b) a remote - * URI. So we can re-use existing connection */ - priv = conn->privateData; - remoteDriverLock(priv); - priv->localUses++; - *genericPrivateData = priv; - remoteDriverUnlock(priv); return VIR_DRV_OPEN_SUCCESS; } return VIR_DRV_OPEN_DECLINED; } -static int -remoteGenericClose(virConnectPtr conn, void **genericPrivateData) -{ - int rv = 0; - struct private_data *priv = *genericPrivateData; - - remoteDriverLock(priv); - priv->localUses--; - if (!priv->localUses) { - rv = doRemoteClose(conn, priv); - *genericPrivateData = NULL; - remoteDriverUnlock(priv); - virMutexDestroy(&priv->lock); - VIR_FREE(priv); - } - if (priv) - remoteDriverUnlock(priv); - return rv; -} - static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteNetworkOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->networkPrivateData); + return remoteGenericOpen(conn); } static int -remoteNetworkClose(virConnectPtr conn) +remoteNetworkClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->networkPrivateData); + return 0; } /*----------------------------------------------------------------------*/ @@ -3589,13 +3559,13 @@ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteInterfaceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->interfacePrivateData); + return remoteGenericOpen(conn); } static int -remoteInterfaceClose(virConnectPtr conn) +remoteInterfaceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->interfacePrivateData); + return 0; } /*----------------------------------------------------------------------*/ @@ -3604,13 +3574,13 @@ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteStorageOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->storagePrivateData); + return remoteGenericOpen(conn); } static int -remoteStorageClose(virConnectPtr conn) +remoteStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->storagePrivateData); + return 0; } static char * @@ -3622,7 +3592,7 @@ remoteConnectFindStoragePoolSources(virConnectPtr conn, char *rv = NULL; remote_connect_find_storage_pool_sources_args args; remote_connect_find_storage_pool_sources_ret ret; - struct private_data *priv = conn->storagePrivateData; + struct private_data *priv = conn->privateData; remoteDriverLock(priv); @@ -3786,13 +3756,13 @@ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteNodeDeviceOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->nodeDevicePrivateData); + return remoteGenericOpen(conn); } static int -remoteNodeDeviceClose(virConnectPtr conn) +remoteNodeDeviceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->nodeDevicePrivateData); + return 0; } static int @@ -3800,8 +3770,6 @@ remoteNodeDeviceDettach(virNodeDevicePtr dev) { int rv = -1; remote_node_device_dettach_args args; - /* This method is unusual in that it uses the HV driver, not the devMon driver - * hence its use of privateData, instead of nodeDevicePrivateData */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -3827,10 +3795,6 @@ remoteNodeDeviceDetachFlags(virNodeDevicePtr dev, { int rv = -1; remote_node_device_detach_flags_args args; - /* This method is unusual in that it uses the HV driver, not the - * devMon driver hence its use of privateData, instead of - * nodeDevicePrivateData - */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -3856,8 +3820,6 @@ remoteNodeDeviceReAttach(virNodeDevicePtr dev) { int rv = -1; remote_node_device_re_attach_args args; - /* This method is unusual in that it uses the HV driver, not the devMon driver - * hence its use of privateData, instead of nodeDevicePrivateData */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -3907,13 +3869,13 @@ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteNWFilterOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->nwfilterPrivateData); + return remoteGenericOpen(conn); } static int -remoteNWFilterClose(virConnectPtr conn) +remoteNWFilterClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->nwfilterPrivateData); + return 0; } /*----------------------------------------------------------------------*/ @@ -5563,13 +5525,13 @@ static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { - return remoteGenericOpen(conn, &conn->secretPrivateData); + return remoteGenericOpen(conn); } static int -remoteSecretClose(virConnectPtr conn) +remoteSecretClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - return remoteGenericClose(conn, &conn->secretPrivateData); + return 0; } static unsigned char * @@ -5579,7 +5541,7 @@ remoteSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned char *rv = NULL; remote_secret_get_value_args args; remote_secret_get_value_ret ret; - struct private_data *priv = secret->conn->secretPrivateData; + struct private_data *priv = secret->conn->privateData; remoteDriverLock(priv); @@ -7660,7 +7622,7 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net, { int rv = -1; size_t i; - struct private_data *priv = net->conn->networkPrivateData; + struct private_data *priv = net->conn->privateData; remote_network_get_dhcp_leases_args args; remote_network_get_dhcp_leases_ret ret; @@ -7728,7 +7690,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, virDomainStatsRecordPtr **retStats, unsigned int flags) { - struct private_data *priv = conn->networkPrivateData; + struct private_data *priv = conn->privateData; int rv = -1; size_t i; remote_connect_get_all_domain_stats_args args; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 27093d2..b38d5bb 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1090,7 +1090,6 @@ elsif ($mode eq "client") { my @setters_list2 = (); my @free_list = (); my $priv_src = "conn"; - my $priv_name = "privateData"; my $call_args = "&args"; if ($argtype eq "void") { @@ -1106,7 +1105,6 @@ elsif ($mode eq "client") { !($argtype =~ m/^remote_node_device_lookup_by_name_/) and !($argtype =~ m/^remote_node_device_create_xml_/)) { $has_node_device = 1; - $priv_name = "nodeDevicePrivateData"; } foreach my $args_member (@{$call->{args_members}}) { @@ -1125,12 +1123,6 @@ elsif ($mode eq "client") { } else { $priv_src = "$arg_name->conn"; } - - if ($name =~ m/^storage_/) { - $priv_name = "storagePrivateData"; - } elsif (!($name =~ m/^domain/)) { - $priv_name = "${name}PrivateData"; - } } push(@args_list, "vir${type_name}Ptr $arg_name"); @@ -1258,16 +1250,6 @@ elsif ($mode eq "client") { push(@args_list, "virConnectPtr conn"); } - # fix priv_name for the NumOf* functions - if ($priv_name eq "privateData" and - !($call->{ProcName} =~ m/(Domains|DomainSnapshot)/) and - ($call->{ProcName} =~ m/NumOf(Defined|Domain)*(\S+)s/ or - $call->{ProcName} =~ m/List(Defined|Domain)*(\S+)s/)) { - my $prefix = lc $2; - $prefix =~ s/(pool|vol)$//; - $priv_name = "${prefix}PrivateData"; - } - # handle return values of the function my @ret_list = (); my @ret_list2 = (); @@ -1342,14 +1324,6 @@ elsif ($mode eq "client") { my $arg_name = $2; my $type_name = name_to_TypeName($name); - if ($name eq "node_device") { - $priv_name = "nodeDevicePrivateData"; - } elsif ($name =~ m/^storage_/) { - $priv_name = "storagePrivateData"; - } elsif (!($name =~ m/^domain/)) { - $priv_name = "${name}PrivateData"; - } - if ($call->{ProcName} eq "DomainCreateWithFlags") { # SPECIAL: virDomainCreateWithFlags updates the given # domain object instead of returning a new one @@ -1475,7 +1449,7 @@ elsif ($mode eq "client") { print ")\n"; print "{\n"; print " $single_ret_var;\n"; - print " struct private_data *priv = $priv_src->$priv_name;\n"; + print " struct private_data *priv = $priv_src->privateData;\n"; foreach my $var (@vars_list) { print " $var;\n"; -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
Since the secondary drivers are only active when the primary driver is also the remote driver, there is no need to use the different type specific privateData fields. --- src/remote/remote_driver.c | 84 +++++++++++++--------------------------------- src/rpc/gendispatch.pl | 28 +--------------- 2 files changed, 24 insertions(+), 88 deletions(-)
Another nice cleanup. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Since the secondary drivers are only active when the primary driver is also the ESX driver, there is no need to use the different type specific privateData fields. --- src/esx/esx_device_monitor.c | 6 +---- src/esx/esx_interface_driver.c | 16 +++++--------- src/esx/esx_network_driver.c | 20 +++++++---------- src/esx/esx_nwfilter_driver.c | 6 +---- src/esx/esx_secret_driver.c | 6 +---- src/esx/esx_storage_backend_iscsi.c | 26 +++++++++++----------- src/esx/esx_storage_backend_vmfs.c | 36 +++++++++++++++--------------- src/esx/esx_storage_driver.c | 44 +++++++++++++++++-------------------- 8 files changed, 68 insertions(+), 92 deletions(-) diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c index c02b00f..613cef6 100644 --- a/src/esx/esx_device_monitor.c +++ b/src/esx/esx_device_monitor.c @@ -47,18 +47,14 @@ esxNodeDeviceOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->nodeDevicePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxNodeDeviceClose(virConnectPtr conn) +esxNodeDeviceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nodeDevicePrivateData = NULL; - return 0; } diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c index ecc5313..5fb9b1a 100644 --- a/src/esx/esx_interface_driver.c +++ b/src/esx/esx_interface_driver.c @@ -50,18 +50,14 @@ esxInterfaceOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->interfacePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxInterfaceClose(virConnectPtr conn) +esxInterfaceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->interfacePrivateData = NULL; - return 0; } @@ -70,7 +66,7 @@ esxInterfaceClose(virConnectPtr conn) static int esxConnectNumOfInterfaces(virConnectPtr conn) { - esxPrivate *priv = conn->interfacePrivateData; + esxPrivate *priv = conn->privateData; esxVI_PhysicalNic *physicalNicList = NULL; esxVI_PhysicalNic *physicalNic = NULL; int count = 0; @@ -96,7 +92,7 @@ static int esxConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames) { bool success = false; - esxPrivate *priv = conn->interfacePrivateData; + esxPrivate *priv = conn->privateData; esxVI_PhysicalNic *physicalNicList = NULL; esxVI_PhysicalNic *physicalNic = NULL; int count = 0; @@ -161,7 +157,7 @@ static virInterfacePtr esxInterfaceLookupByName(virConnectPtr conn, const char *name) { virInterfacePtr iface = NULL; - esxPrivate *priv = conn->interfacePrivateData; + esxPrivate *priv = conn->privateData; esxVI_PhysicalNic *physicalNic = NULL; if (esxVI_EnsureSession(priv->primary) < 0 || @@ -183,7 +179,7 @@ static virInterfacePtr esxInterfaceLookupByMACString(virConnectPtr conn, const char *mac) { virInterfacePtr iface = NULL; - esxPrivate *priv = conn->interfacePrivateData; + esxPrivate *priv = conn->privateData; esxVI_PhysicalNic *physicalNic = NULL; if (esxVI_EnsureSession(priv->primary) < 0 || @@ -205,7 +201,7 @@ static char * esxInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags) { char *xml = NULL; - esxPrivate *priv = iface->conn->interfacePrivateData; + esxPrivate *priv = iface->conn->privateData; esxVI_PhysicalNic *physicalNic = NULL; virInterfaceDef def; bool hasAddress = false; diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index c6f325c..6fae4ff 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -56,18 +56,14 @@ esxNetworkOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->networkPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxNetworkClose(virConnectPtr conn) +esxNetworkClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->networkPrivateData = NULL; - return 0; } @@ -76,7 +72,7 @@ esxNetworkClose(virConnectPtr conn) static int esxConnectNumOfNetworks(virConnectPtr conn) { - esxPrivate *priv = conn->networkPrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; int count = 0; @@ -103,7 +99,7 @@ static int esxConnectListNetworks(virConnectPtr conn, char **const names, int maxnames) { bool success = false; - esxPrivate *priv = conn->networkPrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; int count = 0; @@ -169,7 +165,7 @@ static virNetworkPtr esxNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virNetworkPtr network = NULL; - esxPrivate *priv = conn->networkPrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; unsigned char md5[MD5_DIGEST_SIZE]; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -214,7 +210,7 @@ static virNetworkPtr esxNetworkLookupByName(virConnectPtr conn, const char *name) { virNetworkPtr network = NULL; - esxPrivate *priv = conn->networkPrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; unsigned char md5[MD5_DIGEST_SIZE]; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -318,7 +314,7 @@ static virNetworkPtr esxNetworkDefineXML(virConnectPtr conn, const char *xml) { virNetworkPtr network = NULL; - esxPrivate *priv = conn->networkPrivateData; + esxPrivate *priv = conn->privateData; virNetworkDefPtr def = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; esxVI_HostPortGroup *hostPortGroupList = NULL; @@ -527,7 +523,7 @@ static int esxNetworkUndefine(virNetworkPtr network) { int result = -1; - esxPrivate *priv = network->conn->networkPrivateData; + esxPrivate *priv = network->conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; esxVI_HostPortGroup *hostPortGroupList = NULL; esxVI_String *hostPortGroupKey = NULL; @@ -670,7 +666,7 @@ static char * esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) { char *xml = NULL; - esxPrivate *priv = network_->conn->networkPrivateData; + esxPrivate *priv = network_->conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; int count = 0; esxVI_PhysicalNic *physicalNicList = NULL; diff --git a/src/esx/esx_nwfilter_driver.c b/src/esx/esx_nwfilter_driver.c index 3cf70d0..d7ab60f 100644 --- a/src/esx/esx_nwfilter_driver.c +++ b/src/esx/esx_nwfilter_driver.c @@ -47,18 +47,14 @@ esxNWFilterOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->nwfilterPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxNWFilterClose(virConnectPtr conn) +esxNWFilterClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nwfilterPrivateData = NULL; - return 0; } diff --git a/src/esx/esx_secret_driver.c b/src/esx/esx_secret_driver.c index 558cf07..4ce8ae4 100644 --- a/src/esx/esx_secret_driver.c +++ b/src/esx/esx_secret_driver.c @@ -45,18 +45,14 @@ esxSecretOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, return VIR_DRV_OPEN_DECLINED; } - conn->secretPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxSecretClose(virConnectPtr conn) +esxSecretClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->secretPrivateData = NULL; - return 0; } diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 41f895d..2e2a270 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -54,7 +54,7 @@ esxConnectNumOfStoragePools(virConnectPtr conn) { bool success = false; int count = 0; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; esxVI_HostInternetScsiHbaStaticTarget *target; @@ -99,7 +99,7 @@ esxConnectListStoragePools(virConnectPtr conn, char **const names, { bool success = false; int count = 0; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; esxVI_HostInternetScsiHbaStaticTarget *target; size_t i; @@ -156,7 +156,7 @@ static virStoragePoolPtr esxStoragePoolLookupByName(virConnectPtr conn, const char *name) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHbaStaticTarget *target = NULL; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ unsigned char md5[MD5_DIGEST_SIZE]; @@ -199,7 +199,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virStoragePoolPtr pool = NULL; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; esxVI_HostInternetScsiHbaStaticTarget *target; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -248,7 +248,7 @@ esxStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) { int result = -1; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; virCheckFlags(0, -1); @@ -295,7 +295,7 @@ static char * esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) { char *xml = NULL; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; esxVI_HostInternetScsiHbaStaticTarget *target; virStoragePoolDef def; @@ -358,7 +358,7 @@ static int esxStoragePoolNumOfVolumes(virStoragePoolPtr pool) { int count = 0; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL; esxVI_HostScsiTopologyLun *hostScsiTopologyLun; @@ -386,7 +386,7 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, { bool success = false; int count = 0; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL; esxVI_HostScsiTopologyLun *hostScsiTopologyLun; esxVI_ScsiLun *scsiLunList = NULL; @@ -445,7 +445,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) { virStorageVolPtr volume = NULL; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -491,7 +491,7 @@ static virStorageVolPtr esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { virStorageVolPtr volume = NULL; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; @@ -539,7 +539,7 @@ static virStorageVolPtr esxStorageVolLookupByKey(virConnectPtr conn, const char *key) { virStorageVolPtr volume = NULL; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; char *poolName = NULL; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; @@ -626,7 +626,7 @@ esxStorageVolGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) { int result = -1; - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; @@ -672,7 +672,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, unsigned int flags) { char *xml = NULL; - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStoragePoolDef pool; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index cf0da84..2e163c1 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -116,7 +116,7 @@ static int esxConnectNumOfStoragePools(virConnectPtr conn) { int count = 0; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_ObjectContent *datastoreList = NULL; esxVI_ObjectContent *datastore = NULL; @@ -141,7 +141,7 @@ esxConnectListStoragePools(virConnectPtr conn, char **const names, const int maxnames) { bool success = false; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_String *propertyNameList = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_ObjectContent *datastoreList = NULL; @@ -204,7 +204,7 @@ static virStoragePoolPtr esxStoragePoolLookupByName(virConnectPtr conn, const char *name) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -257,7 +257,7 @@ static virStoragePoolPtr esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastoreList = NULL; esxVI_ObjectContent *datastore = NULL; @@ -326,7 +326,7 @@ static int esxStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) { int result = -1; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_ObjectContent *datastore = NULL; virCheckFlags(0, -1); @@ -352,7 +352,7 @@ esxStoragePoolGetInfo(virStoragePoolPtr pool, virStoragePoolInfoPtr info) { int result = -1; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; @@ -411,7 +411,7 @@ esxStoragePoolGetInfo(virStoragePoolPtr pool, static char * esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; @@ -531,7 +531,7 @@ static int esxStoragePoolNumOfVolumes(virStoragePoolPtr pool) { bool success = false; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; esxVI_FileInfo *fileInfo = NULL; @@ -566,7 +566,7 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, int maxnames) { bool success = false; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; esxVI_FileInfo *fileInfo = NULL; @@ -646,7 +646,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) { virStorageVolPtr volume = NULL; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; char *datastorePath = NULL; char *key = NULL; @@ -674,7 +674,7 @@ static virStorageVolPtr esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { virStorageVolPtr volume = NULL; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; char *datastoreName = NULL; char *directoryAndFileName = NULL; char *key = NULL; @@ -706,7 +706,7 @@ static virStorageVolPtr esxStorageVolLookupByKey(virConnectPtr conn, const char *key) { virStorageVolPtr volume = NULL; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastoreList = NULL; esxVI_ObjectContent *datastore = NULL; @@ -842,7 +842,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, unsigned int flags) { virStorageVolPtr volume = NULL; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; virStorageVolDefPtr def = NULL; char *tmp; @@ -1064,7 +1064,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, unsigned int flags) { virStorageVolPtr volume = NULL; - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; char *sourceDatastorePath = NULL; virStorageVolDefPtr def = NULL; @@ -1248,7 +1248,7 @@ static int esxStorageVolDelete(virStorageVolPtr volume, unsigned int flags) { int result = -1; - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; @@ -1291,7 +1291,7 @@ static int esxStorageVolWipe(virStorageVolPtr volume, unsigned int flags) { int result = -1; - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; @@ -1335,7 +1335,7 @@ esxStorageVolGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) { int result = -1; - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL; @@ -1379,7 +1379,7 @@ static char * esxStorageVolGetXMLDesc(virStorageVolPtr volume, unsigned int flags) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStoragePoolDef pool; char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index cae53ed..ff3f167 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -64,18 +64,14 @@ esxStorageOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->storagePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -esxStorageClose(virConnectPtr conn) +esxStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->storagePrivateData = NULL; - return 0; } @@ -85,7 +81,7 @@ static int esxConnectNumOfStoragePools(virConnectPtr conn) { int count = 0; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; size_t i; int tmp; @@ -112,7 +108,7 @@ static int esxConnectListStoragePools(virConnectPtr conn, char **const names, int maxnames) { bool success = false; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; int count = 0; size_t i; int tmp; @@ -174,7 +170,7 @@ esxConnectListDefinedStoragePools(virConnectPtr conn ATTRIBUTE_UNUSED, static virStoragePoolPtr esxStoragePoolLookupByName(virConnectPtr conn, const char *name) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; size_t i; virStoragePoolPtr pool; @@ -203,7 +199,7 @@ esxStoragePoolLookupByName(virConnectPtr conn, const char *name) static virStoragePoolPtr esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; size_t i; virStoragePoolPtr pool; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; @@ -242,7 +238,7 @@ esxStoragePoolLookupByVolume(virStorageVolPtr volume) static int esxStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, -1); @@ -259,7 +255,7 @@ esxStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) static int esxStoragePoolGetInfo(virStoragePoolPtr pool, virStoragePoolInfoPtr info) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, -1); @@ -278,7 +274,7 @@ esxStoragePoolGetInfo(virStoragePoolPtr pool, virStoragePoolInfoPtr info) static char * esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, NULL); @@ -325,7 +321,7 @@ esxStoragePoolSetAutostart(virStoragePoolPtr pool ATTRIBUTE_UNUSED, static int esxStoragePoolNumOfVolumes(virStoragePoolPtr pool) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, -1); @@ -343,7 +339,7 @@ static int esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, int maxnames) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, -1); @@ -360,7 +356,7 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, static virStorageVolPtr esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, NULL); @@ -377,7 +373,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) static virStorageVolPtr esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; @@ -409,7 +405,7 @@ static virStorageVolPtr esxStorageVolLookupByKey(virConnectPtr conn, const char *key) { virStorageVolPtr volume; - esxPrivate *priv = conn->storagePrivateData; + esxPrivate *priv = conn->privateData; size_t i; if (esxVI_EnsureSession(priv->primary) < 0) { @@ -437,7 +433,7 @@ static virStorageVolPtr esxStorageVolCreateXML(virStoragePoolPtr pool, const char *xmldesc, unsigned int flags) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, NULL); @@ -455,7 +451,7 @@ static virStorageVolPtr esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, const char *xmldesc, virStorageVolPtr sourceVolume, unsigned int flags) { - esxPrivate *priv = pool->conn->storagePrivateData; + esxPrivate *priv = pool->conn->privateData; virStorageDriverPtr backend = pool->privateData; virCheckNonNullArgReturn(pool->privateData, NULL); @@ -472,7 +468,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, const char *xmldesc, static int esxStorageVolDelete(virStorageVolPtr volume, unsigned int flags) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStorageDriverPtr backend = volume->privateData; virCheckNonNullArgReturn(volume->privateData, -1); @@ -489,7 +485,7 @@ esxStorageVolDelete(virStorageVolPtr volume, unsigned int flags) static int esxStorageVolWipe(virStorageVolPtr volume, unsigned int flags) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStorageDriverPtr backend = volume->privateData; virCheckNonNullArgReturn(volume->privateData, -1); @@ -506,7 +502,7 @@ esxStorageVolWipe(virStorageVolPtr volume, unsigned int flags) static int esxStorageVolGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStorageDriverPtr backend = volume->privateData; virCheckNonNullArgReturn(volume->privateData, -1); @@ -523,7 +519,7 @@ esxStorageVolGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) static char * esxStorageVolGetXMLDesc(virStorageVolPtr volume, unsigned int flags) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStorageDriverPtr backend = volume->privateData; virCheckNonNullArgReturn(volume->privateData, NULL); @@ -540,7 +536,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, unsigned int flags) static char * esxStorageVolGetPath(virStorageVolPtr volume) { - esxPrivate *priv = volume->conn->storagePrivateData; + esxPrivate *priv = volume->conn->privateData; virStorageDriverPtr backend = volume->privateData; virCheckNonNullArgReturn(volume->privateData, NULL); -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
Since the secondary drivers are only active when the primary driver is also the ESX driver, there is no need to use the different type specific privateData fields. --- src/esx/esx_device_monitor.c | 6 +---- src/esx/esx_interface_driver.c | 16 +++++--------- src/esx/esx_network_driver.c | 20 +++++++---------- src/esx/esx_nwfilter_driver.c | 6 +---- src/esx/esx_secret_driver.c | 6 +---- src/esx/esx_storage_backend_iscsi.c | 26 +++++++++++----------- src/esx/esx_storage_backend_vmfs.c | 36 +++++++++++++++--------------- src/esx/esx_storage_driver.c | 44 +++++++++++++++++-------------------- 8 files changed, 68 insertions(+), 92 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Since the secondary drivers are only active when the primary driver is also the Hyper-V driver, there is no need to use the different type specific privateData fields. --- src/hyperv/hyperv_device_monitor.c | 6 +----- src/hyperv/hyperv_interface_driver.c | 6 +----- src/hyperv/hyperv_network_driver.c | 6 +----- src/hyperv/hyperv_nwfilter_driver.c | 6 +----- src/hyperv/hyperv_secret_driver.c | 6 +----- src/hyperv/hyperv_storage_driver.c | 6 +----- 6 files changed, 6 insertions(+), 30 deletions(-) diff --git a/src/hyperv/hyperv_device_monitor.c b/src/hyperv/hyperv_device_monitor.c index 5332eb2..23411cd 100644 --- a/src/hyperv/hyperv_device_monitor.c +++ b/src/hyperv/hyperv_device_monitor.c @@ -44,18 +44,14 @@ hypervNodeDeviceOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->nodeDevicePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervNodeDeviceClose(virConnectPtr conn) +hypervNodeDeviceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nodeDevicePrivateData = NULL; - return 0; } diff --git a/src/hyperv/hyperv_interface_driver.c b/src/hyperv/hyperv_interface_driver.c index b93cf30..9b9e4e6 100644 --- a/src/hyperv/hyperv_interface_driver.c +++ b/src/hyperv/hyperv_interface_driver.c @@ -44,18 +44,14 @@ hypervInterfaceOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->interfacePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervInterfaceClose(virConnectPtr conn) +hypervInterfaceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->interfacePrivateData = NULL; - return 0; } diff --git a/src/hyperv/hyperv_network_driver.c b/src/hyperv/hyperv_network_driver.c index 6f54f44..ab2daad 100644 --- a/src/hyperv/hyperv_network_driver.c +++ b/src/hyperv/hyperv_network_driver.c @@ -44,18 +44,14 @@ hypervNetworkOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->networkPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervNetworkClose(virConnectPtr conn) +hypervNetworkClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->networkPrivateData = NULL; - return 0; } diff --git a/src/hyperv/hyperv_nwfilter_driver.c b/src/hyperv/hyperv_nwfilter_driver.c index a82db92..6165bf7 100644 --- a/src/hyperv/hyperv_nwfilter_driver.c +++ b/src/hyperv/hyperv_nwfilter_driver.c @@ -44,18 +44,14 @@ hypervNWFilterOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->nwfilterPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervNWFilterClose(virConnectPtr conn) +hypervNWFilterClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nwfilterPrivateData = NULL; - return 0; } diff --git a/src/hyperv/hyperv_secret_driver.c b/src/hyperv/hyperv_secret_driver.c index 8176484..65587f0 100644 --- a/src/hyperv/hyperv_secret_driver.c +++ b/src/hyperv/hyperv_secret_driver.c @@ -44,18 +44,14 @@ hypervSecretOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->secretPrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervSecretClose(virConnectPtr conn) +hypervSecretClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->secretPrivateData = NULL; - return 0; } diff --git a/src/hyperv/hyperv_storage_driver.c b/src/hyperv/hyperv_storage_driver.c index af3cca4..e3d4170 100644 --- a/src/hyperv/hyperv_storage_driver.c +++ b/src/hyperv/hyperv_storage_driver.c @@ -44,18 +44,14 @@ hypervStorageOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; } - conn->storagePrivateData = conn->privateData; - return VIR_DRV_OPEN_SUCCESS; } static int -hypervStorageClose(virConnectPtr conn) +hypervStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->storagePrivateData = NULL; - return 0; } -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
Since the secondary drivers are only active when the primary driver is also the Hyper-V driver, there is no need to use the different type specific privateData fields. --- src/hyperv/hyperv_device_monitor.c | 6 +----- src/hyperv/hyperv_interface_driver.c | 6 +----- src/hyperv/hyperv_network_driver.c | 6 +----- src/hyperv/hyperv_nwfilter_driver.c | 6 +----- src/hyperv/hyperv_secret_driver.c | 6 +----- src/hyperv/hyperv_storage_driver.c | 6 +----- 6 files changed, 6 insertions(+), 30 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The phyp_driver.h file has various declarations that are only ever used from phyp_driver.c, so they should be in the .c file only. --- src/phyp/phyp_driver.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ src/phyp/phyp_driver.h | 53 -------------------------------------------------- 2 files changed, 49 insertions(+), 53 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index a56f25d..caab499 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -41,6 +41,7 @@ #include <fcntl.h> #include <domain_event.h> #include <poll.h> +#include <libssh2.h> #include "internal.h" #include "virauth.h" @@ -63,6 +64,54 @@ VIR_LOG_INIT("phyp.phyp_driver"); +# define LPAR_EXEC_ERR -1 +# define SSH_CONN_ERR -2 /* error while trying to connect to remote host */ +# define SSH_CMD_ERR -3 /* error while trying to execute the remote cmd */ + +typedef struct _ConnectionData ConnectionData; +typedef ConnectionData *ConnectionDataPtr; +struct _ConnectionData { + LIBSSH2_SESSION *session; + int sock; +}; + +/* This is the lpar (domain) struct that relates + * the ID with UUID generated by the API + * */ +typedef struct _lpar lpar_t; +typedef lpar_t *lparPtr; +struct _lpar { + unsigned char uuid[VIR_UUID_BUFLEN]; + int id; +}; + +/* Struct that holds how many lpars (domains) we're + * handling and a pointer to an array of lpar structs + * */ +typedef struct _uuid_table uuid_table_t; +typedef uuid_table_t *uuid_tablePtr; +struct _uuid_table { + size_t nlpars; + lparPtr *lpars; +}; + +/* This is the main structure of the driver + * */ +typedef struct _phyp_driver phyp_driver_t; +typedef phyp_driver_t *phyp_driverPtr; +struct _phyp_driver { + uuid_tablePtr uuid_table; + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + int vios_id; + + /* system_type: + * 0 = hmc + * 127 = ivm + * */ + int system_type; + char *managed_system; +}; /* * URI: phyp://user@[hmc|ivm]/managed_system diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h index a5e7369..a82aafd 100644 --- a/src/phyp/phyp_driver.h +++ b/src/phyp/phyp_driver.h @@ -25,59 +25,6 @@ #ifndef PHYP_DRIVER_H # define PHYP_DRIVER_H -# include "conf/capabilities.h" -# include "conf/domain_conf.h" -# include <libssh2.h> - -# define LPAR_EXEC_ERR -1 -# define SSH_CONN_ERR -2 /* error while trying to connect to remote host */ -# define SSH_CMD_ERR -3 /* error while trying to execute the remote cmd */ - -typedef struct _ConnectionData ConnectionData; -typedef ConnectionData *ConnectionDataPtr; -struct _ConnectionData { - LIBSSH2_SESSION *session; - int sock; -}; - -/* This is the lpar (domain) struct that relates - * the ID with UUID generated by the API - * */ -typedef struct _lpar lpar_t; -typedef lpar_t *lparPtr; -struct _lpar { - unsigned char uuid[VIR_UUID_BUFLEN]; - int id; -}; - -/* Struct that holds how many lpars (domains) we're - * handling and a pointer to an array of lpar structs - * */ -typedef struct _uuid_table uuid_table_t; -typedef uuid_table_t *uuid_tablePtr; -struct _uuid_table { - size_t nlpars; - lparPtr *lpars; -}; - -/* This is the main structure of the driver - * */ -typedef struct _phyp_driver phyp_driver_t; -typedef phyp_driver_t *phyp_driverPtr; -struct _phyp_driver { - uuid_tablePtr uuid_table; - virCapsPtr caps; - virDomainXMLOptionPtr xmlopt; - int vios_id; - - /* system_type: - * 0 = hmc - * 127 = ivm - * */ - int system_type; - char *managed_system; -}; - int phypRegister(void); #endif /* PHYP_DRIVER_H */ -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
The phyp_driver.h file has various declarations that are only ever used from phyp_driver.c, so they should be in the .c file only. --- src/phyp/phyp_driver.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ src/phyp/phyp_driver.h | 53 -------------------------------------------------- 2 files changed, 49 insertions(+), 53 deletions(-)
ACK.
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index a56f25d..caab499 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -41,6 +41,7 @@ #include <fcntl.h> #include <domain_event.h> #include <poll.h> +#include <libssh2.h>
#include "internal.h" #include "virauth.h" @@ -63,6 +64,54 @@
VIR_LOG_INIT("phyp.phyp_driver");
+# define LPAR_EXEC_ERR -1 +# define SSH_CONN_ERR -2 /* error while trying to connect to remote host */ +# define SSH_CMD_ERR -3 /* error while trying to execute the remote cmd */
Hmm. These are under-parenthesized. Maybe you can touch them up while moving them? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
For inexplicable reasons the phyp driver defined two separate structs for holding its private data. One it keeps in privateData and the other it keeps in networkPrivateData. It uses them both from all API driver methods. Merge the two separate structs into one to remove this horrible abuse. --- src/phyp/phyp_driver.c | 190 +++++++++++++++++-------------------------------- 1 file changed, 66 insertions(+), 124 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index caab499..8d9e2bf 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -68,13 +68,6 @@ VIR_LOG_INIT("phyp.phyp_driver"); # define SSH_CONN_ERR -2 /* error while trying to connect to remote host */ # define SSH_CMD_ERR -3 /* error while trying to execute the remote cmd */ -typedef struct _ConnectionData ConnectionData; -typedef ConnectionData *ConnectionDataPtr; -struct _ConnectionData { - LIBSSH2_SESSION *session; - int sock; -}; - /* This is the lpar (domain) struct that relates * the ID with UUID generated by the API * */ @@ -100,6 +93,9 @@ struct _uuid_table { typedef struct _phyp_driver phyp_driver_t; typedef phyp_driver_t *phyp_driverPtr; struct _phyp_driver { + LIBSSH2_SESSION *session; + int sock; + uuid_tablePtr uuid_table; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; @@ -151,14 +147,14 @@ static char * phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, virConnectPtr conn) { + phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_CHANNEL *channel; - ConnectionData *connection_data = conn->networkPrivateData; virBuffer tex_ret = VIR_BUFFER_INITIALIZER; char *buffer = NULL; size_t buffer_size = 16384; int exitcode; int bytecount = 0; - int sock = connection_data->sock; + int sock = phyp_driver->sock; int rc = 0; if (VIR_ALLOC_N(buffer, buffer_size) < 0) @@ -296,8 +292,8 @@ phypExecInt(LIBSSH2_SESSION *session, virBufferPtr buf, virConnectPtr conn, static int phypGetSystemType(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; + phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; char *ret = NULL; int exit_status = 0; @@ -310,9 +306,8 @@ phypGetSystemType(virConnectPtr conn) static int phypGetVIOSPartitionID(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int id = -1; char *managed_system = phyp_driver->managed_system; @@ -376,9 +371,8 @@ phypCapsInit(void) static int phypConnectNumOfDomainsGeneric(virConnectPtr conn, unsigned int type) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int ndom = -1; char *managed_system = phyp_driver->managed_system; @@ -417,9 +411,8 @@ static int phypConnectListDomainsGeneric(virConnectPtr conn, int *ids, int nids, unsigned int type) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -506,8 +499,8 @@ phypUUIDTable_WriteFile(virConnectPtr conn) static int phypUUIDTable_Push(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; + phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; LIBSSH2_CHANNEL *channel = NULL; struct stat local_fileinfo; char buffer[1024]; @@ -691,8 +684,8 @@ phypUUIDTable_ReadFile(virConnectPtr conn) static int phypUUIDTable_Pull(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; + phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; LIBSSH2_CHANNEL *channel = NULL; struct stat fileinfo; char buffer[1024]; @@ -1134,7 +1127,6 @@ phypConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { LIBSSH2_SESSION *session = NULL; - ConnectionData *connection_data = NULL; int internal_socket; uuid_tablePtr uuid_table = NULL; phyp_driverPtr phyp_driver = NULL; @@ -1158,12 +1150,10 @@ phypConnectOpen(virConnectPtr conn, if (VIR_ALLOC(phyp_driver) < 0) goto failure; - if (VIR_ALLOC(uuid_table) < 0) - goto failure; + phyp_driver->sock = -1; - if (VIR_ALLOC(connection_data) < 0) + if (VIR_ALLOC(uuid_table) < 0) goto failure; - connection_data->sock = -1; if (conn->uri->path) { /* need to shift one byte in order to remove the first "/" of URI component */ @@ -1193,8 +1183,8 @@ phypConnectOpen(virConnectPtr conn, goto failure; } - connection_data->session = session; - connection_data->sock = internal_socket; + phyp_driver->session = session; + phyp_driver->sock = internal_socket; uuid_table->nlpars = 0; uuid_table->lpars = NULL; @@ -1211,7 +1201,6 @@ phypConnectOpen(virConnectPtr conn, goto failure; conn->privateData = phyp_driver; - conn->networkPrivateData = connection_data; if ((phyp_driver->system_type = phypGetSystemType(conn)) == -1) goto failure; @@ -1242,9 +1231,7 @@ phypConnectOpen(virConnectPtr conn, libssh2_session_free(session); } - if (connection_data) - VIR_FORCE_CLOSE(connection_data->sock); - VIR_FREE(connection_data); + VIR_FORCE_CLOSE(phyp_driver->sock); return VIR_DRV_OPEN_ERROR; } @@ -1252,9 +1239,8 @@ phypConnectOpen(virConnectPtr conn, static int phypConnectClose(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; libssh2_session_disconnect(session, "Disconnecting..."); libssh2_session_free(session); @@ -1265,8 +1251,7 @@ phypConnectClose(virConnectPtr conn) VIR_FREE(phyp_driver->managed_system); VIR_FREE(phyp_driver); - VIR_FORCE_CLOSE(connection_data->sock); - VIR_FREE(connection_data); + VIR_FORCE_CLOSE(phyp_driver->sock); return 0; } @@ -1290,13 +1275,12 @@ phypConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) static int phypConnectIsAlive(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; - + phyp_driverPtr phyp_driver = conn->privateData; /* XXX we should be able to do something better but this is simple, safe, * and good enough for now. In worst case, the function will return true * even though the connection is not alive. */ - if (connection_data && connection_data->session) + if (phyp_driver->session) return 1; else return 0; @@ -1383,9 +1367,8 @@ static unsigned long phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, int type) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int memory = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1407,9 +1390,8 @@ static unsigned long phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, int lpar_id, int type) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int vcpus = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1455,9 +1437,8 @@ static int phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, const char *lpar_name) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int remote_slot = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1478,9 +1459,8 @@ static char * phypGetBackingDevice(virConnectPtr conn, const char *managed_system, char *lpar_name) { - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *ret = NULL; int remote_slot = 0; @@ -1540,9 +1520,8 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, static char * phypGetLparProfile(virConnectPtr conn, int lpar_id) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; @@ -1565,9 +1544,8 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) static int phypGetVIOSNextSlotNumber(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1600,9 +1578,8 @@ static int phypCreateServerSCSIAdapter(virConnectPtr conn) { int result = -1; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1687,9 +1664,8 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) static char * phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1720,9 +1696,8 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) { int result = -1; virConnectPtr conn = domain->conn; - ConnectionData *connection_data = domain->conn->networkPrivateData; phyp_driverPtr phyp_driver = domain->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1874,9 +1849,8 @@ phypDomainAttachDevice(virDomainPtr domain, const char *xml) static char * phypStorageVolGetKey(virConnectPtr conn, const char *name) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1904,9 +1878,8 @@ phypStorageVolGetKey(virConnectPtr conn, const char *name) static char * phypGetStoragePoolDevice(virConnectPtr conn, char *name) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1934,9 +1907,8 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) static unsigned long int phypGetStoragePoolSize(virConnectPtr conn, char *name) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -1961,9 +1933,8 @@ static char * phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, unsigned int capacity) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; @@ -2113,9 +2084,8 @@ static char * phypStorageVolGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) { virConnectPtr conn = vol->conn; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2143,9 +2113,8 @@ phypStorageVolGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) static virStorageVolPtr phypStorageVolLookupByPath(virConnectPtr conn, const char *volname) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2189,9 +2158,8 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, const char *name) { int result = -1; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2316,9 +2284,8 @@ static char * phypStorageVolGetPath(virStorageVolPtr vol) { virConnectPtr conn = vol->conn; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2365,9 +2332,8 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, { bool success = false; virConnectPtr conn = pool->conn; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2429,9 +2395,8 @@ static int phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) { virConnectPtr conn = pool->conn; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int nvolumes = -1; char *managed_system = phyp_driver->managed_system; @@ -2457,9 +2422,8 @@ phypStoragePoolDestroy(virStoragePoolPtr pool) { int result = -1; virConnectPtr conn = pool->conn; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int vios_id = phyp_driver->vios_id; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; @@ -2494,9 +2458,8 @@ static int phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) { int result = -1; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virStoragePoolSource source = def->source; int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; @@ -2540,9 +2503,8 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) static int phypConnectNumOfStoragePools(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; int nsp = -1; char *managed_system = phyp_driver->managed_system; @@ -2567,9 +2529,8 @@ static int phypConnectListStoragePools(virConnectPtr conn, char **const pools, int npools) { bool success = false; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -2767,9 +2728,8 @@ phypInterfaceDestroy(virInterfacePtr iface, { virCheckFlags(0, -1); - ConnectionData *connection_data = iface->conn->networkPrivateData; phyp_driverPtr phyp_driver = iface->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virBuffer buf = VIR_BUFFER_INITIALIZER; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; @@ -2831,9 +2791,8 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, { virCheckFlags(0, NULL); - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virBuffer buf = VIR_BUFFER_INITIALIZER; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; @@ -2939,9 +2898,8 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, static virInterfacePtr phypInterfaceLookupByName(virConnectPtr conn, const char *name) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virBuffer buf = VIR_BUFFER_INITIALIZER; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; @@ -3002,9 +2960,8 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) static int phypInterfaceIsActive(virInterfacePtr iface) { - ConnectionData *connection_data = iface->conn->networkPrivateData; phyp_driverPtr phyp_driver = iface->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virBuffer buf = VIR_BUFFER_INITIALIZER; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; @@ -3025,9 +2982,8 @@ phypInterfaceIsActive(virInterfacePtr iface) static int phypConnectListInterfaces(virConnectPtr conn, char **const names, int nnames) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; @@ -3081,9 +3037,8 @@ phypConnectListInterfaces(virConnectPtr conn, char **const names, int nnames) static int phypConnectNumOfInterfaces(virConnectPtr conn) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; @@ -3104,9 +3059,8 @@ phypConnectNumOfInterfaces(virConnectPtr conn) static int phypGetLparState(virConnectPtr conn, unsigned int lpar_id) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *ret = NULL; int exit_status = 0; @@ -3141,8 +3095,7 @@ static int phypDiskType(virConnectPtr conn, char *backing_device) { phyp_driverPtr phyp_driver = conn->privateData; - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *ret = NULL; int exit_status = 0; @@ -3194,9 +3147,8 @@ static int phypConnectListDefinedDomains(virConnectPtr conn, char **const names, int nnames) { bool success = false; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -3251,9 +3203,8 @@ phypConnectListDefinedDomains(virConnectPtr conn, char **const names, int nnames static virDomainPtr phypDomainLookupByName(virConnectPtr conn, const char *lpar_name) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virDomainPtr dom = NULL; int lpar_id = 0; char *managed_system = phyp_driver->managed_system; @@ -3277,9 +3228,8 @@ phypDomainLookupByName(virConnectPtr conn, const char *lpar_name) static virDomainPtr phypDomainLookupByID(virConnectPtr conn, int lpar_id) { - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virDomainPtr dom = NULL; char *managed_system = phyp_driver->managed_system; unsigned char lpar_uuid[VIR_UUID_BUFLEN]; @@ -3304,9 +3254,8 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) static char * phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { - ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; virDomainDef def; char *managed_system = phyp_driver->managed_system; @@ -3358,9 +3307,8 @@ static int phypDomainResume(virDomainPtr dom) { int result = -1; - ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -3389,10 +3337,9 @@ static int phypDomainReboot(virDomainPtr dom, unsigned int flags) { int result = -1; - ConnectionData *connection_data = dom->conn->networkPrivateData; virConnectPtr conn = dom->conn; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -3424,10 +3371,9 @@ static int phypDomainShutdown(virDomainPtr dom) { int result = -1; - ConnectionData *connection_data = dom->conn->networkPrivateData; virConnectPtr conn = dom->conn; - LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -3494,9 +3440,8 @@ phypDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { int result = -1; - ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; @@ -3536,9 +3481,8 @@ static int phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) { int result = -1; - ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; char *ret = NULL; @@ -3606,11 +3550,10 @@ phypDomainCreateXML(virConnectPtr conn, { virCheckFlags(0, NULL); - ConnectionData *connection_data = conn->networkPrivateData; - LIBSSH2_SESSION *session = connection_data->session; + phyp_driverPtr phyp_driver = conn->privateData; + LIBSSH2_SESSION *session = phyp_driver->session; virDomainDefPtr def = NULL; virDomainPtr dom = NULL; - phyp_driverPtr phyp_driver = conn->privateData; uuid_tablePtr uuid_table = phyp_driver->uuid_table; lparPtr *lpars = uuid_table->lpars; size_t i = 0; @@ -3667,9 +3610,8 @@ static int phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { - ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; - LIBSSH2_SESSION *session = connection_data->session; + LIBSSH2_SESSION *session = phyp_driver->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
For inexplicable reasons the phyp driver defined two separate structs for holding its private data. One it keeps in privateData and the other it keeps in networkPrivateData. It uses them both from all API driver methods. Merge the two separate structs into one to remove this horrible abuse. --- src/phyp/phyp_driver.c | 190 +++++++++++++++++-------------------------------- 1 file changed, 66 insertions(+), 124 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Since the secondary drivers are only active when the primary driver is also the Parallels driver, there is no need to use the different type specific privateData fields. The object that was being stored in the storagePrivateData can easily be kept in the parallelsConn struct instead. --- src/parallels/parallels_network.c | 2 -- src/parallels/parallels_storage.c | 12 ++++++------ src/parallels/parallels_utils.h | 1 + 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index f41c97f..90217df 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -328,8 +328,6 @@ parallelsNetworkOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Parallels")) return VIR_DRV_OPEN_DECLINED; - conn->networkPrivateData = conn->privateData; - if (parallelsLoadNetworks(conn->privateData) < 0) return VIR_DRV_OPEN_DECLINED; diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index 53bcfcb..e1b6ea8 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -72,8 +72,8 @@ parallelsStorageClose(virConnectPtr conn) { parallelsConnPtr privconn = conn->privateData; - virStorageDriverStatePtr storageState = conn->storagePrivateData; - conn->storagePrivateData = NULL; + virStorageDriverStatePtr storageState = conn->privateData->storageState; + conn->privateData->storageState = NULL; parallelsStorageLock(storageState); virStoragePoolObjListFree(&privconn->pools); @@ -189,7 +189,7 @@ parallelsPoolCreateByPath(virConnectPtr conn, const char *path) if (!(pool = virStoragePoolObjAssignDef(pools, def))) goto error; - if (virStoragePoolObjSaveDef(conn->storagePrivateData, pool, def) < 0) { + if (virStoragePoolObjSaveDef(conn->privateData->storageState, pool, def) < 0) { virStoragePoolObjRemove(pools, pool); goto error; } @@ -404,7 +404,7 @@ parallelsPoolsAdd(virDomainObjPtr dom, static int parallelsLoadPools(virConnectPtr conn) { parallelsConnPtr privconn = conn->privateData; - virStorageDriverStatePtr storageState = conn->storagePrivateData; + virStorageDriverStatePtr storageState = conn->privateData->storageState; char *base = NULL; size_t i; @@ -475,7 +475,7 @@ parallelsStorageOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; } - conn->storagePrivateData = storageState; + conn->privateData->storageState = storageState; parallelsStorageLock(storageState); if (parallelsLoadPools(conn)) @@ -728,7 +728,7 @@ parallelsStoragePoolDefineXML(virConnectPtr conn, if (!(pool = virStoragePoolObjAssignDef(&privconn->pools, def))) goto cleanup; - if (virStoragePoolObjSaveDef(conn->storagePrivateData, pool, def) < 0) { + if (virStoragePoolObjSaveDef(conn->privateData->storageState, pool, def) < 0) { virStoragePoolObjRemove(&privconn->pools, pool); def = NULL; goto cleanup; diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index aef590f..0d770c5 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -49,6 +49,7 @@ struct _parallelsConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; + virStorageDriverStatePtr storageState; }; typedef struct _parallelsConn parallelsConn; -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
Since the secondary drivers are only active when the primary driver is also the Parallels driver, there is no need to use the different type specific privateData fields. The object that was being stored in the storagePrivateData can easily be kept in the parallelsConn struct instead. --- src/parallels/parallels_network.c | 2 -- src/parallels/parallels_storage.c | 12 ++++++------ src/parallels/parallels_utils.h | 1 + 3 files changed, 7 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Since the secondary drivers are only active when the primary driver is also the Test driver, there is no need to use the different type specific privateData fields. --- src/test/test_driver.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f435d03..0fe5dc9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3488,13 +3488,11 @@ static virDrvOpenStatus testNetworkOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->networkPrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testNetworkClose(virConnectPtr conn) +static int testNetworkClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->networkPrivateData = NULL; return 0; } @@ -4048,13 +4046,11 @@ static virDrvOpenStatus testInterfaceOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->interfacePrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testInterfaceClose(virConnectPtr conn) +static int testInterfaceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->interfacePrivateData = NULL; return 0; } @@ -4487,13 +4483,11 @@ static virDrvOpenStatus testStorageOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->storagePrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testStorageClose(virConnectPtr conn) +static int testStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->storagePrivateData = NULL; return 0; } @@ -5827,13 +5821,11 @@ static virDrvOpenStatus testNodeDeviceOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->nodeDevicePrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testNodeDeviceClose(virConnectPtr conn) +static int testNodeDeviceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nodeDevicePrivateData = NULL; return 0; } @@ -6298,13 +6290,11 @@ static virDrvOpenStatus testSecretOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->secretPrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testSecretClose(virConnectPtr conn) +static int testSecretClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->secretPrivateData = NULL; return 0; } @@ -6318,13 +6308,11 @@ static virDrvOpenStatus testNWFilterOpen(virConnectPtr conn, if (STRNEQ(conn->driver->name, "Test")) return VIR_DRV_OPEN_DECLINED; - conn->nwfilterPrivateData = conn->privateData; return VIR_DRV_OPEN_SUCCESS; } -static int testNWFilterClose(virConnectPtr conn) +static int testNWFilterClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->nwfilterPrivateData = NULL; return 0; } -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
Since the secondary drivers are only active when the primary driver is also the Test driver, there is no need to use the different type specific privateData fields. --- src/test/test_driver.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The shared storage driver is stateful and inside the daemon so there is no need to use the storagePrivateData field to get the driver handle. Just access the global driver handle directly. --- src/storage/storage_driver.c | 194 +++++++++++++++++++------------------------ 1 file changed, 86 insertions(+), 108 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7c518bf..fe6059a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -55,7 +55,7 @@ VIR_LOG_INIT("storage.storage_driver"); -static virStorageDriverStatePtr driverState; +static virStorageDriverStatePtr driver; static int storageStateCleanup(void); @@ -65,23 +65,23 @@ struct _virStorageVolStreamInfo { char *pool_name; }; -static void storageDriverLock(virStorageDriverStatePtr driver) +static void storageDriverLock(void) { virMutexLock(&driver->lock); } -static void storageDriverUnlock(virStorageDriverStatePtr driver) +static void storageDriverUnlock(void) { virMutexUnlock(&driver->lock); } static void -storageDriverAutostart(virStorageDriverStatePtr driver) +storageDriverAutostart(void) { size_t i; virConnectPtr conn = NULL; /* XXX Remove hardcoding of QEMU URI */ - if (driverState->privileged) + if (driver->privileged) conn = virConnectOpen("qemu:///system"); else conn = virConnectOpen("qemu:///session"); @@ -155,14 +155,14 @@ storageStateInitialize(bool privileged, { char *base = NULL; - if (VIR_ALLOC(driverState) < 0) + if (VIR_ALLOC(driver) < 0) return -1; - if (virMutexInit(&driverState->lock) < 0) { - VIR_FREE(driverState); + if (virMutexInit(&driver->lock) < 0) { + VIR_FREE(driver); return -1; } - storageDriverLock(driverState); + storageDriverLock(); if (privileged) { if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0) @@ -172,32 +172,32 @@ storageStateInitialize(bool privileged, if (!base) goto error; } - driverState->privileged = privileged; + driver->privileged = privileged; /* Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/... (session) or * /etc/libvirt/storage/... (system). */ - if (virAsprintf(&driverState->configDir, + if (virAsprintf(&driver->configDir, "%s/storage", base) == -1) goto error; - if (virAsprintf(&driverState->autostartDir, + if (virAsprintf(&driver->autostartDir, "%s/storage/autostart", base) == -1) goto error; VIR_FREE(base); - if (virStoragePoolLoadAllConfigs(&driverState->pools, - driverState->configDir, - driverState->autostartDir) < 0) + if (virStoragePoolLoadAllConfigs(&driver->pools, + driver->configDir, + driver->autostartDir) < 0) goto error; - storageDriverUnlock(driverState); + storageDriverUnlock(); return 0; error: VIR_FREE(base); - storageDriverUnlock(driverState); + storageDriverUnlock(); storageStateCleanup(); return -1; } @@ -210,12 +210,12 @@ storageStateInitialize(bool privileged, static void storageStateAutoStart(void) { - if (!driverState) + if (!driver) return; - storageDriverLock(driverState); - storageDriverAutostart(driverState); - storageDriverUnlock(driverState); + storageDriverLock(); + storageDriverAutostart(); + storageDriverUnlock(); } /** @@ -227,15 +227,15 @@ storageStateAutoStart(void) static int storageStateReload(void) { - if (!driverState) + if (!driver) return -1; - storageDriverLock(driverState); - virStoragePoolLoadAllConfigs(&driverState->pools, - driverState->configDir, - driverState->autostartDir); - storageDriverAutostart(driverState); - storageDriverUnlock(driverState); + storageDriverLock(); + virStoragePoolLoadAllConfigs(&driver->pools, + driver->configDir, + driver->autostartDir); + storageDriverAutostart(); + storageDriverUnlock(); return 0; } @@ -249,19 +249,19 @@ storageStateReload(void) static int storageStateCleanup(void) { - if (!driverState) + if (!driver) return -1; - storageDriverLock(driverState); + storageDriverLock(); /* free inactive pools */ - virStoragePoolObjListFree(&driverState->pools); + virStoragePoolObjListFree(&driver->pools); - VIR_FREE(driverState->configDir); - VIR_FREE(driverState->autostartDir); - storageDriverUnlock(driverState); - virMutexDestroy(&driverState->lock); - VIR_FREE(driverState); + VIR_FREE(driver->configDir); + VIR_FREE(driver->autostartDir); + storageDriverUnlock(); + virMutexDestroy(&driver->lock); + VIR_FREE(driver); return 0; } @@ -272,13 +272,12 @@ static virStoragePoolPtr storagePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - virStorageDriverStatePtr driver = conn->storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; - storageDriverLock(driver); + storageDriverLock(); pool = virStoragePoolObjFindByUUID(&driver->pools, uuid); - storageDriverUnlock(driver); + storageDriverUnlock(); if (!pool) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -303,13 +302,12 @@ static virStoragePoolPtr storagePoolLookupByName(virConnectPtr conn, const char *name) { - virStorageDriverStatePtr driver = conn->storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; - storageDriverLock(driver); + storageDriverLock(); pool = virStoragePoolObjFindByName(&driver->pools, name); - storageDriverUnlock(driver); + storageDriverUnlock(); if (!pool) { virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -331,13 +329,12 @@ storagePoolLookupByName(virConnectPtr conn, static virStoragePoolPtr storagePoolLookupByVolume(virStorageVolPtr vol) { - virStorageDriverStatePtr driver = vol->conn->storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolPtr ret = NULL; - storageDriverLock(driver); + storageDriverLock(); pool = virStoragePoolObjFindByName(&driver->pools, vol->pool); - storageDriverUnlock(driver); + storageDriverUnlock(); if (!pool) { virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -357,37 +354,34 @@ storagePoolLookupByVolume(virStorageVolPtr vol) } static virDrvOpenStatus -storageOpen(virConnectPtr conn, +storageOpen(virConnectPtr conn ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); - if (!driverState) + if (!driver) return VIR_DRV_OPEN_DECLINED; - conn->storagePrivateData = driverState; return VIR_DRV_OPEN_SUCCESS; } static int -storageClose(virConnectPtr conn) +storageClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->storagePrivateData = NULL; return 0; } static int storageConnectNumOfStoragePools(virConnectPtr conn) { - virStorageDriverStatePtr driver = conn->storagePrivateData; size_t i; int nactive = 0; if (virConnectNumOfStoragePoolsEnsureACL(conn) < 0) return -1; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); @@ -396,7 +390,7 @@ storageConnectNumOfStoragePools(virConnectPtr conn) nactive++; virStoragePoolObjUnlock(obj); } - storageDriverUnlock(driver); + storageDriverUnlock(); return nactive; } @@ -406,14 +400,13 @@ storageConnectListStoragePools(virConnectPtr conn, char **const names, int nnames) { - virStorageDriverStatePtr driver = conn->storagePrivateData; int got = 0; size_t i; if (virConnectListStoragePoolsEnsureACL(conn) < 0) return -1; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count && got < nnames; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); @@ -427,11 +420,11 @@ storageConnectListStoragePools(virConnectPtr conn, } virStoragePoolObjUnlock(obj); } - storageDriverUnlock(driver); + storageDriverUnlock(); return got; cleanup: - storageDriverUnlock(driver); + storageDriverUnlock(); for (i = 0; i < got; i++) VIR_FREE(names[i]); memset(names, 0, nnames * sizeof(*names)); @@ -441,14 +434,13 @@ storageConnectListStoragePools(virConnectPtr conn, static int storageConnectNumOfDefinedStoragePools(virConnectPtr conn) { - virStorageDriverStatePtr driver = conn->storagePrivateData; size_t i; int nactive = 0; if (virConnectNumOfDefinedStoragePoolsEnsureACL(conn) < 0) return -1; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); @@ -457,7 +449,7 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) nactive++; virStoragePoolObjUnlock(obj); } - storageDriverUnlock(driver); + storageDriverUnlock(); return nactive; } @@ -467,14 +459,13 @@ storageConnectListDefinedStoragePools(virConnectPtr conn, char **const names, int nnames) { - virStorageDriverStatePtr driver = conn->storagePrivateData; int got = 0; size_t i; if (virConnectListDefinedStoragePoolsEnsureACL(conn) < 0) return -1; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count && got < nnames; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; virStoragePoolObjLock(obj); @@ -488,11 +479,11 @@ storageConnectListDefinedStoragePools(virConnectPtr conn, } virStoragePoolObjUnlock(obj); } - storageDriverUnlock(driver); + storageDriverUnlock(); return got; cleanup: - storageDriverUnlock(driver); + storageDriverUnlock(); for (i = 0; i < got; i++) { VIR_FREE(names[i]); } @@ -543,18 +534,17 @@ storageConnectFindStoragePoolSources(virConnectPtr conn, static virStoragePoolObjPtr virStoragePoolObjFromStoragePool(virStoragePoolPtr pool) { - virStorageDriverStatePtr driver = pool->conn->storagePrivateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; virStoragePoolObjPtr ret; - storageDriverLock(driver); + storageDriverLock(); if (!(ret = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid))) { virUUIDFormat(pool->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, _("no storage pool with matching uuid '%s' (%s)"), uuidstr, pool->name); } - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -602,7 +592,6 @@ storagePoolCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStorageDriverStatePtr driver = conn->storagePrivateData; virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; @@ -610,7 +599,7 @@ storagePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - storageDriverLock(driver); + storageDriverLock(); if (!(def = virStoragePoolDefParseString(xml))) goto cleanup; @@ -654,7 +643,7 @@ storagePoolCreateXML(virConnectPtr conn, virStoragePoolDefFree(def); if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -663,14 +652,13 @@ storagePoolDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStorageDriverStatePtr driver = conn->storagePrivateData; virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; virCheckFlags(0, NULL); - storageDriverLock(driver); + storageDriverLock(); if (!(def = virStoragePoolDefParseString(xml))) goto cleanup; @@ -704,18 +692,17 @@ storagePoolDefineXML(virConnectPtr conn, virStoragePoolDefFree(def); if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } static int storagePoolUndefine(virStoragePoolPtr obj) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; int ret = -1; - storageDriverLock(driver); + storageDriverLock(); if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); @@ -762,7 +749,7 @@ storagePoolUndefine(virStoragePoolPtr obj) cleanup: if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -848,12 +835,11 @@ storagePoolBuild(virStoragePoolPtr obj, static int storagePoolDestroy(virStoragePoolPtr obj) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; - storageDriverLock(driver); + storageDriverLock(); if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); @@ -904,7 +890,7 @@ storagePoolDestroy(virStoragePoolPtr obj) cleanup: if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -959,14 +945,13 @@ static int storagePoolRefresh(virStoragePoolPtr obj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; virCheckFlags(0, -1); - storageDriverLock(driver); + storageDriverLock(); if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); @@ -1013,7 +998,7 @@ storagePoolRefresh(virStoragePoolPtr obj, cleanup: if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -1106,11 +1091,10 @@ static int storagePoolSetAutostart(virStoragePoolPtr obj, int autostart) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; int ret = -1; - storageDriverLock(driver); + storageDriverLock(); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { @@ -1164,7 +1148,7 @@ storagePoolSetAutostart(virStoragePoolPtr obj, cleanup: if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -1349,11 +1333,10 @@ static virStorageVolPtr storageVolLookupByKey(virConnectPtr conn, const char *key) { - virStorageDriverStatePtr driver = conn->storagePrivateData; size_t i; virStorageVolPtr ret = NULL; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count && !ret; i++) { virStoragePoolObjLock(driver->pools.objs[i]); if (virStoragePoolObjIsActive(driver->pools.objs[i])) { @@ -1381,7 +1364,7 @@ storageVolLookupByKey(virConnectPtr conn, _("no storage vol with matching key %s"), key); cleanup: - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -1389,7 +1372,6 @@ static virStorageVolPtr storageVolLookupByPath(virConnectPtr conn, const char *path) { - virStorageDriverStatePtr driver = conn->storagePrivateData; size_t i; virStorageVolPtr ret = NULL; char *cleanpath; @@ -1398,7 +1380,7 @@ storageVolLookupByPath(virConnectPtr conn, if (!cleanpath) return NULL; - storageDriverLock(driver); + storageDriverLock(); for (i = 0; i < driver->pools.count && !ret; i++) { virStoragePoolObjPtr pool = driver->pools.objs[i]; virStorageVolDefPtr vol; @@ -1476,7 +1458,7 @@ storageVolLookupByPath(virConnectPtr conn, cleanup: VIR_FREE(cleanpath); - storageDriverUnlock(driver); + storageDriverUnlock(); return ret; } @@ -1532,14 +1514,13 @@ virStorageVolDefFromVol(virStorageVolPtr obj, virStoragePoolObjPtr *pool, virStorageBackendPtr *backend) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStorageVolDefPtr vol = NULL; *pool = NULL; - storageDriverLock(driver); + storageDriverLock(); *pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); - storageDriverUnlock(driver); + storageDriverUnlock(); if (!*pool) { virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -1622,7 +1603,6 @@ storageVolCreateXML(virStoragePoolPtr obj, const char *xmldesc, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; virStorageVolDefPtr voldef = NULL; @@ -1703,9 +1683,9 @@ storageVolCreateXML(virStoragePoolPtr obj, buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags); - storageDriverLock(driver); + storageDriverLock(); virStoragePoolObjLock(pool); - storageDriverUnlock(driver); + storageDriverUnlock(); voldef->building = false; pool->asyncjobs--; @@ -1745,7 +1725,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virStorageVolPtr vobj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool, origpool = NULL; virStorageBackendPtr backend; virStorageVolDefPtr origvol = NULL, newvol = NULL; @@ -1755,14 +1734,14 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); - storageDriverLock(driver); + storageDriverLock(); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (pool && STRNEQ(obj->name, vobj->pool)) { virStoragePoolObjUnlock(pool); origpool = virStoragePoolObjFindByName(&driver->pools, vobj->pool); virStoragePoolObjLock(pool); } - storageDriverUnlock(driver); + storageDriverUnlock(); if (!pool) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); @@ -1876,11 +1855,11 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags); - storageDriverLock(driver); + storageDriverLock(); virStoragePoolObjLock(pool); if (origpool) virStoragePoolObjLock(origpool); - storageDriverUnlock(driver); + storageDriverUnlock(); origvol->in_use--; newvol->building = false; @@ -1991,8 +1970,8 @@ virStorageVolPoolRefreshThread(void *opaque) virStoragePoolObjPtr pool = NULL; virStorageBackendPtr backend; - storageDriverLock(driverState); - if (!(pool = virStoragePoolObjFindByName(&driverState->pools, + storageDriverLock(); + if (!(pool = virStoragePoolObjFindByName(&driver->pools, cbdata->pool_name))) goto cleanup; @@ -2006,7 +1985,7 @@ virStorageVolPoolRefreshThread(void *opaque) cleanup: if (pool) virStoragePoolObjUnlock(pool); - storageDriverUnlock(driverState); + storageDriverUnlock(); virStorageVolPoolRefreshDataFree(cbdata); } @@ -2345,7 +2324,6 @@ storageConnectListAllStoragePools(virConnectPtr conn, virStoragePoolPtr **pools, unsigned int flags) { - virStorageDriverStatePtr driver = conn->storagePrivateData; int ret = -1; virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1); @@ -2353,10 +2331,10 @@ storageConnectListAllStoragePools(virConnectPtr conn, if (virConnectListAllStoragePoolsEnsureACL(conn) < 0) goto cleanup; - storageDriverLock(driver); + storageDriverLock(); ret = virStoragePoolObjListExport(conn, driver->pools, pools, virConnectListAllStoragePoolsCheckACL, flags); - storageDriverUnlock(driver); + storageDriverUnlock(); cleanup: return ret; -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
The shared storage driver is stateful and inside the daemon so there is no need to use the storagePrivateData field to get the driver handle. Just access the global driver handle directly. --- src/storage/storage_driver.c | 194 +++++++++++++++++++------------------------ 1 file changed, 86 insertions(+), 108 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The shared network driver is stateful and inside the daemon so there is no need to use the networkPrivateData field to get the driver handle. Just access the global driver handle directly. Many places already directly accessed the global driver handle in any case, so the code could never work without relying on this. --- src/network/bridge_driver.c | 435 ++++++++++++++++++++------------------------ 1 file changed, 196 insertions(+), 239 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5f2e778..4c9b098 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -86,37 +86,34 @@ VIR_LOG_INIT("network.bridge_driver"); -static void networkDriverLock(virNetworkDriverStatePtr driver) +static virNetworkDriverStatePtr driver = NULL; + + +static void networkDriverLock(void) { virMutexLock(&driver->lock); } -static void networkDriverUnlock(virNetworkDriverStatePtr driver) +static void networkDriverUnlock(void) { virMutexUnlock(&driver->lock); } static int networkStateCleanup(void); -static int networkStartNetwork(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkStartNetwork(virNetworkObjPtr network); -static int networkShutdownNetwork(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkShutdownNetwork(virNetworkObjPtr network); -static int networkStartNetworkVirtual(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkStartNetworkVirtual(virNetworkObjPtr network); -static int networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkShutdownNetworkVirtual(virNetworkObjPtr network); -static int networkStartNetworkExternal(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkStartNetworkExternal(virNetworkObjPtr network); -static int networkShutdownNetworkExternal(virNetworkDriverStatePtr driver, - virNetworkObjPtr network); +static int networkShutdownNetworkExternal(virNetworkObjPtr network); -static void networkReloadFirewallRules(virNetworkDriverStatePtr driver); -static void networkRefreshDaemons(virNetworkDriverStatePtr driver); +static void networkReloadFirewallRules(void); +static void networkRefreshDaemons(void); static int networkPlugBandwidth(virNetworkObjPtr net, virDomainNetDefPtr iface); @@ -126,18 +123,16 @@ static int networkUnplugBandwidth(virNetworkObjPtr net, static void networkNetworkObjTaint(virNetworkObjPtr net, virNetworkTaintFlags taint); -static virNetworkDriverStatePtr driverState = NULL; static virNetworkObjPtr networkObjFromNetwork(virNetworkPtr net) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network; char uuidstr[VIR_UUID_STRING_BUFLEN]; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virUUIDFormat(net->uuid, uuidstr); @@ -211,7 +206,7 @@ networkDnsmasqLeaseFileNameDefault(const char *netname) char *leasefile; ignore_value(virAsprintf(&leasefile, "%s/%s.leases", - driverState->dnsmasqStateDir, netname)); + driver->dnsmasqStateDir, netname)); return leasefile; } @@ -224,7 +219,7 @@ networkDnsmasqLeaseFileNameCustom(const char *bridge) char *leasefile; ignore_value(virAsprintf(&leasefile, "%s/%s.status", - driverState->dnsmasqStateDir, bridge)); + driver->dnsmasqStateDir, bridge)); return leasefile; } @@ -234,7 +229,7 @@ networkDnsmasqConfigFileName(const char *netname) char *conffile; ignore_value(virAsprintf(&conffile, "%s/%s.conf", - driverState->dnsmasqStateDir, netname)); + driver->dnsmasqStateDir, netname)); return conffile; } @@ -254,14 +249,13 @@ networkRadvdConfigFileName(const char *netname) char *configfile; ignore_value(virAsprintf(&configfile, "%s/%s-radvd.conf", - driverState->radvdStateDir, netname)); + driver->radvdStateDir, netname)); return configfile; } /* do needed cleanup steps and remove the network from the list */ static int -networkRemoveInactive(virNetworkDriverStatePtr driver, - virNetworkObjPtr net) +networkRemoveInactive(virNetworkObjPtr net) { char *leasefile = NULL; char *customleasefile = NULL; @@ -276,7 +270,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, /* remove the (possibly) existing dnsmasq and radvd files */ if (!(dctx = dnsmasqContextNew(def->name, - driverState->dnsmasqStateDir))) { + driver->dnsmasqStateDir))) { goto cleanup; } @@ -296,7 +290,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, goto cleanup; if (!(statusfile - = virNetworkConfigFile(driverState->stateDir, def->name))) + = virNetworkConfigFile(driver->stateDir, def->name))) goto cleanup; /* dnsmasq */ @@ -307,7 +301,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, /* radvd */ unlink(radvdconfigfile); - virPidFileDelete(driverState->pidDir, radvdpidbase); + virPidFileDelete(driver->pidDir, radvdpidbase); /* remove status file */ unlink(statusfile); @@ -356,7 +350,7 @@ networkBridgeDummyNicName(const char *brname) * according to external conditions on the host (i.e. anything that * isn't stored directly in each network's state file). */ static void -networkUpdateAllState(virNetworkDriverStatePtr driver) +networkUpdateAllState(void) { size_t i; @@ -402,14 +396,14 @@ networkUpdateAllState(virNetworkDriverStatePtr driver) if (obj->active && obj->def->ips && (obj->def->nips > 0)) { char *radvdpidbase; - ignore_value(virPidFileReadIfAlive(driverState->pidDir, + ignore_value(virPidFileReadIfAlive(driver->pidDir, obj->def->name, &obj->dnsmasqPid, dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps))); radvdpidbase = networkRadvdPidfileBasename(obj->def->name); if (!radvdpidbase) break; - ignore_value(virPidFileReadIfAlive(driverState->pidDir, + ignore_value(virPidFileReadIfAlive(driver->pidDir, radvdpidbase, &obj->radvdPid, RADVD)); VIR_FREE(radvdpidbase); @@ -425,7 +419,7 @@ networkUpdateAllState(virNetworkDriverStatePtr driver) virNetworkObjLock(obj); if (!obj->persistent && !obj->active) { - networkRemoveInactive(driver, obj); + networkRemoveInactive(obj); continue; } @@ -436,7 +430,7 @@ networkUpdateAllState(virNetworkDriverStatePtr driver) static void -networkAutostartConfigs(virNetworkDriverStatePtr driver) +networkAutostartConfigs(void) { size_t i; @@ -444,7 +438,7 @@ networkAutostartConfigs(virNetworkDriverStatePtr driver) virNetworkObjLock(driver->networks.objs[i]); if (driver->networks.objs[i]->autostart && !virNetworkObjIsActive(driver->networks.objs[i])) { - if (networkStartNetwork(driver, driver->networks.objs[i]) < 0) { + if (networkStartNetwork(driver->networks.objs[i]) < 0) { /* failed to start but already logged */ } } @@ -455,17 +449,15 @@ networkAutostartConfigs(virNetworkDriverStatePtr driver) #if HAVE_FIREWALLD static DBusHandlerResult firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, - DBusMessage *message, void *user_data) + DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { - virNetworkDriverStatePtr _driverState = user_data; - if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in bridge_driver because of firewalld."); - networkReloadFirewallRules(_driverState); + networkReloadFirewallRules(); } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; @@ -473,7 +465,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED, #endif static int -networkMigrateStateFiles(virNetworkDriverStatePtr driver) +networkMigrateStateFiles(void) { /* Due to a change in location of network state xml beginning in * libvirt 1.2.4 (from /var/lib/libvirt/network to @@ -564,31 +556,31 @@ networkStateInitialize(bool privileged, DBusConnection *sysbus = NULL; #endif - if (VIR_ALLOC(driverState) < 0) + if (VIR_ALLOC(driver) < 0) goto error; - if (virMutexInit(&driverState->lock) < 0) { - VIR_FREE(driverState); + if (virMutexInit(&driver->lock) < 0) { + VIR_FREE(driver); goto error; } - networkDriverLock(driverState); + networkDriverLock(); /* configuration/state paths are one of * ~/.config/libvirt/... (session/unprivileged) * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). */ if (privileged) { - if (VIR_STRDUP(driverState->networkConfigDir, + if (VIR_STRDUP(driver->networkConfigDir, SYSCONFDIR "/libvirt/qemu/networks") < 0 || - VIR_STRDUP(driverState->networkAutostartDir, + VIR_STRDUP(driver->networkAutostartDir, SYSCONFDIR "/libvirt/qemu/networks/autostart") < 0 || - VIR_STRDUP(driverState->stateDir, + VIR_STRDUP(driver->stateDir, LOCALSTATEDIR "/run/libvirt/network") < 0 || - VIR_STRDUP(driverState->pidDir, + VIR_STRDUP(driver->pidDir, LOCALSTATEDIR "/run/libvirt/network") < 0 || - VIR_STRDUP(driverState->dnsmasqStateDir, + VIR_STRDUP(driver->dnsmasqStateDir, LOCALSTATEDIR "/lib/libvirt/dnsmasq") < 0 || - VIR_STRDUP(driverState->radvdStateDir, + VIR_STRDUP(driver->radvdStateDir, LOCALSTATEDIR "/lib/libvirt/radvd") < 0) goto error; @@ -596,7 +588,7 @@ networkStateInitialize(bool privileged, * privileged mode - unprivileged mode directories haven't * changed location. */ - if (networkMigrateStateFiles(driverState) < 0) + if (networkMigrateStateFiles() < 0) goto error; } else { configdir = virGetUserConfigDirectory(); @@ -604,48 +596,48 @@ networkStateInitialize(bool privileged, if (!(configdir && rundir)) goto error; - if ((virAsprintf(&driverState->networkConfigDir, + if ((virAsprintf(&driver->networkConfigDir, "%s/qemu/networks", configdir) < 0) || - (virAsprintf(&driverState->networkAutostartDir, + (virAsprintf(&driver->networkAutostartDir, "%s/qemu/networks/autostart", configdir) < 0) || - (virAsprintf(&driverState->stateDir, + (virAsprintf(&driver->stateDir, "%s/network/lib", rundir) < 0) || - (virAsprintf(&driverState->pidDir, + (virAsprintf(&driver->pidDir, "%s/network/run", rundir) < 0) || - (virAsprintf(&driverState->dnsmasqStateDir, + (virAsprintf(&driver->dnsmasqStateDir, "%s/dnsmasq/lib", rundir) < 0) || - (virAsprintf(&driverState->radvdStateDir, + (virAsprintf(&driver->radvdStateDir, "%s/radvd/lib", rundir) < 0)) { goto error; } } - if (virFileMakePath(driverState->stateDir) < 0) { + if (virFileMakePath(driver->stateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driverState->stateDir); + driver->stateDir); goto error; } /* if this fails now, it will be retried later with dnsmasqCapsRefresh() */ - driverState->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); + driver->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ); - if (virNetworkLoadAllState(&driverState->networks, - driverState->stateDir) < 0) + if (virNetworkLoadAllState(&driver->networks, + driver->stateDir) < 0) goto error; - if (virNetworkLoadAllConfigs(&driverState->networks, - driverState->networkConfigDir, - driverState->networkAutostartDir) < 0) + if (virNetworkLoadAllConfigs(&driver->networks, + driver->networkConfigDir, + driver->networkAutostartDir) < 0) goto error; - networkUpdateAllState(driverState); - networkReloadFirewallRules(driverState); - networkRefreshDaemons(driverState); + networkUpdateAllState(); + networkReloadFirewallRules(); + networkRefreshDaemons(); - driverState->networkEventState = virObjectEventStateNew(); + driver->networkEventState = virObjectEventStateNew(); - networkDriverUnlock(driverState); + networkDriverUnlock(); #ifdef HAVE_FIREWALLD if (!(sysbus = virDBusGetSystemBus())) { @@ -669,7 +661,7 @@ networkStateInitialize(bool privileged, ",member='Reloaded'", NULL); dbus_connection_add_filter(sysbus, firewalld_dbus_filter_bridge, - driverState, NULL); + NULL, NULL); } #endif @@ -680,8 +672,8 @@ networkStateInitialize(bool privileged, return ret; error: - if (driverState) - networkDriverUnlock(driverState); + if (driver) + networkDriverUnlock(); networkStateCleanup(); goto cleanup; } @@ -694,12 +686,12 @@ networkStateInitialize(bool privileged, static void networkStateAutoStart(void) { - if (!driverState) + if (!driver) return; - networkDriverLock(driverState); - networkAutostartConfigs(driverState); - networkDriverUnlock(driverState); + networkDriverLock(); + networkAutostartConfigs(); + networkDriverUnlock(); } /** @@ -711,19 +703,19 @@ networkStateAutoStart(void) static int networkStateReload(void) { - if (!driverState) + if (!driver) return 0; - networkDriverLock(driverState); - virNetworkLoadAllState(&driverState->networks, - driverState->stateDir); - virNetworkLoadAllConfigs(&driverState->networks, - driverState->networkConfigDir, - driverState->networkAutostartDir); - networkReloadFirewallRules(driverState); - networkRefreshDaemons(driverState); - networkAutostartConfigs(driverState); - networkDriverUnlock(driverState); + networkDriverLock(); + virNetworkLoadAllState(&driver->networks, + driver->stateDir); + virNetworkLoadAllConfigs(&driver->networks, + driver->networkConfigDir, + driver->networkAutostartDir); + networkReloadFirewallRules(); + networkRefreshDaemons(); + networkAutostartConfigs(); + networkDriverUnlock(); return 0; } @@ -736,29 +728,29 @@ networkStateReload(void) static int networkStateCleanup(void) { - if (!driverState) + if (!driver) return -1; - networkDriverLock(driverState); + networkDriverLock(); - virObjectEventStateFree(driverState->networkEventState); + virObjectEventStateFree(driver->networkEventState); /* free inactive networks */ - virNetworkObjListFree(&driverState->networks); + virNetworkObjListFree(&driver->networks); - VIR_FREE(driverState->networkConfigDir); - VIR_FREE(driverState->networkAutostartDir); - VIR_FREE(driverState->stateDir); - VIR_FREE(driverState->pidDir); - VIR_FREE(driverState->dnsmasqStateDir); - VIR_FREE(driverState->radvdStateDir); + VIR_FREE(driver->networkConfigDir); + VIR_FREE(driver->networkAutostartDir); + VIR_FREE(driver->stateDir); + VIR_FREE(driver->pidDir); + VIR_FREE(driver->dnsmasqStateDir); + VIR_FREE(driver->radvdStateDir); - virObjectUnref(driverState->dnsmasqCaps); + virObjectUnref(driver->dnsmasqCaps); - networkDriverUnlock(driverState); - virMutexDestroy(&driverState->lock); + networkDriverUnlock(); + virMutexDestroy(&driver->lock); - VIR_FREE(driverState); + VIR_FREE(driver); return 0; } @@ -1301,8 +1293,7 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, } static int -networkStartDhcpDaemon(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkStartDhcpDaemon(virNetworkObjPtr network) { virCommandPtr cmd = NULL; char *pidfile = NULL; @@ -1315,25 +1306,25 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver, goto cleanup; } - if (virFileMakePath(driverState->pidDir) < 0) { + if (virFileMakePath(driver->pidDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driverState->pidDir); + driver->pidDir); goto cleanup; } - if (!(pidfile = virPidFileBuildPath(driverState->pidDir, + if (!(pidfile = virPidFileBuildPath(driver->pidDir, network->def->name))) goto cleanup; - if (virFileMakePath(driverState->dnsmasqStateDir) < 0) { + if (virFileMakePath(driver->dnsmasqStateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driverState->dnsmasqStateDir); + driver->dnsmasqStateDir); goto cleanup; } - dctx = dnsmasqContextNew(network->def->name, driverState->dnsmasqStateDir); + dctx = dnsmasqContextNew(network->def->name, driver->dnsmasqStateDir); if (dctx == NULL) goto cleanup; @@ -1362,7 +1353,7 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver, * pid */ - ret = virPidFileRead(driverState->pidDir, network->def->name, + ret = virPidFileRead(driver->pidDir, network->def->name, &network->dnsmasqPid); if (ret < 0) goto cleanup; @@ -1383,8 +1374,7 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver, * Returns 0 on success, -1 on failure. */ static int -networkRefreshDhcpDaemon(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkRefreshDhcpDaemon(virNetworkObjPtr network) { int ret = -1; size_t i; @@ -1397,11 +1387,11 @@ networkRefreshDhcpDaemon(virNetworkDriverStatePtr driver, /* if there's no running dnsmasq, just start it */ if (network->dnsmasqPid <= 0 || (kill(network->dnsmasqPid, 0) < 0)) - return networkStartDhcpDaemon(driver, network); + return networkStartDhcpDaemon(network); VIR_INFO("Refreshing dnsmasq for network %s", network->def->bridge); if (!(dctx = dnsmasqContextNew(network->def->name, - driverState->dnsmasqStateDir))) { + driver->dnsmasqStateDir))) { goto cleanup; } @@ -1451,8 +1441,7 @@ networkRefreshDhcpDaemon(virNetworkDriverStatePtr driver, * Returns 0 on success, -1 on failure. */ static int -networkRestartDhcpDaemon(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkRestartDhcpDaemon(virNetworkObjPtr network) { /* if there is a running dnsmasq, kill it */ if (network->dnsmasqPid > 0) { @@ -1461,7 +1450,7 @@ networkRestartDhcpDaemon(virNetworkDriverStatePtr driver, network->dnsmasqPid = -1; } /* now start dnsmasq if it should be started */ - return networkStartDhcpDaemon(driver, network); + return networkStartDhcpDaemon(network); } static char radvd1[] = " AdvOtherConfigFlag off;\n\n"; @@ -1588,8 +1577,7 @@ networkRadvdConfWrite(virNetworkObjPtr network, char **configFile) } static int -networkStartRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, - virNetworkObjPtr network) +networkStartRadvd(virNetworkObjPtr network) { char *pidfile = NULL; char *radvdpidbase = NULL; @@ -1619,23 +1607,23 @@ networkStartRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, goto cleanup; } - if (virFileMakePath(driverState->pidDir) < 0) { + if (virFileMakePath(driver->pidDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driverState->pidDir); + driver->pidDir); goto cleanup; } - if (virFileMakePath(driverState->radvdStateDir) < 0) { + if (virFileMakePath(driver->radvdStateDir) < 0) { virReportSystemError(errno, _("cannot create directory %s"), - driverState->radvdStateDir); + driver->radvdStateDir); goto cleanup; } /* construct pidfile name */ if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) goto cleanup; - if (!(pidfile = virPidFileBuildPath(driverState->pidDir, radvdpidbase))) + if (!(pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) goto cleanup; if (networkRadvdConfWrite(network, &configfile) < 0) @@ -1661,7 +1649,7 @@ networkStartRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (virPidFileRead(driverState->pidDir, radvdpidbase, &network->radvdPid) < 0) + if (virPidFileRead(driver->pidDir, radvdpidbase, &network->radvdPid) < 0) goto cleanup; ret = 0; @@ -1674,8 +1662,7 @@ networkStartRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, } static int -networkRefreshRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, - virNetworkObjPtr network) +networkRefreshRadvd(virNetworkObjPtr network) { char *radvdpidbase; @@ -1688,7 +1675,7 @@ networkRefreshRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, network->def->name) >= 0) && ((radvdpidbase = networkRadvdPidfileBasename(network->def->name)) != NULL)) { - virPidFileDelete(driverState->pidDir, radvdpidbase); + virPidFileDelete(driver->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } network->radvdPid = -1; @@ -1697,7 +1684,7 @@ networkRefreshRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, /* if there's no running radvd, just start it */ if (network->radvdPid <= 0 || (kill(network->radvdPid, 0) < 0)) - return networkStartRadvd(driver, network); + return networkStartRadvd(network); if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0)) { /* no IPv6 addresses, so we don't need to run radvd */ @@ -1713,8 +1700,7 @@ networkRefreshRadvd(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, #if 0 /* currently unused, so it causes a build error unless we #if it out */ static int -networkRestartRadvd(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkRestartRadvd(virNetworkObjPtr network) { char *radvdpidbase; @@ -1728,7 +1714,7 @@ networkRestartRadvd(virNetworkDriverStatePtr driver, network->def->name) >= 0) && ((radvdpidbase = networkRadvdPidfileBasename(network->def->name)) != NULL)) { - virPidFileDelete(driverState->pidDir, radvdpidbase); + virPidFileDelete(driver->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } network->radvdPid = -1; @@ -1742,7 +1728,7 @@ networkRestartRadvd(virNetworkDriverStatePtr driver, * This should be called when libvirtd is restarted. */ static void -networkRefreshDaemons(virNetworkDriverStatePtr driver) +networkRefreshDaemons(void) { size_t i; @@ -1762,15 +1748,15 @@ networkRefreshDaemons(virNetworkDriverStatePtr driver) * dnsmasq and/or radvd, or restart them if they've * disappeared. */ - networkRefreshDhcpDaemon(driver, network); - networkRefreshRadvd(driver, network); + networkRefreshDhcpDaemon(network); + networkRefreshRadvd(network); } virNetworkObjUnlock(network); } } static void -networkReloadFirewallRules(virNetworkDriverStatePtr driver) +networkReloadFirewallRules(void) { size_t i; @@ -1959,8 +1945,7 @@ networkAddRouteToBridge(virNetworkObjPtr network, } static int -networkStartNetworkVirtual(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkStartNetworkVirtual(virNetworkObjPtr network) { size_t i; bool v4present = false, v6present = false; @@ -2065,11 +2050,11 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, /* start dnsmasq if there are any IP addresses (v4 or v6) */ if ((v4present || v6present) && - networkStartDhcpDaemon(driver, network) < 0) + networkStartDhcpDaemon(network) < 0) goto err3; /* start radvd if there are any ipv6 addresses */ - if (v6present && networkStartRadvd(driver, network) < 0) + if (v6present && networkStartRadvd(network) < 0) goto err4; /* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the @@ -2134,8 +2119,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, return -1; } -static int networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, - virNetworkObjPtr network) +static int networkShutdownNetworkVirtual(virNetworkObjPtr network) { virNetDevBandwidthClear(network->def->bridge); @@ -2145,7 +2129,7 @@ static int networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver ATTRIBU kill(network->radvdPid, SIGTERM); /* attempt to delete the pidfile we created */ if ((radvdpidbase = networkRadvdPidfileBasename(network->def->name))) { - virPidFileDelete(driverState->pidDir, radvdpidbase); + virPidFileDelete(driver->pidDir, radvdpidbase); VIR_FREE(radvdpidbase); } } @@ -2289,8 +2273,7 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) static int -networkStartNetworkExternal(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, - virNetworkObjPtr network) +networkStartNetworkExternal(virNetworkObjPtr network) { /* put anything here that needs to be done each time a network of * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On @@ -2300,8 +2283,7 @@ networkStartNetworkExternal(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, return networkCreateInterfacePool(network->def); } -static int networkShutdownNetworkExternal(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, - virNetworkObjPtr network ATTRIBUTE_UNUSED) +static int networkShutdownNetworkExternal(virNetworkObjPtr network ATTRIBUTE_UNUSED) { /* put anything here that needs to be done each time a network of * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On @@ -2312,8 +2294,7 @@ static int networkShutdownNetworkExternal(virNetworkDriverStatePtr driver ATTRIB } static int -networkStartNetwork(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +networkStartNetwork(virNetworkObjPtr network) { int ret = -1; @@ -2343,7 +2324,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: - if (networkStartNetworkVirtual(driver, network) < 0) + if (networkStartNetworkVirtual(network) < 0) goto cleanup; break; @@ -2352,7 +2333,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: case VIR_NETWORK_FORWARD_HOSTDEV: - if (networkStartNetworkExternal(driver, network) < 0) + if (networkStartNetworkExternal(network) < 0) goto cleanup; break; } @@ -2367,7 +2348,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, * is setup. */ VIR_DEBUG("Writing network status to disk"); - if (virNetworkSaveStatus(driverState->stateDir, network) < 0) + if (virNetworkSaveStatus(driver->stateDir, network) < 0) goto cleanup; network->active = 1; @@ -2379,7 +2360,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, virNetworkObjUnsetDefTransient(network); virErrorPtr save_err = virSaveLastError(); int save_errno = errno; - networkShutdownNetwork(driver, network); + networkShutdownNetwork(network); virSetError(save_err); virFreeError(save_err); errno = save_errno; @@ -2387,8 +2368,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, return ret; } -static int networkShutdownNetwork(virNetworkDriverStatePtr driver, - virNetworkObjPtr network) +static int networkShutdownNetwork(virNetworkObjPtr network) { int ret = 0; char *stateFile; @@ -2398,7 +2378,7 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, if (!virNetworkObjIsActive(network)) return 0; - stateFile = virNetworkConfigFile(driverState->stateDir, + stateFile = virNetworkConfigFile(driver->stateDir, network->def->name); if (!stateFile) return -1; @@ -2411,7 +2391,7 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: - ret = networkShutdownNetworkVirtual(driver, network); + ret = networkShutdownNetworkVirtual(network); break; case VIR_NETWORK_FORWARD_BRIDGE: @@ -2419,7 +2399,7 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: case VIR_NETWORK_FORWARD_HOSTDEV: - ret = networkShutdownNetworkExternal(driver, network); + ret = networkShutdownNetworkExternal(network); break; } @@ -2436,13 +2416,12 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, static virNetworkPtr networkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; virNetworkObjPtr network; virNetworkPtr ret = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, uuid); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, "%s", _("no network with matching uuid")); @@ -2463,13 +2442,12 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn, static virNetworkPtr networkLookupByName(virConnectPtr conn, const char *name) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; virNetworkObjPtr network; virNetworkPtr ret = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByName(&driver->networks, name); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), name); @@ -2487,22 +2465,20 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn, return ret; } -static virDrvOpenStatus networkOpen(virConnectPtr conn, +static virDrvOpenStatus networkOpen(virConnectPtr conn ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); - if (!driverState) + if (!driver) return VIR_DRV_OPEN_DECLINED; - conn->networkPrivateData = driverState; return VIR_DRV_OPEN_SUCCESS; } -static int networkClose(virConnectPtr conn) +static int networkClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - conn->networkPrivateData = NULL; return 0; } @@ -2510,12 +2486,11 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) { int nactive = 0; size_t i; - virNetworkDriverStatePtr driver = conn->networkPrivateData; if (virConnectNumOfNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(driver); + networkDriverLock(); for (i = 0; i < driver->networks.count; i++) { virNetworkObjPtr obj = driver->networks.objs[i]; virNetworkObjLock(obj); @@ -2524,20 +2499,19 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) nactive++; virNetworkObjUnlock(obj); } - networkDriverUnlock(driver); + networkDriverUnlock(); return nactive; } static int networkConnectListNetworks(virConnectPtr conn, char **const names, int nnames) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; int got = 0; size_t i; if (virConnectListNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(driver); + networkDriverLock(); for (i = 0; i < driver->networks.count && got < nnames; i++) { virNetworkObjPtr obj = driver->networks.objs[i]; virNetworkObjLock(obj); @@ -2551,12 +2525,12 @@ static int networkConnectListNetworks(virConnectPtr conn, char **const names, in } virNetworkObjUnlock(obj); } - networkDriverUnlock(driver); + networkDriverUnlock(); return got; cleanup: - networkDriverUnlock(driver); + networkDriverUnlock(); for (i = 0; i < got; i++) VIR_FREE(names[i]); return -1; @@ -2566,12 +2540,11 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) { int ninactive = 0; size_t i; - virNetworkDriverStatePtr driver = conn->networkPrivateData; if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(driver); + networkDriverLock(); for (i = 0; i < driver->networks.count; i++) { virNetworkObjPtr obj = driver->networks.objs[i]; virNetworkObjLock(obj); @@ -2580,20 +2553,19 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn) ninactive++; virNetworkObjUnlock(obj); } - networkDriverUnlock(driver); + networkDriverUnlock(); return ninactive; } static int networkConnectListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; int got = 0; size_t i; if (virConnectListDefinedNetworksEnsureACL(conn) < 0) return -1; - networkDriverLock(driver); + networkDriverLock(); for (i = 0; i < driver->networks.count && got < nnames; i++) { virNetworkObjPtr obj = driver->networks.objs[i]; virNetworkObjLock(obj); @@ -2607,11 +2579,11 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn, char **const na } virNetworkObjUnlock(obj); } - networkDriverUnlock(driver); + networkDriverUnlock(); return got; cleanup: - networkDriverUnlock(driver); + networkDriverUnlock(); for (i = 0; i < got; i++) VIR_FREE(names[i]); return -1; @@ -2622,7 +2594,6 @@ networkConnectListAllNetworks(virConnectPtr conn, virNetworkPtr **nets, unsigned int flags) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; int ret = -1; virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1); @@ -2630,11 +2601,11 @@ networkConnectListAllNetworks(virConnectPtr conn, if (virConnectListAllNetworksEnsureACL(conn) < 0) goto cleanup; - networkDriverLock(driver); + networkDriverLock(); ret = virNetworkObjListExport(conn, driver->networks, nets, virConnectListAllNetworksCheckACL, flags); - networkDriverUnlock(driver); + networkDriverUnlock(); cleanup: return ret; @@ -2648,7 +2619,6 @@ networkConnectNetworkEventRegisterAny(virConnectPtr conn, void *opaque, virFreeCallback freecb) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; int ret = -1; if (virConnectNetworkEventRegisterAnyEnsureACL(conn) < 0) @@ -2667,7 +2637,6 @@ static int networkConnectNetworkEventDeregisterAny(virConnectPtr conn, int callbackID) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; int ret = -1; if (virConnectNetworkEventDeregisterAnyEnsureACL(conn) < 0) @@ -2724,8 +2693,7 @@ static int networkIsPersistent(virNetworkPtr net) static int -networkValidate(virNetworkDriverStatePtr driver, - virNetworkDefPtr def, +networkValidate(virNetworkDefPtr def, bool check_active) { size_t i; @@ -2891,13 +2859,12 @@ networkValidate(virNetworkDriverStatePtr driver, static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; virNetworkDefPtr def; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - networkDriverLock(driver); + networkDriverLock(); if (!(def = virNetworkDefParseString(xml))) goto cleanup; @@ -2905,7 +2872,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) if (virNetworkCreateXMLEnsureACL(conn, def) < 0) goto cleanup; - if (networkValidate(driver, def, true) < 0) + if (networkValidate(def, true) < 0) goto cleanup; /* NB: even though this transient network hasn't yet been started, @@ -2916,7 +2883,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) goto cleanup; def = NULL; - if (networkStartNetwork(driver, network) < 0) { + if (networkStartNetwork(network) < 0) { virNetworkRemoveInactive(&driver->networks, network); network = NULL; @@ -2937,20 +2904,19 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml) virObjectEventStateQueue(driver->networkEventState, event); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) { - virNetworkDriverStatePtr driver = conn->networkPrivateData; virNetworkDefPtr def = NULL; bool freeDef = true; virNetworkObjPtr network = NULL; virNetworkPtr ret = NULL; virObjectEventPtr event = NULL; - networkDriverLock(driver); + networkDriverLock(); if (!(def = virNetworkDefParseString(xml))) goto cleanup; @@ -2958,7 +2924,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) if (virNetworkDefineXMLEnsureACL(conn, def) < 0) goto cleanup; - if (networkValidate(driver, def, false) < 0) + if (networkValidate(def, false) < 0) goto cleanup; if (!(network = virNetworkAssignDef(&driver->networks, def, false))) @@ -2995,20 +2961,19 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml) virNetworkDefFree(def); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } static int networkUndefine(virNetworkPtr net) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network; int ret = -1; bool active = false; virObjectEventPtr event = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { @@ -3037,7 +3002,7 @@ networkUndefine(virNetworkPtr net) VIR_INFO("Undefining network '%s'", network->def->name); if (!active) { - if (networkRemoveInactive(driver, network) < 0) { + if (networkRemoveInactive(network) < 0) { network = NULL; goto cleanup; } @@ -3057,7 +3022,7 @@ networkUndefine(virNetworkPtr net) virObjectEventStateQueue(driver->networkEventState, event); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } @@ -3069,7 +3034,6 @@ networkUpdate(virNetworkPtr net, const char *xml, unsigned int flags) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network = NULL; int isActive, ret = -1; size_t i; @@ -3082,7 +3046,7 @@ networkUpdate(virNetworkPtr net, VIR_NETWORK_UPDATE_AFFECT_CONFIG, -1); - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { @@ -3174,7 +3138,7 @@ networkUpdate(virNetworkPtr net, /* these sections all change things on the dnsmasq commandline, * so we need to kill and restart dnsmasq. */ - if (networkRestartDhcpDaemon(driver, network) < 0) + if (networkRestartDhcpDaemon(network) < 0) goto cleanup; } else if (section == VIR_NETWORK_SECTION_IP_DHCP_HOST) { @@ -3195,8 +3159,8 @@ networkUpdate(virNetworkPtr net, } if ((newDhcpActive != oldDhcpActive && - networkRestartDhcpDaemon(driver, network) < 0) || - networkRefreshDhcpDaemon(driver, network) < 0) { + networkRestartDhcpDaemon(network) < 0) || + networkRefreshDhcpDaemon(network) < 0) { goto cleanup; } @@ -3207,7 +3171,7 @@ networkUpdate(virNetworkPtr net, * can just update the config files and send SIGHUP to * dnsmasq. */ - if (networkRefreshDhcpDaemon(driver, network) < 0) + if (networkRefreshDhcpDaemon(network) < 0) goto cleanup; } @@ -3216,12 +3180,12 @@ networkUpdate(virNetworkPtr net, /* only a change in IP addresses will affect radvd, and all of radvd's * config is stored in the conf file which will be re-read with a SIGHUP. */ - if (networkRefreshRadvd(driver, network) < 0) + if (networkRefreshRadvd(network) < 0) goto cleanup; } /* save current network state to disk */ - if ((ret = virNetworkSaveStatus(driverState->stateDir, + if ((ret = virNetworkSaveStatus(driver->stateDir, network)) < 0) { goto cleanup; } @@ -3230,18 +3194,17 @@ networkUpdate(virNetworkPtr net, cleanup: if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } static int networkCreate(virNetworkPtr net) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network; int ret = -1; virObjectEventPtr event = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { @@ -3253,7 +3216,7 @@ static int networkCreate(virNetworkPtr net) if (virNetworkCreateEnsureACL(net->conn, network->def) < 0) goto cleanup; - ret = networkStartNetwork(driver, network); + ret = networkStartNetwork(network); event = virNetworkEventLifecycleNew(network->def->name, network->def->uuid, @@ -3265,18 +3228,17 @@ static int networkCreate(virNetworkPtr net) virObjectEventStateQueue(driver->networkEventState, event); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } static int networkDestroy(virNetworkPtr net) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network; int ret = -1; virObjectEventPtr event = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { @@ -3295,7 +3257,7 @@ static int networkDestroy(virNetworkPtr net) goto cleanup; } - if ((ret = networkShutdownNetwork(driver, network)) < 0) + if ((ret = networkShutdownNetwork(network)) < 0) goto cleanup; event = virNetworkEventLifecycleNew(network->def->name, @@ -3304,7 +3266,7 @@ static int networkDestroy(virNetworkPtr net) 0); if (!network->persistent) { - if (networkRemoveInactive(driver, network) < 0) { + if (networkRemoveInactive(network) < 0) { network = NULL; ret = -1; goto cleanup; @@ -3317,7 +3279,7 @@ static int networkDestroy(virNetworkPtr net) virObjectEventStateQueue(driver->networkEventState, event); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } @@ -3398,12 +3360,11 @@ static int networkGetAutostart(virNetworkPtr net, static int networkSetAutostart(virNetworkPtr net, int autostart) { - virNetworkDriverStatePtr driver = net->conn->networkPrivateData; virNetworkObjPtr network; char *configFile = NULL, *autostartLink = NULL; int ret = -1; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { @@ -3461,7 +3422,7 @@ static int networkSetAutostart(virNetworkPtr net, VIR_FREE(autostartLink); if (network) virNetworkObjUnlock(network); - networkDriverUnlock(driver); + networkDriverUnlock(); return ret; } @@ -3724,7 +3685,6 @@ int networkAllocateActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { - virNetworkDriverStatePtr driver = driverState; virDomainNetType actualType = iface->type; virNetworkObjPtr network = NULL; virNetworkDefPtr netdef = NULL; @@ -3742,9 +3702,9 @@ networkAllocateActualDevice(virDomainDefPtr dom, virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByName(&driver->networks, iface->data.network.name); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4128,7 +4088,6 @@ int networkNotifyActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { - virNetworkDriverStatePtr driver = driverState; virDomainNetType actualType = virDomainNetGetActualType(iface); virNetworkObjPtr network; virNetworkDefPtr netdef; @@ -4139,9 +4098,9 @@ networkNotifyActualDevice(virDomainDefPtr dom, if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByName(&driver->networks, iface->data.network.name); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4318,7 +4277,6 @@ int networkReleaseActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { - virNetworkDriverStatePtr driver = driverState; virDomainNetType actualType = virDomainNetGetActualType(iface); virNetworkObjPtr network; virNetworkDefPtr netdef; @@ -4329,9 +4287,9 @@ networkReleaseActualDevice(virDomainDefPtr dom, if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) return 0; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByName(&driver->networks, iface->data.network.name); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4480,7 +4438,6 @@ int networkGetNetworkAddress(const char *netname, char **netaddr) { int ret = -1; - virNetworkDriverStatePtr driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; virNetworkIpDefPtr ipdef; @@ -4489,9 +4446,9 @@ networkGetNetworkAddress(const char *netname, char **netaddr) char *dev_name = NULL; *netaddr = NULL; - networkDriverLock(driver); + networkDriverLock(); network = virNetworkFindByName(&driver->networks, netname); - networkDriverUnlock(driver); + networkDriverUnlock(); if (!network) { virReportError(VIR_ERR_NO_NETWORK, _("no network with matching name '%s'"), @@ -4705,7 +4662,7 @@ networkPlugBandwidth(virNetworkObjPtr net, /* update sum of 'floor'-s of attached NICs */ net->floor_sum += ifaceBand->in->floor; /* update status file */ - if (virNetworkSaveStatus(driverState->stateDir, net) < 0) { + if (virNetworkSaveStatus(driver->stateDir, net) < 0) { ignore_value(virBitmapClearBit(net->class_id, class_id)); net->floor_sum -= ifaceBand->in->floor; iface->data.network.actual->class_id = 0; @@ -4756,7 +4713,7 @@ networkUnplugBandwidth(virNetworkObjPtr net, ignore_value(virBitmapClearBit(net->class_id, iface->data.network.actual->class_id)); /* update status file */ - if (virNetworkSaveStatus(driverState->stateDir, net) < 0) { + if (virNetworkSaveStatus(driver->stateDir, net) < 0) { net->floor_sum += ifaceBand->in->floor; ignore_value(virBitmapSetBit(net->class_id, iface->data.network.actual->class_id)); -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
The shared network driver is stateful and inside the daemon so there is no need to use the networkPrivateData field to get the driver handle. Just access the global driver handle directly.
Many places already directly accessed the global driver handle in any case, so the code could never work without relying on this. --- src/network/bridge_driver.c | 435 ++++++++++++++++++++------------------------ 1 file changed, 196 insertions(+), 239 deletions(-)
ACK
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5f2e778..4c9b098 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -86,37 +86,34 @@
VIR_LOG_INIT("network.bridge_driver");
-static void networkDriverLock(virNetworkDriverStatePtr driver) +static virNetworkDriverStatePtr driver = NULL;
The NULL initializer here is not strictly necessary.
@@ -126,18 +123,16 @@ static int networkUnplugBandwidth(virNetworkObjPtr net, static void networkNetworkObjTaint(virNetworkObjPtr net, virNetworkTaintFlags taint);
-static virNetworkDriverStatePtr driverState = NULL;
Then again, you were just hoisting it earlier. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The shared netcf driver is stateful and inside the daemon so there is no need to use the networkPrivateData field to get the driver handle. Just access the global driver handle directly. --- src/interface/interface_backend_netcf.c | 71 +++++++++++---------------------- 1 file changed, 23 insertions(+), 48 deletions(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index c55a080..3d01b08 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -63,16 +63,16 @@ virNetcfDriverStateOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetcfDriverState) -static virNetcfDriverStatePtr driverState = NULL; +static virNetcfDriverStatePtr driver = NULL; static void virNetcfDriverStateDispose(void *obj) { - virNetcfDriverStatePtr driver = obj; + virNetcfDriverStatePtr _driver = obj; - if (driver->netcf) - ncf_close(driver->netcf); + if (_driver->netcf) + ncf_close(_driver->netcf); } @@ -84,15 +84,15 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, if (virNetcfDriverStateInitialize() < 0) return -1; - if (!(driverState = virObjectLockableNew(virNetcfDriverStateClass))) + if (!(driver = virObjectLockableNew(virNetcfDriverStateClass))) return -1; /* open netcf */ - if (ncf_init(&driverState->netcf, NULL) != 0) { + if (ncf_init(&driver->netcf, NULL) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to initialize netcf")); - virObjectUnref(driverState); - driverState = NULL; + virObjectUnref(driver); + driver = NULL; return -1; } return 0; @@ -102,16 +102,16 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, static int netcfStateCleanup(void) { - if (!driverState) + if (!driver) return -1; - if (virObjectUnref(driverState)) { + if (virObjectUnref(driver)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Attempt to close netcf state driver " "with open connections")); return -1; } - driverState = NULL; + driver = NULL; return 0; } @@ -121,12 +121,12 @@ netcfStateReload(void) { int ret = -1; - if (!driverState) + if (!driver) return 0; - virObjectLock(driverState); - ncf_close(driverState->netcf); - if (ncf_init(&driverState->netcf, NULL) != 0) { + virObjectLock(driver); + ncf_close(driver->netcf); + if (ncf_init(&driver->netcf, NULL) != 0) { /* this isn't a good situation, because we can't shut down the * driver as there may still be connections to it. If we set * the netcf handle to NULL, any subsequent calls to netcf @@ -135,13 +135,13 @@ netcfStateReload(void) */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to re-init netcf")); - driverState->netcf = NULL; + driver->netcf = NULL; goto cleanup; } ret = 0; cleanup: - virObjectUnlock(driverState); + virObjectUnlock(driver); return ret; } @@ -245,10 +245,10 @@ netcfInterfaceObjIsActive(struct netcf_if *iface, int ret = -1; unsigned int flags = 0; - virObjectRef(driverState); + virObjectRef(driver); if (ncf_if_status(iface, &flags) < 0) { const char *errmsg, *details; - int errcode = ncf_error(driverState->netcf, &errmsg, &details); + int errcode = ncf_error(driver->netcf, &errmsg, &details); virReportError(netcf_to_vir_err(errcode), _("failed to get status of interface %s: %s%s%s"), ncf_if_name(iface), errmsg, details ? " - " : "", @@ -260,33 +260,26 @@ netcfInterfaceObjIsActive(struct netcf_if *iface, ret = 0; cleanup: - virObjectUnref(driverState); + virObjectUnref(driver); return ret; } static virDrvOpenStatus -netcfInterfaceOpen(virConnectPtr conn, +netcfInterfaceOpen(virConnectPtr conn ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); - if (!driverState) + if (!driver) return VIR_DRV_OPEN_ERROR; - virObjectRef(driverState); - conn->interfacePrivateData = driverState; return VIR_DRV_OPEN_SUCCESS; } static int -netcfInterfaceClose(virConnectPtr conn) +netcfInterfaceClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - - if (conn->interfacePrivateData != NULL) { - virObjectUnref(conn->interfacePrivateData); - conn->interfacePrivateData = NULL; - } return 0; } @@ -294,7 +287,6 @@ static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn, int status, virInterfaceObjListFilter filter) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int count; int want = 0; int ret = -1; @@ -388,7 +380,6 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn, char **const names, int nnames, virInterfaceObjListFilter filter) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int count = 0; int want = 0; int ret = -1; @@ -487,7 +478,6 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn, static int netcfConnectNumOfInterfaces(virConnectPtr conn) { int count; - virNetcfDriverStatePtr driver = conn->interfacePrivateData; if (virConnectNumOfInterfacesEnsureACL(conn) < 0) return -1; @@ -502,7 +492,6 @@ static int netcfConnectNumOfInterfaces(virConnectPtr conn) static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int nnames) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int count; if (virConnectListInterfacesEnsureACL(conn) < 0) @@ -521,7 +510,6 @@ static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, in static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn) { int count; - virNetcfDriverStatePtr driver = conn->interfacePrivateData; if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0) return -1; @@ -536,7 +524,6 @@ static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn) static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const names, int nnames) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int count; if (virConnectListDefinedInterfacesEnsureACL(conn) < 0) @@ -558,7 +545,6 @@ netcfConnectListAllInterfaces(virConnectPtr conn, virInterfacePtr **ifaces, unsigned int flags) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int count; size_t i; struct netcf_if *iface = NULL; @@ -704,7 +690,6 @@ netcfConnectListAllInterfaces(virConnectPtr conn, static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn, const char *name) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; struct netcf_if *iface; virInterfacePtr ret = NULL; virInterfaceDefPtr def = NULL; @@ -744,7 +729,6 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn, static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn, const char *macstr) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; struct netcf_if *iface; int niface; virInterfacePtr ret = NULL; @@ -793,7 +777,6 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn, static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo, unsigned int flags) { - virNetcfDriverStatePtr driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; char *xmlstr = NULL; virInterfaceDefPtr ifacedef = NULL; @@ -855,7 +838,6 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; struct netcf_if *iface = NULL; char *xmlstr = NULL; virInterfaceDefPtr ifacedef = NULL; @@ -903,7 +885,6 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn, static int netcfInterfaceUndefine(virInterfacePtr ifinfo) { - virNetcfDriverStatePtr driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; virInterfaceDefPtr def = NULL; int ret = -1; @@ -944,7 +925,6 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo) static int netcfInterfaceCreate(virInterfacePtr ifinfo, unsigned int flags) { - virNetcfDriverStatePtr driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; virInterfaceDefPtr def = NULL; int ret = -1; @@ -997,7 +977,6 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, static int netcfInterfaceDestroy(virInterfacePtr ifinfo, unsigned int flags) { - virNetcfDriverStatePtr driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; virInterfaceDefPtr def = NULL; int ret = -1; @@ -1049,7 +1028,6 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, static int netcfInterfaceIsActive(virInterfacePtr ifinfo) { - virNetcfDriverStatePtr driver = ifinfo->conn->interfacePrivateData; struct netcf_if *iface = NULL; virInterfaceDefPtr def = NULL; int ret = -1; @@ -1084,7 +1062,6 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo) #ifdef HAVE_NETCF_TRANSACTIONS static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int ret; virCheckFlags(0, -1); /* currently flags must be 0 */ @@ -1110,7 +1087,6 @@ static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags) static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int ret; virCheckFlags(0, -1); /* currently flags must be 0 */ @@ -1136,7 +1112,6 @@ static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags) static int netcfInterfaceChangeRollback(virConnectPtr conn, unsigned int flags) { - virNetcfDriverStatePtr driver = conn->interfacePrivateData; int ret; virCheckFlags(0, -1); /* currently flags must be 0 */ -- 2.1.0
On 10/23/2014 09:14 AM, Daniel P. Berrange wrote:
The shared netcf driver is stateful and inside the daemon so there is no need to use the networkPrivateData field to get the driver handle. Just access the global driver handle directly. --- src/interface/interface_backend_netcf.c | 71 +++++++++++---------------------- 1 file changed, 23 insertions(+), 48 deletions(-)
ACK.
diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index c55a080..3d01b08 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -63,16 +63,16 @@ virNetcfDriverStateOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virNetcfDriverState)
-static virNetcfDriverStatePtr driverState = NULL; +static virNetcfDriverStatePtr driver = NULL;
static void virNetcfDriverStateDispose(void *obj) { - virNetcfDriverStatePtr driver = obj; + virNetcfDriverStatePtr _driver = obj;
Annoying that you had to use a name with leading underscore, but this usage is safe. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/23/2014 11:14 AM, Daniel P. Berrange wrote:
Currently we have a bit of a craz setup for activating drivers for virConnectPtr. We iterate over the primary hypervisor drivers until one matches and activates. This bit is fine.
Then we do the same for the network, storage, interface, etc secondary drivers. This is where the fun starts.
In the case of the stateless drivers that run outside of libvirtd, they always want the secondary drivers to match the primary driver. Unfortunately if they don't register any dummy secondary driver they'll get given the reomte driver instead. The probing of secondary drivers is really unhelpful here.
In the case of stateful drivers that run inside of libvirtd, there is (almost) always only one driver instance that they want to work with. So at best the probing of secondary drivers is a waste of time. With the increasing feature set though we're getting tight dependancies from the hypervisor driver to the secondary driver. eg QEMU depends on the specific network driver we have because it calls various internal APIs it exposes.
I have never liked the sneakiness of those backdoor private APIs into the network driver, but didn't see a reasonable way around it at the time I added them in, and they worked, and nobody NACKed them... I think they should maybe be replaced by full fledged public APIs, but they need to be defined in a much more complete and future-proof fashion. Of course calling them would then require the ability to call a public libvirt API from within another public libvirt API (actually, I guess we already do this when we call virNetworkLookupByName() and virNetworkGetBridgeName() from qemuNetworkIfaceConnect(), but I think it's slightly accidental that that works).
We also have circular dependancy problems during libvirtd startup:
https://www.redhat.com/archives/libvir-list/2014-January/msg01143.html
There the storage driver needs a virConnectPtr instance in order to talk to the secret driver. At the same time the storage driver must run before the hypervisor driver because the hypervisor driver needs to use it for autostart of VMs. This is a horrible mess.
The solution I think is to remove the concept of probing for secondary drivers entirely. Instead I suggest creating a
struct virDriver { ... }
which contains a pointer to virHypervisorDriver, virNetworkDriver, virInterfaceDriver, etc. now virConnectPtr will only reference the single virDriverPtr and when we open a hypervisor driver, we immediately get a full set of APIs for everything. This way all the hypervisor drivers will always know exactly what secondary driver they are working with.
The one thing this will obstruct is the ability to use two different secondary drivers at the same time; I don't know if this will be an issue. An example of what could lead to this - we might decide to implement a separate network driver that uses Open vSwitch as the backend rather than the Linux host bridge, but want to allow use of both on the same system. Or perhaps we might decide we want to have a separate backend for nwfilter that uses the OVS version of flow management (or whatever they call it) instead of iptables. (Well, those are actually probably not very good examples, because (for the network case) we already demonstrate how both could be supported by the same driver in our parallel support of "unmanaged" networks that can point to either a host bridge or and OVS bridge, and (for the nwfilter case) it isn't apparent how or by whom the choice of which driver to use would be made.) So maybe that isn't a problem at all :-)
We'll be able to remove all the xxxPrivateData elements from the virConnectPtr. The stateless drivers can just access the main privateData. The stateful drivers can avoid any use of privateData at all - just access their global static variable with the driverState.
Yeah, I always wondered why the *privateData was necessary, especially since (as you say) some functions are directly accessing the global anyway.
On Fri, Oct 24, 2014 at 02:40:27PM -0400, Laine Stump wrote:
On 10/23/2014 11:14 AM, Daniel P. Berrange wrote:
Currently we have a bit of a craz setup for activating drivers for virConnectPtr. We iterate over the primary hypervisor drivers until one matches and activates. This bit is fine.
Then we do the same for the network, storage, interface, etc secondary drivers. This is where the fun starts.
In the case of the stateless drivers that run outside of libvirtd, they always want the secondary drivers to match the primary driver. Unfortunately if they don't register any dummy secondary driver they'll get given the reomte driver instead. The probing of secondary drivers is really unhelpful here.
In the case of stateful drivers that run inside of libvirtd, there is (almost) always only one driver instance that they want to work with. So at best the probing of secondary drivers is a waste of time. With the increasing feature set though we're getting tight dependancies from the hypervisor driver to the secondary driver. eg QEMU depends on the specific network driver we have because it calls various internal APIs it exposes.
I have never liked the sneakiness of those backdoor private APIs into the network driver, but didn't see a reasonable way around it at the time I added them in, and they worked, and nobody NACKed them... I think they should maybe be replaced by full fledged public APIs, but they need to be defined in a much more complete and future-proof fashion. Of course calling them would then require the ability to call a public libvirt API from within another public libvirt API (actually, I guess we already do this when we call virNetworkLookupByName() and virNetworkGetBridgeName() from qemuNetworkIfaceConnect(), but I think it's slightly accidental that that works).
Yeah, at some point we should figure out a less hacky way to achieve this all, but I dont have good ideas right now either.
We also have circular dependancy problems during libvirtd startup:
https://www.redhat.com/archives/libvir-list/2014-January/msg01143.html
There the storage driver needs a virConnectPtr instance in order to talk to the secret driver. At the same time the storage driver must run before the hypervisor driver because the hypervisor driver needs to use it for autostart of VMs. This is a horrible mess.
The solution I think is to remove the concept of probing for secondary drivers entirely. Instead I suggest creating a
struct virDriver { ... }
which contains a pointer to virHypervisorDriver, virNetworkDriver, virInterfaceDriver, etc. now virConnectPtr will only reference the single virDriverPtr and when we open a hypervisor driver, we immediately get a full set of APIs for everything. This way all the hypervisor drivers will always know exactly what secondary driver they are working with.
The one thing this will obstruct is the ability to use two different secondary drivers at the same time; I don't know if this will be an issue. An example of what could lead to this - we might decide to implement a separate network driver that uses Open vSwitch as the backend rather than the Linux host bridge, but want to allow use of both on the same system. Or perhaps we might decide we want to have a separate backend for nwfilter that uses the OVS version of flow management (or whatever they call it) instead of iptables. (Well, those are actually probably not very good examples, because (for the network case) we already demonstrate how both could be supported by the same driver in our parallel support of "unmanaged" networks that can point to either a host bridge or and OVS bridge, and (for the nwfilter case) it isn't apparent how or by whom the choice of which driver to use would be made.)
So maybe that isn't a problem at all :-)
The two places we have multiple impls are - node devices - HAL vs udev. We figure out the impl at time libvirtd starts up, so not a problem - interfaces - netcf vs udev. Again we figure out ipl at startup So I don't think we need to care about multiple drivers for the same APIs, at virConnectOpen() time. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Laine Stump