Nir Soffer has posted comments on this change. Change subject: vm: Libvirt quering after disk detach operation addition. ......................................................................
Patch Set 1: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/45138/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2636: params=drive.custom) Line 2637: try: Line 2638: self._dom.detachDevice(driveXml) Line 2639: opStarted = datetime.datetime.now() Line 2640: timoutSec = 10 time.time and datetime.datetime are effected by system time changes, and not good way to implement timeouts. Use: deadline = utils.monotonic_time() Line 2641: while drive.serial in self._dom.XMLDesc(0): Line 2642: self.log.debug("Waiting for hotunplug to finish.") Line 2643: time.sleep(1) Line 2644: if (datetime.datetime.now() - opStarted).seconds > timoutSec: Line 2637: try: Line 2638: self._dom.detachDevice(driveXml) Line 2639: opStarted = datetime.datetime.now() Line 2640: timoutSec = 10 Line 2641: while drive.serial in self._dom.XMLDesc(0): While drive serial is a truncated random uuid, so is unlikely to appear in the xml, but this check is too dirty. Lets do this in a clean way using one of the findDriveByXXX helpers. Line 2642: self.log.debug("Waiting for hotunplug to finish.") Line 2643: time.sleep(1) Line 2644: if (datetime.datetime.now() - opStarted).seconds > timoutSec: Line 2645: return response.error('hotunplugDisk', Line 2640: timoutSec = 10 Line 2641: while drive.serial in self._dom.XMLDesc(0): Line 2642: self.log.debug("Waiting for hotunplug to finish.") Line 2643: time.sleep(1) Line 2644: if (datetime.datetime.now() - opStarted).seconds > timoutSec: Use: if utils.monotonic_time() > deadline: Line 2645: return response.error('hotunplugDisk', Line 2646: "libvirt could not detach the disk.") Line 2647: except libvirt.libvirtError as e: Line 2648: self.log.exception("Hotunplug failed") -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> 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
