Nir Soffer has posted comments on this change. Change subject: Support for mapping image ids to physical devices ......................................................................
Patch Set 4: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/31465/4/scripts/diskmapper/diskmapper.el6 File scripts/diskmapper/diskmapper.el6: Line 1: #!/bin/sh Line 2: Line 3: for DEVICE in `ls /sys/block`; DEVICE is a variable, not a constant, or not a value from the environment - any reason to use this notation and not the more expected "device"? Line 4: do Line 5: if [ -e /sys/block/$DEVICE/serial ]; Line 6: then Line 7: echo -n /dev/$DEVICE Line 1: #!/bin/sh Line 2: Line 3: for DEVICE in `ls /sys/block`; Line 4: do Line 5: if [ -e /sys/block/$DEVICE/serial ]; I think that -f is better here, you are looking for a file, right? Line 6: then Line 7: echo -n /dev/$DEVICE Line 8: echo -n '|' Line 9: cat /sys/block/$DEVICE/serial Line 6: then Line 7: echo -n /dev/$DEVICE Line 8: echo -n '|' Line 9: cat /sys/block/$DEVICE/serial Line 10: echo Contents of sysfs items typically terminated by a newline, but it can be also empty. If serial is xxx\n, this will create an empty line in the output. So I think this would be more reliable: for device in `ls /sys/block`; do if [ -f /sys/block/$device/serial ]; then serial=`cat /sys/block/$device/serial` echo "$device|$serial"; fi done I tested with with empty serial file, empty line, value without newline, and value with a newline (expected). Line 11: fi -- 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: 4 Gerrit-Project: ovirt-guest-agent Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@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
