5:12 a.m.
See patch 1 for explanation.
Peter Krempa (2):
storage: Fix crash when parsing backing store URI with schema
test: Add test to verify helpers used for backing file name parsing
src/util/virstoragefile.c | 10 +++++-----
tests/virstoragetest.c | 31 +++++++++++++++++++++++++------
tests/virstringtest.c | 3 +++
tests/viruritest.c | 1 +
4 files changed, 34 insertions(+), 11 deletions(-)
--
2.1.0
5:12 a.m.
New subject: [libvirt] [PATCH 1/2] storage: Fix crash when parsing backing store URI with schema
The code that parses the schema from the URI touches the "hosts[0]"
member of the storage file source structure in case the URI contains a
schema. The hosts array was not yet allocated at the point in the code
where the transport protocol was parsed and set. This lead to a crash of
libvirtd.
Fix the code by allocating the "hosts" array upfront and add a test case
to verify this scenario. (Unfortunately this requires shuffling the test
case numbers too).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1156288
---
src/util/virstoragefile.c | 10 +++++-----
tests/virstoragetest.c | 31 +++++++++++++++++++++++++------
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 960aa23..8e9d115 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2133,6 +2133,11 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
goto cleanup;
}
+ if (VIR_ALLOC(src->hosts) < 0)
+ goto cleanup;
+
+ src->nhosts = 1;
+
if (!(scheme = virStringSplit(uri->scheme, "+", 2)))
goto cleanup;
@@ -2183,11 +2188,6 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
tmp[0] = '\0';
}
- if (VIR_ALLOC(src->hosts) < 0)
- goto cleanup;
-
- src->nhosts = 1;
-
if (uri->port > 0) {
if (virAsprintf(&src->hosts->port, "%d", uri->port) <
0)
goto cleanup;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 29f5c7a..05e48f3 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -835,6 +835,25 @@ mymain(void)
(&qcow2, &nbd), EXP_PASS,
(&qcow2, &nbd), ALLOW_PROBE | EXP_PASS);
+ /* Rewrite qcow2 to use an nbd: protocol as backend */
+ virCommandFree(cmd);
+ cmd = virCommandNewArgList(qemuimg, "rebase", "-u",
"-f", "qcow2",
+ "-F", "raw", "-b",
"nbd+tcp://example.org:6000/blah",
+ "qcow2", NULL);
+ if (virCommandRun(cmd, NULL) < 0)
+ ret = -1;
+ qcow2.expBackingStoreRaw = "nbd+tcp://example.org:6000/blah";
+
+ /* Qcow2 file with backing protocol instead of file */
+ testFileData nbd2 = {
+ .path = "blah",
+ .type = VIR_STORAGE_TYPE_NETWORK,
+ .format = VIR_STORAGE_FILE_RAW,
+ };
+ TEST_CHAIN(12, absqcow2, VIR_STORAGE_FILE_QCOW2,
+ (&qcow2, &nbd2), EXP_PASS,
+ (&qcow2, &nbd2), ALLOW_PROBE | EXP_PASS);
+
/* qed file */
testFileData qed = {
.expBackingStoreRaw = absraw,
@@ -848,7 +867,7 @@ mymain(void)
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
};
- TEST_CHAIN(12, absqed, VIR_STORAGE_FILE_AUTO,
+ TEST_CHAIN(13, absqed, VIR_STORAGE_FILE_AUTO,
(&qed_as_raw), EXP_PASS,
(&qed, &raw), ALLOW_PROBE | EXP_PASS);
@@ -858,10 +877,10 @@ mymain(void)
.type = VIR_STORAGE_TYPE_DIR,
.format = VIR_STORAGE_FILE_DIR,
};
- TEST_CHAIN(13, absdir, VIR_STORAGE_FILE_AUTO,
+ TEST_CHAIN(14, absdir, VIR_STORAGE_FILE_AUTO,
(&dir), EXP_PASS,
(&dir), ALLOW_PROBE | EXP_PASS);
- TEST_CHAIN(14, absdir, VIR_STORAGE_FILE_DIR,
+ TEST_CHAIN(15, absdir, VIR_STORAGE_FILE_DIR,
(&dir), EXP_PASS,
(&dir), ALLOW_PROBE | EXP_PASS);
@@ -900,7 +919,7 @@ mymain(void)
raw.path = datadir "/sub/../sub/../raw";
raw.pathRel = "../raw";
- TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2,
+ TEST_CHAIN(16, abslink2, VIR_STORAGE_FILE_QCOW2,
(&link2, &link1, &raw), EXP_PASS,
(&link2, &link1, &raw), ALLOW_PROBE | EXP_PASS);
#endif
@@ -914,7 +933,7 @@ mymain(void)
qcow2.expBackingStoreRaw = "qcow2";
/* Behavior of an infinite loop chain */
- TEST_CHAIN(16, absqcow2, VIR_STORAGE_FILE_QCOW2,
+ TEST_CHAIN(17, absqcow2, VIR_STORAGE_FILE_QCOW2,
(&qcow2), EXP_WARN,
(&qcow2), ALLOW_PROBE | EXP_WARN);
@@ -933,7 +952,7 @@ mymain(void)
qcow2.expBackingStoreRaw = "wrap";
/* Behavior of an infinite loop chain */
- TEST_CHAIN(17, abswrap, VIR_STORAGE_FILE_QCOW2,
+ TEST_CHAIN(18, abswrap, VIR_STORAGE_FILE_QCOW2,
(&wrap, &qcow2), EXP_WARN,
(&wrap, &qcow2), ALLOW_PROBE | EXP_WARN);
--
2.1.0
5:12 a.m.
New subject: [libvirt] [PATCH 2/2] test: Add test to verify helpers used for backing file name parsing
Add two test cases to verify that the helpers split and parse the
backing store components properly.
---
tests/virstringtest.c | 3 +++
tests/viruritest.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index 841531f..a0bfd61 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -597,6 +597,9 @@ mymain(void)
const char *tokens7[] = { "The", "quick", "brown",
"fox", "", NULL };
TEST_SPLIT("The quick brown fox ", " ", 0, tokens7);
+ const char *tokens8[] = { "gluster", "rdma", NULL };
+ TEST_SPLIT("gluster+rdma", "+", 2, tokens8);
+
if (virtTestRun("strdup", testStrdup, NULL) < 0)
ret = -1;
diff --git a/tests/viruritest.c b/tests/viruritest.c
index dbcd877..48b5865 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -173,6 +173,7 @@ mymain(void)
TEST_PARSE("test://127.0.0.1:123/system", "test",
"127.0.0.1", 123, "/system", NULL, NULL, NULL, NULL);
TEST_PARSE("test://[::1]:123/system", "test", "::1",
123, "/system", NULL, NULL, NULL, NULL);
TEST_PARSE("test://[2001:41c8:1:4fd4::2]:123/system", "test",
"2001:41c8:1:4fd4::2", 123, "/system", NULL, NULL, NULL, NULL);
+ TEST_PARSE("gluster+rdma://example.com:1234/gv0/vol.img",
"gluster+rdma", "example.com", 1234, "/gv0/vol.img", NULL,
NULL, NULL, NULL);
virURIParam params1[] = {
{ (char*)"foo", (char*)"one", false },
--
2.1.0
10:50 a.m.
On 10/29/2014 04:12 AM, Peter Krempa wrote:
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
See patch 1 for explanation.
Peter Krempa (2):
storage: Fix crash when parsing backing store URI with schema
test: Add test to verify helpers used for backing file name parsing
src/util/virstoragefile.c | 10 +++++-----
tests/virstoragetest.c | 31 +++++++++++++++++++++++++------
tests/virstringtest.c | 3 +++
tests/viruritest.c | 1 +
4 files changed, 34 insertions(+), 11 deletions(-)
ACK series
11:28 a.m.
On 10/29/14 16:50, Eric Blake wrote:
On 10/29/2014 04:12 AM, Peter Krempa wrote:
> See patch 1 for explanation.
>
> Peter Krempa (2):
> storage: Fix crash when parsing backing store URI with schema
> test: Add test to verify helpers used for backing file name parsing
>
> src/util/virstoragefile.c | 10 +++++-----
> tests/virstoragetest.c | 31 +++++++++++++++++++++++++------
> tests/virstringtest.c | 3 +++
> tests/viruritest.c | 1 +
> 4 files changed, 34 insertions(+), 11 deletions(-)
ACK series
Pushed; Thanks.
Peter
3677
days inactive
3677
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa