Nir Soffer has posted comments on this change. Change subject: vm: snapshot - adding appropriate logs ......................................................................
Patch Set 1: (7 comments) The logs are not appropriate :-) https://gerrit.ovirt.org/#/c/50775/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3243 Line 3244 Line 3245 Line 3246 Line 3247 freeze() has its own info log, so we don't want to log about the snapshot before that log. Line 3244 Line 3245 Line 3246 Line 3247 Line 3248 log.info message about the snapshot, drives and memory here. I think something like: Taking snapshot with drives 'vda', 'vdb' (memory=True) Line 3245 Line 3246 Line 3247 Line 3248 Line 3249 log.info message here when snapshot is complete. Line 3131: padToBlockSize(memoryVolPath) Line 3132: else: Line 3133: fileUtils.padToBlockSize(memoryVolPath) Line 3134: Line 3135: self.log.info("Taking a live snapshot on VM %s.", self.conf['vmId']) We already have a log about taking a snapshot in jsonrpc bridge, so we don't need another log here. There is no need to include the vm id, it is already prefixed to the message (vmid=`xxxyyy`::message) Line 3136: Line 3137: snap = vmxml.Element('domainsnapshot') Line 3138: disks = vmxml.Element('disks') Line 3139: newDrives = {} Line 3183: vmDrives[vmDevName] = vmDrive Line 3184: Line 3185: if len(newDrives) == 0: Line 3186: self.log.info("Include no disks in live snapshot on VM %s.", Line 3187: self.conf['vmId']) Instead of this, lets log the drives we take a snapshot with later. Line 3188: Line 3189: preparedDrives = {} Line 3190: Line 3191: for vmDevName, vmDevice in newDrives.iteritems(): Line 3212: snapFlags = (libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | Line 3213: libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA) Line 3214: Line 3215: if memoryParams: Line 3216: self.log.info("Include memory in live snapshot on VM %s.", Lets log this in the single log before taking a snapshot. Line 3217: self.conf['vmId']) Line 3218: # Save the needed vm configuration Line 3219: # TODO: this, as other places that use pickle.dump Line 3220: # directly to files, should be done with outOfProcess Line 3286: self.startDisksStatsCollection() Line 3287: if memoryParams: Line 3288: self.cif.teardownVolumePath(memoryVol) Line 3289: Line 3290: self.log.info("Completed live snapshot on VM %s.", self.conf['vmId']) Move this when snapshot is finished, before calling thaw(), again, to avoid thaw info log mixed with snapshot log. Also we have a completion log in the json bridge, so there is not need to log anything here. Line 3291: Line 3292: # Returning quiesce to notify the manager whether the guest agent Line 3293: # froze and flushed the filesystems or not. Line 3294: quiesce = should_freeze and freezed["status"]["code"] == 0 -- To view, visit https://gerrit.ovirt.org/50775 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e948863f762de9cfc281bdff1fc92c88b678d75 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches