Ayal Baron has posted comments on this change.
Change subject: WIP: storage: Introduce prepareBackupDisk vdsm verb
......................................................................
Patch Set 4: (10 inline comments)
....................................................
File vdsm/API.py
Line 744: def tearDown(self):
Line 745: return self._irs.teardownVolume(self._sdUUID, self._spUUID,
Line 746: self._imgUUID, self._UUID)
Line 747:
Line 748: def prepareForBackup(self, bkupType):
Backup is a specific usage of this functionality (policy), but we're providing
a more generic mechanism here that may be used in other ways in the future, so
the name of the method at this level should not be prepareForBackup.
exposeVDiskDevice ?
Line 749: return self._irs.prepareBackupDisk(self._sdUUID, self._spUUID,
Line 750: self._imgUUID, self._UUID,
bkupType)
Line 751:
Line 752:
....................................................
File vdsm/storage/hsm.py
Line 3130:
Line 3131: @public
Line 3132: def prepareBackupDisk(self, sdUUID, spUUID, imgUUID, volUUID,
bkupType):
Line 3133:
Line 3134: if bkupType not in volume.BACKUP_APP_TYPES:
instead of bkupType, why not use connectionType which could get either "local"
or "remote"
Line 3135: raise se.prepareBackupDiskError()
Line 3136:
Line 3137: dom = sdCache.produce(sdUUID=sdUUID)
Line 3138: snapVol = dom.produceVolume(imgUUID=imgUUID, volUUID=volUUID)
Line 3141: sockType = 'unix'
Line 3142: if bkupType == volume.BACKUP_HOST:
Line 3143: sockType = 'tcp'
Line 3144:
Line 3145: nbdPath = snapVol.exportAsNbd(sockType)
you are correct, this is about an entire disk, not a single volume and indeed
the chain has to be activated. Even if it didn't it would still be an image
action since it pertains to the virtual disk and not any single part of it.
Line 3146:
Line 3147: cbtInfo = snapVol.queryCBTInfo()
Line 3148: return dict(protocol=sockType, path=nbdPath, cbtInfo=cbtInfo)
Line 3149:
....................................................
File vdsm/storage/volume.py
Line 675: def getNbdPid(self):
Line 676: return int(self.getMetaParam(NBDPID))
Line 677:
Line 678: def setNbdPid(self, pid):
Line 679: self.setMetaParam(NBDPID, pid)
not to mention that only the spm is allowed to touch the metadata.
This data should be saved on the object and persisted to disk (in case vdsm
restarts while the nbd device is active)
Line 680:
Line 681: def isShared(self):
Line 682: return self.getVolType() == type2name(SHARED_VOL)
Line 683:
Line 939: def exportAsNbd(self, sockType):
Line 940: volPath = self.getVolumePath()
Line 941:
Line 942: if sockType == 'unix':
Line 943: rc, ret1, ret2 = qemuExportNbdOverUnix(volPath)
s/ret1/pid/
s/ret2/nbdSockPath/
Line 944: else:
Line 945: # TODO: Implement this later
Line 946: return None
Line 947:
Line 941:
Line 942: if sockType == 'unix':
Line 943: rc, ret1, ret2 = qemuExportNbdOverUnix(volPath)
Line 944: else:
Line 945: # TODO: Implement this later
comment should at least state what is 'this' to implement
Line 946: return None
Line 947:
Line 948: if rc:
Line 949: self.log.error("Error exporting volume as NBD, out=%s,
err=%s",
Line 947:
Line 948: if rc:
Line 949: self.log.error("Error exporting volume as NBD, out=%s,
err=%s",
Line 950: (ret1, ret2))
Line 951: raise se.VolumeExportAsNbdError()
why not state which disk failed?
Line 952:
Line 953: self.setNbdPid(ret1)
Line 954: self.nbdSockPath = ret2
Line 955:
Line 959: # For now, return the entire volume as part of changed info
Line 960: size = int(self.getMetaParam(SIZE))
Line 961: changedArea = [dict(startOffset=0, length=size)]
Line 962:
Line 963: changes = {'startOffset': 0, 'length': size,
why the duplicity?
Line 964: 'changedArea': changedArea}
Line 965: return changes
Line 966:
Line 967:
Line 1107: # qemu-nbd starts as a user space nbd server, doesn't return,
until killed.
Line 1108: proc = misc.execCmd(cmd, sync=False)
Line 1109: rc = proc.returncode
Line 1110:
Line 1111: if proc.returncode:
s/proc.returncode/rc/
Line 1112: # qemu-nbd bad happened
Line 1113: log.error('(qemuExportNbdOverUnix): QEMU-NBD BAD HAPPENED
!!! ')
Line 1114: os.unlink(sockPath)
Line 1115:
Line 1112: # qemu-nbd bad happened
Line 1113: log.error('(qemuExportNbdOverUnix): QEMU-NBD BAD HAPPENED
!!! ')
Line 1114: os.unlink(sockPath)
Line 1115:
Line 1116: rc = proc.returncode
this line is redundant
Line 1117: out = misc.stripNewLines(proc.stdout)
Line 1118: err = misc.stripNewLines(proc.stderr)
Line 1119:
Line 1120: return (rc, out, err)
--
To view, visit http://gerrit.ovirt.org/14403
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ccacda2b09810a71f445ed65de6a09336ea3fb1
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
