Message ID | 20190529143106.11789-1-lvivier@redhat.com |
---|---|
Headers | show |
Series | rng-builtin: add an RNG backend that uses qemu_guest_getrandom() | expand |
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?
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
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 :)
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
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!
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 >
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 >> > >
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
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
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,
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