Moti Asayag has posted comments on this change.

Change subject: core: Adds custom properties to VmDevice
......................................................................


Patch Set 12: (1 inline comment)

....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDeviceDAODbFacadeImpl.java
Line 50:                 .addValue("is_managed", entity.getIsManaged())
Line 51:                 .addValue("is_plugged", entity.getIsPlugged())
Line 52:                 .addValue("is_readonly", entity.getIsReadOnly())
Line 53:                 .addValue("alias", entity.getAlias())
Line 54:                 .addValue("custom_properties", 
entity.getCustomProperties());
The assumption here is that the implementation class of the Map (LinkedHashMap 
in this case) will take care of the serialization.

However, when reading the string from the DB and converting it into string, the 
parsing is done manually.

I think we should use the same semantics as specParams uses (line 46) to store 
and to read (line 146).

In that way we won't rely on LinkedHashMap expected format if changes.
Line 55:     }
Line 56: 
Line 57:     @Override
Line 58:     protected RowMapper<VmDevice> createEntityRowMapper() {


--
To view, visit http://gerrit.ovirt.org/14814
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I07daa5cccbf8e3cbd6b4191bbc8ebf0729d9d9a0
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to