On 28.09.2018 15:13, Bruce Richardson wrote:
> On Fri, Sep 28, 2018 at 01:40:25PM +0300, Ilya Maximets wrote:
>> On 27.09.2018 11:28, Ilya Maximets wrote:
>>> On 26.09.2018 10:26, Maxime Coquelin wrote:
>>>> This patch opens a userfaultfd and sends it back to Qemu's
>>>> VHOST_USER_POSTCOPY_ADVISE request.
>>>>
>>>> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>>>> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
>>>> ---
>>>> lib/librte_vhost/vhost.h | 2 ++
>>>> lib/librte_vhost/vhost_user.c | 37 +++++++++++++++++++++++++++++++++++
>>>> lib/librte_vhost/vhost_user.h | 3 ++-
>>>> 3 files changed, 41 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
>>>> index 25ffd7614..21722d8a8 100644
>>>> --- a/lib/librte_vhost/vhost.h
>>>> +++ b/lib/librte_vhost/vhost.h
>>>> @@ -363,6 +363,8 @@ struct virtio_net {
>>>> int slave_req_fd;
>>>> rte_spinlock_t slave_req_lock;
>>>>
>>>> + int postcopy_ufd;
>>>> +
>>>> /*
>>>> * Device id to identify a specific backend device.
>>>> * It's set to -1 for the default software implementation.
>>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>>>> index a9b429598..bdfe2cac0 100644
>>>> --- a/lib/librte_vhost/vhost_user.c
>>>> +++ b/lib/librte_vhost/vhost_user.c
>>>> @@ -24,9 +24,13 @@
>>>> #include <stdlib.h>
>>>> #include <string.h>
>>>> #include <unistd.h>
>>>> +#include <fcntl.h>
>>>> +#include <linux/userfaultfd.h>
>>>
>>> Maybe we need compile time check for this header existence?
>>> Otherwise, this will bump minimal kernel version for default linux build
>>> to something like 4.3.
>>
>> We'll need a config option here (disabled by default) and guard all
>> the postcopy related code.
>> Meson build will be able to detect the header file and enable
>> the config if possible. Like this:
>>
>> lib/librte_vhost/meson.build:
>> if cc.has_header('linux/userfaultfd.h')
>> dpdk_conf.set10('RTE_LIBRTE_VHOST_POSTCOPY', true)
>
> Are you sure you want 'set10' rather than 'set'. Set is probably easier
> because it ensures no define on false, while set10 has a define of 0. This
> has caught me out before.
>
> FYI, you can also avoid the if by putting the condition into the define:
>
> dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY', cc.has_header('...'))
Sure, this variant looks better. Thanks for suggestions.
I just copied my version from the similar code for 'RTE_HAS_LIBNUMA'.
Best regards, Ilya Maximets.
