[Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

Laurent Vivier posted 4 patches 4 years, 11 months ago
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190529143106.11789-1-lvivier@redhat.com
Maintainers: Amit Shah <amit@kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
backends/Makefile.objs         |  2 +-
backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
backends/rng-random.c          |  2 +-
hw/virtio/virtio-rng.c         | 19 ++++-----
include/hw/virtio/virtio-rng.h |  2 -
include/sysemu/rng.h           |  2 +
qemu-options.hx                |  9 +++-
7 files changed, 97 insertions(+), 16 deletions(-)
create mode 100644 backends/rng-builtin.c
[Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 11 months ago
Add a new RNG backend using QEMU builtin getrandom function.

v7: rebase on master
    Make rng-builtin asynchronous with QEMUBH (removed existing R-b)

v6: remove "sysemu/rng-random.h" from virtio-rng.c
    rebase on qemu_getrandom v8

v5: PATCH 1 s/linux/Linux/
    remove superfluous includes from rng-builtin.c
    don't update rng-random documentation
    add a patch from Markus to keep the default backend out of VirtIORNGConf
    move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h

v4: update PATCH 1 commit message

v3: Include Kashyap's patch in the series
    Add a patch to change virtio-rng default backend to rng-builtin

v2: Update qemu-options.hx
    describe the new backend and specify virtio-rng uses the
    rng-random by default

Kashyap Chamarthy (1):
  VirtIO-RNG: Update default entropy source to `/dev/urandom`

Laurent Vivier (2):
  rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  virtio-rng: change default backend to rng-builtin

Markus Armbruster (1):
  virtio-rng: Keep the default backend out of VirtIORNGConf

 backends/Makefile.objs         |  2 +-
 backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
 backends/rng-random.c          |  2 +-
 hw/virtio/virtio-rng.c         | 19 ++++-----
 include/hw/virtio/virtio-rng.h |  2 -
 include/sysemu/rng.h           |  2 +
 qemu-options.hx                |  9 +++-
 7 files changed, 97 insertions(+), 16 deletions(-)
 create mode 100644 backends/rng-builtin.c

-- 
2.20.1


Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Michael S. Tsirkin 4 years, 9 months ago
On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
> Add a new RNG backend using QEMU builtin getrandom function.
> 
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>     rebase on qemu_getrandom v8
> 
> v5: PATCH 1 s/linux/Linux/
>     remove superfluous includes from rng-builtin.c
>     don't update rng-random documentation
>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
> 
> v4: update PATCH 1 commit message
> 
> v3: Include Kashyap's patch in the series
>     Add a patch to change virtio-rng default backend to rng-builtin
> 
> v2: Update qemu-options.hx
>     describe the new backend and specify virtio-rng uses the
>     rng-random by default


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

feel free to merge.

> Kashyap Chamarthy (1):
>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
> 
> Laurent Vivier (2):
>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>   virtio-rng: change default backend to rng-builtin
> 
> Markus Armbruster (1):
>   virtio-rng: Keep the default backend out of VirtIORNGConf
> 
>  backends/Makefile.objs         |  2 +-
>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>  backends/rng-random.c          |  2 +-
>  hw/virtio/virtio-rng.c         | 19 ++++-----
>  include/hw/virtio/virtio-rng.h |  2 -
>  include/sysemu/rng.h           |  2 +
>  qemu-options.hx                |  9 +++-
>  7 files changed, 97 insertions(+), 16 deletions(-)
>  create mode 100644 backends/rng-builtin.c
> 
> -- 
> 2.20.1

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 9 months ago
On 02/07/2019 15:21, Michael S. Tsirkin wrote:
> On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>
>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>     rebase on qemu_getrandom v8
>>
>> v5: PATCH 1 s/linux/Linux/
>>     remove superfluous includes from rng-builtin.c
>>     don't update rng-random documentation
>>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
>>
>> v4: update PATCH 1 commit message
>>
>> v3: Include Kashyap's patch in the series
>>     Add a patch to change virtio-rng default backend to rng-builtin
>>
>> v2: Update qemu-options.hx
>>     describe the new backend and specify virtio-rng uses the
>>     rng-random by default
> 
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> feel free to merge.

Thank you Michael.

I've already included PATCH 1 in a pull-request for trivial patches branch.

I'm not sure the other patches are good candidates for trivial patches
branch, but is there any other maintainer that can include them in a
pull request (before the freeze)?

Amit?
[Do you want I manage a virtio-rng pull-request for you?]

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Amit Shah 4 years, 9 months ago
On Tue, 2019-07-02 at 18:48 +0200, Laurent Vivier wrote:
> On 02/07/2019 15:21, Michael S. Tsirkin wrote:
> > On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
> > > Add a new RNG backend using QEMU builtin getrandom function.
> > > 
> > > v7: rebase on master
> > >     Make rng-builtin asynchronous with QEMUBH (removed existing
> > > R-b)
> > > 
> > > v6: remove "sysemu/rng-random.h" from virtio-rng.c
> > >     rebase on qemu_getrandom v8
> > > 
> > > v5: PATCH 1 s/linux/Linux/
> > >     remove superfluous includes from rng-builtin.c
> > >     don't update rng-random documentation
> > >     add a patch from Markus to keep the default backend out of
> > > VirtIORNGConf
> > >     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-
> > > builtin.h
> > > 
> > > v4: update PATCH 1 commit message
> > > 
> > > v3: Include Kashyap's patch in the series
> > >     Add a patch to change virtio-rng default backend to rng-
> > > builtin
> > > 
> > > v2: Update qemu-options.hx
> > >     describe the new backend and specify virtio-rng uses the
> > >     rng-random by default
> > 
> > 
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > feel free to merge.
> 
> Thank you Michael.
> 
> I've already included PATCH 1 in a pull-request for trivial patches
> branch.
> 
> I'm not sure the other patches are good candidates for trivial
> patches
> branch, but is there any other maintainer that can include them in a
> pull request (before the freeze)?
> 
> Amit?
> [Do you want I manage a virtio-rng pull-request for you?]

Hello Laurent,

Apologies as I haven't been around for a bit.

I don't mind you doing the pull req yourself if you have sufficient
reviews.  Do you also want to consider maintaining rng yourself?

Thanks,



Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 9 months ago
On 03/07/2019 16:23, Amit Shah wrote:
> On Tue, 2019-07-02 at 18:48 +0200, Laurent Vivier wrote:
>> On 02/07/2019 15:21, Michael S. Tsirkin wrote:
>>> On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>
>>>> v7: rebase on master
>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing
>>>> R-b)
>>>>
>>>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>>>     rebase on qemu_getrandom v8
>>>>
>>>> v5: PATCH 1 s/linux/Linux/
>>>>     remove superfluous includes from rng-builtin.c
>>>>     don't update rng-random documentation
>>>>     add a patch from Markus to keep the default backend out of
>>>> VirtIORNGConf
>>>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-
>>>> builtin.h
>>>>
>>>> v4: update PATCH 1 commit message
>>>>
>>>> v3: Include Kashyap's patch in the series
>>>>     Add a patch to change virtio-rng default backend to rng-
>>>> builtin
>>>>
>>>> v2: Update qemu-options.hx
>>>>     describe the new backend and specify virtio-rng uses the
>>>>     rng-random by default
>>>
>>>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> feel free to merge.
>>
>> Thank you Michael.
>>
>> I've already included PATCH 1 in a pull-request for trivial patches
>> branch.
>>
>> I'm not sure the other patches are good candidates for trivial
>> patches
>> branch, but is there any other maintainer that can include them in a
>> pull request (before the freeze)?
>>
>> Amit?
>> [Do you want I manage a virtio-rng pull-request for you?]
> 
> Hello Laurent,
> 
> Apologies as I haven't been around for a bit.

No problem.

> I don't mind you doing the pull req yourself if you have sufficient
> reviews.

I think it's too late to push this in 4.1., so it this will wait next
release.

>   Do you also want to consider maintaining rng yourself?

It's up to you: if you think you don't have time to maintain virtio-rng,
I can manage this.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Markus Armbruster 4 years, 10 months ago
Laurent Vivier <lvivier@redhat.com> writes:

> Add a new RNG backend using QEMU builtin getrandom function.
>
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)

Pardon the ignorant question: why is that necessary?

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 10 months ago
On 05/06/2019 15:05, Markus Armbruster wrote:
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> Pardon the ignorant question: why is that necessary?
> 

Because request_entropy() function is called while the request is not in
the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
doens't process it. The request is added just after the call.

thanks,
Laurent





Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Markus Armbruster 4 years, 10 months ago
Laurent Vivier <lvivier@redhat.com> writes:

> On 05/06/2019 15:05, Markus Armbruster wrote:
>> Laurent Vivier <lvivier@redhat.com> writes:
>> 
>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>
>>> v7: rebase on master
>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>> 
>> Pardon the ignorant question: why is that necessary?
>> 
>
> Because request_entropy() function is called while the request is not in
> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
> doens't process it. The request is added just after the call.

In rng_backend_request_entropy().  I see.  Any particular reason for
this order?  "I don't know" is an acceptable answer :)

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 10 months ago
On 05/06/2019 19:56, Markus Armbruster wrote:
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> On 05/06/2019 15:05, Markus Armbruster wrote:
>>> Laurent Vivier <lvivier@redhat.com> writes:
>>>
>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>
>>>> v7: rebase on master
>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>>
>>> Pardon the ignorant question: why is that necessary?
>>>
>>
>> Because request_entropy() function is called while the request is not in
>> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
>> doens't process it. The request is added just after the call.
> 
> In rng_backend_request_entropy().  I see.  Any particular reason for
> this order?  "I don't know" is an acceptable answer :)
> 

Yes...

and there is a reason:

in rng_random_request_entropy(), QSIMPLEQ_EMPTY() is used to know if we
have to register an fd handler with qemu_set_fd_handler().

For me, it seemed easier to use QEMUBH rather than to change the
existing algorithm, as the backend has been thought to be asynchronous.

Thanks,
Laurent




Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Markus Armbruster 4 years, 10 months ago
Laurent Vivier <lvivier@redhat.com> writes:

> On 05/06/2019 19:56, Markus Armbruster wrote:
>> Laurent Vivier <lvivier@redhat.com> writes:
>> 
>>> On 05/06/2019 15:05, Markus Armbruster wrote:
>>>> Laurent Vivier <lvivier@redhat.com> writes:
>>>>
>>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>>
>>>>> v7: rebase on master
>>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>>>
>>>> Pardon the ignorant question: why is that necessary?
>>>>
>>>
>>> Because request_entropy() function is called while the request is not in
>>> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
>>> doens't process it. The request is added just after the call.
>> 
>> In rng_backend_request_entropy().  I see.  Any particular reason for
>> this order?  "I don't know" is an acceptable answer :)
>> 
>
> Yes...
>
> and there is a reason:
>
> in rng_random_request_entropy(), QSIMPLEQ_EMPTY() is used to know if we
> have to register an fd handler with qemu_set_fd_handler().
>
> For me, it seemed easier to use QEMUBH rather than to change the
> existing algorithm, as the backend has been thought to be asynchronous.

In your shoes, I'd be tempted to explore whether changing the order
simplifies things overall.  I'm not asking you to do that; your patch is
okay as is.

Thanks!

Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 10 months ago
Michael,

Could you pick this series in the next virtio pull request?

If you disagree with some of my patches, could you take at least the
first one (from Kashyap)?

Thanks,
Laurent

On 29/05/2019 16:31, Laurent Vivier wrote:
> Add a new RNG backend using QEMU builtin getrandom function.
> 
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>     rebase on qemu_getrandom v8
> 
> v5: PATCH 1 s/linux/Linux/
>     remove superfluous includes from rng-builtin.c
>     don't update rng-random documentation
>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
> 
> v4: update PATCH 1 commit message
> 
> v3: Include Kashyap's patch in the series
>     Add a patch to change virtio-rng default backend to rng-builtin
> 
> v2: Update qemu-options.hx
>     describe the new backend and specify virtio-rng uses the
>     rng-random by default
> 
> Kashyap Chamarthy (1):
>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
> 
> Laurent Vivier (2):
>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>   virtio-rng: change default backend to rng-builtin
> 
> Markus Armbruster (1):
>   virtio-rng: Keep the default backend out of VirtIORNGConf
> 
>  backends/Makefile.objs         |  2 +-
>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>  backends/rng-random.c          |  2 +-
>  hw/virtio/virtio-rng.c         | 19 ++++-----
>  include/hw/virtio/virtio-rng.h |  2 -
>  include/sysemu/rng.h           |  2 +
>  qemu-options.hx                |  9 +++-
>  7 files changed, 97 insertions(+), 16 deletions(-)
>  create mode 100644 backends/rng-builtin.c
> 


Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
Posted by Laurent Vivier 4 years, 10 months ago
On 11/06/2019 10:42, Laurent Vivier wrote:
> Michael,
> 
> Could you pick this series in the next virtio pull request?

Or perhaps Amit?

Thanks,
Laurent

> 
> If you disagree with some of my patches, could you take at least the
> first one (from Kashyap)?
> 
> Thanks,
> Laurent
> 
> On 29/05/2019 16:31, Laurent Vivier wrote:
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>
>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>     rebase on qemu_getrandom v8
>>
>> v5: PATCH 1 s/linux/Linux/
>>     remove superfluous includes from rng-builtin.c
>>     don't update rng-random documentation
>>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
>>
>> v4: update PATCH 1 commit message
>>
>> v3: Include Kashyap's patch in the series
>>     Add a patch to change virtio-rng default backend to rng-builtin
>>
>> v2: Update qemu-options.hx
>>     describe the new backend and specify virtio-rng uses the
>>     rng-random by default
>>
>> Kashyap Chamarthy (1):
>>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
>>
>> Laurent Vivier (2):
>>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>>   virtio-rng: change default backend to rng-builtin
>>
>> Markus Armbruster (1):
>>   virtio-rng: Keep the default backend out of VirtIORNGConf
>>
>>  backends/Makefile.objs         |  2 +-
>>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>>  backends/rng-random.c          |  2 +-
>>  hw/virtio/virtio-rng.c         | 19 ++++-----
>>  include/hw/virtio/virtio-rng.h |  2 -
>>  include/sysemu/rng.h           |  2 +
>>  qemu-options.hx                |  9 +++-
>>  7 files changed, 97 insertions(+), 16 deletions(-)
>>  create mode 100644 backends/rng-builtin.c
>>
> 
>