On Fri, Jun 19, 2020 at 12:31:22PM +0200, Peter Krempa wrote:
On Fri, Jun 19, 2020 at 11:17:49 +0100, Daniel Berrange wrote:
On Fri, Jun 19, 2020 at 11:56:07AM +0200, Peter Krempa wrote:
On Fri, Jun 19, 2020 at 10:32:43 +0100, Daniel Berrange wrote:
We can't change the term used by scsi_id for its CLI arg, but we can avoid it by picking the short form instead.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/util/virstoragefile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a6357abb08..b4337c2736 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1365,7 +1365,7 @@ virStorageFileGetSCSIKey(const char *path,
cmd = virCommandNewArgList("/lib/udev/scsi_id", "--replace-whitespace", - "--whitelisted", + "-g",
IMO this decreases the readability of the code.
I agree, but I think that doesn't really matter in the big picture as this is not code that is a serious maint burden in libvirt.
It still a parameter of a tool we use rather than our own. Since there is no technical reason for it and IMO makes the code worse I will not provide the R-B for this patch.
No matter what the spelling used is, I've no idea what the option actually does, or why we need it, not helped by the lack of a manpage for scsi_id on Fedora. So in terms of understanding the code, both the before and after state are bad. What I'll do is add a comment to describe the purpose of adding the option....once I find out what that purpose actually is.... Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|