[libvirt] [PATCH] conf: fix title and description for virDomainSetMetadata API

Pavel Hrdina posted 1 patch 5 years ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/792b73ced18326c4af96ae83510ddd2830d74f18.1551279469.git.phrdina@redhat.com
src/conf/domain_conf.c |  6 +++---
tests/metadatatest.c   | 23 ++++++++++++++++-------
2 files changed, 19 insertions(+), 10 deletions(-)
[libvirt] [PATCH] conf: fix title and description for virDomainSetMetadata API
Posted by Pavel Hrdina 5 years ago
If we pass XML to virDomainDefineXML API with these two elements:

    ...
    <title></title>
    <description></description>
    ...

libvirt correctly ignores these two elements and they will not appear
in the parsed XML.

However, if we use virDomainSetMetadata API and with "" as value for
title or description we will end up with the parsed XML that contains
these empty elements.

Let's fix the behavior of this API to behave the same as
virDomainDefineXML.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1518042

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_conf.c |  6 +++---
 tests/metadatatest.c   | 23 ++++++++++++++++-------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 477deb777e..2fba58e5c4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30363,7 +30363,7 @@ virDomainDefSetMetadata(virDomainDefPtr def,
     xmlDocPtr doc = NULL;
     xmlNodePtr old;
     xmlNodePtr new = NULL;
-    char *tmp;
+    char *tmp = NULL;
     int ret = -1;
 
     if (type >= VIR_DOMAIN_METADATA_LAST) {
@@ -30374,7 +30374,7 @@ virDomainDefSetMetadata(virDomainDefPtr def,
 
     switch ((virDomainMetadataType) type) {
     case VIR_DOMAIN_METADATA_DESCRIPTION:
-        if (VIR_STRDUP(tmp, metadata) < 0)
+        if (STRNEQ_NULLABLE(metadata, "") && VIR_STRDUP(tmp, metadata) < 0)
             goto cleanup;
 
         VIR_FREE(def->description);
@@ -30382,7 +30382,7 @@ virDomainDefSetMetadata(virDomainDefPtr def,
         break;
 
     case VIR_DOMAIN_METADATA_TITLE:
-        if (VIR_STRDUP(tmp, metadata) < 0)
+        if (STRNEQ_NULLABLE(metadata, "") && VIR_STRDUP(tmp, metadata) < 0)
             goto cleanup;
 
         VIR_FREE(def->title);
diff --git a/tests/metadatatest.c b/tests/metadatatest.c
index 786c62e623..a9080b32d7 100644
--- a/tests/metadatatest.c
+++ b/tests/metadatatest.c
@@ -167,6 +167,7 @@ struct metadataTest {
     virDomainPtr dom;
 
     const char *data;
+    const char *expect;
     int type;
     bool fail;
 };
@@ -232,7 +233,7 @@ testTextMetadata(const void *data)
 
     actual = virDomainGetMetadata(test->dom, test->type, NULL, 0);
 
-    if (STRNEQ_NULLABLE(test->data, actual)) {
+    if (STRNEQ_NULLABLE(test->expect, actual)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "expected metadata doesn't match actual: "
                        "expected:'%s'\ngot: '%s'",
@@ -248,10 +249,11 @@ testTextMetadata(const void *data)
     return ret;
 }
 
-#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, FAIL) \
+#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, EXPECT, FAIL) \
     do { \
         test.type = VIR_DOMAIN_METADATA_ ## TYPE; \
         test.data = DATA; \
+        test.expect = EXPECT; \
         test.fail = FAIL; \
  \
         if (virTestRun("text metadata: " #TYPE " " INDEX " ", \
@@ -259,9 +261,16 @@ testTextMetadata(const void *data)
             ret = EXIT_FAILURE; \
     } while (0)
 
-#define TEST_TITLE(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, false)
-#define TEST_TITLE_FAIL(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, true)
-#define TEST_DESCR(INDEX, DATA) TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, false)
+#define TEST_TITLE(INDEX, DATA) \
+    TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, false)
+#define TEST_TITLE_EXPECT(INDEX, DATA, EXPECT) \
+    TEST_TEXT_METADATA(INDEX, TITLE, DATA, EXPECT, false)
+#define TEST_TITLE_FAIL(INDEX, DATA) \
+    TEST_TEXT_METADATA(INDEX, TITLE, DATA, DATA, true)
+#define TEST_DESCR(INDEX, DATA) \
+    TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, DATA, false)
+#define TEST_DESCR_EXPECT(INDEX, DATA, EXPECT) \
+    TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, EXPECT, false)
 
 static int
 mymain(void)
@@ -290,7 +299,7 @@ mymain(void)
     TEST_TITLE("2", NULL);
     TEST_TITLE("3", "blah");
     TEST_TITLE_FAIL("4", "qwe\nrt");
-    TEST_TITLE("5", "");
+    TEST_TITLE_EXPECT("5", "", NULL);
     TEST_TITLE_FAIL("6", "qwert\n");
     TEST_TITLE_FAIL("7", "\n");
 
@@ -298,7 +307,7 @@ mymain(void)
     TEST_DESCR("2", NULL);
     TEST_DESCR("3", "qwert");
     TEST_DESCR("4", "\n");
-    TEST_DESCR("5", "");
+    TEST_DESCR_EXPECT("5", "", NULL);
 
     virDomainFree(test.dom);
     virConnectClose(test.conn);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix title and description for virDomainSetMetadata API
Posted by John Ferlan 5 years ago

On 2/27/19 10:01 AM, Pavel Hrdina wrote:
> If we pass XML to virDomainDefineXML API with these two elements:
> 
>     ...
>     <title></title>
>     <description></description>
>     ...
> 
> libvirt correctly ignores these two elements and they will not appear
> in the parsed XML.
> 
> However, if we use virDomainSetMetadata API and with "" as value for
> title or description we will end up with the parsed XML that contains
> these empty elements.
> 
> Let's fix the behavior of this API to behave the same as
> virDomainDefineXML.

virDomainDefParseXML via interaction with virXPathString which ignores
empty strings.

> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1518042
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/domain_conf.c |  6 +++---
>  tests/metadatatest.c   | 23 ++++++++++++++++-------
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

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