Tomas Jelinek has posted comments on this change. Change subject: restapi: added support for io_threads ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/40094/4/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 3259: <xs:sequence> Line 3260: <xs:element name="type" type="xs:string" minOccurs="0"/> Line 3261: <xs:element ref="status" minOccurs="0" maxOccurs="1"/> Line 3262: <xs:element name="memory" type="xs:long" minOccurs="0"/> Line 3263: <xs:element name="num_of_iothreads" type="xs:int" minOccurs="0" maxOccurs="1"/> > Currently the RESTAPI doesn't use prefixes like "num_of_" or "amount_of_", you are right, in future some thread pinning or some other tuning could be added - OK, will change. Line 3264: <xs:element name="cpu" type="CPU" minOccurs="0"/> Line 3265: <xs:element name="cpu_shares" type="xs:int" minOccurs="0" maxOccurs="1"/> Line 3266: <xs:element name="bios" type="Bios" minOccurs="0" maxOccurs="1" /> Line 3267: <xs:element name="os" type="OperatingSystem" minOccurs="0"/> https://gerrit.ovirt.org/#/c/40094/4/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmBaseMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmBaseMapper.java: Line 182: model.setMemory((long) entity.getMemSizeMb() * BYTES_PER_MB); Line 183: Line 184: if (entity.getNumOfIoThreads() != 0) { Line 185: model.setNumOfIothreads(entity.getNumOfIoThreads()); Line 186: } > It would be better if the backend returned "null" when there is no value. but it is a small "int", not an object. It makes a bit easier the DB layer which can be non-null and also the backend since it is null safe. I could remove the if here so the user will see "<iothereds>0</.." which is actually also correct because there are really be 0 IO Threads. Line 187: Line 188: if (entity.getVdsGroupId() != null) { Line 189: Cluster cluster = new Cluster(); Line 190: cluster.setId(entity.getVdsGroupId().toString()); https://gerrit.ovirt.org/#/c/40094/4/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java: Line 168: staticVm.setMigrationDowntime(entity.getMigrationDowntime()); Line 169: staticVm.setMinAllocatedMem(entity.getMinAllocatedMem()); Line 170: if (FeatureSupported.isIoThreadsSupported(clusterVersion)) { Line 171: staticVm.setNumOfIoThreads(entity.getNumOfIoThreads()); Line 172: } > Shouldn't the backend return "null" for this if the cluster doesn't support this is when the IO Threads are copied from an instance type / blank template to the target VM, where the IO Threads may be enabled on the source (since instance types and blank template are above cluster so everything is enabled there) and not on target cluster. The same approach is with e.g. serialNumberPolicy() in "map(VmTemplate entity, VmStatic template, Version version) {" in this class. Line 173: return staticVm; Line 174: } Line 175: Line 176: @Mapping(from = VM.class, to = VmStatic.class) -- To view, visit https://gerrit.ovirt.org/40094 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e6f9b76d424027ef21e17cc3b3c9ffd7023b52d Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
