Bug 1214346 - getStorageDomainInfo does not return the value promised in the schema
Summary: getStorageDomainInfo does not return the value promised in the schema
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: vdsm
Classification: oVirt
Component: General
Version: ---
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: ovirt-4.1.0-beta
: ---
Assignee: Daniel Erez
QA Contact: Elad
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-04-22 13:45 UTC by Nir Soffer
Modified: 2017-02-15 14:47 UTC (History)
15 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-02-15 14:47:28 UTC
oVirt Team: Storage
Embargoed:
rule-engine: ovirt-4.1+
rule-engine: planning_ack+
rule-engine: devel_ack+
ratamir: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 69228 0 master MERGED vdsm-api: StorageDomainInfo - rename domainType to type 2020-08-29 12:08:16 UTC
oVirt gerrit 69354 0 master MERGED vdsm-api: StorageDomainInfo - rename domainClass to class 2020-08-29 12:08:16 UTC
oVirt gerrit 69355 0 master ABANDONED rpc: bridge - removing domainClass fixup 2020-08-29 12:08:17 UTC
oVirt gerrit 70412 0 ovirt-4.1 MERGED vdsm-api: StorageDomainInfo - rename domainClass to class 2020-08-29 12:08:17 UTC
oVirt gerrit 70413 0 ovirt-4.1 MERGED vdsm-api: StorageDomainInfo - rename domainType to type 2020-08-29 12:08:16 UTC

Description Nir Soffer 2015-04-22 13:45:10 UTC
Description of problem:

getStorageDomainInfo returns the key "type" instead of the documented "domainType" key.

Version-Release number of selected component (if applicable):
all

How reproducible:
always

Steps to Reproduce:
1. Invoke using vdsClient -s 0 getStorageDomainInfo
   # vdsClient -s 0 getStorageDomainInfo 661f1926-04cc-405a-85eb-802563b48ed3

Actual results:

	type = ISCSI

Expected results:

        domainType = ISCSI

Additional info:

The schema promise a "domainType" key:

##
# @StorageDomainInfo:
#
# Information about a Storage Domain.
#
# @uuid:         The Storage Domain UUID 
#
# @domainType:   The type of backing storage used by this domain
#
# @domainClass:  The Storage Domain class
#
# @name:         The human-readable name for this Storage Domain
#
# @role:         The Storage Domain role 
#
# @pool:         The Storage Pool associated with this Storage Domain
#
# @version:      The version of this Storage Domain
#
# @lver:         The lock version of the associated Storage Pool 
#
# @spm_id:       Contains the Host ID of the Storage Pool Manager
#
# @master_ver:   The version of the master Storage Domain
#
# Since: 4.10.0
##
{'type': 'StorageDomainInfo',
 'data': {'uuid': 'UUID', 'domainType': 'StorageDomainType',
          'domainClass': 'StorageDomainImageClass', 'name': 'str',
          'role': 'StorageDomainRole', 'pool': ['UUID'], 'version': 'int',
          'lver': 'int', 'spm_id': 'int', 'master_ver': 'int'}}

How to fix:

Since engine depends on this behavior, and vdsm must be compatible with 
older engines, we must fix the schema to fit the existing code.

Also, "type" is much better then "domainType". There is no reason why
vdsm result should not include the word "type". This was added to make
it easier to create vdsm bindings for languages where type is a reserved
keyword. But this should be handled by the binding, and not make everyone
else life harder by modifying the strings vdsm returns.

Comment 1 Nir Soffer 2015-04-22 13:56:47 UTC
Adam, what is you view on this?

Comment 2 Adam Litke 2015-04-23 18:09:33 UTC
type is a terrible variable name (as is state, status, mode, class, role, etc).  We introduced the schema change and appropriate fixup code in Bridge.py to move away from these bad things that cannot be changed in XMLRPC due to BC requirements.  If this code is breaking somewhere I think we should fix the jsonrpc bridge and not revert back to bad names.

Comment 3 Nir Soffer 2015-04-23 18:51:05 UTC
(In reply to Adam Litke from comment #2)
> type is a terrible variable name (as is state, status, mode, class, role,
> etc).  We introduced the schema change and appropriate fixup code in
> Bridge.py to move away from these bad things that cannot be changed in
> XMLRPC due to BC requirements.  If this code is breaking somewhere I think
> we should fix the jsonrpc bridge and not revert back to bad names.

domain.type is the best name for the type of a domain. domain.domainType
a repeating "domain".

This is true also for state, status, mode, class and role :-)

I think the correct solution is removing the magical fixup code in the
bridge and use the short and nicer names.

Comment 4 Allon Mureinik 2015-08-12 15:16:50 UTC
This isn't the rest api, but vdsm's api.

Comment 5 Red Hat Bugzilla Rules Engine 2015-10-19 10:54:45 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 6 Sandro Bonazzola 2015-10-26 12:28:25 UTC
this is an automated message. oVirt 3.6.0 RC3 has been released and GA is targeted to next week, Nov 4th 2015.
Please review this bug and if not a blocker, please postpone to a later release.
All bugs not postponed on GA release will be automatically re-targeted to

- 3.6.1 if severity >= high
- 4.0 if severity < high

Comment 7 Sandro Bonazzola 2016-05-02 09:47:02 UTC
Moving from 4.0 alpha to 4.0 beta since 4.0 alpha has been already released and bug is not ON_QA.

Comment 8 Yaniv Lavi 2016-05-23 13:12:19 UTC
oVirt 4.0 beta has been released, moving to RC milestone.

Comment 9 Nir Soffer 2016-12-29 21:06:21 UTC
Same error for the "domainClass", the actual value is "class":

# vdsm-client StorageDomain getInfo \
    storagedomainID=7df95b16-1bd3-4c23-bbbe-b21d403bdcd8
{
    "uuid": "7df95b16-1bd3-4c23-bbbe-b21d403bdcd8", 
    "type": "ISCSI", 
    "vguuid": "yQyqnB-Y7x6-EGjl-WrFJ-fc9O-Oa4x-ShsdLa", 
    "metadataDevice": "360014052c81462a280847e8a6e3af8cd", 
    "state": "OK", 
    "version": "4", 
    "role": "Regular", 
    "vgMetadataDevice": "360014052c81462a280847e8a6e3af8cd", 
    "class": "Data", 
    "pool": [
        "ec544277-c00c-46ea-b330-7daac6d5c09d"
    ], 
    "name": "dumbo-iscsi-01"
}

The schema for this response:

StorageDomain.getInfo:
    added: '3.1' 
    description: Get information about a Storage Domain.
    params:
    -   description: The UUID of the Storage Domain
        name: storagedomainID
        type: *UUID 
    return:
        description: Storage domain information
        type: *StorageDomainInfo

    StorageDomainInfo: &StorageDomainInfo
        added: '3.1'
        description: Information about a Storage Domain.
        name: StorageDomainInfo
        properties:
        -   description: The Storage Domain role
            name: role
            type: *StorageDomainRole

        -   description: The lock version of the associated Storage Pool
            name: lver
            type: int

        -   description: The version of this Storage Domain
            name: version
            type: int

        -   description: The type of backing storage used by this domain
            name: domainType
            type: *StorageDomainType

        -   description: The human-readable name for this Storage Domain
            name: name
            type: string

        -   description: The Storage Pool associated with this Storage Domain
            name: pool
            type:
            - *UUID

        -   description: The version of the master Storage Domain
            name: master_ver
            type: int

        -   description: The Storage Domain class
            name: domainClass
            type: *StorageDomainImageClass

        -   description: Contains the Host ID of the Storage Pool Manager
            name: spm_id
            type: int

        -   description: The Storage Domain UUID
            name: uuid
            type: *UUID

        -   defaultvalue: null
            description: The GUID of the first device containing the domain
                metadata lv for block storage domains only
            name: metadataDevice
            type: string

        -   defaultvalue: null
            description: The GUID of the device containing the domain vg
                metadata for block storage domains (optional)
            name: vgMetadataDevice
            type: string
        type: object

So the fix should be for both "type" and "class".

Comment 10 Daniel Erez 2017-01-01 07:51:22 UTC
@Nir - addressed in consecutive patches.

Comment 11 Elad 2017-01-25 07:20:43 UTC
Using latest build [1], still getting 'type' and 'class' instead if 'domainType' and 'domainClass':


[root@storage-ge2-vdsm1 yum.repos.d]# vdsm-client StorageDomain getInfo storagedomainID=1eb49be0-bcc4-4f3b-94af-a10c669ad73a
{
    "uuid": "1eb49be0-bcc4-4f3b-94af-a10c669ad73a", 
    "type": "ISCSI", 
    "vguuid": "SKlPbo-YcD7-scWp-AHNe-QSFx-Ebqa-6mektL", 
    "metadataDevice": "3514f0c5a516008d5", 
    "state": "OK", 
    "version": "4", 
    "role": "Regular", 
    "vgMetadataDevice": "3514f0c5a516008d5", 
    "class": "Data", 
    "pool": [
        "fc52e0e4-5246-4f14-8269-a256c62608f9"
    ], 
    "name": "iscsi_1"
}



[1]
vdsm-4.19.2-2.el7ev.x86_64
vdsm-cli-4.19.2-2.el7ev.noarch
vdsm-jsonrpc-4.19.2-2.el7ev.noarch
vdsm-client-4.19.2-2.el7ev.noarch
vdsm-xmlrpc-4.19.2-2.el7ev.noarch
vdsm-yajsonrpc-4.19.2-2.el7ev.noarch
vdsm-python-4.19.2-2.el7ev.noarch
vdsm-api-4.19.2-2.el7ev.noarch

Comment 12 Daniel Erez 2017-01-25 07:48:37 UTC
Hi Elad,

I guess the description was a bit misleading.
The fix here is *only* in the vdsm API schema:
- from 'domainType' to 'type'
- from 'domainClass' to 'class'

You can check the schema html file at:
'/usr/share/doc/vdsm-api-*/vdsm-api.html' (StorageDomainInfo section).

Moving back to ON_QA

Comment 13 Elad 2017-01-25 09:01:39 UTC
Thanks Daniel, 

[root@storage-ge2-vdsm1 ~]# vim /usr/share/doc/vdsm-api-4.19.2/vdsm-api.html 

<a name="StorageDomainInfo" />

<b>StorageDomainInfo:</b><br/>
Information about a Storage Domain.<br/>


<tr><td class="attrlist">type</td><td class="attrlist"><a href="#StorageDomainType">StorageDomainType</a></td><td class="attrlist">The type of backing storage used by this domain</td></tr>

<tr><td class="attrlist">class</td><td class="attrlist"><a href="#StorageDomainImageClass">StorageDomainImageClass</a></td><td class="attrlist">The Storage Domain class</td></tr>


========================
Used:
vdsm-api-4.19.2-2.el7ev.noarch


Note You need to log in before you can comment on or make changes to this bug.