[libvirt PATCH 06/23] util: use short form -g arg to scsi_id

Daniel P. Berrangé berrange at redhat.com
Fri Jun 19 11:09:07 UTC 2020


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 at 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 :|




More information about the libvir-list mailing list