[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH-for-5.2 v3] configure: Check vhost-user is available for vhos
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH-for-5.2 v3] configure: Check vhost-user is available for vhost-user-blk-server |
Date: |
Tue, 10 Nov 2020 08:54:19 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
On 11/10/20 4:15 AM, Thomas Huth wrote:
> On 09/11/2020 20.15, Philippe Mathieu-Daudé wrote:
>> On 11/9/20 7:52 PM, Philippe Mathieu-Daudé wrote:
>>> On 11/9/20 7:44 PM, Thomas Huth wrote:
>>>> On 09/11/2020 19.41, Philippe Mathieu-Daudé wrote:
>>>>> On Mon, Nov 9, 2020 at 6:22 PM Thomas Huth <thuth@redhat.com> wrote:
>>>>>> On 09/11/2020 15.07, Philippe Mathieu-Daudé wrote:
>>>>>>> Check vhost-user is available when building vhost-user-blk-server.
>>>>>>>
>>>>>>> This fixes:
>>>>>>>
>>>>>>> $ ../configure \
>>>>>>> --disable-vhost-user --enable-vhost-user-blk-server && \
>>>>>>> make qemu-nbd
>>>>>>> ...
>>>>>>> [505/505] Linking target qemu-nbd
>>>>>>> FAILED: qemu-nbd
>>>>>>> cc -o qemu-nbd qemu-nbd.p/qemu-nbd.c.o -Wl,--as-needed
>>>>>>> -Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa libblock.fa
>>>>>>> libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--no-whole-archive
>>>>>>> -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m64 -fstack-protector-strong
>>>>>>> -Wl,--start-group libqemuutil.a libblockdev.fa libblock.fa libcrypto.fa
>>>>>>> libauthz.fa libqom.fa libio.fa @block.syms -lgio-2.0 -lgobject-2.0
>>>>>>> -lglib-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -pthread -lgnutls -lutil
>>>>>>> -lm -lgthread-2.0 -lglib-2.0 -lbz2 -lgthread-2.0 -lglib-2.0 -lssh -lrbd
>>>>>>> -lrados -lcurl -lxml2 -lzstd -lacl -lgfapi -lglusterfs -lgfrpc -lgfxdr
>>>>>>> -luuid -laio /usr/lib64/libz.so -L/usr/lib64/iscsi -liscsi -lnettle
>>>>>>> -lgnutls -lpam -Wl,--end-group
>>>>>>> /usr/bin/ld: libblockdev.fa(block_export_vhost-user-blk-server.c.o):
>>>>>>> in function `vu_blk_process_vq':
>>>>>>> block/export/vhost-user-blk-server.c:203: undefined reference to
>>>>>>> `vu_get_queue'
>>>>>>> /usr/bin/ld: block/export/vhost-user-blk-server.c:208: undefined
>>>>>>> reference to `vu_queue_pop'
>>>>>>> /usr/bin/ld: libblockdev.fa(block_export_vhost-user-blk-server.c.o):
>>>>>>> in function `vu_blk_queue_set_started':
>>>>>>> block/export/vhost-user-blk-server.c:228: undefined reference to
>>>>>>> `vu_get_queue'
>>>>>>> /usr/bin/ld: libblockdev.fa(block_export_vhost-user-blk-server.c.o):
>>>>>>> in function `vu_blk_req_complete':
>>>>>>> block/export/vhost-user-blk-server.c:55: undefined reference to
>>>>>>> `vu_queue_push'
>>>>>>> /usr/bin/ld: block/export/vhost-user-blk-server.c:56: undefined
>>>>>>> reference to `vu_queue_notify'
>>>>>>> /usr/bin/ld: libblockdev.fa(block_export_vhost-user-blk-server.c.o):
>>>>>>> in function `vu_blk_queue_set_started':
>>>>>>> block/export/vhost-user-blk-server.c:229: undefined reference to
>>>>>>> `vu_set_queue_handler'
>>>>>>> /usr/bin/ld: libqemuutil.a(util_vhost-user-server.c.o): in function
>>>>>>> `vu_client_trip':
>>>>>>> util/vhost-user-server.c:176: undefined reference to `vu_dispatch'
>>>>>>> /usr/bin/ld: util/vhost-user-server.c:180: undefined reference to
>>>>>>> `vu_deinit'
>>>>>>> /usr/bin/ld: libqemuutil.a(util_vhost-user-server.c.o): in function
>>>>>>> `vu_accept':
>>>>>>> util/vhost-user-server.c:291: undefined reference to `vu_init'
>>>>>>> collect2: error: ld returned 1 exit status
>>>>>>> ninja: build stopped: subcommand failed.
>>>>>>> make: *** [Makefile:171: run-ninja] Error 1
>>>>>>>
>>>>>>> Now we get:
>>>>>>>
>>>>>>> $ ../configure \
>>>>>>> --disable-vhost-user --enable-vhost-user-blk-server && \
>>>>>>> ERROR: --enable-vhost-user-blk-server requires --enable-vhost-user
>>>>>>>
>>>>>>> Fixes: bc15e44cb21 ("configure: introduce
>>>>>>> --enable-vhost-user-blk-server")
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>> ---
>>>>>>> Since v1:
>>>>>>> - Addressed Thomas review comments
>>>>>>> https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02188.html
>>>>>>> ---
>>>>>>> configure | 4 ++++
>>>>>>> 1 file changed, 4 insertions(+)
>>>>>>>
>>>>>>> diff --git a/configure b/configure
>>>>>>> index 805f7791503..3124be82da1 100755
>>>>>>> --- a/configure
>>>>>>> +++ b/configure
>>>>>>> @@ -2375,6 +2375,10 @@ test "$vhost_user_fs" = "" &&
>>>>>>> vhost_user_fs=$vhost_user
>>>>>>> if test "$vhost_user_fs" = "yes" && test "$vhost_user" = "no"; then
>>>>>>> error_exit "--enable-vhost-user-fs requires --enable-vhost-user"
>>>>>>> fi
>>>>>>> +test "$vhost_user_blk_server" = "" && vhost_user_blk_server=$vhost_user
>>>>>>> +if test "$vhost_user_blk_server" = "yes" && test "$vhost_user" = "no";
>>>>>>> then
>>>>>>> + error_exit "--enable-vhost-user-blk-server requires
>>>>>>> --enable-vhost-user"
>>>>>>> +fi
>>>>>>
>>>>>> A little bit later in this file, we've got :
>>>>>>
>>>>>> # libvhost-user is Linux-only
>>>>>> test "$vhost_user_blk_server" = "" && vhost_user_blk_server=$linux
>>>>>> if test "$vhost_user_blk_server" = "yes" && test "$linux" = "no"; then
>>>>>> error_exit "--enable-vhost-user-blk-server is only available on Linux"
>>>>>> fi
>>>>>>
>>>>>> I think it would be better to add the new code there instead.
>>>>>> Also maybe better do something like:
>>>>>>
>>>>>> test "$vhost_user_blk_server" = "" && test "$vhost_user" = "yes" \
>>>>>> && vhost_user_blk_server=$linux
>>>>>>
>>>>>> instead?
>>>>>
>>>>> I understood this option is similar to the other vhost-user related ones
>>>>> and should be treated the same way.
>>>>
>>>> But the second check does not make much sense anymore after your change,
>>>> since "$vhost_user_blk_server" is certainly not empty anymore. So the two
>>>> checks IMHO have to be combined.
>>>
>>> Ah now I see it... You are right, thanks for the review :)
>>
>> So we are back to v2:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02333.html
>
> That looks wrong to me, too, since the default value is now $vhost_user and
> not $linux ... so it could get enabled by default on non-Linux systems, too.
I'm giving up. Let see if Alex wants this fixed (as he added the job)
or if we disable/remove the job if nobody worries about this config.
>
> Thomas
>