Saggi Mizrahi has posted comments on this change.
Change subject: [WIP] vdsm API and libvdsm ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(9 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 18: import json Line 19: Line 20: import struct Line 21: Line 22: Size = struct.Struct("!Q") change to _Size, we don't want people using this outside this module. Line 23: Line 24: __log__ = None Line 25: __bridge__ = None Line 26:
Line 24: __log__ = None Line 25: __bridge__ = None Line 26: Line 27: class BindingJsonRpc: Line 28: def __init__(self, bridge, log, ip, port): Use default args if they are actually optional Line 29: self.bridge = bridge Line 30: self.log = log Line 31: self.serverPort = port Line 32: if not ip:
Line 71: msg = json.loads(self.request.recv(msgLen)) Line 72: except: Line 73: return Line 74: Line 75: log.debug("{} wrote:".format(self.client_address[0])) I think we can consolidate this to a single log line. I also kind of doubt how much we really care about logging this. Line 76: log.debug(msg) Line 77: Line 78: try: Line 79: ret = bridge._dispatch(msg['methodName'], msg.get('args', {}))
.................................................... File vdsm_api/generate.py Line 782: f.write(cgen(''' Line 783: /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ Line 784: Line 785: /* Line 786: * Copyright (C) 2012 Adam Litke, IBM Corporation Maybe the year are should be dynamic? Can autogenerated code be copyrighted? Line 787: * Line 788: * This program is free software; you can redistribute it and/or modify Line 789: * it under the terms of the GNU General Public License as published by Line 790: * the Free Software Foundation; either version 2 of the License, or
.................................................... File vdsm_api/Makefile.am Line 54: --target-glib=2.32 \ Line 55: --pkg gio-2.0 \ Line 56: --pkg json-glib-1.0 \ Line 57: --pkg gee-0.8 \ Line 58: -g \ Again, -g is for debug symbols. Should only be active in debug builds Line 59: -X -fPIC -X -shared \ Line 60: --library=libvdsm \ Line 61: --gir=vdsm-0.1.gir \ Line 62: -o libvdsm.so \
.................................................... File vdsm/config.py.in Line 246: ('rest_port', '8088', Line 247: 'Port on which the vdsmd REST server listens to network ' Line 248: 'clients.'), Line 249: Line 250: ('json_port', '4444', according to /etc/services 4444 is reserved
krb524 4444/tcp # Kerberos 5 to 4 ticket xlator krb524 4444/udp # Kerberos 5 to 4 ticket xlator Line 251: 'Port on which the vdsmd Json RPC server listens to network ' Line 252: 'clients.'), Line 253: Line 254: ('management_ip', '', None),
.................................................... File vdsm.spec.in Line 134: Line 135: Requires: %{name}-python = %{version}-%{release} Line 136: Line 137: %description jsonrpc Line 138: A Json-based RPC interface that serves as the transport for libvdsm. Protocol not transport Line 139: Line 140: %package bootstrap Line 141: Summary: VDSM bootstrapping package Line 142: BuildArch: noarch
Line 182: %package api Line 183: Summary: VDSM API Line 184: Line 185: %description api Line 186: Library and utilities for clients to use the vdsm API gobject-api
To prevent collision with java-api later on Line 187: Line 188: %package hook-vhostmd Line 189: Summary: VDSM hook set for interaction with vhostmd Line 190: Requires: vhostmd
Line 753: %{_libdir}/libvdsm-0.1.a Line 754: %{_libdir}/libvdsm-0.1.la Line 755: %{_libdir}/libvdsm-0.1.so Line 756: %{_libdir}/libvdsm-0.1.so.0 Line 757: %{_libdir}/libvdsm-0.1.so.0.0.0 Library versions are customarily variables so they can be easily modified. Line 758: Line 759: %files hook-vhostmd Line 760: %defattr(-, root, root, -) Line 761: %doc COPYING
-- To view, visit http://gerrit.ovirt.org/7516 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: If6bd34700b86aa84c7e289f02c0e9f2ac6fcba63 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server