On Thu, Aug 31, 2017 at 08:25:05 -0400, John Ferlan wrote:
>
>
> 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@xxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxx>
> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> >> ---
> >>
> >> 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.
It makes sense both ways. I'd slightly prefer if the test and the change
to the backing store parser were separate.
Also as I've pointed out in a different thread, the object name should
be actually parsed and remembered, so that it can be pre-generated and
the generator code does not have to be specifically aware of disks.
(this becomes necessary, as with full backing chain tracking there won't
be only the top level to have secret objects for, but also any arbitrary
layer below will need to have a secret object, which will be referenced
by node name, rather than the top layer disk name)
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
