Change in vdsm[master]: vm: add the transient disk support
fsimonce at redhat.com
fsimonce at redhat.com
Wed Oct 9 13:46:31 UTC 2013
Federico Simoncelli has posted comments on this change.
Change subject: vm: add the transient disk support
......................................................................
Patch Set 11:
(9 comments)
....................................................
File lib/vdsm/config.py.in
Line 157:
Line 158: ('libvirt_env_variable_log_outputs', '',
Line 159: 'Specify the output to track libvirt calls'),
Line 160:
Line 161: ('transient_disks_repository', '@VDSMLIBDIR@/transient',
It's not only about size, it's also the fact that /var/run might be in-memory (tmpfs) and it's not the correct place for data files (in fact also libvirt uses /var/lib/libvirt for data, e.g. images, networks, etc.).
Line 162: 'Local path to the transient disks repository.'),
Line 163: ]),
Line 164:
Line 165: # Section: [ksm]
....................................................
File lib/vdsm/tool/transient.py
Line 27: from vdsm.utils import execCmd, CommandPath
Line 28:
Line 29:
Line 30: SELINUX_VIRT_IMAGE_LABEL = "system_u:object_r:virt_image_t:s0"
Line 31: TRANSIENT_DISKS_REPO = config.get('vars', 'transient_disks_repository')
Some appliances (ovirt-node?) might prefer to have it somewhere else. We already need the logic for the cleanup, creating it is just a small nit. Making this configurable was also a request coming from the previous review.
Line 32:
Line 33: _fuser = CommandPath(
Line 34: "fuser",
Line 35: "/sbin/fuser", # Fedora, EL6
Line 32:
Line 33: _fuser = CommandPath(
Line 34: "fuser",
Line 35: "/sbin/fuser", # Fedora, EL6
Line 36: )
Already answered (with more considerations) on patch set 7.
Line 37:
Line 38:
Line 39: @expose("setup-transient-repository")
Line 40: def setup_transient_repository(*args):
Line 50: raise
Line 51:
Line 52: os.chown(TRANSIENT_DISKS_REPO, vdsm_uid, vdsm_gid)
Line 53: os.chmod(TRANSIENT_DISKS_REPO, 0750)
Line 54: selinux.chcon(TRANSIENT_DISKS_REPO, SELINUX_VIRT_IMAGE_LABEL)
No it doesn't "explode", I am not changing a sebool, I am changing the file context. Yes a restorecon would un-do the change. We'll need to file a request for that but for now this is good enough.
Line 55:
Line 56:
Line 57: @expose("cleanup-transient-repository")
Line 58: def cleanup_transient_repository(*args):
Line 61: (NOTE: it is recommended to NOT execute this command when the vdsm
Line 62: daemon is running)
Line 63: """
Line 64: transient_images = set((os.path.join(TRANSIENT_DISKS_REPO, x)
Line 65: for x in os.listdir(TRANSIENT_DISKS_REPO)))
Done
Line 66:
Line 67: if len(transient_images) == 0:
Line 68: return # Nothing to do
Line 69:
....................................................
File vdsm/vm.py
Line 412: class DRIVE_SHARED_TYPE:
Line 413: NONE = "none"
Line 414: EXCLUSIVE = "exclusive"
Line 415: SHARED = "shared"
Line 416: TRANSIENT = "transient"
Exclusive (locking) is for a volume on a shared storage. Transient is obviously also exclusive as it's on the local machine storage (no real need for locking).
Line 417:
Line 418: @classmethod
Line 419: def getAllValues(cls):
Line 420: # TODO: use introspection
Line 3298: params=customProps)
Line 3299: return {'status': doneCode, 'vmList': self.status()}
Line 3300:
Line 3301: def _createTransientDisk(self, drive):
Line 3302: if drive.get('shared', None) != DRIVE_SHARED_TYPE.TRANSIENT:
I prefer not to spread this check in multiple places.
Line 3303: return
Line 3304:
Line 3305: # FIXME: This should be replaced in future the support for transient
Line 3306: # disk in libvirt (BZ#832194)
Line 3327: drive['path'] = transientPath
Line 3328: drive['format'] = 'cow'
Line 3329:
Line 3330: def _removeTransientDisk(self, drive):
Line 3331: if getattr(drive, 'transient', False):
leftover to be fixed
Line 3332: os.unlink(drive.path)
Line 3333:
Line 3334: def hotplugDisk(self, params):
Line 3335: if self.isMigrating():
Line 3390: diskParams = params.get('drive', {})
Line 3391: diskParams['path'] = self.cif.prepareVolumePath(diskParams)
Line 3392:
Line 3393: try:
Line 3394: drive = self._findDriveByUUIDs(diskParams)
it's related to this patch since now we can't use a path to identify a (transient) disk.
Line 3395: except LookupError:
Line 3396: self.log.error("Hotunplug disk failed - Disk not found: %s",
Line 3397: diskParams)
Line 3398: return {'status': {
--
To view, visit http://gerrit.ovirt.org/18326
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list