Martin Sivák has posted comments on this change. Change subject: Use str type for vcpuQuota to prevent xml-rpc errors ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/28960/1/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 6305: '*watchdogEvent': 'WatchdogEvent', 'guestFQDN': 'str', Line 6306: '*migrationProgress': 'uint', 'guestCPUCount': 'int', Line 6307: 'displayInfo': ['VmDisplayInfo'], '*vmJobs': 'VmJobsMap', Line 6308: '*vNodeRuntimeInfo': 'VmNumaNodeRuntimeInfoMap', Line 6309: 'displayInfo': ['VmDisplayInfo'], '*vcpuQuota': 'str', > it's quite horrifying, but when the schema says "int" it means that type is I see we use str incorrectly on more places then.. I reverted it to int (as it can be reported as -1 in some cases) Line 6310: '*vcpuPeriod': 'int', '*vcpuCount': 'int', '*vcpuUserLimit': 'int'}} Line 6311: Line 6312: ## Line 6313: # @VmStats: Line 7462: # Line 7463: # Since: 4.15.0 Line 7464: ## Line 7465: {'command': {'class': 'VM', 'name': 'setCpuTuneQuota'}, Line 7466: 'data': {'quota': 'str'}, > looks OK, I see Vm.setCpuTuneQuota has already an explicit int() conversion It does. Line 7467: 'returns': 'TasksStatus'} Line 7468: Line 7469: Line 7470: ## http://gerrit.ovirt.org/#/c/28960/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 392 Line 393 Line 394 Line 395 Line 396 > Needs to be fixed here too. Nope, not needed. The allowed values fit into int easily. The optional period element specifies the enforcement interval(unit: microseconds). Within period, each vcpu of the domain will not be allowed to consume more than quota worth of runtime. The value should be in range [1000, 1000000]. Line 386: # Handling the case where quota is not set, setting to 0. Line 387: # According to libvirt API:"A quota with value 0 means no value." Line 388: # The value does not have to be present in some transient cases Line 389: if eInfo.get('vcpu_quota', _NO_CPU_QUOTA) != _NO_CPU_QUOTA: Line 390: stats['vcpuQuota'] = str(eInfo['vcpu_quota']) > Minor nit: _NO_CPU_QUOTA is int() here. Should be harmless, however The return value from the stats is int as well, the conversion to str is the last step. Line 391: Line 392: # Handling the case where period is not set, setting to 0. Line 393: # According to libvirt API:"A period with value 0 means no value." Line 394: # The value does not have to be present in some transient cases -- To view, visit http://gerrit.ovirt.org/28960 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7c38759ee65e58530c03751ca12706396697166 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Sivák <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
