Change in vdsm[master]: vm: snapshot - adding appropriate logs

nsoffer at redhat.com nsoffer at redhat.com
Sun Dec 20 23:14:09 UTC 2015


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 <derez at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list