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

[Qemu-devel] [PATCH v1 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

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

 Makefile  |    4 ++++
 configure |   16 +++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

-- 
1.7.7.6

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

[Meador Inge]

Signed-off-by: Meador Inge <meadori@codesourcery.com>
---
 Makefile  |    4 ++++
 configure |   16 +++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 47acf3d..030619c 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
@@ -162,8 +164,10 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
+ifdef CONFIG_VIRTFS
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y)
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
+endif
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
diff --git a/configure b/configure
index 763db24..081d720 100755
--- a/configure
+++ b/configure
@@ -121,6 +121,7 @@ docs=""
 fdt=""
 nptl=""
 sdl=""
+virtfs="yes"
 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,7 +2815,7 @@ confdir=$sysconfdir$confsuffix
 tools=
 if test "$softmmu" = yes ; then
   tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
-  if [ "$cap" = "yes" -a "$linux" = "yes" ] ; then
+  if [ "$cap" = "yes" -a "$linux" = "yes" -a "$virtfs" = "yes" ] ; then
       tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
   fi
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
@@ -2874,6 +2881,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"
@@ -3164,8 +3172,10 @@ 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
+  if test "$virtfs" = "yes" ; then
+    if test "$attr" = "yes" ; then
+      echo "CONFIG_VIRTFS=y" >> $config_host_mak
+    fi
   fi
 fi
 if test "$blobs" = "yes" ; then
-- 
1.7.7.6

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

[Peter Maydell]

On 7 February 2012 20:44, Meador Inge <meadori@codesourcery.com> wrote:
> Signed-off-by: Meador Inge <meadori@codesourcery.com>
> ---
>  Makefile  |    4 ++++
>  configure |   16 +++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 47acf3d..030619c 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
> @@ -162,8 +164,10 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
>
>  qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>
> +ifdef CONFIG_VIRTFS
>  fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y)
>  fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
> +endif

We don't need to make this conditional, we will just not put
the proxy-helper into TOOLS if we don't want to build it and
this dependency will be ignored.

>  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
>        $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
> diff --git a/configure b/configure
> index 763db24..081d720 100755
> --- a/configure
> +++ b/configure
> @@ -121,6 +121,7 @@ docs=""
>  fdt=""
>  nptl=""
>  sdl=""
> +virtfs="yes"
>  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"

This should be handled the same way as a number of other optional
components : default is "" meaning 'probe and use if possible'.
So you end up with a test like:

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

and then later just
if test "$virtfs" = "yes" ; then
   echo "CONFIG_VIRTFS=y" >> $config_host_mak
fi

-- PMM

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

[Meador Inge]

On 02/09/2012 04:39 PM, Peter Maydell wrote:

> On 7 February 2012 20:44, Meador Inge <meadori@codesourcery.com> wrote:
>> Signed-off-by: Meador Inge <meadori@codesourcery.com>
>> ---
>>  Makefile  |    4 ++++
>>  configure |   16 +++++++++++++---
>>  2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 47acf3d..030619c 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
>> @@ -162,8 +164,10 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
>>
>>  qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>>
>> +ifdef CONFIG_VIRTFS
>>  fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y)
>>  fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
>> +endif
> 
> We don't need to make this conditional, we will just not put
> the proxy-helper into TOOLS if we don't want to build it and
> this dependency will be ignored.

Fixed.

>>  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
>>        $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
>> diff --git a/configure b/configure
>> index 763db24..081d720 100755
>> --- a/configure
>> +++ b/configure
>> @@ -121,6 +121,7 @@ docs=""
>>  fdt=""
>>  nptl=""
>>  sdl=""
>> +virtfs="yes"
>>  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"
> 
> This should be handled the same way as a number of other optional
> components : default is "" meaning 'probe and use if possible'.
> So you end up with a test like:
> 
> 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
> 
> and then later just
> if test "$virtfs" = "yes" ; then
>    echo "CONFIG_VIRTFS=y" >> $config_host_mak
> fi

That's much cleaner.  Thanks for the review.

v2 patch coming up soon.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software

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

[Aneesh Kumar K.V]

On Tue, 7 Feb 2012 14:44:05 -0600, 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
> 
> Meador Inge (1):
>   ./configure: add option for disabling VirtFS
> 
>  Makefile  |    4 ++++
>  configure |   16 +++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)

I like the patch because it help to get qemu build on platforms where
the build failures are only due to virtfs. VirtFS do depend on some of
the recent linux APIs, so sometime we do break build on old Linux
distros.

Anthony any objection here ?

-aneesh

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

[Daniel P. Berrange]

On Wed, Feb 08, 2012 at 01:45:15PM +0530, Aneesh Kumar K.V wrote:
> On Tue, 7 Feb 2012 14:44:05 -0600, 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.

[snip]

> I like the patch because it help to get qemu build on platforms where
> the build failures are only due to virtfs. VirtFS do depend on some of
> the recent linux APIs, so sometime we do break build on old Linux
> distros.

Allowing build to be disabled is a fine goal in itself. I think someone
should still fix the code to be more portable though. f_frsize is a
non-standardized field in 'struct stat', so its usage should be made
conditional.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Meador Inge]

On 02/08/2012 02:15 AM, Aneesh Kumar K.V wrote:

> On Tue, 7 Feb 2012 14:44:05 -0600, 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
>>
>> Meador Inge (1):
>>   ./configure: add option for disabling VirtFS
>>
>>  Makefile  |    4 ++++
>>  configure |   16 +++++++++++++---
>>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> I like the patch because it help to get qemu build on platforms where
> the build failures are only due to virtfs. VirtFS do depend on some of
> the recent linux APIs, so sometime we do break build on old Linux
> distros.

Great.  Thanks for the review.  Can someone commit this for me?

> Anthony any objection here ?
> 
> -aneesh
> 


-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software