Amit Aviram has posted comments on this change. Change subject: vm: Libvirt quering after disk detach operation addition. ......................................................................
Patch Set 6: (5 comments) https://gerrit.ovirt.org/#/c/45138/6/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 26: parameters = [ Line 27: # Section: [vars] Line 28: ('vars', [ Line 29: Line 30: ('wait_for_detach', '20', > This name is named like a boolean option; lets rename it to hotunplug_timeo Done Line 31: 'Time amount to wait for a VM to detach its disk'), Line 32: Line 33: ('core_dump_enable', 'true', Line 34: 'Enable core dump.'), Line 27: # Section: [vars] Line 28: ('vars', [ Line 29: Line 30: ('wait_for_detach', '20', Line 31: 'Time amount to wait for a VM to detach its disk'), > Time amount is not clear - should be "Number of seconds to wait..." Done Line 32: Line 33: ('core_dump_enable', 'true', Line 34: 'Enable core dump.'), Line 35: https://gerrit.ovirt.org/#/c/45138/6/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2625: params=drive.custom) Line 2626: try: Line 2627: self._dom.detachDevice(driveXml) Line 2628: deadline = utils.monotonic_time() + \ Line 2629: float(config.get('vars', 'wait_for_detach')) > Do not use \ for long lines, it is very fragile syntax. Instead put the exp Done Line 2630: while drive.serial in self._dom.XMLDesc(0): Line 2631: self.log.debug("Waiting for hotunplug to finish.") Line 2632: time.sleep(1) Line 2633: if utils.monotonic_time() > deadline: Line 2626: try: Line 2627: self._dom.detachDevice(driveXml) Line 2628: deadline = utils.monotonic_time() + \ Line 2629: float(config.get('vars', 'wait_for_detach')) Line 2630: while drive.serial in self._dom.XMLDesc(0): > This is too dirty, please find the disk using the apis in this module. As replied in patch set #1, we cannot use the apis in this module, these methods are based on the VM's state, that is being changes only after this operation succeeds.. Line 2631: self.log.debug("Waiting for hotunplug to finish.") Line 2632: time.sleep(1) Line 2633: if utils.monotonic_time() > deadline: Line 2634: return response.error('hotunplugDisk', Line 2631: self.log.debug("Waiting for hotunplug to finish.") Line 2632: time.sleep(1) Line 2633: if utils.monotonic_time() > deadline: Line 2634: return response.error('hotunplugDisk', Line 2635: "libvirt could not detach the disk.") > Log info message when disk was finally detached (in a separate patch). Done, will add another patch. Line 2636: except libvirt.libvirtError as e: Line 2637: self.log.exception("Hotunplug failed") Line 2638: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 2639: return response.error('noVM') -- To view, visit https://gerrit.ovirt.org/45138 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I393ce55dd761ac825cb96bd499976fd74c366b09 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
