Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
Michael Tokarev m...@tls.msk.ru writes: 11.06.2013 00:47, Michael Tokarev wrote: Or else ./configure --disable-system --enable-virtfs (which makes no sense by its own but does not error out) will fail to build, because it will define CONFIG_VIRTFS, and the makefile will try to build virtfs-proxy-helper manpage (but not the executable). The build fails in this case in a separate build tree, because the fsdev directory is not created and scripts/texi2pod.pl will be called with output = fsdev/virtfs-proxy-helper.pod, which can't be created because fsdev/ does not exist. Hi, I tried ./configure --disable-system --enable-virtfs and make. But didnt face any build failure. Could you please share your build failure information? virtfs-proxy-helper.1 is created inside the fsdev folder. Cc: qemu-triv...@nongnu.org Cc: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index a3f0b7a..0ff0380 100755 --- a/configure +++ b/configure @@ -3423,6 +3423,8 @@ if test $softmmu = yes ; then tools=qemu-ga\$(EXESUF) $tools fi fi +else + virtfs=no fi # Mac OS X ships with a broken assembler
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
On 11 June 2013 10:22, M. Mohan Kumar mo...@in.ibm.com wrote: I tried ./configure --disable-system --enable-virtfs and make. But didnt face any build failure. Could you please share your build failure information? virtfs-proxy-helper.1 is created inside the fsdev folder. Michael wrote The build fails in this case in a separate build tree but your configure line suggests you were building inside the source tree. thanks -- PMM
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
11.06.2013 13:22, M. Mohan Kumar wrote: Michael Tokarev m...@tls.msk.ru writes: 11.06.2013 00:47, Michael Tokarev wrote: Or else ./configure --disable-system --enable-virtfs (which makes no sense by its own but does not error out) will fail to build, because it will define CONFIG_VIRTFS, and the makefile will try to build virtfs-proxy-helper manpage (but not the executable). The build fails in this case in a separate build tree, because ^ the fsdev directory is not created and scripts/texi2pod.pl will be called with output = fsdev/virtfs-proxy-helper.pod, which can't be created because fsdev/ does not exist. Hi, I tried ./configure --disable-system --enable-virtfs and make. But didnt face any build failure. Could you please share your build failure information? virtfs-proxy-helper.1 is created inside the fsdev folder. Well, it shouldn't be created to start with ;) But the key component to this is to build in a separate subdir: mkdir build cd build ../configure --disable-system --enable-virtfs make this will fail. It succeeds when run in the source tree because there, fsdev/ dir does exist. But in a separate build dir, when system targets aren't enabled, it isn't created. Thanks, /mjt
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
11.06.2013 01:45, Peter Maydell wrote: On 10 June 2013 21:47, Michael Tokarev m...@tls.msk.ru wrote: Or else ./configure --disable-system --enable-virtfs (which makes no sense by its own but does not error out) will fail to build, because it will define CONFIG_VIRTFS, and the makefile will try to build virtfs-proxy-helper manpage (but not the executable). Cc: qemu-triv...@nongnu.org Cc: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index a3f0b7a..0ff0380 100755 --- a/configure +++ b/configure @@ -3423,6 +3423,8 @@ if test $softmmu = yes ; then tools=qemu-ga\$(EXESUF) $tools fi fi +else + virtfs=no fi This doesn't feel to me like it's quite the right way to fix this bug. The current code in configure seems to tangle up (a) was virtfs requested and can we do it? with (b) what do we need to do if it was? (build some extra tools) and (c) when does it make sense? not for linux-user targets. So you end up with an 'else virtfs=no' clause added in an odd place. If the mess was untangled then this probably wouldn't be necessary. Um. I don't think that tangling is a bad thing really. Having different variables or options for it will be too bloated, in my opinion. I don't think there should be anything done with it. How about something like this: --- a/configure +++ b/configure @@ -3810,7 +3810,7 @@ fi if test $libattr = yes ; then echo CONFIG_LIBATTR=y $config_host_mak fi -if test $virtfs = yes ; then +if test $virtfs = yes test $target_softmmu = yes ; then echo CONFIG_VIRTFS=y $config_host_mak fi if test $vhost_scsi = yes ; then (BTW, why configure uses this test instead of more readable [ ] ? ) Also, disabling building tools and docs in general seems broken: --disable-tools disables building qemu-img, for instance, but not its documentation. So maybe we should fix this by generally making sure we don't build the docs unless we build the tool as well. This has been addressed by a separate patch sent by afaerber. Thanks, /mjt
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
On 11 June 2013 13:29, Michael Tokarev m...@tls.msk.ru wrote: 11.06.2013 01:45, Peter Maydell wrote: This doesn't feel to me like it's quite the right way to fix this bug. The current code in configure seems to tangle up (a) was virtfs requested and can we do it? with (b) what do we need to do if it was? (build some extra tools) and (c) when does it make sense? not for linux-user targets. So you end up with an 'else virtfs=no' clause added in an odd place. If the mess was untangled then this probably wouldn't be necessary. Um. I don't think that tangling is a bad thing really. Having different variables or options for it will be too bloated, in my opinion. I don't think there should be anything done with it. I don't want more variables. I just don't think we should have if not softmmu then do some other thing; just check for whether the user asked for virtfs and we can do it, and if so set virtfs=yes. How about something like this: --- a/configure +++ b/configure @@ -3810,7 +3810,7 @@ fi if test $libattr = yes ; then echo CONFIG_LIBATTR=y $config_host_mak fi -if test $virtfs = yes ; then +if test $virtfs = yes test $target_softmmu = yes ; then echo CONFIG_VIRTFS=y $config_host_mak fi This seems like a step backwards to me. virtfs=yes should just translate straight to CONFIG_VIRTFS and the makefile should just not care if it's set if we happen not to be building anything virtfs related. Also, disabling building tools and docs in general seems broken: --disable-tools disables building qemu-img, for instance, but not its documentation. So maybe we should fix this by generally making sure we don't build the docs unless we build the tool as well. This has been addressed by a separate patch sent by afaerber. That patch didn't touch anything virtfs proxy related. thanks -- PMM
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
Peter Maydell peter.mayd...@linaro.org writes: How about this approach? [PATCH] configure: Disable virtfs if softmmu not enabled Signed-off-by: M. Mohan Kumar mo...@in.ibm.com --- configure | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 -- 1.7.11.7 Tested with following configure options $ ./configure '--target-list=x86_64-linux-user' --disable-system [snip] VirtFS supportno $ ./configure '--target-list=x86_64-linux-user' --disable-system --enable-virtfs ERROR: VirtFS is supported only on Linux and requires softmmu $ ./configure '--target-list=x86_64-softmmu' --enable-virtfs [snip] VirtFS supportyes $ ./configure '--target-list=x86_64-softmmu' VirtFS supportyes On 11 June 2013 13:29, Michael Tokarev m...@tls.msk.ru wrote: 11.06.2013 01:45, Peter Maydell wrote: This doesn't feel to me like it's quite the right way to fix this bug. The current code in configure seems to tangle up (a) was virtfs requested and can we do it? with (b) what do we need to do if it was? (build some extra tools) and (c) when does it make sense? not for linux-user targets. So you end up with an 'else virtfs=no' clause added in an odd place. If the mess was untangled then this probably wouldn't be necessary. Um. I don't think that tangling is a bad thing really. Having different variables or options for it will be too bloated, in my opinion. I don't think there should be anything done with it. I don't want more variables. I just don't think we should have if not softmmu then do some other thing; just check for whether the user asked for virtfs and we can do it, and if so set virtfs=yes. How about something like this: --- a/configure +++ b/configure @@ -3810,7 +3810,7 @@ fi if test $libattr = yes ; then echo CONFIG_LIBATTR=y $config_host_mak fi -if test $virtfs = yes ; then +if test $virtfs = yes test $target_softmmu = yes ; then echo CONFIG_VIRTFS=y $config_host_mak fi This seems like a step backwards to me. virtfs=yes should just translate straight to CONFIG_VIRTFS and the makefile should just not care if it's set if we happen not to be building anything virtfs related. Also, disabling building tools and docs in general seems broken: --disable-tools disables building qemu-img, for instance, but not its documentation. So maybe we should fix this by generally making sure we don't build the docs unless we build the tool as well. This has been addressed by a separate patch sent by afaerber. That patch didn't touch anything virtfs proxy related. thanks -- PMM
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
11.06.2013 21:23, M. Mohan Kumar wrote: Peter Maydell peter.mayd...@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
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
On 11 June 2013 19:19, Michael Tokarev m...@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
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
11.06.2013 23:21, Peter Maydell пишет: On 11 June 2013 19:19, Michael Tokarev m...@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. Heh. It is just easier to not generate the config variable than to use more complex conditions. How about this: --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ 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 +ifeq ($(CONFIG_VIRTFS)$(CONFIG_SOFTMMU),yy) DOCS+=fsdev/virtfs-proxy-helper.1 endif else @@ -313,7 +313,7 @@ ifneq ($(TOOLS),) $(INSTALL_DATA) qemu-nbd.8 $(DESTDIR)$(mandir)/man8 endif endif -ifdef CONFIG_VIRTFS +ifeq ($(CONFIG_VIRTFS)$(CONFIG_SOFTMMU),yy) $(INSTALL_DIR) $(DESTDIR)$(mandir)/man1 $(INSTALL_DATA) fsdev/virtfs-proxy-helper.1 $(DESTDIR)$(mandir)/man1 endif Or this: --- a/Makefile +++ b/Makefile @@ -64,6 +64,10 @@ LIBS+=-lz $(LIBS_TOOLS) HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF) +ifneq ($(CONFIG_SOFTMMU),y) +CONFIG_VIRTFS := +endif + 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 Or this: --- a/Makefile +++ b/Makefile @@ -64,9 +64,13 @@ LIBS+=-lz $(LIBS_TOOLS) HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF) +ifeq ($(CONFIG_VIRTFS)$(CONFIG_SOFTMMU),yy) +VIRTFS_DOCS = y +endif + 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 +ifdef VIRTFS_DOCS DOCS+=fsdev/virtfs-proxy-helper.1 endif else @@ -313,7 +317,7 @@ ifneq ($(TOOLS),) $(INSTALL_DATA) qemu-nbd.8 $(DESTDIR)$(mandir)/man8 endif endif -ifdef CONFIG_VIRTFS +ifdef VIRTFS_DOCS $(INSTALL_DIR) $(DESTDIR)$(mandir)/man1 $(INSTALL_DATA) fsdev/virtfs-proxy-helper.1 $(DESTDIR)$(mandir)/man1 endif I don't care any way ;) But all that is more verbose than just turning the feature off in ./configure. Thanks, /mjt
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
Michael Tokarev m...@tls.msk.ru writes: 11.06.2013 21:23, M. Mohan Kumar wrote: Peter Maydell peter.mayd...@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
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
11.06.2013 00:47, Michael Tokarev wrote: Or else ./configure --disable-system --enable-virtfs (which makes no sense by its own but does not error out) will fail to build, because it will define CONFIG_VIRTFS, and the makefile will try to build virtfs-proxy-helper manpage (but not the executable). The build fails in this case in a separate build tree, because the fsdev directory is not created and scripts/texi2pod.pl will be called with output = fsdev/virtfs-proxy-helper.pod, which can't be created because fsdev/ does not exist. Cc: qemu-triv...@nongnu.org Cc: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index a3f0b7a..0ff0380 100755 --- a/configure +++ b/configure @@ -3423,6 +3423,8 @@ if test $softmmu = yes ; then tools=qemu-ga\$(EXESUF) $tools fi fi +else + virtfs=no fi # Mac OS X ships with a broken assembler
Re: [Qemu-devel] [PATCH trivial] configure: explicitly disable virtfs if softmmu=no
On 10 June 2013 21:47, Michael Tokarev m...@tls.msk.ru wrote: Or else ./configure --disable-system --enable-virtfs (which makes no sense by its own but does not error out) will fail to build, because it will define CONFIG_VIRTFS, and the makefile will try to build virtfs-proxy-helper manpage (but not the executable). Cc: qemu-triv...@nongnu.org Cc: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index a3f0b7a..0ff0380 100755 --- a/configure +++ b/configure @@ -3423,6 +3423,8 @@ if test $softmmu = yes ; then tools=qemu-ga\$(EXESUF) $tools fi fi +else + virtfs=no fi This doesn't feel to me like it's quite the right way to fix this bug. The current code in configure seems to tangle up (a) was virtfs requested and can we do it? with (b) what do we need to do if it was? (build some extra tools) and (c) when does it make sense? not for linux-user targets. So you end up with an 'else virtfs=no' clause added in an odd place. If the mess was untangled then this probably wouldn't be necessary. Also, disabling building tools and docs in general seems broken: --disable-tools disables building qemu-img, for instance, but not its documentation. So maybe we should fix this by generally making sure we don't build the docs unless we build the tool as well. thanks -- PMM
