3:25 p.m.
Our recent refactors broke relative backing names in storage pools, fix it partially.
Peter Krempa (6):
storage: backend: Fix formatting of function arguments
storage: Track backing store of a volume in the target struct
storage: backend: fs: Touch up coding style
storage: fs: Process backing store data in
virStorageBackendProbeTarget
storage: fs: Properly parse backing store info
storage: fs: Don't fail volume update if backing store isn't
accessible
src/conf/storage_conf.c | 53 +++++++++--------
src/conf/storage_conf.h | 1 -
src/storage/storage_backend.c | 40 ++++++-------
src/storage/storage_backend_fs.c | 103 ++++++++++++++++++----------------
src/storage/storage_backend_gluster.c | 20 +++++--
src/storage/storage_backend_logical.c | 11 ++--
6 files changed, 129 insertions(+), 99 deletions(-)
--
2.0.0
3:25 p.m.
New subject: [libvirt] [PATCH 1/6] storage: backend: Fix formatting of function arguments
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_fs.c | 6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 7b17ca4..a36996f 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1455,16 +1455,16 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
int ret;
if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target,
- updateCapacity,
- withBlockVolFormat,
- openflags)) < 0)
+ updateCapacity,
+ withBlockVolFormat,
+ openflags)) < 0)
return ret;
if (vol->backingStore.path &&
(ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
- updateCapacity,
- withBlockVolFormat,
- VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0)
+ updateCapacity,
+ withBlockVolFormat,
+ VIR_STORAGE_VOL_OPEN_DEFAULT)) <
0)
return ret;
return 0;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1615c12..5e65f53 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -893,9 +893,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
vol->backingStore.path = backingStore;
vol->backingStore.format = backingStoreFormat;
- ignore_value(virStorageBackendUpdateVolTargetInfo(
- &vol->backingStore, true, false,
- VIR_STORAGE_VOL_OPEN_DEFAULT));
+ ignore_value(virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
+ true, false,
+
VIR_STORAGE_VOL_OPEN_DEFAULT));
/* If this failed, the backing file is currently unavailable,
* the capacity, allocation, owner, group and mode are unknown.
* An error message was raised, but we just continue. */
--
2.0.0
6:15 p.m.
New subject: [libvirt] [PATCH 1/6] storage: backend: Fix formatting of function arguments
On 07/15/2014 07:25 AM, Peter Krempa wrote:
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_fs.c | 6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
ACK; mechanical.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:25 p.m.
New subject: [libvirt] [PATCH 2/6] storage: Track backing store of a volume in the target struct
As we have a nested pointer for storing the backing store of a volume
there's no need to store it in a separate struct.
---
src/conf/storage_conf.c | 53 ++++++++++++++++++++---------------
src/conf/storage_conf.h | 1 -
src/storage/storage_backend.c | 27 +++++++++---------
src/storage/storage_backend_fs.c | 9 ++++--
src/storage/storage_backend_gluster.c | 20 +++++++++----
src/storage/storage_backend_logical.c | 11 +++++---
6 files changed, 72 insertions(+), 49 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index aa29658..c79aebd 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -331,7 +331,6 @@ virStorageVolDefFree(virStorageVolDefPtr def)
VIR_FREE(def->source.extents);
virStorageSourceClear(&def->target);
- virStorageSourceClear(&def->backingStore);
VIR_FREE(def);
}
@@ -1168,6 +1167,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
char *allocation = NULL;
char *capacity = NULL;
char *unit = NULL;
+ char *backingStore = NULL;
xmlNodePtr node;
xmlNodePtr *nodes = NULL;
size_t i;
@@ -1252,21 +1252,35 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
goto error;
}
- ret->backingStore.path = virXPathString("string(./backingStore/path)",
ctxt);
- if (options->formatFromString) {
- char *format = virXPathString("string(./backingStore/format/@type)",
ctxt);
- if (format == NULL)
- ret->backingStore.format = options->defaultFormat;
- else
- ret->backingStore.format = (options->formatFromString)(format);
+ if ((backingStore = virXPathString("string(./backingStore/path)", ctxt)))
{
+ if (VIR_ALLOC(ret->target.backingStore) < 0)
+ goto error;
- if (ret->backingStore.format < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown volume format type %s"), format);
+ ret->target.backingStore->path = backingStore;
+ backingStore = NULL;
+
+ if (options->formatFromString) {
+ char *format =
virXPathString("string(./backingStore/format/@type)", ctxt);
+ if (format == NULL)
+ ret->target.backingStore->format = options->defaultFormat;
+ else
+ ret->target.backingStore->format =
(options->formatFromString)(format);
+
+ if (ret->target.backingStore->format < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown volume format type %s"), format);
+ VIR_FREE(format);
+ goto error;
+ }
VIR_FREE(format);
- goto error;
}
- VIR_FREE(format);
+
+ if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
+ goto error;
+ if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
+ "./backingStore/permissions",
+ DEFAULT_VOL_PERM_MODE) < 0)
+ goto error;
}
ret->target.compat = virXPathString("string(./target/compat)", ctxt);
@@ -1308,19 +1322,13 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(nodes);
}
- if (VIR_ALLOC(ret->backingStore.perms) < 0)
- goto error;
- if (virStorageDefParsePerms(ctxt, ret->backingStore.perms,
- "./backingStore/permissions",
- DEFAULT_VOL_PERM_MODE) < 0)
- goto error;
-
cleanup:
VIR_FREE(nodes);
VIR_FREE(allocation);
VIR_FREE(capacity);
VIR_FREE(unit);
VIR_FREE(type);
+ VIR_FREE(backingStore);
return ret;
error:
@@ -1544,9 +1552,10 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
&def->target, "target") < 0)
goto cleanup;
- if (def->backingStore.path &&
+ if (def->target.backingStore &&
virStorageVolTargetDefFormat(options, &buf,
- &def->backingStore, "backingStore")
< 0)
+ def->target.backingStore,
+ "backingStore") < 0)
goto cleanup;
virBufferAdjustIndent(&buf, -2);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 47f769b..badf7a3 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -68,7 +68,6 @@ struct _virStorageVolDef {
virStorageVolSource source;
virStorageSource target;
- virStorageSource backingStore;
};
typedef struct _virStorageVolDefList virStorageVolDefList;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a36996f..f5bfdee 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -844,11 +844,11 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
}
- if (vol->backingStore.path) {
+ if (vol->target.backingStore) {
int accessRetCode = -1;
char *absolutePath = NULL;
- backingType = virStorageFileFormatTypeToString(vol->backingStore.format);
+ backingType =
virStorageFileFormatTypeToString(vol->target.backingStore->format);
if (preallocate) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -862,7 +862,8 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
* may cause issues with lvm. Untested essentially.
*/
if (inputvol &&
- STRNEQ_NULLABLE(inputvol->backingStore.path, vol->backingStore.path))
{
+ STRNEQ_NULLABLE(inputvol->target.backingStore->path,
+ vol->target.backingStore->path)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("a different backing store cannot be
specified."));
return NULL;
@@ -871,24 +872,24 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
if (backingType == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown storage vol backing store type %d"),
- vol->backingStore.format);
+ vol->target.backingStore->format);
return NULL;
}
/* Convert relative backing store paths to absolute paths for access
* validation.
*/
- if ('/' != *(vol->backingStore.path) &&
+ if ('/' != *(vol->target.backingStore->path) &&
virAsprintf(&absolutePath, "%s/%s",
pool->def->target.path,
- vol->backingStore.path) < 0)
+ vol->target.backingStore->path) < 0)
return NULL;
accessRetCode = access(absolutePath ? absolutePath
- : vol->backingStore.path, R_OK);
+ : vol->target.backingStore->path, R_OK);
VIR_FREE(absolutePath);
if (accessRetCode != 0) {
virReportSystemError(errno,
_("inaccessible backing store volume %s"),
- vol->backingStore.path);
+ vol->target.backingStore->path);
return NULL;
}
}
@@ -929,7 +930,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
cmd = virCommandNew(create_tool);
convert = !!inputvol;
- backing = !inputvol && vol->backingStore.path;
+ backing = !inputvol && vol->target.backingStore;
if (convert)
virCommandAddArgList(cmd, "convert", "-f", inputType,
"-O", type, NULL);
@@ -937,7 +938,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
virCommandAddArgList(cmd, "create", "-f", type, NULL);
if (backing)
- virCommandAddArgList(cmd, "-b", vol->backingStore.path, NULL);
+ virCommandAddArgList(cmd, "-b", vol->target.backingStore->path,
NULL);
if (imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS) {
if (vol->target.format == VIR_STORAGE_FILE_QCOW2 && !compat
&&
@@ -1055,7 +1056,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
vol->target.format);
return -1;
}
- if (vol->backingStore.path != NULL) {
+ if (vol->target.backingStore != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("copy-on-write image not supported with "
"qcow-create"));
@@ -1460,8 +1461,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
openflags)) < 0)
return ret;
- if (vol->backingStore.path &&
- (ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
+ if (vol->target.backingStore &&
+ (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
updateCapacity,
withBlockVolFormat,
VIR_STORAGE_VOL_OPEN_DEFAULT)) <
0)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 5e65f53..8b0beea 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -890,10 +890,13 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
vol->type = VIR_STORAGE_VOL_DIR;
if (backingStore != NULL) {
- vol->backingStore.path = backingStore;
- vol->backingStore.format = backingStoreFormat;
+ if (VIR_ALLOC(vol->target.backingStore) < 0)
+ goto cleanup;
+
+ vol->target.backingStore->path = backingStore;
+ vol->target.backingStore->format = backingStoreFormat;
- ignore_value(virStorageBackendUpdateVolTargetInfo(&vol->backingStore,
+
ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
true, false,
VIR_STORAGE_VOL_OPEN_DEFAULT));
/* If this failed, the backing file is currently unavailable,
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 76d2461..0f8f0f3 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -246,6 +246,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr
state,
virStorageSourcePtr meta = NULL;
char *header = NULL;
ssize_t len = VIR_STORAGE_MAX_HEADER;
+ int backingFormat;
*volptr = NULL;
@@ -295,16 +296,23 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr
state,
if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
VIR_STORAGE_FILE_AUTO,
- &vol->backingStore.format)))
+ &backingFormat)))
goto cleanup;
- vol->backingStore.path = meta->backingStoreRaw;
- meta->backingStoreRaw = NULL;
+ if (meta->backingStoreRaw) {
+ if (VIR_ALLOC(vol->target.backingStore) < 0)
+ goto cleanup;
+
+ vol->target.backingStore->path = meta->backingStoreRaw;
+
+ if (backingFormat < 0)
+ vol->target.backingStore->format = VIR_STORAGE_FILE_RAW;
+ else
+ vol->target.backingStore->format = backingFormat;
+ meta->backingStoreRaw = NULL;
+ }
vol->target.format = meta->format;
- if (vol->backingStore.path &&
- vol->backingStore.format < 0)
- vol->backingStore.format = VIR_STORAGE_FILE_RAW;
if (meta->capacity)
vol->target.capacity = meta->capacity;
if (meta->encryption) {
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index faa9a4b..60ad5f2 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -139,11 +139,14 @@ virStorageBackendLogicalMakeVol(char **const groups,
* lv is created with "--virtualsize").
*/
if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] !=
'[')) {
- if (virAsprintf(&vol->backingStore.path, "%s/%s",
+ if (VIR_ALLOC(vol->target.backingStore) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&vol->target.backingStore->path, "%s/%s",
pool->def->target.path, groups[1]) < 0)
goto cleanup;
- vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
+ vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
}
if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
@@ -752,8 +755,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
}
virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity,
1024));
- if (vol->backingStore.path)
- virCommandAddArgList(cmd, "-s", vol->backingStore.path, NULL);
+ if (vol->target.backingStore)
+ virCommandAddArgList(cmd, "-s", vol->target.backingStore->path,
NULL);
else
virCommandAddArg(cmd, pool->def->source.name);
--
2.0.0
9:21 p.m.
New subject: [libvirt] [PATCH 2/6] storage: Track backing store of a volume in the target struct
On 07/15/2014 07:25 AM, Peter Krempa wrote:
As we have a nested pointer for storing the backing store of a
volume
there's no need to store it in a separate struct.
---
src/conf/storage_conf.c | 53 ++++++++++++++++++++---------------
src/conf/storage_conf.h | 1 -
src/storage/storage_backend.c | 27 +++++++++---------
src/storage/storage_backend_fs.c | 9 ++++--
src/storage/storage_backend_gluster.c | 20 +++++++++----
src/storage/storage_backend_logical.c | 11 +++++---
6 files changed, 72 insertions(+), 49 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:25 p.m.
New subject: [libvirt] [PATCH 3/6] storage: backend: fs: Touch up coding style
virStorageBackendFileSystemRefresh() used "cleanup" label just for error
exits and didn't meet libvirt's standard for braces in one case.
---
src/storage/storage_backend_fs.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 8b0beea..6455505 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -840,7 +840,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
virReportSystemError(errno,
_("cannot open path '%s'"),
pool->def->target.path);
- goto cleanup;
+ goto error;
}
while ((direrr = virDirRead(dir, &ent, pool->def->target.path)) > 0) {
@@ -849,20 +849,20 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
int backingStoreFormat;
if (VIR_ALLOC(vol) < 0)
- goto cleanup;
+ goto error;
if (VIR_STRDUP(vol->name, ent->d_name) < 0)
- goto cleanup;
+ goto error;
vol->type = VIR_STORAGE_VOL_FILE;
vol->target.format = VIR_STORAGE_FILE_RAW; /* Real value is filled in during
probe */
if (virAsprintf(&vol->target.path, "%s/%s",
pool->def->target.path,
vol->name) == -1)
- goto cleanup;
+ goto error;
if (VIR_STRDUP(vol->key, vol->target.path) < 0)
- goto cleanup;
+ goto error;
if ((ret = virStorageBackendProbeTarget(&vol->target,
&backingStore,
@@ -881,8 +881,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
* break virStorageVolTargetDefFormat() generating the line
* <format type='...'/>. */
backingStoreFormat = VIR_STORAGE_FILE_RAW;
- } else
- goto cleanup;
+ } else {
+ goto error;
+ }
}
/* directory based volume */
@@ -891,7 +892,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (backingStore != NULL) {
if (VIR_ALLOC(vol->target.backingStore) < 0)
- goto cleanup;
+ goto error;
vol->target.backingStore->path = backingStore;
vol->target.backingStore->format = backingStoreFormat;
@@ -906,10 +907,10 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <
0)
- goto cleanup;
+ goto error;
}
if (direrr < 0)
- goto cleanup;
+ goto error;
closedir(dir);
if (statvfs(pool->def->target.path, &sb) < 0) {
@@ -926,7 +927,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
return 0;
- cleanup:
+ error:
if (dir)
closedir(dir);
virStorageVolDefFree(vol);
--
2.0.0
6:28 p.m.
New subject: [libvirt] [PATCH 3/6] storage: backend: fs: Touch up coding style
On 07/15/2014 07:25 AM, Peter Krempa wrote:
virStorageBackendFileSystemRefresh() used "cleanup" label
just for error
exits and didn't meet libvirt's standard for braces in one case.
---
src/storage/storage_backend_fs.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:25 p.m.
New subject: [libvirt] [PATCH 4/6] storage: fs: Process backing store data in virStorageBackendProbeTarget
Move the processing of the backend metadata directly to the helper
instead of passing it through arguments to the function.
---
src/storage/storage_backend_fs.c | 72 ++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 39 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 6455505..0d01dca 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,20 +61,17 @@ VIR_LOG_INIT("storage.storage_backend_fs");
#define VIR_STORAGE_VOL_FS_PROBE_FLAGS (VIR_STORAGE_VOL_FS_OPEN_FLAGS | \
VIR_STORAGE_VOL_OPEN_NOERROR)
-static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+static int
virStorageBackendProbeTarget(virStorageSourcePtr target,
- char **backingStore,
- int *backingStoreFormat,
virStorageEncryptionPtr *encryption)
{
+ int backingStoreFormat;
int fd = -1;
int ret = -1;
int rc;
virStorageSourcePtr meta = NULL;
struct stat sb;
- *backingStore = NULL;
- *backingStoreFormat = VIR_STORAGE_FILE_AUTO;
if (encryption)
*encryption = NULL;
@@ -95,32 +92,41 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
if (!(meta = virStorageFileGetMetadataFromFD(target->path,
fd,
VIR_STORAGE_FILE_AUTO,
- backingStoreFormat)))
- goto cleanup;
-
- if (VIR_STRDUP(*backingStore, meta->backingStoreRaw) < 0)
+ &backingStoreFormat)))
goto cleanup;
- /* Default to success below this point */
- ret = 0;
+ if (meta->backingStoreRaw) {
+ if (VIR_ALLOC(target->backingStore) < 0)
+ goto cleanup;
+
+ target->backingStore->path = meta->backingStoreRaw;
+ meta->backingStoreRaw = NULL;
+ target->backingStore->format = backingStoreFormat;
+
+ if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) {
+ if (!virStorageIsFile(target->backingStore->path) ||
+ (rc = virStorageFileProbeFormat(target->backingStore->path,
+ -1, -1)) < 0) {
+ /* If the backing file is currently unavailable or is
+ * accessed via remote protocol only log an error, fake the
+ * format as RAW and continue. Returning -1 here would
+ * disable the whole storage pool, making it unavailable for
+ * even maintenance. */
+ target->backingStore->format = VIR_STORAGE_FILE_RAW;
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot probe backing volume format: %s"),
+ target->backingStore->path);
+ ret = -3;
+ } else {
+ target->backingStore->format = rc;
+ }
+ }
+ }
target->format = meta->format;
- if (*backingStore &&
- *backingStoreFormat == VIR_STORAGE_FILE_AUTO &&
- virStorageIsFile(*backingStore)) {
- if ((rc = virStorageFileProbeFormat(*backingStore, -1, -1)) < 0) {
- /* If the backing file is currently unavailable, only log an error,
- * but continue. Returning -1 here would disable the whole storage
- * pool, making it unavailable for even maintenance. */
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot probe backing volume format: %s"),
- *backingStore);
- ret = -3;
- } else {
- *backingStoreFormat = rc;
- }
- }
+ /* Default to success below this point */
+ ret = 0;
if (meta->capacity)
target->capacity = meta->capacity;
@@ -845,8 +851,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
while ((direrr = virDirRead(dir, &ent, pool->def->target.path)) > 0) {
int ret;
- char *backingStore;
- int backingStoreFormat;
if (VIR_ALLOC(vol) < 0)
goto error;
@@ -865,8 +869,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto error;
if ((ret = virStorageBackendProbeTarget(&vol->target,
- &backingStore,
- &backingStoreFormat,
&vol->target.encryption)) < 0)
{
if (ret == -2) {
/* Silently ignore non-regular files,
@@ -880,7 +882,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
* failed: continue with faked RAW format, since AUTO will
* break virStorageVolTargetDefFormat() generating the line
* <format type='...'/>. */
- backingStoreFormat = VIR_STORAGE_FILE_RAW;
} else {
goto error;
}
@@ -890,13 +891,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (vol->target.format == VIR_STORAGE_FILE_DIR)
vol->type = VIR_STORAGE_VOL_DIR;
- if (backingStore != NULL) {
- if (VIR_ALLOC(vol->target.backingStore) < 0)
- goto error;
-
- vol->target.backingStore->path = backingStore;
- vol->target.backingStore->format = backingStoreFormat;
-
+ if (vol->target.backingStore) {
ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
true, false,
VIR_STORAGE_VOL_OPEN_DEFAULT));
@@ -905,7 +900,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
* An error message was raised, but we just continue. */
}
-
if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <
0)
goto error;
}
--
2.0.0
9:29 p.m.
New subject: [libvirt] [PATCH 4/6] storage: fs: Process backing store data in virStorageBackendProbeTarget
On 07/15/2014 07:25 AM, Peter Krempa wrote:
Move the processing of the backend metadata directly to the helper
instead of passing it through arguments to the function.
---
src/storage/storage_backend_fs.c | 72 ++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 39 deletions(-)
+
+ if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) {
+ if (!virStorageIsFile(target->backingStore->path) ||
+ (rc = virStorageFileProbeFormat(target->backingStore->path,
+ -1, -1)) < 0) {
Indentation is off.
ACK with that nit fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:25 p.m.
New subject: [libvirt] [PATCH 5/6] storage: fs: Properly parse backing store info
Use the backing store parser to properly create the information about a
volume's backing store. Unfortunately as the storage driver isn't
perpared to allow volumes backed by networked filesystems add a
workaroud that will avoid changing the XML output.
---
src/storage/storage_backend_fs.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0d01dca..728e640 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -96,16 +96,27 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
goto cleanup;
if (meta->backingStoreRaw) {
- if (VIR_ALLOC(target->backingStore) < 0)
+ if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
goto cleanup;
- target->backingStore->path = meta->backingStoreRaw;
- meta->backingStoreRaw = NULL;
target->backingStore->format = backingStoreFormat;
+ /* XXX: Remote storage doesn't play nicely with volumes backed by
+ * remote storage. To avoid trouble, just fake the ou */
+ if (!virStorageSourceIsLocalStorage(target->backingStore)) {
+ virStorageSourceFree(target->backingStore);
+
+ if (VIR_ALLOC(target->backingStore) < 0)
+ goto cleanup;
+
+ target->backingStore->type = VIR_STORAGE_TYPE_FILE;
+ target->backingStore->path = meta->backingStoreRaw;
+ meta->backingStoreRaw = NULL;
+ target->backingStore->format = VIR_STORAGE_FILE_RAW;
+ }
+
if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) {
- if (!virStorageIsFile(target->backingStore->path) ||
- (rc = virStorageFileProbeFormat(target->backingStore->path,
+ if ((rc = virStorageFileProbeFormat(target->backingStore->path,
-1, -1)) < 0) {
/* If the backing file is currently unavailable or is
* accessed via remote protocol only log an error, fake the
--
2.0.0
10:11 p.m.
New subject: [libvirt] [PATCH 5/6] storage: fs: Properly parse backing store info
On 07/15/2014 07:25 AM, Peter Krempa wrote:
Use the backing store parser to properly create the information about
a
volume's backing store. Unfortunately as the storage driver isn't
perpared to allow volumes backed by networked filesystems add a
s/perpared/prepared/
workaroud that will avoid changing the XML output.
s/workaroud/workaround/
---
src/storage/storage_backend_fs.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
+ /* XXX: Remote storage doesn't play nicely with volumes backed by
+ * remote storage. To avoid trouble, just fake the ou */
Incomplete thought?
+
if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) {
- if (!virStorageIsFile(target->backingStore->path) ||
- (rc = virStorageFileProbeFormat(target->backingStore->path,
+ if ((rc = virStorageFileProbeFormat(target->backingStore->path,
-1, -1)) < 0) {
Indentation is off.
ACK with those fixes.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:25 p.m.
New subject: [libvirt] [PATCH 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
When the backing store of a volume wasn't accessible while updating the
volume definition the call would fail alltogether. In cases where we
currently (incorrectly) treat remote backing stores as local one this
might lead to strange errors.
Ignore the opening errors until we figure out how to track proper volume
metadata.
---
src/storage/storage_backend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index f5bfdee..fdcaada 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
(ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
updateCapacity,
withBlockVolFormat,
- VIR_STORAGE_VOL_OPEN_DEFAULT)) <
0)
+ VIR_STORAGE_VOL_OPEN_DEFAULT |
+ VIR_STORAGE_VOL_OPEN_NOERROR) <
0))
return ret;
return 0;
--
2.0.0
10:29 p.m.
New subject: [libvirt] [PATCH 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
On 07/15/2014 07:25 AM, Peter Krempa wrote:
When the backing store of a volume wasn't accessible while
updating the
volume definition the call would fail alltogether. In cases where we
s/alltogether/altogether/
currently (incorrectly) treat remote backing stores as local one
this
might lead to strange errors.
Ignore the opening errors until we figure out how to track proper volume
metadata.
---
src/storage/storage_backend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index f5bfdee..fdcaada 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
(ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
updateCapacity,
withBlockVolFormat,
- VIR_STORAGE_VOL_OPEN_DEFAULT)) <
0)
+ VIR_STORAGE_VOL_OPEN_DEFAULT |
+ VIR_STORAGE_VOL_OPEN_NOERROR) <
0))
return ret;
Works for me as a stopgap. (On IRC, we were discussing that we probably
want to update the storage volume XML for <backingStore> to look a bit
more like domain <disk> backingstore, at least for cases where we know
the backing store is not in the same pool as the source - but that's a
bigger project so worth later patches).
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:48 a.m.
New subject: [libvirt] [PATCH 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
On 07/15/14 22:29, Eric Blake wrote:
On 07/15/2014 07:25 AM, Peter Krempa wrote:
> When the backing store of a volume wasn't accessible while updating the
> volume definition the call would fail alltogether. In cases where we
s/alltogether/altogether/
> currently (incorrectly) treat remote backing stores as local one this
> might lead to strange errors.
>
> Ignore the opening errors until we figure out how to track proper volume
> metadata.
> ---
> src/storage/storage_backend.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index f5bfdee..fdcaada 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
> (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
> updateCapacity,
> withBlockVolFormat,
> - VIR_STORAGE_VOL_OPEN_DEFAULT))
< 0)
> + VIR_STORAGE_VOL_OPEN_DEFAULT |
> + VIR_STORAGE_VOL_OPEN_NOERROR)
< 0))
> return ret;
>
Works for me as a stopgap. (On IRC, we were discussing that we probably
want to update the storage volume XML for <backingStore> to look a bit
more like domain <disk> backingstore, at least for cases where we know
the backing store is not in the same pool as the source - but that's a
bigger project so worth later patches).
ACK.
I've fixed the problems pointed out in the individual reviews and pushed
this series.
Thanks.
Peter
3718
days inactive
3719
days old
13 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa