[PATCH 04/15] virStorageSourceParseBackingJSON: Allow 'json:' pseudo URIs without 'file' wrapper

Peter Krempa posted 15 patches 6 years ago
There is a newer version of this series
[PATCH 04/15] virStorageSourceParseBackingJSON: Allow 'json:' pseudo URIs without 'file' wrapper
Posted by Peter Krempa 6 years ago
There are two possibilities:
1) json:{"file":{"driver":...}}
2) json:{"driver":...}

Our code didn't work properly with the second one as it was expecting
the 'file' wrapper. Conditionalize the removal to only the situation
when the top level doesn't have "driver".

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

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 565ba08319..ddf837c3b3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3521,10 +3521,17 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src,
                                     const char *jsonstr,
                                     int opaque G_GNUC_UNUSED)
 {
-    /* There are no interesting attributes in raw driver.
-     * Treat it as pass-through.
-     */
-    return virStorageSourceParseBackingJSONInternal(src, json, jsonstr);
+    virJSONValuePtr file;
+
+    /* 'raw' is a format driver so it can have protocol driver children */
+    if (!(file = virJSONValueObjectGetObject(json, "file"))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("JSON backing volume definition '%s' lacks 'file' object"),
+                       jsonstr);
+        return -1;
+    }
+
+    return virStorageSourceParseBackingJSONInternal(src, file, jsonstr);
 }


@@ -3601,18 +3608,10 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src,
                                          virJSONValuePtr json,
                                          const char *jsonstr)
 {
-    virJSONValuePtr file;
     const char *drvname;
     size_t i;

-    if (!(file = virJSONValueObjectGetObject(json, "file"))) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("JSON backing volume definition '%s' lacks 'file' object"),
-                       jsonstr);
-        return -1;
-    }
-
-    if (!(drvname = virJSONValueObjectGetString(file, "driver"))) {
+    if (!(drvname = virJSONValueObjectGetString(json, "driver"))) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("JSON backing volume definition '%s' lacks driver name"),
                        jsonstr);
@@ -3621,7 +3620,7 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src,

     for (i = 0; i < G_N_ELEMENTS(jsonParsers); i++) {
         if (STREQ(drvname, jsonParsers[i].drvname))
-            return jsonParsers[i].func(src, file, jsonstr, jsonParsers[i].opaque);
+            return jsonParsers[i].func(src, json, jsonstr, jsonParsers[i].opaque);
     }

     virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3637,6 +3636,7 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src,
 {
     g_autoptr(virJSONValue) root = NULL;
     g_autoptr(virJSONValue) deflattened = NULL;
+    virJSONValuePtr file = NULL;

     if (!(root = virJSONValueFromString(json)))
         return -1;
@@ -3644,7 +3644,18 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr src,
     if (!(deflattened = virJSONValueObjectDeflatten(root)))
         return -1;

-    return virStorageSourceParseBackingJSONInternal(src, deflattened, json);
+    /* There are 2 possible syntaxes:
+     * 1) json:{"file":{"driver":...}}
+     * 2) json:{"driver":...}
+     * Remove the 'file' wrapper object in case 1.
+     */
+    if (!virJSONValueObjectHasKey(deflattened, "driver"))
+        file = virJSONValueObjectGetObject(deflattened, "file");
+
+    if (!file)
+        file = deflattened;
+
+    return virStorageSourceParseBackingJSONInternal(src, file, json);
 }


-- 
2.24.1

Re: [PATCH 04/15] virStorageSourceParseBackingJSON: Allow 'json:' pseudo URIs without 'file' wrapper
Posted by Ján Tomko 6 years ago
On Thu, Feb 06, 2020 at 08:51:56AM +0100, Peter Krempa wrote:
>There are two possibilities:
>1) json:{"file":{"driver":...}}
>2) json:{"driver":...}
>
>Our code didn't work properly with the second one as it was expecting
>the 'file' wrapper. Conditionalize the removal to only the situation
>when the top level doesn't have "driver".
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/util/virstoragefile.c | 41 +++++++++++++++++++++++++--------------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>

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

Jano