[Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Stefan Hajnoczi]

According to ./configure all options should have both --enable-foo and
--disable-foo:

  # Always add --enable-foo and --disable-foo command line args.
  # Distributions want to ensure that several features are compiled in, and it
  # is impossible without a --enable-foo that exits if a feature is not found.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 configure | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configure b/configure
index 7a1d08d..75b4fa5 100755
--- a/configure
+++ b/configure
@@ -787,6 +787,9 @@ for opt do
   --enable-modules)
       modules="yes"
   ;;
+  --disable-modules)
+      modules="no"
+  ;;
   --cpu=*)
   ;;
   --target-list=*) target_list="$optarg"
-- 
2.4.3

Re: [Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Fam Zheng]

On Mon, 11/02 14:06, Stefan Hajnoczi wrote:
> According to ./configure all options should have both --enable-foo and
> --disable-foo:
> 
>   # Always add --enable-foo and --disable-foo command line args.
>   # Distributions want to ensure that several features are compiled in, and it
>   # is impossible without a --enable-foo that exits if a feature is not found.

Actually the document is wrong about module support, the default is off:

> Optional features, enabled with --enable-FEATURE and
> disabled with --disable-FEATURE, default is enabled if available:

Maybe we should move it out of this section instead?

What about vnc-tls? It's described in the same place, but there is no option
for it.

Fam

Re: [Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Tue, Nov 03, 2015 at 08:24:47AM +0800, Fam Zheng wrote:
> On Mon, 11/02 14:06, Stefan Hajnoczi wrote:
> Actually the document is wrong about module support, the default is off:

These issues are independent of this patch, but...

> > Optional features, enabled with --enable-FEATURE and
> > disabled with --disable-FEATURE, default is enabled if available:
> 
> Maybe we should move it out of this section instead?

There are other foo="no" default options in this section.

I think "default is enabled if available" really refers to foo=""
autodetection only.

> What about vnc-tls? It's described in the same place, but there is no option
> for it.

Regarding vnc-tls, it was removed in
3e305e4a4752f70c0b5c3cf5b43ec957881714f7.  Maybe Dan can send a
follow-up patch that removes it from the ./configure --help output?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Daniel P. Berrange]

On Tue, Nov 03, 2015 at 11:22:13AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 03, 2015 at 08:24:47AM +0800, Fam Zheng wrote:
> > On Mon, 11/02 14:06, Stefan Hajnoczi wrote:
> > Actually the document is wrong about module support, the default is off:
> 
> These issues are independent of this patch, but...
> 
> > > Optional features, enabled with --enable-FEATURE and
> > > disabled with --disable-FEATURE, default is enabled if available:
> > 
> > Maybe we should move it out of this section instead?
> 
> There are other foo="no" default options in this section.
> 
> I think "default is enabled if available" really refers to foo=""
> autodetection only.
> 
> > What about vnc-tls? It's described in the same place, but there is no option
> > for it.
> 
> Regarding vnc-tls, it was removed in
> 3e305e4a4752f70c0b5c3cf5b43ec957881714f7.  Maybe Dan can send a
> follow-up patch that removes it from the ./configure --help output?

Sure, will do.


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] configure: add missing --disable-modules option

[Peter Maydell]

On 3 November 2015 at 11:22, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Tue, Nov 03, 2015 at 08:24:47AM +0800, Fam Zheng wrote:
>> On Mon, 11/02 14:06, Stefan Hajnoczi wrote:
>> Actually the document is wrong about module support, the default is off:
>
> These issues are independent of this patch, but...
>
>> > Optional features, enabled with --enable-FEATURE and
>> > disabled with --disable-FEATURE, default is enabled if available:
>>
>> Maybe we should move it out of this section instead?
>
> There are other foo="no" default options in this section.
>
> I think "default is enabled if available" really refers to foo=""
> autodetection only.

I think what it ought to say is something like

"Optional features: these can be enabled with --enable-FEATURE
or disabled with --disable-FEATURE. The default (unless otherwise
specified below) is to autodetect support for the feature, and
enable it if possible, and disable it if not."

Then options in the list which don't default to 'autodetect'
should say so, like 'debug-tcg' does now.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Fam Zheng]

On Tue, 11/03 11:56, Peter Maydell wrote:
> On 3 November 2015 at 11:22, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Tue, Nov 03, 2015 at 08:24:47AM +0800, Fam Zheng wrote:
> >> On Mon, 11/02 14:06, Stefan Hajnoczi wrote:
> >> Actually the document is wrong about module support, the default is off:
> >
> > These issues are independent of this patch, but...
> >
> >> > Optional features, enabled with --enable-FEATURE and
> >> > disabled with --disable-FEATURE, default is enabled if available:
> >>
> >> Maybe we should move it out of this section instead?
> >
> > There are other foo="no" default options in this section.
> >
> > I think "default is enabled if available" really refers to foo=""
> > autodetection only.
> 
> I think what it ought to say is something like
> 
> "Optional features: these can be enabled with --enable-FEATURE
> or disabled with --disable-FEATURE. The default (unless otherwise
> specified below) is to autodetect support for the feature, and
> enable it if possible, and disable it if not."
> 
> Then options in the list which don't default to 'autodetect'
> should say so, like 'debug-tcg' does now.

Sounds good to me. For this patch:

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH] configure: add missing --disable-modules option

[Peter Maydell]

On 2 November 2015 at 14:06, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> According to ./configure all options should have both --enable-foo and
> --disable-foo:
>
>   # Always add --enable-foo and --disable-foo command line args.
>   # Distributions want to ensure that several features are compiled in, and it
>   # is impossible without a --enable-foo that exits if a feature is not found.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  configure | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/configure b/configure
> index 7a1d08d..75b4fa5 100755
> --- a/configure
> +++ b/configure
> @@ -787,6 +787,9 @@ for opt do
>    --enable-modules)
>        modules="yes"
>    ;;
> +  --disable-modules)
> +      modules="no"
> +  ;;
>    --cpu=*)
>    ;;
>    --target-list=*) target_list="$optarg"
> --
> 2.4.3

Applied to master, thanks.

-- PMM