Vinzenz Feenstra has posted comments on this change.

Change subject: Support for mapping image ids to physical devices
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.ovirt.org/#/c/31465/5/scripts/diskmapper/diskmapper.libudev
File scripts/diskmapper/diskmapper.libudev:

Line 105:                     print '%s|%s' % (devnode, serial)
Line 106:             entry = ludev.udev_list_entry_get_next(entry)
Line 107:     finally:
Line 108:         ludev.udev_enumerate_unref(udevenum)
Line 109:         ludev.udev_unref(udev)
> I think this is too complex and unmaintainable. It will be much simpler and
well that's a question of PoV. I was considering to use a python udev wrapper 
which does basically the same. Therefore I switched to this version. I don't 
think it's unmaintainable though.
Switching to a C version might be a consideration on a later time. I think that 
this version is the easiest approach. Adding a C binary will require additional 
effort in packaging a new platform specific subpackage just for this, which 
would be not really reasonable for such a small task.


-- 
To view, visit http://gerrit.ovirt.org/31465
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8961b3703703df573dbf2f0fca2143b463ae76b
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-guest-agent
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to