[libvirt] [PATCH v4] storage: netfs and iscsi need option srcSpec for resource discovery

Guannan Ren gren at redhat.com
Tue Jul 31 08:56:41 UTC 2012


The option 'srcSpec' to virsh command find-storage-pool-sources
is optional for logical type of storage pool, but mandatory for
netfs and iscsi type.
When missing the option for netfs and iscsi, libvirt reports XML
parsing error due to null string option srcSpec.

before
error: Failed to find any netfs pool sources
error: (storage_source_specification):1: Document is empty
(null)

after:
error: pool type 'iscsi' requires option --srcSpec for source discovery
---
 src/remote/remote_driver.c          |   14 +-------------
 src/storage/storage_backend_fs.c    |   13 +++++++++----
 src/storage/storage_backend_iscsi.c |    7 +++++++
 tools/virsh-pool.c                  |    6 ++++++
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9ac27b2..9354cb4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2648,23 +2648,11 @@ remoteFindStoragePoolSources (virConnectPtr conn,
     remote_find_storage_pool_sources_args args;
     remote_find_storage_pool_sources_ret ret;
     struct private_data *priv = conn->storagePrivateData;
-    const char *emptyString = "";
 
     remoteDriverLock(priv);
 
     args.type = (char*)type;
-    /*
-     * I'd think the following would work here:
-     *    args.srcSpec = (char**)&srcSpec;
-     * since srcSpec is a remote_string (not a remote_nonnull_string).
-     *
-     * But when srcSpec is NULL, this yields:
-     *    libvir: Remote error : marshaling args
-     *
-     * So for now I'm working around this by turning NULL srcSpecs
-     * into empty strings.
-     */
-    args.srcSpec = srcSpec ? (char **)&srcSpec : (char **)&emptyString;
+    args.srcSpec = srcSpec ? (char **)&srcSpec : NULL;
     args.flags = flags;
 
     memset (&ret, 0, sizeof(ret));
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 01b517a..92a3228 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -258,10 +258,15 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE
 
     virCheckFlags(0, NULL);
 
-    source = virStoragePoolDefParseSourceString(srcSpec,
-                                                VIR_STORAGE_POOL_NETFS);
-    if (!source)
-        goto cleanup;
+    if (!srcSpec) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       "%s", _("hostname must be specified for netfs sources"));
+        return NULL;
+    }
+
+    if (!(source = virStoragePoolDefParseSourceString(srcSpec,
+                                                      VIR_STORAGE_POOL_NETFS)))
+        return NULL;
 
     if (source->nhost != 1) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index b2f0c6c..75c2e52 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -582,6 +582,13 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     virCheckFlags(0, NULL);
 
+    if (!srcSpec) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       "%s", _("hostname and device path "
+                               "must be specified for iscsi sources"));
+        return NULL;
+    }
+
     if (!(source = virStoragePoolDefParseSourceString(srcSpec,
                                                       list.type)))
         return NULL;
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index af80427..0f0b21e 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -1093,6 +1093,12 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED)
     if (srcSpecFile && virFileReadAll(srcSpecFile, VIRSH_MAX_XML_FILE, &srcSpec) < 0)
         return false;
 
+    if (!srcSpec && (STREQ(type, "netfs") || STREQ(type, "iscsi"))) {
+        vshError(ctl, _("pool type '%s' requires option --srcSpec "
+                        "for source discovery"), type);
+        return false;
+    }
+
     srcList = virConnectFindStoragePoolSources(ctl->conn, type, srcSpec, 0);
     VIR_FREE(srcSpec);
     if (srcList == NULL) {
-- 
1.7.7.6




More information about the libvir-list mailing list