Patchwork [trivial] configure: explicitly disable virtfs if softmmu=no

login
register
mail settings
Submitter Mohan Kumar M
Date June 11, 2013, 5:23 p.m.
Message ID <87ehc8tvd5.fsf@gmail.com>
Download mbox | patch
Permalink /patch/250579/
State New
Headers show

Comments

Mohan Kumar M - June 11, 2013, 5:23 p.m.
Peter Maydell <peter.maydell@linaro.org> writes:

How about this approach?

[PATCH] configure: Disable virtfs if softmmu not enabled

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 configure | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
Michael Tokarev - June 11, 2013, 6:19 p.m.
11.06.2013 21:23, M. Mohan Kumar wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> How about this approach?

Well, this is definitely wrong :)

> -if test "$softmmu" = yes ; then
> -  if test "$virtfs" != no ; then
> +
> +if test "$virtfs" != no ; then
> +  if test "$softmmu" = yes ; then
>      if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; then
>        virtfs=yes
>        tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
> @@ -3415,6 +3416,12 @@ if test "$softmmu" = yes ; then
>        fi
>        virtfs=no
>      fi
> +  else
> +    if test "$virtfs" = yes; then
> +      error_exit "VirtFS is supported only on Linux and requires softmmu"
> +    else
> +     virtfs=no
> +    fi
>    fi
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>      if [ "$guest_agent" = "yes" ]; then

Now this "if [ $linux..." test is only checked
if $virtfs != no.  Before, it was checked when
$softmmu != no...

FWIW, I still don't understand what Peter Maydell dislikes
in a simplest case I posted initially, where we merely ignore
(disable) virtfs in case !softmmu.  We should probably do the
same for alot of other features which makes sense only if
softmmu==yes, and omit many configure tests which are still
done even if softmmu is disabled, but that's a different
patch for sure.  Maube we should separate out this last linux|bsd|solaris
test and add another if softmmu there, for readability, so that
disabling of virtfs will be closer to other virtfs tests.

I applied my initial patch to our debian tree to fix build
failure for now, because else it fails during build.

Thanks,

/mjt
Peter Maydell - June 11, 2013, 7:21 p.m.
On 11 June 2013 19:19, Michael Tokarev <mjt@tls.msk.ru> wrote:
> FWIW, I still don't understand what Peter Maydell dislikes
> in a simplest case I posted initially, where we merely ignore
> (disable) virtfs in case !softmmu.

It just seems to me that rather than fixing a bug in the
makefile (it still tries to build docs for the tools even
when the tools aren't being built) you're trying to tweak
the configure script to avoid generating the combinations
of config values that trigger the makefile bug.

-- PMM
Mohan Kumar M - June 12, 2013, 5:26 a.m.
Michael Tokarev <mjt@tls.msk.ru> writes:

> 11.06.2013 21:23, M. Mohan Kumar wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> 
>> How about this approach?
>
> Well, this is definitely wrong :)
>
>> -if test "$softmmu" = yes ; then
>> -  if test "$virtfs" != no ; then
>> +
>> +if test "$virtfs" != no ; then
>> +  if test "$softmmu" = yes ; then
>>      if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; then
>>        virtfs=yes
>>        tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
>> @@ -3415,6 +3416,12 @@ if test "$softmmu" = yes ; then
>>        fi
>>        virtfs=no
>>      fi
>> +  else
>> +    if test "$virtfs" = yes; then
>> +      error_exit "VirtFS is supported only on Linux and requires softmmu"
>> +    else
>> +     virtfs=no
>> +    fi
>>    fi
>>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>>      if [ "$guest_agent" = "yes" ]; then
>
> Now this "if [ $linux..." test is only checked
> if $virtfs != no.  Before, it was checked when
> $softmmu != no...
My bad :(, I missed check for guest_agent inside softmmu case.

>
> FWIW, I still don't understand what Peter Maydell dislikes
> in a simplest case I posted initially, where we merely ignore
> (disable) virtfs in case !softmmu.  We should probably do the
> same for alot of other features which makes sense only if
> softmmu==yes, and omit many configure tests which are still
> done even if softmmu is disabled, but that's a different
> patch for sure.  Maube we should separate out this last linux|bsd|solaris
> test and add another if softmmu there, for readability, so that
> disabling of virtfs will be closer to other virtfs tests.
>
> I applied my initial patch to our debian tree to fix build
> failure for now, because else it fails during build.
>
> Thanks,
>
> /mjt

Patch

diff --git a/configure b/configure
index 1654413..88c2b0f 100755
--- a/configure
+++ b/configure
@@ -3404,8 +3404,9 @@  if test "$want_tools" = "yes" ; then
     tools="qemu-nbd\$(EXESUF) $tools"
   fi
 fi
-if test "$softmmu" = yes ; then
-  if test "$virtfs" != no ; then
+
+if test "$virtfs" != no ; then
+  if test "$softmmu" = yes ; then
     if test "$cap" = yes && test "$linux" = yes && test "$attr" = yes ; then
       virtfs=yes
       tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
@@ -3415,6 +3416,12 @@  if test "$softmmu" = yes ; then
       fi
       virtfs=no
     fi
+  else
+    if test "$virtfs" = yes; then
+      error_exit "VirtFS is supported only on Linux and requires softmmu"
+    else
+     virtfs=no
+    fi
   fi
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
     if [ "$guest_agent" = "yes" ]; then