Change in vdsm[master]: vm payload: add file injection to vm
danken at redhat.com
danken at redhat.com
Sun Mar 11 14:58:27 UTC 2012
Dan Kenigsberg has posted comments on this change.
Change subject: vm payload: add file injection to vm
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(8 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 1973: #TODO: delete the payload devices
what does think mean, exactly? remove the iso image from /var/run/vdsm? why postpone this cleanup?
....................................................
File vdsm/mkimage.py
Line 32: _PAYLOAD_PATH_PREFIX = '/var/run/vdsm/payload/'
you forgot to base this on constants.P_VDSM_RUN
Line 47: dirname = tempfile.mkdtemp(prefix=_PAYLOAD_PATH_PREFIX)
I do not understand the "security" issue - this directory should be created with limited accessibility anyway. Could you elaborate?
in any case, I think dir=_PAYLOAD_PATH_PREFIX arg should be used to specify the directory in which the tempdir is created.
Line 74: m = storage.mount.Mount()
you are not using this object correctly. you should pass the (image, mntpoint) as its args. Current code would explode.
Line 76: m.mount(mntOpts='-o loop %s %s' % (floppy, tmp))
mntOpts='loop' only
Line 78: for file in os.listdir(dirname):
I mean that you should call _decodeFilesIntoDir(files, dirname)
only now.
and have _decodeFilesIntoDir write the files to dirname (if specified)
Line 92: isopath = tempfile.NamedTemporaryFile(prefix=_PAYLOAD_PATH_PREFIX, delete=False)
again, I believe "dir" is the right arg to use.
....................................................
File vdsm/vm.py
Line 448: devices[DISK_DEVICES].append(drv)
who defined this API with Engine?
It would make more sense to let Engine choose the exact address of this device. Is it still possible to change this?
--
To view, visit http://gerrit.ovirt.org/2321
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I256475342c79690a95ad999335522f99714cdc8b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
More information about the vdsm-patches
mailing list