[libvirt] [PATCH 11/14] util: storagefile: Add handling of unusable storage sources

Peter Krempa posted 14 patches 6 years, 5 months ago
[libvirt] [PATCH 11/14] util: storagefile: Add handling of unusable storage sources
Posted by Peter Krempa 6 years, 5 months ago
Introduce new semantics to virStorageSourceNewFromBacking and some
of the helpers used by it which propagate the return value from the
callers.

The new return value introduced by this patch allows to notify the
calller that the parsed virStorageSource correctly describes the source
but contains data such as inline authentication which libvirt does not
want to support directly. This means that such file would e.g. unusable
as a storage source (e.g. when actively commiting the overlay to it) or
would not work with blockdev.

The caller will then be able to decide whether to consider this backing
file as viable or just fall back to qemu dealing with it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virstoragefile.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8447c014f0..fa0233376b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3554,6 +3554,13 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,

 struct virStorageSourceJSONDriverParser {
     const char *drvname;
+    /**
+     * The callback gets a pre-allocated storage source @src and the JSON
+     * object to parse. The callback shall return -1 on error and report error
+     * 0 on success and 1 in cases when the configuration itself is valid, but
+     * can't be converted to libvirt's configuration (e.g. inline authentication
+     * credentials are present).
+     */
     int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
     int opaque;
 };
@@ -3638,15 +3645,17 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src,
  * @path: string representing absolute location of a storage source
  * @src: filled with virStorageSource object representing @path
  *
- * Returns 0 on success and fills @src or -1 on error and reports appropriate
- * error.
+ * Returns 0 on success, 1 if we could parse all location data but @path
+ * specified other data unrepresentable by libvirt (e.g. inline authentication).
+ * In both cases @src is filled. On error -1 is returned @src is NULL and an
+ * error is reported.
  */
 int
 virStorageSourceNewFromBackingAbsolute(const char *path,
                                        virStorageSourcePtr *src)
 {
     const char *json;
-    int rc;
+    int rc = 0;
     VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;

     *src = NULL;
@@ -3687,7 +3696,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
     }

     VIR_STEAL_PTR(*src, def);
-    return 0;
+    return rc;
 }


@@ -3701,7 +3710,11 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
  * and other data. Note that for local storage this function interrogates the
  * actual type of the backing store.
  *
- * Returns 0 and fills @backing, or -1 on error (with appropriate error reported).
+ * Returns 0 on success, 1 if we could parse all location data but the backinig
+ * store specification contained other data unrepresentable by libvirt (e.g.
+ * inline authentication).
+ * In both cases @src is filled. On error -1 is returned @src is NULL and an
+ * error is reported.
  */
 int
 virStorageSourceNewFromBacking(virStorageSourcePtr parent,
@@ -3709,6 +3722,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
 {
     struct stat st;
     VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;
+    int rc = 0;

     *backing = NULL;

@@ -3717,8 +3731,8 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
                                                            parent->backingStoreRaw)))
             return -1;
     } else {
-        if (virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
-                                                   &def) < 0)
+        if ((rc = virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
+                                                         &def)) < 0)
             return -1;
     }

@@ -3742,7 +3756,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
     def->detected = true;

     VIR_STEAL_PTR(*backing, def);
-    return 0;
+    return rc;
 }


-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/14] util: storagefile: Add handling of unusable storage sources
Posted by Ján Tomko 6 years, 5 months ago
On Fri, Aug 16, 2019 at 12:39:32PM +0200, Peter Krempa wrote:
>Introduce new semantics to virStorageSourceNewFromBacking and some
>of the helpers used by it which propagate the return value from the
>callers.
>
>The new return value introduced by this patch allows to notify the
>calller that the parsed virStorageSource correctly describes the source
>but contains data such as inline authentication which libvirt does not
>want to support directly. This means that such file would e.g. unusable
>as a storage source (e.g. when actively commiting the overlay to it) or
>would not work with blockdev.
>
>The caller will then be able to decide whether to consider this backing
>file as viable or just fall back to qemu dealing with it.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/util/virstoragefile.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list