[libvirt] [PATCH 8/8] Removing probing of secondary drivers

Michal Privoznik mprivozn at redhat.com
Wed Jan 21 10:08:54 UTC 2015


On 20.01.2015 17:37, Daniel P. Berrange wrote:
> For stateless, client side drivers, it is never correct to
> probe for secondary drivers. It is only ever appropriate to
> use the secondary driver that is associated with the
> hypervisor in question. As a result the ESX & HyperV drivers
> have both been forced to do hacks where they register no-op
> drivers for the ones they don't implement.
> 
> For stateful, server side drivers, we always just want to
> use the same built-in shared driver. The exception is
> virtualbox which is really a stateless driver and so wants
> to use its own server side secondary drivers. To deal with
> this virtualbox has to be built as 3 separate loadable
> modules to allow registration to work in the right order.
> 
> This can all be simplified by introducing a new struct
> recording the precise set of secondary drivers each
> hypervisor driver wants
> 
> struct _virConnectDriver {
>     virHypervisorDriverPtr hypervisorDriver;
>     virInterfaceDriverPtr interfaceDriver;
>     virNetworkDriverPtr networkDriver;
>     virNodeDeviceDriverPtr nodeDeviceDriver;
>     virNWFilterDriverPtr nwfilterDriver;
>     virSecretDriverPtr secretDriver;
>     virStorageDriverPtr storageDriver;
> };
> 
> Instead of registering the hypervisor driver, we now
> just register a virConnectDriver instead. This allows
> us to remove all probing of secondary drivers. Once we
> have chosen the primary driver, we immediately know the
> correct secondary drivers to use.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  daemon/libvirtd.c                       |  19 +-
>  src/Makefile.am                         |  63 +------
>  src/bhyve/bhyve_driver.c                |  14 +-
>  src/check-driverimpls.pl                |   3 +-
>  src/datatypes.c                         |  12 --
>  src/driver-hypervisor.h                 |   3 -
>  src/driver-interface.h                  |   9 -
>  src/driver-network.h                    |  11 +-
>  src/driver-nodedev.h                    |   8 +-
>  src/driver-nwfilter.h                   |  13 +-
>  src/driver-secret.h                     |  12 +-
>  src/driver-storage.h                    |  12 +-
>  src/driver.h                            |  54 +++---
>  src/esx/esx_device_monitor.c            |  74 --------
>  src/esx/esx_device_monitor.h            |  28 ---
>  src/esx/esx_driver.c                    |  25 +--
>  src/esx/esx_interface_driver.c          |  37 +---
>  src/esx/esx_interface_driver.h          |   4 +-
>  src/esx/esx_network_driver.c            |  37 +---
>  src/esx/esx_network_driver.h            |   4 +-
>  src/esx/esx_nwfilter_driver.c           |  74 --------
>  src/esx/esx_nwfilter_driver.h           |  28 ---
>  src/esx/esx_secret_driver.c             |  72 -------
>  src/esx/esx_secret_driver.h             |  27 ---
>  src/esx/esx_storage_driver.c            |  37 +---
>  src/esx/esx_storage_driver.h            |   4 +-
>  src/hyperv/hyperv_device_monitor.c      |  71 -------
>  src/hyperv/hyperv_device_monitor.h      |  28 ---
>  src/hyperv/hyperv_driver.c              |  25 +--
>  src/hyperv/hyperv_interface_driver.c    |  71 -------
>  src/hyperv/hyperv_interface_driver.h    |  28 ---
>  src/hyperv/hyperv_network_driver.c      |  71 -------
>  src/hyperv/hyperv_network_driver.h      |  28 ---
>  src/hyperv/hyperv_nwfilter_driver.c     |  71 -------
>  src/hyperv/hyperv_nwfilter_driver.h     |  28 ---
>  src/hyperv/hyperv_secret_driver.c       |  71 -------
>  src/hyperv/hyperv_secret_driver.h       |  28 ---
>  src/hyperv/hyperv_storage_driver.c      |  71 -------
>  src/hyperv/hyperv_storage_driver.h      |  28 ---
>  src/interface/interface_backend_netcf.c |  26 +--
>  src/interface/interface_backend_udev.c  |  25 +--
>  src/libvirt.c                           | 323 ++++++++++++++------------------
>  src/libvirt_private.syms                |  14 +-
>  src/libxl/libxl_driver.c                |  10 +-
>  src/lxc/lxc_driver.c                    |  10 +-
>  src/network/bridge_driver.c             |  25 +--
>  src/node_device/node_device_driver.c    |  10 +-
>  src/node_device/node_device_hal.c       |  26 +--
>  src/node_device/node_device_udev.c      |  23 +--
>  src/nwfilter/nwfilter_driver.c          |  25 +--
>  src/openvz/openvz_driver.c              |  12 +-
>  src/phyp/phyp_driver.c                  |  64 +------
>  src/qemu/qemu_driver.c                  |  10 +-
>  src/remote/remote_driver.c              | 152 ++-------------
>  src/secret/secret_driver.c              |  25 +--
>  src/storage/storage_driver.c            |  25 +--
>  src/test/test_driver.c                  | 156 ++-------------
>  src/uml/uml_driver.c                    |  10 +-
>  src/vbox/vbox_common.c                  |   1 -
>  src/vbox/vbox_driver.c                  |  47 ++---
>  src/vbox/vbox_network.c                 |  32 ----
>  src/vbox/vbox_storage.c                 |  30 ---
>  src/vmware/vmware_driver.c              |  12 +-
>  src/xen/xen_driver.c                    |  11 +-
>  src/xenapi/xenapi_driver.c              |  10 +-
>  tests/qemuxml2argvtest.c                |  17 --
>  tests/virdrivermoduletest.c             |  18 +-
>  67 files changed, 336 insertions(+), 2116 deletions(-)
>  delete mode 100644 src/esx/esx_device_monitor.c
>  delete mode 100644 src/esx/esx_device_monitor.h
>  delete mode 100644 src/esx/esx_nwfilter_driver.c
>  delete mode 100644 src/esx/esx_nwfilter_driver.h
>  delete mode 100644 src/esx/esx_secret_driver.c
>  delete mode 100644 src/esx/esx_secret_driver.h
>  delete mode 100644 src/hyperv/hyperv_device_monitor.c
>  delete mode 100644 src/hyperv/hyperv_device_monitor.h
>  delete mode 100644 src/hyperv/hyperv_interface_driver.c
>  delete mode 100644 src/hyperv/hyperv_interface_driver.h
>  delete mode 100644 src/hyperv/hyperv_network_driver.c
>  delete mode 100644 src/hyperv/hyperv_network_driver.h
>  delete mode 100644 src/hyperv/hyperv_nwfilter_driver.c
>  delete mode 100644 src/hyperv/hyperv_nwfilter_driver.h
>  delete mode 100644 src/hyperv/hyperv_secret_driver.c
>  delete mode 100644 src/hyperv/hyperv_secret_driver.h
>  delete mode 100644 src/hyperv/hyperv_storage_driver.c
>  delete mode 100644 src/hyperv/hyperv_storage_driver.h

Instead of finding a needle in haystack (read pointing out line in
the diff that needs to be fixed), I'm pasting diff that you need to
squash in. I'm sure you'll get the idea why:

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 5531d6d..b569160 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -262,7 +262,9 @@ parallelsConnectOpen(virConnectPtr conn,
         return VIR_DRV_OPEN_ERROR;
     }
 
-    if ((ret = parallelsOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS)
+    if ((ret = parallelsOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS ||
+        (ret = parallelsStorageOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS ||
+        (ret = parallelsNetworkOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS)
         return ret;
 
     return VIR_DRV_OPEN_SUCCESS;
@@ -273,6 +275,9 @@ parallelsConnectClose(virConnectPtr conn)
 {
     parallelsConnPtr privconn = conn->privateData;
 
+    parallelsNetworkClose(conn);
+    parallelsStorageClose(conn);
+
     parallelsDriverLock(privconn);
     prlsdkUnsubscribeFromPCSEvents(privconn);
     virObjectUnref(privconn->caps);
@@ -952,7 +957,6 @@ parallelsDomainUndefine(virDomainPtr domain)
 }
 
 static virHypervisorDriver parallelsDriver = {
-    .no = VIR_DRV_PARALLELS,
     .name = "Parallels",
     .connectOpen = parallelsConnectOpen,            /* 0.10.0 */
     .connectClose = parallelsConnectClose,          /* 0.10.0 */
@@ -995,6 +999,12 @@ static virHypervisorDriver parallelsDriver = {
     .connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */
 };
 
+static virConnectDriver parallelsConnectDriver = {
+    .hypervisorDriver = &parallelsDriver,
+    .storageDriver = &parallelsStorageDriver,
+    .networkDriver = &parallelsNetworkDriver,
+};
+
 /**
  * parallelsRegister:
  *
@@ -1013,11 +1023,7 @@ parallelsRegister(void)
 
     VIR_FREE(prlctl_path);
 
-    if (virRegisterHypervisorDriver(&parallelsDriver) < 0)
-        return -1;
-    if (parallelsStorageRegister())
-        return -1;
-    if (parallelsNetworkRegister())
+    if (virRegisterConnectDriver(&parallelsConnectDriver, false) < 0)
         return -1;
 
     return 0;
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 90217df..3e7087d 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -318,9 +318,8 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
     return ret;
 }
 
-static virDrvOpenStatus
+virDrvOpenStatus
 parallelsNetworkOpen(virConnectPtr conn,
-                     virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                      unsigned int flags)
 {
     virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
@@ -334,7 +333,7 @@ parallelsNetworkOpen(virConnectPtr conn,
     return VIR_DRV_OPEN_SUCCESS;
 }
 
-static int parallelsNetworkClose(virConnectPtr conn)
+int parallelsNetworkClose(virConnectPtr conn)
 {
     parallelsConnPtr privconn = conn->privateData;
     parallelsDriverLock(privconn);
@@ -597,10 +596,9 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
         virNetworkObjUnlock(network);
     return ret;
 }
-static virNetworkDriver parallelsNetworkDriver = {
-    "Parallels",
-    .networkOpen = parallelsNetworkOpen, /* 1.0.1 */
-    .networkClose = parallelsNetworkClose, /* 1.0.1 */
+
+virNetworkDriver parallelsNetworkDriver = {
+    .name = "Parallels",
     .connectNumOfNetworks = parallelsConnectNumOfNetworks, /* 1.0.1 */
     .connectListNetworks = parallelsConnectListNetworks, /* 1.0.1 */
     .connectNumOfDefinedNetworks = parallelsConnectNumOfDefinedNetworks, /* 1.0.1 */
@@ -613,12 +611,3 @@ static virNetworkDriver parallelsNetworkDriver = {
     .networkIsActive = parallelsNetworkIsActive, /* 1.0.1 */
     .networkIsPersistent = parallelsNetworkIsPersistent, /* 1.0.1 */
 };
-
-int
-parallelsNetworkRegister(void)
-{
-    if (virRegisterNetworkDriver(&parallelsNetworkDriver) < 0)
-        return -1;
-
-    return 0;
-}
diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c
index e916e0f..d2c5bf2 100644
--- a/src/parallels/parallels_storage.c
+++ b/src/parallels/parallels_storage.c
@@ -67,7 +67,7 @@ parallelsStorageUnlock(virStorageDriverStatePtr driver)
     virMutexUnlock(&driver->lock);
 }
 
-static int
+int
 parallelsStorageClose(virConnectPtr conn)
 {
     parallelsConnPtr privconn = conn->privateData;
@@ -456,9 +456,8 @@ static int parallelsLoadPools(virConnectPtr conn)
     return -1;
 }
 
-static virDrvOpenStatus
+virDrvOpenStatus
 parallelsStorageOpen(virConnectPtr conn,
-                     virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                      unsigned int flags)
 {
     parallelsConnPtr privconn = conn->privateData;
@@ -1612,10 +1611,8 @@ parallelsStorageVolGetPath(virStorageVolPtr vol)
     return ret;
 }
 
-static virStorageDriver parallelsStorageDriver = {
+virStorageDriver parallelsStorageDriver = {
     .name = "Parallels",
-    .storageOpen = parallelsStorageOpen,     /* 0.10.0 */
-    .storageClose = parallelsStorageClose,   /* 0.10.0 */
 
     .connectNumOfStoragePools = parallelsConnectNumOfStoragePools,   /* 0.10.0 */
     .connectListStoragePools = parallelsConnectListStoragePools,   /* 0.10.0 */
@@ -1648,12 +1645,3 @@ static virStorageDriver parallelsStorageDriver = {
     .storagePoolIsActive = parallelsStoragePoolIsActive,     /* 0.10.0 */
     .storagePoolIsPersistent = parallelsStoragePoolIsPersistent,     /* 0.10.0 */
 };
-
-int
-parallelsStorageRegister(void)
-{
-    if (virRegisterStorageDriver(&parallelsStorageDriver) < 0)
-        return -1;
-
-    return 0;
-}
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index b5aa209..bebf841 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -75,8 +75,13 @@ struct parallelsDomObj {
 
 typedef struct parallelsDomObj *parallelsDomObjPtr;
 
-int parallelsStorageRegister(void);
-int parallelsNetworkRegister(void);
+virDrvOpenStatus parallelsStorageOpen(virConnectPtr conn, unsigned int flags);
+int parallelsStorageClose(virConnectPtr conn);
+extern virStorageDriver parallelsStorageDriver;
+
+virDrvOpenStatus parallelsNetworkOpen(virConnectPtr conn, unsigned int flags);
+int parallelsNetworkClose(virConnectPtr conn);
+extern virNetworkDriver parallelsNetworkDriver;
 
 virJSONValuePtr parallelsParseOutput(const char *binary, ...)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 3ea91de..eefe9ab 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -2045,7 +2045,7 @@ static virConnectDriver xenapiConnectDriver = {
 int
 xenapiRegister(void)
 {
-    return virRegisterConnectDriver(&xenapiConnectDriver);
+    return virRegisterConnectDriver(&xenapiConnectDriver, true);
 }
 
 /*


Michal




More information about the libvir-list mailing list