Francesco Romani has posted comments on this change. Change subject: API: modernize VM methods ......................................................................
Patch Set 18: (6 comments) The problem is that I don't know how the meaningfully split this patch without using one patch per method, which is unmanageable as well. https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py File vdsm/API.py: Line 346 Line 347 Line 348 Line 349 Line 350 > and since the hooks are typically quite stupid it may cause a problem indee Right, I missed this. Will fix. Line 125: Line 126: @property Line 127: def vm(self): Line 128: v = self._cif.vmContainer.get(self._UUID) Line 129: if not v: > isn't it the opposite? Yes, it could be if v is None not sure what's more readable, we can't have a falsey VM. Line 130: raise exception.NoSuchVM() Line 131: return v Line 132: Line 133: @api.method Line 139: :type vmId: UUID Line 140: :param driveSpec: specification of the new CD image. Either an Line 141: image path or a `storage`-centric quartet. Line 142: """ Line 143: self.vm.changeCD(driveSpec) > No return value? Right, will fix. Line 144: Line 145: @api.method Line 146: def changeFloppy(self, driveSpec): Line 147: """ Line 151: :type vmId: UUID Line 152: :param driveSpec: specification of the new CD image. Either an Line 153: image path or a `storage`-centric quartet. Line 154: """ Line 155: self.vm.changeFloppy(driveSpec) > No return value? Right, will fix. Line 156: Line 157: @api.method Line 158: def cont(self): Line 159: return self.vm.cont() Line 263: """ Line 264: Lock user session in guest operating system using guest agent. Line 265: """ Line 266: self.vm.guestAgent.desktopLock() Line 267: if self.vm.guestAgent.isResponsive(): > +1 on current version Let's keep this version as-is, I have further (minor) improvements in the pipeline Line 268: return {'status': doneCode} Line 269: else: Line 270: return errCode['nonresp'] Line 271: Line 333: """ Line 334: return self._getStats() Line 335: Line 336: def _getStats(self, runHooks=True): Line 337: if runHooks: > Nothing to do when runHooks is false? No, no changes with respect to the hold code. Line 338: try: Line 339: hooks.before_get_vm_stats() Line 340: except exception.HookError as e: Line 341: return response.error('hookError', -- To view, visit https://gerrit.ovirt.org/61475 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
