[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/7] storage: Process storage pool capabilities



On 1/16/19 2:15 AM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1581670

During storage driver backend initialization, let's save
which backends are available in the storage pool capabilities.

In order to format those, we need add a connectGetCapabilities
processor to the storageHypervisorDriver. This allows a storage
connection, such as "storage:///system" to find the API and
format the results.

NB: This is not available from other hypervisor drivers such as
QEMU type connections.

Signed-off-by: John Ferlan <jferlan redhat com>
---
  src/conf/virstorageobj.h      |  4 ++++
  src/storage/storage_backend.c | 16 ++++++++++++++++
  src/storage/storage_backend.h |  3 +++
  src/storage/storage_driver.c  | 17 +++++++++++++++++
  4 files changed, 40 insertions(+)

diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 1106aa71bd..34adc26df3 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -24,6 +24,8 @@
# include "storage_conf.h" +# include "capabilities.h"
+
  typedef struct _virStoragePoolObj virStoragePoolObj;
  typedef virStoragePoolObj *virStoragePoolObjPtr;
@@ -45,6 +47,8 @@ struct _virStorageDriverState { /* Immutable pointer, self-locking APIs */
      virObjectEventStatePtr storageEventState;
+
+    virCapsPtr caps;

Would be nice to put a comment here:
/* Immutable pointer, read only after initialized */

  };
typedef bool
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a54c338cf0..0ccc616db4 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -187,3 +187,19 @@ virStorageBackendForType(int type)
                     type, NULLSTR(virStoragePoolTypeToString(type)));
      return NULL;
  }
+
+
+virCapsPtr
+virStorageBackendGetCapabilities(void)
+{
+    virCapsPtr caps;
+    size_t i;
+
+    if (!(caps = virCapabilitiesNew(virArchFromHost(), false, false)))
+        return NULL;
+

The fact that you need to pass virArchFromHost() and false, false means, that we need to abstract our virCaps even more. I mean, what has the host architecture has to do with storage capabilities? Moreover, this is how the beginning of storage caps look like:

# virsh -c storage:///system capabilities
<capabilities>

  <host>
    <cpu>
      <arch>x86_64</arch>
    </cpu>
    <power_management/>
    <iommu support='no'/>
  </host>

+    for (i = 0; i < virStorageBackendsCount; i++)
+        virCapabilitiesAddStoragePool(caps, virStorageBackends[i]->type);
+
+    return caps;
+}
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 2b178494ae..c670c66287 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -126,4 +126,7 @@ int virStorageBackendDriversRegister(bool allmodules);
int virStorageBackendRegister(virStorageBackendPtr backend); +virCapsPtr
+virStorageBackendGetCapabilities(void);
+
  #endif /* LIBVIRT_STORAGE_BACKEND_H */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4a13e90481..04beff6b07 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -300,6 +300,9 @@ storageStateInitialize(bool privileged,
driver->storageEventState = virObjectEventStateNew(); + if (!(driver->caps = virStorageBackendGetCapabilities()))
+        goto error;
+

Initially, I suspected that we want to refresh the caps on every 'virsh capabilities' (just like we're doing in say qemu driver). But this is different because we wouldn't load a storage driver out of the blue (where as admin might install new qemu binaries once we're running). Makes sense.


Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]