Re: [Libguestfs] libnbd | Failed pipeline for master | 0cd77478
On Sun, Mar 6, 2022 at 10:40 PM Richard W.M. Jones wrote: > > On Sun, Mar 06, 2022 at 08:28:09PM +, GitLab wrote: > > GitLab > >✖ Pipeline #485634933 has failed! > > > > Project nbdkit / libnbd > > Branch● master > > Commit● 0cd77478 > > copy: Minor cleanups Minor fixes suggested by ... > > Commit Author ● Nir Soffer > > > > Pipeline #485634933 triggered by ● Nir Soffer > >had 2 failed jobs. > > Failed jobs > > ✖ builds x86_64-centos-8 > > ✖ builds x86_64-ubuntu-2004 > > I'll fix this. In brief it happened because centos:8 is no longer a > thing (sadly), we have to replace it with almalinux; and the Ubuntu > problem looks like a temporary failure. I retried the ubuntu build and this time it was successful: https://gitlab.com/nbdkit/libnbd/-/jobs/2168746668 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] libnbd | Failed pipeline for master | 0cd77478
On Sun, Mar 06, 2022 at 08:40:03PM +, Richard W.M. Jones wrote: > On Sun, Mar 06, 2022 at 08:28:09PM +, GitLab wrote: > > GitLab > >✖ Pipeline #485634933 has failed! > > > > Project nbdkit / libnbd > > Branch● master > > Commit● 0cd77478 > > copy: Minor cleanups Minor fixes suggested by ... > > Commit Author ● Nir Soffer > > > > Pipeline #485634933 triggered by ● Nir Soffer > >had 2 failed jobs. > > Failed jobs > > ✖ builds x86_64-centos-8 > > ✖ builds x86_64-ubuntu-2004 > > I'll fix this. In brief it happened because centos:8 is no longer a > thing (sadly), we have to replace it with almalinux; and the Ubuntu > problem looks like a temporary failure. Well it failed again, but I think that's because the OpenSUSE LEAP platform is known not to be stable. The tests above now worked. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 1/3] golang: examples: Do not initialize pread buffer
On Sun, Mar 6, 2022 at 10:35 PM Richard W.M. Jones wrote:
>
> On Sun, Mar 06, 2022 at 10:27:28PM +0200, Nir Soffer wrote:
> > The aio_copy example checks errors properly, so it will not leak
> > uninitialized memory to the destination image. Testing shows 5% speedup
> > when copying a real image.
> >
> > $ qemu-nbd --read-only --persistent --shared 8 --cache none --aio native \
> > --socket /tmp/src.sock --format raw fedora-35-data.raw &
> >
> > $ hyperfine -p "sleep 5" "./aio_copy-init $SRC >/dev/null"
> > "./aio_copy-no-init $SRC >/dev/null"
> >
> > Benchmark 1: ./aio_copy-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
> > Time (mean ± σ): 1.452 s ± 0.027 s[User: 0.330 s, System: 0.489
> > s]
> > Range (min … max):1.426 s … 1.506 s10 runs
> >
> > Benchmark 2: ./aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
> > Time (mean ± σ): 1.378 s ± 0.009 s[User: 0.202 s, System: 0.484
> > s]
> > Range (min … max):1.369 s … 1.399 s10 runs
> >
> > Summary
> > './aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null' ran
> > 1.05 ± 0.02 times faster than './aio_copy-init
> > nbd+unix:///?socket=/tmp/src.sock >/dev/null'
> >
> > Signed-off-by: Nir Soffer
> > ---
> > golang/examples/aio_copy/aio_copy.go | 5 +
> > golang/examples/simple_copy/simple_copy.go | 5 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/golang/examples/aio_copy/aio_copy.go
> > b/golang/examples/aio_copy/aio_copy.go
> > index bb20b478..89eac4df 100644
> > --- a/golang/examples/aio_copy/aio_copy.go
> > +++ b/golang/examples/aio_copy/aio_copy.go
> > @@ -84,20 +84,25 @@ func main() {
> > err = h.ConnectUri(flag.Arg(0))
> > if err != nil {
> > panic(err)
> > }
> >
> > size, err := h.GetSize()
> > if err != nil {
> > panic(err)
> > }
> >
> > + err = h.SetPreadInitialize(false)
> > + if err != nil {
> > + panic(err)
> > + }
> > +
>
> In patch #2 you added a comment above the call.
>
> Because this is an example and so people may just copy the code
> blindly without understanding it, I think adding a comment here and
> below is worth doing too.
>
> > diff --git a/golang/examples/simple_copy/simple_copy.go
> > b/golang/examples/simple_copy/simple_copy.go
> > index e8fa1f76..2a2ed0ff 100644
> > --- a/golang/examples/simple_copy/simple_copy.go
> > +++ b/golang/examples/simple_copy/simple_copy.go
> > @@ -63,20 +63,25 @@ func main() {
> > err = h.ConnectUri(flag.Arg(0))
> > if err != nil {
> > panic(err)
> > }
> >
> > size, err := h.GetSize()
> > if err != nil {
> > panic(err)
> > }
> >
> > + err = h.SetPreadInitialize(false)
> > + if err != nil {
> > + panic(err)
> > + }
> > +
>
> And above this one.
Yes, good idea to explain why it is needed and the risk when working
with a bad server.
Nir
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] libnbd | Failed pipeline for master | 0cd77478
On Sun, Mar 06, 2022 at 08:28:09PM +, GitLab wrote: > GitLab >✖ Pipeline #485634933 has failed! > > Project nbdkit / libnbd > Branch● master > Commit● 0cd77478 > copy: Minor cleanups Minor fixes suggested by ... > Commit Author ● Nir Soffer > > Pipeline #485634933 triggered by ● Nir Soffer >had 2 failed jobs. > Failed jobs > ✖ builds x86_64-centos-8 > ✖ builds x86_64-ubuntu-2004 I'll fix this. In brief it happened because centos:8 is no longer a thing (sadly), we have to replace it with almalinux; and the Ubuntu problem looks like a temporary failure. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 3/3] copy: Do not initialize read buffer
On Sun, Mar 06, 2022 at 10:27:30PM +0200, Nir Soffer wrote:
> nbdcopy checks pread error now, so we will never leak uninitialized data
> from the heap to the destination server. Testing show 3-8% speedup when
> copying a real image.
>
> On laptop with 12 cores and 2 consumer NVMe drives:
>
> $ qemu-nbd --read-only --persistent --shared 8 --cache none --aio native \
> --socket /tmp/src.sock --format raw fedora-35-data.raw &
>
> $ qemu-nbd --persistent --shared 8 --cache none --aio native --discard unmap \
> --socket /tmp/dst.sock --format raw dst.raw &
>
> $ hyperfine -p "sleep 5" "nbdcopy $SRC $DST" ".libs/nbdcopy $SRC $DST"
>
> Benchmark 1: nbdcopy nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock
> Time (mean ± σ): 2.065 s ± 0.057 s[User: 0.296 s, System: 1.414 s]
> Range (min … max):2.000 s … 2.163 s10 runs
>
> Benchmark 2: .libs/nbdcopy nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock
> Time (mean ± σ): 1.911 s ± 0.050 s[User: 0.059 s, System: 1.544 s]
> Range (min … max):1.827 s … 1.980 s10 runs
>
> Summary
> '.libs/nbdcopy nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock' ran
> 1.08 ± 0.04 times faster than 'nbdcopy nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock'
>
> On server with 80 cores and a fast NVMe drive:
>
> $ hyperfine "./nbdcopy-init $SRC $DST" "./nbdcopy-no-init $SRC $DST"
>
> Benchmark 1: ./nbdcopy-init nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock
> Time (mean ± σ): 2.652 s ± 0.033 s[User: 0.345 s, System: 1.306 s]
> Range (min … max):2.619 s … 2.709 s10 runs
>
> Benchmark 2: ./nbdcopy-no-init nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock
> Time (mean ± σ): 2.572 s ± 0.031 s[User: 0.055 s, System: 1.409 s]
> Range (min … max):2.537 s … 2.629 s10 runs
>
> Summary
> './nbdcopy-no-init nbd+unix:///?socket=/tmp/src.sock
> nbd+unix:///?socket=/tmp/dst.sock' ran
> 1.03 ± 0.02 times faster than './nbdcopy-init
> nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'
>
> Signed-off-by: Nir Soffer
> ---
> copy/nbd-ops.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c
> index adfe4de5..1218d6d0 100644
> --- a/copy/nbd-ops.c
> +++ b/copy/nbd-ops.c
> @@ -52,20 +52,21 @@ static void
> open_one_nbd_handle (struct rw_nbd *rwn)
> {
>struct nbd_handle *nbd;
>
>nbd = nbd_create ();
>if (nbd == NULL) {
> fprintf (stderr, "%s: %s\n", prog, nbd_get_error ());
> exit (EXIT_FAILURE);
>}
>
> + nbd_set_pread_initialize (nbd, false);
I feel this is worth a small comment too even though it's not an
example. Something simple like this is enough:
+ /* This is only safe because we always check for errors from pread. */
+ nbd_set_pread_initialize (nbd, false);
ACK series - with comments added.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 1/3] golang: examples: Do not initialize pread buffer
On Sun, Mar 06, 2022 at 10:27:28PM +0200, Nir Soffer wrote:
> The aio_copy example checks errors properly, so it will not leak
> uninitialized memory to the destination image. Testing shows 5% speedup
> when copying a real image.
>
> $ qemu-nbd --read-only --persistent --shared 8 --cache none --aio native \
> --socket /tmp/src.sock --format raw fedora-35-data.raw &
>
> $ hyperfine -p "sleep 5" "./aio_copy-init $SRC >/dev/null"
> "./aio_copy-no-init $SRC >/dev/null"
>
> Benchmark 1: ./aio_copy-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
> Time (mean ± σ): 1.452 s ± 0.027 s[User: 0.330 s, System: 0.489 s]
> Range (min … max):1.426 s … 1.506 s10 runs
>
> Benchmark 2: ./aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
> Time (mean ± σ): 1.378 s ± 0.009 s[User: 0.202 s, System: 0.484 s]
> Range (min … max):1.369 s … 1.399 s10 runs
>
> Summary
> './aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null' ran
> 1.05 ± 0.02 times faster than './aio_copy-init
> nbd+unix:///?socket=/tmp/src.sock >/dev/null'
>
> Signed-off-by: Nir Soffer
> ---
> golang/examples/aio_copy/aio_copy.go | 5 +
> golang/examples/simple_copy/simple_copy.go | 5 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/golang/examples/aio_copy/aio_copy.go
> b/golang/examples/aio_copy/aio_copy.go
> index bb20b478..89eac4df 100644
> --- a/golang/examples/aio_copy/aio_copy.go
> +++ b/golang/examples/aio_copy/aio_copy.go
> @@ -84,20 +84,25 @@ func main() {
> err = h.ConnectUri(flag.Arg(0))
> if err != nil {
> panic(err)
> }
>
> size, err := h.GetSize()
> if err != nil {
> panic(err)
> }
>
> + err = h.SetPreadInitialize(false)
> + if err != nil {
> + panic(err)
> + }
> +
In patch #2 you added a comment above the call.
Because this is an example and so people may just copy the code
blindly without understanding it, I think adding a comment here and
below is worth doing too.
> diff --git a/golang/examples/simple_copy/simple_copy.go
> b/golang/examples/simple_copy/simple_copy.go
> index e8fa1f76..2a2ed0ff 100644
> --- a/golang/examples/simple_copy/simple_copy.go
> +++ b/golang/examples/simple_copy/simple_copy.go
> @@ -63,20 +63,25 @@ func main() {
> err = h.ConnectUri(flag.Arg(0))
> if err != nil {
> panic(err)
> }
>
> size, err := h.GetSize()
> if err != nil {
> panic(err)
> }
>
> + err = h.SetPreadInitialize(false)
> + if err != nil {
> + panic(err)
> + }
> +
And above this one.
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH libnbd 3/3] copy: Do not initialize read buffer
nbdcopy checks pread error now, so we will never leak uninitialized data
from the heap to the destination server. Testing show 3-8% speedup when
copying a real image.
On laptop with 12 cores and 2 consumer NVMe drives:
$ qemu-nbd --read-only --persistent --shared 8 --cache none --aio native \
--socket /tmp/src.sock --format raw fedora-35-data.raw &
$ qemu-nbd --persistent --shared 8 --cache none --aio native --discard unmap \
--socket /tmp/dst.sock --format raw dst.raw &
$ hyperfine -p "sleep 5" "nbdcopy $SRC $DST" ".libs/nbdcopy $SRC $DST"
Benchmark 1: nbdcopy nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock
Time (mean ± σ): 2.065 s ± 0.057 s[User: 0.296 s, System: 1.414 s]
Range (min … max):2.000 s … 2.163 s10 runs
Benchmark 2: .libs/nbdcopy nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock
Time (mean ± σ): 1.911 s ± 0.050 s[User: 0.059 s, System: 1.544 s]
Range (min … max):1.827 s … 1.980 s10 runs
Summary
'.libs/nbdcopy nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock' ran
1.08 ± 0.04 times faster than 'nbdcopy nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock'
On server with 80 cores and a fast NVMe drive:
$ hyperfine "./nbdcopy-init $SRC $DST" "./nbdcopy-no-init $SRC $DST"
Benchmark 1: ./nbdcopy-init nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock
Time (mean ± σ): 2.652 s ± 0.033 s[User: 0.345 s, System: 1.306 s]
Range (min … max):2.619 s … 2.709 s10 runs
Benchmark 2: ./nbdcopy-no-init nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock
Time (mean ± σ): 2.572 s ± 0.031 s[User: 0.055 s, System: 1.409 s]
Range (min … max):2.537 s … 2.629 s10 runs
Summary
'./nbdcopy-no-init nbd+unix:///?socket=/tmp/src.sock
nbd+unix:///?socket=/tmp/dst.sock' ran
1.03 ± 0.02 times faster than './nbdcopy-init
nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'
Signed-off-by: Nir Soffer
---
copy/nbd-ops.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c
index adfe4de5..1218d6d0 100644
--- a/copy/nbd-ops.c
+++ b/copy/nbd-ops.c
@@ -52,20 +52,21 @@ static void
open_one_nbd_handle (struct rw_nbd *rwn)
{
struct nbd_handle *nbd;
nbd = nbd_create ();
if (nbd == NULL) {
fprintf (stderr, "%s: %s\n", prog, nbd_get_error ());
exit (EXIT_FAILURE);
}
+ nbd_set_pread_initialize (nbd, false);
nbd_set_debug (nbd, verbose);
/* Set the handle name for debugging. We could use rwn->rw.name
* here but it is usually set to the lengthy NBD URI
* (eg. "nbd://localhost:10809") which makes debug messages very
* long.
*/
if (verbose) {
char *name;
const size_t index = rwn->handles.len;
--
2.35.1
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH libnbd 0/3] Disablble pread buffer initiialization
Disbaling pread initialization is atually measurable and gives small speedup in nbdcopy and some examples. Some cases are safer; in copy-libev example, we allocate all buffers with calloc() and resuse them for the entire copy, so the initialization is completly useless. In nbdcopy and Go aio_copy example, we allocate new buffer using malloc(), so when working with bad server that does not return all data chunks in structured reply, we can leak uninitialized memory to the destination server. I think this issue should be solved by libnbd; it should verfify that the server return all the expected chunks and fail the request if not. Nir Soffer (3): golang: examples: Do not initialize pread buffer examples: copy-libev: Do not initialize pread buffer copy: Do not initialize read buffer copy/nbd-ops.c | 1 + examples/copy-libev.c | 6 ++ golang/examples/aio_copy/aio_copy.go | 5 + golang/examples/simple_copy/simple_copy.go | 5 + 4 files changed, 17 insertions(+) -- 2.35.1 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH libnbd 1/3] golang: examples: Do not initialize pread buffer
The aio_copy example checks errors properly, so it will not leak
uninitialized memory to the destination image. Testing shows 5% speedup
when copying a real image.
$ qemu-nbd --read-only --persistent --shared 8 --cache none --aio native \
--socket /tmp/src.sock --format raw fedora-35-data.raw &
$ hyperfine -p "sleep 5" "./aio_copy-init $SRC >/dev/null" "./aio_copy-no-init
$SRC >/dev/null"
Benchmark 1: ./aio_copy-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
Time (mean ± σ): 1.452 s ± 0.027 s[User: 0.330 s, System: 0.489 s]
Range (min … max):1.426 s … 1.506 s10 runs
Benchmark 2: ./aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null
Time (mean ± σ): 1.378 s ± 0.009 s[User: 0.202 s, System: 0.484 s]
Range (min … max):1.369 s … 1.399 s10 runs
Summary
'./aio_copy-no-init nbd+unix:///?socket=/tmp/src.sock >/dev/null' ran
1.05 ± 0.02 times faster than './aio_copy-init
nbd+unix:///?socket=/tmp/src.sock >/dev/null'
Signed-off-by: Nir Soffer
---
golang/examples/aio_copy/aio_copy.go | 5 +
golang/examples/simple_copy/simple_copy.go | 5 +
2 files changed, 10 insertions(+)
diff --git a/golang/examples/aio_copy/aio_copy.go
b/golang/examples/aio_copy/aio_copy.go
index bb20b478..89eac4df 100644
--- a/golang/examples/aio_copy/aio_copy.go
+++ b/golang/examples/aio_copy/aio_copy.go
@@ -84,20 +84,25 @@ func main() {
err = h.ConnectUri(flag.Arg(0))
if err != nil {
panic(err)
}
size, err := h.GetSize()
if err != nil {
panic(err)
}
+ err = h.SetPreadInitialize(false)
+ if err != nil {
+ panic(err)
+ }
+
var offset uint64
for offset < size || queue.Len() > 0 {
for offset < size && inflightRequests() < *requests {
length := *requestSize
if size-offset < uint64(length) {
length = uint(size - offset)
}
startRead(offset, length)
diff --git a/golang/examples/simple_copy/simple_copy.go
b/golang/examples/simple_copy/simple_copy.go
index e8fa1f76..2a2ed0ff 100644
--- a/golang/examples/simple_copy/simple_copy.go
+++ b/golang/examples/simple_copy/simple_copy.go
@@ -63,20 +63,25 @@ func main() {
err = h.ConnectUri(flag.Arg(0))
if err != nil {
panic(err)
}
size, err := h.GetSize()
if err != nil {
panic(err)
}
+ err = h.SetPreadInitialize(false)
+ if err != nil {
+ panic(err)
+ }
+
buf := make([]byte, *requestSize)
var offset uint64
for offset < size {
if size-offset < uint64(len(buf)) {
buf = buf[:offset-size]
}
err = h.Pread(buf, offset, nil)
if err != nil {
--
2.35.1
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 8/8] copy: Adaptive queue size
On Wed, Feb 23, 2022 at 3:56 PM Nir Soffer wrote: > > On Wed, Feb 23, 2022 at 3:29 PM Richard W.M. Jones wrote: > > > > On Wed, Feb 23, 2022 at 02:53:47PM +0200, Nir Soffer wrote: > > > I'll send more patches for the suggested improvements next week. > > > > I'd like to an upstream stable release early next week, ideally Monday > > if possible. > > The additional changes are internal details that do not need to be in > the release. For example make free_command() accept NULL. Both fixes suggested by Eric push in 0cd77478a7ac863ef092b9e4295b6f1de6d687ac. ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
