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

Peter Krempa pkrempa at redhat.com
Thu Aug 31 12:48:39 UTC 2017


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 at veritas.com>
> >>
> >> 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 at veritas.com>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >>
> >>  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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170831/5228b950/attachment-0001.sig>


More information about the libvir-list mailing list