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