Saggi Mizrahi has posted comments on this change.
Change subject: vm payload: add file injection to vm
......................................................................
Patch Set 27: I would prefer that you didn't submit this
(7 inline comments)
Also, this whole system isn't scalable at all, but I imagine it wasn't meant to
be.
....................................................
File vdsm/clientIF.py
Line 200: #TODO: REMOVE payload from getVmStats
Did you do it?
....................................................
File vdsm/mkimage.py
Line 36: def _decodeFilesIntoDir(files, dirname=None):
Always have the caller create the dirname
Line 43: :param files: [{'filename': 'content' }, {'filename':
'content'}]
it's actually {'filename: 'content', ...}
Line 48: dirname = tempfile.mkdtemp()
Don't put things in temp, it's a free for all security wise and also more and more
distros are making tmp tmpfs so it has very limited space.
Line 51: with file(filename, 'w') as f:
f.write(base64.b64decode(content))
Lines don't cost money
Line 81: m.mount(mntOpts='loop')
If you move this out you don't need to check is mounted
....................................................
File vdsm.spec.in
Line 593: %ghost %dir %{_localstatedir}/run/%{vdsm_name}/pools
pools is here twice
--
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: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>