Bug 1139817 - Storage Domain version 0 created when version 3 is requested
Summary: Storage Domain version 0 created when version 3 is requested
Alias: None
Product: oVirt
Classification: Retired
Component: ovirt-engine-core
Version: 3.5
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
: 3.5.0
Assignee: Piotr Kliczewski
QA Contact: Jiri Belka
Whiteboard: infra
Depends On:
Blocks: 1073943 1139401
TreeView+ depends on / blocked
Reported: 2014-09-09 17:25 UTC by Adam Litke
Modified: 2016-02-10 19:34 UTC (History)
11 users (show)

Fixed In Version: ovirt-3.5.0_rc2
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2014-10-17 12:22:55 UTC
oVirt Team: Infra

Attachments (Terms of Use)
vdsm log during domain create (9.65 MB, text/plain)
2014-09-09 17:25 UTC, Adam Litke
no flags Details
engine.log during create (70.71 KB, text/plain)
2014-09-09 17:25 UTC, Adam Litke
no flags Details

System ID Priority Status Summary Last Updated
oVirt gerrit 32739 master MERGED core: optional parameter storageFormatType not sent to vdsm Never
oVirt gerrit 32750 None NEW core: optional parameter storageFormatType not sent to vdsm Never

Description Adam Litke 2014-09-09 17:25:13 UTC
Created attachment 935844 [details]
vdsm log during domain create

Description of problem:
When creating a new storage domain in a 3.5 datacenter, the only allowable version is 3.  However, when creating the domain, 0 is passed to vdsm and an old format storage domain is created.  This causes many errors (such is invalid handling of snapshot file permissions and InquireNotSupported exceptions).

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

How reproducible: always

Steps to Reproduce:
1. Create a NFS storage domain in a 3.5 datacenter.  Version 3 is selected and unchangeable.
2. On the SPM host, examine the metadata (/rhev/data-center/<poolID>/<domID>/dom_md).

Actual results:
The metadata contains VERSION=0

Expected results:
The metadata contains VERSION=3

Additional info:

Comment 1 Adam Litke 2014-09-09 17:25:40 UTC
Created attachment 935845 [details]
engine.log during create

Comment 2 Nir Soffer 2014-09-09 17:32:20 UTC
Sounds like a trivial fix.

Comment 3 Nir Soffer 2014-09-09 17:41:23 UTC
This happen when using jsonrpc.

Thread-3406::DEBUG::2014-09-09 11:32:59,067::__init__::467::jsonrpc.JsonRpcServer::(_serveRequest) Calling 'StorageDomain.create' in bridge with {u'domainClass': 1, u'typeArgs': u'', u'name': u'data2-nfs', u'domainType': 1, u'storagedomainID': u'85442237-be11-4bd0-b6c3-43ba60c1b87f'}
Thread-3406::DEBUG::2014-09-09 11:32:59,068::task::595::Storage.TaskManager.Task::(_updateState) Task=`0f9733c1-a367-4968-b853-de77457f5356`::moving from state init -> state preparing
Thread-3406::INFO::2014-09-09 11:32:59,068::logUtils::44::dispatcher::(wrapper) Run and protect: createStorageDomain(storageType=1, sdUUID=u'85442237-be11-4bd0-b6c3-43ba60c1b87f', domainName=u'data2-nfs', typeSpecificArg=u'', domClass=1, domVersion=0, options=None)

Piotr, can you take a look at this?

Comment 4 Piotr Kliczewski 2014-09-09 20:05:50 UTC
I checked JsonRpcVdsServer and it seems that jsonrpc code gets this value from parameters provided to CreateStorageDomainVDSCommand.

Have you checked how it works with xmlrpc?

Comment 5 Adam Litke 2014-09-09 20:31:01 UTC
Yes, xmlrpc works fine as documented and proven in https://bugzilla.redhat.com/show_bug.cgi?id=1139401

Comment 6 Allon Mureinik 2014-09-09 20:53:58 UTC
Looking at the engine side:

    public StatusOnlyReturnForXmlRpc createStorageDomain(int domainType,
            String sdUUID,
            String domainName,
            String arg,
            int storageType,
            String storageFormatType) {
        // storageFormatType not used and it can be removed from interface
        JsonRpcRequest request =
                new RequestBuilder("StorageDomain.create").withParameter("storagedomainID", sdUUID)
                        .withParameter("domainType", domainType)
                        .withParameter("typeArgs", arg)
                        .withParameter("name", domainName)
                        .withParameter("domainClass", storageType)
        Map<String, Object> response = new FutureMap(this.client, request);
        return new StatusOnlyReturnForXmlRpc(response);

The comment is clearly wrong, and volFormat is clearly required.

In API.py:

   def create(self, size, volFormat, preallocate, diskType, desc,
               srcImgUUID, srcVolUUID):
        return self._irs.createVolume(self._sdUUID, self._spUUID,
                                      self._imgUUID, size, volFormat,
                                      preallocate, diskType, self._UUID, desc,
                                      srcImgUUID, srcVolUUID)

However, it seems to be missing from the shcema:

{'command': {'class': 'StorageDomain', 'name': 'create'},
 'data': {'storagedomainID': 'UUID', 'domainType': 'StorageDomainType',
          'typeArgs': 'StorageDomainCreateArguments', 'name': 'str',
          'domainClass': 'StorageDomainImageClass', '*version': 'int'}}

IIUC, it should be added to the schema and the engine calling code.

Since this works properly in XMLRPC, moving to infra.

Comment 7 Nir Soffer 2014-09-09 21:43:08 UTC
(In reply to Allon Mureinik from comment #6)
This is the correct create method from StorageDomain:

    def create(self, type, typeArgs, name, domainClass, version=None):
        if version is None:
            version = constants.SUPPORTED_DOMAIN_VERSIONS[0]
        return self._irs.createStorageDomain(type, self._UUID, name, typeArgs,
                                             domainClass, version)

And it *exists* in the schema - '*version': 'int'

So this seems to be a missing parameter in the Engine side when using

Comment 8 Jiri Belka 2014-09-17 07:54:30 UTC
ok, ovirt-engine-backend-3.5.0-0.0.master.20140911085455.gite1c5ffd.el6.noarch

# grep ^VER /mnt/testovic/ad331fa7-258b-4ef7-b37b-e99202191ad0/dom_md/metadata 

Comment 9 Nir Soffer 2014-09-17 09:27:56 UTC
(In reply to Jiri Belka from comment #8)
Jiri, just to be sure - the host you were testing was using jsonrpc - right?

Comment 10 Jiri Belka 2014-09-17 09:53:52 UTC
Yes, it's checked by default, so I suppose it uses that :)

Comment 11 Sandro Bonazzola 2014-10-17 12:22:55 UTC
oVirt 3.5 has been released and should include the fix for this issue.

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