| Message ID | 20200903153524.98168-1-brogers@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | meson: install ivshmem-client and ivshmem-server | expand |
Hi On Thu, Sep 3, 2020 at 7:35 PM Bruce Rogers <brogers@suse.com> wrote: > > Turn on the meson install flag for these executables > > Signed-off-by: Bruce Rogers <brogers@suse.com> I am not sure we want to install binaries that don't even have maintainers (or am I wrong?). > --- > contrib/ivshmem-client/meson.build | 2 +- > contrib/ivshmem-server/meson.build | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build > index 1b171efb4f..83a559117f 100644 > --- a/contrib/ivshmem-client/meson.build > +++ b/contrib/ivshmem-client/meson.build > @@ -1,4 +1,4 @@ > executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), > dependencies: glib, > build_by_default: targetos == 'linux', > - install: false) > + install: true) > diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build > index 3a53942201..a1c39aa3b3 100644 > --- a/contrib/ivshmem-server/meson.build > +++ b/contrib/ivshmem-server/meson.build > @@ -1,4 +1,4 @@ > executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), > dependencies: [qemuutil, rt], > build_by_default: targetos == 'linux', > - install: false) > + install: true) > -- > 2.28.0 >
On 03/09/20 17:35, Bruce Rogers wrote: > Turn on the meson install flag for these executables > > Signed-off-by: Bruce Rogers <brogers@suse.com> > --- > contrib/ivshmem-client/meson.build | 2 +- > contrib/ivshmem-server/meson.build | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build > index 1b171efb4f..83a559117f 100644 > --- a/contrib/ivshmem-client/meson.build > +++ b/contrib/ivshmem-client/meson.build > @@ -1,4 +1,4 @@ > executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), > dependencies: glib, > build_by_default: targetos == 'linux', > - install: false) > + install: true) > diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build > index 3a53942201..a1c39aa3b3 100644 > --- a/contrib/ivshmem-server/meson.build > +++ b/contrib/ivshmem-server/meson.build > @@ -1,4 +1,4 @@ > executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), > dependencies: [qemuutil, rt], > build_by_default: targetos == 'linux', > - install: false) > + install: true) > They weren't installed before the conversion, were they? Paolo
On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: > On 03/09/20 17:35, Bruce Rogers wrote: > > Turn on the meson install flag for these executables > > > > Signed-off-by: Bruce Rogers <brogers@suse.com> > > --- > > contrib/ivshmem-client/meson.build | 2 +- > > contrib/ivshmem-server/meson.build | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build > > index 1b171efb4f..83a559117f 100644 > > --- a/contrib/ivshmem-client/meson.build > > +++ b/contrib/ivshmem-client/meson.build > > @@ -1,4 +1,4 @@ > > executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), > > dependencies: glib, > > build_by_default: targetos == 'linux', > > - install: false) > > + install: true) > > diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build > > index 3a53942201..a1c39aa3b3 100644 > > --- a/contrib/ivshmem-server/meson.build > > +++ b/contrib/ivshmem-server/meson.build > > @@ -1,4 +1,4 @@ > > executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), > > dependencies: [qemuutil, rt], > > build_by_default: targetos == 'linux', > > - install: false) > > + install: true) > > > > They weren't installed before the conversion, were they? Unfortunately they were historically installed with --enable-tools. IMHO nothing in contrib/ should be installed by default. If we consider something maintained and high enough quality for end users, it should migrate out of contrib/ into tools/. vhost-user-gpu and elf2dmp are also both in contrib/ but installed by default :-( Regards, Daniel
On Thu, 2020-09-03 at 16:49 +0100, Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: > > On 03/09/20 17:35, Bruce Rogers wrote: > > > Turn on the meson install flag for these executables > > > > > > Signed-off-by: Bruce Rogers <brogers@suse.com> > > > --- > > > contrib/ivshmem-client/meson.build | 2 +- > > > contrib/ivshmem-server/meson.build | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/contrib/ivshmem-client/meson.build > > > b/contrib/ivshmem-client/meson.build > > > index 1b171efb4f..83a559117f 100644 > > > --- a/contrib/ivshmem-client/meson.build > > > +++ b/contrib/ivshmem-client/meson.build > > > @@ -1,4 +1,4 @@ > > > executable('ivshmem-client', files('ivshmem-client.c', > > > 'main.c'), > > > dependencies: glib, > > > build_by_default: targetos == 'linux', > > > - install: false) > > > + install: true) > > > diff --git a/contrib/ivshmem-server/meson.build > > > b/contrib/ivshmem-server/meson.build > > > index 3a53942201..a1c39aa3b3 100644 > > > --- a/contrib/ivshmem-server/meson.build > > > +++ b/contrib/ivshmem-server/meson.build > > > @@ -1,4 +1,4 @@ > > > executable('ivshmem-server', files('ivshmem-server.c', > > > 'main.c'), > > > dependencies: [qemuutil, rt], > > > build_by_default: targetos == 'linux', > > > - install: false) > > > + install: true) > > > > > > > They weren't installed before the conversion, were they? > > Unfortunately they were historically installed with --enable-tools. > Yeah, that's how I was getting them installed in what we are packaging up. - Bruce
On 03/09/20 17:49, Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: >> On 03/09/20 17:35, Bruce Rogers wrote: >>> Turn on the meson install flag for these executables >>> >>> Signed-off-by: Bruce Rogers <brogers@suse.com> >>> --- >>> contrib/ivshmem-client/meson.build | 2 +- >>> contrib/ivshmem-server/meson.build | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build >>> index 1b171efb4f..83a559117f 100644 >>> --- a/contrib/ivshmem-client/meson.build >>> +++ b/contrib/ivshmem-client/meson.build >>> @@ -1,4 +1,4 @@ >>> executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), >>> dependencies: glib, >>> build_by_default: targetos == 'linux', >>> - install: false) >>> + install: true) >>> diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build >>> index 3a53942201..a1c39aa3b3 100644 >>> --- a/contrib/ivshmem-server/meson.build >>> +++ b/contrib/ivshmem-server/meson.build >>> @@ -1,4 +1,4 @@ >>> executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), >>> dependencies: [qemuutil, rt], >>> build_by_default: targetos == 'linux', >>> - install: false) >>> + install: true) >>> >> >> They weren't installed before the conversion, were they? > > Unfortunately they were historically installed with --enable-tools. > > IMHO nothing in contrib/ should be installed by default. If we consider > something maintained and high enough quality for end users, it should > migrate out of contrib/ into tools/. Ah, via the tools variable in configure. Nice maze. I've queued the patch. Paolo > vhost-user-gpu and elf2dmp are also both in contrib/ but installed > by default :-( > > > Regards, > Daniel >
Paolo Bonzini <pbonzini@redhat.com> writes: > On 03/09/20 17:49, Daniel P. Berrangé wrote: >> On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: >>> On 03/09/20 17:35, Bruce Rogers wrote: >>>> Turn on the meson install flag for these executables >>>> >>>> Signed-off-by: Bruce Rogers <brogers@suse.com> >>>> --- >>>> contrib/ivshmem-client/meson.build | 2 +- >>>> contrib/ivshmem-server/meson.build | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build >>>> index 1b171efb4f..83a559117f 100644 >>>> --- a/contrib/ivshmem-client/meson.build >>>> +++ b/contrib/ivshmem-client/meson.build >>>> @@ -1,4 +1,4 @@ >>>> executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), >>>> dependencies: glib, >>>> build_by_default: targetos == 'linux', >>>> - install: false) >>>> + install: true) >>>> diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build >>>> index 3a53942201..a1c39aa3b3 100644 >>>> --- a/contrib/ivshmem-server/meson.build >>>> +++ b/contrib/ivshmem-server/meson.build >>>> @@ -1,4 +1,4 @@ >>>> executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), >>>> dependencies: [qemuutil, rt], >>>> build_by_default: targetos == 'linux', >>>> - install: false) >>>> + install: true) >>>> >>> >>> They weren't installed before the conversion, were they? >> >> Unfortunately they were historically installed with --enable-tools. >> >> IMHO nothing in contrib/ should be installed by default. If we consider >> something maintained and high enough quality for end users, it should >> migrate out of contrib/ into tools/. > > Ah, via the tools variable in configure. Nice maze. I've queued the patch. Please don't. These programs are examples to help people understand how the ivhsmem device works. They might even be useful for debugging. They are *not* to be used in production. docs/specs/ivshmem-spec.txt: Example server code is in contrib/ivshmem-server/. Not to be used in production. It assumes all clients use the same number of interrupt vectors. A standalone client is in contrib/ivshmem-client/. It can be useful for debugging. Installing example *source* might make sense (but I doubt it). Installing the binaries does not.
On 04/09/20 09:24, Markus Armbruster wrote: >> Ah, via the tools variable in configure. Nice maze. I've queued the patch. > Please don't. > > These programs are examples to help people understand how the ivhsmem > device works. They might even be useful for debugging. They are *not* > to be used in production. Oooookay. :) It needs to be documented in the changelog. Paolo > docs/specs/ivshmem-spec.txt: > > Example server code is in contrib/ivshmem-server/. Not to be used in > production. It assumes all clients use the same number of interrupt > vectors. > > A standalone client is in contrib/ivshmem-client/. It can be useful > for debugging. > > Installing example *source* might make sense (but I doubt it). > Installing the binaries does not. >
Paolo Bonzini <pbonzini@redhat.com> writes: > On 04/09/20 09:24, Markus Armbruster wrote: >>> Ah, via the tools variable in configure. Nice maze. I've queued the patch. >> Please don't. >> >> These programs are examples to help people understand how the ivhsmem >> device works. They might even be useful for debugging. They are *not* >> to be used in production. > > Oooookay. :) It needs to be documented in the changelog. I'll take care of updating <https://wiki.qemu.org/ChangeLog/5.2>.
On 9/4/20 9:24 AM, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> On 03/09/20 17:49, Daniel P. Berrangé wrote: >>> On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: >>>> On 03/09/20 17:35, Bruce Rogers wrote: >>>>> Turn on the meson install flag for these executables >>>>> >>>>> Signed-off-by: Bruce Rogers <brogers@suse.com> >>>>> --- >>>>> contrib/ivshmem-client/meson.build | 2 +- >>>>> contrib/ivshmem-server/meson.build | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build >>>>> index 1b171efb4f..83a559117f 100644 >>>>> --- a/contrib/ivshmem-client/meson.build >>>>> +++ b/contrib/ivshmem-client/meson.build >>>>> @@ -1,4 +1,4 @@ >>>>> executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), >>>>> dependencies: glib, >>>>> build_by_default: targetos == 'linux', >>>>> - install: false) >>>>> + install: true) >>>>> diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build >>>>> index 3a53942201..a1c39aa3b3 100644 >>>>> --- a/contrib/ivshmem-server/meson.build >>>>> +++ b/contrib/ivshmem-server/meson.build >>>>> @@ -1,4 +1,4 @@ >>>>> executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), >>>>> dependencies: [qemuutil, rt], >>>>> build_by_default: targetos == 'linux', >>>>> - install: false) >>>>> + install: true) >>>>> >>>> >>>> They weren't installed before the conversion, were they? >>> >>> Unfortunately they were historically installed with --enable-tools. >>> >>> IMHO nothing in contrib/ should be installed by default. If we consider >>> something maintained and high enough quality for end users, it should >>> migrate out of contrib/ into tools/. >> >> Ah, via the tools variable in configure. Nice maze. I've queued the patch. > > Please don't. > > These programs are examples to help people understand how the ivhsmem > device works. They might even be useful for debugging. They are *not* > to be used in production. > > docs/specs/ivshmem-spec.txt: > > Example server code is in contrib/ivshmem-server/. Not to be used in > production. It assumes all clients use the same number of interrupt > vectors. > > A standalone client is in contrib/ivshmem-client/. It can be useful > for debugging. Renaming the file names to contain "example" would make this clear. > > Installing example *source* might make sense (but I doubt it). > Installing the binaries does not. > >
On Fri, Sep 04, 2020 at 11:37:58AM +0200, Philippe Mathieu-Daudé wrote: > On 9/4/20 9:24 AM, Markus Armbruster wrote: > > Paolo Bonzini <pbonzini@redhat.com> writes: > > > >> On 03/09/20 17:49, Daniel P. Berrangé wrote: > >>> On Thu, Sep 03, 2020 at 05:44:25PM +0200, Paolo Bonzini wrote: > >>>> On 03/09/20 17:35, Bruce Rogers wrote: > >>>>> Turn on the meson install flag for these executables > >>>>> > >>>>> Signed-off-by: Bruce Rogers <brogers@suse.com> > >>>>> --- > >>>>> contrib/ivshmem-client/meson.build | 2 +- > >>>>> contrib/ivshmem-server/meson.build | 2 +- > >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build > >>>>> index 1b171efb4f..83a559117f 100644 > >>>>> --- a/contrib/ivshmem-client/meson.build > >>>>> +++ b/contrib/ivshmem-client/meson.build > >>>>> @@ -1,4 +1,4 @@ > >>>>> executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), > >>>>> dependencies: glib, > >>>>> build_by_default: targetos == 'linux', > >>>>> - install: false) > >>>>> + install: true) > >>>>> diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build > >>>>> index 3a53942201..a1c39aa3b3 100644 > >>>>> --- a/contrib/ivshmem-server/meson.build > >>>>> +++ b/contrib/ivshmem-server/meson.build > >>>>> @@ -1,4 +1,4 @@ > >>>>> executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), > >>>>> dependencies: [qemuutil, rt], > >>>>> build_by_default: targetos == 'linux', > >>>>> - install: false) > >>>>> + install: true) > >>>>> > >>>> > >>>> They weren't installed before the conversion, were they? > >>> > >>> Unfortunately they were historically installed with --enable-tools. > >>> > >>> IMHO nothing in contrib/ should be installed by default. If we consider > >>> something maintained and high enough quality for end users, it should > >>> migrate out of contrib/ into tools/. > >> > >> Ah, via the tools variable in configure. Nice maze. I've queued the patch. > > > > Please don't. > > > > These programs are examples to help people understand how the ivhsmem > > device works. They might even be useful for debugging. They are *not* > > to be used in production. > > > > docs/specs/ivshmem-spec.txt: > > > > Example server code is in contrib/ivshmem-server/. Not to be used in > > production. It assumes all clients use the same number of interrupt > > vectors. > > > > A standalone client is in contrib/ivshmem-client/. It can be useful > > for debugging. > > Renaming the file names to contain "example" would make this clear. That's essentially what 'contrib' already means. We shold probablt add a contrib/README.rst file though to explicitly tell people what we consider the state of these things to be. Regards, Daniel
On Fri, 2020-09-04 at 11:20 +0200, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > > > On 04/09/20 09:24, Markus Armbruster wrote: > > > > Ah, via the tools variable in configure. Nice maze. I've > > > > queued the patch. > > > Please don't. > > > > > > These programs are examples to help people understand how the > > > ivhsmem > > > device works. They might even be useful for debugging. They are > > > *not* > > > to be used in production. > > > > Oooookay. :) It needs to be documented in the changelog. > > I'll take care of updating <https://wiki.qemu.org/ChangeLog/5.2>;. > Thanks all for the clarification on this one. It was packaged up simply because it got installed, and I never got to looking at the use case for these binaries. I'll remove them from what we package in SUSE's SLE and openSUSE. - Bruce
Bruce Rogers <brogers@suse.com> writes: > On Fri, 2020-09-04 at 11:20 +0200, Markus Armbruster wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >> > On 04/09/20 09:24, Markus Armbruster wrote: >> > > > Ah, via the tools variable in configure. Nice maze. I've >> > > > queued the patch. >> > > Please don't. >> > > >> > > These programs are examples to help people understand how the >> > > ivhsmem >> > > device works. They might even be useful for debugging. They are >> > > *not* >> > > to be used in production. >> > >> > Oooookay. :) It needs to be documented in the changelog. >> >> I'll take care of updating <https://wiki.qemu.org/ChangeLog/5.2>;. >> > > Thanks all for the clarification on this one. It was packaged up simply > because it got installed, and I never got to looking at the use case > for these binaries. > > I'll remove them from what we package in SUSE's SLE and openSUSE. Thanks!
diff --git a/contrib/ivshmem-client/meson.build b/contrib/ivshmem-client/meson.build index 1b171efb4f..83a559117f 100644 --- a/contrib/ivshmem-client/meson.build +++ b/contrib/ivshmem-client/meson.build @@ -1,4 +1,4 @@ executable('ivshmem-client', files('ivshmem-client.c', 'main.c'), dependencies: glib, build_by_default: targetos == 'linux', - install: false) + install: true) diff --git a/contrib/ivshmem-server/meson.build b/contrib/ivshmem-server/meson.build index 3a53942201..a1c39aa3b3 100644 --- a/contrib/ivshmem-server/meson.build +++ b/contrib/ivshmem-server/meson.build @@ -1,4 +1,4 @@ executable('ivshmem-server', files('ivshmem-server.c', 'main.c'), dependencies: [qemuutil, rt], build_by_default: targetos == 'linux', - install: false) + install: true)
Turn on the meson install flag for these executables Signed-off-by: Bruce Rogers <brogers@suse.com> --- contrib/ivshmem-client/meson.build | 2 +- contrib/ivshmem-server/meson.build | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)