Project

General

Profile

Bug #10515

libtopo's XML serialization code is broken and incomplete

Added by Rob Johnston 8 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-03-09
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

libtopo supports the ability to create a serialized representation of a topo snapshot as XML. However, the serialization code is incomplete in that it there are a number of node property types that aren't supported. For example, libtopo supports the following types for node properties (excerpted from libtopo.h):

typedef enum {
    TOPO_TYPE_INVALID = 0,
    TOPO_TYPE_BOOLEAN,    /* boolean */
    TOPO_TYPE_INT32,    /* int32_t */
    TOPO_TYPE_UINT32,    /* uint32_t */
    TOPO_TYPE_INT64,    /* int64_t */
    TOPO_TYPE_UINT64,    /* uint64_t */
    TOPO_TYPE_STRING,    /* const char* */
    TOPO_TYPE_TIME,        /* uint64_t */
    TOPO_TYPE_SIZE,        /* uint64_t */
    TOPO_TYPE_FMRI,        /* nvlist_t */
    TOPO_TYPE_INT32_ARRAY,    /* array of int32_t */
    TOPO_TYPE_UINT32_ARRAY,    /* array of uint32_t */
    TOPO_TYPE_INT64_ARRAY,    /* array of int64_t */
    TOPO_TYPE_UINT64_ARRAY,    /* array of uint64_t */
    TOPO_TYPE_STRING_ARRAY,    /* array of const char* */
    TOPO_TYPE_FMRI_ARRAY,    /* array of nvlist_t */
    TOPO_TYPE_DOUBLE    /* double */
} topo_type_t;

But the serialization code does not currently implement code to handle the following types (see txml_print_prop() in usr/src/lib/fm/topo/libtopo/common/topo_2xml.c):

    TOPO_TYPE_BOOLEAN,    /* boolean */
    TOPO_TYPE_TIME,        /* uint64_t */
    TOPO_TYPE_SIZE,        /* uint64_t */
    TOPO_TYPE_INT32_ARRAY,    /* array of int32_t */
    TOPO_TYPE_INT64_ARRAY,    /* array of int64_t */
    TOPO_TYPE_UINT64_ARRAY,    /* array of uint64_t */
    TOPO_TYPE_STRING_ARRAY,    /* array of const char* */
    TOPO_TYPE_FMRI_ARRAY,    /* array of nvlist_t */
    TOPO_TYPE_DOUBLE    /* double */

Note that the TOPO_TYPE_BOOLEAN, TOPO_TYPE_SIZE and TOPO_TYPE_TIME property types are defined in libtopo.h, but there is no support for them in the topo_prop_get|set code paths and there are no consumers of these types. Furthermore, the DTD for the XML representation of a topo snapshot does not allow for these types. I'm not sure what the original intention was for them. Anyways, we won't bother adding any special handling for these three types as part of this fix.

Another significant issue with the current serialization code. It does not serialize any of the array property types correctly. Currently, array property values are serialized by placing the values of all of the array elements in the "value" attribute of the "propval" element, delimited by spaces - for example:

<propval name='assigned-addresses' type='uint32_array' value='0x83020010 0x0 0xfc000000 0x0 0x20000 0x83020018 0x0 0xfc020000 0x0 0x10000 0x81020020 0x0 0x1040 0x0 0x40' />

But this is not what libtopo's XML parser expects. The DTD specifies that the value for each array element should be specified in a "propitem" element that is a child of the "propval" element. For example:

         <propval name='entity-list' type='string_array' >
           <propitem value='Intrusion' />
           <propitem value='Pwr Consumption' />
           <propitem value='Inlet Temp' />
           <propitem value='Exhaust Temp' />
         </propval>

One more piece of brokenness. The serialization code always sets the "static" attribute to "true" for "node" elements. This has the effect of causing the parser to not create the node, if it doesn't exist. The static attribute is primarily there for when we want to use XML to override a property value on a topo node that was already created by an enumerator module. But in this case we're trying to serialize the whole topology in a fashion such that we could reconstitute it from the generated XML. In which case, we relly need it to create all the nodes becuase no enumerator modules will be running.

History

#1

Updated by Rob Johnston 8 months ago

Note that this issue has already been fixed in illumos-joyent via the following commit:

commit f2b29c149057da13a23c7c86492e7baf539e4b7f
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   Sat Feb 16 05:03:54 2019 +0000

    OS-7560 libtopo's XML serialization code is broken and incomplete
    OS-7609 fmtopo is missing code to handle properties of type TOPO_TYPE_FMRI_ARRAY
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Approved by: Jordan Hendricks <jordan.hendricks@joyent.com>

So this issue is simply to track getting the abve change pushed upstream to illumos-gate.

Testing

See the detailed testing notes in the corresponding SmartOS ticket:

https://smartos.org/bugview/OS-7560

#2

Updated by Electric Monk 7 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ebee07ff4f102cbd3179db7c5070283da35a79f3

commit  ebee07ff4f102cbd3179db7c5070283da35a79f3
Author: Rob Johnston <rob.johnston@joyent.com>
Date:   2019-03-13T20:38:43.000Z

    10515 libtopo's XML serialization code is broken and incomplete
    10516 fmtopo is missing code to handle properties of type TOPO_TYPE_FMRI_ARRAY
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF