[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] storage pool discovery



Hi Jim -
  Thanks for your comments.  I really appreciate the detailed look.
I'll implement your suggestions (including the refactoring of the code
to turn a virStringList into a single XML string), though I have a
question about one of them:

On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote:
> > diff --git a/configure.in b/configure.in
> > index 9479f1c..430a097 100644
> > --- a/configure.in
> > +++ b/configure.in
> > @@ -671,6 +671,11 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then
> >    fi
> >  fi
> >  AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"])
> > +if test "$with_storage_fs" = "yes"; then
> > +  AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin])
> > +  AC_DEFINE_UNQUOTED([SHOWMOUNT], ["$SHOWMOUNT"],
> > +    [Location or name of the showmount program])
> > +fi
> 
> Do we want to require the "showmount" package via the spec file?

I was a little worried about this too.  It's probably better to handle
this more like "iscsiadmin"/--with-storage-iscsi: defaults to enabling
iscsi backend if and only if iscsiadmin is present.  Fails with
--with-storage-iscsi=yes explicitly specified and iscsiadmin not found.

But I'm not sure we want to disable the storage_fs backend just because
we can't find showmount.  So maybe there should be another config option
--with-storage-netfs-discovery to cover the only code that actually uses
showmount?  Either of these sounds reasonable to me.  Do you have a
preference?

Dave



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]