[libvirt] [PATCH 1/2] storage: zfs: enable on Linux

[Roman Bogorodskiy]

  John Ferlan wrote:

> On 01/25/2016 08:37 PM, Roman Bogorodskiy wrote:
> >   John Ferlan wrote:
> > 
> >>
> >>
> >> On 01/02/2016 09:25 PM, Roman Bogorodskiy wrote:
> >>> ZFS-on-Linux implementation of ZFS starting with version 0.6.4
> >>> contains all the features we use, so there's no reason to limit
> >>> libvirt ZFS storage backend to FreeBSD only.
> >>>
> >>> There's still one difference between these implementations: ZFS on
> >>> FreeBSD requires to set 'volmode' option to 'dev' when creating
> >>> volumes, while ZFS-on-Linux does not need that.
> >>>
> >>> Handle it by checking if 'volmode' option is needed by parsing usage
> >>> information of the 'zfs get' command.
> >>> ---
> >>>  configure.ac                      |  8 ------
> >>>  src/storage/storage_backend_zfs.c | 51 +++++++++++++++++++++++++++++++++++++--
> >>>  2 files changed, 49 insertions(+), 10 deletions(-)
> >>>
> >>
> >> Caveat - I know very little about zfs, FreeBSD...  But it's languishing
> >> so...
> > 
> > Heh. Thanks for stepping in! :)
> > 
> 
> Sorry for the delay - backlog grows fast these days.
> 
> >>> diff --git a/configure.ac b/configure.ac
> >>> index a566f5b..d768341 100644
> >>> --- a/configure.ac
> >>> +++ b/configure.ac
> >>> @@ -2005,14 +2005,6 @@ if test "$with_storage_gluster" = "yes"; then
> >>>  fi
> >>>  AM_CONDITIONAL([WITH_STORAGE_GLUSTER], [test "$with_storage_gluster" = "yes"])
> >>>  
> >>> -if test "$with_storage_zfs" = "check"; then
> >>> -    with_storage_zfs=$with_freebsd
> >>> -fi
> >>> -
> >>> -if test "$with_storage_zfs" = "yes" && test "$with_freebsd" = "no"; then
> >>> -    AC_MSG_ERROR([The ZFS storage driver can be enabled on FreeBSD only.])
> >>> -fi
> >>> -
> >>>  if test "$with_storage_zfs" = "yes" ||
> >>>     test "$with_storage_zfs" = "check"; then
> >>>    AC_PATH_PROG([ZFS], [zfs], [], [$PATH:/sbin:/usr/sbin])
> >>
> >> So it would seem this hunk and patch 2 would be more related.  That is
> >> the virsh WITH_STORAGE_ZFS. The order of the patches is up to you.
> > 
> > Actually, these two patches are unrelated. I should have been added the
> > virsh part when I added ZFS driver in the first place, but I forgot
> > about it at that time.
> > 
> > I remembered about it when I was testing my configure.ac changes on
> > Linux and saw that virsh -V output does not change.
> > 
> 
> It would seem this should be a separate patch then since it's not
> related to the 'volmode' support.  The configure checks aren't my area
> of expertise, but as long as you split it out, ACK to that.
> 
> 
> Based on the explanation you provide - an ACK for the following change
> as well.

I have separated the patches and pushed. Thanks for review!

Roman Bogorodskiy