Re: [libvirt] [PATCH v6 09/13] util: Add virstoragetest to parse/format a tls='yes'

On 08/31/2017 07:56 AM, Peter Krempa wrote: > On Wed, Aug 30, 2017 at 18:46:09 -0400, John Ferlan wrote: >> From: Ashish Mittal <Ashish.Mittal(a)veritas.com&gt; >> >> Add a test case to verify TLS arguments are parsed correctly for a VxHS disk >> >> Test case verifies that XML is generated correctly for a VxHS disk >> having TLS enabled. >> >> Signed-off-by: Ashish Mittal <Ashish.Mittal(a)veritas.com&gt; >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> >> This is essentially the v5 patch7 with a couple of minor adjustments >> (port == 9999 and "type":"tcp" added). >> >> tests/virstoragetest.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c >> index ffebd4d..f437de4 100644 >> --- a/tests/virstoragetest.c >> +++ b/tests/virstoragetest.c >> @@ -1603,6 +1603,18 @@ mymain(void) >> "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" >> " <host name='example.com' port='9999'/>\n" >> "</source>\n"); >> + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\"," >> + "\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\"," >> + "\"server\": { \"type\":\"tcp\"," >> + "\"host\":\"example.com\"," >> + "\"port\":\"9999\"" >> + "}," >> + "\"tls\":\"yes\"" > > There is no 'tls' property in the QAPI schema for VXHS [1] drives, so this > test is bogus. > Yeah - I was wondering about this too. I just forgot to note it while working through things. Of course the problem is in the previous patch... Where "tls" is incorrectly handled in virStorageSourceParseBackingJSONVxHS. It should have been: const char *haveTLS = virJSONValueObjectGetString(json, "tls-creds"); and then if (haveTLS) src->haveTLS = VIR_TRISTATE_BOOL_YES; Making this patch alter the "tls":"yes" above to be: "\"tls-creds\":\"objvirtio-disk0_tls0\"" Whether this patch combines with the previous one or the previous one doesn't make the virStorageSourceParseBackingJSONVxHS change and this patch does is a matter of taste I suppose. I think combining them by this point in time would be fine, but I can separate too.