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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
