[Qemu-devel] [PATCH v3 0/1] Allow the building of VirtFS to be disabled

[Qemu-devel] [PATCH v3 0/1] Allow the building of VirtFS to be disabled

[Meador Inge]

There have been reports [1, 2] where folks have had issues building
VirtFS and the virtio backend on older systems.  I personally saw
problems due to the use of features (struct statfs f_frsize field,
fdopendir, O_NOATIME) in this code that are not available on much older
Linux systems.  Given, the system I ran into this on is ancient (RH8 sysroot),
but I still need to build QEMU on it nonetheless.

This patch adds a new configure option for disabling the building of
VirtFS all together.  Tested by building with and without --disable-virtfs
on older (RH8 sysroot) and newer systems (x64 Fedora 16).

[1] http://lists.nongnu.org/archive/html/qemu-devel/2011-12/msg00171.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg00404.html

* Changes from v2
  - Use 'test "$foo" = yes && test "$bar" = yes' instead of
    '"$foo" = "yes" -a "$bar" = "yes"'.
* Changes from v1
  - Simplify the configure logic and support the standard behavior
    of defaulting virtfs="".  Changes suggested by Peter Maydell.

Meador Inge (1):
  ./configure: add option for disabling VirtFS

 Makefile  |    2 ++
 configure |   25 +++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH v3 1/1] ./configure: add option for disabling VirtFS

[Meador Inge]

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 Makefile  |    2 ++
 configure |   25 +++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index e66e885..3dd67e2 100644
--- a/Makefile
+++ b/Makefile
@@ -40,7 +40,9 @@ HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt
+ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
+endif
 else
 DOCS=
 endif
diff --git a/configure b/configure
index 763db24..88521d2 100755
--- a/configure
+++ b/configure
@@ -121,6 +121,7 @@ docs=""
 fdt=""
 nptl=""
 sdl=""
+virtfs=""
 vnc="yes"
 sparse="no"
 uuid=""
@@ -586,6 +587,10 @@ for opt do
   ;;
   --enable-sdl) sdl="yes"
   ;;
+  --disable-virtfs) virtfs="no"
+  ;;
+  --enable-virtfs) virtfs="yes"
+  ;;
   --disable-vnc) vnc="no"
   ;;
   --enable-vnc) vnc="yes"
@@ -993,6 +998,8 @@ echo "  --disable-strip          disable stripping binaries"
 echo "  --disable-werror         disable compilation abort on warning"
 echo "  --disable-sdl            disable SDL"
 echo "  --enable-sdl             enable SDL"
+echo "  --disable-virtfs         disable VirtFS"
+echo "  --enable-virtfs          enable VirtFS"
 echo "  --disable-vnc            disable VNC"
 echo "  --enable-vnc             enable VNC"
 echo "  --enable-cocoa           enable COCOA (Mac OS X only)"
@@ -2808,8 +2815,15 @@ confdir=$sysconfdir$confsuffix
 tools=
 if test "$softmmu" = yes ; then
   tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
-  if [ "$cap" = "yes" -a "$linux" = "yes" ] ; then
-      tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
+  if test "$virtfs" != no ; then
+      if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; then
+	  virtfs=yes
+	  tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
+      else
+	  if test "$virtfs" = yes; then
+	      feature_not_found "virtfs"
+	  fi
+      fi
   fi
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
       tools="qemu-nbd\$(EXESUF) $tools"
@@ -2874,6 +2888,7 @@ echo "Audio drivers     $audio_drv_list"
 echo "Extra audio cards $audio_card_list"
 echo "Block whitelist   $block_drv_whitelist"
 echo "Mixer emulation   $mixemu"
+echo "VirtFS support    $virtfs"
 echo "VNC support       $vnc"
 if test "$vnc" = "yes" ; then
     echo "VNC TLS support   $vnc_tls"
@@ -3163,10 +3178,8 @@ fi
 if test "$libattr" = "yes" ; then
   echo "CONFIG_LIBATTR=y" >> $config_host_mak
 fi
-if test "$linux" = "yes" ; then
-  if test "$attr" = "yes" ; then
-    echo "CONFIG_VIRTFS=y" >> $config_host_mak
-  fi
+if test "$virtfs" = "yes" ; then
+  echo "CONFIG_VIRTFS=y" >> $config_host_mak
 fi
 if test "$blobs" = "yes" ; then
   echo "INSTALL_BLOBS=yes" >> $config_host_mak
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v3 0/1] Allow the building of VirtFS to be disabled

[Meador Inge]

Ping... Any comments on the v3 patch?

http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg01236.html

On Thu, Feb 9, 2012 at 8:31 PM, Meador Inge <meadori@codesourcery.com> wrote:
> There have been reports [1, 2] where folks have had issues building
> VirtFS and the virtio backend on older systems.  I personally saw
> problems due to the use of features (struct statfs f_frsize field,
> fdopendir, O_NOATIME) in this code that are not available on much older
> Linux systems.  Given, the system I ran into this on is ancient (RH8 sysroot),
> but I still need to build QEMU on it nonetheless.
>
> This patch adds a new configure option for disabling the building of
> VirtFS all together.  Tested by building with and without --disable-virtfs
> on older (RH8 sysroot) and newer systems (x64 Fedora 16).
>
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2011-12/msg00171.html
> [2] http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg00404.html
>
> * Changes from v2
>  - Use 'test "$foo" = yes && test "$bar" = yes' instead of
>    '"$foo" = "yes" -a "$bar" = "yes"'.
> * Changes from v1
>  - Simplify the configure logic and support the standard behavior
>    of defaulting virtfs="".  Changes suggested by Peter Maydell.
>
> Meador Inge (1):
>  ./configure: add option for disabling VirtFS
>
>  Makefile  |    2 ++
>  configure |   25 +++++++++++++++++++------
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> --
> 1.7.7.6
>
>



-- 
# Meador

Re: [Qemu-devel] [PATCH v3 1/1] ./configure: add option for disabling VirtFS

[Aneesh Kumar K.V]

On Thu, 9 Feb 2012 20:31:17 -0600, Meador Inge <meadori@codesourcery.com> wrote:
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
>  Makefile  |    2 ++
>  configure |   25 +++++++++++++++++++------
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e66e885..3dd67e2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -40,7 +40,9 @@ HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
> 
>  ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt
> +ifdef CONFIG_VIRTFS
>  DOCS+=fsdev/virtfs-proxy-helper.1
> +endif
>  else
>  DOCS=
>  endif
> diff --git a/configure b/configure
> index 763db24..88521d2 100755
> --- a/configure
> +++ b/configure
> @@ -121,6 +121,7 @@ docs=""
>  fdt=""
>  nptl=""
>  sdl=""
> +virtfs=""

do we want to say

virtfs="yes"

That would make it explicit like vnc below.

>  vnc="yes"
>  sparse="no"
>  uuid=""
> @@ -586,6 +587,10 @@ for opt do
>    ;;
>    --enable-sdl) sdl="yes"
>    ;;
> +  --disable-virtfs) virtfs="no"
> +  ;;
> +  --enable-virtfs) virtfs="yes"
> +  ;;
>    --disable-vnc) vnc="no"
>    ;;
>    --enable-vnc) vnc="yes"
> @@ -993,6 +998,8 @@ echo "  --disable-strip          disable stripping binaries"
>  echo "  --disable-werror         disable compilation abort on warning"
>  echo "  --disable-sdl            disable SDL"
>  echo "  --enable-sdl             enable SDL"
> +echo "  --disable-virtfs         disable VirtFS"
> +echo "  --enable-virtfs          enable VirtFS"
>  echo "  --disable-vnc            disable VNC"
>  echo "  --enable-vnc             enable VNC"
>  echo "  --enable-cocoa           enable COCOA (Mac OS X only)"
> @@ -2808,8 +2815,15 @@ confdir=$sysconfdir$confsuffix
>  tools=
>  if test "$softmmu" = yes ; then
>    tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
> -  if [ "$cap" = "yes" -a "$linux" = "yes" ] ; then
> -      tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
> +  if test "$virtfs" != no ; then
> +      if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; then
> +	  virtfs=yes

is that needed, if we set that earlier ?

> +	  tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
> +      else
> +	  if test "$virtfs" = yes; then
> +	      feature_not_found "virtfs"
> +	  fi
> +      fi


We need $cap only for virtfs-proxy-helper. Ie, we can build rest of
virtfs even if $cap is "no". Should we enable that ?


>    fi
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>        tools="qemu-nbd\$(EXESUF) $tools"
> @@ -2874,6 +2888,7 @@ echo "Audio drivers     $audio_drv_list"
>  echo "Extra audio cards $audio_card_list"
>  echo "Block whitelist   $block_drv_whitelist"
>  echo "Mixer emulation   $mixemu"
> +echo "VirtFS support    $virtfs"
>  echo "VNC support       $vnc"
>  if test "$vnc" = "yes" ; then
>      echo "VNC TLS support   $vnc_tls"
> @@ -3163,10 +3178,8 @@ fi
>  if test "$libattr" = "yes" ; then
>    echo "CONFIG_LIBATTR=y" >> $config_host_mak
>  fi
> -if test "$linux" = "yes" ; then
> -  if test "$attr" = "yes" ; then
> -    echo "CONFIG_VIRTFS=y" >> $config_host_mak
> -  fi
> +if test "$virtfs" = "yes" ; then
> +  echo "CONFIG_VIRTFS=y" >> $config_host_mak
>  fi
>  if test "$blobs" = "yes" ; then
>    echo "INSTALL_BLOBS=yes" >> $config_host_mak
> -- 
> 1.7.7.6
> 
> 

Re: [Qemu-devel] [PATCH v3 1/1] ./configure: add option for disabling VirtFS

[Peter Maydell]

On 17 February 2012 08:22, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Thu, 9 Feb 2012 20:31:17 -0600, Meador Inge <meadori@codesourcery.com> wrote:
>> --- a/configure
>> +++ b/configure
>> @@ -121,6 +121,7 @@ docs=""
>>  fdt=""
>>  nptl=""
>>  sdl=""
>> +virtfs=""
>
> do we want to say
>
> virtfs="yes"

No. The standard pattern for configure feature options should be:
 * default is to probe and use if present, not use if not present
 * --enable-foo should probe and fail configure if not present
 * --disable-foo should not probe and not use

See the discussion on this point on v1 of this patch:
 http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg01211.html

> That would make it explicit like vnc below.

Some of our legacy options don't follow the standard pattern, alas.

-- PMM