On 3/17/20 6:56 AM, Richard W.M. Jones wrote:
v2 was here: https://www.redhat.com/archives/libguestfs/2020-March/msg00154.html
v3:
- Micro-optimize tmpdir.
- Quote $disk in default command shell fragment.
- Don't redirect mkfs output to /dev/null. Instead use exec </dev/null >/dev/null before the shell fragment.
We may want to do this in other places where we run external shell scripts, or more generally for all plugins, but this commit does not fix this.
Yep, any further work here deserves separate patches; what you have here is good enough for now.
- Improve can_multi_conn comment.
- Use mkostemp if available. If not we have to use a racy mkstemp + set_cloexec instead.
I guess the race only affects Haiku, and only happens in the very tight window where one client tries to connect while another is between the mkstemp and fcntl. The comment is good enough, without trying to add a mutex to work around the race (and maybe the more we keep pointing it out to Haiku folks, the more inclined they will be to fix their libc).
- I still didn't implement .zero, because the implementation (see plugins/file/file.c) is really complicated even if you remove the block device code. I guess it would be nice to isolate all this complexity into common/ at some point, which would allow us to implement efficient zero (and trim) in other file-backed plugins.
Yeah, a separate patch making this easier through common/ seems like a good idea.
- I left WIFSTOPPED, but I guess I could remove it. Are we sure it can never happen (eg. if the user is running nbdkit in the foreground and uses ^Z)?
Leaving it doesn't hurt.
- Rerun the tests & valgrind.
LGTM; I think we're ready for this to go in now. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org