[libvirt] [PATCH 00/10] Enable direct use of secondary drivers

Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers. This also allows us to fix long standing problems with most code that uses secrets internally. We need to pass a virConnectPtr into such code but some call stacks don't have a connection available. In some cases we open a temporary connection to the QEMU driver, but this is suboptimal for deployments without the QEMU driver present. Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions src/interface/interface_backend_netcf.c | 98 ++++++++- src/interface/interface_backend_udev.c | 97 ++++++++- src/network/bridge_driver.c | 185 +++++++++++++---- src/network/bridge_driver_platform.h | 3 + src/node_device/node_device_driver.c | 73 ++++++- src/node_device/node_device_driver.h | 9 + src/node_device/node_device_hal.c | 18 ++ src/node_device/node_device_udev.c | 19 ++ src/nwfilter/nwfilter_driver.c | 83 ++++++++ src/secret/secret_driver.c | 95 +++++++++ src/storage/storage_backend.h | 45 ++-- src/storage/storage_backend_disk.c | 30 +-- src/storage/storage_backend_fs.c | 15 +- src/storage/storage_backend_gluster.c | 9 +- src/storage/storage_backend_iscsi.c | 24 +-- src/storage/storage_backend_logical.c | 38 ++-- src/storage/storage_backend_mpath.c | 5 +- src/storage/storage_backend_rbd.c | 53 ++--- src/storage/storage_backend_scsi.c | 46 +++-- src/storage/storage_backend_sheepdog.c | 33 ++- src/storage/storage_backend_vstorage.c | 10 +- src/storage/storage_backend_zfs.c | 15 +- src/storage/storage_driver.c | 351 ++++++++++++++++++++------------ src/storage/storage_util.c | 146 ++++++------- src/storage/storage_util.h | 39 ++-- tests/storagevolxml2argvtest.c | 7 +- 26 files changed, 1047 insertions(+), 499 deletions(-) -- 2.14.3

By convention the last thing in the driver.c files should be the driver callback table and function to register it. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_driver.c | 172 +++++++++++++++++++++---------------------- 1 file changed, 86 insertions(+), 86 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 3b66d51719..f68acc75be 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2687,92 +2687,6 @@ storageConnectStoragePoolEventDeregisterAny(virConnectPtr conn, } - -static virStorageDriver storageDriver = { - .name = "storage", - .connectNumOfStoragePools = storageConnectNumOfStoragePools, /* 0.4.0 */ - .connectListStoragePools = storageConnectListStoragePools, /* 0.4.0 */ - .connectNumOfDefinedStoragePools = storageConnectNumOfDefinedStoragePools, /* 0.4.0 */ - .connectListDefinedStoragePools = storageConnectListDefinedStoragePools, /* 0.4.0 */ - .connectListAllStoragePools = storageConnectListAllStoragePools, /* 0.10.2 */ - .connectStoragePoolEventRegisterAny = storageConnectStoragePoolEventRegisterAny, /* 2.0.0 */ - .connectStoragePoolEventDeregisterAny = storageConnectStoragePoolEventDeregisterAny, /* 2.0.0 */ - .connectFindStoragePoolSources = storageConnectFindStoragePoolSources, /* 0.4.0 */ - .storagePoolLookupByName = storagePoolLookupByName, /* 0.4.0 */ - .storagePoolLookupByUUID = storagePoolLookupByUUID, /* 0.4.0 */ - .storagePoolLookupByVolume = storagePoolLookupByVolume, /* 0.4.0 */ - .storagePoolCreateXML = storagePoolCreateXML, /* 0.4.0 */ - .storagePoolDefineXML = storagePoolDefineXML, /* 0.4.0 */ - .storagePoolBuild = storagePoolBuild, /* 0.4.0 */ - .storagePoolUndefine = storagePoolUndefine, /* 0.4.0 */ - .storagePoolCreate = storagePoolCreate, /* 0.4.0 */ - .storagePoolDestroy = storagePoolDestroy, /* 0.4.0 */ - .storagePoolDelete = storagePoolDelete, /* 0.4.0 */ - .storagePoolRefresh = storagePoolRefresh, /* 0.4.0 */ - .storagePoolGetInfo = storagePoolGetInfo, /* 0.4.0 */ - .storagePoolGetXMLDesc = storagePoolGetXMLDesc, /* 0.4.0 */ - .storagePoolGetAutostart = storagePoolGetAutostart, /* 0.4.0 */ - .storagePoolSetAutostart = storagePoolSetAutostart, /* 0.4.0 */ - .storagePoolNumOfVolumes = storagePoolNumOfVolumes, /* 0.4.0 */ - .storagePoolListVolumes = storagePoolListVolumes, /* 0.4.0 */ - .storagePoolListAllVolumes = storagePoolListAllVolumes, /* 0.10.2 */ - - .storageVolLookupByName = storageVolLookupByName, /* 0.4.0 */ - .storageVolLookupByKey = storageVolLookupByKey, /* 0.4.0 */ - .storageVolLookupByPath = storageVolLookupByPath, /* 0.4.0 */ - .storageVolCreateXML = storageVolCreateXML, /* 0.4.0 */ - .storageVolCreateXMLFrom = storageVolCreateXMLFrom, /* 0.6.4 */ - .storageVolDownload = storageVolDownload, /* 0.9.0 */ - .storageVolUpload = storageVolUpload, /* 0.9.0 */ - .storageVolDelete = storageVolDelete, /* 0.4.0 */ - .storageVolWipe = storageVolWipe, /* 0.8.0 */ - .storageVolWipePattern = storageVolWipePattern, /* 0.9.10 */ - .storageVolGetInfo = storageVolGetInfo, /* 0.4.0 */ - .storageVolGetInfoFlags = storageVolGetInfoFlags, /* 3.0.0 */ - .storageVolGetXMLDesc = storageVolGetXMLDesc, /* 0.4.0 */ - .storageVolGetPath = storageVolGetPath, /* 0.4.0 */ - .storageVolResize = storageVolResize, /* 0.9.10 */ - - .storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ - .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ -}; - - -static virStateDriver stateDriver = { - .name = "storage", - .stateInitialize = storageStateInitialize, - .stateAutoStart = storageStateAutoStart, - .stateCleanup = storageStateCleanup, - .stateReload = storageStateReload, -}; - -static int -storageRegisterFull(bool allbackends) -{ - if (virStorageBackendDriversRegister(allbackends) < 0) - return -1; - if (virSetSharedStorageDriver(&storageDriver) < 0) - return -1; - if (virRegisterStateDriver(&stateDriver) < 0) - return -1; - return 0; -} - - -int -storageRegister(void) -{ - return storageRegisterFull(false); -} - - -int -storageRegisterAll(void) -{ - return storageRegisterFull(true); -} - - static int virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) @@ -3065,3 +2979,89 @@ virStoragePoolObjBuildTempFilePath(virStoragePoolObjPtr obj, driver->stateDir, def->name, voldef->name)); return tmp; } + + +static virStorageDriver storageDriver = { + .name = "storage", + .connectNumOfStoragePools = storageConnectNumOfStoragePools, /* 0.4.0 */ + .connectListStoragePools = storageConnectListStoragePools, /* 0.4.0 */ + .connectNumOfDefinedStoragePools = storageConnectNumOfDefinedStoragePools, /* 0.4.0 */ + .connectListDefinedStoragePools = storageConnectListDefinedStoragePools, /* 0.4.0 */ + .connectListAllStoragePools = storageConnectListAllStoragePools, /* 0.10.2 */ + .connectStoragePoolEventRegisterAny = storageConnectStoragePoolEventRegisterAny, /* 2.0.0 */ + .connectStoragePoolEventDeregisterAny = storageConnectStoragePoolEventDeregisterAny, /* 2.0.0 */ + .connectFindStoragePoolSources = storageConnectFindStoragePoolSources, /* 0.4.0 */ + .storagePoolLookupByName = storagePoolLookupByName, /* 0.4.0 */ + .storagePoolLookupByUUID = storagePoolLookupByUUID, /* 0.4.0 */ + .storagePoolLookupByVolume = storagePoolLookupByVolume, /* 0.4.0 */ + .storagePoolCreateXML = storagePoolCreateXML, /* 0.4.0 */ + .storagePoolDefineXML = storagePoolDefineXML, /* 0.4.0 */ + .storagePoolBuild = storagePoolBuild, /* 0.4.0 */ + .storagePoolUndefine = storagePoolUndefine, /* 0.4.0 */ + .storagePoolCreate = storagePoolCreate, /* 0.4.0 */ + .storagePoolDestroy = storagePoolDestroy, /* 0.4.0 */ + .storagePoolDelete = storagePoolDelete, /* 0.4.0 */ + .storagePoolRefresh = storagePoolRefresh, /* 0.4.0 */ + .storagePoolGetInfo = storagePoolGetInfo, /* 0.4.0 */ + .storagePoolGetXMLDesc = storagePoolGetXMLDesc, /* 0.4.0 */ + .storagePoolGetAutostart = storagePoolGetAutostart, /* 0.4.0 */ + .storagePoolSetAutostart = storagePoolSetAutostart, /* 0.4.0 */ + .storagePoolNumOfVolumes = storagePoolNumOfVolumes, /* 0.4.0 */ + .storagePoolListVolumes = storagePoolListVolumes, /* 0.4.0 */ + .storagePoolListAllVolumes = storagePoolListAllVolumes, /* 0.10.2 */ + + .storageVolLookupByName = storageVolLookupByName, /* 0.4.0 */ + .storageVolLookupByKey = storageVolLookupByKey, /* 0.4.0 */ + .storageVolLookupByPath = storageVolLookupByPath, /* 0.4.0 */ + .storageVolCreateXML = storageVolCreateXML, /* 0.4.0 */ + .storageVolCreateXMLFrom = storageVolCreateXMLFrom, /* 0.6.4 */ + .storageVolDownload = storageVolDownload, /* 0.9.0 */ + .storageVolUpload = storageVolUpload, /* 0.9.0 */ + .storageVolDelete = storageVolDelete, /* 0.4.0 */ + .storageVolWipe = storageVolWipe, /* 0.8.0 */ + .storageVolWipePattern = storageVolWipePattern, /* 0.9.10 */ + .storageVolGetInfo = storageVolGetInfo, /* 0.4.0 */ + .storageVolGetInfoFlags = storageVolGetInfoFlags, /* 3.0.0 */ + .storageVolGetXMLDesc = storageVolGetXMLDesc, /* 0.4.0 */ + .storageVolGetPath = storageVolGetPath, /* 0.4.0 */ + .storageVolResize = storageVolResize, /* 0.9.10 */ + + .storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ + .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ +}; + + +static virStateDriver stateDriver = { + .name = "storage", + .stateInitialize = storageStateInitialize, + .stateAutoStart = storageStateAutoStart, + .stateCleanup = storageStateCleanup, + .stateReload = storageStateReload, +}; + + +static int +storageRegisterFull(bool allbackends) +{ + if (virStorageBackendDriversRegister(allbackends) < 0) + return -1; + if (virSetSharedStorageDriver(&storageDriver) < 0) + return -1; + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; + return 0; +} + + +int +storageRegister(void) +{ + return storageRegisterFull(false); +} + + +int +storageRegisterAll(void) +{ + return storageRegisterFull(true); +} -- 2.14.3

Allow the possibility of opening a connection to only the storage driver, by defining storage:///system and storage:///session URIs and registering a fake hypervisor driver that supports them. The hypervisor drivers can now directly open a storage driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_driver.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f68acc75be..5d21007edb 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -388,6 +388,79 @@ storageStateCleanup(void) return 0; } +static virDrvOpenStatus storageConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "storage")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("storage state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage URI path '%s', try storage:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage URI path '%s', try storage:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int storageConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int storageConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int storageConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int storageConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + static virStoragePoolObjPtr storagePoolObjFindByUUID(const unsigned char *uuid, @@ -3031,6 +3104,21 @@ static virStorageDriver storageDriver = { }; +static virHypervisorDriver storageHypervisorDriver = { + .name = "storage", + .connectOpen = storageConnectOpen, /* 4.1.0 */ + .connectClose = storageConnectClose, /* 4.1.0 */ + .connectIsEncrypted = storageConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = storageConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = storageConnectIsAlive, /* 4.1.0 */ +}; + +static virConnectDriver storageConnectDriver = { + .hypervisorDriver = &storageHypervisorDriver, + .storageDriver = &storageDriver, +}; + + static virStateDriver stateDriver = { .name = "storage", .stateInitialize = storageStateInitialize, @@ -3043,6 +3131,8 @@ static virStateDriver stateDriver = { static int storageRegisterFull(bool allbackends) { + if (virRegisterConnectDriver(&storageConnectDriver, false) < 0) + return -1; if (virStorageBackendDriversRegister(allbackends) < 0) return -1; if (virSetSharedStorageDriver(&storageDriver) < 0) -- 2.14.3

On Fri, Jan 26, 2018 at 13:35:29 +0000, Daniel Berrange wrote:
Allow the possibility of opening a connection to only the storage driver, by defining storage:///system and storage:///session URIs and registering a fake hypervisor driver that supports them.
The hypervisor drivers can now directly open a storage driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_driver.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f68acc75be..5d21007edb 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -388,6 +388,79 @@ storageStateCleanup(void) return 0; }
+static virDrvOpenStatus storageConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags)
Coding style is not consistent with the rest of the file.
+{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);

By convention the last thing in the driver.c files should be the driver callback table and function to register it. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/network/bridge_driver.c | 90 ++++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 334da7a85d..7f21381bd4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4241,51 +4241,6 @@ networkGetDHCPLeases(virNetworkPtr net, } -static virNetworkDriver networkDriver = { - .name = "bridge", - .connectNumOfNetworks = networkConnectNumOfNetworks, /* 0.2.0 */ - .connectListNetworks = networkConnectListNetworks, /* 0.2.0 */ - .connectNumOfDefinedNetworks = networkConnectNumOfDefinedNetworks, /* 0.2.0 */ - .connectListDefinedNetworks = networkConnectListDefinedNetworks, /* 0.2.0 */ - .connectListAllNetworks = networkConnectListAllNetworks, /* 0.10.2 */ - .connectNetworkEventRegisterAny = networkConnectNetworkEventRegisterAny, /* 1.2.1 */ - .connectNetworkEventDeregisterAny = networkConnectNetworkEventDeregisterAny, /* 1.2.1 */ - .networkLookupByUUID = networkLookupByUUID, /* 0.2.0 */ - .networkLookupByName = networkLookupByName, /* 0.2.0 */ - .networkCreateXML = networkCreateXML, /* 0.2.0 */ - .networkDefineXML = networkDefineXML, /* 0.2.0 */ - .networkUndefine = networkUndefine, /* 0.2.0 */ - .networkUpdate = networkUpdate, /* 0.10.2 */ - .networkCreate = networkCreate, /* 0.2.0 */ - .networkDestroy = networkDestroy, /* 0.2.0 */ - .networkGetXMLDesc = networkGetXMLDesc, /* 0.2.0 */ - .networkGetBridgeName = networkGetBridgeName, /* 0.2.0 */ - .networkGetAutostart = networkGetAutostart, /* 0.2.1 */ - .networkSetAutostart = networkSetAutostart, /* 0.2.1 */ - .networkIsActive = networkIsActive, /* 0.7.3 */ - .networkIsPersistent = networkIsPersistent, /* 0.7.3 */ - .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.2.6 */ -}; - -static virStateDriver networkStateDriver = { - .name = "bridge", - .stateInitialize = networkStateInitialize, - .stateAutoStart = networkStateAutoStart, - .stateCleanup = networkStateCleanup, - .stateReload = networkStateReload, -}; - -int -networkRegister(void) -{ - if (virSetSharedNetworkDriver(&networkDriver) < 0) - return -1; - if (virRegisterStateDriver(&networkStateDriver) < 0) - return -1; - return 0; -} - - /* A unified function to log network connections and disconnections */ static void @@ -5716,3 +5671,48 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, virNetworkObjEndAPI(&obj); return ret; } + + +static virNetworkDriver networkDriver = { + .name = "bridge", + .connectNumOfNetworks = networkConnectNumOfNetworks, /* 0.2.0 */ + .connectListNetworks = networkConnectListNetworks, /* 0.2.0 */ + .connectNumOfDefinedNetworks = networkConnectNumOfDefinedNetworks, /* 0.2.0 */ + .connectListDefinedNetworks = networkConnectListDefinedNetworks, /* 0.2.0 */ + .connectListAllNetworks = networkConnectListAllNetworks, /* 0.10.2 */ + .connectNetworkEventRegisterAny = networkConnectNetworkEventRegisterAny, /* 1.2.1 */ + .connectNetworkEventDeregisterAny = networkConnectNetworkEventDeregisterAny, /* 1.2.1 */ + .networkLookupByUUID = networkLookupByUUID, /* 0.2.0 */ + .networkLookupByName = networkLookupByName, /* 0.2.0 */ + .networkCreateXML = networkCreateXML, /* 0.2.0 */ + .networkDefineXML = networkDefineXML, /* 0.2.0 */ + .networkUndefine = networkUndefine, /* 0.2.0 */ + .networkUpdate = networkUpdate, /* 0.10.2 */ + .networkCreate = networkCreate, /* 0.2.0 */ + .networkDestroy = networkDestroy, /* 0.2.0 */ + .networkGetXMLDesc = networkGetXMLDesc, /* 0.2.0 */ + .networkGetBridgeName = networkGetBridgeName, /* 0.2.0 */ + .networkGetAutostart = networkGetAutostart, /* 0.2.1 */ + .networkSetAutostart = networkSetAutostart, /* 0.2.1 */ + .networkIsActive = networkIsActive, /* 0.7.3 */ + .networkIsPersistent = networkIsPersistent, /* 0.7.3 */ + .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.2.6 */ +}; + +static virStateDriver networkStateDriver = { + .name = "bridge", + .stateInitialize = networkStateInitialize, + .stateAutoStart = networkStateAutoStart, + .stateCleanup = networkStateCleanup, + .stateReload = networkStateReload, +}; + +int +networkRegister(void) +{ + if (virSetSharedNetworkDriver(&networkDriver) < 0) + return -1; + if (virRegisterStateDriver(&networkStateDriver) < 0) + return -1; + return 0; +} -- 2.14.3

Allow the possibility of opening a connection to only the network driver, by defining network:///system and network:///session URIs and registering a fake hypervisor driver that supports them. The hypervisor drivers can now directly open a network driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/network/bridge_driver.c | 95 ++++++++++++++++++++++++++++++++++++ src/network/bridge_driver_platform.h | 3 ++ 2 files changed, 98 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7f21381bd4..7aea8079d4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -671,6 +671,8 @@ networkStateInitialize(bool privileged, goto error; } + network_driver->privileged = privileged; + /* configuration/state paths are one of * ~/.config/libvirt/... (session/unprivileged) * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). @@ -868,6 +870,80 @@ networkStateCleanup(void) } +static virDrvOpenStatus networkConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "network")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (network_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("network state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (network_driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected network URI path '%s', try network:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected network URI path '%s', try network:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int networkConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int networkConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int networkConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int networkConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + + /* networkKillDaemon: * * kill the specified pid/name, and wait a bit to make sure it's dead. @@ -5699,6 +5775,23 @@ static virNetworkDriver networkDriver = { .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.2.6 */ }; + +static virHypervisorDriver networkHypervisorDriver = { + .name = "network", + .connectOpen = networkConnectOpen, /* 4.1.0 */ + .connectClose = networkConnectClose, /* 4.1.0 */ + .connectIsEncrypted = networkConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = networkConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = networkConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver networkConnectDriver = { + .hypervisorDriver = &networkHypervisorDriver, + .networkDriver = &networkDriver, +}; + + static virStateDriver networkStateDriver = { .name = "bridge", .stateInitialize = networkStateInitialize, @@ -5710,6 +5803,8 @@ static virStateDriver networkStateDriver = { int networkRegister(void) { + if (virRegisterConnectDriver(&networkConnectDriver, false) < 0) + return -1; if (virSetSharedNetworkDriver(&networkDriver) < 0) return -1; if (virRegisterStateDriver(&networkStateDriver) < 0) diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index f04c0c48b4..706000df4e 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -34,6 +34,9 @@ struct _virNetworkDriverState { virMutex lock; + /* Read-only */ + bool privileged; + /* Immutable pointer, self-locking APIs */ virNetworkObjListPtr networks; -- 2.14.3

Allow the possibility of opening a connection to only the storage driver, by defining a nwfilter:///system URI and registering a fake hypervisor driver that supports it. The hypervisor drivers can now directly open a nwfilter driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/nwfilter/nwfilter_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 885dbcc282..5787152adc 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -363,6 +363,71 @@ nwfilterStateCleanup(void) } +static virDrvOpenStatus nwfilterConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "nwfilter")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nwfilter state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected nwfilter URI path '%s', try nwfilter:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int nwfilterConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int nwfilterConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int nwfilterConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int nwfilterConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + + static virNWFilterObjPtr nwfilterObjFromNWFilter(const unsigned char *uuid) { @@ -635,6 +700,22 @@ static virNWFilterDriver nwfilterDriver = { }; +static virHypervisorDriver nwfilterHypervisorDriver = { + .name = "nwfilter", + .connectOpen = nwfilterConnectOpen, /* 4.1.0 */ + .connectClose = nwfilterConnectClose, /* 4.1.0 */ + .connectIsEncrypted = nwfilterConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = nwfilterConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = nwfilterConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver nwfilterConnectDriver = { + .hypervisorDriver = &nwfilterHypervisorDriver, + .nwfilterDriver = &nwfilterDriver, +}; + + static virStateDriver stateDriver = { .name = "NWFilter", .stateInitialize = nwfilterStateInitialize, @@ -651,6 +732,8 @@ static virDomainConfNWFilterDriver domainNWFilterDriver = { int nwfilterRegister(void) { + if (virRegisterConnectDriver(&nwfilterConnectDriver, false) < 0) + return -1; if (virSetSharedNWFilterDriver(&nwfilterDriver) < 0) return -1; if (virRegisterStateDriver(&stateDriver) < 0) -- 2.14.3

Allow the possibility of opening a connection to only the interface driver, by defining interface:///system and interface:///session URIs and registering a fake hypervisor driver that supports them. The hypervisor drivers can now directly open a interface driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/interface/interface_backend_netcf.c | 98 ++++++++++++++++++++++++++++++++- src/interface/interface_backend_udev.c | 97 +++++++++++++++++++++++++++++++- 2 files changed, 193 insertions(+), 2 deletions(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index c7cc07122a..9b04271647 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -46,6 +46,7 @@ typedef struct { virObjectLockable parent; struct netcf *netcf; + bool privileged; } virNetcfDriverState, *virNetcfDriverStatePtr; static virClassPtr virNetcfDriverStateClass; @@ -78,7 +79,7 @@ virNetcfDriverStateDispose(void *obj) static int -netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, +netcfStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -88,6 +89,8 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, if (!(driver = virObjectLockableNew(virNetcfDriverStateClass))) return -1; + driver->privileged = privileged; + /* open netcf */ if (ncf_init(&driver->netcf, NULL) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -148,6 +151,80 @@ netcfStateReload(void) } +static virDrvOpenStatus netcfConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "interface")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("interface state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected interface URI path '%s', try interface:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected interface URI path '%s', try interface:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int netcfConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int netcfConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int netcfConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int netcfConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + + /* * Get a minimal virInterfaceDef containing enough metadata * for access control checks to be performed. Currently @@ -1134,6 +1211,23 @@ static virInterfaceDriver interfaceDriver = { #endif /* HAVE_NETCF_TRANSACTIONS */ }; + +static virHypervisorDriver interfaceHypervisorDriver = { + .name = "interface", + .connectOpen = netcfConnectOpen, /* 4.1.0 */ + .connectClose = netcfConnectClose, /* 4.1.0 */ + .connectIsEncrypted = netcfConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = netcfConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = netcfConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver interfaceConnectDriver = { + .hypervisorDriver = &interfaceHypervisorDriver, + .interfaceDriver = &interfaceDriver, +}; + + static virStateDriver interfaceStateDriver = { .name = INTERFACE_DRIVER_NAME, .stateInitialize = netcfStateInitialize, @@ -1143,6 +1237,8 @@ static virStateDriver interfaceStateDriver = { int netcfIfaceRegister(void) { + if (virRegisterConnectDriver(&interfaceConnectDriver, false) < 0) + return -1; if (virSetSharedInterfaceDriver(&interfaceDriver) < 0) return -1; if (virRegisterStateDriver(&interfaceStateDriver) < 0) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 1cd84060a8..2e8e105ffc 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -41,6 +41,7 @@ struct udev_iface_driver { struct udev *udev; + bool privileged; }; typedef enum { @@ -1158,7 +1159,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) static int -udevStateInitialize(bool privileged ATTRIBUTE_UNUSED, +udevStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -1173,6 +1174,7 @@ udevStateInitialize(bool privileged ATTRIBUTE_UNUSED, _("failed to create udev context")); goto cleanup; } + driver->privileged = privileged; ret = 0; @@ -1193,6 +1195,80 @@ udevStateCleanup(void) } +static virDrvOpenStatus udevConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "interface")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("interface state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected interface URI path '%s', try interface:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected interface URI path '%s', try interface:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int udevConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int udevConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int udevConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int udevConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + + static virInterfaceDriver udevIfaceDriver = { .name = "udev", .connectNumOfInterfaces = udevConnectNumOfInterfaces, /* 1.0.0 */ @@ -1206,6 +1282,23 @@ static virInterfaceDriver udevIfaceDriver = { .interfaceGetXMLDesc = udevInterfaceGetXMLDesc, /* 1.0.0 */ }; + +static virHypervisorDriver udevHypervisorDriver = { + .name = "interface", + .connectOpen = udevConnectOpen, /* 4.1.0 */ + .connectClose = udevConnectClose, /* 4.1.0 */ + .connectIsEncrypted = udevConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = udevConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = udevConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver udevConnectDriver = { + .hypervisorDriver = &udevHypervisorDriver, + .interfaceDriver = &udevIfaceDriver, +}; + + static virStateDriver interfaceStateDriver = { .name = "udev", .stateInitialize = udevStateInitialize, @@ -1215,6 +1308,8 @@ static virStateDriver interfaceStateDriver = { int udevIfaceRegister(void) { + if (virRegisterConnectDriver(&udevConnectDriver, false) < 0) + return -1; if (virSetSharedInterfaceDriver(&udevIfaceDriver) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to register udev interface driver")); -- 2.14.3

Allow the possibility of opening a connection to only the nodedev driver, by defining nodedev:///system and nodedev:///session URIs and registering a fake hypervisor driver that supports them. The hypervisor drivers can now directly open a nodedev driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/node_device/node_device_driver.c | 73 +++++++++++++++++++++++++++++++++++- src/node_device/node_device_driver.h | 9 +++++ src/node_device/node_device_hal.c | 18 +++++++++ src/node_device/node_device_udev.c | 19 ++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 6216a69773..efbe898249 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -47,6 +47,78 @@ virNodeDeviceDriverStatePtr driver; +virDrvOpenStatus nodeConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "nodedev")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nodedev state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected nodedev URI path '%s', try nodedev:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected nodedev URI path '%s', try nodedev:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +int nodeConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +int nodeConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +int nodeConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +int nodeConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} static int nodeDeviceUpdateCaps(virNodeDeviceDefPtr def) @@ -661,7 +733,6 @@ nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, return ret; } - int nodedevRegister(void) { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 109c717815..83a9449139 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -51,6 +51,15 @@ extern virNodeDeviceDriverStatePtr driver; int nodedevRegister(void); +virDrvOpenStatus nodeConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth, + virConfPtr conf, + unsigned int flags); +int nodeConnectClose(virConnectPtr conn); +int nodeConnectIsSecure(virConnectPtr conn); +int nodeConnectIsEncrypted(virConnectPtr conn); +int nodeConnectIsAlive(virConnectPtr conn); + int nodeNumOfDevices(virConnectPtr conn, const char *cap, diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index c19e327c96..9cd5bb3eec 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -773,6 +773,22 @@ static virNodeDeviceDriver halNodeDeviceDriver = { }; +static virHypervisorDriver halHypervisorDriver = { + .name = "nodedev", + .connectOpen = nodeConnectOpen, /* 4.1.0 */ + .connectClose = nodeConnectClose, /* 4.1.0 */ + .connectIsEncrypted = nodeConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = nodeConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = nodeConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver halConnectDriver = { + .hypervisorDriver = &halHypervisorDriver, + .nodeDeviceDriver = &halNodeDeviceDriver, +}; + + static virStateDriver halStateDriver = { .name = "HAL", .stateInitialize = nodeStateInitialize, /* 0.5.0 */ @@ -783,6 +799,8 @@ static virStateDriver halStateDriver = { int halNodeRegister(void) { + if (virRegisterConnectDriver(&halConnectDriver, false) < 0) + return -1; if (virSetSharedNodeDeviceDriver(&halNodeDeviceDriver) < 0) return -1; return virRegisterStateDriver(&halStateDriver); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index e0fca6159e..f7645b6876 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -2065,6 +2065,23 @@ static virNodeDeviceDriver udevNodeDeviceDriver = { .nodeDeviceDestroy = nodeDeviceDestroy, /* 0.7.3 */ }; + +static virHypervisorDriver udevHypervisorDriver = { + .name = "nodedev", + .connectOpen = nodeConnectOpen, /* 4.1.0 */ + .connectClose = nodeConnectClose, /* 4.1.0 */ + .connectIsEncrypted = nodeConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = nodeConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = nodeConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver udevConnectDriver = { + .hypervisorDriver = &udevHypervisorDriver, + .nodeDeviceDriver = &udevNodeDeviceDriver, +}; + + static virStateDriver udevStateDriver = { .name = "udev", .stateInitialize = nodeStateInitialize, /* 0.7.3 */ @@ -2078,6 +2095,8 @@ udevNodeRegister(void) { VIR_DEBUG("Registering udev node device backend"); + if (virRegisterConnectDriver(&udevConnectDriver, false) < 0) + return -1; if (virSetSharedNodeDeviceDriver(&udevNodeDeviceDriver) < 0) return -1; -- 2.14.3

Allow the possibility of opening a connection to only the secret driver, by defining secret:///system and secret:///session URIs and registering a fake hypervisor driver that supports them. The hypervisor drivers can now directly open a secret driver connection at time of need, instead of having to pass around a virConnectPtr through many functions. This will facilitate the later change to support separate daemons for each driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/secret/secret_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index d833a863f6..e9e67b8aa5 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -57,6 +57,7 @@ typedef struct _virSecretDriverState virSecretDriverState; typedef virSecretDriverState *virSecretDriverStatePtr; struct _virSecretDriverState { virMutex lock; + bool privileged; /* readonly */ virSecretObjListPtr secrets; char *configDir; @@ -464,6 +465,7 @@ secretStateInitialize(bool privileged, secretDriverLock(); driver->secretEventState = virObjectEventStateNew(); + driver->privileged = privileged; if (privileged) { if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0) @@ -514,6 +516,80 @@ secretStateReload(void) } +static virDrvOpenStatus secretConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + /* Verify uri was specified */ + if (conn->uri == NULL) { + /* Only hypervisor drivers are permitted to auto-open on NULL uri */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (STRNEQ_NULLABLE(conn->uri->scheme, "secret")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("secret state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + if (driver->privileged) { + if (STRNEQ(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected secret URI path '%s', try secret:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ(conn->uri->path, "/session")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected secret URI path '%s', try secret:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int secretConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int secretConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Trivially secure, since always inside the daemon */ + return 1; +} + + +static int secretConnectIsEncrypted(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* Not encrypted, but remote driver takes care of that */ + return 0; +} + + +static int secretConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + + static int secretConnectSecretEventRegisterAny(virConnectPtr conn, virSecretPtr secret, @@ -573,6 +649,23 @@ static virSecretDriver secretDriver = { .connectSecretEventDeregisterAny = secretConnectSecretEventDeregisterAny, /* 3.0.0 */ }; + +static virHypervisorDriver secretHypervisorDriver = { + .name = "secret", + .connectOpen = secretConnectOpen, /* 4.1.0 */ + .connectClose = secretConnectClose, /* 4.1.0 */ + .connectIsEncrypted = secretConnectIsEncrypted, /* 4.1.0 */ + .connectIsSecure = secretConnectIsSecure, /* 4.1.0 */ + .connectIsAlive = secretConnectIsAlive, /* 4.1.0 */ +}; + + +static virConnectDriver secretConnectDriver = { + .hypervisorDriver = &secretHypervisorDriver, + .secretDriver = &secretDriver, +}; + + static virStateDriver stateDriver = { .name = "secret", .stateInitialize = secretStateInitialize, @@ -584,6 +677,8 @@ static virStateDriver stateDriver = { int secretRegister(void) { + if (virRegisterConnectDriver(&secretConnectDriver, false) < 0) + return -1; if (virSetSharedSecretDriver(&secretDriver) < 0) return -1; if (virRegisterStateDriver(&stateDriver) < 0) -- 2.14.3

Instead of passing around a virConnectPtr object, just open a connection to the secret driver at time of use. Opening connections on demand will be beneficial when the secret driver is in a separate daemon. It also solves the problem that a number of callers just pass in a NULL connection today which prevents secret lookup working at all. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend_iscsi.c | 14 +++--- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_rbd.c | 41 +++++++-------- src/storage/storage_util.c | 95 ++++++++++++++++------------------- src/storage/storage_util.h | 6 +-- 5 files changed, 71 insertions(+), 87 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b0c5096adb..921215c9e9 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -273,13 +273,13 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, static int virStorageBackendISCSISetAuth(const char *portal, - virConnectPtr conn, virStoragePoolSourcePtr source) { unsigned char *secret_value = NULL; size_t secret_size; virStorageAuthDefPtr authdef = source->auth; int ret = -1; + virConnectPtr conn = NULL; if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; @@ -292,12 +292,9 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; } - if (!conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("iscsi 'chap' authentication not supported " - "for autostarted pools")); + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); + if (!conn) return -1; - } if (virSecretGetSecretString(conn, &authdef->seclookupdef, VIR_SECRET_USAGE_TYPE_ISCSI, @@ -322,11 +319,12 @@ virStorageBackendISCSISetAuth(const char *portal, cleanup: VIR_DISPOSE_N(secret_value, secret_size); + virObjectUnref(conn); return ret; } static int -virStorageBackendISCSIStartPool(virConnectPtr conn, +virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -362,7 +360,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, if (virISCSINodeNew(portal, def->source.devices[0].path) < 0) goto cleanup; - if (virStorageBackendISCSISetAuth(portal, conn, &def->source) < 0) + if (virStorageBackendISCSISetAuth(portal, &def->source) < 0) goto cleanup; if (virISCSIConnectionLogin(portal, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 5df30de29d..64bfc8c976 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -997,7 +997,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, return -1; if (vol->target.encryption && - virStorageBackendCreateVolUsingQemuImg(conn, pool, vol, NULL, 0) < 0) + virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0) goto error; if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 7f9597cabe..e901f370d5 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -71,7 +71,6 @@ virStorageBackendRBDRADOSConfSet(rados_t cluster, static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, - virConnectPtr conn, virStoragePoolSourcePtr source) { int ret = -1; @@ -87,6 +86,7 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, const char *mon_op_timeout = "30"; const char *osd_op_timeout = "30"; const char *rbd_default_format = "2"; + virConnectPtr conn = NULL; if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); @@ -96,12 +96,9 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, goto cleanup; } - if (!conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("'ceph' authentication not supported " - "for autostarted pools")); + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); + if (!conn) return -1; - } if (virSecretGetSecretString(conn, &authdef->seclookupdef, VIR_SECRET_USAGE_TYPE_CEPH, @@ -201,6 +198,7 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, VIR_DISPOSE_N(secret_value, secret_value_size); VIR_DISPOSE_STRING(rados_key); + virObjectUnref(conn); virBufferFreeAndReset(&mon_host); VIR_FREE(mon_buff); return ret; @@ -252,8 +250,7 @@ virStorageBackendRBDFreeState(virStorageBackendRBDStatePtr *ptr) static virStorageBackendRBDStatePtr -virStorageBackendRBDNewState(virConnectPtr conn, - virStoragePoolObjPtr pool) +virStorageBackendRBDNewState(virStoragePoolObjPtr pool) { virStorageBackendRBDStatePtr ptr; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -261,7 +258,7 @@ virStorageBackendRBDNewState(virConnectPtr conn, if (VIR_ALLOC(ptr) < 0) return NULL; - if (virStorageBackendRBDOpenRADOSConn(ptr, conn, &def->source) < 0) + if (virStorageBackendRBDOpenRADOSConn(ptr, &def->source) < 0) goto error; if (virStorageBackendRBDOpenIoCTX(ptr, pool) < 0) @@ -423,7 +420,7 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, } static int -virStorageBackendRBDRefreshPool(virConnectPtr conn, +virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { size_t max_size = 1024; @@ -436,7 +433,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, struct rados_cluster_stat_t clusterstat; struct rados_pool_stat_t poolstat; - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if ((r = rados_cluster_stat(ptr->cluster, &clusterstat)) < 0) { @@ -605,7 +602,7 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, } static int -virStorageBackendRBDDeleteVol(virConnectPtr conn, +virStorageBackendRBDDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) @@ -623,7 +620,7 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn, if (flags & VIR_STORAGE_VOL_DELETE_ZEROED) VIR_WARN("%s", "This storage backend does not support zeroed removal of volumes"); - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) { @@ -685,7 +682,7 @@ static int virStorageBackendRBDCreateImage(rados_ioctx_t io, } static int -virStorageBackendRBDBuildVol(virConnectPtr conn, +virStorageBackendRBDBuildVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) @@ -718,7 +715,7 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, goto cleanup; } - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if ((r = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, @@ -1041,7 +1038,7 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, } static int -virStorageBackendRBDBuildVolFrom(virConnectPtr conn, +virStorageBackendRBDBuildVolFrom(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr newvol, virStorageVolDefPtr origvol, @@ -1056,7 +1053,7 @@ virStorageBackendRBDBuildVolFrom(virConnectPtr conn, virCheckFlags(0, -1); - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if ((virStorageBackendRBDCloneImage(ptr->ioctx, origvol->name, @@ -1071,14 +1068,14 @@ virStorageBackendRBDBuildVolFrom(virConnectPtr conn, } static int -virStorageBackendRBDRefreshVol(virConnectPtr conn, +virStorageBackendRBDRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { virStorageBackendRBDStatePtr ptr = NULL; int ret = -1; - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if (volStorageBackendRBDRefreshVolInfo(vol, pool, ptr) < 0) @@ -1105,7 +1102,7 @@ virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if ((r = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { @@ -1204,7 +1201,7 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, } static int -virStorageBackendRBDVolWipe(virConnectPtr conn, +virStorageBackendRBDVolWipe(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, @@ -1222,7 +1219,7 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, VIR_DEBUG("Wiping RBD image %s/%s", def->source.name, vol->name); - if (!(ptr = virStorageBackendRBDNewState(conn, pool))) + if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; if ((r = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 9e1b63a436..5995921570 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -497,7 +497,7 @@ virStorageGenerateSecretUUID(virConnectPtr conn, _("unable to generate uuid")); return -1; } - tmp = conn->secretDriver->secretLookupByUUID(conn, uuid); + tmp = virSecretLookupByUUID(conn, uuid); if (tmp == NULL) return 0; @@ -511,8 +511,7 @@ virStorageGenerateSecretUUID(virConnectPtr conn, } static int -virStorageGenerateQcowEncryption(virConnectPtr conn, - virStorageVolDefPtr vol) +virStorageGenerateQcowEncryption(virStorageVolDefPtr vol) { virSecretDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -522,15 +521,11 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, char *xml; unsigned char value[VIR_STORAGE_QCOW_PASSPHRASE_SIZE]; int ret = -1; + virConnectPtr conn = NULL; - if (conn->secretDriver == NULL || - conn->secretDriver->secretLookupByUUID == NULL || - conn->secretDriver->secretDefineXML == NULL || - conn->secretDriver->secretSetValue == NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("secret storage not supported")); - goto cleanup; - } + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); + if (!conn) + return -1; enc = vol->target.encryption; if (enc->nsecrets != 0) { @@ -557,7 +552,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, if (xml == NULL) goto cleanup; - secret = conn->secretDriver->secretDefineXML(conn, xml, 0); + secret = virSecretDefineXML(conn, xml, 0); if (secret == NULL) { VIR_FREE(xml); goto cleanup; @@ -567,7 +562,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, if (virStorageGenerateQcowPassphrase(value) < 0) goto cleanup; - if (conn->secretDriver->secretSetValue(secret, value, sizeof(value), 0) < 0) + if (virSecretSetValue(secret, value, sizeof(value), 0) < 0) goto cleanup; enc_secret->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; @@ -582,11 +577,11 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, cleanup: if (secret != NULL) { - if (ret != 0 && - conn->secretDriver->secretUndefine != NULL) - conn->secretDriver->secretUndefine(secret); + if (ret != 0) + virSecretUndefine(secret); virObjectUnref(secret); } + virObjectUnref(conn); virBufferFreeAndReset(&buf); virSecretDefFree(def); VIR_FREE(enc_secret); @@ -942,7 +937,6 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc, static int storageBackendCreateQemuImgCheckEncryption(int format, const char *type, - virConnectPtr conn, virStorageVolDefPtr vol) { virStorageEncryptionPtr enc = vol->target.encryption; @@ -962,7 +956,7 @@ storageBackendCreateQemuImgCheckEncryption(int format, } if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || enc->nsecrets == 0) { - if (virStorageGenerateQcowEncryption(conn, vol) < 0) + if (virStorageGenerateQcowEncryption(vol) < 0) return -1; } } else if (format == VIR_STORAGE_FILE_RAW) { @@ -1178,8 +1172,7 @@ storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, * volume definitions and imgformat */ virCommandPtr -virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags, @@ -1264,7 +1257,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, if (info.encryption && storageBackendCreateQemuImgCheckEncryption(info.format, type, - conn, vol) < 0) + vol) < 0) return NULL; @@ -1317,8 +1310,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, static char * -storageBackendCreateQemuImgSecretPath(virConnectPtr conn, - virStoragePoolObjPtr pool, +storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStorageEncryptionPtr enc = vol->target.encryption; @@ -1326,6 +1318,7 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn, int fd = -1; uint8_t *secret = NULL; size_t secretlen = 0; + virConnectPtr conn = NULL; if (!enc) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1333,14 +1326,9 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn, return NULL; } - if (!conn || !conn->secretDriver || - !conn->secretDriver->secretLookupByUUID || - !conn->secretDriver->secretLookupByUsage || - !conn->secretDriver->secretGetValue) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unable to look up encryption secret")); + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); + if (!conn) return NULL; - } if (!(secretPath = virStoragePoolObjBuildTempFilePath(pool, vol))) goto cleanup; @@ -1374,6 +1362,7 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn, } cleanup: + virObjectUnref(conn); VIR_DISPOSE_N(secret, secretlen); VIR_FORCE_CLOSE(fd); @@ -1387,7 +1376,7 @@ storageBackendCreateQemuImgSecretPath(virConnectPtr conn, static int -storageBackendCreateQemuImg(virConnectPtr conn, +storageBackendCreateQemuImg(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, @@ -1417,11 +1406,11 @@ storageBackendCreateQemuImg(virConnectPtr conn, vol->target.encryption && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { if (!(secretPath = - storageBackendCreateQemuImgSecretPath(conn, pool, vol))) + storageBackendCreateQemuImgSecretPath(pool, vol))) goto cleanup; } - cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, pool, vol, inputvol, + cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, imgformat, secretPath); if (!cmd) @@ -1442,7 +1431,6 @@ storageBackendCreateQemuImg(virConnectPtr conn, /** * virStorageBackendCreateVolUsingQemuImg - * @conn: Connection pointer * @pool: Storage Pool Object * @vol: Volume definition * @inputvol: Volume to use for creation @@ -1458,8 +1446,7 @@ storageBackendCreateQemuImg(virConnectPtr conn, * Returns: 0 on success, -1 on failure. */ int -virStorageBackendCreateVolUsingQemuImg(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendCreateVolUsingQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -1472,7 +1459,7 @@ virStorageBackendCreateVolUsingQemuImg(virConnectPtr conn, changeFormat = true; } - ret = storageBackendCreateQemuImg(conn, pool, vol, inputvol, flags); + ret = storageBackendCreateQemuImg(NULL, pool, vol, inputvol, flags); if (changeFormat) vol->target.format = VIR_STORAGE_FILE_NONE; @@ -2290,7 +2277,6 @@ virStorageBackendVolDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED, /* storageBackendLoadDefaultSecrets: - * @conn: Connection pointer to fetch secret * @vol: volume being refreshed * * If the volume had a secret generated, we need to regenerate the @@ -2300,15 +2286,19 @@ virStorageBackendVolDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED, * -1 on failures w/ error message set */ static int -storageBackendLoadDefaultSecrets(virConnectPtr conn, - virStorageVolDefPtr vol) +storageBackendLoadDefaultSecrets(virStorageVolDefPtr vol) { virSecretPtr sec; virStorageEncryptionSecretPtr encsec = NULL; + virConnectPtr conn = NULL; if (!vol->target.encryption || vol->target.encryption->nsecrets != 0) return 0; + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); + if (!conn) + return -1; + /* The encryption secret for qcow2 and luks volumes use the path * to the volume, so look for a secret with the path. If not found, * then we cannot generate the secret after a refresh (or restart). @@ -2316,8 +2306,10 @@ storageBackendLoadDefaultSecrets(virConnectPtr conn, * a usage string that although matched with the secret usage string, * didn't contain the path to the volume. We won't error in that case, * but we also cannot find the secret. */ - if (!(sec = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_VOLUME, - vol->target.path))) + sec = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_VOLUME, + vol->target.path); + virObjectUnref(conn); + if (!sec) return 0; if (VIR_ALLOC_N(vol->target.encryption->secrets, 1) < 0 || @@ -2343,7 +2335,7 @@ storageBackendLoadDefaultSecrets(virConnectPtr conn, * Update info about a volume's capacity/allocation */ int -virStorageBackendVolRefreshLocal(virConnectPtr conn, +virStorageBackendVolRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { @@ -2356,13 +2348,12 @@ virStorageBackendVolRefreshLocal(virConnectPtr conn, return ret; /* Load any secrets if possible */ - return storageBackendLoadDefaultSecrets(conn, vol); + return storageBackendLoadDefaultSecrets(vol); } static int -storageBackendResizeQemuImg(virConnectPtr conn, - virStoragePoolObjPtr pool, +storageBackendResizeQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity) { @@ -2386,14 +2377,14 @@ storageBackendResizeQemuImg(virConnectPtr conn, else type = virStorageFileFormatTypeToString(vol->target.format); - storageBackendLoadDefaultSecrets(conn, vol); + storageBackendLoadDefaultSecrets(vol); if (storageBackendCreateQemuImgCheckEncryption(vol->target.format, - type, NULL, vol) < 0) + type, vol) < 0) goto cleanup; if (!(secretPath = - storageBackendCreateQemuImgSecretPath(conn, pool, vol))) + storageBackendCreateQemuImgSecretPath(pool, vol))) goto cleanup; if (virAsprintf(&secretAlias, "%s_luks0", vol->name) < 0) @@ -2438,7 +2429,7 @@ storageBackendResizeQemuImg(virConnectPtr conn, * Resize a volume */ int -virStorageBackendVolResizeLocal(virConnectPtr conn, +virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, @@ -2459,7 +2450,7 @@ virStorageBackendVolResizeLocal(virConnectPtr conn, return -1; } - return storageBackendResizeQemuImg(conn, pool, vol, capacity); + return storageBackendResizeQemuImg(pool, vol, capacity); } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { return storagePloopResize(vol, capacity); } else { @@ -2470,7 +2461,7 @@ virStorageBackendVolResizeLocal(virConnectPtr conn, return -1; } - return storageBackendResizeQemuImg(conn, pool, vol, capacity); + return storageBackendResizeQemuImg(pool, vol, capacity); } } diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index dc7e62517b..ffc83c60ab 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -29,8 +29,7 @@ /* File creation/cloning functions used for cloning between backends */ int -virStorageBackendCreateVolUsingQemuImg(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendCreateVolUsingQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags); @@ -166,8 +165,7 @@ char *virStorageBackendStablePath(virStoragePoolObjPtr pool, bool loop); virCommandPtr -virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags, -- 2.14.3

On 01/26/2018 02:35 PM, Daniel P. Berrangé wrote:
Instead of passing around a virConnectPtr object, just open a connection to the secret driver at time of use. Opening connections on demand will be beneficial when the secret driver is in a separate daemon. It also solves the problem that a number of callers just pass in a NULL connection today which prevents secret lookup working at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend_iscsi.c | 14 +++--- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_rbd.c | 41 +++++++-------- src/storage/storage_util.c | 95 ++++++++++++++++------------------- src/storage/storage_util.h | 6 +-- 5 files changed, 71 insertions(+), 87 deletions(-)
ACK with this squashed in: diff --git i/tests/storagevolxml2argvtest.c w/tests/storagevolxml2argvtest.c index 1cd083766..efb80b9cf 100644 --- i/tests/storagevolxml2argvtest.c +++ w/tests/storagevolxml2argvtest.c @@ -84,7 +84,7 @@ testCompareXMLToArgvFiles(bool shouldFail, testSetVolumeType(vol, def); testSetVolumeType(inputvol, inputpool); - cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol, + cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, imgformat, NULL); Michal

On Mon, Jan 29, 2018 at 07:44:18AM +0100, Michal Privoznik wrote:
On 01/26/2018 02:35 PM, Daniel P. Berrangé wrote:
Instead of passing around a virConnectPtr object, just open a connection to the secret driver at time of use. Opening connections on demand will be beneficial when the secret driver is in a separate daemon. It also solves the problem that a number of callers just pass in a NULL connection today which prevents secret lookup working at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend_iscsi.c | 14 +++--- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_rbd.c | 41 +++++++-------- src/storage/storage_util.c | 95 ++++++++++++++++------------------- src/storage/storage_util.h | 6 +-- 5 files changed, 71 insertions(+), 87 deletions(-)
ACK with this squashed in:
diff --git i/tests/storagevolxml2argvtest.c w/tests/storagevolxml2argvtest.c index 1cd083766..efb80b9cf 100644 --- i/tests/storagevolxml2argvtest.c +++ w/tests/storagevolxml2argvtest.c @@ -84,7 +84,7 @@ testCompareXMLToArgvFiles(bool shouldFail, testSetVolumeType(vol, def); testSetVolumeType(inputvol, inputpool);
- cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol, + cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, imgformat, NULL);
Opps, yes. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Fri, Jan 26, 2018 at 13:35:36 +0000, Daniel Berrange wrote:
Instead of passing around a virConnectPtr object, just open a connection to the secret driver at time of use. Opening connections on demand will be beneficial when the secret driver is in a separate daemon. It also solves the problem that a number of callers just pass in a NULL connection today which prevents secret lookup working at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend_iscsi.c | 14 +++--- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_rbd.c | 41 +++++++-------- src/storage/storage_util.c | 95 ++++++++++++++++------------------- src/storage/storage_util.h | 6 +-- 5 files changed, 71 insertions(+), 87 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b0c5096adb..921215c9e9 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -273,13 +273,13 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool,
static int virStorageBackendISCSISetAuth(const char *portal, - virConnectPtr conn, virStoragePoolSourcePtr source) { unsigned char *secret_value = NULL; size_t secret_size; virStorageAuthDefPtr authdef = source->auth; int ret = -1; + virConnectPtr conn = NULL;
if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; @@ -292,12 +292,9 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; }
- if (!conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("iscsi 'chap' authentication not supported " - "for autostarted pools")); + conn = virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session");
You should add this as a helper function. If we decide that geteuid() is not a good enough check whether a connection is privileged or anything else we'd need to fix a lot of similar ugly ternary conditions. Same for the connection to the secret driver in this patch.

Now that we can open connections to the secondary drivers on demand, there is no need to pass a virConnectPtr into all the backend functions. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend.h | 45 ++++++----------- src/storage/storage_backend_disk.c | 30 +++++------- src/storage/storage_backend_fs.c | 15 ++---- src/storage/storage_backend_gluster.c | 9 ++-- src/storage/storage_backend_iscsi.c | 12 ++--- src/storage/storage_backend_logical.c | 36 +++++--------- src/storage/storage_backend_mpath.c | 5 +- src/storage/storage_backend_rbd.c | 24 +++------ src/storage/storage_backend_scsi.c | 46 ++++++++++-------- src/storage/storage_backend_sheepdog.c | 33 +++++-------- src/storage/storage_backend_vstorage.c | 10 ++-- src/storage/storage_backend_zfs.c | 15 ++---- src/storage/storage_driver.c | 89 +++++++++++++++------------------- src/storage/storage_util.c | 59 ++++++++-------------- src/storage/storage_util.h | 33 +++++-------- tests/storagevolxml2argvtest.c | 7 +-- 16 files changed, 179 insertions(+), 289 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 193cf134d6..8dbe344149 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -25,22 +25,16 @@ # include "virstorageobj.h" # include "storage_driver.h" -typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn, - const char *srcSpec, +typedef char * (*virStorageBackendFindPoolSources)(const char *srcSpec, unsigned int flags); typedef int (*virStorageBackendCheckPool)(virStoragePoolObjPtr pool, bool *active); -typedef int (*virStorageBackendStartPool)(virConnectPtr conn, - virStoragePoolObjPtr pool); -typedef int (*virStorageBackendBuildPool)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendStartPool)(virStoragePoolObjPtr pool); +typedef int (*virStorageBackendBuildPool)(virStoragePoolObjPtr pool, unsigned int flags); -typedef int (*virStorageBackendRefreshPool)(virConnectPtr conn, - virStoragePoolObjPtr pool); -typedef int (*virStorageBackendStopPool)(virConnectPtr conn, - virStoragePoolObjPtr pool); -typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendRefreshPool)(virStoragePoolObjPtr pool); +typedef int (*virStorageBackendStopPool)(virStoragePoolObjPtr pool); +typedef int (*virStorageBackendDeletePool)(virStoragePoolObjPtr pool, unsigned int flags); /* A 'buildVol' backend must remove any volume created on error since @@ -52,46 +46,37 @@ typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, * was not aware of between checking the pool and the create attempt. It * also avoids extra round trips to just delete a file. */ -typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendBuildVol)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); -typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendCreateVol)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendRefreshVol)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendDeleteVol)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); -typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendBuildVolFrom)(virStoragePoolObjPtr pool, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); -typedef int (*virStorageBackendVolumeResize)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendVolumeResize)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags); -typedef int (*virStorageBackendVolumeDownload)(virConnectPtr conn, - virStoragePoolObjPtr obj, +typedef int (*virStorageBackendVolumeDownload)(virStoragePoolObjPtr obj, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long length, unsigned int flags); -typedef int (*virStorageBackendVolumeUpload)(virConnectPtr conn, - virStoragePoolObjPtr obj, +typedef int (*virStorageBackendVolumeUpload)(virStoragePoolObjPtr obj, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long len, unsigned int flags); -typedef int (*virStorageBackendVolumeWipe)(virConnectPtr conn, - virStoragePoolObjPtr pool, +typedef int (*virStorageBackendVolumeWipe)(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags); diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index f862a896b0..7b4549c34d 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -439,8 +439,7 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) } static int -virStorageBackendDiskRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendDiskRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -464,8 +463,7 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendDiskStartPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *format; @@ -493,8 +491,7 @@ virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, * Write a new partition table header */ static int -virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendDiskBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -751,7 +748,6 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool, /* virStorageBackendDiskDeleteVol - * @conn: Pointer to a libvirt connection * @pool: Pointer to the storage pool * @vol: Pointer to the volume definition * @flags: flags (unused for now) @@ -776,8 +772,7 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool, * Returns 0 on success, -1 on failure with error message set. */ static int -virStorageBackendDiskDeleteVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -856,7 +851,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, * here is pointless */ virStoragePoolObjClearVols(pool); - if (virStorageBackendDiskRefreshPool(conn, pool) < 0) + if (virStorageBackendDiskRefreshPool(pool) < 0) goto cleanup; rc = 0; @@ -868,8 +863,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, static int -virStorageBackendDiskCreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { int res = -1; @@ -921,7 +915,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, * since we could be calling this with vol->target.path == NULL */ virErrorPtr save_err = virSaveLastError(); - ignore_value(virStorageBackendDiskDeleteVol(conn, pool, vol, 0)); + ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); virSetError(save_err); virFreeError(save_err); goto cleanup; @@ -936,8 +930,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, } static int -virStorageBackendDiskBuildVolFrom(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendDiskBuildVolFrom(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -948,19 +941,18 @@ virStorageBackendDiskBuildVolFrom(virConnectPtr conn, if (!build_func) return -1; - return build_func(conn, pool, vol, inputvol, flags); + return build_func(pool, vol, inputvol, flags); } static int -virStorageBackendDiskVolWipe(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendDiskVolWipe(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags) { if (vol->source.partType != VIR_STORAGE_VOL_DISK_TYPE_EXTENDED) - return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, flags); + return virStorageBackendVolWipeLocal(pool, vol, algorithm, flags); /* Wiping an extended partition is not support */ virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index f54759983c..b3bae38437 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -145,8 +145,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) static char * -virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *srcSpec, +virStorageBackendFileSystemNetFindPoolSources(const char *srcSpec, unsigned int flags) { virNetfsDiscoverState state = { @@ -427,7 +426,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) /** - * @conn connection to report errors against * @pool storage pool to start * * Starts a directory or FS based storage pool. The underlying source @@ -436,8 +434,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) * Returns 0 on success, -1 on error */ static int -virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendFileSystemStart(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -450,7 +447,6 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, /** - * @conn connection to report errors against * @pool storage pool to unmount * * Stops a file storage pool. The underlying source device is unmounted @@ -462,8 +458,7 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, * Returns 0 if successfully unmounted, -1 on error */ static int -virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; @@ -609,7 +604,6 @@ virStorageBackendMakeFileSystem(virStoragePoolObjPtr pool, /** - * @conn connection to report errors against * @pool storage pool to build * @flags controls the pool formatting behaviour * @@ -630,8 +624,7 @@ virStorageBackendMakeFileSystem(virStoragePoolObjPtr pool, * Returns 0 on success, -1 on error */ static int -virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendFileSystemBuild(virStoragePoolObjPtr pool, unsigned int flags) { virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5eea84f16e..6e4f19f76d 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -342,8 +342,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, } static int -virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendGlusterRefreshPool(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -423,8 +422,7 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendGlusterVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendGlusterVolDelete(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -485,8 +483,7 @@ virStorageBackendGlusterVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, static char * -virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *srcSpec, +virStorageBackendGlusterFindPoolSources(const char *srcSpec, unsigned int flags) { virStoragePoolSourceList list = { .type = VIR_STORAGE_POOL_GLUSTER, diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 921215c9e9..11addec9a8 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -158,8 +158,7 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, static char * -virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *srcSpec, +virStorageBackendISCSIFindPoolSources(const char *srcSpec, unsigned int flags) { virStoragePoolSourcePtr source = NULL; @@ -324,8 +323,7 @@ virStorageBackendISCSISetAuth(const char *portal, } static int -virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendISCSIStartPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *portal = NULL; @@ -377,8 +375,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendISCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendISCSIRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *session = NULL; @@ -402,8 +399,7 @@ virStorageBackendISCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendISCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendISCSIStopPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *portal; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 64bfc8c976..6a7d59bd57 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -588,8 +588,7 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) static char * -virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *srcSpec ATTRIBUTE_UNUSED, +virStorageBackendLogicalFindPoolSources(const char *srcSpec ATTRIBUTE_UNUSED, unsigned int flags) { virStoragePoolSourceList sourceList; @@ -728,8 +727,7 @@ virStorageBackendLogicalCheckPool(virStoragePoolObjPtr pool, } static int -virStorageBackendLogicalStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendLogicalStartPool(virStoragePoolObjPtr pool) { /* Let's make sure that the pool's name matches the pvs output and * that the pool's source devices match the pvs output. @@ -743,8 +741,7 @@ virStorageBackendLogicalStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -799,8 +796,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) { /* * # vgs --separator : --noheadings --units b --unbuffered --nosuffix --options "vg_size,vg_free" VGNAME @@ -862,8 +858,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, * "Can't deactivate volume group "VolGroup00" with 3 open logical volume(s)" */ static int -virStorageBackendLogicalStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendLogicalStopPool(virStoragePoolObjPtr pool) { if (virStorageBackendLogicalSetActive(pool, 0) < 0) return -1; @@ -872,8 +867,7 @@ virStorageBackendLogicalStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendLogicalDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -903,8 +897,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendLogicalDeleteVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { @@ -977,8 +970,7 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, static int -virStorageBackendLogicalCreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { int fd = -1; @@ -1042,15 +1034,14 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, error: err = virSaveLastError(); VIR_FORCE_CLOSE(fd); - virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); + virStorageBackendLogicalDeleteVol(pool, vol, 0); virSetError(err); virFreeError(err); return -1; } static int -virStorageBackendLogicalBuildVolFrom(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendLogicalBuildVolFrom(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -1061,18 +1052,17 @@ virStorageBackendLogicalBuildVolFrom(virConnectPtr conn, if (!build_func) return -1; - return build_func(conn, pool, vol, inputvol, flags); + return build_func(pool, vol, inputvol, flags); } static int -virStorageBackendLogicalVolWipe(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendLogicalVolWipe(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags) { if (!vol->target.sparse) - return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, flags); + return virStorageBackendVolWipeLocal(pool, vol, algorithm, flags); /* The wiping algorithms will write something to the logical volume. * Writing to a sparse logical volume causes it to be filled resulting diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 4bc39c24eb..5dcc40f601 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -256,13 +256,12 @@ virStorageBackendMpathCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, static int -virStorageBackendMpathRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendMpathRefreshPool(virStoragePoolObjPtr pool) { int retval = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - VIR_DEBUG("conn=%p, pool=%p", conn, pool); + VIR_DEBUG("pool=%p", pool); def->allocation = def->capacity = def->available = 0; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index e901f370d5..d646b86c73 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -420,8 +420,7 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, } static int -virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) { size_t max_size = 1024; int ret = -1; @@ -602,8 +601,7 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, } static int -virStorageBackendRBDDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendRBDDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -647,8 +645,7 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendRBDCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -682,8 +679,7 @@ static int virStorageBackendRBDCreateImage(rados_ioctx_t io, } static int -virStorageBackendRBDBuildVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendRBDBuildVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -1038,8 +1034,7 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, } static int -virStorageBackendRBDBuildVolFrom(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendRBDBuildVolFrom(virStoragePoolObjPtr pool, virStorageVolDefPtr newvol, virStorageVolDefPtr origvol, unsigned int flags) @@ -1068,8 +1063,7 @@ virStorageBackendRBDBuildVolFrom(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendRBDRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendRBDRefreshVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { virStorageBackendRBDStatePtr ptr = NULL; @@ -1089,8 +1083,7 @@ virStorageBackendRBDRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendRBDResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendRBDResizeVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags) @@ -1201,8 +1194,7 @@ virStorageBackendRBDVolWipeDiscard(rbd_image_t image, } static int -virStorageBackendRBDVolWipe(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 9347d66384..115df6c847 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -245,20 +245,20 @@ checkName(const char *name) * sysfs tree to get the parent 'scsi_host#' to ensure it matches. */ static bool -checkParent(virConnectPtr conn, - const char *name, +checkParent(const char *name, const char *parent_name) { unsigned int host_num; char *scsi_host_name = NULL; char *vhba_parent = NULL; bool retval = false; + virConnectPtr conn = NULL; - VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name); + VIR_DEBUG("name=%s, parent_name=%s", name, parent_name); - /* autostarted pool - assume we're OK */ + conn = virConnectOpen(geteuid() == 0 ? "nodedev:///system" : "nodedev:///session"); if (!conn) - return true; + return -1; if (virSCSIHostGetNumber(parent_name, &host_num) < 0) { virReportError(VIR_ERR_XML_ERROR, @@ -291,6 +291,7 @@ checkParent(virConnectPtr conn, retval = true; cleanup: + virObjectUnref(conn); VIR_FREE(vhba_parent); VIR_FREE(scsi_host_name); return retval; @@ -298,8 +299,7 @@ checkParent(virConnectPtr conn, static int -createVport(virConnectPtr conn, - virStoragePoolDefPtr def, +createVport(virStoragePoolDefPtr def, const char *configFile, virStorageAdapterFCHostPtr fchost) { @@ -308,8 +308,8 @@ createVport(virConnectPtr conn, virThread thread; int ret = -1; - VIR_DEBUG("conn=%p, configFile='%s' parent='%s', wwnn='%s' wwpn='%s'", - conn, NULLSTR(configFile), NULLSTR(fchost->parent), + VIR_DEBUG("configFile='%s' parent='%s', wwnn='%s' wwpn='%s'", + NULLSTR(configFile), NULLSTR(fchost->parent), fchost->wwnn, fchost->wwpn); /* If we find an existing HBA/vHBA within the fc_host sysfs @@ -322,7 +322,7 @@ createVport(virConnectPtr conn, /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent. If not this will cause failure. */ - if (!fchost->parent || checkParent(conn, name, fchost->parent)) + if (!fchost->parent || checkParent(name, fchost->parent)) ret = 0; goto cleanup; @@ -411,8 +411,7 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, } static int -virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *name = NULL; @@ -443,14 +442,13 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSCSIStartPool(virConnectPtr conn, - virStoragePoolObjPtr pool) +virStorageBackendSCSIStartPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *configFile = virStoragePoolObjGetConfigFile(pool); if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) - return createVport(conn, def, configFile, + return createVport(def, configFile, &def->source.adapter.data.fchost); return 0; @@ -458,14 +456,22 @@ virStorageBackendSCSIStartPool(virConnectPtr conn, static int -virStorageBackendSCSIStopPool(virConnectPtr conn, - virStoragePoolObjPtr pool) +virStorageBackendSCSIStopPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) - return virNodeDeviceDeleteVport(conn, - &def->source.adapter.data.fchost); + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + virConnectPtr conn; + int ret; + conn = virConnectOpen(geteuid() == 0 ? "nodedev:///system" : "nodedev:///session"); + if (!conn) + return -1; + + ret = virNodeDeviceDeleteVport(conn, + &def->source.adapter.data.fchost); + virObjectUnref(conn); + return ret; + } return 0; } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 3d9c341a11..34c118fb32 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -38,8 +38,7 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE -static int virStorageBackendSheepdogRefreshVol(virConnectPtr conn, - virStoragePoolObjPtr pool, +static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol); void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, @@ -112,8 +111,7 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, } static int -virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, const char *diskInfo) +virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskInfo) { virStorageVolDefPtr vol = NULL; @@ -128,7 +126,7 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, vol->type = VIR_STORAGE_VOL_NETWORK; - if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + if (virStorageBackendSheepdogRefreshVol(pool, vol) < 0) goto error; if (virStoragePoolObjAddVol(pool, vol) < 0) @@ -142,8 +140,7 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) { int ret = -1; char *output = NULL; @@ -170,7 +167,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, if (cells != NULL && virStringListLength((const char * const *)cells) > 2) { - if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0) + if (virStorageBackendSheepdogAddVolume(pool, cells[1]) < 0) goto cleanup; } @@ -190,8 +187,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) { int ret = -1; char *output = NULL; @@ -207,7 +203,7 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, output) < 0) goto cleanup; - ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); + ret = virStorageBackendSheepdogRefreshAllVol(pool); cleanup: virCommandFree(cmd); VIR_FREE(output); @@ -216,8 +212,7 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendSheepdogDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -234,8 +229,7 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendSheepdogCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -263,8 +257,7 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSheepdogBuildVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendSheepdogBuildVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -354,8 +347,7 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol, } static int -virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { int ret; @@ -389,8 +381,7 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendSheepdogResizeVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags) diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index 2dc26af387..076617aeb6 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -17,7 +17,6 @@ VIR_LOG_INIT("storage.storage_backend_vstorage"); /** - * @conn connection to report errors against * @pool storage pool to build * @flags controls the pool formatting behaviour * @@ -26,8 +25,7 @@ VIR_LOG_INIT("storage.storage_backend_vstorage"); * Returns 0 on success, -1 on error */ static int -virStorageBackendVzPoolBuild(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendVzPoolBuild(virStoragePoolObjPtr pool, unsigned int flags) { virCheckFlags(0, -1); @@ -37,8 +35,7 @@ virStorageBackendVzPoolBuild(virConnectPtr conn ATTRIBUTE_UNUSED, static int -virStorageBackendVzPoolStart(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -125,8 +122,7 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) static int -virStorageBackendVzPoolStop(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 198c788aca..ec39d9403b 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -228,8 +228,7 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, } static int -virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) +virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; @@ -305,8 +304,7 @@ virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -377,8 +375,7 @@ virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendZFSDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendZFSDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { @@ -403,8 +400,7 @@ virStorageBackendZFSDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendZFSBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -437,8 +433,7 @@ virStorageBackendZFSBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -virStorageBackendZFSDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendZFSDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 5d21007edb..ca5ed3d970 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -136,9 +136,9 @@ storagePoolUpdateStateCallback(virStoragePoolObjPtr obj, */ if (active) { virStoragePoolObjClearVols(obj); - if (backend->refreshPool(NULL, obj) < 0) { + if (backend->refreshPool(obj) < 0) { if (backend->stopPool) - backend->stopPool(NULL, obj); + backend->stopPool(obj); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restart storage pool '%s': %s"), def->name, virGetLastErrorMessage()); @@ -171,10 +171,9 @@ storagePoolUpdateAllState(void) static void storageDriverAutostartCallback(virStoragePoolObjPtr obj, - const void *opaque) + const void *opaque ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); - virConnectPtr conn = (virConnectPtr) opaque; virStorageBackendPtr backend; bool started = false; @@ -184,7 +183,7 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj, if (virStoragePoolObjIsAutostart(obj) && !virStoragePoolObjIsActive(obj)) { if (backend->startPool && - backend->startPool(conn, obj) < 0) { + backend->startPool(obj) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart storage pool '%s': %s"), def->name, virGetLastErrorMessage()); @@ -200,11 +199,11 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj, stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); if (!stateFile || virStoragePoolSaveState(stateFile, def) < 0 || - backend->refreshPool(conn, obj) < 0) { + backend->refreshPool(obj) < 0) { if (stateFile) unlink(stateFile); if (backend->stopPool) - backend->stopPool(conn, obj); + backend->stopPool(obj); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart storage pool '%s': %s"), def->name, virGetLastErrorMessage()); @@ -221,16 +220,9 @@ storageDriverAutostart(void) { virConnectPtr conn = NULL; - /* XXX Remove hardcoding of QEMU URI */ - if (driver->privileged) - conn = virConnectOpen("qemu:///system"); - else - conn = virConnectOpen("qemu:///session"); - /* Ignoring NULL conn - let backends decide */ - virStoragePoolObjListForEach(driver->pools, storageDriverAutostartCallback, - conn); + NULL); virObjectUnref(conn); } @@ -651,7 +643,7 @@ storageConnectFindStoragePoolSources(virConnectPtr conn, goto cleanup; } - ret = backend->findPoolSources(conn, srcSpec, flags); + ret = backend->findPoolSources(srcSpec, flags); cleanup: return ret; @@ -751,7 +743,7 @@ storagePoolCreateXML(virConnectPtr conn, if (build_flags || (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) { - if (backend->buildPool(conn, obj, build_flags) < 0) { + if (backend->buildPool(obj, build_flags) < 0) { virStoragePoolObjRemove(driver->pools, obj); virObjectUnref(obj); obj = NULL; @@ -761,7 +753,7 @@ storagePoolCreateXML(virConnectPtr conn, } if (backend->startPool && - backend->startPool(conn, obj) < 0) { + backend->startPool(obj) < 0) { virStoragePoolObjRemove(driver->pools, obj); virObjectUnref(obj); obj = NULL; @@ -772,11 +764,11 @@ storagePoolCreateXML(virConnectPtr conn, virStoragePoolObjClearVols(obj); if (!stateFile || virStoragePoolSaveState(stateFile, def) < 0 || - backend->refreshPool(conn, obj) < 0) { + backend->refreshPool(obj) < 0) { if (stateFile) unlink(stateFile); if (backend->stopPool) - backend->stopPool(conn, obj); + backend->stopPool(obj); virStoragePoolObjRemove(driver->pools, obj); virObjectUnref(obj); obj = NULL; @@ -963,25 +955,25 @@ storagePoolCreate(virStoragePoolPtr pool, if (build_flags || (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) { - if (backend->buildPool(pool->conn, obj, build_flags) < 0) + if (backend->buildPool(obj, build_flags) < 0) goto cleanup; } } VIR_INFO("Starting up storage pool '%s'", def->name); if (backend->startPool && - backend->startPool(pool->conn, obj) < 0) + backend->startPool(obj) < 0) goto cleanup; stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); virStoragePoolObjClearVols(obj); if (!stateFile || virStoragePoolSaveState(stateFile, def) < 0 || - backend->refreshPool(pool->conn, obj) < 0) { + backend->refreshPool(obj) < 0) { if (stateFile) unlink(stateFile); if (backend->stopPool) - backend->stopPool(pool->conn, obj); + backend->stopPool(obj); goto cleanup; } @@ -1029,7 +1021,7 @@ storagePoolBuild(virStoragePoolPtr pool, } if (backend->buildPool && - backend->buildPool(pool->conn, obj, flags) < 0) + backend->buildPool(obj, flags) < 0) goto cleanup; event = virStoragePoolEventLifecycleNew(def->name, @@ -1089,7 +1081,7 @@ storagePoolDestroy(virStoragePoolPtr pool) VIR_FREE(stateFile); if (backend->stopPool && - backend->stopPool(pool->conn, obj) < 0) + backend->stopPool(obj) < 0) goto cleanup; virStoragePoolObjClearVols(obj); @@ -1160,7 +1152,7 @@ storagePoolDelete(virStoragePoolPtr pool, "%s", _("pool does not support pool deletion")); goto cleanup; } - if (backend->deletePool(pool->conn, obj, flags) < 0) + if (backend->deletePool(obj, flags) < 0) goto cleanup; event = virStoragePoolEventLifecycleNew(def->name, @@ -1214,9 +1206,9 @@ storagePoolRefresh(virStoragePoolPtr pool, } virStoragePoolObjClearVols(obj); - if (backend->refreshPool(pool->conn, obj) < 0) { + if (backend->refreshPool(obj) < 0) { if (backend->stopPool) - backend->stopPool(pool->conn, obj); + backend->stopPool(obj); event = virStoragePoolEventLifecycleNew(def->name, def->uuid, @@ -1723,8 +1715,7 @@ storagePoolLookupByTargetPath(virConnectPtr conn, static int -storageVolDeleteInternal(virStorageVolPtr vol, - virStorageBackendPtr backend, +storageVolDeleteInternal(virStorageBackendPtr backend, virStoragePoolObjPtr obj, virStorageVolDefPtr voldef, unsigned int flags, @@ -1740,7 +1731,7 @@ storageVolDeleteInternal(virStorageVolPtr vol, goto cleanup; } - if (backend->deleteVol(vol->conn, obj, voldef, flags) < 0) + if (backend->deleteVol(obj, voldef, flags) < 0) goto cleanup; /* The disk backend updated the pool data including removing the @@ -1838,7 +1829,7 @@ storageVolDelete(virStorageVolPtr vol, goto cleanup; } - if (storageVolDeleteInternal(vol, backend, obj, voldef, flags, true) < 0) + if (storageVolDeleteInternal(backend, obj, voldef, flags, true) < 0) goto cleanup; ret = 0; @@ -1906,7 +1897,7 @@ storageVolCreateXML(virStoragePoolPtr pool, /* Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); - if (backend->createVol(pool->conn, obj, voldef) < 0) + if (backend->createVol(obj, voldef) < 0) goto cleanup; if (!(newvol = virGetStorageVol(pool->conn, def->name, voldef->name, @@ -1937,7 +1928,7 @@ storageVolCreateXML(virStoragePoolPtr pool, voldef->building = true; virObjectUnlock(obj); - buildret = backend->buildVol(pool->conn, obj, buildvoldef, flags); + buildret = backend->buildVol(obj, buildvoldef, flags); VIR_FREE(buildvoldef); @@ -1956,8 +1947,8 @@ storageVolCreateXML(virStoragePoolPtr pool, } if (backend->refreshVol && - backend->refreshVol(pool->conn, obj, voldef) < 0) { - storageVolDeleteInternal(newvol, backend, obj, voldef, + backend->refreshVol(obj, voldef) < 0) { + storageVolDeleteInternal(backend, obj, voldef, 0, false); voldef = NULL; goto cleanup; @@ -2096,14 +2087,14 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, } if (backend->refreshVol && - backend->refreshVol(pool->conn, obj, voldefsrc) < 0) + backend->refreshVol(obj, voldefsrc) < 0) goto cleanup; /* 'Define' the new volume so we get async progress reporting. * Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); - if (backend->createVol(pool->conn, obj, voldef) < 0) + if (backend->createVol(obj, voldef) < 0) goto cleanup; /* Make a shallow copy of the 'defined' volume definition, since the @@ -2134,7 +2125,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, virObjectUnlock(objsrc); } - buildret = backend->buildVolFrom(pool->conn, obj, shadowvol, voldefsrc, flags); + buildret = backend->buildVolFrom(obj, shadowvol, voldefsrc, flags); virObjectLock(obj); if (objsrc) @@ -2151,8 +2142,8 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, if (buildret < 0 || (backend->refreshVol && - backend->refreshVol(pool->conn, obj, voldef) < 0)) { - storageVolDeleteInternal(newvol, backend, obj, voldef, 0, false); + backend->refreshVol(obj, voldef) < 0)) { + storageVolDeleteInternal(backend, obj, voldef, 0, false); voldef = NULL; goto cleanup; } @@ -2215,7 +2206,7 @@ storageVolDownload(virStorageVolPtr vol, goto cleanup; } - ret = backend->downloadVol(vol->conn, obj, voldef, stream, + ret = backend->downloadVol(obj, voldef, stream, offset, length, flags); cleanup: @@ -2317,7 +2308,7 @@ virStorageVolPoolRefreshThread(void *opaque) goto cleanup; virStoragePoolObjClearVols(obj); - if (backend->refreshPool(NULL, obj) < 0) + if (backend->refreshPool(obj) < 0) VIR_DEBUG("Failed to refresh storage pool"); event = virStoragePoolEventRefreshNew(def->name, def->uuid); @@ -2410,7 +2401,7 @@ storageVolUpload(virStorageVolPtr vol, VIR_STRDUP(cbdata->vol_path, voldef->target.path) < 0) goto cleanup; - if ((ret = backend->uploadVol(vol->conn, obj, voldef, stream, + if ((ret = backend->uploadVol(obj, voldef, stream, offset, length, flags)) < 0) goto cleanup; @@ -2508,7 +2499,7 @@ storageVolResize(virStorageVolPtr vol, goto cleanup; } - if (backend->resizeVol(vol->conn, obj, voldef, abs_capacity, flags) < 0) + if (backend->resizeVol(obj, voldef, abs_capacity, flags) < 0) goto cleanup; voldef->target.capacity = abs_capacity; @@ -2578,7 +2569,7 @@ storageVolWipePattern(virStorageVolPtr vol, goto cleanup; } - if (backend->wipeVol(vol->conn, obj, voldef, algorithm, flags) < 0) + if (backend->wipeVol(obj, voldef, algorithm, flags) < 0) goto cleanup; /* Instead of using the refreshVol, since much changes on the target @@ -2625,7 +2616,7 @@ storageVolGetInfoFlags(virStorageVolPtr vol, goto cleanup; if (backend->refreshVol && - backend->refreshVol(vol->conn, obj, voldef) < 0) + backend->refreshVol(obj, voldef) < 0) goto cleanup; memset(info, 0, sizeof(*info)); @@ -2671,7 +2662,7 @@ storageVolGetXMLDesc(virStorageVolPtr vol, goto cleanup; if (backend->refreshVol && - backend->refreshVol(vol->conn, obj, voldef) < 0) + backend->refreshVol(obj, voldef) < 0) goto cleanup; ret = virStorageVolDefFormat(def, voldef); diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 5995921570..7fb4a7733e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -222,8 +222,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, } static int -storageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -390,8 +389,7 @@ createRawFile(int fd, virStorageVolDefPtr vol, } static int -storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +storageBackendCreateRaw(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -690,8 +688,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, /* Create ploop directory with ploop image and DiskDescriptor.xml * if function fails to create image file the directory will be deleted.*/ static int -storageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -1376,8 +1373,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, static int -storageBackendCreateQemuImg(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +storageBackendCreateQemuImg(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -1459,7 +1455,7 @@ virStorageBackendCreateVolUsingQemuImg(virStoragePoolObjPtr pool, changeFormat = true; } - ret = storageBackendCreateQemuImg(NULL, pool, vol, inputvol, flags); + ret = storageBackendCreateQemuImg(pool, vol, inputvol, flags); if (changeFormat) vol->target.format = VIR_STORAGE_FILE_NONE; @@ -2078,8 +2074,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, /* Common/Local File System/Directory Volume API's */ static int -createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +createFileDir(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -2125,8 +2120,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, * function), and can drop the parent pool lock during the (slow) allocation. */ int -virStorageBackendVolCreateLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendVolCreateLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -2164,8 +2158,7 @@ virStorageBackendVolCreateLocal(virConnectPtr conn ATTRIBUTE_UNUSED, static int -storageBackendVolBuildLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +storageBackendVolBuildLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) @@ -2194,7 +2187,7 @@ storageBackendVolBuildLocal(virConnectPtr conn, create_func = storageBackendCreateQemuImg; } - if (create_func(conn, pool, vol, inputvol, flags) < 0) + if (create_func(pool, vol, inputvol, flags) < 0) return -1; return 0; } @@ -2206,12 +2199,11 @@ storageBackendVolBuildLocal(virConnectPtr conn, * special kinds of files */ int -virStorageBackendVolBuildLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendVolBuildLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - return storageBackendVolBuildLocal(conn, pool, vol, NULL, flags); + return storageBackendVolBuildLocal(pool, vol, NULL, flags); } @@ -2219,13 +2211,12 @@ virStorageBackendVolBuildLocal(virConnectPtr conn, * Create a storage vol using 'inputvol' as input */ int -virStorageBackendVolBuildFromLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +virStorageBackendVolBuildFromLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags) { - return storageBackendVolBuildLocal(conn, pool, vol, inputvol, flags); + return storageBackendVolBuildLocal(pool, vol, inputvol, flags); } @@ -2233,8 +2224,7 @@ virStorageBackendVolBuildFromLocal(virConnectPtr conn, * Remove a volume - no support for BLOCK and NETWORK yet */ int -virStorageBackendVolDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolDeleteLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { @@ -2335,8 +2325,7 @@ storageBackendLoadDefaultSecrets(virStorageVolDefPtr vol) * Update info about a volume's capacity/allocation */ int -virStorageBackendVolRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolRefreshLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { int ret; @@ -2429,8 +2418,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, * Resize a volume */ int -virStorageBackendVolResizeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags) @@ -2509,8 +2497,7 @@ storageBackendPloopHasSnapshots(char *path) int -virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, @@ -2556,8 +2543,7 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, } int -virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolDownloadLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, @@ -2854,8 +2840,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, int -virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags) @@ -2954,8 +2939,7 @@ virStorageBackendBuildLocal(virStoragePoolObjPtr pool) * Returns 0 on success, -1 on error */ int -virStorageBackendDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, +virStorageBackendDeleteLocal(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); @@ -3696,8 +3680,7 @@ virStorageBackendRefreshVolTargetUpdate(virStorageVolDefPtr vol) * within it. This is non-recursive. */ int -virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool) +virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); DIR *dir; diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index ffc83c60ab..e9cb982115 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -38,54 +38,45 @@ virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); -int virStorageBackendVolCreateLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolCreateLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -int virStorageBackendVolBuildLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolBuildLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); -int virStorageBackendVolBuildFromLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolBuildFromLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags); -int virStorageBackendVolDeleteLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolDeleteLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); -int virStorageBackendVolRefreshLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolRefreshLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -int virStorageBackendVolResizeLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags); -int virStorageBackendVolUploadLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long len, unsigned int flags); -int virStorageBackendVolDownloadLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolDownloadLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long len, unsigned int flags); -int virStorageBackendVolWipeLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags); @@ -93,15 +84,13 @@ int virStorageBackendVolWipeLocal(virConnectPtr conn, /* Local/Common Storage Pool Backend APIs */ int virStorageBackendBuildLocal(virStoragePoolObjPtr pool); -int virStorageBackendDeleteLocal(virConnectPtr conn, - virStoragePoolObjPtr pool, +int virStorageBackendDeleteLocal(virStoragePoolObjPtr pool, unsigned int flags); int virStorageBackendRefreshVolTargetUpdate(virStorageVolDefPtr vol); -int virStorageBackendRefreshLocal(virConnectPtr conn, - virStoragePoolObjPtr pool); +int virStorageBackendRefreshLocal(virStoragePoolObjPtr pool); int virStorageUtilGlusterExtractPoolSources(const char *host, const char *xml, diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 1cd083766a..5857d5e350 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -47,16 +47,12 @@ testCompareXMLToArgvFiles(bool shouldFail, int ret = -1; virCommandPtr cmd = NULL; - virConnectPtr conn; virStorageVolDefPtr vol = NULL, inputvol = NULL; virStoragePoolDefPtr def = NULL; virStoragePoolDefPtr inputpool = NULL; virStoragePoolObjPtr obj = NULL; - if (!(conn = virGetConnect())) - goto cleanup; - if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -84,7 +80,7 @@ testCompareXMLToArgvFiles(bool shouldFail, testSetVolumeType(vol, def); testSetVolumeType(inputvol, inputpool); - cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol, + cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, imgformat, NULL); @@ -111,7 +107,6 @@ testCompareXMLToArgvFiles(bool shouldFail, virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); - virObjectUnref(conn); return ret; } -- 2.14.3

On Fri, Jan 26, 2018 at 13:35:37 +0000, Daniel Berrange wrote:
Now that we can open connections to the secondary drivers on demand, there is no need to pass a virConnectPtr into all the backend functions.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/storage/storage_backend.h | 45 ++++++----------- src/storage/storage_backend_disk.c | 30 +++++------- src/storage/storage_backend_fs.c | 15 ++---- src/storage/storage_backend_gluster.c | 9 ++-- src/storage/storage_backend_iscsi.c | 12 ++--- src/storage/storage_backend_logical.c | 36 +++++--------- src/storage/storage_backend_mpath.c | 5 +- src/storage/storage_backend_rbd.c | 24 +++------ src/storage/storage_backend_scsi.c | 46 ++++++++++-------- src/storage/storage_backend_sheepdog.c | 33 +++++-------- src/storage/storage_backend_vstorage.c | 10 ++-- src/storage/storage_backend_zfs.c | 15 ++---- src/storage/storage_driver.c | 89 +++++++++++++++------------------- src/storage/storage_util.c | 59 ++++++++-------------- src/storage/storage_util.h | 33 +++++-------- tests/storagevolxml2argvtest.c | 7 +-- 16 files changed, 179 insertions(+), 289 deletions(-)
[...]
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 9347d66384..115df6c847 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -245,20 +245,20 @@ checkName(const char *name) * sysfs tree to get the parent 'scsi_host#' to ensure it matches. */ static bool -checkParent(virConnectPtr conn, - const char *name, +checkParent(const char *name, const char *parent_name) { unsigned int host_num; char *scsi_host_name = NULL; char *vhba_parent = NULL; bool retval = false; + virConnectPtr conn = NULL;
- VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name); + VIR_DEBUG("name=%s, parent_name=%s", name, parent_name);
- /* autostarted pool - assume we're OK */ + conn = virConnectOpen(geteuid() == 0 ? "nodedev:///system" : "nodedev:///session");
Apparently we also need a helper for 'nodedev'.

On 01/26/2018 02:35 PM, Daniel P. Berrangé wrote:
Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers.
This also allows us to fix long standing problems with most code that uses secrets internally. We need to pass a virConnectPtr into such code but some call stacks don't have a connection available. In some cases we open a temporary connection to the QEMU driver, but this is suboptimal for deployments without the QEMU driver present.
Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions
src/interface/interface_backend_netcf.c | 98 ++++++++- src/interface/interface_backend_udev.c | 97 ++++++++- src/network/bridge_driver.c | 185 +++++++++++++---- src/network/bridge_driver_platform.h | 3 + src/node_device/node_device_driver.c | 73 ++++++- src/node_device/node_device_driver.h | 9 + src/node_device/node_device_hal.c | 18 ++ src/node_device/node_device_udev.c | 19 ++ src/nwfilter/nwfilter_driver.c | 83 ++++++++ src/secret/secret_driver.c | 95 +++++++++ src/storage/storage_backend.h | 45 ++-- src/storage/storage_backend_disk.c | 30 +-- src/storage/storage_backend_fs.c | 15 +- src/storage/storage_backend_gluster.c | 9 +- src/storage/storage_backend_iscsi.c | 24 +-- src/storage/storage_backend_logical.c | 38 ++-- src/storage/storage_backend_mpath.c | 5 +- src/storage/storage_backend_rbd.c | 53 ++--- src/storage/storage_backend_scsi.c | 46 +++-- src/storage/storage_backend_sheepdog.c | 33 ++- src/storage/storage_backend_vstorage.c | 10 +- src/storage/storage_backend_zfs.c | 15 +- src/storage/storage_driver.c | 351 ++++++++++++++++++++------------ src/storage/storage_util.c | 146 ++++++------- src/storage/storage_util.h | 39 ++-- tests/storagevolxml2argvtest.c | 7 +- 26 files changed, 1047 insertions(+), 499 deletions(-)
ACK series, but there's a small problem in 09/10. Michal

On Fri, Jan 26, 2018 at 13:35:27 +0000, Daniel Berrange wrote:
Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers.
I'm so glad to see this, it felt awkward to hand off the connection pointer through massive call chains. The only thing I'm afraid of in the future is that once the daemons are split, if the user has a valid connection pointer, the code may still fail if it fails to open a secondary connection to e.g. the storage driver. All this while the original caller already had a valid pointer.
This also allows us to fix long standing problems with most code that uses secrets internally. We need to pass a virConnectPtr into such code but some call stacks don't have a connection available. In some cases we open a temporary connection to the QEMU driver, but this is suboptimal for deployments without the QEMU driver present.
That always grossed me out.
Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs
All of the patches above copy-paste code which has wrong coding style, so all need to be fixed.
storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions
And the opening of the helper connection really needs a helper function. ACK with the cosmetic tweaks

On Mon, Jan 29, 2018 at 11:58:11AM +0100, Peter Krempa wrote:
On Fri, Jan 26, 2018 at 13:35:27 +0000, Daniel Berrange wrote:
Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers.
I'm so glad to see this, it felt awkward to hand off the connection pointer through massive call chains.
The only thing I'm afraid of in the future is that once the daemons are split, if the user has a valid connection pointer, the code may still fail if it fails to open a secondary connection to e.g. the storage driver. All this while the original caller already had a valid pointer.
Yep, that would be a new failure scenario, but I don't think it is too serious in the greater scheme of all possible things that can go wrong. Most likely problem would be that someone/something failed to start the extra daemons. On most distros this won't be a problem, because systemd socket activation will start then on-demand. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Jan 29, 2018 at 11:58:11AM +0100, Peter Krempa wrote:
Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs
All of the patches above copy-paste code which has wrong coding style, so all need to be fixed.
Yes will fix.
storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions
And the opening of the helper connection really needs a helper function.
I'm putting in this patch just before those two: commit 409e665f3b1cb3049c588116713ffd37dd287e29 Author: Daniel P. Berrangé <berrange@redhat.com> Date: Wed Jan 31 18:21:52 2018 +0000 driver: add some helpers for opening secondary driver connections Various parts of libvirt will want to open connections to secondary drivers. The right URI to use will depend on the context, so rather than duplicating that logic in various places, use some helper APIs. This will also make it easier for us to later pre-open/cache connections to avoid repeated opening & closing the same connectiong during autostart. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> diff --git a/src/driver.c b/src/driver.c index 04dd0a4431..a6a7ff925a 100644 --- a/src/driver.c +++ b/src/driver.c @@ -167,3 +167,33 @@ virDriverLoadModule(const char *name, /* XXX unload modules, but we can't until we can unregister libvirt drivers */ + +virConnectPtr virGetConnectInterface(void) +{ + return virConnectOpen(geteuid() == 0 ? "interface:///system" : "interface:///session"); +} + +virConnectPtr virGetConnectNetwork(void) +{ + return virConnectOpen(geteuid() == 0 ? "network:///system" : "network:///session"); +} + +virConnectPtr virGetConnectNWFilter(void) +{ + return virConnectOpen(geteuid() == 0 ? "nwfilter:///system" : "nwfilter:///session"); +} + +virConnectPtr virGetConnectNodeDev(void) +{ + return virConnectOpen(geteuid() == 0 ? "nodedev:///system" : "nodedev:///session"); +} + +virConnectPtr virGetConnectSecret(void) +{ + return virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session"); +} + +virConnectPtr virGetConnectStorage(void) +{ + return virConnectOpen(geteuid() == 0 ? "storage:///system" : "storage:///session"); +} diff --git a/src/driver.h b/src/driver.h index 936c981603..fe0cec0923 100644 --- a/src/driver.h +++ b/src/driver.h @@ -105,4 +105,11 @@ int virDriverLoadModuleFull(const char *name, const char *regfunc, void **handle); +virConnectPtr virGetConnectInterface(void); +virConnectPtr virGetConnectNetwork(void); +virConnectPtr virGetConnectNWFilter(void); +virConnectPtr virGetConnectNodeDev(void); +virConnectPtr virGetConnectSecret(void); +virConnectPtr virGetConnectStorage(void); + #endif /* __VIR_DRIVER_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 43deca9a52..c6e59e889a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1183,6 +1183,15 @@ virStorageVolClass; virStreamClass; +# driver.h +virGetConnectInterface; +virGetConnectNetwork; +virGetConnectNWFilter; +virGetConnectNodeDev; +virGetConnectSecret; +virGetConnectStorage; + + # libvirt_internal.h virConnectSupportsFeature; virDomainMigrateBegin3; Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Fri, Jan 26, 2018 at 01:35:27PM +0000, Daniel P. Berrangé wrote:
Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers.
This also allows us to fix long standing problems with most code that uses secrets internally. We need to pass a virConnectPtr into such code but some call stacks don't have a connection available. In some cases we open a temporary connection to the QEMU driver, but this is suboptimal for deployments without the QEMU driver present.
Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions
Doc update would be nice as well, or are you planning on adding that as part of separate series? Erik

On Mon, Jan 29, 2018 at 12:46:11PM +0100, Erik Skultety wrote:
On Fri, Jan 26, 2018 at 01:35:27PM +0000, Daniel P. Berrangé wrote:
Currently the secondary drivers can only be used if you have a connection to a primary hypervisor driver. This series introduces explicit URIs that allow opening a connection that only talks to a specific secondary driver. In the future these URIs will resolve to individual daemons containing those drivers.
This also allows us to fix long standing problems with most code that uses secrets internally. We need to pass a virConnectPtr into such code but some call stacks don't have a connection available. In some cases we open a temporary connection to the QEMU driver, but this is suboptimal for deployments without the QEMU driver present.
Daniel P. Berrangé (10): storage: move driver registration back to end of the file storage: allow opening with storage:///system and storage:///session URIs network: move driver registration back to end of the file network: allow opening with network:///system and network:///session URIs nwfilter: allow opening with nwfilter:///system URI interface: allow opening with interface:///system and interface:///session URIs nodedev: allow opening with nodedev:///system and nodedev:///session URIs secret: allow opening with secret:///system and secret:///session URIs storage: open secret driver connection at time of use storage: remove virConnectPtr from all backend functions
Doc update would be nice as well, or are you planning on adding that as part of separate series?
Good point, i'll look at that. Since this series is acked, I might as well post docs separately now. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Daniel P. Berrangé
-
Erik Skultety
-
Michal Privoznik
-
Peter Krempa