Martin Sivák has posted comments on this change. Change subject: Separate MOM to an external process ......................................................................
Patch Set 8: (7 comments) https://gerrit.ovirt.org/#/c/41602/8/init/systemd/mom-vdsm.service.in File init/systemd/mom-vdsm.service.in: Line 6: Line 7: [Service] Line 8: Type=simple Line 9: LimitCORE=infinity Line 10: ExecStart=/usr/sbin/momd -c /etc/vdsm/mom.conf > consider to use the daemonAdapter that being used for vdsmd and supervdsmd By default stdout and stderr of a systemd unit are sent to syslog. But there is no real output except in a case of crash (and it ends up in journal in that case as it should.) Line 11: Restart=on-failure Line 12: RestartSec=10 Line 13: User=@VDSMUSER@ Line 14: Group=@VDSMGROUP@ Line 10: ExecStart=/usr/sbin/momd -c /etc/vdsm/mom.conf Line 11: Restart=on-failure Line 12: RestartSec=10 Line 13: User=@VDSMUSER@ Line 14: Group=@VDSMGROUP@ > why does it run as vdsm user? it contains all its sudoer permissions.. bett There is no special user for mom and it has always had those permissions. MOM itself is usually started under root so it can talk to KSM and libvirt. Line 15: TimeoutStopSec=@SERVICE_STOP_TIMEOUT@ Line 16: Line 17: [Install] https://gerrit.ovirt.org/#/c/41602/8/init/systemd/vdsmd.service.in File init/systemd/vdsmd.service.in: Line 4: iscsid.service rpcbind.service supervdsmd.service sanlock.service \ Line 5: vdsm-network.service Line 6: After=multipathd.service libvirtd.service iscsid.service rpcbind.service \ Line 7: supervdsmd.service sanlock.service vdsm-network.service Line 8: Conflicts=libvirt-guests.service ksmtuned.service > now we can remove the ksmtuned conflict from here? Yep, we could probably do that. Line 9: Wants=mom-vdsm.service Line 10: Line 11: [Service] Line 12: Type=simple Line 5: vdsm-network.service Line 6: After=multipathd.service libvirtd.service iscsid.service rpcbind.service \ Line 7: supervdsmd.service sanlock.service vdsm-network.service Line 8: Conflicts=libvirt-guests.service ksmtuned.service Line 9: Wants=mom-vdsm.service > why not in requires? Because Requires means VDSM won't be started when MOM crashed. The idea is what VDSM can run without MOM if needed. Line 10: Line 11: [Service] Line 12: Type=simple Line 13: LimitCORE=infinity https://gerrit.ovirt.org/#/c/41602/8/vdsm/momIF.py File vdsm/momIF.py: Line 35: class MomNotAvailableError(RuntimeError): Line 36: pass Line 37: Line 38: Line 39: class MomThread(object): > MomServer could be better name for it.. Yep, we can rename it. Line 40: Line 41: def __init__(self, momconf): Line 42: if not _momAvailable: Line 43: raise MomNotAvailableError() Line 51: if port == "-1": Line 52: self.log.error("MOM's RPC interface is disabled") Line 53: raise MomNotAvailableError() Line 54: Line 55: self._mom = xmlrpclib.ServerProxy("http://localhost:"+port) > souldn't you have some kind of self._mom.stop somewhere? Why? Server proxy does not start any thread and does not hold any socket references. It is absolutely stateless and makes new requests for each call. Line 56: self._policy = {} Line 57: Line 58: def getKsmStats(self): Line 59: """ Line 70: ret['memShared'] = stats['ksm_pages_sharing'] * PAGE_SIZE_BYTES Line 71: ret['memShared'] /= Mbytes Line 72: ret['ksmCpu'] = stats['ksmd_cpu_usage'] Line 73: except (AttributeError, socket.error): Line 74: self.log.warning("MOM not available, KSM stats will be missing.") > better to print traceback to know what attribute is missing AttributeError is often reported by XML-RPC when the other side does not report proper data. The missing attribute I saw was something internal and not really helpful. Line 75: Line 76: return ret Line 77: Line 78: def setPolicy(self, policyStr): -- To view, visit https://gerrit.ovirt.org/41602 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99b81d21080834b7031824dab0a26f45e7eac7af Gerrit-PatchSet: 8 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: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
