10:56 a.m.
From: Peter Krempa <pkrempa(a)redhat.com>
While historically we've stored the 'pool' and 'image' properties of
RBD
and gluster images in separate fields but they are presented in a single
field in the XML. This creates multiple points where they need to be
separated and combined.
Introduce helper 'virStorageSourceNetworkProtocolPathSplit' which will
do that at the point of use rather than everywhere in the code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 30 ++--------
src/conf/storage_source_conf.c | 56 ++++++++++++++++++-
src/conf/storage_source_conf.h | 7 ++-
src/libvirt_private.syms | 1 +
src/libxl/libxl_conf.c | 2 +-
src/libxl/xen_xl.c | 2 +-
src/qemu/qemu_block.c | 32 +++++++----
.../storage_file_backend_gluster.c | 41 +++++++-------
src/storage_file/storage_source.c | 2 -
.../storage_source_backingstore.c | 35 ++----------
10 files changed, 113 insertions(+), 95 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f1057b406c..c7bad53ae6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7381,6 +7381,9 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
return -1;
}
+ if (virStorageSourceNetworkProtocolPathSplit(src->path, src->protocol, NULL,
NULL) < 0)
+ return -1;
+
if (virXMLPropTristateBool(node, "tls", VIR_XML_PROP_NONE,
&src->haveTLS) < 0)
return -1;
@@ -7404,27 +7407,6 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
}
}
- /* for historical reasons we store the volume and image name in one XML
- * element although it complicates thing when attempting to access them. */
- if (src->path &&
- (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER ||
- src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
- char *tmp;
- if (!(tmp = strchr(src->path, '/')) ||
- tmp == src->path) {
- virReportError(VIR_ERR_XML_ERROR,
- _("can't split path '%1$s' into pool name and
image name"),
- src->path);
- return -1;
- }
-
- src->volume = src->path;
-
- src->path = g_strdup(tmp + 1);
-
- tmp[0] = '\0';
- }
-
/* snapshot currently works only for remote disks */
src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
@@ -23172,15 +23154,11 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
unsigned int flags)
{
size_t n;
- g_autofree char *path = NULL;
virBufferAsprintf(attrBuf, " protocol='%s'",
virStorageNetProtocolTypeToString(src->protocol));
- if (src->volume)
- path = g_strdup_printf("%s/%s", src->volume, src->path);
-
- virBufferEscapeString(attrBuf, " name='%s'", path ? path :
src->path);
+ virBufferEscapeString(attrBuf, " name='%s'", src->path);
virBufferEscapeString(attrBuf, " query='%s'", src->query);
if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT &&
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index 8a063be244..9014fa37e6 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -820,7 +820,6 @@ virStorageSourceCopy(const virStorageSource *src,
def->path = g_strdup(src->path);
def->fdgroup = g_strdup(src->fdgroup);
- def->volume = g_strdup(src->volume);
def->relPath = g_strdup(src->relPath);
def->backingStoreRaw = g_strdup(src->backingStoreRaw);
def->backingStoreRawFormat = src->backingStoreRawFormat;
@@ -945,7 +944,6 @@ virStorageSourceIsSameLocation(virStorageSource *a,
return false;
if (STRNEQ_NULLABLE(a->path, b->path) ||
- STRNEQ_NULLABLE(a->volume, b->volume) ||
STRNEQ_NULLABLE(a->snapshot, b->snapshot))
return false;
@@ -1152,7 +1150,6 @@ virStorageSourceClear(virStorageSource *def)
VIR_FREE(def->path);
VIR_FREE(def->fdgroup);
- VIR_FREE(def->volume);
VIR_FREE(def->vdpadev);
VIR_FREE(def->snapshot);
VIR_FREE(def->configFile);
@@ -1445,3 +1442,56 @@ virStorageSourceFDTupleNew(void)
{
return g_object_new(vir_storage_source_fd_tuple_get_type(), NULL);
}
+
+
+/**
+ * virStorageSourceNetworkProtocolPathSplit:
+ * @path: path to split
+ * @protocol: protocol
+ * @pool: filled with pool name (may be NULL)
+ * @image: filled with image name (may be NULL)
+ *
+ * Historically libvirt accepted the specification of Gluster's volume and
+ * RBD's pool as part of the 'path' field, but internally many places
require
+ * individual components.
+ *
+ * This helper validates and splits the path as appropriate for given protocol.
+ * It's useful for 'gluster' and 'rbd' protocol but for validation
can be called
+ * with any protocol.
+ */
+int
+virStorageSourceNetworkProtocolPathSplit(const char *path,
+ virStorageNetProtocol protocol,
+ char **pool,
+ char **image)
+{
+
+ g_autofree char *pathcopy = g_strdup(path);
+ char *tmp;
+
+ if (protocol != VIR_STORAGE_NET_PROTOCOL_GLUSTER &&
+ protocol != VIR_STORAGE_NET_PROTOCOL_RBD) {
+
+ if (image)
+ *image = g_steal_pointer(&pathcopy);
+
+ return 0;
+ }
+
+ if (!(tmp = strchr(pathcopy, '/')) || tmp == pathcopy) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("can't split path '%1$s' into pool name and
image name"),
+ pathcopy);
+ return -1;
+ }
+
+ tmp[0] = '\0';
+
+ if (pool)
+ *pool = g_steal_pointer(&pathcopy);
+
+ if (image)
+ *image = g_strdup(tmp + 1);
+
+ return 0;
+}
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index 362897f058..798ca20ac4 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -302,7 +302,6 @@ struct _virStorageSource {
char *path;
char *fdgroup; /* name of group of file descriptors the user wishes to use instead of
'path' */
virStorageNetProtocol protocol;
- char *volume; /* volume name for remote storage */
char *snapshot; /* for storage systems supporting internal snapshots */
char *configFile; /* some storage systems use config file as part of
the source definition */
@@ -597,3 +596,9 @@ void
virStorageSourceInitiatorClear(virStorageSourceInitiatorDef *initiator);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageAuthDef, virStorageAuthDefFree);
+
+int
+virStorageSourceNetworkProtocolPathSplit(const char *path,
+ virStorageNetProtocol protocol,
+ char **pool,
+ char **image);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a8ebf9efd8..01a13048c5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1186,6 +1186,7 @@ virStorageSourceIsRelative;
virStorageSourceIsSameLocation;
virStorageSourceNetCookiesValidate;
virStorageSourceNetworkAssignDefaultPorts;
+virStorageSourceNetworkProtocolPathSplit;
virStorageSourceNew;
virStorageSourceNVMeDefFree;
virStorageSourcePoolDefFree;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 36fba4a555..9d8301169b 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1096,7 +1096,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSource *src,
return NULL;
}
- virBufferStrcat(&buf, "rbd:", src->volume, "/",
src->path, NULL);
+ virBufferStrcat(&buf, "rbd:", src->path, NULL);
if (username) {
virBufferEscape(&buf, '\\', ":", ":id=%s",
username);
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index b83ed45c46..b2ff0edcf2 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1476,7 +1476,7 @@ xenFormatXLDiskSrcNet(virStorageSource *src)
return NULL;
}
- virBufferStrcat(&buf, "rbd:", src->volume, "/",
src->path, NULL);
+ virBufferStrcat(&buf, "rbd:", src->path, NULL);
virBufferAddLit(&buf, ":auth_supported=none");
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index a59c0a0a03..9b370d4c7c 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -213,13 +213,9 @@ qemuBlockStorageSourceGetURI(virStorageSource *src)
}
if (src->path) {
- if (src->volume) {
- uri->path = g_strdup_printf("/%s/%s", src->volume,
src->path);
- } else {
- uri->path = g_strdup_printf("%s%s",
- g_path_is_absolute(src->path) ? "" :
"/",
- src->path);
- }
+ uri->path = g_strdup_printf("%s%s",
+ g_path_is_absolute(src->path) ? "" :
"/",
+ src->path);
}
uri->query = g_strdup(src->query);
@@ -401,10 +397,17 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSource *src,
{
g_autoptr(virJSONValue) servers = NULL;
g_autoptr(virJSONValue) props = NULL;
+ g_autofree char *volume = NULL;
+ g_autofree char *path = NULL;
if (!(servers = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src)))
return NULL;
+ if (virStorageSourceNetworkProtocolPathSplit(src->path,
+ VIR_STORAGE_NET_PROTOCOL_GLUSTER,
+ &volume, &path) < 0)
+ return NULL;
+
/* { driver:"gluster",
* volume:"testvol",
* path:"/a.img",
@@ -412,8 +415,8 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSource *src,
* {type:"unix", socket:"/tmp/glusterd.socket"},
...]}
*/
if (virJSONValueObjectAdd(&props,
- "s:volume", src->volume,
- "s:path", src->path,
+ "s:volume", volume,
+ "s:path", path,
"a:server", &servers, NULL) < 0)
return NULL;
@@ -662,6 +665,13 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
const char *username = NULL;
g_autoptr(virJSONValue) authmodes = NULL;
const char *keysecret = NULL;
+ g_autofree char *pool = NULL;
+ g_autofree char *image = NULL;
+
+ if (virStorageSourceNetworkProtocolPathSplit(src->path,
+ VIR_STORAGE_NET_PROTOCOL_RBD,
+ &pool, &image) < 0)
+ return NULL;
if (src->nhosts > 0 &&
!(servers = qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(src)))
@@ -715,8 +725,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
}
if (virJSONValueObjectAdd(&ret,
- "s:pool", src->volume,
- "s:image", src->path,
+ "s:pool", pool,
+ "s:image", image,
"S:snapshot", src->snapshot,
"S:conf", src->configFile,
"A:server", &servers,
diff --git a/src/storage_file/storage_file_backend_gluster.c
b/src/storage_file/storage_file_backend_gluster.c
index abb1c47309..8778995b6c 100644
--- a/src/storage_file/storage_file_backend_gluster.c
+++ b/src/storage_file/storage_file_backend_gluster.c
@@ -37,6 +37,7 @@ VIR_LOG_INIT("storage.storage_file_gluster");
typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv;
struct _virStorageFileBackendGlusterPriv {
glfs_t *vol;
+ char *image;
};
static void
@@ -45,12 +46,13 @@ virStorageFileBackendGlusterDeinit(virStorageSource *src)
virStorageDriverData *drv = src->drv;
virStorageFileBackendGlusterPriv *priv = drv->priv;
- VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%u/%s%s)",
- src, src->hosts->name, src->hosts->port, src->volume,
src->path);
+ VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%u/%s)",
+ src, src->hosts->name, src->hosts->port, src->path);
if (priv->vol)
glfs_fini(priv->vol);
+ VIR_FREE(priv->image);
VIR_FREE(priv);
drv->priv = NULL;
}
@@ -98,25 +100,25 @@ virStorageFileBackendGlusterInit(virStorageSource *src)
{
virStorageDriverData *drv = src->drv;
g_autofree virStorageFileBackendGlusterPriv *priv = NULL;
+ g_autofree char *volume = NULL;
+ g_autofree char *image = NULL;
size_t i;
- if (!src->volume) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("missing gluster volume name for path
'%1$s'"),
- src->path);
+ if (virStorageSourceNetworkProtocolPathSplit(src->path,
+ VIR_STORAGE_NET_PROTOCOL_GLUSTER,
+ &volume, &image) < 0)
return -1;
- }
priv = g_new0(virStorageFileBackendGlusterPriv, 1);
VIR_DEBUG("initializing gluster storage file %p "
"(priv='%p' volume='%s' path='%s') as
[%u:%u]",
- src, priv, src->volume, src->path,
+ src, priv, volume, image,
(unsigned int)drv->uid, (unsigned int)drv->gid);
- if (!(priv->vol = glfs_new(src->volume))) {
+ if (!(priv->vol = glfs_new(volume))) {
virReportError(VIR_ERR_OPERATION_FAILED,
- _("failed to create glfs object for '%1$s'"),
src->volume);
+ _("failed to create glfs object for '%1$s'"),
volume);
return -1;
}
@@ -135,6 +137,7 @@ virStorageFileBackendGlusterInit(virStorageSource *src)
return -1;
}
+ priv->image = g_steal_pointer(&image);
drv->priv = g_steal_pointer(&priv);
return 0;
@@ -148,7 +151,7 @@ virStorageFileBackendGlusterCreate(virStorageSource *src)
virStorageFileBackendGlusterPriv *priv = drv->priv;
glfs_fd_t *fd = NULL;
- if (!(fd = glfs_creat(priv->vol, src->path,
+ if (!(fd = glfs_creat(priv->vol, priv->image,
O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR)))
return -1;
@@ -163,7 +166,7 @@ virStorageFileBackendGlusterUnlink(virStorageSource *src)
virStorageDriverData *drv = src->drv;
virStorageFileBackendGlusterPriv *priv = drv->priv;
- return glfs_unlink(priv->vol, src->path);
+ return glfs_unlink(priv->vol, priv->image);
}
@@ -174,7 +177,7 @@ virStorageFileBackendGlusterStat(virStorageSource *src,
virStorageDriverData *drv = src->drv;
virStorageFileBackendGlusterPriv *priv = drv->priv;
- return glfs_stat(priv->vol, src->path, st);
+ return glfs_stat(priv->vol, priv->image, st);
}
@@ -193,15 +196,15 @@ virStorageFileBackendGlusterRead(virStorageSource *src,
*buf = NULL;
- if (!(fd = glfs_open(priv->vol, src->path, O_RDONLY))) {
+ if (!(fd = glfs_open(priv->vol, priv->image, O_RDONLY))) {
virReportSystemError(errno, _("Failed to open file '%1$s'"),
- src->path);
+ priv->image);
return -1;
}
if (offset > 0) {
if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) {
- virReportSystemError(errno, _("cannot seek into '%1$s'"),
src->path);
+ virReportSystemError(errno, _("cannot seek into '%1$s'"),
priv->image);
goto cleanup;
}
}
@@ -216,7 +219,7 @@ virStorageFileBackendGlusterRead(virStorageSource *src,
continue;
if (r < 0) {
VIR_FREE(*buf);
- virReportSystemError(errno, _("unable to read '%1$s'"),
src->path);
+ virReportSystemError(errno, _("unable to read '%1$s'"),
priv->image);
return r;
}
if (r == 0)
@@ -243,7 +246,7 @@ virStorageFileBackendGlusterAccess(virStorageSource *src,
virStorageDriverData *drv = src->drv;
virStorageFileBackendGlusterPriv *priv = drv->priv;
- return glfs_access(priv->vol, src->path, mode);
+ return glfs_access(priv->vol, priv->image, mode);
}
static int
@@ -254,7 +257,7 @@ virStorageFileBackendGlusterChown(const virStorageSource *src,
virStorageDriverData *drv = src->drv;
virStorageFileBackendGlusterPriv *priv = drv->priv;
- return glfs_chown(priv->vol, src->path, uid, gid);
+ return glfs_chown(priv->vol, priv->image, uid, gid);
}
diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c
index fa59949cf2..843910a0d8 100644
--- a/src/storage_file/storage_source.c
+++ b/src/storage_file/storage_source.c
@@ -392,8 +392,6 @@ virStorageSourceNewFromBackingRelative(virStorageSource *parent,
def->nhosts = parent->nhosts;
}
-
- def->volume = g_strdup(parent->volume);
} else {
/* set the type to _FILE, the caller shall update it to the actual type */
def->type = VIR_STORAGE_TYPE_FILE;
diff --git a/src/storage_file/storage_source_backingstore.c
b/src/storage_file/storage_source_backingstore.c
index 4a273fe46c..700a2f5dcb 100644
--- a/src/storage_file/storage_source_backingstore.c
+++ b/src/storage_file/storage_source_backingstore.c
@@ -108,27 +108,9 @@ virStorageSourceParseBackingURI(virStorageSource *src,
src->path = g_strdup(path);
if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
- char *tmp;
-
- if (!src->path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("missing volume name and path for gluster
volume"));
- return -1;
- }
-
- if (!(tmp = strchr(src->path, '/')) ||
- tmp == src->path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("missing volume name or file name in gluster source
path '%1$s'"),
- src->path);
+ if (virStorageSourceNetworkProtocolPathSplit(src->path, src->protocol,
+ NULL, NULL) < 0)
return -1;
- }
-
- src->volume = src->path;
-
- src->path = g_strdup(tmp + 1);
-
- tmp[0] = '\0';
}
src->hosts->port = uri->port;
@@ -211,13 +193,6 @@ virStorageSourceParseRBDColonString(const char *rbdstr,
*p = '\0';
}
- /* pool vs. image name */
- if ((p = strchr(src->path, '/'))) {
- src->volume = g_steal_pointer(&src->path);
- src->path = g_strdup(p + 1);
- *p = '\0';
- }
-
/* options */
if (!options)
return 0; /* all done */
@@ -703,8 +678,7 @@ virStorageSourceParseBackingJSONGluster(virStorageSource *src,
src->type = VIR_STORAGE_TYPE_NETWORK;
src->protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER;
- src->volume = g_strdup(volume);
- src->path = g_strdup(path);
+ src->path = g_strdup_printf("%s/%s", volume, path);
nservers = virJSONValueArraySize(server);
if (nservers == 0) {
@@ -959,8 +933,7 @@ virStorageSourceParseBackingJSONRBD(virStorageSource *src,
return -1;
}
- src->volume = g_strdup(pool);
- src->path = g_strdup(image);
+ src->path = g_strdup_printf("%s/%s", pool, image);
src->snapshot = g_strdup(snapshot);
src->configFile = g_strdup(conf);
--
2.49.0