[libvirt] [PATCH v2] storage: netfs and iscsi need option srcSpec for resource discovery
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. error: Failed to find any netfs pool sources error: (storage_source_specification):1: Document is empty (null) This patch adds a check for it, error info changed to: error: Failed to find any netfs pool sources error: internal error pool type 'netfs' need option \ --srcSpec for source discovery --- src/storage/storage_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fbc630d..8486fdf 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -458,6 +458,14 @@ storageFindPoolSources(virConnectPtr conn, goto cleanup; } + if ((*srcSpec == '\0') && (backend_type == VIR_STORAGE_POOL_ISCSI || + backend_type == VIR_STORAGE_POOL_NETFS)) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("pool type '%s' needs option --srcSpec " + "for source discovery"), type); + goto cleanup; + } + ret = backend->findPoolSources(conn, srcSpec, flags); cleanup: -- 1.7.7.5
On 2012年07月18日 18:26, Guannan Ren wrote:
The option 'srcSpec' to virsh command find-storage-pool-sources
s/to/of/
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
s/and/or/
parsing error due to null string option srcSpec.
error: Failed to find any netfs pool sources error: (storage_source_specification):1: Document is empty (null)
This patch adds a check for it, error info changed to:
s/changed/is changed/
error: Failed to find any netfs pool sources error: internal error pool type 'netfs' need option \ --srcSpec for source discovery --- src/storage/storage_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fbc630d..8486fdf 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -458,6 +458,14 @@ storageFindPoolSources(virConnectPtr conn, goto cleanup; }
+ if ((*srcSpec == '\0')&& (backend_type == VIR_STORAGE_POOL_ISCSI ||
Is "srcSpec" guaranteed not NULL? it will cause libvird to crash if not.
+ backend_type == VIR_STORAGE_POOL_NETFS)) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("pool type '%s' needs option --srcSpec " + "for source discovery"), type);
It should be changed to virReportError now.
+ goto cleanup; + } +
The checking should be in virStoragePoolDefParseSourceString instead. And it should error out regardless of the pool type. Any use of the function will get error if srcSpec is NULL. Logical backend doesn't get the error just because of it doesn't use it. Regards, Osier
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. error: Failed to find any netfs pool sources error: (storage_source_specification):1: Document is empty (null) error: Failed to find any netfs pool sources error: internal error pool type 'netfs' requires option \ --srcSpec for source discovery --- src/remote/remote_driver.c | 14 +------------- src/storage/storage_driver.c | 8 ++++++++ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index acae5d0..5f8d378 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2621,23 +2621,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_driver.c b/src/storage/storage_driver.c index c9b8021..3d491eb 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -458,6 +458,14 @@ storageFindPoolSources(virConnectPtr conn, goto cleanup; } + if ((!srcSpec) && (backend_type == VIR_STORAGE_POOL_ISCSI || + backend_type == VIR_STORAGE_POOL_NETFS)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("pool type '%s' requires option --srcSpec " + "for source discovery"), type); + goto cleanup; + } + ret = backend->findPoolSources(conn, srcSpec, flags); cleanup: -- 1.7.7.5
participants (2)
-
Guannan Ren -
Osier Yang