Re: [Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
On Mon, Feb 04, 2019 at 08:48:14PM -0500, Michael S. Tsirkin wrote: > On Mon, Feb 04, 2019 at 03:29:21PM +, Daniel P. Berrangé wrote: > > On Mon, Feb 04, 2019 at 10:19:42AM -0500, Michael S. Tsirkin wrote: > > > Hmm I do think we want to build the contrib tools, > > > otherwise they bitrot too quickly, witness follow-up > > > patches that fix the compilation. > > > > > > And I think we need tests that actually use them. > > > > > > However I agree adding them to tools and installing > > > is probably rushing things, e.g. there's no > > > manpage even. > > > > Yes, if we build, but do not install, the binary that would be ok > > > > > > Regards, > > Daniel > > OK I reverted this for now. Yes, I requested this patch to avoid further bitrot. Installing isn't necessary though. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
On Mon, Feb 04, 2019 at 03:29:21PM +, Daniel P. Berrangé wrote: > On Mon, Feb 04, 2019 at 10:19:42AM -0500, Michael S. Tsirkin wrote: > > Hmm I do think we want to build the contrib tools, > > otherwise they bitrot too quickly, witness follow-up > > patches that fix the compilation. > > > > And I think we need tests that actually use them. > > > > However I agree adding them to tools and installing > > is probably rushing things, e.g. there's no > > manpage even. > > Yes, if we build, but do not install, the binary that would be ok > > > Regards, > Daniel OK I reverted this for now. > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
On Mon, Feb 04, 2019 at 10:19:42AM -0500, Michael S. Tsirkin wrote: > Hmm I do think we want to build the contrib tools, > otherwise they bitrot too quickly, witness follow-up > patches that fix the compilation. > > And I think we need tests that actually use them. > > However I agree adding them to tools and installing > is probably rushing things, e.g. there's no > manpage even. Yes, if we build, but do not install, the binary that would be ok Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
No explanation of /why/ we want to build this by default ? The source header calls it a demo application and it has no man page. Given this IMHO we should *not* be building & installing it by default, as doing so defacto turns it into a user tool we have to support. On Mon, Feb 04, 2019 at 09:43:48AM -0500, Michael S. Tsirkin wrote: > From: Changpeng Liu > > Signed-off-by: Changpeng Liu > Reviewed-by: Stefan Hajnoczi > Reviewed-by: Stefano Garzarella > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > --- > configure | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/configure b/configure > index 31cf6f584d..5c619d4e03 100755 > --- a/configure > +++ b/configure > @@ -5831,6 +5831,9 @@ if test "$want_tools" = "yes" ; then >if [ "$posix" = "yes" ] && [ "$curl" = "yes" ]; then > tools="elf2dmp $tools" >fi > + if [ "$linux" = "yes" ]; then > +tools="vhost-user-blk\$(EXESUF) $tools" > + fi > fi > if test "$softmmu" = yes ; then >if test "$linux" = yes; then > -- > MST > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
Hmm I do think we want to build the contrib tools, otherwise they bitrot too quickly, witness follow-up patches that fix the compilation. And I think we need tests that actually use them. However I agree adding them to tools and installing is probably rushing things, e.g. there's no manpage even. Changpeng Liu could you post a patch that moves this away from tools, so it builds but isn't installed? If it's tricky I think I will revert this one for now .. On Mon, Feb 04, 2019 at 03:07:48PM +, Daniel P. Berrangé wrote: > > No explanation of /why/ we want to build this by default ? > > The source header calls it a demo application and it has no man > page. > > Given this IMHO we should *not* be building & installing it by > default, as doing so defacto turns it into a user tool we have > to support. > > > On Mon, Feb 04, 2019 at 09:43:48AM -0500, Michael S. Tsirkin wrote: > > From: Changpeng Liu > > > > Signed-off-by: Changpeng Liu > > Reviewed-by: Stefan Hajnoczi > > Reviewed-by: Stefano Garzarella > > Reviewed-by: Michael S. Tsirkin > > Signed-off-by: Michael S. Tsirkin > > --- > > configure | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/configure b/configure > > index 31cf6f584d..5c619d4e03 100755 > > --- a/configure > > +++ b/configure > > @@ -5831,6 +5831,9 @@ if test "$want_tools" = "yes" ; then > >if [ "$posix" = "yes" ] && [ "$curl" = "yes" ]; then > > tools="elf2dmp $tools" > >fi > > + if [ "$linux" = "yes" ]; then > > +tools="vhost-user-blk\$(EXESUF) $tools" > > + fi > > fi > > if test "$softmmu" = yes ; then > >if test "$linux" = yes; then > > -- > > MST > > > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PULL 15/25] contrib: compile vhost-user-blk tool by default
From: Changpeng Liu Signed-off-by: Changpeng Liu Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- configure | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure b/configure index 31cf6f584d..5c619d4e03 100755 --- a/configure +++ b/configure @@ -5831,6 +5831,9 @@ if test "$want_tools" = "yes" ; then if [ "$posix" = "yes" ] && [ "$curl" = "yes" ]; then tools="elf2dmp $tools" fi + if [ "$linux" = "yes" ]; then +tools="vhost-user-blk\$(EXESUF) $tools" + fi fi if test "$softmmu" = yes ; then if test "$linux" = yes; then -- MST