June 2022 - Libguestfs - libguestfs List Archives

[libnbd PATCH v3 0/5] python: Speed up [aio_]pread_structured
by Eric Blake 10 Jun '22

10 Jun '22

Less copying is always better. But I was quite surprised by some of my test cases in trying to prove that I had speedups; there's a huge difference between: for i in range(size // m): buf = h.pread_structured(m, m*i, f) and for i in range(size // m): buf = h.pread_structured(m, m*i, f) buf = None The former takes around 4.5s with libnbd 1.12.3, the latter around 15s, even though I was expecting the latter to be faster. But with more thought, I think what is happening is that Python's compilation and garbage collection implementation looks for opportunities where it can prove that a pool of allocated memory can be easily reused (assigning buf to hold a new bytearray that occupies the same size as the previous bytearray that is now down to zero references) vs. where it allocates new memory (the intermediate state of buf going to None throwing away the intermediate knowledge about relations between its other values). But with either python script, this patch makes a noticeable difference by exposing the subbuf in [aio_]pread_structured as a slice of the original python object, rather than yet another memory copy. Eric Blake (5): python: Simplify passing of mutable *error to callbacks python: Alter lock for persistent buffer python: Accept all buffer-like objects in aio_p{read,write} python: Support len(nbd.Buffer(n)) python: Slice structured read callback buffer from original generator/Python.ml | 54 ++++++++--------- python/handle.c | 19 ++++-- python/t/405-pread-structured.py | 95 +++++++++--------------------- python/t/500-aio-pread.py | 21 +++++++ python/t/505-aio-pread-callback.py | 93 ++++++++--------------------- python/t/510-aio-pwrite.py | 17 ++++++ python/t/580-aio-is-zero.py | 2 + python/utils.c | 53 +++++++++++++++++ 8 files changed, 186 insertions(+), 168 deletions(-) -- 2.36.1

2 17

libnbd | Failed pipeline for master | 67dd054e
by GitLab 10 Jun '22

10 Jun '22

Pipeline #561026597 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: 67dd054e ( https://gitlab.com/nbdkit/libnbd/-/commit/67dd054ecb86eee89edb172843f1e7a90… ) Commit Message: python: Correctly use PyGILState The python do... Commit Author: Eric Blake ( https://gitlab.com/ebblake ) Pipeline #561026597 ( https://gitlab.com/nbdkit/libnbd/-/pipelines/561026597 ) triggered by Eric Blake ( https://gitlab.com/ebblake ) had 1 failed job. Job #2575910841 ( https://gitlab.com/nbdkit/libnbd/-/jobs/2575910841/raw ) Stage: builds Name: x86_64-opensuse-leap-153 -- You're receiving this email because of your account on gitlab.com.

1 0

[libnbd PATCH] python: Correctly use PyGILState
by Eric Blake 10 Jun '22

10 Jun '22

The python docs are clear that in a multi-threaded app, we CANNOT call any python functions from C code (other than a short-list of exceptions) without first owning the GIL in the calling thread. Although many users of nbdsh are single-threaded (where the GIL does not matter), it is indeed possible to write a python script that creates python threads to manage h.poll(-1) in one thread while using other threads to trigger aio_pread and aio_pwrite requests, so callbacks can be reached from a different thread than the corresponding thread that initiated the nbd I/O request. Unfortunately, our generated code was not doing it correctly (for example, debug_wrapper() to handle h.set_debug_callback() calls Py_BuildValue outside PyGILState_Ensure()). But it appears that we have so far been lucky that our callback actions generally occur based on state machine actions triggered by another Python-based action in the same thread, coupled with the fact that we have not been explicitly releasing the GIL around calls into libnbd C API, so in practice all our callbacks have already owned the GIL in spite of coding it in the wrong place. However, I cannot rule out the risk of a mysterious crash in a multi-threaded python program using the nbd module. Worse, holding the GIL across a potentially-blocking C operation is bad form; no other Python thread can make progress if we don't release the GIL. Fix this by widening the scope of PyGILState in all callback wrappers (all other code calling into Python is assumed to already own the GIL), and by dropping the GIL around calls into libnbd C code. Note that the Python docs also state that manipulation of PyGILState is incompatible with the use of sub-interpreters with Py_NewInterpreter(); but if someone actually wants to write a complex program that uses sub-interpreters to interact with libnbd via Python independently from some other use of python, rather than just directly interacting with libnbd via C, I'd be surprised. Fixes: 936488d4 ("python: Implement Callback properly.", v0.1) --- generator/Python.ml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/generator/Python.ml b/generator/Python.ml index e94f37b..f196e8e 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -158,7 +158,7 @@ let pr " const struct user_data *data = user_data;\n"; pr " int ret = -1;\n"; pr "\n"; - pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n"; + pr " PyGILState_STATE py_save = PyGILState_Ensure();\n"; pr " PyObject *py_args, *py_ret;\n"; List.iter ( function @@ -227,9 +227,7 @@ let pr ");\n"; pr " if (!py_args) { PyErr_PrintEx (0); goto out; }\n"; pr "\n"; - pr " py_save = PyGILState_Ensure ();\n"; pr " py_ret = PyObject_CallObject (data->fn, py_args);\n"; - pr " PyGILState_Release (py_save);\n"; pr "\n"; pr " Py_DECREF (py_args);\n"; pr "\n"; @@ -268,6 +266,7 @@ let | CBUInt _ | CBUInt64 _ -> () | CBArrayAndLen _ | CBMutable _ -> assert false ) cbargs; + pr " PyGILState_Release (py_save);\n"; pr " return ret;\n"; pr "}\n"; pr "\n" @@ -478,7 +477,8 @@ let | BytesPersistOut (n, _) -> pr " if (nbd_internal_py_init_aio_buffer (%s) < 0) goto out;\n" n | _ -> () - ) args; + ) args; + pr " Py_BEGIN_ALLOW_THREADS\n"; pr " ret = nbd_%s (" name; pr_wrap ',' (fun () -> pr "h"; @@ -487,6 +487,7 @@ let | _, _, n -> pr ", %s" n ) params); pr ");\n"; + pr " Py_END_ALLOW_THREADS\n"; List.iter ( function | Closure { cbname } -> pr " %s_user_data = NULL;\n" cbname @@ -613,8 +614,10 @@ let pr " struct user_data *data = user_data;\n"; pr "\n"; pr " if (data) {\n"; + pr " PyGILState_STATE py_save = PyGILState_Ensure();\n"; pr " Py_XDECREF (data->fn);\n"; pr " Py_XDECREF (data->buf);\n"; + pr " PyGILState_Release (py_save);\n"; pr " free (data);\n"; pr " }\n"; pr "}\n"; -- 2.36.1

2 2

[PATCH nbdkit] luks: Don't advertise zero support
by Richard W.M. Jones 10 Jun '22

10 Jun '22

Eric: This seems like a bug in nbdkit, but this patch just does the easy fix. What do you think? Two other filters can return NBDKIT_ZERO_EMULATE: filters/ext2/ext2.c: return NBDKIT_ZERO_EMULATE; filters/nozero/nozero.c: return NBDKIT_ZERO_EMULATE; nbdkit-ext2-filter doesn't crash, but it doesn't emulate either and it seems broken. It calls down to the underlying plugin (which will either emulate or true zero). I believe it may be zeroing a random offset in the underlying disk. Rich.

2 3

luks filter breaks zeroing
by Richard W.M. Jones 10 Jun '22

10 Jun '22

(More of a note to self to investigate) $ qemu-img create -f luks --object secret,data=LETMEPASS,id=sec0 -o key-secret=sec0 encrypted.img 100M Formatting 'encrypted.img', fmt=luks size=104857600 key-secret=sec0 $ rm -f data.img $ truncate -s 100M data.img $ nbdkit file encrypted.img --filter=luks passphrase=LETMEPASS --run 'nbdcopy data.img $nbd' nbdkit: backend.c:718: backend_zero: Assertion `c->can_zero > NBDKIT_ZERO_NONE' failed. write at offset 0 failed: Transport endpoint is not connected nbdkit: nbdkit command was killed by signal 6 -- 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

2 2

libnbd | Failed pipeline for master | b23b099c
by GitLab 09 Jun '22

09 Jun '22

Pipeline #560147648 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: b23b099c ( https://gitlab.com/nbdkit/libnbd/-/commit/b23b099c7810b994acbf43e890a7555b6… ) Commit Message: Version 1.13.1. Commit Author: Richard W_M_ Jones ( https://gitlab.com/rwmjones ) Pipeline #560147648 ( https://gitlab.com/nbdkit/libnbd/-/pipelines/560147648 ) triggered by Richard W_M_ Jones ( https://gitlab.com/rwmjones ) had 1 failed job. Job #2570994648 ( https://gitlab.com/nbdkit/libnbd/-/jobs/2570994648/raw ) Stage: builds Name: x86_64-opensuse-leap-153 -- You're receiving this email because of your account on gitlab.com.

1 0

libnbd golang failure on RISC-V
by Richard W.M. Jones 09 Jun '22

09 Jun '22

make[2]: Entering directory '/home/rjones/d/libnbd/golang' perl /home/rjones/d/libnbd/podwrapper.pl --section=3 --man libnbd-golang.3 \ --html ../html/libnbd-golang.3.html \ libnbd-golang.pod /home/rjones/d/libnbd/run go build write of Go pointer 0x3fa8028000 to non-Go memory 0x3fd2c0fb20 fatal error: Go pointer stored into non-Go memory runtime stack: runtime_mstart ../../../libgo/runtime/proc.c:593 goroutine 1 [running, locked to thread]: Not sure how I should diagnose this one further? I wouldn't normally worry about RISC-V failures, but they may indicate some kind of arch generic problem that is just not exposed on x86. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

3 12

libnbd | Failed pipeline for master | 02ddf116
by GitLab 09 Jun '22

09 Jun '22

Pipeline #560071073 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: 02ddf116 ( https://gitlab.com/nbdkit/libnbd/-/commit/02ddf11681c07dbe20dce303bcf250c5a… ) Commit Message: golang: aio_buffer.go: Explicit panic() on inva... Commit Author: Nir Soffer ( https://gitlab.com/nirs ) Committed by: Richard W_M_ Jones ( https://gitlab.com/rwmjones ) Pipeline #560071073 ( https://gitlab.com/nbdkit/libnbd/-/pipelines/560071073 ) triggered by Richard W_M_ Jones ( https://gitlab.com/rwmjones ) had 1 failed job. Job #2570546056 ( https://gitlab.com/nbdkit/libnbd/-/jobs/2570546056/raw ) Stage: builds Name: x86_64-opensuse-leap-153 -- You're receiving this email because of your account on gitlab.com.

1 0

[PATCH libnbd] golang: aio_buffer.go: Explicit panic() on invalid usage
by Nir Soffer 09 Jun '22

09 Jun '22

Previously we depended on the behavior on common platforms to panic when trying to use a nil pointer, but Richard reported that it segfault on RISC-V. Avoid the undocumented assumptions and panic explicitly with a useful panic message. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- golang/aio_buffer.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/golang/aio_buffer.go b/golang/aio_buffer.go index 52ea54de..325dbc98 100644 --- a/golang/aio_buffer.go +++ b/golang/aio_buffer.go @@ -65,28 +65,37 @@ func (b *AioBuffer) Free() { if b.P != nil { C.free(b.P) b.P = nil } } // Bytes copies the underlying C array to Go allocated memory and return a // slice. Modifying the returned slice does not modify the underlying buffer // backing array. func (b *AioBuffer) Bytes() []byte { + if b.P == nil { + panic("Using AioBuffer after Free()") + } return C.GoBytes(b.P, C.int(b.Size)) } // Slice creates a slice backed by the underlying C array. The slice can be // used to access or modify the contents of the underlying array. The slice // must not be used after caling Free(). func (b *AioBuffer) Slice() []byte { + if b.P == nil { + panic("Using AioBuffer after Free()") + } // See https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices // TODO: Use unsafe.Slice() when we require Go 1.17. return (*[1<<30]byte)(b.P)[:b.Size:b.Size] } // Get returns a pointer to a byte in the underlying C array. The pointer can // be used to modify the underlying array. The pointer must not be used after // calling Free(). func (b *AioBuffer) Get(i uint) *byte { + if b.P == nil { + panic("Using AioBuffer after Free()") + } return (*byte)(unsafe.Pointer(uintptr(b.P) + uintptr(i))) } -- 2.36.1

2 1

libnbd | Failed pipeline for master | 8a1c75c0
by GitLab 09 Jun '22

09 Jun '22

Pipeline #560026031 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: 8a1c75c0 ( https://gitlab.com/nbdkit/libnbd/-/commit/8a1c75c0c2f102de897dadbb395b5fa5f… ) Commit Message: golang: Whitespace change caused by gofmt Commit Author: Richard W_M_ Jones ( https://gitlab.com/rwmjones ) Pipeline #560026031 ( https://gitlab.com/nbdkit/libnbd/-/pipelines/560026031 ) triggered by Richard W_M_ Jones ( https://gitlab.com/rwmjones ) had 1 failed job. Job #2570278642 ( https://gitlab.com/nbdkit/libnbd/-/jobs/2570278642/raw ) Stage: builds Name: x86_64-opensuse-leap-153 -- You're receiving this email because of your account on gitlab.com.

1 0