Re: [ovirt-devel] [vdsm] VmDevices rework
On Tue, Apr 28, 2015 at 04:28:12AM -0400, Martin Polednik wrote:
Hello everyone,
I have started working on line of patches that deal with current state of VM devices
in VDSM. There is a wiki page at [1] describing the issues, phases and final goals
for this work. Additionally, there is formal naming system for code related to
devices. I would love to hear your opinions and comments about this effort!
(and please note that this is very long term work)
[1] http://www.ovirt.org/Feature/VmDevices_rework
Hi Martin,
It's a great and just initiative. Renaming is good, but I'd like to hear
more about your final goal - besides the hope that it the code is
shorter.
In my opion you should state your high-level goal explicitly:
device_object should be self-sufficient regarding: initiation, state
change, stat extraction and stat reporting.
Comments:
1. We want to drop the legacy conf support. In
https://gerrit.ovirt.org/40104 we have removed support of engine<3.3
so production environment does not need it any more.
There might be an issue with command-line test scripts, though that
might be abusing the legacy args. At the very least it should be
marked as deprecated.
2. dev_map is an unfortunate historical mistake. The fact that it is a
map of device type to device_object lists helps no one. It
should be hidden behind one or two lookup functions. Each object
class should be able to iterate its instances, and add filtering on
top of that.
3. A definition like "Type of the device is dev_type" does not help for
someone who knows what is dev_type. Please give examples.
The "formal naming system" could use some wiki styling.
4. I did not understand "Libvirt XML parsing and processing is dumb -
missing orchestration object to delegate XML chunks to devices themself"
5. It seems that you have further "phases" planned, but not written.
Could you provide at least the title, or "charter" of each phase? 1.1
is about renaming. 1.2 is about legacy. Do you have phases 2, 3..?
Death (or at least diet) to vm.py!
Dan.