Libguestfs - Aug 2022 - Benchmarks of asynch nbdublk (was: Re: [PATCH libnbd] ublk: Add new nbdublk program)

Alright, so final version for now is here:

  https://gitlab.com/rwmjones/libnbd/-/tree/nbdublk/ublk

I resolved the data corruption problem once I realised that
->handle_event must only retire commands which are on the same queue
(obvious in hindsight).  I ran a parallel job overnight where I did
various heavyweight operations repeatedly -- a bunch of git stuff,
large copies, a big compile etc -- on an nbdublk filesystem.  It
handled everything perfectly.

Before this can go upstream:

 - Does libubdsrv now have a stable API, or could it change?

 - We need to enable the device in the Fedora kernel.

 - We need libubdsrv + ublk tool in Fedora.

 - Possible we need to add tests into the libnbd tree.  I couldn't
   think of a good way to test this that doesn't require root and
   potentially do some horrible stuff to the testing machine.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Sat, Aug 27, 2022 at 06:40:29PM +0100, Richard W.M. Jones
wrote:
> Alright, so final version for now is here:
> 
>   https://gitlab.com/rwmjones/libnbd/-/tree/nbdublk/ublk
Just clone the above branch, but can't build because of missing
ocamlbuild & ocamlcc.

But I will review the implementation from ublk viewpoint next week.
> 
> I resolved the data corruption problem once I realised that
> ->handle_event must only retire commands which are on the same queue
> (obvious in hindsight).  I ran a parallel job overnight where I did
> various heavyweight operations repeatedly -- a bunch of git stuff,
> large copies, a big compile etc -- on an nbdublk filesystem.  It
> handled everything perfectly.
It isn't strange since ublk-loop does survive xfstests before.
> 
> Before this can go upstream:
> 
>  - Does libubdsrv now have a stable API, or could it change?
It is hard to answer this question now, given ublk driver is just
merged to v6.0-rc1, from then on, I started to focus on ublksrv
userspace.

But can you share what status nbdublk is now? Is it enough as one
basic nbd product?

IMO, it is pretty easy to make one toy, but very hard to build one
product, I really appreciate that you can provide some feedback
on libublksrv from viewpoint of nbdublk as product, such as,
current APIs are enough to build basic product of nbd? Are these
APIs enough to build stable enough nbd product? Are they fine to
reach expected performance?

IMO, this feedback is very helpful for evaluating if current APIs are
stable enough.
> 
>  - We need to enable the device in the Fedora kernel.
OK, what is the Fedora release you plan to enable nbdublk?
> 
>  - We need libubdsrv + ublk tool in Fedora.
I guess libublksrv is enough, why do you need ublk too for nbdublk?
> 
>  - Possible we need to add tests into the libnbd tree.  I couldn't
>    think of a good way to test this that doesn't require root and
>    potentially do some horrible stuff to the testing machine.
So far, root is still required, but in future we hope root privilege
becomes not needed.


Thanks,
Ming

On Sun, Aug 28, 2022 at 11:21:25PM +0800, Ming Lei
wrote:
> On Sat, Aug 27, 2022 at 06:40:29PM +0100, Richard W.M. Jones wrote:
> > Alright, so final version for now is here:
> > 
> >   https://gitlab.com/rwmjones/libnbd/-/tree/nbdublk/ublk
> 
> Just clone the above branch, but can't build because of missing
> ocamlbuild & ocamlcc.
You should only need /usr/bin/ocamlc I think.  It is used to generate
the state machine and language bindings.
> But can you share what status nbdublk is now? Is it enough as one
> basic nbd product?
It uses libnbd so it supports pretty much the whole NBD protocol.
URIs, Unix & TCP, TLS, discard, zero, block size preferences, etc.
That means it's already more featureful than nbd.ko which doesn't
support NBD URIs or block size prefs.  Is there a particular feature
you're thinking about?
> IMO, it is pretty easy to make one toy, but very hard to build one
> product, I really appreciate that you can provide some feedback
> on libublksrv from viewpoint of nbdublk as product, such as,
> current APIs are enough to build basic product of nbd? Are these
> APIs enough to build stable enough nbd product? Are they fine to
> reach expected performance?
Performance was within 25% of disk access over virtio-scsi, which I
think is pretty good.
> IMO, this feedback is very helpful for evaluating if current APIs are
> stable enough.
> 
> > 
> >  - We need to enable the device in the Fedora kernel.
> 
> OK, what is the Fedora release you plan to enable nbdublk?
You'll need to open a BZ again the kernel and ask them to enable the
config setting.
> >  - We need libubdsrv + ublk tool in Fedora.
> 
> I guess libublksrv is enough, why do you need ublk too for nbdublk?
It's not really possible to use nbdublk on its own.  You need at least
"ublk list" and "ublk del" commands to make something
useful.
> >  - Possible we need to add tests into the libnbd tree.  I couldn't
> >    think of a good way to test this that doesn't require root and
> >    potentially do some horrible stuff to the testing machine.
> 
> So far, root is still required, but in future we hope root privilege
> becomes not needed.
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On 08/27/22 19:40, Richard W.M. Jones wrote:
> Alright, so final version for now is here:
> 
>   https://gitlab.com/rwmjones/libnbd/-/tree/nbdublk/ublk
I'm slightly interested to review this, but I'm also OK if we just push
it and call it "initial addition" or "experimental" or
whatever (I don't
think it can regress anything).

For a review, I'd likely need a good amount of time, and also the
"final" version to be on the list. Also, speaking relative to commit
cafc4dbbc8fa, it would be nice to squash / distribute the fixes into the
original patches as appropriate, and maybe to split up the initial code
drop into a gradual build-up of the new feature.

The latter is a lot of extra work, admittedly, I just find it does
wonders to my review throughput. At the same time, I absolutely don't
insist on reviewing this -- I'm perfectly fine if we merge it at once.

Thanks!
Laszlo
> 
> I resolved the data corruption problem once I realised that
> ->handle_event must only retire commands which are on the same queue
> (obvious in hindsight).  I ran a parallel job overnight where I did
> various heavyweight operations repeatedly -- a bunch of git stuff,
> large copies, a big compile etc -- on an nbdublk filesystem.  It
> handled everything perfectly.
> 
> Before this can go upstream:
> 
>  - Does libubdsrv now have a stable API, or could it change?
> 
>  - We need to enable the device in the Fedora kernel.
> 
>  - We need libubdsrv + ublk tool in Fedora.
> 
>  - Possible we need to add tests into the libnbd tree.  I couldn't
>    think of a good way to test this that doesn't require root and
>    potentially do some horrible stuff to the testing machine.
> 
> Rich.
>