Re: [Libguestfs] libnbd golang failure on RISC-V

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 02:14:07PM +0100, Daniel P. Berrangé wrote:
> Setting GODEBUG=cgocheck=1   or GODEBUG=cgocheck=2  can sometimes
> get more info.

Took me a while to work out that we are actually setting that already
(in ./run) which explains a couple of things: why I couldn't reproduce
the error using go build directly, and why things like
"GODEBUG=cgocheck=1 ../run go build" make no difference.

> If you can get the test to core dump, then plain old GDB core
> dump could also be useful - might identify which specific API
> is being called, which can help restrict the size of the haystack
> for code review.

Eventually careful reading of this page revealed how to do this:
https://pkg.go.dev/runtime

The results unfortunately don't seem very useful.  The stack trace
appears to point to where the error was detected rather than where it
was caused, but I've copied it below anyway.

Also attached is the failure in the test suite if you turn *off*
cgocheck, maybe that is useful.

Rich.

$ GODEBUG=cgocheck=2,invalidptr=1 GOTRACEBACK=crash go test -count=1 -v
write of Go pointer 0x3f94026000 to non-Go memory 0x3fbfd6fb20
fatal error: Go pointer stored into non-Go memory

runtime stack:
runtime.dopanic_m
../../../libgo/go/runtime/panic.go:1211
runtime.fatalthrow
../../../libgo/go/runtime/panic.go:1071
runtime.throw
../../../libgo/go/runtime/panic.go:1042
runtime.cgoCheckWriteBarrier..func1
../../../libgo/go/runtime/cgocheck.go:55
runtime.systemstack..func1
../../../libgo/go/runtime/stubs.go:63
runtime_mstart
../../../libgo/runtime/proc.c:593

goroutine 1 [running, locked to thread]:
runtime.mcall
../../../libgo/runtime/proc.c:343
runtime.systemstack
../../../libgo/go/runtime/stubs.go:66
runtime.cgoCheckWriteBarrier
../../../libgo/go/runtime/cgocheck.go:53
runtime.wbBufFlush
../../../libgo/go/runtime/mwbbuf.go:196
runtime.gcWriteBarrier
../../../libgo/go/runtime/mgc_gccgo.go:168
runtime.main
../../../libgo/go/runtime/proc.go:209
Aborted (core dumped)

$ coredumpctl gdb
   PID: 191012 (go)
   UID: 1001 (rjones)
   GID: 1001 (rjones)
Signal: 6 (ABRT)
 Timestamp: Thu 2022-06-09 10:04:10 EDT (1min 24s ago)
  Command Line: go test -count=1 -v
Executable: /usr/bin/go.gcc
 Control Group: /user.slice/user-1001.slice/session-11.scope
  Unit: session-11.scope
 Slice: user-1001.slice
   Session: 11
 Owner UID: 1001 (rjones)
   Boot ID: c2a10a42ad324704a32e04b47621d3a9
Machine ID: 1640aa86edd8444fb3261e75d4390014
  Hostname: nufive.home.annexia.org
   Storage: 
/var/lib/systemd/coredump/core.go.1001.c2a10a42ad324704a32e04b47621d3a9.191012.165478345000.zst
   Message: Process 191012 (go) of user 1001 dumped core.

Stack trace of thread 191012:
#0  0x003fbe65b084 raise (libc.so.6 + 0x33084)
#1  0x003fbf524cca runtime.dieFromSignal (libgo.so.16 + 
0xd49cca)
#2  0x003fbf542bdc runtime.sigfwdgo (libgo.so.16 + 0xd67bdc)
#3  0x003fbfdbb800 n/a (linux-vdso.so.1 + 0x800)

GNU gdb (GDB) Fedora 10.2-4.0.riscv64.fc33
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/go.gcc...
[New LWP 191012]
[New LWP 191028]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/lp64d/libthread_db.so.1".
Core was generated by `go test -count=1 -v'.
Program terminated with signal SIGABRT, Aborted.
#0  0x003fbe65b084 in raise () from /lib64/lp64d/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install 
gcc-go-10.3.1-1.fc33.riscv64
--Type  for more, q to quit, c to continue without paging--
[Current thread is 1 (Thread 0x3fbe5f5630 (LWP 191012))]

(gdb) t a a bt

Thread 2 (Thread 0x3f99563150 (LWP 191028)):
#0  0x003fbe6cda60 in syscall () from /lib64/lp64d/libc.so.6
#1  0x003fbf246f88 in __go_syscall6 (flag=, a1=, a2=, a3=, a4=, a5=, a6=) at ../../../libgo/runtime/go-varargs.c:109
#2  0x003fbf51f05c in runtime.futex (val3=, addr2=, ts=, val=, op=, 
addr=) at ../../../libgo/go/runtime/os_linux.go:17
#3  runtime.futexsleep (addr=0x3fbfd6e878 , val=, ns=) at ../../../libgo/go/runtime/os_linux.go:59
#4  0x003fbf52d70c in 

[Libguestfs] libnbd golang failure on RISC-V

[Richard W.M. Jones]

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v 0/3] tests: Add a phony Fedora image for testing

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 11:58:32AM +0200, Laszlo Ersek wrote:
> On 06/09/22 10:46, Richard W.M. Jones wrote:
> > On Thu, Jun 09, 2022 at 10:25:08AM +0200, Laszlo Ersek wrote:
> >> On 06/09/22 10:11, Richard W.M. Jones wrote:
> >>> On Thu, Jun 09, 2022 at 10:02:54AM +0200, Laszlo Ersek wrote:
> >>>> On 06/08/22 18:48, Richard W.M. Jones wrote:
> >>>>> When we split virt-v2v from libguestfs many moons ago, I copied the
> >>>>> test-data/ subdirectory over.  I didn't modify it much, and it
> >>>>> contains much test data that is irrelevant to virt-v2v.  (This change
> >>>>> does _not_ clean up any of that ...)  However we did use the phony
> >>>>> Windows image (test-data/phony-guests/windows.img) to do a semblance
> >>>>> of testing Windows conversions, or as much as can be done without
> >>>>> having the proprietary operating system itself around.
> >>>>>
> >>>>> We never used any of the Linux images, and in fact (before this
> >>>>> change) they could not be used.
> >>>>
> >>>> Ah, indeed; for the LUKS-on-LV stuff, I synched
> >>>> "test-data/phony-guests/make-fedora-img.pl" from libguestfs and
> >>>> guestfs-tools (commits 8f2bbc3d50d8 and 27da4b0c4991), but didn't touch
> >>>> virt-v2v's copy. There was no need, and even prior differences existed.
> >>>>
> >>>> ... Indeed, libguestfs commit 0b223a287711 ("test-data: Replace
> >>>> deprecated luks_open with cryptsetup_open.", 2021-05-27) had not been
> >>>> ported to virt-v2v's copy.
> >>>
> >>> It probably makes sense to backport these commits if they apply
> >>> easily.  Can you do that?  I don't think they will conflict with this
> >>> series, but would allow us to test the luks-on-lv case additionally.
> >>
> >> I'm marking this for later in my mailbox.
> > 
> > OK I won't push anything for now.
> 
> I think your series can go in first; I don't expect conflicts due to
> your series. I do expect some navel gazing on my part for the backport
> (or forward port) per se, though... :)

OK, I pushed my series as:

   0d1b2ec1b7..3600f81ec5  master -> master

But I forgot to add the acked/reviewed tags :-(

Thanks!

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v 0/3] tests: Add a phony Fedora image for testing

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 10:25:08AM +0200, Laszlo Ersek wrote:
> On 06/09/22 10:11, Richard W.M. Jones wrote:
> > On Thu, Jun 09, 2022 at 10:02:54AM +0200, Laszlo Ersek wrote:
> >> On 06/08/22 18:48, Richard W.M. Jones wrote:
> >>> When we split virt-v2v from libguestfs many moons ago, I copied the
> >>> test-data/ subdirectory over.  I didn't modify it much, and it
> >>> contains much test data that is irrelevant to virt-v2v.  (This change
> >>> does _not_ clean up any of that ...)  However we did use the phony
> >>> Windows image (test-data/phony-guests/windows.img) to do a semblance
> >>> of testing Windows conversions, or as much as can be done without
> >>> having the proprietary operating system itself around.
> >>>
> >>> We never used any of the Linux images, and in fact (before this
> >>> change) they could not be used.
> >>
> >> Ah, indeed; for the LUKS-on-LV stuff, I synched
> >> "test-data/phony-guests/make-fedora-img.pl" from libguestfs and
> >> guestfs-tools (commits 8f2bbc3d50d8 and 27da4b0c4991), but didn't touch
> >> virt-v2v's copy. There was no need, and even prior differences existed.
> >>
> >> ... Indeed, libguestfs commit 0b223a287711 ("test-data: Replace
> >> deprecated luks_open with cryptsetup_open.", 2021-05-27) had not been
> >> ported to virt-v2v's copy.
> > 
> > It probably makes sense to backport these commits if they apply
> > easily.  Can you do that?  I don't think they will conflict with this
> > series, but would allow us to test the luks-on-lv case additionally.
> 
> I'm marking this for later in my mailbox.

OK I won't push anything for now.

Thanks,

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v 3/3] tests: Add test cases for converting the phony Fedora images

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 10:20:47AM +0200, Laszlo Ersek wrote:
> On 06/08/22 18:49, Richard W.M. Jones wrote:
> > As well as testing a full Fedora conversion which was not really
> > tested properly before, this also adds tests of conversions of Btrfs,
> > RAID and LUKS guests.
> > ---
> >  tests/Makefile.am |  8 ++
> >  tests/test-v2v-fedora-btrfs-conversion.sh | 31 +
> >  tests/test-v2v-fedora-conversion.sh   | 31 +
> >  tests/test-v2v-fedora-luks-conversion.sh  | 32 ++
> >  tests/test-v2v-fedora-md-conversion.sh| 33 +++
> >  5 files changed, 135 insertions(+)
> > 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 1244279836..63e654a3d1 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -96,6 +96,10 @@ TESTS = \
> > test-v2v-print-source.sh \
> > test-v2v-sound.sh \
> > test-v2v-virtio-win-iso.sh \
> > +   test-v2v-fedora-conversion.sh \
> > +   test-v2v-fedora-btrfs-conversion.sh \
> > +   test-v2v-fedora-luks-conversion.sh \
> > +   test-v2v-fedora-md-conversion.sh \
> > test-v2v-windows-conversion.sh \
> > rhbz1232192.sh \
> > $(SLOW_TESTS) \
> > @@ -171,6 +175,10 @@ EXTRA_DIST += \
> > test-v2v-bad-networks-and-bridges.sh \
> > test-v2v-cdrom.expected \
> > test-v2v-cdrom.sh \
> > +   test-v2v-fedora-conversion.sh \
> > +   test-v2v-fedora-btrfs-conversion.sh \
> > +   test-v2v-fedora-luks-conversion.sh \
> > +   test-v2v-fedora-md-conversion.sh \
> > test-v2v-floppy.expected \
> > test-v2v-floppy.sh \
> > test-v2v-i-disk.sh \
> 
> Under TESTS, you insert the new shell script names between
> "test-v2v-virtio-win-iso.sh" and "test-v2v-windows-conversion.sh". The
> same would be possible under EXTRA_DIST too, but there you go with
> alphabetical odering (AFAICT). Conversely, the lexicographical ordering
> (placement between cdrom and floppy) would be possible under TESTS.
> 
> Is this difference intentional?

Sort of.  I kept EXTRA_DIST in alphabetical order.

> > diff --git a/tests/test-v2v-fedora-btrfs-conversion.sh 
> > b/tests/test-v2v-fedora-btrfs-conversion.sh
> > new file mode 100755
> > index 00..c78f8ae246
> > --- /dev/null
> > +++ b/tests/test-v2v-fedora-btrfs-conversion.sh
> > @@ -0,0 +1,31 @@
> > +#!/bin/bash -
> > +# libguestfs virt-v2v test script
> > +# Copyright (C) 2014-2022 Red Hat Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> > USA.
> > +
> > +# Test virt-v2v (Phony) Fedora conversion.
> > +
> > +set -e
> > +
> > +source ./functions.sh
> > +set -e
> > +set -x
> > +
> > +skip_if_skipped
> > +f=../test-data/phony-guests/fedora-btrfs.img
> > +requires test -f $f
> > +
> > +$VG virt-v2v --debug-gc -i disk $f -o null
> > diff --git a/tests/test-v2v-fedora-conversion.sh 
> > b/tests/test-v2v-fedora-conversion.sh
> > new file mode 100755
> > index 00..92f4bbe8ea
> > --- /dev/null
> > +++ b/tests/test-v2v-fedora-conversion.sh
> > @@ -0,0 +1,31 @@
> > +#!/bin/bash -
> > +# libguestfs virt-v2v test script
> > +# Copyright (C) 2014-2022 Red Hat Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General P

Re: [Libguestfs] [PATCH virt-v2v 2/3] test-data/phony-guests: Allow virt-v2v to work against phony Fedora

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 10:13:52AM +0200, Laszlo Ersek wrote:
> Looks OK to me, I just suggest using a different function name rather
> than "basename". While the C code is certainly OK, conceptually we
> already have two standard basename() functions, a POSIX compatible one
> from , and a glibc (_GNU_SOURCE) override.
> 
> (While looking for sources on this, I stumbled upon
> , so
> apparently the glibc override does exactly what your version does. So I
> think we should either rename the internal basename() to something else,
> or just use (= statically link) the _GNU_SOURCE variant from glibc).

Renamed the function in my copy to "get_basename", and also deleted
the unnecessary include , left over from an earlier version
of the code.

Rich.

> With that:
> 
> Acked-by: Laszlo Ersek 
> 
> 
> Thanks
> Laszlo

-- 
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

Re: [Libguestfs] [PATCH virt-v2v 0/3] tests: Add a phony Fedora image for testing

[Richard W.M. Jones]

On Thu, Jun 09, 2022 at 10:02:54AM +0200, Laszlo Ersek wrote:
> On 06/08/22 18:48, Richard W.M. Jones wrote:
> > When we split virt-v2v from libguestfs many moons ago, I copied the
> > test-data/ subdirectory over.  I didn't modify it much, and it
> > contains much test data that is irrelevant to virt-v2v.  (This change
> > does _not_ clean up any of that ...)  However we did use the phony
> > Windows image (test-data/phony-guests/windows.img) to do a semblance
> > of testing Windows conversions, or as much as can be done without
> > having the proprietary operating system itself around.
> > 
> > We never used any of the Linux images, and in fact (before this
> > change) they could not be used.
> 
> Ah, indeed; for the LUKS-on-LV stuff, I synched
> "test-data/phony-guests/make-fedora-img.pl" from libguestfs and
> guestfs-tools (commits 8f2bbc3d50d8 and 27da4b0c4991), but didn't touch
> virt-v2v's copy. There was no need, and even prior differences existed.
> 
> ... Indeed, libguestfs commit 0b223a287711 ("test-data: Replace
> deprecated luks_open with cryptsetup_open.", 2021-05-27) had not been
> ported to virt-v2v's copy.

It probably makes sense to backport these commits if they apply
easily.  Can you do that?  I don't think they will conflict with this
series, but would allow us to test the luks-on-lv case additionally.

Rich.

> > They simply don't appear close enough
> > to a real guest to work with virt-v2v.  In particular they lack
> > installed kernels, modules, bootloader config and the program needed
> > to rebuild the initramfs.
> > 
> > This change fixes the Fedora image(s) so they can be used for testing,
> > and adds conversion of those to the testsuite.
> > 
> > I already pushed the first commit in this series since it was a big
> > binary blob update to the phony Fedora RPM database that was not
> > reviewable:
> > 
> > https://github.com/libguestfs/virt-v2v/commit/0d1b2ec1b733db1ca0bebf2e4a9e8d5fd7db7f93
> > 
> > Rich.
> > 
> > 
> 
> Thanks
> Laszlo

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v 2/3] test-data/phony-guests: Allow virt-v2v to work against phony Fedora

[Richard W.M. Jones]

On Wed, Jun 08, 2022 at 05:49:01PM +0100, Richard W.M. Jones wrote:
> +# Virt-v2v also needs a kernel, initrd and modules path.
> +$g->touch ("/boot/vmlinuz-$kver");
> +$g->touch ("/boot/initramfs-$kver.img");
> +$g->mkdir_p ("/lib/modules/$kver/kernel/drivers/block");
> +$g->upload ($ENV{SRCDIR}.'/../binaries/bin-x86_64-dynamic',
> +"/lib/modules/$kver/kernel/drivers/block/virtio_blk.ko");

Surprisingly this works even on !x86-64.  I just tested it on aarch64.

Here:

https://github.com/libguestfs/virt-v2v/blob/0d1b2ec1b733db1ca0bebf2e4a9e8d5fd7db7f93/convert/linux_kernels.ml#L192

we use guestfs_file_architecture on a kernel module to determine the
architecture of the kernel.  Because we uploaded an x86-64 binary this
means we detect the kernel as being x86_64:

installed kernel packages in this guest:
* kernel 5.19.0-0.rc1.14.fc37.x86_64 (x86_64) <--- ki_arch
/boot/vmlinuz-5.19.0-0.rc1.14.fc37.x86_64
/boot/initramfs-5.19.0-0.rc1.14.fc37.x86_64.img
no config
/lib/modules/5.19.0-0.rc1.14.fc37.x86_64
1 modules found
virtio: blk=true net=false rng=false balloon=false
pvpanic=false vsock=false xen=false debug=false

Then we run dracut, which would fail on a real guest if the host is
aarch64, but succeeds here because the phony dracut binary is aarch64:

libguestfs: trace: v2v: command "/sbin/dracut --verbose --add-drivers 
virtio_blk /boot/initramfs-5.19.0-0.rc1.14.fc37.x86_64.img 
5.19.0-0.rc1.14.fc37.x86_64"

Later:

gcaps_block_bus = virtio-blk
gcaps_net_bus = e1000
gcaps_virtio_rng = false
gcaps_virtio_balloon = false
gcaps_isa_pvpanic = false
gcaps_virtio_socket = false
gcaps_machine = i440fx
gcaps_arch = x86_64  <-
gcaps_acpi = true
gcaps_virtio_1_0 = false
gcaps_default_cpu = true

Anyway this is kind of a bug in virt-v2v (because it should reject
conversion of a !hostarch guest), and also in this test, but this is a
fairly obscure corner case so I guess I'll leave it for now.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH virt-v2v 2/3] test-data/phony-guests: Allow virt-v2v to work against phony Fedora

[Richard W.M. Jones]

We didn't use the phony Fedora guest before with virt-v2v (only the
phony Windows image).  This commit makes miscellaneous changes so that
it can be used for testing:

 - Add dummy rpm and dracut commands.

 - Add dummy kernel, initramfs and modules directory.

 - Add dummy grub configuration pointing to the kernel.
---
 .gitignore|  1 +
 test-data/phony-guests/Makefile.am| 19 +--
 test-data/phony-guests/fedora.c   | 67 +++
 test-data/phony-guests/make-fedora-img.pl | 26 -
 4 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/.gitignore b/.gitignore
index ac7d6a3ce0..04ab847dcd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -117,6 +117,7 @@ Makefile.in
 /test-data/phony-guests/fedora-luks.img
 /test-data/phony-guests/fedora-md1.img
 /test-data/phony-guests/fedora-md2.img
+/test-data/phony-guests/fedora-static-bin
 /test-data/phony-guests/fedora.db
 /test-data/phony-guests/guests.xml
 /test-data/phony-guests/guests-all-good.xml
diff --git a/test-data/phony-guests/Makefile.am 
b/test-data/phony-guests/Makefile.am
index 60313548af..c45ddc1123 100644
--- a/test-data/phony-guests/Makefile.am
+++ b/test-data/phony-guests/Makefile.am
@@ -76,7 +76,8 @@ blank-%.img:
 # Make a (dummy) Fedora image.
 fedora.img: make-fedora-img.pl \
fedora-journal.tar.xz \
-   fedora.db
+   fedora.db \
+   fedora-static-bin
SRCDIR=$(srcdir) LAYOUT=partitions $(top_builddir)/run --test ./$<
 
 # Make a (dummy) Fedora image using md devices
@@ -84,7 +85,8 @@ fedora-md1.img fedora-md2.img: stamp-fedora-md.img
 
 stamp-fedora-md.img: make-fedora-img.pl \
fedora-journal.tar.xz \
-   fedora.db
+   fedora.db \
+   fedora-static-bin
rm -f $@
SRCDIR=$(srcdir) LAYOUT=partitions-md $(top_builddir)/run --test ./$<
touch $@
@@ -93,13 +95,15 @@ stamp-fedora-md.img: make-fedora-img.pl \
 # for root and home.
 fedora-btrfs.img: make-fedora-img.pl \
fedora-journal.tar.xz \
-   fedora.db
+   fedora.db \
+   fedora-static-bin
SRCDIR=$(srcdir) LAYOUT=btrfs $(top_builddir)/run --test ./$<
 
 # Make a (dummy) Fedora image with LVM encrypted with LUKS.
 fedora-luks.img: make-fedora-img.pl \
fedora-journal.tar.xz \
-   fedora.db
+   fedora.db \
+   fedora-static-bin
SRCDIR=$(srcdir) LAYOUT=lvm-luks $(top_builddir)/run --test ./$<
 
 # Make a (dummy) Debian image.
@@ -137,6 +141,13 @@ fedora.db: fedora-db.sql.xz
xzcat $< | $(SQLITE3) $@-t
mv $@-t $@
 
+# This is included in the phony Fedora image to act as a phony "rpm"
+# and "dracut" command.  For the use of -all-static here, see
+# libguestfs/tests/Makefile.am
+check_PROGRAMS = fedora-static-bin
+fedora_static_bin_SOURCES = fedora.c
+fedora_static_bin_LDFLAGS = -all-static
+
 windows-software: windows-software.reg
rm -f $@ $@-t
cp $(srcdir)/minimal-hive $@-t
diff --git a/test-data/phony-guests/fedora.c b/test-data/phony-guests/fedora.c
new file mode 100644
index 00..36eaa233fc
--- /dev/null
+++ b/test-data/phony-guests/fedora.c
@@ -0,0 +1,67 @@
+/* libguestfs test images
+ * Copyright (C) 2009-2020 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
USA.
+ */
+
+/* This is "just enough" of a binary to look like RPM and dracut, as
+ * far as virt-v2v is concerned.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* NB: This is also defined in make-fedora-img.pl */
+#define KVER "5.19.0-0.rc1.14.fc37.x86_64"
+
+static const char *
+basename (const char *str)
+{
+  const char *ret = strrchr (str, '/');
+  return ret == NULL ? str : ret + 1;
+}
+
+int
+main (int argc, char *argv[])
+{
+  if (argc == 3 &&
+  strcmp (basename (argv[0]), "rpm") == 0 &&
+  strcmp (argv[1], "-ql") == 0 &&
+  strncmp (argv[2], "kernel-", 7) == 0) {
+/* XXX These files and directories actually exist.  It would be
+ * better to list files in /boot and /lib/modules matching a
+ * pattern rather than hard-coding the list here, which duplicates
+ * information in make-fedora-img.pl.
+ */
+printf ("/boot/vmlinuz-" KVER "\n");
+printf 

[Libguestfs] [PATCH virt-v2v 3/3] tests: Add test cases for converting the phony Fedora images

[Richard W.M. Jones]

As well as testing a full Fedora conversion which was not really
tested properly before, this also adds tests of conversions of Btrfs,
RAID and LUKS guests.
---
 tests/Makefile.am |  8 ++
 tests/test-v2v-fedora-btrfs-conversion.sh | 31 +
 tests/test-v2v-fedora-conversion.sh   | 31 +
 tests/test-v2v-fedora-luks-conversion.sh  | 32 ++
 tests/test-v2v-fedora-md-conversion.sh| 33 +++
 5 files changed, 135 insertions(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1244279836..63e654a3d1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -96,6 +96,10 @@ TESTS = \
test-v2v-print-source.sh \
test-v2v-sound.sh \
test-v2v-virtio-win-iso.sh \
+   test-v2v-fedora-conversion.sh \
+   test-v2v-fedora-btrfs-conversion.sh \
+   test-v2v-fedora-luks-conversion.sh \
+   test-v2v-fedora-md-conversion.sh \
test-v2v-windows-conversion.sh \
rhbz1232192.sh \
$(SLOW_TESTS) \
@@ -171,6 +175,10 @@ EXTRA_DIST += \
test-v2v-bad-networks-and-bridges.sh \
test-v2v-cdrom.expected \
test-v2v-cdrom.sh \
+   test-v2v-fedora-conversion.sh \
+   test-v2v-fedora-btrfs-conversion.sh \
+   test-v2v-fedora-luks-conversion.sh \
+   test-v2v-fedora-md-conversion.sh \
test-v2v-floppy.expected \
test-v2v-floppy.sh \
test-v2v-i-disk.sh \
diff --git a/tests/test-v2v-fedora-btrfs-conversion.sh 
b/tests/test-v2v-fedora-btrfs-conversion.sh
new file mode 100755
index 00..c78f8ae246
--- /dev/null
+++ b/tests/test-v2v-fedora-btrfs-conversion.sh
@@ -0,0 +1,31 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test virt-v2v (Phony) Fedora conversion.
+
+set -e
+
+source ./functions.sh
+set -e
+set -x
+
+skip_if_skipped
+f=../test-data/phony-guests/fedora-btrfs.img
+requires test -f $f
+
+$VG virt-v2v --debug-gc -i disk $f -o null
diff --git a/tests/test-v2v-fedora-conversion.sh 
b/tests/test-v2v-fedora-conversion.sh
new file mode 100755
index 00..92f4bbe8ea
--- /dev/null
+++ b/tests/test-v2v-fedora-conversion.sh
@@ -0,0 +1,31 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test virt-v2v (Phony) Fedora conversion.
+
+set -e
+
+source ./functions.sh
+set -e
+set -x
+
+skip_if_skipped
+f=../test-data/phony-guests/fedora.img
+requires test -f $f
+
+$VG virt-v2v --debug-gc -i disk $f -o null
diff --git a/tests/test-v2v-fedora-luks-conversion.sh 
b/tests/test-v2v-fedora-luks-conversion.sh
new file mode 100755
index 00..2922c31da1
--- /dev/null
+++ b/tests/test-v2v-fedora-luks-conversion.sh
@@ -0,0 +1,32 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, 

[Libguestfs] [PATCH virt-v2v 0/3] tests: Add a phony Fedora image for testing

[Richard W.M. Jones]

When we split virt-v2v from libguestfs many moons ago, I copied the
test-data/ subdirectory over.  I didn't modify it much, and it
contains much test data that is irrelevant to virt-v2v.  (This change
does _not_ clean up any of that ...)  However we did use the phony
Windows image (test-data/phony-guests/windows.img) to do a semblance
of testing Windows conversions, or as much as can be done without
having the proprietary operating system itself around.

We never used any of the Linux images, and in fact (before this
change) they could not be used.  They simply don't appear close enough
to a real guest to work with virt-v2v.  In particular they lack
installed kernels, modules, bootloader config and the program needed
to rebuild the initramfs.

This change fixes the Fedora image(s) so they can be used for testing,
and adds conversion of those to the testsuite.

I already pushed the first commit in this series since it was a big
binary blob update to the phony Fedora RPM database that was not
reviewable:

https://github.com/libguestfs/virt-v2v/commit/0d1b2ec1b733db1ca0bebf2e4a9e8d5fd7db7f93

Rich.


___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH virt-v2v 1/3] test-data/phony-guests: Increase size of root filesystem

[Richard W.M. Jones]

Avoid this error in virt-v2v when trying to convert the phony Fedora
guest image:

[   8.1] Checking for sufficient free disk space in the guest
virt-v2v: error: not enough free space for conversion on filesystem
‘/’.  21.6 MB free < 100 MB needed
---
 test-data/phony-guests/make-fedora-img.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test-data/phony-guests/make-fedora-img.pl 
b/test-data/phony-guests/make-fedora-img.pl
index 90492b814e..f340f4d744 100755
--- a/test-data/phony-guests/make-fedora-img.pl
+++ b/test-data/phony-guests/make-fedora-img.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/env perl
 # libguestfs
-# Copyright (C) 2010-2020 Red Hat Inc.
+# Copyright (C) 2010-2022 Red Hat Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -210,7 +210,7 @@ sub init_lvm_root {
 
 $g->pvcreate ($rootdev);
 $g->vgcreate ('VG', [$rootdev]);
-$g->lvcreate ('Root', 'VG', 32);
+$g->lvcreate ('Root', 'VG', 256);
 $g->lvcreate ('LV1', 'VG', 32);
 $g->lvcreate ('LV2', 'VG', 32);
 $g->lvcreate ('LV3', 'VG', 64);
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 4/4] convert_linux: install the QEMU guest agent with a firstboot script

[Richard W.M. Jones]

On Tue, Jun 07, 2022 at 01:59:30PM +0100, Richard W.M. Jones wrote:
> On Mon, Jun 06, 2022 at 04:19:41PM +0200, Laszlo Ersek wrote:
> > +(* Disable SELinux temporarily around package installation. 
> > Refer to
> > + * <https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c7> and
> > + * <https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c8>.
> > + *)
> > +fbs "setenforce 0"
> > +  (sprintf "#!/bin/sh\n\
> > +rm -f %s\n\
> > +if command -v getenforce >/dev/null &&\n\
> > +\ \ test Enforcing = \"$(getenforce)\"\n\
> > +then\n\
> > +\ \ touch %s\n\
> > +\ \ setenforce 0\n\
> > +fi\n" selinux_enforcing selinux_enforcing);
> > +fbs "install qga" inst_cmd;
> > +fbs "setenforce restore"
> > +  (sprintf "#!/bin/sh\n\
> > +if test -f %s; then\n\
> > +\ \ setenforce 1\n\
> > +\ \ rm -f %s\n\
> > +fi\n" selinux_enforcing selinux_enforcing);
> 
> Sounds horrible!  But if that's what is needed ...

OK, now I caught up with the BZ comments, it really seems odd to me
that a service or script can run dnf, but that dnf doesn't transition
to the right SELinux context in order to do its work, but also dnf
doesn't fail immediately ("error: wrong context!") either.

However I don't know enough about SELinux to really understand whether
this is how it's supposed to work or not.

In reply to your other comment about --firstboot-install, it is
possible that this did work but has seen been broken by some change.
I don't believe we test it thoroughly anywhere.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 4/8] python: Reformat generated methods.c in a few places

[Richard W.M. Jones]

On Tue, Jun 07, 2022 at 08:53:23AM -0500, Eric Blake wrote:
> On Tue, Jun 07, 2022 at 09:00:08AM +0100, Richard W.M. Jones wrote:
> > On Mon, Jun 06, 2022 at 09:08:29PM -0500, Eric Blake wrote:
> > > +  pr ":nbd_%s\",\n" name;
> > 
> > You could put this pr (but without the \n) ...
> > 
> > > +  pr " ";
> > > +  pr_wrap ',' (fun () ->
> > 
> > ... inside pr_wrap here, and it would mean you wouldn't need to print
> > spaces to indent (because pr_wrap should do it for you).
> 
> Will do. It rearranges a few more lines of generated code (now the
> _h argument can sometimes be a line earlier), but is still legible.
> 
> > 
> > It all looks sensible and equivalent to the old code, and the output
> > is cleaner too, so:
> > 
> > Reviewed-by: Richard W.M. Jones 
> >
> 
> I get when we have more than one statement to bundle how the OCaml
> (fun () -> ... ) construct makes sense, but do we need that for this hunk?
> 
> > @@ -284,7 +285,10 @@ let
> >  (* Generate the Python binding. *)
> >  let print_python_binding name { args; optargs; ret; may_set_error } =
> >pr "PyObject *\n";
> > -  pr "nbd_internal_py_%s (PyObject *self, PyObject *args)\n" name;
> > +  pr "nbd_internal_py_%s (" name;
> > +  pr_wrap ',' (fun () ->
> > +  pr "PyObject *self, PyObject *args");
> > +  pr ")\n";
> >pr "{\n";
> >pr "  PyObject *py_h;\n";
> >pr "  struct nbd_handle *h;\n";
> 
> or is there a shorter way to write that one?

I don't think so (except to write it all on a single line).

The pr_wrap function is a bit weird in that it requires a sub-function
that generates the buffer that it then wraps, rather than just taking
a string parameter directly.  Could imagine a pr_wrap_string function
that took the string buffer directly, but that doesn't exist today.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 8/8] python: Make nbd.Buffer lighter-weight

[Richard W.M. Jones]

 buf = malloc (sizeof *buf);
> -  if (buf == NULL) {
> -PyErr_NoMemory ();
> -return NULL;
> -  }
> -
> -  buf->initialized = false;
>if (!PyArg_ParseTuple (args, "n:nbd_internal_py_alloc_aio_buffer",
> - >len)) {
> -free (buf);
> + ))
>  return NULL;
> -  }
> 
> -  if (buf->len < 0) {
> -PyErr_SetString (PyExc_RuntimeError, "length < 0");
> -free (buf);
> -return NULL;
> -  }
> -  buf->data = malloc (buf->len);
> -  if (buf->data == NULL) {
> -PyErr_NoMemory ();
> -free (buf);
> -return NULL;
> -  }
> -
> -  ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
> -  if (ret == NULL) {
> -free (buf->data);
> -free (buf);
> -return NULL;
> -  }
> -
> -  return ret;
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  PyObject *arr = NULL;
> -  Py_ssize_t len;
> -  void *data;
> -  struct py_aio_buffer *buf;
> -  PyObject *ret;
> -
> -  if (!PyArg_ParseTuple (args,
> - "O:nbd_internal_py_aio_buffer_from_bytearray",
> - ))
> -return NULL;
> -
> -  if (! PyByteArray_Check (obj)) {
> -arr = PyByteArray_FromObject (obj);
> -if (arr == NULL)
> -  return NULL;
> -obj = arr;
> -  }
> -  data = PyByteArray_AsString (obj);
> -  if (!data) {
> -PyErr_SetString (PyExc_RuntimeError,
> - "parameter is not a bytearray or buffer");
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -  len = PyByteArray_Size (obj);
> -
> -  buf = malloc (sizeof *buf);
> -  if (buf == NULL) {
> -PyErr_NoMemory ();
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -
> -  buf->len = len;
> -  buf->data = malloc (len);
> -  if (buf->data == NULL) {
> -PyErr_NoMemory ();
> -free (buf);
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -  memcpy (buf->data, data, len);
> -  Py_XDECREF (arr);
> -  buf->initialized = true;
> -
> -  ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
> -  if (ret == NULL) {
> -free (buf->data);
> -free (buf);
> -return NULL;
> -  }
> -
> -  return ret;
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  PyObject *view;
> -  PyObject *ret;
> -
> -  if (!PyArg_ParseTuple (args,
> - "O:nbd_internal_py_aio_buffer_to_bytearray",
> - ))
> -return NULL;
> -
> -  view = nbd_internal_py_get_aio_view (obj, true);
> -  if (view == NULL)
> -return NULL;
> -
> -  ret = PyByteArray_FromObject (view);
> -  Py_DECREF (view);
> -  return ret;
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  struct py_aio_buffer *buf;
> -
> -  if (!PyArg_ParseTuple (args,
> - "O:nbd_internal_py_aio_buffer_size",
> - ))
> -return NULL;
> -
> -  buf = nbd_internal_py_get_aio_buffer (obj);
> -  if (buf == NULL)
> -return NULL;
> -
> -  return PyLong_FromSsize_t (buf->len);
> +  /* Constructing bytearray(len) in python zeroes the memory; doing it this
> +   * way gives uninitialized memory.  This correctly flags negative len.
> +   */
> +  return PyByteArray_FromStringAndSize (NULL, len);
>  }
> 
>  PyObject *
>  nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args)
>  {
> -  PyObject *obj;
> -  struct py_aio_buffer *buf;
> +  Py_buffer buf;
>Py_ssize_t offset, size;
> +  int init;
> +  PyObject *ret = NULL;
> 
>if (!PyArg_ParseTuple (args,
> -     "Onn:nbd_internal_py_aio_buffer_is_zero",
> - , , ))
> + "y*nnp:nbd_internal_py_aio_buffer_is_zero",
> + , , , ))
>  return NULL;
> 
> -  if (size == 0)
> -Py_RETURN_TRUE;
> -
> -  buf = nbd_internal_py_get_aio_buffer (obj);
> -  if (buf == NULL)
> -return NULL;
> +  if (size == 0) {
> +ret = Py_True;
> +goto out;
> +  }
> 
>/* Check the bounds of the offset. */
> -  if (offset < 0 || offset > buf->len) {
> +  if (offset < 0 || offset > buf.len) {
>  PyErr_SetString (PyExc_IndexError, "offset out of range");
> -return NULL;
> +goto out;
>}
> 
>/* Compute or check the length. */
>if (size == -1)
> -size = buf->len - offset;
> +size = buf.len - offset;
>else if (size < 0) {
>  PyErr_SetString (PyExc_IndexError,
>   "size cannot be negative, "
>   "except -1 to mean to the end of the buffer");
> -return NULL;
> +goto out;
>}
> -  else if ((size_t) offset + size > buf->len) {
> +  else if ((size_t) offset + size > buf.len) {
>  PyErr_SetString (PyExc_IndexError, "size out of range");
> -return NULL;
> +goto out;
>}
> 
> -  if (!buf->initialized || is_zero (buf->data + offset, size))
> -Py_RETURN_TRUE;
> +  if (!init || is_zero (buf.buf + offset, size))
> +ret = Py_True;
>else
> -Py_RETURN_FALSE;
> +ret = Py_False;
> + out:
> +  PyBuffer_Release ();
> +  Py_XINCREF (ret);
> +  return ret;
>  }

Looks like a nice simplification!

Reviewed-by: Richard W.M. Jones 

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

Re: [Libguestfs] [libnbd PATCH v2 7/8] python: Simplify python generator

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 09:08:32PM -0500, Eric Blake wrote:
> Now that none of our parameter types uses a getter sequence, we can
> simplify the code for generating nbd.py.  No change to generated
> output.
> ---
>  generator/Python.ml | 49 ++---
>  1 file changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 101f3e0..c49af4f 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -797,31 +797,31 @@ let
>let args =
>  List.map (
>function
> -  | Bool n -> n, None, None
> -  | BytesIn (n, _) -> n, None, None
> -  | BytesOut (_, count) -> count, None, None
> -  | BytesPersistIn (n, _) -> n, None, None
> -  | BytesPersistOut (n, _) -> n, None, None
> -  | Closure { cbname } -> cbname, None, None
> -  | Enum (n, _) -> n, None, None
> -  | Flags (n, _) -> n, None, None
> -  | Fd n | Int n -> n, None, None
> -  | Int64 n -> n, None, None
> -  | Path n -> n, None, None
> -  | SizeT n -> n, None, None
> -  | SockAddrAndLen (n, _) -> n, None, None
> -  | String n -> n, None, None
> -  | StringList n -> n, None, None
> -  | UInt n -> n, None, None
> -  | UInt32 n -> n, None, None
> -  | UInt64 n -> n, None, None
> -  | UIntPtr n -> n, None, None
> +  | Bool n -> n, None
> +  | BytesIn (n, _) -> n, None
> +  | BytesOut (_, count) -> count, None
> +  | BytesPersistIn (n, _) -> n, None
> +  | BytesPersistOut (n, _) -> n, None
> +  | Closure { cbname } -> cbname, None
> +  | Enum (n, _) -> n, None
> +  | Flags (n, _) -> n, None
> +  | Fd n | Int n -> n, None
> +  | Int64 n -> n, None
> +  | Path n -> n, None
> +  | SizeT n -> n, None
> +  | SockAddrAndLen (n, _) -> n, None
> +  | String n -> n, None
> +  | StringList n -> n, None
> +  | UInt n -> n, None
> +  | UInt32 n -> n, None
> +  | UInt64 n -> n, None
> +  | UIntPtr n -> n, None
>  ) args in
>let optargs =
>  List.map (
>function
> -  | OClosure { cbname } -> cbname, Some "None", None
> -  | OFlags (n, _, _) -> n, Some "0", None
> +  | OClosure { cbname } -> cbname, Some "None"
> +  | OFlags (n, _, _) -> n, Some "0"
>  ) optargs in
>let args = args @ optargs in
>pr "def %s(" name;
> @@ -829,8 +829,8 @@ let
>pr "self";
>List.iter (
>  function
> -| n, None, _ -> pr ", %s" n
> -| n, Some default, _ -> pr ", %s=%s" n default
> +| n, None -> pr ", %s" n
> +| n, Some default -> pr ", %s=%s" n default
>) args);
>pr "):\n";
>let longdesc = Str.global_replace py_fn_rex "C" longdesc in
> @@ -842,8 +842,7 @@ let
>pr "self._o";
>List.iter (
>  function
> -| _, _, Some getter -> pr ", %s" getter
> -| n, _, None -> pr ", %s" n
> +| n, _ -> pr ", %s" n
>) args);
>pr ")\n";
>pr "\n"

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 6/8] python: Don't unwrap nbd.Buffer in nbd.py

[Richard W.M. Jones]

(object, nbd_internal_py_get_nbd_buffer_type ())) {
> +PyObject *capsule = PyObject_GetAttrString(object, "_o");
> +
> +return PyCapsule_GetPointer (capsule, aio_buffer_name);
> +  }
> +
> +  PyErr_SetString (PyExc_TypeError,
> +   "aio_buffer: expecting nbd.Buffer instance");
> +  return NULL;
>  }
> 
>  /* Return new reference to MemoryView wrapping aio_buffer contents */
>  PyObject *
> -nbd_internal_py_get_aio_view (PyObject *capsule, bool require_init)
> +nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
>  {
> -  struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule);
> +  struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
> 
>if (!buf)
>  return NULL;
> @@ -130,9 +138,9 @@ nbd_internal_py_get_aio_view (PyObject *capsule, bool 
> require_init)
>  }
> 
>  int
> -nbd_internal_py_init_aio_buffer (PyObject *capsule)
> +nbd_internal_py_init_aio_buffer (PyObject *object)
>  {
> -  struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule);
> +  struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
> 
>assert (buf);
>buf->initialized = true;
> diff --git a/python/utils.c b/python/utils.c
> index 37f0c55..e0df181 100644
> --- a/python/utils.c
> +++ b/python/utils.c
> @@ -1,5 +1,5 @@
>  /* NBD client library in userspace
> - * Copyright (C) 2013-2019 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -155,3 +155,21 @@ nbd_internal_py_get_sockaddr (PyObject *addr,
>  return -1;
>}
>  }
> +
> +/* Obtain the type object for nbd.Buffer */
> +PyObject *
> +nbd_internal_py_get_nbd_buffer_type (void)
> +{
> +  static PyObject *type;
> +
> +  if (!type) {
> +PyObject *modname = PyUnicode_FromString ("nbd");
> +PyObject *module = PyImport_Import (modname);
> +assert (module);
> +type = PyObject_GetAttrString (module, "Buffer");
> +assert (type);
> +Py_DECREF (modname);
> +Py_DECREF (module);
> +  }
> +  return type;
> +}

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 5/8] python: Make py_aio_buffer a private struct

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 09:08:30PM -0500, Eric Blake wrote:
> Instead of having methods.c poking into the internals of a struct that
> we defined, have it use PyMemoryView* and Py_buffer*; this separation
> makes it easier to separate how we store the persistent data (for now,
> in a PyCapsule) from how we access the data.  Eventually, the use of
> PyMemoryView* will make it easier for future patches to get rid of
> some layers of copying.  nbd_internal_py_init_aio_buffer() is new; for
> now it doesn't fail, but it can in a future patch.
> 
> For now, we still have to hang on to a python reference to the
> PyCapsule object holding our malloc'd C pointer, and since the
> MemoryView we just created is not backed by a Python object, we must
> ensure we don't leak it to Python code.  But this will change in an
> upcoming patch.
> 
[...]
> @@ -477,13 +481,8 @@ let
>(* Second pass, and call the underlying C function. *)
>List.iter (
>  function
> -| BytesPersistIn (n, _) ->
> -   pr "  if (!%s_buf->initialized) {\n" n;
> -   pr "memset (%s_buf->data, 0, %s_buf->len);\n" n n;
> -   pr "%s_buf->initialized = true;\n" n;
> -   pr "  }\n"
>  | BytesPersistOut (n, _) ->
> -   pr "  %s_buf->initialized = true;\n" n
> +   pr "  if (nbd_internal_py_init_aio_buffer (%s) < 0) goto out;\n" n

Can't fail, but that was noted in the commit message so that's OK.

Reviewed-by: Richard W.M. Jones 

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 3/8] python: Enhance tests of nbd.Buffer

[Richard W.M. Jones]

00\x00\x00\x00\x00\x00\x03x'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\x80'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\x88'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\x90'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\x98'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xa0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xa8'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xb0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xb8'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xc0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xc8'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xd0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xd8'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xe0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xe8'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xf0'
> +   + b'\x00\x00\x00\x00\x00\x00\x03\xf8')
> diff --git a/python/t/510-aio-pwrite.py b/python/t/510-aio-pwrite.py
> index 89599fc..d09e249 100644
> --- a/python/t/510-aio-pwrite.py
> +++ b/python/t/510-aio-pwrite.py
> @@ -45,6 +45,12 @@ while not h.aio_command_completed(cookie):
> 
>  assert buf == buf2.to_bytearray()
> 
> +# Check that .from_bytearray() defaults to copying
> +buf[511] = 0x55
> +assert buf != buf1.to_bytearray()
> +buf[511] = 0xAA
> +assert buf == buf1.to_bytearray()
> +
>  with open(datafile, "rb") as f:
>  content = f.read()
> 

Reviewed-by: Richard W.M. Jones 

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] [libnbd PATCH v2 2/8] python: Plug uninit leak in nbd.Buffer.to_bytearray

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 09:08:27PM -0500, Eric Blake wrote:
> @@ -79,7 +80,7 @@ let
> "aio_buffer_from_bytearray";
> "aio_buffer_to_bytearray";
> "aio_buffer_size";
> -   "aio_buffer_is_zero" ] @ List.map fst handle_calls);
> +   "aio_buffer_is_zero"] @ List.map fst handle_calls);
> 
>pr "\n";
>pr "#endif /* LIBNBD_METHODS_H */\n"
> @@ -111,7 +112,7 @@ let
> "aio_buffer_from_bytearray";
> "aio_buffer_to_bytearray";
> "aio_buffer_size";
> -   "aio_buffer_is_zero" ] @ List.map fst handle_calls);
> +   "aio_buffer_is_zero"] @ List.map fst handle_calls);
>pr "  { NULL, NULL, 0, NULL }\n";
>pr "};\n";
>pr "\n";

Unintended whitespace changes?  Without those two hunks:

Reviewed-by: Richard W.M. Jones 

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 1/8] python: Improve doc comments for nbd.py

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 09:08:26PM -0500, Eric Blake wrote:
> PEP257 recommends that doc comments start with a one line summary and
> be a complete sentence.  Tweak our existing comments to comply.  It
> also recommends the use of """ strings, and for u""" when a string
> contains Unicode; however, for since it is easier to write ' than " in
> OCaml pr statements, we stick with ''' and u''' strings rather than
> follow the PEP completely.
> 
> Also, enhance the documentation for nbd.Buffer.from_bytearray to match
> the semantic changes from commit d477f7c7, before we change those
> semantics yet again in an upcoming patch (copying is inefficient, we
> are moving towards reusing the underlying buffer directly).
> ---
>  generator/Python.ml | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 3f672ba..392be87 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -763,28 +763,33 @@ let
>  '''Asynchronous I/O persistent buffer'''
> 
>  def __init__(self, len):
> -'''allocate an uninitialized AIO buffer used for nbd.aio_pread'''
> +'''Allocate an uninitialized AIO buffer used for nbd.aio_pread.'''
>  self._o = libnbdmod.alloc_aio_buffer(len)
> 
>  @classmethod
>  def from_bytearray(cls, ba):
> -'''create an AIO buffer from a bytearray'''
> +'''Create an AIO buffer from a bytearray or other buffer-like object.
> +
> +If ba is not a buffer, it is tried as the parameter to the
> +bytearray constructor.  Otherwise, ba is copied.  Either way, the
> +resulting AIO buffer is independent from the original.
> +'''
>  o = libnbdmod.aio_buffer_from_bytearray(ba)
>  self = cls(0)
>  self._o = o
>  return self
> 
>  def to_bytearray(self):
> -'''copy an AIO buffer into a bytearray'''
> +'''Copy an AIO buffer into a bytearray.'''
>  return libnbdmod.aio_buffer_to_bytearray(self._o)
> 
>  def size(self):
> -'''return the size of an AIO buffer'''
> +'''Return the size of an AIO buffer.'''
>  return libnbdmod.aio_buffer_size(self._o)
> 
>  def is_zero(self, offset=0, size=-1):
> -'''
> -Returns true if and only if all bytes in the buffer are zeroes.
> +'''Returns true if and only if all bytes in the buffer are zeroes.
> +
>  Note that a freshly allocated buffer is uninitialized, not zero.
> 
>  By default this tests the whole buffer, but you can restrict
> @@ -801,11 +806,11 @@ let
>  '''NBD handle'''
> 
>  def __init__(self):
> -'''create a new NBD handle'''
> +'''Create a new NBD handle.'''
>  self._o = libnbdmod.create()
> 
>  def __del__(self):
> -'''close the NBD handle and underlying connection'''
> +'''Close the NBD handle and underlying connection.'''
>  if hasattr(self, '_o'):
>  libnbdmod.close(self._o)
> 
> @@ -855,7 +860,7 @@ let
>let longdesc = Str.global_replace py_fn_rex "C" longdesc in
>let longdesc = Str.global_replace py_const_rex "C<" longdesc in
>let longdesc = pod2text longdesc in
> -  pr "    '''▶ %s\n\n%s'''\n" shortdesc (String.concat "\n" 
> longdesc);
> +  pr "u'''▶ %s\n\n%s'''\n" shortdesc (String.concat "\n" 
> longdesc);
>pr "return libnbdmod.%s(" name;
>pr_wrap ',' (fun () ->
>pr "self._o";

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [guestfs-tools PATCH] customize: rebase to the common/mlcustomize/Guest_packages interface

[Richard W.M. Jones]

This patch and the companion patch to libguestfs-common:

Reviewed-by: Richard W.M. Jones 

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 0/4] convert_linux: install the QEMU guest agent with a firstboot script

[Richard W.M. Jones]

On Tue, Jun 07, 2022 at 01:39:53PM +0100, Richard W.M. Jones wrote:
> OTOH ... my comment here:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c2
> 
> was about more social issues where we've not been able to put the
> qemu-ga RPMs on the ISO, and firstboot seemed like the easiest way to
> get around that.  AFAIK there is no way for a host subscribed to (eg)
> RHEL 9 channels to download RHEL 7/8 RPMs.  Maybe even hard to grab
> SUSE or Debian packages.

Just to clarify this.

Obviously virt-customize has an --install option which can install
packages into Linux guests.  This works because the network is
available and the yum/apt/etc commands within the guests are able to
connect out to the distro package repositories.

Virt-v2v is however a bit special here.  While we might simply use the
exact same method as virt-customize (and the network is also enabled),
we have historically preferred not to do operations which require
network access.

This is mainly for the benefit of Red Hat's downstream customers.
They may run virt-v2v in an environment where there is no network, or
the guest may not be subscribed to RHN without some manual
intervention.

I wonder if we want to revisit this assumption?  From an upstream
point of view, using the network while virt-v2v is running to install
qemu-ga from the distro repositories is fine (and considerably
simpler).  Even from a downstream point of view I guess it would work
in most cases, and maybe installing qemu-ga is a "nice-to-have" so as
long as it doesn't hard-fail the whole conversion, would that be
sufficient?  RH customers with isolated networks can install qemu-ga
themselves using Ansible post-conversion scripts etc.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 4/4] convert_linux: install the QEMU guest agent with a firstboot script

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 04:19:41PM +0200, Laszlo Ersek wrote:
> Register a firstboot script, for installing the guest agent with the
> guest's own package manager -- that is, "Guest_packages.install_command".
> 
> For installing the package, network connectivity is required; for lack of
> a universal "wait online" dependency expression, register "sleep 60" as a
> preceding firstboot script.
> 
> The source domain's SELinux policy may not allow our firstboot service to
> execute the package's installation scripts (if any). For that reason,
> temporarily disable SELinux around package installation.
> 
> After installation, register another script for launching the agent.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
> Signed-off-by: Laszlo Ersek 
> ---
>  convert/convert_linux.ml | 73 +++-
>  common   |  2 +-
>  2 files changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
> index 2ddbc07aa86a..1dcd7abbcee2 100644
> --- a/convert/convert_linux.ml
> +++ b/convert/convert_linux.ml
> @@ -562,8 +562,77 @@ let convert (g : G.guestfs) source inspect 
> keep_serial_console _ =
>name = qga_pkg
>) inspect.i_apps in
>  if not has_qemu_guest_agent then
> -  (* FIXME -- install qemu-guest-agent here *)
> -  ()
> +  let pkg_mgmt = g#inspect_get_package_management inspect.i_root in

You can just use: inspect.i_package_management to avoid round-tripping
through the appliance for information that we already have locally.

> +  try
> +let inst_cmd = Guest_packages.install_command [qga_pkg] pkg_mgmt 
> in
> +
> +(* Use only the portable filename character set in this. *)
> +let selinux_enforcing = "/root/virt-v2v-fb-selinux-enforcing" in
> +let fbs =
> +  Firstboot.add_firstboot_script g inspect.i_root
> +in
> +info (f_"The QEMU Guest Agent will be installed for this guest 
> at \
> + first boot.");
> +
> +(* Waiting for the guest to go "online" is very complicated. On
> + * systemd-based distros, our firstboot service could perhaps 
> depend
> + * on "systemd-networkd-wait-online.service" or
> + * "NetworkManager-wait-online.service". But not all Linux 
> distros
> + * are based on systemd, and even with systemd, a guest could use
> + * systemd-networkd vs. NetworkManager... So just do what we do 
> in
> + * various Windows firstboot scripts: wait. We could cut the 
> waiting
> + * short by pinging a well-known IP address, or by resolving a
> + * well-known FQDN, but those might qualify as "phoning home"... 
> So
> + * just wait.
> + *)
> +fbs "wait online"
> +  "#!/bin/sh\n\
> +   sleep 60\n";

Hmmm.

virt-p2v uses:

  nm-online -t 30

Apparently systemd-networkd (which I've never knowingly used) has
another command:

  /usr/lib/systemd/systemd-networkd-wait-online

which ought to do the same thing.  (It defaults to 120 second timeout.)
Can we not do something like:

  nm-online -t 30 ||:
  /usr/lib/systemd/systemd-networkd-wait-online ||:

as a best effort?  Seems a lot better idea than sleeping.

Long term we really ought to add this as a feature of the Firstboot
module, ie. a parameter which allows a firstboot script to be flagged
as needing the network, and we'll insert the appropriate code for
those scripts automatically.

> +(* Disable SELinux temporarily around package installation. 
> Refer to
> + *  and
> + * .
> + *)
> +fbs "setenforce 0"
> +  (sprintf "#!/bin/sh\n\
> +rm -f %s\n\
> +if command -v getenforce >/dev/null &&\n\
> +\ \ test Enforcing = \"$(getenforce)\"\n\
> +then\n\
> +\ \ touch %s\n\
> +\ \ setenforce 0\n\
> +fi\n" selinux_enforcing selinux_enforcing);
> +fbs "install qga" inst_cmd;
> +fbs "setenforce restore"
> +  (sprintf "#!/bin/sh\n\
> +if test -f %s; then\n\
> +\ \ setenforce 1\n\
> +\ \ rm -f %s\n\
> +fi\n" selinux_enforcing selinux_enforcing);

Sounds horrible!  But if that's what is needed ...

> +(* Start the agent now and at subsequent boots. The following
> + * commands should work on both sysvinit distros / distro 
> versions
> + * (regardless of "/etc/rc.d/" vs. "/etc/init.d/" being the 
> scheme
> + * in use) and systemd 

Re: [Libguestfs] [v2v PATCH 3/4] convert_linux: extract qemu-guest-agent package name

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 04:19:40PM +0200, Laszlo Ersek wrote:
> In commit a30383e35d34 ("v2v: linux: do not install qemu-guest-agent if
> already installed", 2019-09-20), the name of the package providing the
> QEMU guest agent was hard-coded as "qemu-guest-agent", regardless of
> distro family. Turns out this is actually correct (and may have been
> intentional, only it was not specifically documented): in all OS families
> currently recognized by our "family" function (`RHEL_family, `ALT_family,
> `SUSE_family, `Debian_family), the *binary* package is indeed called
> "qemu-guest-agent":
> 
> - https://brewweb.engineering.redhat.com/brew/packageinfo?packageID=47646
> - 
> http://rpmfind.net/linux/rpm2html/search.php?query=qemu-guest-agent=Search+...==
> - https://packages.altlinux.org/en/sisyphus/srpms/qemu/
> - 
> https://packages.debian.org/search?keywords=qemu-guest-agent=names=all=all
> 
> As a way of documenting this, extract the mapping to a new helper function
> named "qga_pkg_of_family".
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
> Signed-off-by: Laszlo Ersek 
> ---
>  convert/convert_linux.ml | 33 +++-
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
> index 79462aa1e7ae..2ddbc07aa86a 100644
> --- a/convert/convert_linux.ml
> +++ b/convert/convert_linux.ml
> @@ -56,6 +56,16 @@ let convert (g : G.guestfs) source inspect 
> keep_serial_console _ =
>  | "debian" | "ubuntu" | "linuxmint" | "kalilinux" -> `Debian_family
>  | _ -> assert false in
>  
> +  (* map the OS family name to the qemu-guest-agent package name *)
> +  let qga_pkg_of_family =
> +function
> +| `RHEL_family
> +| `ALT_family
> +| `SUSE_family
> +| `Debian_family -> Some "qemu-guest-agent"
> +| _ -> None
> +  in
> +
>assert (inspect.i_package_format = "rpm" || inspect.i_package_format = 
> "deb");
>  
>(* Fail early if i_apps is empty.  Certain steps such as kernel
> @@ -539,14 +549,21 @@ let convert (g : G.guestfs) source inspect 
> keep_serial_console _ =
>  
>and install_linux_tools () =
>  (* It is not fatal if we fail to install the QEMU guest agent. *)
> -let has_qemu_guest_agent =
> -  List.exists (
> -fun { G.app2_name = name } ->
> -  name = "qemu-guest-agent"
> -  ) inspect.i_apps in
> -if not has_qemu_guest_agent then
> -  (* FIXME -- install qemu-guest-agent here *)
> -  ()
> +match qga_pkg_of_family family with
> +| None -> warning (f_"The name of the package that provides the QEMU 
> Guest \
> +  Agent for this guest OS is unknown.  The guest 
> agent \
> +  will not be installed.  Please consider reporting 
> a \
> +  bug according to the BUGS section of the 
> virt-v2v(1) \
> +  manual.")
> +| Some qga_pkg ->
> +let has_qemu_guest_agent =
> +  List.exists (
> +fun { G.app2_name = name } ->
> +  name = qga_pkg
> +  ) inspect.i_apps in
> +if not has_qemu_guest_agent then
> +  (* FIXME -- install qemu-guest-agent here *)
> +  ()
>  
>and configure_kernel () =
>  (* Previously this function would try to install kernels, but we
> -- 
> 2.19.1.3.g30247aa5d201

Neutral change, so:

Reviewed-by: Richard W.M. Jones 

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 2/4] windows_virtio: remove "install_linux_tools"

[Richard W.M. Jones]

I'm not a big fan of dead code/features, so we should probably just do
this regardless of any other changes, so:

Acked-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 1/4] output/create_libvirt_xml: wire up the QEMU guest agent

[Richard W.M. Jones]

The patch looks OK, but qemu-ga supports vsock (-m listen-vsock), so
wouldn't it be easier to use that?  I thought that virtio-serial was
unmaintained these days and so vsock would be preferred.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 0/4] convert_linux: install the QEMU guest agent with a firstboot script

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 04:19:37PM +0200, Laszlo Ersek wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
> 
> I'm going to post the pre-requisite libguestfs-common and guestfs-tools
> patches (one patch for each project) in response to this cover letter,
> too.
> 
> I'm not sure why we want to perform the installation specifically at
> *firstboot* .
> Virt-v2v already only supports conversions where host and guest arches
> are identical; thus, the appliance kernel could run native guest
> binaries (if necessary). I've read
> , but I'm not
> overly convinced. Firstboot comes with *lots* of complications. Even
> 
> mentions some of them:
> 
> > The downsides are that it will take the guest a lot longer to boot
> > first time, and there’s nothing much you can do if package
> > installation fails (eg. if a network problem means the guest can't
> > reach the package repositories).

It is true that for Linux guests, and because virt-v2v assumes host
arch == guest arch, it is usually possible to run commands in the
guest at conversion time.  This is why we can run (for example)
dracut/mkinitrd to regenerate the initramfs.  Firstboot is mainly
useful for Windows where we cannot easily run Windows binaries at
conversion time.

It's possible there is still an issue with conversions where the host
kernel is older or newer than the guest kernel (especially older host)
and so might not be able to run the qemu-ga installation commands.
We've not seen that with dracut so far.

SELinux is mentioned in the bug, but should not be a concern.
(Virt-v2v runs commands in an appliance that has no SELinux, and we
relabel the guest filesystem after that.)

So firstboot shouldn't be necessary from a technical point of view.

OTOH ... my comment here:

https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c2

was about more social issues where we've not been able to put the
qemu-ga RPMs on the ISO, and firstboot seemed like the easiest way to
get around that.  AFAIK there is no way for a host subscribed to (eg)
RHEL 9 channels to download RHEL 7/8 RPMs.  Maybe even hard to grab
SUSE or Debian packages.

> Anyway, here goes.

I'll go ahead and review it as it is now!

Rich.

> Thanks,
> Laszlo
> 
> Laszlo Ersek (4):
>   output/create_libvirt_xml: wire up the QEMU guest agent
>   windows_virtio: remove "install_linux_tools"
>   convert_linux: extract qemu-guest-agent package name
>   convert_linux: install the QEMU guest agent with a firstboot script
> 
>  common   |   2 +-
>  convert/convert_linux.ml | 102 ++--
>  convert/linux.ml |  35 ---
>  convert/linux.mli|  11 ---
>  convert/windows_virtio.ml|  42 
>  convert/windows_virtio.mli   |   4 -
>  output/create_libvirt_xml.ml |  11 +++
>  tests/test-v2v-i-ova.xml |   4 +
>  8 files changed, 111 insertions(+), 100 deletions(-)
> 
> -- 
> 2.19.1.3.g30247aa5d201
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs

-- 
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH v2 4/8] python: Reformat generated methods.c in a few places

[Richard W.M. Jones]

On Mon, Jun 06, 2022 at 09:08:29PM -0500, Eric Blake wrote:
> +  pr ":nbd_%s\",\n" name;

You could put this pr (but without the \n) ...

> +  pr " ";
> +  pr_wrap ',' (fun () ->

... inside pr_wrap here, and it would mean you wouldn't need to print
spaces to indent (because pr_wrap should do it for you).

It all looks sensible and equivalent to the old code, and the output
is cleaner too, so:

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 5/5] python: Accept all buffer-like objects in aio_p{read, write}

[Richard W.M. Jones]

On Fri, Jun 03, 2022 at 05:26:35PM -0500, Eric Blake wrote:
> After the work in the previous patch, this one is a trivial feature
> addition ;)  Now you can do h.aio_pwrite(b'123', 0).
> ---
>  python/handle.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/python/handle.c b/python/handle.c
> index 7f67159..8575097 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -104,6 +104,11 @@ nbd_internal_py_get_aio_buffer (PyObject *buffer)
>if (PyObject_IsInstance (buffer, nbd_internal_py_get_nbd_buffer_type ()))
>  return PyObject_GetAttrString(buffer, "_o");
> 
> +  if (PyObject_CheckBuffer (buffer)) {
> +Py_INCREF (buffer);
> +return buffer;
> +  }
> +
>PyErr_SetString (PyExc_TypeError,
> "aio_buffer: expecting nbd.Buffer instance");
>return NULL;

Sorry, a late comment on this one.  Should we add a test for this,
since it's plausibly something that could regress in future?

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 5/5] python: Accept all buffer-like objects in aio_p{read, write}

[Richard W.M. Jones]

On Fri, Jun 03, 2022 at 05:26:35PM -0500, Eric Blake wrote:
> After the work in the previous patch, this one is a trivial feature
> addition ;)  Now you can do h.aio_pwrite(b'123', 0).
> ---
>  python/handle.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/python/handle.c b/python/handle.c
> index 7f67159..8575097 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -104,6 +104,11 @@ nbd_internal_py_get_aio_buffer (PyObject *buffer)
>if (PyObject_IsInstance (buffer, nbd_internal_py_get_nbd_buffer_type ()))
>  return PyObject_GetAttrString(buffer, "_o");
> 
> +  if (PyObject_CheckBuffer (buffer)) {
> +Py_INCREF (buffer);
> +return buffer;
> +  }
> +
>PyErr_SetString (PyExc_TypeError,
> "aio_buffer: expecting nbd.Buffer instance");
>return NULL;

Reviewed-by: Richard W.M. Jones 

Looks like a good series, thanks.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 4/5] python: Make nbd.Buffer lighter-weight

[Richard W.M. Jones]

(buffer, nbd_internal_py_get_nbd_buffer_type ()))
> +return PyObject_GetAttrString(buffer, "_o");
> 
>PyErr_SetString (PyExc_TypeError,
> "aio_buffer: expecting nbd.Buffer instance");
>return NULL;
>  }
> 
> -static void
> -free_aio_buffer (PyObject *capsule)
> -{
> -  struct py_aio_buffer *buf = PyCapsule_GetPointer (capsule, 
> aio_buffer_name);
> -
> -  if (buf)
> -free (buf->data);
> -  free (buf);
> -}
> -
>  /* Allocate a persistent buffer used for nbd_aio_pread. */
>  PyObject *
>  nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args)
>  {
> -  struct py_aio_buffer *buf;
> -  PyObject *ret;
> -
> -  buf = malloc (sizeof *buf);
> -  if (buf == NULL) {
> -PyErr_NoMemory ();
> -return NULL;
> -  }
> -
> -  if (!PyArg_ParseTuple (args, (char *) "n:nbd_internal_py_alloc_aio_buffer",
> - >len)) {
> -free (buf);
> -return NULL;
> -  }
> -
> -  if (buf->len < 0) {
> -PyErr_SetString (PyExc_RuntimeError, "length < 0");
> -free (buf);
> -return NULL;
> -  }
> -  buf->data = malloc (buf->len);
> -  if (buf->data == NULL) {
> -PyErr_NoMemory ();
> -free (buf);
> -return NULL;
> -  }
> -
> -  ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
> -  if (ret == NULL) {
> -free (buf->data);
> -free (buf);
> -return NULL;
> -  }
> -
> -  return ret;
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  PyObject *arr = NULL;
>Py_ssize_t len;
> -  void *data;
> -  struct py_aio_buffer *buf;
> -  PyObject *ret;
> 
> -  if (!PyArg_ParseTuple (args,
> - (char *) 
> "O:nbd_internal_py_aio_buffer_from_bytearray",
> - ))
> +  if (!PyArg_ParseTuple (args, (char *) "n:nbd_internal_py_alloc_aio_buffer",
> + ))
>  return NULL;
> 
> -  if (! PyByteArray_Check (obj)) {
> -arr = PyByteArray_FromObject (obj);
> -if (arr == NULL)
> -  return NULL;
> -obj = arr;
> -  }
> -  data = PyByteArray_AsString (obj);
> -  if (!data) {
> -PyErr_SetString (PyExc_RuntimeError,
> - "parameter is not a bytearray or buffer");
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -  len = PyByteArray_Size (obj);
> -
> -  buf = malloc (sizeof *buf);
> -  if (buf == NULL) {
> -PyErr_NoMemory ();
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -
> -  buf->len = len;
> -  buf->data = malloc (len);
> -  if (buf->data == NULL) {
> -PyErr_NoMemory ();
> -free (buf);
> -Py_XDECREF (arr);
> -return NULL;
> -  }
> -  memcpy (buf->data, data, len);
> -  Py_XDECREF (arr);
> -
> -  ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
> -  if (ret == NULL) {
> -free (buf->data);
> -free (buf);
> -return NULL;
> -  }
> -
> -  return ret;
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  struct py_aio_buffer *buf;
> -
> -  if (!PyArg_ParseTuple (args,
> - (char *) 
> "O:nbd_internal_py_aio_buffer_to_bytearray",
> - ))
> -return NULL;
> -
> -  buf = nbd_internal_py_get_aio_buffer (obj);
> -  if (buf == NULL)
> -return NULL;
> -
> -  return PyByteArray_FromStringAndSize (buf->data, buf->len);
> -}
> -
> -PyObject *
> -nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args)
> -{
> -  PyObject *obj;
> -  struct py_aio_buffer *buf;
> -
> -  if (!PyArg_ParseTuple (args,
> - (char *) "O:nbd_internal_py_aio_buffer_size",
> - ))
> -return NULL;
> -
> -  buf = nbd_internal_py_get_aio_buffer (obj);
> -  if (buf == NULL)
> -return NULL;
> -
> -  return PyLong_FromSsize_t (buf->len);
> +  /* Constructing bytearray(len) in python zeroes the memory; doing it this
> +   * way gives uninitialized memory.  This correctly flags negative len.
> +   */
> +  return PyByteArray_FromStringAndSize (NULL, len);
>  }
> 
>  PyObject *
>  nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args)
>  {
> -  PyObject *obj;
> -  struct py_aio_buffer *buf;
> +  Py_buffer buf;
>Py_ssize_t offset, size;
> +  PyObject *ret = NULL;
> 
>if (!PyArg_ParseTuple (args,
> - (char *) "Onn:nbd_internal_py_aio_buffer_is_zero",
> - , , ))
> + (char *) "y*nn:nbd_internal_py_aio_buffer_is_zero",
> + , , ))
>  return NULL;
> 
> -  if (size == 0)
> -Py_RETURN_TRUE;
> -
> -  buf = nbd_internal_py_get_aio_buffer (obj);
> -  if (buf == NULL)
> -return NULL;
> +  if (size == 0) {
> +ret = Py_True;
> +Py_INCREF (ret);
> +goto out;
> +  }
> 
>/* Check the bounds of the offset. */
> -  if (offset < 0 || offset > buf->len) {
> +  if (offset < 0 || offset > buf.len) {
>  PyErr_SetString (PyExc_IndexError, "offset out of range");
> -return NULL;
> +goto out;
>}
> 
>/* Compute or check the length. */
>if (size == -1)
> -size = buf->len - offset;
> +size = buf.len - offset;
>else if (size < 0) {
>  PyErr_SetString (PyExc_IndexError,
>   "size cannot be negative, "
>   "except -1 to mean to the end of the buffer");
> -return NULL;
> +goto out;
>}
> -  else if ((size_t) offset + size > buf->len) {
> +  else if ((size_t) offset + size > buf.len) {
>  PyErr_SetString (PyExc_IndexError, "size out of range");
> -return NULL;
> +goto out;
>}
> 
> -  if (is_zero (buf->data + offset, size))
> -Py_RETURN_TRUE;
> -  else
> -Py_RETURN_FALSE;
> +  ret = PyBool_FromLong (is_zero (buf.buf + offset, size));
> + out:
> +  PyBuffer_Release();
> +  return ret;
>  }
> -- 
> 2.36.1
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs

Reviewed-by: Richard W.M. Jones 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 3/5] python: Simplify python generator

[Richard W.M. Jones]

On Fri, Jun 03, 2022 at 05:26:33PM -0500, Eric Blake wrote:
> Now that none of our parameter types uses a getter sequence, we can
> simplify the code for generating nbd.py.
> ---
>  generator/Python.ml | 49 ++---
>  1 file changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index fcab6bd..b862b44 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -816,31 +816,31 @@ let
>let args =
>  List.map (
>function
> -  | Bool n -> n, None, None
> -  | BytesIn (n, _) -> n, None, None
> -  | BytesOut (_, count) -> count, None, None
> -  | BytesPersistIn (n, _) -> n, None, None
> -  | BytesPersistOut (n, _) -> n, None, None
> -  | Closure { cbname } -> cbname, None, None
> -  | Enum (n, _) -> n, None, None
> -  | Flags (n, _) -> n, None, None
> -  | Fd n | Int n -> n, None, None
> -  | Int64 n -> n, None, None
> -  | Path n -> n, None, None
> -  | SizeT n -> n, None, None
> -  | SockAddrAndLen (n, _) -> n, None, None
> -  | String n -> n, None, None
> -  | StringList n -> n, None, None
> -  | UInt n -> n, None, None
> -  | UInt32 n -> n, None, None
> -  | UInt64 n -> n, None, None
> -  | UIntPtr n -> n, None, None
> +  | Bool n -> n, None
> +  | BytesIn (n, _) -> n, None
> +  | BytesOut (_, count) -> count, None
> +  | BytesPersistIn (n, _) -> n, None
> +  | BytesPersistOut (n, _) -> n, None
> +  | Closure { cbname } -> cbname, None
> +  | Enum (n, _) -> n, None
> +  | Flags (n, _) -> n, None
> +  | Fd n | Int n -> n, None
> +  | Int64 n -> n, None
> +  | Path n -> n, None
> +  | SizeT n -> n, None
> +  | SockAddrAndLen (n, _) -> n, None
> +  | String n -> n, None
> +  | StringList n -> n, None
> +  | UInt n -> n, None
> +  | UInt32 n -> n, None
> +  | UInt64 n -> n, None
> +  | UIntPtr n -> n, None
>  ) args in
>let optargs =
>  List.map (
>function
> -  | OClosure { cbname } -> cbname, Some "None", None
> -  | OFlags (n, _, _) -> n, Some "0", None
> +  | OClosure { cbname } -> cbname, Some "None"
> +  | OFlags (n, _, _) -> n, Some "0"
>  ) optargs in
>let args = args @ optargs in
>pr "def %s(" name;
> @@ -848,8 +848,8 @@ let
>pr "self";
>List.iter (
>  function
> -| n, None, _ -> pr ", %s" n
> -| n, Some default, _ -> pr ", %s=%s" n default
> +| n, None -> pr ", %s" n
> +| n, Some default -> pr ", %s=%s" n default
>) args);
>pr "):\n";
>let longdesc = Str.global_replace py_fn_rex "C" longdesc in
> @@ -861,8 +861,7 @@ let
>pr "self._o";
>List.iter (
>  function
> -| _, _, Some getter -> pr ", %s" getter
> -| n, _, None -> pr ", %s" n
> +| n, _ -> pr ", %s" n
>) args);
>pr ")\n";
>pr "\n"

Reviewed-by: Richard W.M. Jones 

-- 
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 2/5] python: Don't unwrap nbd.Buffer in nbd.py

[Richard W.M. Jones]

o(self, offset, size)
> 
> 
>  class NBD(object):
> @@ -819,8 +819,8 @@ let
>| Bool n -> n, None, None
>| BytesIn (n, _) -> n, None, None
>| BytesOut (_, count) -> count, None, None
> -  | BytesPersistIn (n, _) -> n, None, Some (sprintf "%s._o" n)
> -  | BytesPersistOut (n, _) -> n, None, Some (sprintf "%s._o" n)
> +  | BytesPersistIn (n, _) -> n, None, None
> +  | BytesPersistOut (n, _) -> n, None, None
>| Closure { cbname } -> cbname, None, None
>| Enum (n, _) -> n, None, None
>| Flags (n, _) -> n, None, None

(Checks patch #3 ... yes ... this code does get simplified further :-)

> diff --git a/python/handle.c b/python/handle.c
> index 9fe3f8e..f84c6e0 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -99,9 +99,16 @@ nbd_internal_py_display_version (PyObject *self, PyObject 
> *args)
>  static const char aio_buffer_name[] = "nbd.Buffer";
> 
>  struct py_aio_buffer *
> -nbd_internal_py_get_aio_buffer (PyObject *capsule)
> +nbd_internal_py_get_aio_buffer (PyObject *buffer)
>  {
> -  return PyCapsule_GetPointer (capsule, aio_buffer_name);
> +  if (PyObject_IsInstance (buffer, nbd_internal_py_get_nbd_buffer_type ())) {
> +PyObject *capsule = PyObject_GetAttrString(buffer, "_o");
> +    return PyCapsule_GetPointer (capsule, aio_buffer_name);
> +  }
> +
> +  PyErr_SetString (PyExc_TypeError,
> +   "aio_buffer: expecting nbd.Buffer instance");
> +  return NULL;
>  }
> 
>  static void
> diff --git a/python/utils.c b/python/utils.c
> index 37f0c55..bc7d6a1 100644
> --- a/python/utils.c
> +++ b/python/utils.c
> @@ -1,5 +1,5 @@
>  /* NBD client library in userspace
> - * Copyright (C) 2013-2019 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -155,3 +155,21 @@ nbd_internal_py_get_sockaddr (PyObject *addr,
>  return -1;
>}
>  }
> +
> +/* Obtain the type object for nbd.Buffer */
> +PyObject *
> +nbd_internal_py_get_nbd_buffer_type (void)
> +{
> +  static PyObject *type;
> +
> +  if (!type) {
> +PyObject *modname = PyUnicode_FromString ("nbd");
> +PyObject *module = PyImport_Import (modname);
> +assert (module);
> +type = PyObject_GetAttrString (module, "Buffer");
> +Py_DECREF (modname);
> +Py_DECREF (module);
> +  }
> +  assert (type);
> +  return type;
> +}

Reviewed-by: Richard W.M. Jones 

-- 
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] [libnbd PATCH 1/5] python: Avoid memleak on (unlikely) module failure

[Richard W.M. Jones]

On Fri, Jun 03, 2022 at 05:26:31PM -0500, Eric Blake wrote:
> Python 3.10 added PyModule_AddObjectRef() to more easily avoid a
> common memory leak when ading to a module fails (unlikely in our case,
> since we initialize early in the python process, but still something
> we must worry about for corner-case correctness).  But since we target
> older Python, we must check for errors and clean up ourselves.
> 
> Fixes: 259d46cb ("python: Raise a custome exception containing error string 
> and errno.", v0.1.6)
> ---
>  generator/Python.ml | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 1c4446e..3f672ba 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -143,9 +143,11 @@ let
>  return NULL;
> 
>nbd_internal_py_Error = PyErr_NewException (\"nbd.Error\", NULL, NULL);
> -  if (nbd_internal_py_Error == NULL)
> +  if (PyModule_AddObject (mod, \"Error\", nbd_internal_py_Error) < 0) {
> +Py_XDECREF (nbd_internal_py_Error);
> +Py_DECREF (mod);
>  return NULL;
> -  PyModule_AddObject (mod, \"Error\", nbd_internal_py_Error);
> +  }
> 

Reviewed-by: Richard W.M. Jones 

-- 
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

Re: [Libguestfs] [libnbd PATCH 3/2] python: Slice pread_structured buffer from original

[Richard W.M. Jones]

I agree with Nir's feedback and R-b's and don't have anything else to add.

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

Re: [Libguestfs] [libnbd PATCH 3/2] python: Slice pread_structured buffer from original

[Richard W.M. Jones]

On Tue, May 31, 2022 at 07:24:03PM -0500, Eric Blake wrote:
> On Wed, Jun 01, 2022 at 02:20:48AM +0300, Nir Soffer wrote:
> > Does this work?
> > 
> > PySlice_New(NULL, NULL, NULL);
> > PySlice_AdjustIndices(length, start, stop, step);
> 
> New in python 3.6.1.  README says we still target python 3.3.

There are two things we could do here.  Either move the baseline up.
Python 3.6.1 was released in March 2017 (5 years ago), and more
importantly is available in RHEL 7 which is the oldest Linux distro we
care about.

Or (harder, not necessarily better) detect the required function in
configure.ac, and add a utility function to hide the difference.

> Worse, look at the signature - it does NOT take a PyObject *, and
> therefore it cannot modify an existing slice object (rather, it is
> for easing the computation of how to turn [-1:-2:2] into a slice
> with positive bounds) - you still have to go through a step that
> converts integers into PyObject* before creating a PySlice object.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 3/2] python: Slice pread_structured buffer from original

[Richard W.M. Jones]

On Tue, May 31, 2022 at 10:52:35AM -0500, Eric Blake wrote:
> And I really wish python didn't make it so hard to grab a slice of
> another object using C code.  Having to create 3 temporary PyObjects
> instead of having a utility C function that takes normal integers was
> annoying.

You can add hand-written utility functions to python/utils.c if that
makes things easier.  Note you will need to add a prototype for the
new function(s) to the generated code in python/methods.h.

See nbd_internal_py_get_string_list for an example of this.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] linuxdisk: Reduce size of test

[Richard W.M. Jones]

On Tue, May 31, 2022 at 04:10:07PM -0500, Eric Blake wrote:
> Using all of ../plugins as the contents for the linuxdisk gets
> progressively bigger over time with incremental builds.  Before I
> cleaned it up, my plugins/rust/target/debug had accumulated over 6G of
> cruft, causing test-linuxdisk-copy-out.sh to fail due to ENOSPC during
> mke2fs.  Pick a smaller directory tree to export, but still test that
> we expose a subdirectory.
> 
> Fixes: 5da0f2fc9 ("Add linuxdisk plugin.")
> ---
>  plugins/linuxdisk/subdir/.gitignore |  0
>  tests/test-linuxdisk-copy-out.sh| 18 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
>  create mode 100644 plugins/linuxdisk/subdir/.gitignore
> 
> diff --git a/plugins/linuxdisk/subdir/.gitignore 
> b/plugins/linuxdisk/subdir/.gitignore
> new file mode 100644
> index ..e69de29b
> diff --git a/tests/test-linuxdisk-copy-out.sh 
> b/tests/test-linuxdisk-copy-out.sh
> index 3a38fb58..f926a55c 100755
> --- a/tests/test-linuxdisk-copy-out.sh
> +++ b/tests/test-linuxdisk-copy-out.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2018-2020 Red Hat Inc.
> +# Copyright (C) 2018-2022 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -49,25 +49,25 @@ cleanup_fn rm -f $files
> 
>  nbdkit -f -v -U - \
> --filter=partition \
> -   linuxdisk $srcdir/../plugins partition=1 label=ROOT \
> +   linuxdisk $srcdir/../plugins/linuxdisk partition=1 label=ROOT \
> --run 'nbdcopy "$uri" linuxdisk-copy-out.img'
> 
>  # Check the disk content.
>  guestfish --ro -a linuxdisk-copy-out.img -m /dev/sda <  # Check some known files and directories exist.
>ll /
> -  ll /linuxdisk
> -  is-dir /linuxdisk
> -  is-file /linuxdisk/Makefile.am
> +  ll /subdir
> +  is-dir /subdir
> +  is-file /Makefile.am
> 
>  # This reads out all the directory entries and all file contents.
>tar-out / - | cat >/dev/null
> 
>  # Download some files and compare to local copies.
> -  download /linuxdisk/Makefile linuxdisk-copy-out.test1
> -  download /linuxdisk/Makefile.am linuxdisk-copy-out.test2
> -  download /linuxdisk/nbdkit-linuxdisk-plugin.pod linuxdisk-copy-out.test3
> -  download /linuxdisk/filesystem.c linuxdisk-copy-out.test4
> +  download /Makefile linuxdisk-copy-out.test1
> +  download /Makefile.am linuxdisk-copy-out.test2
> +  download /nbdkit-linuxdisk-plugin.pod linuxdisk-copy-out.test3
> +  download /filesystem.c linuxdisk-copy-out.test4
>  EOF
> 
>  # Compare downloaded files to local versions.
> -- 

Looks good, so:

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libguestfs PATCH] appliance, daemon: disable lvm2 devicesfile

[Richard W.M. Jones]

On Mon, May 30, 2022 at 04:10:27PM +0200, Laszlo Ersek wrote:
> In guestfs-tools commit 4fe8a03cd2d3 ('sysprep: remove lvm2's default
> "system.devices" file', 2022-04-11), we disabled the use of LVM2's new
> "devicesfile" feature, which could interfere with the cloning of virtual
> machines.
> 
> We suspected in
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c6
> 
> that the same lvm2 feature could affect the libguestfs appliance itself,
> but decided in
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c8
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c10
> 
> that this would not be the case, because "appliance/init" already
> constructed a pristine LVM_SYSTEM_DIR.
> 
> Unfortunately, that's not enough: due to the "use_devicesfile=1" default
> (on RHEL9 anyway), some "lvm" invocation, possibly inside the
> lvm-set-filter API, *creates* "$LVM_SYSTEM_DIR/devices/system.devices".
> And then we get (minimally) warnings such as
> 
> > Please remove the lvm.conf global_filter, it is ignored with the devices
> > file.
> > Please remove the lvm.conf filter, it is ignored with the devices file.
> 
> when using the lvm-set-filter API.
> 
> Explicitly disable the "devices file" in "appliance/init", and also
> whenever we rewrite "lvm.conf" -- that is, in set_filter()
> [daemon/lvm-filter.c]. In the former, check for the feature by locating
> the devicesfile-related utilities "lvmdevices" and "vgimportdevices". In
> the C code, invoke the utilities with the "--help" option instead. (In
> "appliance/init",  I thought it was best not to call any lvm2 utilities
> even with "--help", with our lvm2.conf still under construction there.) If
> either utility is available, set "use_devicesfile = 0".
> 
> Cc: David Teigland 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1965941
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> Tested:
> 
> - on top of "master" (7afbf5ee4415), using a Fedora 35 host, where the
>   lvm2 packages copied into the appliance do not have the "devicesfile"
>   feature (-> regression test);
> 
> - on top of "rhel-9.1" (ad24b9f4d695), in the form of a Brew scratch
>   build, using a nightly RHEL-9.1 install, where the lvm2 packages
>   copied into the appliance have the "devicesfile" feature (-> bugfix
>   test).
> 
>  daemon/lvm-filter.c | 19 +++
>  appliance/init  | 11 +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
> index c6dd35156d37..f75e2198831e 100644
> --- a/daemon/lvm-filter.c
> +++ b/daemon/lvm-filter.c
> @@ -68,6 +68,18 @@ free_lvm_system_dir (void)
>free (lvm_system_dir);
>  }
>  
> +static bool devicesfile_feature (void)
> +{
> +  static bool checked, available;
> +
> +  if (!checked) {
> +checked = true;
> +available = command (NULL, NULL, "lvmdevices", "--help", NULL) == 0 ||
> +command (NULL, NULL, "vgimportdevices", "--help", NULL) == 0;
> +  }
> +  return available;
> +}
> +
>  /* Rewrite the 'filter = [ ... ]' line in lvm.conf. */
>  static int
>  set_filter (char *const *filters)
> @@ -88,6 +100,13 @@ set_filter (char *const *filters)
>}
>  
>fprintf (fp, "devices {\n");
> +
> +  /* If lvm2 supports a "devices file", we need to disable its use
> +   * (RHBZ#1965941).
> +   */
> +  if (devicesfile_feature ())
> +fprintf (fp, "use_devicesfile = 0\n");
> +
>for (j = 0; filter_types[j] != NULL; ++j) {
>  fprintf (fp, "%s = [\n", filter_types[j]);
>  fprintf (fp, "");
> diff --git a/appliance/init b/appliance/init
> index 7076821d2250..19aa151b73aa 100755
> --- a/appliance/init
> +++ b/appliance/init
> @@ -134,6 +134,17 @@ mdadm -As --auto=yes --no-degraded
>  # Empty LVM configuration file means "all defaults".
>  mkdir -p /tmp/lvm
>  touch /tmp/lvm/lvm.conf
> +
> +# If lvm2 supports a "devices file", we need to disable its use
> +# (RHBZ#1965941).
> +if command -v lvmdevices || command -v vgimportdevices; then
> +  {
> +printf 'devices {\n'
> +printf '\tuse_devicesfile = 0\n'
> +printf '}\n'
> +  } >> /tmp/lvm/lvm.conf
> +fi
> +
>  LVM_SYSTEM_DIR=/tmp/lvm
>  export LVM_SYSTEM_DIR
>  lvmetad
> 
> base-commit: 7afbf5ee4415f6fa2553898d3af238e794062096
> -- 
> 2.19.1.3.g30247aa5d201

Looks reasonable to me, so:

Acked-by: Richard W.M. Jones 

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] libnbd: Failed to update to Fedora 36 & OpenSUSE Leap 15.3

[Richard W.M. Jones]

On Mon, May 30, 2022 at 09:12:48AM +0100, Richard W.M. Jones wrote:
> > > git push -o ci.variable="LIBVIRT_CI_CONTAINERS=1"
> 
> Let's see if this works:
> 
> https://gitlab.com/nbdkit/libnbd/-/pipelines/550978621

That did in fact work.

The OpenSUSE Leap test failed because nbdkit is segfaulting on that
platform.  Unclear why because the logs don't collect enough
information.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] libnbd: Failed to update to Fedora 36 & OpenSUSE Leap 15.3

[Richard W.M. Jones]

> > git push -o ci.variable="LIBVIRT_CI_CONTAINERS=1"

Let's see if this works:

https://gitlab.com/nbdkit/libnbd/-/pipelines/550978621

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] libnbd: Failed to update to Fedora 36 & OpenSUSE Leap 15.3

[Richard W.M. Jones]

On Mon, May 30, 2022 at 09:25:11AM +0200, Martin Kletzander wrote:
> On Sun, May 29, 2022 at 11:22:05AM +0100, Richard W.M. Jones wrote:
> >
> >I added this commit and regenerated the CI files:
> >
> >https://gitlab.com/nbdkit/libnbd/-/commit/b6a98aacbe22d599f000d4d1c84c27081ec06957
> >https://gitlab.com/nbdkit/libnbd/-/commit/2439fd5c7a07b314ce47728c6fbae16b9a26dcdb
> >
> >However apparently gitlab CI cannot create the Fedora 36 & OpenSUSE
> >Leap 15.3 containers:
> >
> >https://gitlab.com/nbdkit/libnbd/-/jobs/2519037050
> >https://gitlab.com/nbdkit/libnbd/-/jobs/2519037032
> >
> >with weird errors:
> >
> >ERROR: Job failed: failed to pull image 
> >"registry.gitlab.com/nbdkit/libnbd/ci-fedora-36:latest" with specified 
> >policies [always]: Error response from daemon: manifest for 
> >registry.gitlab.com/nbdkit/libnbd/ci-fedora-36:latest not found: manifest 
> >unknown: manifest unknown (manager.go:203:0s)
> >
> >I have no idea what this means.  Does something else need to be done
> >to create those containers?
> >
> 
> Looks like the containers were not built.  I suspect the following rule:
> 
> - if: '$CI_PROJECT_NAMESPACE == "nbdkit" && $CI_PIPELINE_SOURCE == "push" 
> && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'

Just to check, CI_PROJECT_NAMESPACE refers to the top-level
(https://gitlab.com/nbdkit) not the name of the project
(https://gitlab.com/nbdkit/libnbd)?

> unles CI_DEFAULT_BRANCH is already set.  I'm not sure who sets that
> (whether this is in the ci/cd settings or part of something else)
> because that was added not so long ago and I did not keep up with the
> changes.  If that is already set, then it might be that you need to push
> with an extra variable according to the comment in
> /ci/gitlab/container-templates.yml:
> 
> # For upstream
> #
> #   - Push to default branch:
> #   -> rebuild if dockerfile changed, no cache
> #   - Otherwise
> #   -> rebuild if LIBVIRT_CI_CONTAINERS=1, no cache,
> #  to pick up new published distro packages or
> #  recover from deleted tag
> #
> # For forks
> #   - Always rebuild, with cache
> #
> 
> so you could try pushing such commits with something like:
> 
> git push -o ci.variable="LIBVIRT_CI_CONTAINERS=1"

It was very non-obvious how to set those "environment variables", I
was wondering about it before too.  I think every time lcitool
comments or documentation mentions one it should actually use the full
command like you did above.  (I decided the same thing for nbdkit
documentation - every single time, show the complete runnable command,
not fragments.)

> but as I said I did not keep up with these changes and Dan will know for
> sure which one is needed.

OK, thanks.

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

[Libguestfs] libnbd: Failed to update to Fedora 36 & OpenSUSE Leap 15.3

[Richard W.M. Jones]

I added this commit and regenerated the CI files:

https://gitlab.com/nbdkit/libnbd/-/commit/b6a98aacbe22d599f000d4d1c84c27081ec06957
https://gitlab.com/nbdkit/libnbd/-/commit/2439fd5c7a07b314ce47728c6fbae16b9a26dcdb

However apparently gitlab CI cannot create the Fedora 36 & OpenSUSE
Leap 15.3 containers:

https://gitlab.com/nbdkit/libnbd/-/jobs/2519037050
https://gitlab.com/nbdkit/libnbd/-/jobs/2519037032

with weird errors:

ERROR: Job failed: failed to pull image 
"registry.gitlab.com/nbdkit/libnbd/ci-fedora-36:latest" with specified policies 
[always]: Error response from daemon: manifest for 
registry.gitlab.com/nbdkit/libnbd/ci-fedora-36:latest not found: manifest 
unknown: manifest unknown (manager.go:203:0s)

I have no idea what this means.  Does something else need to be done
to create those containers?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] libnbd | Failed pipeline for master | 2439fd5c

[Richard W.M. Jones]

On Sun, May 29, 2022 at 09:48:22AM +, GitLab wrote:
> GitLab
>✖ Pipeline #550544496 has failed!

https://gitlab.com/nbdkit/libnbd/-/pipelines/550544496

  "Found errors in your .gitlab-ci.yml:
   Local file `/ci/gitlab/sanity-checks.yml` does not exist!"

>  
> Project   nbdkit / libnbd
> Branch● master
> Commit● 2439fd5c
>   ci: Update generated files With the latest lib...
> Commit Author ● Richard W.M. Jones
>  
>     Pipeline #550544496 triggered by ●   Richard W.M. Jones
>had 0 failed jobs.
>   Failed jobs
> GitLab
> You're receiving this email because of your account on gitlab.com. Manage all
> notifications · Help

This seems to be a bug in libvirt-ci:

https://gitlab.com/libvirt/libvirt-ci/-/issues/14

I have temporarily worked around it by removing the offending line
from the generated code.

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] [libnbd PATCH] api: Speed up nbd_pread_structured when reading holes

[Richard W.M. Jones]

On Fri, May 27, 2022 at 05:25:43PM -0500, Eric Blake wrote:
> Commit c7fbc71d missed an optimization: when we guarantee that a pread
> buffer is pre-zeroed, we don't need to waste time memset()ing it when
> the server sends a hole.  But the next commit e0953cb7 lets the user
> skip pre-zeroing, at which point the memset is required to avoid data
> corruption.  Thankfully, we are not leaking bogus bytes when a server
> sends a hole, but we CAN speed up the case where we have pre-zeroed
> the buffer to avoid a second memset().  Because the user can change
> the state of pread_initialize on the fly (including while a pread is
> still in flight), we must track the state per-command as it was before
> we send the request to the server.
> ---
>  lib/internal.h  |  1 +
>  generator/states-reply-structured.c |  5 +++--
>  lib/rw.c| 18 +++---
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/internal.h b/lib/internal.h
> index 6aaced3..885cee1 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -353,6 +353,7 @@ struct command {
>struct command_cb cb;
>enum state state; /* State to resume with on next POLLIN */
>bool data_seen; /* For read, true if at least one data chunk seen */
> +  bool initialized; /* For read, true if getting a hole may skip memset */
>uint32_t error; /* Local errno value */
>  };
> 
> diff --git a/generator/states-reply-structured.c 
> b/generator/states-reply-structured.c
> index 7001047..12c24f5 100644
> --- a/generator/states-reply-structured.c
> +++ b/generator/states-reply-structured.c
> @@ -1,5 +1,5 @@
>  /* nbd client library in userspace: state machine
> - * Copyright (C) 2013-2019 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -436,7 +436,8 @@ STATE_MACHINE {
>   * 0-length replies are broken. Still, it's easy enough to support
>   * them as an extension, and this works even when length == 0.
>   */
> -memset (cmd->data + offset, 0, length);
> +if (!cmd->initialized)
> +  memset (cmd->data + offset, 0, length);
>  if (CALLBACK_IS_NOT_NULL (cmd->cb.fn.chunk)) {
>int error = cmd->error;
> 
> diff --git a/lib/rw.c b/lib/rw.c
> index c5d041d..f32481a 100644
> --- a/lib/rw.c
> +++ b/lib/rw.c
> @@ -244,14 +244,18 @@ nbd_internal_command_common (struct nbd_handle *h,
>if (cb)
>  cmd->cb = *cb;
> 
> -  /* For NBD_CMD_READ, cmd->data was pre-zeroed in the prologue
> -   * created by the generator.  Thus, if a (non-compliant) server with
> -   * structured replies fails to send back sufficient data to cover
> -   * the whole buffer, we still behave as if it had sent zeroes for
> -   * those portions, rather than leaking any uninitialized data, and
> -   * without having to complicate our state machine to track which
> -   * portions of the read buffer were actually populated.
> +  /* For NBD_CMD_READ, cmd->data defaults to being pre-zeroed in the
> +   * prologue created by the generator.  Thus, if a (non-compliant)
> +   * server with structured replies fails to send back sufficient data
> +   * to cover the whole buffer, we still behave as if it had sent
> +   * zeroes for those portions, rather than leaking any uninitialized
> +   * data, and without having to complicate our state machine to track
> +   * which portions of the read buffer were actually populated.  But
> +   * if the user opts in to disabling set_pread_initialize, then we
> +   * need to memset zeroes as they are read (and the user gets their
> +   * own garbage back in the case of a non-compliant server).
> */
> +  cmd->initialized = h->pread_initialize;
> 
>/* Add the command to the end of the queue. Kick the state machine
> * if there is no other command being processed, otherwise, it will

Reviewed-by: Richard W.M. Jones 

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] [libnbd PATCH] python: Speed up pread

[Richard W.M. Jones]

On Fri, May 27, 2022 at 04:44:16PM -0500, Eric Blake wrote:
> Instead of malloc'ing a C buffer, nbd_pread()ing into it, then copying
> it into an immutable Python bytes object, we can instead pre-create a
> correctly-sized Python bytearray object, then nbd_pread() directly
> into that object's underlying bytes.
> 
> While the data copying might not be the bottleneck compared to the
> networking costs, it does have noticeable results; on my machine:
> 
> $ export script='
> m=1024*1024
> size=h.get_size()
> for i in range(size // m):
>   buf = h.pread(m, m*i)
> '
> $ time ./run nbdkit -U - pattern 10G --run 'nbdsh -u $uri -c "$script"'
> 
> sped up from about 7.8s pre-patch to about 7.1s post-patch,
> approximately a 10% speedup.
> 
> Note that this slightly changes the python API: h.pread[_structured]
> now returns a mutable 'bytearray' object, rather than an immutable
> 'bytes' object.  This makes it possible to modify the just-read string
> in place, rather than having to create yet another memory buffer for
> any modifications, which offers even more speedups when writing a
> read-modify-write paradigm in python.  But the change is
> backwards-compatible - python already states that a read-write buffer
> may be returned instead of readonly for any client that already
> operated only on a buffer in a readonly manner.

Although this is not an API change, in general we have no obligation
to maintain backwards compat for the Python API.  (The C API is quite
a different matter of course.)  Nevertheless, it's nice not to break
things.

> Note that h.pread is more like Python read() semantics in creating a
> buffer, while h.aio_pread is more like Python readinto() semantics in
> modifying a passed-in buffer.  But now that both code paths have a
> python object prior to calling into the C API, my next task is to
> improve the h.*pread_structured callback function to pass its buffer
> as a slice of the Python input buffer, rather than doing yet another
> round of pointless memcpy from C into python objects.

You might want to have a look at the nbdkit Python bindings for
inspiration because Nir made those either zero- or low-copy (I forget
exactly which).

>  generator/Python.ml | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 4ab18f6..1c4446e 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -1,6 +1,6 @@
>  (* hey emacs, this is OCaml code: -*- tuareg -*- *)
>  (* nbd client library in userspace: Python bindings
> - * Copyright (C) 2013-2021 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -293,7 +293,7 @@ let
>  | BytesIn (n, _) ->
> pr "  Py_buffer %s = { .obj = NULL };\n" n
>  | BytesOut (n, count) ->
> -   pr "  char *%s = NULL;\n" n;
> +   pr "  PyObject *%s = NULL;\n" n;
> pr "  Py_ssize_t %s;\n" count
>  | BytesPersistIn (n, _)
>  | BytesPersistOut (n, _) ->
> @@ -432,8 +432,8 @@ let
>  | Bool _ -> ()
>  | BytesIn _ -> ()
>  | BytesOut (n, count) ->
> -   pr "  %s = malloc (%s);\n" n count;
> -   pr "  if (%s == NULL) { PyErr_NoMemory (); goto out; }\n" n
> +   pr "  %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n count;
> +   pr "  if (%s == NULL) goto out;\n" n
>  | BytesPersistIn (n, _) | BytesPersistOut (n, _) ->
> pr "  %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n;
> pr "  if (!%s_buf) goto out;\n" n;
> @@ -479,7 +479,7 @@ let
>  function
>  | Bool n -> pr ", %s" n
>  | BytesIn (n, _) -> pr ", %s.buf, %s.len" n n
> -| BytesOut (n, count) -> pr ", %s, %s" n count
> +| BytesOut (n, count) -> pr ", PyByteArray_AS_STRING (%s), %s" n count
>  | BytesPersistIn (n, _)
>  | BytesPersistOut (n, _) -> pr ", %s_buf->data, %s_buf->len" n n
>  | Closure { cbname } -> pr ", %s" cbname
> @@ -524,8 +524,9 @@ let
>let use_ret = ref true in
>List.iter (
>  function
> -| BytesOut (n, count) ->
> -   pr "  py_ret = PyBytes_FromStringAndSize (%s, %s);\n" n count;
> +| BytesOut (n, _) ->
> +   pr "  py_ret = %s;\n" n;
> +       pr "  %s = NULL;\n" n;
> use_ret := false
>  | Bool _
>  | BytesIn _
> @@ -572,7 +573,7 @@ let
>  | BytesIn (n, _) ->
> pr 

Re: [Libguestfs] [nbdkit PATCH v2] blocksize: Avoid losing aligned writes to RMW race

[Richard W.M. Jones]

+os.unlink(witness)
> +
> +out = h.pread(16,0)
> +print(out)
> +t = end_t - start_t
> +print(t)
> +assert out in [b"4"*4 + b"3"*12, b"4"*16]
> +assert t >= 8
> +
> +# Next pass: try to kick off aligned first
> +print("aligned first")
> +ba5 = bytearray(b"5"*16)
> +ba6 = bytearray(b"6"*12)
> +buf5 = nbd.Buffer.from_bytearray(ba5)
> +buf6 = nbd.Buffer.from_bytearray(ba6)
> +h.zero(16, 0)
> +touch(witness)
> +start_t = time.time()
> +h.aio_pwrite(buf5, 0)
> +h.aio_pwrite(buf6, 4)
> +
> +while h.aio_in_flight() > 0:
> +h.poll(-1)
> +end_t = time.time()
> +os.unlink(witness)
> +
> +out = h.pread(16,0)
> +print(out)
> +t = end_t - start_t
> +print(t)
> +assert out in [b"5"*4 + b"6"*12, b"5"*16]
> +assert t >= 8
> +'
> +
> +# Now run everything
> +truncate --size=16 blocksize-sharding.img
> +export witness="$PWD/blocksize-sharding.tmp"
> +nbdkit -U - --filter=blocksize --filter=delay eval delay-write=2 \
> +config='ln -sf "$(realpath "$3")" $tmpdir/$2' \
> +img="$PWD/blocksize-sharding.img" tmp="$PWD/blocksize-sharding.tmp" \
> +get_size='echo 16' block_size='echo 16 64K 1M' \
> +thread_model='echo parallel' \
> +zero='dd if=/dev/zero of=$tmpdir/img skip=$4 count=$3 \
> +  iflag=count_bytes,skip_bytes' \
> +pread='
> +  dd if=$tmpdir/img skip=$4 count=$3 iflag=count_bytes,skip_bytes
> +  if [ -f $tmpdir/tmp ]; then sleep 4; fi ' \
> +pwrite='dd of=$tmpdir/img seek=$4 conv=notrunc oflag=seek_bytes' \
> +--run 'nbdsh -u "$uri" -c "$script"'

Reviewed-by: Richard W.M. Jones 

I have no preference on whether or not to put the test into a
separate commit.

I'm not sure I understand where the potential CVE is.  In what
scenario could the old filter be exploited?

Rich.


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v] convert: If listing RPM applications fails, rebuild DB and retry

[Richard W.M. Jones]

On Thu, May 26, 2022 at 10:53:59AM +0200, Laszlo Ersek wrote:
> On 05/25/22 18:02, Richard W.M. Jones wrote:
> > In libguestfs we didn't bother to check the return values from any
> > librpm calls.  In some cases where possibly the RPM database is
> > faulty, this caused us to return a zero-length list of installed
> > applications (but no error indication).  Libguestfs has subsequently
> > been fixed so now it returns an error if the RPM database is corrupt.
> > 
> > This commit changes virt-v2v behaviour so that if either
> > guestfs_inspect_list_applications2 returns a zero-length list (ie. old
> > libguestfs) or it throws an error (new libguestfs) then we attempt to
> > rebuild the RPM database and retry the operation.  Rebuilding the
> > database can recover from some but not all RPM DB corruption.
> > 
> > See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12
> 
> What an absolutely horrific error mode. Great job debugging it!
> 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623
> > Reported-by: Xiaodai Wang
> > Reported-by: Ming Xie
> > ---
> >  convert/inspect_source.ml | 26 --
> >  1 file changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/convert/inspect_source.ml b/convert/inspect_source.ml
> > index 1736009629..16058de644 100644
> > --- a/convert/inspect_source.ml
> > +++ b/convert/inspect_source.ml
> > @@ -34,6 +34,7 @@ let rec inspect_source root_choice g =
> >reject_if_not_installed_image g root;
> >  
> >let typ = g#inspect_get_type root in
> > +  let package_format = g#inspect_get_package_format root in
> >  
> >(* Mount up the filesystems. *)
> >let mps = g#inspect_get_mountpoints root in
> > @@ -71,7 +72,7 @@ let rec inspect_source root_choice g =
> >) mps;
> >  
> >(* Get list of applications/packages installed. *)
> > -  let apps = g#inspect_list_applications2 root in
> > +  let apps = list_applications g root package_format in
> >let apps = Array.to_list apps in
> >  
> >(* A map of app2_name -> application2, for easier lookups.  Note
> > @@ -106,7 +107,7 @@ let rec inspect_source root_choice g =
> >  i_arch = g#inspect_get_arch root;
> >  i_major_version = g#inspect_get_major_version root;
> >  i_minor_version = g#inspect_get_minor_version root;
> > -i_package_format = g#inspect_get_package_format root;
> > +i_package_format = package_format;
> >  i_package_management = g#inspect_get_package_management root;
> >  i_product_name = g#inspect_get_product_name root;
> >  i_product_variant = g#inspect_get_product_variant root;
> > @@ -186,6 +187,27 @@ and reject_if_not_installed_image g root =
> >if fmt <> "installed" then
> >  error (f_"libguestfs thinks this is not an installed operating system 
> > (it might be, for example, an installer disk or live CD).  If this is 
> > wrong, it is probably a bug in libguestfs.  root=%s fmt=%s") root fmt
> >  
> > +(* Wrapper around g#inspect_list_applications2 which, for RPM
> > + * guests, on failure tries to rebuild the RPM database before
> > + * repeating the operation.
> > + *)
> > +and list_applications g root = function
> > +  | "rpm" ->
> > + (* RPM guest. *)
> > + (try
> 
> [*]
> 
> > +let apps = g#inspect_list_applications2 root in
> > +if apps = [||] then raise (G.Error "no applications returned");
> > +apps
> > +  with G.Error msg ->
> > +debug "%s" msg;
> > +debug "rebuilding RPM database and retrying ...";
> > +ignore (g#sh "rpmdb --rebuilddb");
> > +g#inspect_list_applications2 root
> > + )
> > +  | _ ->
> > + (* Non-RPM guest, just do it. *)
> > + g#inspect_list_applications2 root
> > +
> >  (* See if this guest could use UEFI to boot.  It should use GPT and
> >   * it should have an EFI System Partition (ESP).
> >   *
> > 
> 
> The commit message explains well why the "g#inspect_list_applications2"
> method call that is at the top of each "match" pattern cannot be
> factored out (to a common call just before the "match"). However,
> looking at the code, it's not easy to understand.
> 
> Can you please:
> 
> (1) Commit the libguestfs patch,
> 
> (2) insert a comment at [*], saying that either of the two lines just
> below may raise an e

Re: [Libguestfs] [PATCH] always 'max' for the appliance CPU model on all targes except ppc

[Richard W.M. Jones]

We always wanted "-cpu best", which means basically the CPU which
always works best (NB: I used the both words "works" and "best").

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] RFC: blocksize: Add test for sharding behavior

[Richard W.M. Jones]

Is there any way to do this without the literal sleeps?  Gitlab CI in
particular appears to be very contended (I guess it runs in parallel
on huge systems with vast numbers of unrelated containers).  I've seen
threads being created that are so starved they never run at all even
in tests running for many tens of seconds.

I'm thinking some kind of custom plugin which orders operations using
its own clock that it is incremented as certain requests arrive, but I
don't have a fully-formed idea of how it would work.

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] [libnbd PATCH] python: Accept buffers in nbd.Buffer.from_bytearray()

[Richard W.M. Jones]

On Wed, May 25, 2022 at 08:27:15PM -0500, Eric Blake wrote:
> Prior to this patch, the following fails, but at least seems to give a
> sensible error:
> 
> $ nbdsh -c 'nbd.Buffer.from_bytearray(b"1"*8)'
> Traceback (most recent call last):
>   File "/usr/lib64/python3.10/runpy.py", line 196, in _run_module_as_main
> return _run_code(code, main_globals, None,
>   File "/usr/lib64/python3.10/runpy.py", line 86, in _run_code
> exec(code, run_globals)
>   File "/usr/lib64/python3.10/site-packages/nbd.py", line 2726, in 
> nbdsh.shell()
>   File "/usr/lib64/python3.10/site-packages/nbdsh.py", line 139, in shell
> exec(c, d, d)
>   File "", line 1, in 
>   File "/usr/lib64/python3.10/site-packages/nbd.py", line 132, in 
> from_bytearray
> o = libnbdmod.aio_buffer_from_bytearray(ba)
> RuntimeError: parameter is not a bytearray
> 
> while this version 1 byte longer flat out segfaults:
> 
> $ nbdsh -c 'nbd.Buffer.from_bytearray(b"1"*9)'
> 
> and this one is subtly different:
> 
> $ nbdsh -c 'nbd.Buffer.from_bytearray(h)'
> Traceback (most recent call last):
> ...
>   File "/usr/lib64/python3.10/site-packages/nbd.py", line 132, in 
> from_bytearray
> o = libnbdmod.aio_buffer_from_bytearray(ba)
> MemoryError
> 
> That's because PyByteArray_AsString() blindly assumes that its
> argument is a PyByteArray, and goes haywire when it is not, unless we
> got lucky that the incorrectly-typed object behaves similarly enough
> (which, for byte literals, is size-dependent).
> 
> But Python already has a handy way to convert any object that supports
> the buffer interface into a bytearray.  Using it, we can now support
> many more parameters; passing in b"1"*9 now correctly creates a 9-byte
> buffer rather than failing.  And the error message for a non-buffer
> also improves:
> 
> $ ./run nbdsh -c 'nbd.Buffer.from_bytearray(h)'
> Traceback (most recent call last):
> ...
>   File "/home/eblake/libnbd/python/nbd.py", line 132, in from_bytearray
> o = libnbdmod.aio_buffer_from_bytearray(ba)
> TypeError: cannot convert 'NBD' object to bytearray
> 
> (A reliable TypeError is always better than an unexpected MemoryError
> or segfault).
> ---
>  python/handle.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/python/handle.c b/python/handle.c
> index 9c08dc1..9fe3f8e 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -1,5 +1,5 @@
>  /* NBD client library in userspace
> - * Copyright (C) 2013-2020 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -159,6 +159,7 @@ PyObject *
>  nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args)
>  {
>PyObject *obj;
> +  PyObject *arr = NULL;
>Py_ssize_t len;
>void *data;
>struct py_aio_buffer *buf;
> @@ -169,9 +170,17 @@ nbd_internal_py_aio_buffer_from_bytearray (PyObject 
> *self, PyObject *args)
>   ))
>  return NULL;
> 
> +  if (! PyByteArray_Check (obj)) {
> +arr = PyByteArray_FromObject (obj);
> +if (arr == NULL)
> +  return NULL;
> +obj = arr;
> +  }
>data = PyByteArray_AsString (obj);
>if (!data) {
> -PyErr_SetString (PyExc_RuntimeError, "parameter is not a bytearray");
> +PyErr_SetString (PyExc_RuntimeError,
> + "parameter is not a bytearray or buffer");
> +Py_XDECREF (arr);
>  return NULL;
>}
>len = PyByteArray_Size (obj);
> @@ -179,6 +188,7 @@ nbd_internal_py_aio_buffer_from_bytearray (PyObject 
> *self, PyObject *args)
>buf = malloc (sizeof *buf);
>if (buf == NULL) {
>  PyErr_NoMemory ();
> +Py_XDECREF (arr);
>  return NULL;
>}
> 
> @@ -187,9 +197,11 @@ nbd_internal_py_aio_buffer_from_bytearray (PyObject 
> *self, PyObject *args)
>if (buf->data == NULL) {
>  PyErr_NoMemory ();
>  free (buf);
> +Py_XDECREF (arr);
>  return NULL;
>}
>memcpy (buf->data, data, len);
> +  Py_XDECREF (arr);
> 
>ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
>if (ret == NULL) {

Reviewed-by: Richard W.M. Jones 

Thanks!

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

Re: [Libguestfs] [PATCH] always 'max' for the appliance CPU model on all targes except ppc

[Richard W.M. Jones]

On Wed, May 25, 2022 at 05:13:53PM +0100, Peter Maydell wrote:
> On Wed, 25 May 2022 at 16:07, Laszlo Ersek  wrote:
> >
> > + Drew & Peter
> >
> > On 05/25/22 15:30, Daniel P. Berrangé wrote:
> > - The patch seems to do what it says in the commit message.
> >
> > - QEMU commit bab52d4bba3f ("target/arm: Add "-cpu max" support",
> >   2018-03-09) confirms what the commit message says, about both TCG and
> >   KVM.
> >
> > - To smoke-test the TCG-related change, I've edited a long-term TCG
> >   aarch64 libvirt domain of mine, replacing "cortex-a57" with "max".
> >   Both edk2 and the Linux guest continued working. So I guess the TCG
> >   change is OK.
> 
> One thing to note here is that if you are using:
>  * TCG -cpu max
>  * 'virt' with no named version or with 'virt-7.0' or later
>  * a Linux kernel version prior to v5.12
> then a bug in Linux means it won't boot. (This is because of
> the LPA2 CPU feature which TCG -cpu max now emulates; older
> kernels were buggy and won't boot on an LPA2 CPU, including
> a real hardware one.)

Is this related at all to the 5-level page tables (la57) failure with
TCG and -cpu max?

https://gitlab.com/qemu-project/qemu/-/issues/1023

Rich.

> You might or might not feel this is something worth noting in
> release notes or equivalent.
> 
> 
> > - In additional support of the above, QEMU commit ddaebdda53fc
> >   ("target/arm: Unindent unnecessary else-clause", 2022-02-21) added a
> >   comment saying
> >
> >   /* '-cpu max' for TCG: we currently do this as "A57 with extra things" */
> >
> > - Although I was more surprised by the TCG-related statement initially
> >   (i.e. that "max" was a superset of "cortex-a57" when using TCG), now
> >   I'm actually more concerned about the KVM case.
> 
> The TCG part is an implementation convenience (mostly it just
> means that 'max' has the A57's IMPDEF registers).
> 
> >   Specifically QEMU commit 0baa21be497d ("target/arm: Make KVM -cpu max
> >   exactly like -cpu host", 2022-02-21) eliminated a difference where
> >   "-cpu max" had been a superset of "-cpu host", featuring the
> >   "sve-max-vq" extra property.
> >
> >   The fix is part of release v7.0.0.
> >
> >   The difference was introduced in commits
> >
> >   [1] 6fa8a37949d9 ("target/arm/cpu64: max cpu: Support sve properties
> >   with KVM", 2019-11-01)
> >
> >   [2] 87014c6b3660 ("target/arm/kvm: host cpu: Add support for sve
> >   properties", 2019-11-01)
> >
> >   and apparently *deliberately*.
> 
> No, this was unintentional. See the discussion in this thread:
> https://lore.kernel.org/qemu-devel/20220203173640.shxkmatdcsfzzvtj@gator/
> 
> In particular, although KVM '-cpu max' had the sve-max-vq
> property, Drew notes "sve-max-vq won't work for any of the machines that
> support SVE that I know of". Which is why we removed it, rather
> than adding it to '-cpu host'.
> 
> >   Therefore it seems that starting with qemu-4.2, but strictly preceding
> >   qemu-7.0, "-cpu max" and "-cpu host" are not "identical" when KVM is
> >   enabled; "-cpu max" has more features. Because of that, I think there
> >   are two options:
> >
> >   (a) This extra feature is actually harmless, so we should only update
> >   the commit message (i.e., generally speaking, "-cpu max" has been
> >   a superset of "-cpu host" on KVM and of "-cpu cortex-a57" on TCG).
> >
> >   (b) The feature actually presents a problem, and qemu in [v4.2.0,
> >   v7.0.0) will not start when KVM accel and "-cpu max" are requested
> >   simultaneously. In this case, I think the appliance needs to stick
> >   with "-cpu host" on KVM.
> 
> I don't understand why you think these are the only two options. The
> actual situation is:
> 
>  (c) -cpu max and -cpu host have always been identical on KVM,
>  and this commit does not change that.
>  There happens to have been a QOM property 'sve-max-vq' on 'max'
>  that should not have existed there and that nobody can actually have
>  been usefully setting, but now there isn't.
> 
> thanks
> -- PMM
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs

-- 
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH libguestfs] daemon: rpm: Check return values from librpm calls

[Richard W.M. Jones]

We previously didn't bother to check the return values from any librpm
calls.  In some cases where possibly the RPM database is faulty, this
caused us to return a zero-length list of installed applications (but
no error indication).

One way to reproduce this is given below.  Note this reproducer will
only work when run on a RHEL 8 host (or more specifically, with
rpm <= 4.16):

$ virt-builder fedora-28
$ guestfish -a fedora-28.img -i rm /var/lib/rpm/Packages
$ guestfish --ro -a fedora-28.img -i inspect-list-applications /dev/sda4 -vx
...
chroot: /sysroot: running 'librpm'
error: cannot open Packages index using db5 - Read-only file system (30)
error: cannot open Packages database in
error: cannot open Packages index using db5 - Read-only file system (30)
error: cannot open Packages database in
librpm returned 0 installed packages
...

With this commit we get an error instead:

...
chroot: /sysroot: running 'librpm'
error: cannot open Packages index using db5 - Read-only file system (30)
error: cannot open Packages database in
ocaml_exn: 'internal_list_rpm_applications' raised 'Failure' exception
guestfsd: error: rpmtsInitIterator
guestfsd: => internal_list_rpm_applications (0x1fe) took 0.01 secs
libguestfs: trace: internal_list_rpm_applications = NULL (error)
libguestfs: error: internal_list_rpm_applications: rpmtsInitIterator
libguestfs: trace: inspect_list_applications2 = NULL (error)
libguestfs: trace: inspect_list_applications = NULL (error)
...

Not in this case, but in some cases of corrupt RPM databases it is
possible to recover them by running "rpmdb --rebuilddb" as a guest
command (ie. with guestfs_sh).

See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623
Fixes: commit c9ee831affed55abe0f928134cbbd2ed83b2f510
Reported-by: Xiaodai Wang
Reported-by: Ming Xie
---
 daemon/rpm-c.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/daemon/rpm-c.c b/daemon/rpm-c.c
index 74d325a17b..2cb50a62d2 100644
--- a/daemon/rpm-c.c
+++ b/daemon/rpm-c.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -79,7 +80,14 @@ value
 guestfs_int_daemon_rpm_init (value unitv)
 {
   CAMLparam1 (unitv);
-  rpmReadConfigFiles (NULL, NULL);
+
+  /* Nothing in actual RPM C code bothers to check if this call
+   * succeeds, so using that as an example, just print a debug message
+   * if it failed, but continue.  (The librpm Python bindings do check)
+   */
+  if (rpmReadConfigFiles (NULL, NULL) == -1)
+fprintf (stderr, "rpmReadConfigFiles: failed, errno=%d\n", errno);
+
   CAMLreturn (Val_unit);
 }
 
@@ -92,6 +100,8 @@ guestfs_int_daemon_rpm_start_iterator (value unitv)
   CAMLparam1 (unitv);
 
   ts = rpmtsCreate ();
+  if (ts == NULL)
+caml_failwith ("rpmtsCreate");
 
 #ifdef RPMVSF_MASK_NOSIGNATURES
   /* Disable signature checking (RHBZ#2064182). */
@@ -99,6 +109,14 @@ guestfs_int_daemon_rpm_start_iterator (value unitv)
 #endif
 
   iter = rpmtsInitIterator (ts, RPMDBI_PACKAGES, NULL, 0);
+  /* This could return NULL in theory if there are no packages, but
+   * that could not happen in a real guest.  However it also returns
+   * NULL when unable to open the database (RHBZ#2089623) which is
+   * something we do need to detect.
+   */
+  if (iter == NULL)
+caml_failwith ("rpmtsInitIterator");
+
   CAMLreturn (Val_unit);
 }
 
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH virt-v2v] convert: If listing RPM applications fails, rebuild DB and retry

[Richard W.M. Jones]

In libguestfs we didn't bother to check the return values from any
librpm calls.  In some cases where possibly the RPM database is
faulty, this caused us to return a zero-length list of installed
applications (but no error indication).  Libguestfs has subsequently
been fixed so now it returns an error if the RPM database is corrupt.

This commit changes virt-v2v behaviour so that if either
guestfs_inspect_list_applications2 returns a zero-length list (ie. old
libguestfs) or it throws an error (new libguestfs) then we attempt to
rebuild the RPM database and retry the operation.  Rebuilding the
database can recover from some but not all RPM DB corruption.

See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623
Reported-by: Xiaodai Wang
Reported-by: Ming Xie
---
 convert/inspect_source.ml | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/convert/inspect_source.ml b/convert/inspect_source.ml
index 1736009629..16058de644 100644
--- a/convert/inspect_source.ml
+++ b/convert/inspect_source.ml
@@ -34,6 +34,7 @@ let rec inspect_source root_choice g =
   reject_if_not_installed_image g root;
 
   let typ = g#inspect_get_type root in
+  let package_format = g#inspect_get_package_format root in
 
   (* Mount up the filesystems. *)
   let mps = g#inspect_get_mountpoints root in
@@ -71,7 +72,7 @@ let rec inspect_source root_choice g =
   ) mps;
 
   (* Get list of applications/packages installed. *)
-  let apps = g#inspect_list_applications2 root in
+  let apps = list_applications g root package_format in
   let apps = Array.to_list apps in
 
   (* A map of app2_name -> application2, for easier lookups.  Note
@@ -106,7 +107,7 @@ let rec inspect_source root_choice g =
 i_arch = g#inspect_get_arch root;
 i_major_version = g#inspect_get_major_version root;
 i_minor_version = g#inspect_get_minor_version root;
-i_package_format = g#inspect_get_package_format root;
+i_package_format = package_format;
 i_package_management = g#inspect_get_package_management root;
 i_product_name = g#inspect_get_product_name root;
 i_product_variant = g#inspect_get_product_variant root;
@@ -186,6 +187,27 @@ and reject_if_not_installed_image g root =
   if fmt <> "installed" then
 error (f_"libguestfs thinks this is not an installed operating system (it 
might be, for example, an installer disk or live CD).  If this is wrong, it is 
probably a bug in libguestfs.  root=%s fmt=%s") root fmt
 
+(* Wrapper around g#inspect_list_applications2 which, for RPM
+ * guests, on failure tries to rebuild the RPM database before
+ * repeating the operation.
+ *)
+and list_applications g root = function
+  | "rpm" ->
+ (* RPM guest. *)
+ (try
+let apps = g#inspect_list_applications2 root in
+if apps = [||] then raise (G.Error "no applications returned");
+apps
+  with G.Error msg ->
+debug "%s" msg;
+debug "rebuilding RPM database and retrying ...";
+ignore (g#sh "rpmdb --rebuilddb");
+g#inspect_list_applications2 root
+ )
+  | _ ->
+ (* Non-RPM guest, just do it. *)
+ g#inspect_list_applications2 root
+
 (* See if this guest could use UEFI to boot.  It should use GPT and
  * it should have an EFI System Partition (ESP).
  *
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit v2 4/5] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

I've updated the HTML man pages on the website:

https://libguestfs.org/nbdkit.1.html

Note the synopsis section of that first page is wrong, but I kind of
knew that already.  That particular section is generated by the
podwrapper --verbatim option, unlike all the others.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libguestfs-common PATCH] mlcustomize: refresh generated files

[Richard W.M. Jones]

I have tested both patches this time with the three affected tools
(virt-builder, virt-customize & virt-sysprep) and the --selinux-relabel
option is now present and ignored.

For the series:
Reviewed-by: Richard W.M. Jones 

Thanks for fixing this, I'll get it into C9S soon.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit v2 4/5] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

Thanks - the series was pushed in:
   b2612d2c..43b341b0  master -> master

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] nbdkit blocksize filter, read-modify-write, and concurrency

[Richard W.M. Jones]

On Tue, May 24, 2022 at 09:01:28AM -0500, Eric Blake wrote:
> On Sat, May 21, 2022 at 01:21:11PM +0100, Nikolaus Rath wrote:
> > Hi,
> > 
> > How does the blocksize filter take into account writes that end-up 
> > overlapping due to read-modify-write cycles?
> > 
> > Specifically, suppose there are two non-overlapping writes handled by two 
> > different threads, that, due to blocksize requirements, overlap when 
> > expanded. I think there is a risk that one thread may partially undo the 
> > work of the other here.
> > 
> > Looking at the code, it seems that writes of unaligned heads and tails are 
> > protected with a global lock., 
> > but writes of aligned data can occur concurrently.
> > 
> > However, does this not miss the case where there is one unaligned write 
> > that overlaps with an aligned one? 
> > 
> > For example, with blocksize 10, we could have:
> 
> The blocksize filter requires blocks to be sized as a power of 2,
> which 10 is not.  I will try to restate your question using hex
> boundaries; please correct me if I'm mis-stating things.
> 
> > 
> > Thread 1: receives write request for offset=0, size=10
> > Thread 2: receives write request for offset=4, size=16
> 
> minblock = 0x10
> Thread 1: receives write request for offset 0x00, size 0x10 (aligned request)
> Thread 2: receives write request for offset 0x04, size 0x16 (unaligned 
> offset, unaligned size)
> 
> Graphically, we are wanting to write the following, given initial disk
> contents of I:
> 
>0   0   0   0   1   1   1   1
>0...4...8...a...0...4...8...a...
> start  
> T1:
> T2:BB
> 
> Because both writes are issued simultaneously, we do not know whether
> bytes 0x04 through 0x0f will be written as A or B.  But our assumption
> is that because blocks are written atomically, we hope to get exactly
> one of the two following results, where either T1 beat T2:

The NBD protocol doesn't mention "atomic" anywhere.  Are writes
actually guaranteed to be atomic like this?  I'd be a bit more
convinced if this could be shown to happen in a non-overlapping case.

(It may be that we still want to fix the blocksize filter as you've
described -- using a rwlock seems like a reasonable and non-invasive
fix -- but I'd like to understand what is guaranteed and what is a
client's problem.)

>0   0   0   0   1   1   1   1
>0...4...8...a...0...4...8...a...
> end1:  BBII
> 
> or where T2 beat T1:
> 
>0   0   0   0   1   1   1   1
>0...4...8...a...0...4...8...a...
> end2:  BBII
> 
> However, you are worried that a third possibility occurs:
> 
> > Thread 1: acquires lock, reads bytes 0-4
> > Thread 2: does aligned write (no locking needed), writes bytes 0-10
> > Thread 1: writes bytes 0-10, overwriting data from Thread 2
> 
> These do not match your initial conditions above (why is thread 1
> reading; why is thread 2 doing aligned action).  Again, I'm rewriting
> this into something that I think matches what you intended to ask, but
> I welcome corrections:
> 
> T2 sees that it needs to do RMW, grabs the lock, and reads 0x00-0x0f
> for the unaligned head (it only needs 0x00-0x03, but we have to read a
> block at a time), to populate its buffer with .
> 
> T1 now writes 0x00-0x0f with , without any lock
> blocking it.
> 
> T2 now writes 0x00-0x0f using the contents of its buffer, resulting in:
> 
>0   0   0   0   1   1   1   1
>0...4...8...a...0...4...8...a...
> end3:  BBII
> 
> which does NOT reflect either of the possibilities where T1 and T2
> write atomically.  Basically, we have become the victim of sharding.
> 
> You are correct that it is annoying that this third possibility (where
> T1 appears to have never run) is possible with the blocksize filter.
> And we should probably consider it as a data-corruption bug.  Your
> blocksize example of 10 (or 0x10) bytes is unlikely, but we are more
> likely to hit scenarios where an older guest assumes it is writing to
> 512-byte aligned hardware, while using the blocksize filter to try and
> guarantee RMW atomic access to 4k modern hardware.  The older client
> will be unaware that it must avoid parallel writes that are
> 512-aligned but land in the same 4k page, so it seems like the
> blocksize filter should be doing that.
> 
> You have just demonstrated that our current approach (grabbing a
> single semaphore, only around the unaligned portions), does not do
> what we hoped.  So what WOULD protect us, while still allowing as much
> parallelism as possible?  It sounds like we want a RW-lock (note: not
> in the sense of parallel pread and exclusive pwrite operations, but
> rather in the sense of parallel aligned operations taking a rdlock
> whether it is a pread or pwrite operation, and exclusive unaligned
> operations taking a wrlock whether 

[Libguestfs] [PATCH nbdkit v2 4/5] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

POD verbatim paragraphs are introduced by a line starting with
whitespace and continue to the next line containing only whitespace
(or empty).  In the output they appear as preformatted text (like
 in HTML).  We use them extensively, but also as it turns out,
wrongly.

I previously assumed that all lines of the verbatim paragraph had to
be indented, but this is not true.  More seriously, adjacent verbatim
paragraphs are always grouped together into a single preformatted
block of output, whether or not there is a line containing whitespace
between the paragraphs.  As an example, these three pieces of POD
would be formatted identically (I have used “|” to mark the beginning
and end of each line to make whitespace clear):

(1)  (2)  (3)
  | nbdkit foo \|  | nbdkit foo \|  | nbdkit foo \|
  | --dump-plugin| |--dump-plugin|  | --dump-plugin|
  | |  | |  ||
  | nbdkit bar|| nbdkit bar|| nbdkit bar|

In all cases, there are two verbatim paragraphs, the first paragraph
has two lines and the second has one line.  The output is 4 lines of
preformatted text, grouped together.  In HTML output that would mean a
single  element.

In previous usage, we assumed that a space on a line on its own would
cause the adjacent verbatim paragraphs to be grouped together, but a
completely empty line (no whitespace) would cause the two verbatim
paragraphs to be split.  As you can see in case (3) above, this does
not work.

It is in fact possible to have adjacent verbatim paragraphs not be
grouped together.  It involves inserting an empty, zero length
ordinary paragraph between the two, which can be done using the
“=for paragraph” syntax:

 nbdkit foo \
 --dump-plugin

 =for paragraph

 nbdkit bar

In HTML output this will be formatted as two adjacent  elements.
Note that the blank lines before and after “=for paragraph” are
necessary.

This commit reformats all existing POD documentation in nbdkit to
conform.  There are two main changes:

(a) Completely empty lines occurring between two verbatim paragraphs
are replaced with “\n=for paragraph\n\n”, which as explained above
stops the verbatim paragraphs from running together.

(b) Lines which contain only whitespace (ie. those we previously used
to mean “group these verbatim paragraphs together”) are replaced with
completely empty lines.

The following commit adds syntax checking to podwrapper to identify
trailing whitespace in POD files, so that we don't introduce this kind
of error in future.

Note that I don't try to unindent the verbatim paragraphs, because
that looks weird and also makes it harder for us to parse the POD in
case we needed to in future.

This change was made mechanically using this Perl script:

 #!/usr/bin/perl -w
 use strict;

 foreach my $input (@ARGV) {
my $content;
{ local $/ = undef;
  open FILE, "<:encoding(UTF-8)", $input or die "$input: $!";
  $content =  }
# Verbatim paragraphs separated by empty line, means do not group.
my $oldcontent;
do {
$oldcontent = $content;
$content =~ s/(\n [^\n]*)\n\n /$1\n\n=for paragraph\n\n /g;
} until ($oldcontent eq $content);
# Any remaining lines containing only whitespace, group.
$content =~ s/\n[ \t]+\n/\n\n/g;
rename ($input, "$input.bak");
open FILE, ">:encoding(UTF-8)", $input or die "$input: $!";
print FILE $content;
 }

Thanks: Laszlo Ersek
See-also: 
https://listman.redhat.com/archives/libguestfs/2022-May/thread.html#28902
---
 docs/nbdkit-captive.pod|  2 ++
 docs/nbdkit-filter.pod | 14 +++---
 docs/nbdkit-plugin.pod | 12 +++-
 docs/nbdkit-probing.pod|  8 
 docs/nbdkit-service.pod|  4 ++--
 docs/nbdkit.pod|  2 ++
 .../checkwrite/nbdkit-checkwrite-filter.pod|  2 ++
 filters/ddrescue/nbdkit-ddrescue-filter.pod|  2 ++
 filters/delay/nbdkit-delay-filter.pod  |  4 
 filters/ext2/nbdkit-ext2-filter.pod|  2 ++
 filters/readahead/nbdkit-readahead-filter.pod  |  4 
 filters/scan/nbdkit-scan-filter.pod|  4 
 filters/stats/nbdkit-stats-filter.pod  |  8 
 filters/swab/nbdkit-swab-filter.pod|  4 
 filters/xz/nbdkit-xz-filter.pod|  2 ++
 plugins/S3/nbdkit-S3-plugin.pod|  2 +-
 plugins/cc/nbdkit-cc-plugin.pod| 18 +-
 plugins/curl/nbdkit-curl-plugin.pod|  4 ++--
 plugins/data/nbdkit-data-plugin.pod|  8 
 plugins/file/nbdkit-file-plugin.pod|  2 ++
 plugins/golang/nbdkit-golang-plugin.pod|  8 
 plugins/guestfs/nbdkit-guestfs-plugin.pod  |  4 
 plugins/info/nbdkit-info-plugin.pod|  2 ++
 plugins/memory/nbdkit-memory-plugin.pod|  4 ++--
 

[Libguestfs] [PATCH nbdkit v2 5/5] docs: Enhance podwrapper to detect trailing whitespace

[Richard W.M. Jones]

Now that trailing whitespace should no longer appear in POD files,
enhance podwrapper to detect this and error out.

I had to modify podwrapper so it doesn't introduce trailing whitespace
when it encounters a blank line in a --verbatim include.

Acked-by: Laszlo Ersek 
Acked-by: Eric Blake 
---
 podwrapper.pl.in | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/podwrapper.pl.in b/podwrapper.pl.in
index bd0cf3ee..e28020a0 100755
--- a/podwrapper.pl.in
+++ b/podwrapper.pl.in
@@ -286,6 +286,10 @@ die "$progname: $input: is not valid utf8" unless 
utf8::is_utf8 ($content);
 die "$progname: $input: =encoding must not be present in input\n"
 if $content =~ /^=encoding/m;
 
+# Don't permit trailing whitespace.
+die "$progname: $input: trailing whitespace in input\n"
+if $content =~ /[ \t]$/m;
+
 # We may add an encoding line, but this breaks RHEL 6-era Pod::Simple
 # with the error "Cannot decode string with wide characters".
 $content =~ s/^=(.*)/\n=encoding utf8\n\n=$1/m
@@ -576,14 +580,15 @@ sub read_whole_file
 sub read_verbatim_file
 {
 my $input = shift;
-my $r = "";
+my @r = ();
 
 open FILE, "<:encoding(UTF-8)", $input or die "$progname: $input: $!";
 while () {
-$r .= " $_";
+chomp;
+if (length) { push @r, " $_" } else { push @r, "" }
 }
 close FILE;
-$r;
+return join ("\n", @r) . "\n";
 }
 
 =head1 SEE ALSO
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit v2 3/5] docs: Remove trailing whitespace from POD file

[Richard W.M. Jones]

Fixes: commit 7d96ec5136c778e54fa27c70b2ca4b4f1456a706
---
 docs/nbdkit-release-notes-1.16.pod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/nbdkit-release-notes-1.16.pod 
b/docs/nbdkit-release-notes-1.16.pod
index 7e8d76fa..5334e33b 100644
--- a/docs/nbdkit-release-notes-1.16.pod
+++ b/docs/nbdkit-release-notes-1.16.pod
@@ -17,7 +17,7 @@ to the fixed versions is highly recommended.  The new
 L man page contains an up to date list of past
 security issues.
 
-=head3 CVE-2019-14850 
+=head3 CVE-2019-14850
 denial of service due to premature opening of back-end connection
 
 See the full announcement and links to mitigation, tests and fixes
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit v2 1/5] docs: Fix whitespace to logically group C code examples together

[Richard W.M. Jones]

Logically group the C code examples together into a single verbatim
section.  Note that a following commit will correct this one.

Fixes: commit 4ca66f70a5865efbad67d719ba84950ddafefc01
Fixes: commit d4c58f93d0589c522d5a3422e61ba7fde1c64255
Thanks: Laszlo Ersek
---
 docs/nbdkit-filter.pod | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 587b012e..416d6fe6 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -500,7 +500,7 @@ from the layer below.  Without error checking it would look 
like this:
struct nbdkit_exports *exports2;
struct nbdkit_export e;
char *name, *desc;
-
+ 
exports2 = nbdkit_exports_new ();
next_list_exports (nxdata, readonly, exports);
for (i = 0; i < nbdkit_exports_count (exports2); ++i) {
@@ -947,7 +947,7 @@ from the layer below.  Without error checking it would look 
like this:
struct nbdkit_extents *extents2;
struct nbdkit_extent e;
int64_t size;
-
+ 
size = next->get_size (next);
extents2 = nbdkit_extents_new (offset + shift, size);
next->extents (next, count, offset + shift, flags, extents2, err);
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit v2 0/5] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

v1 was here:
https://listman.redhat.com/archives/libguestfs/2022-May/thread.html#28934

v2:

 - Patches 1-3 are new.  They fix verbatim grouping and whitespace in
   existing documentation.

 - Patch 4 (was patch 1): I fixed the Perl script since it didn't work
   if you had multiple adjacent non-grouped verbatim paragraphs, as
   you find for example on this page:

   
https://gitlab.com/nbdkit/nbdkit/-/blob/b2612d2c3e24bb40c3cc76ef407d3b46df449dc1/docs/nbdkit-probing.pod#L7-L15

   I reran the new Perl script which now breaks up verbatim paragraphs
   these correctly, and also corrects the corrections in patches 1-3.

 - Patch 5 (was patch 2).  The same, except I moved the logically
   separate docs fix hunk into a new patch.  This should make cherry
   picking into other projects easier.

Rich.


___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit v2 2/5] rust: Logically group example code

[Richard W.M. Jones]

Fixes: commit 666ec24437932b27b393f7670ec0ecc5f11550bc
Thanks: Laszlo Ersek
---
 plugins/rust/nbdkit-rust-plugin.pod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/rust/nbdkit-rust-plugin.pod 
b/plugins/rust/nbdkit-rust-plugin.pod
index 9e3ef10e..ff9d9c09 100644
--- a/plugins/rust/nbdkit-rust-plugin.pod
+++ b/plugins/rust/nbdkit-rust-plugin.pod
@@ -37,7 +37,7 @@ and register it using the C macro.
  impl Server for MyPlugin {
  // ...
  }
-
+ 
  plugin!(MyPlugin {write_at, trim, ...});
 
 =head2 Compiling a Rust nbdkit plugin
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] SELinux relabeling: do it by default

[Richard W.M. Jones]

On Tue, May 24, 2022 at 03:59:24PM +0200, Laszlo Ersek wrote:
> On 05/24/22 12:04, Richard W.M. Jones wrote:
> > So I didn't realise that these commits break --selinux-relabel, eg:
> >
> > $ rpm -q guestfs-tools
> > guestfs-tools-1.49.1-1.fc37.x86_64
> > $ virt-builder fedora-36 --selinux-relabel
> > virt-builder: unrecognized option '--selinux-relabel'
> > Try ‘virt-builder --help’ or consult virt-builder(1) for more
> > information.
> >
> > This is kind of bad since it breaks existing scripts.
> 
> I carefully highlighted it in the cover letter:
> 
> https://listman.redhat.com/archives/libguestfs/2022-May/028826.html

I didn't carefully read it enough :-(

> > I've intentionally avoided introducing "--no-selinux-relabel" *in
> > addition* to "--selinux-relabel". While some utilities support a
> > similar dual form (such as virt-builder's "--network" and
> > "--no-network"), with one being the default, those options are special
> > in that they are *not shared* between different utilities, and they
> > are not generated by the generator in libguestfs. The key difference
> > is that the *non-shared* options use Getopt.Set and Getopt.Clear on
> > the *same* boolean reference cell, whereas the generator introduces a
> > *separate* boolean reference cell for each option it generates (and
> > then it uses *either* Getopt.Clear *or* Getopt.Set when the option is
> > passed on the command line, dependent on the default value of the
> > reference cell). This means that "--no-selinux-relabel" and
> > "--selinux-relabel", if they both existed, would work on different
> > booleans, and that would be the source of a lot of fun (priority?
> > command line order? documentation? etc etc). So, nope to that.
> 
> On 05/24/22 12:04, Richard W.M. Jones wrote:
> > We just had a bug report about this from someone who is testing CentOS
> > Stream 9.1 (where I backported the patches already).
> >
> > This option should continue to exist, but do nothing.
> 
> The problems I outlined above remain.
> 
> What happens if someone passes both options?
> 
> Are we supposed to continue documenting "--selinux-relabel"?

AIUI this option should remain in the code, but do nothing.  If it can
never do anything[0] then we are allowed to drop it from
documentation.

[0] = I'm assuming here that even for a non-SELinux guest this option
would do nothing.  It wouldn't do some doomed attempt to relabel it.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit 1/2] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

FYI I found a few other mistakes, and a mistake in the perl script,
so v2 definitely incoming ...

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit 1/2] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

On Tue, May 24, 2022 at 03:20:19PM +0200, Laszlo Ersek wrote:
> > @@ -118,7 +118,7 @@ that the filter wants to intercept.
> > .config= myfilter_config,
> > /* etc */
> >   };
> > - 
> > +
> >   NBDKIT_REGISTER_FILTER(filter)
> >  
> >  The C<.name> field is the name of the filter.  This is the only field
> > @@ -501,6 +501,8 @@ from the layer below.  Without error checking it would 
> > look like this:
> > struct nbdkit_export e;
> > char *name, *desc;
> >  
> > +=for paragraph
> > +
> > exports2 = nbdkit_exports_new ();
> > next_list_exports (nxdata, readonly, exports);
> > for (i = 0; i < nbdkit_exports_count (exports2); ++i) {
>
> The above insertion splits the myfilter_list_exports() code example,
> which is currently rendered very nicely in a single verbatim
> paragraph:
...
> into two paragraphs -- for no good reason AFAICT.

Yup, that's wrong (although it was also wrong before).

> I didn't go as far as actually applying this patch and rebuilding
> nbdkit, but I did hand-edit the rendered HTML from the website in
> the Firefox inspector:
...
> and that looks like the attachment shows -- the red bar to the left
> is broken up, and there's a white gap between the declarations and
> the code.

Yup, that's what is supposed to happen for logically separate blocks,
but not in this particular case.

> Now if we actually wanted to separate these verbatim paragraphs,
> this output would be visually pleasing, so I'm not complaing about
> that. What I'm saying is that our current use of completely empty
> lines is inconsistent; sometimes it means "split these adjacent
> verbatim paragraphs apart", sometimes it means "keep them
> fused". Therefore we shouldn't automate the "=for paragraph"
> insertion.

The alternative is going to be pretty tedious though, and I should
think more error-prone.

How about fixing this particular problem in a new, prior commit, then
applying the perl script again, which this time would change the space
to an empty line and wouldn't insert "=for paragraph"?

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] SELinux relabeling: do it by default

[Richard W.M. Jones]

On Tue, May 24, 2022 at 11:04:49AM +0100, Richard W.M. Jones wrote:
> So I didn't realise that these commits break --selinux-relabel, eg:
> 
> $ rpm -q guestfs-tools
> guestfs-tools-1.49.1-1.fc37.x86_64
> $ virt-builder fedora-36 --selinux-relabel
> virt-builder: unrecognized option '--selinux-relabel'
> Try ‘virt-builder --help’ or consult virt-builder(1) for more information.
> 
> This is kind of bad since it breaks existing scripts.  We just had a
> bug report about this from someone who is testing CentOS Stream 9.1
> (where I backported the patches already).
> 
> This option should continue to exist, but do nothing.

RHEL 9.1 bug:

https://bugzilla.redhat.com/show_bug.cgi?id=2089748

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] SELinux relabeling: do it by default

[Richard W.M. Jones]

So I didn't realise that these commits break --selinux-relabel, eg:

$ rpm -q guestfs-tools
guestfs-tools-1.49.1-1.fc37.x86_64
$ virt-builder fedora-36 --selinux-relabel
virt-builder: unrecognized option '--selinux-relabel'
Try ‘virt-builder --help’ or consult virt-builder(1) for more information.

This is kind of bad since it breaks existing scripts.  We just had a
bug report about this from someone who is testing CentOS Stream 9.1
(where I backported the patches already).

This option should continue to exist, but do nothing.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] Erro libguestfs missing shared libraries: libpcre2-8.so.0

[Richard W.M. Jones]

On Mon, May 23, 2022 at 10:32:16AM +0200, Attilio Greco wrote:
> mkdir: error while loading shared libraries: libpcre2-8.so.0: cannot
> open shared object file: No such file or directory

This error is weird.  What's the output of:

  rpm -q coreutils pcre2
  ldd /usr/bin/mkdir 
  rpm -qf /lib64/libpcre2-8.so.0

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH libguestfs] build: When parsing distro from /etc/os-release, try $ID_LIKE first

[Richard W.M. Jones]

On Sun, May 22, 2022 at 06:41:14PM +0100, Richard W.M. Jones wrote:
> The current code for working out the distro uses the ID entry from
> /etc/os-release, and then we map those strings into a smaller set of
> values (basically, what package manager to use).  However it was
> suggested that we should try ID_LIKE first so that distros which act
> like other distros would work.  On an Arch Linux 32 system:
> 
> ID=arch32
> ID_LIKE=arch
> 
> See-also: https://github.com/libguestfs/libguestfs/issues/81
> Thanks: S D Rausty
> ---
>  m4/guestfs-appliance.m4 | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)

The bug reporter seems to have tested this and it works, so I pushed
it (commit 63b722b6c094).

Rich.

> diff --git a/m4/guestfs-appliance.m4 b/m4/guestfs-appliance.m4
> index 9e443f151d..4e63ef4356 100644
> --- a/m4/guestfs-appliance.m4
> +++ b/m4/guestfs-appliance.m4
> @@ -102,8 +102,16 @@ AC_ARG_WITH([distro],
>  AC_MSG_RESULT([$DISTRO (manually specified)])
>  ],[
>  if test -f /etc/os-release; then
> -( . /etc/os-release && echo $ID | tr '@<:@:lower:@:>@' 
> '@<:@:upper:@:>@' ) >_MESSAGE_LOG_FD
> -DISTRO="`. /etc/os-release && echo $ID | tr '@<:@:lower:@:>@' 
> '@<:@:upper:@:>@'`"
> +echo "/etc/os-release:" >_MESSAGE_LOG_FD
> +cat /etc/os-release >_MESSAGE_LOG_FD
> +DISTRO="$(
> +. /etc/os-release
> +if test -n "$ID_LIKE"; then
> +echo $ID_LIKE | tr '@<:@:lower:@:>@' '@<:@:upper:@:>@'
> +else
> +echo $ID  | tr '@<:@:lower:@:>@' '@<:@:upper:@:>@'
> +fi
> +)"
>  AS_CASE([$DISTRO],
>  [FEDORA | RHEL | CENTOS | ALMALINUX | CLOUDLINUX | 
> ROCKY],
>  [DISTRO=REDHAT],
> @@ -116,6 +124,7 @@ AC_ARG_WITH([distro],
>  fi
>  ]
>  )
> +AC_SUBST([DISTRO])
>  AM_CONDITIONAL([HAVE_RPM],
>  [AS_CASE([$DISTRO], [REDHAT | SUSE | OPENMANDRIVA | MAGEIA ], [true],
>  [*], [false])])
> @@ -125,7 +134,6 @@ AM_CONDITIONAL([HAVE_DPKG],
>  AM_CONDITIONAL([HAVE_PACMAN],
>  [AS_CASE([$DISTRO], [ARCHLINUX | FRUGALWARE | ARTIX], [true],
>  [*], [false])])
> -AC_SUBST([DISTRO])
>  
>  dnl Add extra packages to the appliance.
>  AC_ARG_WITH([extra-packages],
> -- 
> 2.35.1
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH libguestfs] build: When parsing distro from /etc/os-release, try $ID_LIKE first

[Richard W.M. Jones]

The current code for working out the distro uses the ID entry from
/etc/os-release, and then we map those strings into a smaller set of
values (basically, what package manager to use).  However it was
suggested that we should try ID_LIKE first so that distros which act
like other distros would work.  On an Arch Linux 32 system:

ID=arch32
ID_LIKE=arch

See-also: https://github.com/libguestfs/libguestfs/issues/81
Thanks: S D Rausty
---
 m4/guestfs-appliance.m4 | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/m4/guestfs-appliance.m4 b/m4/guestfs-appliance.m4
index 9e443f151d..4e63ef4356 100644
--- a/m4/guestfs-appliance.m4
+++ b/m4/guestfs-appliance.m4
@@ -102,8 +102,16 @@ AC_ARG_WITH([distro],
 AC_MSG_RESULT([$DISTRO (manually specified)])
 ],[
 if test -f /etc/os-release; then
-( . /etc/os-release && echo $ID | tr '@<:@:lower:@:>@' 
'@<:@:upper:@:>@' ) >_MESSAGE_LOG_FD
-DISTRO="`. /etc/os-release && echo $ID | tr '@<:@:lower:@:>@' 
'@<:@:upper:@:>@'`"
+echo "/etc/os-release:" >_MESSAGE_LOG_FD
+cat /etc/os-release >_MESSAGE_LOG_FD
+DISTRO="$(
+. /etc/os-release
+if test -n "$ID_LIKE"; then
+echo $ID_LIKE | tr '@<:@:lower:@:>@' '@<:@:upper:@:>@'
+else
+echo $ID  | tr '@<:@:lower:@:>@' '@<:@:upper:@:>@'
+fi
+)"
 AS_CASE([$DISTRO],
 [FEDORA | RHEL | CENTOS | ALMALINUX | CLOUDLINUX | ROCKY],
 [DISTRO=REDHAT],
@@ -116,6 +124,7 @@ AC_ARG_WITH([distro],
 fi
 ]
 )
+AC_SUBST([DISTRO])
 AM_CONDITIONAL([HAVE_RPM],
 [AS_CASE([$DISTRO], [REDHAT | SUSE | OPENMANDRIVA | MAGEIA ], [true],
 [*], [false])])
@@ -125,7 +134,6 @@ AM_CONDITIONAL([HAVE_DPKG],
 AM_CONDITIONAL([HAVE_PACMAN],
 [AS_CASE([$DISTRO], [ARCHLINUX | FRUGALWARE | ARTIX], [true],
 [*], [false])])
-AC_SUBST([DISTRO])
 
 dnl Add extra packages to the appliance.
 AC_ARG_WITH([extra-packages],
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] nbdkit blocksize filter, read-modify-write, and concurrency

[Richard W.M. Jones]

On Sat, May 21, 2022 at 05:37:10PM +0100, Nikolaus Rath wrote:
> On May 21 2022, "Richard W.M. Jones"  wrote:
> > On Sat, May 21, 2022 at 01:21:11PM +0100, Nikolaus Rath wrote:
> >> Hi,
> >>
> >> How does the blocksize filter take into account writes that end-up
> >> overlapping due to read-modify-write cycles?
> >>
> >> Specifically, suppose there are two non-overlapping writes handled
> >> by two different threads, that, due to blocksize requirements,
> >> overlap when expanded.  I think there is a risk that one thread may
> >> partially undo the work of the other here.
> >>
> >> Looking at the code, it seems that writes of unaligned heads and
> >> tails are protected with a global lock., but writes of aligned data
> >> can occur concurrently.
> >
> > I agree.
> >
> > Assuming the underlying plugin is NBDKIT_THREAD_MODEL_PARALLEL and no
> > other filters impose thread model limits, the blocksize filter does
> > not limit the thread model, so the thread model of nbdkit would also
> > be NBDKIT_THREAD_MODEL_PARALLEL.
> >
> > That means that two writes either on different connections or
> > pipelined on the same connection could happen at the same time.
> > “blocksize_pwrite” would be called concurrently for the two requests.
> >
> >> However, does this not miss the case where there is one unaligned
> >> write that overlaps with an aligned one?
> >>
> >> For example, with blocksize 10, we could have:
> >> 
> >> Thread 1: receives write request for offset=0, size=10
> >> Thread 2: receives write request for offset=4, size=16
> >> Thread 1: acquires lock, reads bytes 0-4
> >> Thread 2: does aligned write (no locking needed), writes bytes 0-10
> >> Thread 1: writes bytes 0-10, overwriting data from Thread 2
> >
> > I believe this analysis is correct.  (CC'd to Eric who knows a lot
> > more about this.)
> >
> > However I don't think it's a bug.  If a client doesn't want writes to
> > squash each other, then it shouldn't send overlapping requests.  I bet
> > the same thing happens with an SSD.
> 
> But the requests are not overlapping from the client point of view. They
> only become overlapping when the server applies its read-modify-write
> operation to align them to the blocksize.

I'm going to leave this one to Eric who's an expert on this ("write
tearing", I think).

> I think you elsewhere said that the blocksize reported by the NBD server
> is only a preferred blocksize, so I'd be surprised if not following this
> "preference" results in data corruption.

This is true for NBD at the moment, but I think everyone accepts it's
a mistake in the protocol.  Eric was looking into this too.

> > NBD_CMD_FLAG_FUA is provided for clients that wish to ensure that a
> > write has been committed before sending another request.
> >
> > Do you have an example of a client which sends overlapping requests
> > and depends on particular behaviour of the server?  You may be able to
> > get it to work by using nbdkit-noparallel-filter which can be used to
> > serialize nbdkit.
> 
> I'm working with the kernel's NBD client, and it would explain all the
> mysterious data corruption issues that I've seen with the S3 plugin. But
> I have not yet confirmed definitely that this is the root cause.
> 
> For now, I'll avoid the blocksize filter and instead do the
> read-modify-write in the plugin with proper locking. If that fixes it,
> then I think we can conclude that the kernel is sending such requests
> (but, as I said above, I would not consider them overlapping nor would I
> consider this a bug).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] nbdkit blocksize filter, read-modify-write, and concurrency

[Richard W.M. Jones]

On Sat, May 21, 2022 at 01:21:11PM +0100, Nikolaus Rath wrote:
> Hi,
>
> How does the blocksize filter take into account writes that end-up
> overlapping due to read-modify-write cycles?
>
> Specifically, suppose there are two non-overlapping writes handled
> by two different threads, that, due to blocksize requirements,
> overlap when expanded.  I think there is a risk that one thread may
> partially undo the work of the other here.
>
> Looking at the code, it seems that writes of unaligned heads and
> tails are protected with a global lock., but writes of aligned data
> can occur concurrently.

I agree.

Assuming the underlying plugin is NBDKIT_THREAD_MODEL_PARALLEL and no
other filters impose thread model limits, the blocksize filter does
not limit the thread model, so the thread model of nbdkit would also
be NBDKIT_THREAD_MODEL_PARALLEL.

That means that two writes either on different connections or
pipelined on the same connection could happen at the same time.
“blocksize_pwrite” would be called concurrently for the two requests.

> However, does this not miss the case where there is one unaligned
> write that overlaps with an aligned one?
>
> For example, with blocksize 10, we could have:
> 
> Thread 1: receives write request for offset=0, size=10
> Thread 2: receives write request for offset=4, size=16
> Thread 1: acquires lock, reads bytes 0-4
> Thread 2: does aligned write (no locking needed), writes bytes 0-10
> Thread 1: writes bytes 0-10, overwriting data from Thread 2

I believe this analysis is correct.  (CC'd to Eric who knows a lot
more about this.)

However I don't think it's a bug.  If a client doesn't want writes to
squash each other, then it shouldn't send overlapping requests.  I bet
the same thing happens with an SSD.

NBD_CMD_FLAG_FUA is provided for clients that wish to ensure that a
write has been committed before sending another request.

Do you have an example of a client which sends overlapping requests
and depends on particular behaviour of the server?  You may be able to
get it to work by using nbdkit-noparallel-filter which can be used to
serialize nbdkit.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit 2/2] docs: Enhance podwrapper to detect trailing whitespace

[Richard W.M. Jones]

Now that trailing whitespace should no longer appear in POD files,
enhance podwrapper to detect this and error out.

I had to modify podwrapper so it doesn't introduce trailing whitespace
when it encounters a blank line in a --verbatim include.

I also fixed an instance of unwanted trailing whitespace.
---
 docs/nbdkit-release-notes-1.16.pod |  2 +-
 podwrapper.pl.in   | 11 ---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/docs/nbdkit-release-notes-1.16.pod 
b/docs/nbdkit-release-notes-1.16.pod
index 7e8d76fa..5334e33b 100644
--- a/docs/nbdkit-release-notes-1.16.pod
+++ b/docs/nbdkit-release-notes-1.16.pod
@@ -17,7 +17,7 @@ to the fixed versions is highly recommended.  The new
 L man page contains an up to date list of past
 security issues.
 
-=head3 CVE-2019-14850 
+=head3 CVE-2019-14850
 denial of service due to premature opening of back-end connection
 
 See the full announcement and links to mitigation, tests and fixes
diff --git a/podwrapper.pl.in b/podwrapper.pl.in
index bd0cf3ee..e28020a0 100755
--- a/podwrapper.pl.in
+++ b/podwrapper.pl.in
@@ -286,6 +286,10 @@ die "$progname: $input: is not valid utf8" unless 
utf8::is_utf8 ($content);
 die "$progname: $input: =encoding must not be present in input\n"
 if $content =~ /^=encoding/m;
 
+# Don't permit trailing whitespace.
+die "$progname: $input: trailing whitespace in input\n"
+if $content =~ /[ \t]$/m;
+
 # We may add an encoding line, but this breaks RHEL 6-era Pod::Simple
 # with the error "Cannot decode string with wide characters".
 $content =~ s/^=(.*)/\n=encoding utf8\n\n=$1/m
@@ -576,14 +580,15 @@ sub read_whole_file
 sub read_verbatim_file
 {
 my $input = shift;
-my $r = "";
+my @r = ();
 
 open FILE, "<:encoding(UTF-8)", $input or die "$progname: $input: $!";
 while () {
-$r .= " $_";
+chomp;
+if (length) { push @r, " $_" } else { push @r, "" }
 }
 close FILE;
-$r;
+return join ("\n", @r) . "\n";
 }
 
 =head1 SEE ALSO
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH nbdkit 1/2] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

POD verbatim paragraphs are introduced by a line starting with
whitespace and continue to the next line containing only whitespace
(or empty).  In the output they appear as preformatted text (like
 in HTML).  We use them extensively, but also as it turns out,
wrongly.

I previously assumed that all lines of the verbatim paragraph had to
be indented, but this is not true.  More seriously, adjacent verbatim
paragraphs are always run together, whether or not there is a line
containing whitespace between the paragraphs or not.  As an example,
these three pieces of POD would be formatted identically (I have used
“|” to mark the beginning and end of each line to make whitespace
clear):

(1)  (2)  (3)
  | nbdkit foo \|  | nbdkit foo \|  | nbdkit foo \|
  | --dump-plugin| |--dump-plugin|  | --dump-plugin|
  | |  | |  ||
  | nbdkit bar|| nbdkit bar|| nbdkit bar|

In all cases, there are two verbatim paragraphs, the first paragraph
has two lines and the second has one line.  The output is 4 lines of
preformatted text, grouped together.  In HTML output that would mean a
single  element.

In previous usage, we assumed that a space on a line on its own would
cause the adjacent verbatim paragraphs to be grouped together, but a
completely blank line would cause the two verbatim paragraphs to be
split.  As you can see in case (3) above, this does not work.

It is in fact possible to have adjacent verbatim paragraphs not be
grouped together.  It involves inserting an empty, zero length
ordinary paragraph between the two, which can be done using the
“=for paragraph” syntax:

 nbdkit foo \
 --dump-plugin

 =for paragraph

 nbdkit bar

In HTML output this will be formatted as two  elements.  Note
that the blank lines before and after “=for paragraph” are necessary.

This commit reformats all existing POD documentation in nbdkit to
conform.  There are two main changes:

(a) Lines which contain only whitespace (ie. those we previously used
to mean “group these verbatim paragraphs together”) are replaced with
completely empty lines.

(b) Completely empty lines occurring between two verbatim paragraphs
are replaced with “\n=for paragraph\n\n”, ie. inserting a blank
ordinary paragraph to stop the verbatim paragraphs from running
together.

The following commit adds syntax checking to podwrapper to identify
trailing whitespace in POD files, so that we don't introduce this kind
of error in future.

Note that I don't try to unindent the verbatim paragraphs, because
that looks weird and also makes it harder for us to parse the POD in
case we needed to in future.

This commit change was made mechanically using this Perl script:

use strict;

foreach my $input (@ARGV) {
my $content;
{ local $/ = undef;
  open FILE, "<:encoding(UTF-8)", $input or die "$input: $!";
  $content =  }
# Verbatim paragraphs separated by empty line, means do not group.
$content =~ s/(\n [^\n]*)\n\n /$1\n\n=for paragraph\n\n /g;
# Any remaining lines containing only whitespace, group.
$content =~ s/\n[ \t]+\n/\n\n/g;
rename ($input, "$input.bak");
open FILE, ">:encoding(UTF-8)", $input or die "$input: $!";
print FILE $content;
}

Thanks: Laszlo Ersek
See-also: 
https://listman.redhat.com/archives/libguestfs/2022-May/thread.html#28902
---
 docs/nbdkit-captive.pod|  2 ++
 docs/nbdkit-filter.pod | 14 +-
 docs/nbdkit-plugin.pod | 12 +++-
 docs/nbdkit-probing.pod|  4 
 docs/nbdkit-service.pod|  4 ++--
 docs/nbdkit.pod|  2 ++
 .../checkwrite/nbdkit-checkwrite-filter.pod|  2 ++
 filters/ddrescue/nbdkit-ddrescue-filter.pod|  2 ++
 filters/delay/nbdkit-delay-filter.pod  |  2 ++
 filters/ext2/nbdkit-ext2-filter.pod|  2 ++
 filters/readahead/nbdkit-readahead-filter.pod  |  2 ++
 filters/scan/nbdkit-scan-filter.pod|  2 ++
 filters/stats/nbdkit-stats-filter.pod  |  8 
 filters/swab/nbdkit-swab-filter.pod|  4 
 filters/xz/nbdkit-xz-filter.pod|  2 ++
 plugins/S3/nbdkit-S3-plugin.pod|  2 +-
 plugins/cc/nbdkit-cc-plugin.pod| 18 +-
 plugins/curl/nbdkit-curl-plugin.pod|  4 ++--
 plugins/data/nbdkit-data-plugin.pod|  8 
 plugins/file/nbdkit-file-plugin.pod|  2 ++
 plugins/golang/nbdkit-golang-plugin.pod|  8 
 plugins/guestfs/nbdkit-guestfs-plugin.pod  |  4 
 plugins/info/nbdkit-info-plugin.pod|  2 ++
 plugins/memory/nbdkit-memory-plugin.pod|  4 ++--
 plugins/nbd/nbdkit-nbd-plugin.pod  |  4 
 plugins/ocaml/nbdkit-ocaml-plugin.pod  |  6 --
 plugins/ondemand/nbdkit-ondemand-plugin.pod|  2 ++
 

[Libguestfs] [PATCH nbdkit 0/2] docs: Fix POD verbatim paragraphs

[Richard W.M. Jones]

Fix POD verbatim paragraphs in nbdkit, per discussion last week.
If this is right, I'll roll out something similar to other projects.

Rich.


___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] Erro libguestfs missing shared libraries: libpcre2-8.so.0

[Richard W.M. Jones]

On Sat, May 21, 2022 at 10:55:00AM +0100, Richard W.M. Jones wrote:
> On Fri, May 20, 2022 at 12:45:20PM +0200, Attilio Greco wrote:
> > Hi all,
> > I'm hitting an error during disk resize.
> > 
> > Pakage is installed via repo pakage info:
> > libguestfs.x86_64 1:1.48.1-1.fc35 
> > @updates
> > 
> > Disrtro info:
> > 
> > lsb_release -a
> > LSB Version::core-4.1-amd64:core-4.1-noarch
> > Distributor ID: Fedora
> > Description:Fedora release 35 (Thirty Five)
> > Release:35
> > Codename:   ThirtyFive
> > 
> > Kerne Info
> > uname -a
> > Linux fedora 5.17.7-200.fc35.x86_64 #1 SMP PREEMPT Thu May 12 14:56:48
> > UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
> > 
> > mkdir: error while loading shared libraries: libpcre2-8.so.0: cannot
> > open shared object file: No such file or directory
> > mount: error while loading shared libraries: libpcre2-8.so.0: cannot
> > open shared object file: No such file or directory
> 
> Thanks - this is a fault in Fedora in the way the package
> was compiled.
> 
> This is definitely happening with the unmodified Fedora package?

Can you try this update:

https://bodhi.fedoraproject.org/updates/FEDORA-2022-8534a4e671
(https://koji.fedoraproject.org/koji/taskinfo?taskID=87324180)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] Erro libguestfs missing shared libraries: libpcre2-8.so.0

[Richard W.M. Jones]

On Fri, May 20, 2022 at 12:45:20PM +0200, Attilio Greco wrote:
> Hi all,
> I'm hitting an error during disk resize.
> 
> Pakage is installed via repo pakage info:
> libguestfs.x86_64 1:1.48.1-1.fc35 
> @updates
> 
> Disrtro info:
> 
> lsb_release -a
> LSB Version:  :core-4.1-amd64:core-4.1-noarch
> Distributor ID:   Fedora
> Description:  Fedora release 35 (Thirty Five)
> Release:  35
> Codename: ThirtyFive
> 
> Kerne Info
> uname -a
> Linux fedora 5.17.7-200.fc35.x86_64 #1 SMP PREEMPT Thu May 12 14:56:48
> UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
> 
> mkdir: error while loading shared libraries: libpcre2-8.so.0: cannot
> open shared object file: No such file or directory
> mount: error while loading shared libraries: libpcre2-8.so.0: cannot
> open shared object file: No such file or directory

Thanks - this is a fault in Fedora in the way the package
was compiled.

This is definitely happening with the unmodified Fedora package?

Rich.

PS. Sorry about your email to the list being stuck in the moderator
queue.  We're having a problem with the mailing list today.  I've
filed a ticket with the list maintainers.

-- 
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2] docs: Add extra permission needed for non-admin download with VMware 7

[Richard W.M. Jones]

On Thu, May 19, 2022 at 01:27:38PM +0200, Laszlo Ersek wrote:
> On 05/19/22 12:52, Richard W.M. Jones wrote:
> > From: Ming Xie 
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1817050
> > ---
> >  docs/virt-v2v-input-vmware.pod | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
> > index ab2d28e41f..bccf0f7490 100644
> > --- a/docs/virt-v2v-input-vmware.pod
> > +++ b/docs/virt-v2v-input-vmware.pod
> > @@ -559,8 +559,9 @@ write to libvirt or any other supported target.
> >  
> >  Instead of using the vCenter Administrator role, you can create a
> >  custom non-administrator role to perform the conversion.  You will
> > -however need to give it a minimum set of permissions as follows
> > -(using VMware vCenter 6.5):
> > +however need enable the following permissions (or as many as are
> > +available, older versions of VMware were missing some of these
> > +settings):
> >  
> >  =over 4
> >  
> > @@ -585,6 +586,7 @@ Enable (check) the following objects:
> > Provisioning:
> >   - Allow disk access
> >   - Allow read-only disk access
> > + - Allow virtual machine download
> >   
> >   Cryptographic operations:
> >- Decrypt
> > 
> 
> Great, thanks!
> 
> Acked-by: Laszlo Ersek 

Thanks - commit 2bf8fae2a6

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH] docs: Add extra permission needed for non-admin download with VMware 7

[Richard W.M. Jones]

On Thu, May 19, 2022 at 12:29:36PM +0200, Laszlo Ersek wrote:
> On 05/19/22 10:52, Richard W.M. Jones wrote:
> > From: Ming Xie 
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1817050
> > ---
> >  docs/virt-v2v-input-vmware.pod | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
> > index ab2d28e41f..60cff93994 100644
> > --- a/docs/virt-v2v-input-vmware.pod
> > +++ b/docs/virt-v2v-input-vmware.pod
> > @@ -585,6 +585,7 @@ Enable (check) the following objects:
> > Provisioning:
> >   - Allow disk access
> >   - Allow read-only disk access
> > + - Allow virtual machine download
> >   
> >   Cryptographic operations:
> >- Decrypt
> > 
> 
> The context a bit higher up includes "using VMware vCenter 6.5"; does
> that still apply here?
> 
> ... Either way, if this new permission simply didn't exist under VMware
> vCenter 6.5 yet, users on that platform will just ignore this new hint here.
> 
> Acked-by: Laszlo Ersek 

I tightened up the preceeding text to say that users should
set as many as possible.  v2 sent.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2] docs: Add extra permission needed for non-admin download with VMware 7

[Richard W.M. Jones]

From: Ming Xie 

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1817050
---
 docs/virt-v2v-input-vmware.pod | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
index ab2d28e41f..bccf0f7490 100644
--- a/docs/virt-v2v-input-vmware.pod
+++ b/docs/virt-v2v-input-vmware.pod
@@ -559,8 +559,9 @@ write to libvirt or any other supported target.
 
 Instead of using the vCenter Administrator role, you can create a
 custom non-administrator role to perform the conversion.  You will
-however need to give it a minimum set of permissions as follows
-(using VMware vCenter 6.5):
+however need enable the following permissions (or as many as are
+available, older versions of VMware were missing some of these
+settings):
 
 =over 4
 
@@ -585,6 +586,7 @@ Enable (check) the following objects:
Provisioning:
  - Allow disk access
  - Allow read-only disk access
+ - Allow virtual machine download
  
  Cryptographic operations:
   - Decrypt
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] ovirt VM image to BM

[Richard W.M. Jones]

On Thu, May 19, 2022 at 12:25:53PM +0200, Laszlo Ersek wrote:
> On 05/18/22 15:30, Richard W.M. Jones wrote:
> > But the keyboard doesn't work.  This is also surprising because I
> > *thought* that both virt and baremetal basically use emulated or real
> > USB keyboard these days.

[This bit is wrong - on x86 under virt we emulate a PS/2 keyboard]

> The keyboard does not work -- where?
> 
> Once the OS is reached (gdm or console root prompt), or in grub
> (pre-boot, basically)?

It got all the way to gdm but apparently the keyboard didn't work for
login etc.  The baremetal server is a Dell and Guilherme was accessing
this over iDRAC.  No idea what kind of "virtual" keyboard iDRAC
presents to the hardware.

I don't know what happened in the end.  Last I heard Guilherme was
trying to reach the server over ssh.

> The latter, I can explain. UEFI has a phase called "Boot Device
> Selection" (BDS) where the firmware decides exactly what devices to
> "connect" -- meaning what devices should be *bound* by UEFI drivers.
> 
> There is a number of policies here; the gist is that the platform vendor
> can do whatever they want -- BDS is called "platform policy".
> 
> Usually they support at least two configs, "full config" and "fast
> boot". Under "fast boot", the firmware only connects such devices to
> UEFI drivers that the firmware *knows* are needed for booting, and
> ignores everything else, including newly connected devices, potentially.
> Therefore, a USB keyboard could end up "non-driven" by UEFI because it
> is not needed for booting!

Would Linux still recognise the keyboard even in "fast boot" mode?

> The fix is to get into the firmware setup UI by some other way [*], and
> either disable "fast boot", or -- if the firmware is flexible enough --
> explicitly enable the binding of USB devices.
> 
> [*] From "/etc/grub.d/30_uefi-firmware", the command
> 
>   grub2-mkconfig -o /etc/grub2-efi.cfg
> 
> should create a grub2 menu entry, usually at the end of the boot menu,
> that invokes the "fwsetup" grub2 command. The trick is to make this
> entry the default entry, once the OS has fully booted (I guess via
> GRUB_DEFAULT in "/etc/default/grub"), and then reboot. When grub2
> executes "fwsetup", it modifies a particular non-volatile UEFI global
> variable ("OsIndications"), and upon seeing that, the firmware at next
> boot will drop into the setup UI.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 1/2] docs/*.pod: strip trailing space characters

[Richard W.M. Jones]

On Thu, May 19, 2022 at 11:57:21AM +0200, Laszlo Ersek wrote:
> On 05/18/22 12:04, Richard W.M. Jones wrote:
> > On Wed, May 18, 2022 at 08:49:29AM +0200, Laszlo Ersek wrote:
> >> Remove any space characters that directly precede a newline character.
> >>
> >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1938954
> >> Signed-off-by: Laszlo Ersek 
> >> ---
> >>
> >> Notes:
> >> I've verified in the rendered HTMLs that this whitespace stripping does
> >> not break up indented blocks into smaller blocks -- the formatter keeps
> >> runs of indented lines coalesced into indented blocks.
> > 
> > This isn't what I expected at all, but I also checked the HTML and it
> > seems what you're saying is correct.  I wonder if this behaviour has
> > changed in POD ..?  For completeness I also looked at the roff output
> > and that groups everything into a single .Vb/.Ve section as well.
> > 
> > This is true at least as far back as RHEL 7
> > (perl-podlators-2.5.1-3.el7.noarch)
> > 
> > I'm also wondering if there's a way to express "separate but adjacent"
> > verbatim sections.  This led me to the sources where I found that the
> > rules for verbatim paragraphs are a lot more complicated than I
> > thought I knew.  Any paragraph that begins with a space is verbatim,
> > even if following lines are not indented, eg:
> > 
> >  This is
> > a verbatim paragraph
> > 
> > And the spec (perlpodspec(1)) says that adjacent verbatim paragraphs
> > must be run together.
> > 
> > I cannot find a way to do "separate but adjacent".  eg. this does not work:
> > 
> > =begin verbatim
> > 
> >  Para 1
> > 
> > =end verbatim
> > 
> > =begin verbatim
> > 
> >  Para 2
> > 
> > =end verbatim
> > 
> > It ends up as a single verbatim section.
> > 
> >>  docs/virt-v2v-hacking.pod  |  2 +-
> >>  docs/virt-v2v-input-vmware.pod | 10 
> >>  docs/virt-v2v.pod  | 24 ++--
> >>  3 files changed, 18 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/docs/virt-v2v-hacking.pod b/docs/virt-v2v-hacking.pod
> >> index 29b73fb6c3f8..da5e640d9c92 100644
> >> --- a/docs/virt-v2v-hacking.pod
> >> +++ b/docs/virt-v2v-hacking.pod
> >> @@ -1,6 +1,6 @@
> >>  =head1 NAME
> >>  
> >> -virt-v2v-hacking - 
> >> +virt-v2v-hacking -
> >>  
> >>  =head1 DESCRIPTION
> > 
> > (This man page really needs a complete rewrite.)
> > 
> >> diff --git a/docs/virt-v2v-input-vmware.pod 
> >> b/docs/virt-v2v-input-vmware.pod
> >> index ab2d28e41ff2..4f4af2a9d804 100644
> >> --- a/docs/virt-v2v-input-vmware.pod
> >> +++ b/docs/virt-v2v-input-vmware.pod
> >> @@ -278,7 +278,7 @@ to list the guests on the server:
> >>  
> >>   $ virsh -c 'vpx://r...@vcenter.example.com/Datacenter/esxi' list --all
> >>   Enter root's password for vcenter.example.com: ***
> >> - 
> >> +
> >>IdName   State
> >>   
> >>- Fedora 20  shut off
> >> @@ -506,7 +506,7 @@ like this:
> >>  
> >>   $ virsh -c 'vpx://r...@vcenter.example.com/Datacenter/esxi' list --all
> >>   Enter root's password for vcenter.example.com: ***
> >> - 
> >> +
> >>IdName   State
> >>   
> >>- Fedora 20  shut off
> >> @@ -575,17 +575,17 @@ Enable (check) the following objects:
> >>   Datastore:
> >>- Browse datastore
> >>- Low level file operations
> >> - 
> >> +
> >>   Sessions:
> >>- Validate session
> >> - 
> >> +
> >>   Virtual Machine:
> >> Interaction:
> >>   - Guest operating system management by VIX API
> >> Provisioning:
> >>   - Allow disk access
> >>   - Allow read-only disk access
> >> - 
> >> +
> >>   Cryptographic operations:
> >>- Decrypt
> >>- Direct Access
> >> diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
> >> index d627734b0dc3..8849aae86394 100644
> >> --- a/docs/virt-v2v.pod
> >> +++ b/docs/virt-v2v.pod
> >> @@ -830,23 +830,23 @@ installed.  For some older Linux d

[Libguestfs] [PATCH] docs: Add extra permission needed for non-admin download with VMware 7

[Richard W.M. Jones]

From: Ming Xie 

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1817050
---
 docs/virt-v2v-input-vmware.pod | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
index ab2d28e41f..60cff93994 100644
--- a/docs/virt-v2v-input-vmware.pod
+++ b/docs/virt-v2v-input-vmware.pod
@@ -585,6 +585,7 @@ Enable (check) the following objects:
Provisioning:
  - Allow disk access
  - Allow read-only disk access
+ - Allow virtual machine download
  
  Cryptographic operations:
   - Decrypt
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH] git: Add 'git-publish' config file

[Richard W.M. Jones]

Thanks - I pushed both patches.

Now to see if I can remember how to update the website ...

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

Re: [Libguestfs] ovirt VM image to BM

[Richard W.M. Jones]

On Wed, May 18, 2022 at 01:56:36PM +0100, Richard W.M. Jones wrote:
> 
> On Wed, May 18, 2022 at 01:20:27PM +0200, Guilherme De Oliveira Santos wrote:
> > Greetings masters,
> >
> > I got pointed to you guys as experts on smtg I'm struggling with.
> > I'm trying to move an ovirt vm raw image to a bm machine and though
> [bm = baremetal]
> > I could do it successfully using dd (got able to see and work with
> > the partitions and the data fine), I couldn't boot the disk.
> >
> > It may be an issue with drivers (like the virtio drivers), but idk,
> > and I'm wondering if someone has done smtg similar or could help
> > somehow with the process.
> 
> So in IRC we established that it's a RHEL 8.6 guest, and that it
> doesn't get as far as grub.  The virtual machine boots with UEFI and
> the physical machine boots with BIOS which is why grub doesn't work.
> 
> Conversion of a machine from UEFI to BIOS is usually quite tricky.
> Are you sure the baremetal machine doesn't support UEFI booting?
> Almost anything made in the last 10 years should, although you may
> have to adjust the firmware ("BIOS") configuration.
> 
> After that it'll probably not have the right drivers in the initramfs.
> Symptoms would be that it gets to grub, then boots the kernel, but
> fails when mounting the root filesystem.  Luckily this is easier
> to solve - identify which drivers are needed for the target
> hardware and before copying across, run “dracut --add-drivers ..”
> You'll have to read the dracut man page closely.

Update is that after changing the hardware to use UEFI it booted all
the way (a little surprising?)

But the keyboard doesn't work.  This is also surprising because I
*thought* that both virt and baremetal basically use emulated or real
USB keyboard these days.

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] ovirt VM image to BM

[Richard W.M. Jones]

On Wed, May 18, 2022 at 01:20:27PM +0200, Guilherme De Oliveira Santos wrote:
> Greetings masters,
>
> I got pointed to you guys as experts on smtg I'm struggling with.
> I'm trying to move an ovirt vm raw image to a bm machine and though
[bm = baremetal]
> I could do it successfully using dd (got able to see and work with
> the partitions and the data fine), I couldn't boot the disk.
>
> It may be an issue with drivers (like the virtio drivers), but idk,
> and I'm wondering if someone has done smtg similar or could help
> somehow with the process.

So in IRC we established that it's a RHEL 8.6 guest, and that it
doesn't get as far as grub.  The virtual machine boots with UEFI and
the physical machine boots with BIOS which is why grub doesn't work.

Conversion of a machine from UEFI to BIOS is usually quite tricky.
Are you sure the baremetal machine doesn't support UEFI booting?
Almost anything made in the last 10 years should, although you may
have to adjust the firmware ("BIOS") configuration.

After that it'll probably not have the right drivers in the initramfs.
Symptoms would be that it gets to grub, then boots the kernel, but
fails when mounting the root filesystem.  Luckily this is easier
to solve - identify which drivers are needed for the target
hardware and before copying across, run “dracut --add-drivers ..”
You'll have to read the dracut man page closely.

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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [v2v PATCH 2/2] -i vmx -it ssh: document percent encoding in ssh URIs in more detail

[Richard W.M. Jones]

On Wed, May 18, 2022 at 08:49:30AM +0200, Laszlo Ersek wrote:
> On the vmfs file system, ESXi encodes guest name characters that it
> considers reserved with fairly unpredictable, proprietary rules. For
> example, the ESXi webgui forbids backslash characters (\) completely, a
> percent sign (%) is encoded as %25, and a dollar sign ($) is replaced with
> underscore (_). Therefore the user can only construct the pathname part of
> the ssh:// URI in two steps: (1) determine the precise absolute pathname
> of the VMX file by way of logging in to the ESXi server interactively, and
> *reading* (not guessing) whatever ESXi chose for naming directories and
> files, (2) given the absolute, server-local pathname, percent-encode the
> characters that are reserved in URIs, following the open standard(s) that
> cover this.
> 
> Document this procedure: reorder the paragraphs of the affected section so
> that they describe segments of the URI from the left to the right, and
> then elaborate on the pathname segment.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1938954
> Signed-off-by: Laszlo Ersek 
> ---
>  docs/virt-v2v-input-vmware.pod | 19 ---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
> index 4f4af2a9d804..38a0b30cd5fc 100644
> --- a/docs/virt-v2v-input-vmware.pod
> +++ b/docs/virt-v2v-input-vmware.pod
> @@ -167,14 +167,27 @@ C URI pointing to the VMX file.  A typical 
> URI looks like:
>  
>   
> ssh://r...@esxi.example.com/vmfs/volumes/datastore1/my%20guest/my%20guest.vmx
>  
> -Any space must be escaped with C<%20> and other non-ASCII characters
> -may also need to be URI-escaped.
> -
>  The username is not required if it is the same as your local username.
>  
>  You may optionally supply a port number after the hostname if the SSH
>  server is not listening on the default port (22).
>  
> +For determining the pathname component of the URI, log in to the ESXi
> +server via SSH interactively, and identify the absolute pathname of the
> +VMX file on the ESXi server, such as:
> +
> + /vmfs/volumes/datastore1/my guest/my guest.vmx
> +
> +Subsequently, on the virt-v2v command line, L +reserved
> +characters|https://en.wikipedia.org/wiki/Percent-encoding#Reserved_characters>
> +that you find in the individual pathname components.  For example, space
> +characters must be specified as C<%20>:
> +
> + /vmfs/volumes/datastore1/my%20guest/my%20guest.vmx
> +
> +Refer to L<https://bugzilla.redhat.com/1938954>.
> +
>  =head2 VMX: Importing a guest

Acked-by: Richard W.M. Jones 

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

Re: [Libguestfs] [v2v PATCH 1/2] docs/*.pod: strip trailing space characters

[Richard W.M. Jones]

On Wed, May 18, 2022 at 08:49:29AM +0200, Laszlo Ersek wrote:
> Remove any space characters that directly precede a newline character.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1938954
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> I've verified in the rendered HTMLs that this whitespace stripping does
> not break up indented blocks into smaller blocks -- the formatter keeps
> runs of indented lines coalesced into indented blocks.

This isn't what I expected at all, but I also checked the HTML and it
seems what you're saying is correct.  I wonder if this behaviour has
changed in POD ..?  For completeness I also looked at the roff output
and that groups everything into a single .Vb/.Ve section as well.

This is true at least as far back as RHEL 7
(perl-podlators-2.5.1-3.el7.noarch)

I'm also wondering if there's a way to express "separate but adjacent"
verbatim sections.  This led me to the sources where I found that the
rules for verbatim paragraphs are a lot more complicated than I
thought I knew.  Any paragraph that begins with a space is verbatim,
even if following lines are not indented, eg:

 This is
a verbatim paragraph

And the spec (perlpodspec(1)) says that adjacent verbatim paragraphs
must be run together.

I cannot find a way to do "separate but adjacent".  eg. this does not work:

=begin verbatim

 Para 1

=end verbatim

=begin verbatim

 Para 2

=end verbatim

It ends up as a single verbatim section.

>  docs/virt-v2v-hacking.pod  |  2 +-
>  docs/virt-v2v-input-vmware.pod | 10 
>  docs/virt-v2v.pod  | 24 ++--
>  3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/docs/virt-v2v-hacking.pod b/docs/virt-v2v-hacking.pod
> index 29b73fb6c3f8..da5e640d9c92 100644
> --- a/docs/virt-v2v-hacking.pod
> +++ b/docs/virt-v2v-hacking.pod
> @@ -1,6 +1,6 @@
>  =head1 NAME
>  
> -virt-v2v-hacking - 
> +virt-v2v-hacking -
>  
>  =head1 DESCRIPTION

(This man page really needs a complete rewrite.)

> diff --git a/docs/virt-v2v-input-vmware.pod b/docs/virt-v2v-input-vmware.pod
> index ab2d28e41ff2..4f4af2a9d804 100644
> --- a/docs/virt-v2v-input-vmware.pod
> +++ b/docs/virt-v2v-input-vmware.pod
> @@ -278,7 +278,7 @@ to list the guests on the server:
>  
>   $ virsh -c 'vpx://r...@vcenter.example.com/Datacenter/esxi' list --all
>   Enter root's password for vcenter.example.com: ***
> - 
> +
>IdName   State
>   
>- Fedora 20  shut off
> @@ -506,7 +506,7 @@ like this:
>  
>   $ virsh -c 'vpx://r...@vcenter.example.com/Datacenter/esxi' list --all
>   Enter root's password for vcenter.example.com: ***
> - 
> +
>IdName   State
>   
>- Fedora 20  shut off
> @@ -575,17 +575,17 @@ Enable (check) the following objects:
>   Datastore:
>- Browse datastore
>- Low level file operations
> - 
> +
>   Sessions:
>- Validate session
> - 
> +
>   Virtual Machine:
> Interaction:
>   - Guest operating system management by VIX API
> Provisioning:
>   - Allow disk access
>   - Allow read-only disk access
> - 
> +
>   Cryptographic operations:
>- Decrypt
>- Direct Access
> diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
> index d627734b0dc3..8849aae86394 100644
> --- a/docs/virt-v2v.pod
> +++ b/docs/virt-v2v.pod
> @@ -830,23 +830,23 @@ installed.  For some older Linux distributions, this 
> means installing
>  a kernel from the table below:
>  
>   RHEL 3 (Does not apply, as there was no Xen PV kernel)
> - 
> +
>   RHEL 4 i686 with > 10GB of RAM: install 'kernel-hugemem'
>  i686 SMP: install 'kernel-smp'
>  other i686: install 'kernel'
>  x86-64 SMP with > 8 CPUs: install 'kernel-largesmp'
>  x86-64 SMP: install 'kernel-smp'
>  other x86-64: install 'kernel'
> - 
> +
>   RHEL 5 i686: install 'kernel-PAE'
>  x86-64: install 'kernel'
> - 
> +
>   SLES 10i586 with > 10GB of RAM: install 'kernel-bigsmp'
>  i586 SMP: install 'kernel-smp'
>  other i586: install 'kernel-default'
>  x86-64 SMP: install 'kernel-smp'
>  other x86-64: install 'kernel-default'
> - 
> +
>   SLES 11+   i586: install 'kernel-pae'
>  x86-64: install 'kernel-default'
>  
> @@ -868,27 +868,27 @@ packages are installed I conversion, by 
> consulting the table
>  below.
>  
>   RHEL 3 No virtio drivers are available
> - 
> +
>   RHEL 4 kernel >= 2.5.9-89.EL
>  lvm2 >= 2.02.42-5.el4
>  device-mapper >= 1.02.28-2.el4
>  selinux-policy-targeted >= 1.17.30-2.152.el4
>  policycoreutils >= 1.18.1-4.13
> - 
> +
>   RHEL 5 kernel >= 

Re: [Libguestfs] [libguestfs PATCH] guestfs.pod: document encrypted RBD disk limitation

[Richard W.M. Jones]

On Wed, May 18, 2022 at 10:30:14AM +0200, Laszlo Ersek wrote:
> Under "REMOTE STORAGE", the "NETWORK BLOCK DEVICE" section already
> documents some limitations. Turns out we need to describe a quirky
> exception for accessing encrypted RBD disks, too.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2033247
> Signed-off-by: Laszlo Ersek 
> ---
>  lib/guestfs.pod | 33 
>  1 file changed, 33 insertions(+)
> 
> diff --git a/lib/guestfs.pod b/lib/guestfs.pod
> index b04c28d62750..1ad44e7c2878 100644
> --- a/lib/guestfs.pod
> +++ b/lib/guestfs.pod
> @@ -679,6 +679,39 @@ servers.  The server string is documented in
>  L. The C and C parameters are
>  also optional, and if not given, then no authentication will be used.
>  
> +An encrypted RBD disk -- I opening which would require the
> +C and C parameters -- cannot be accessed if the
> +following conditions all hold:
> +
> +=over 4
> +
> +=item *
> +
> +the L is libvirt,
> +
> +=item *
> +
> +the image specified by the C parameter is different from the
> +encrypted RBD disk,
> +
> +=item *
> +
> +the image specified by the C parameter has L +format|/COMMON VIRTUAL DISK IMAGE FORMATS>,
> +
> +=item *
> +
> +the encrypted RBD disk is specified as a backing file at some level in
> +the qcow2 backing chain.
> +
> +=back
> +
> +This limitation is due to libvirt's (justified) separate handling of
> +disks vs. secrets.  When the RBD username and secret are provided inside
> +a qcow2 backing file specification, libvirt does not construct an
> +ephemeral secret object from those, for Ceph authentication.  Refer to
> +L<https://bugzilla.redhat.com/2033247>.
> +
>  =head3 FTP, HTTP AND TFTP

Seems fine.  I had to look at perlpod(1) to check that the piped L<>
references were correct, but they seem to be!

Acked-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

Thanks - upstream in:

https://github.com/libguestfs/virt-v2v/commit/4ad661b545f476f8dfa00f3cdc28be01f6cc0510
https://github.com/libguestfs/virt-v2v/commit/e06cf5b5dcbfd3f0a798eaf75c8778502c9e3bc6

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

Re: [Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

On Tue, May 17, 2022 at 01:10:32PM +0200, Laszlo Ersek wrote:
> On 05/17/22 12:59, Richard W.M. Jones wrote:
> > These devices can be left around by grub2 when it runs the osprober
> > tool after we run “/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg”.
> > They are read-only mirrors of existing filesystems.  These confuse
> > later steps in conversion, specifically fstrim.
> > 
> > Reported-by: Ming Xie
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> > ---
> >  convert/linux_bootloaders.ml | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml
> > index a70b65a41c..7c5fb0be3f 100644
> > --- a/convert/linux_bootloaders.ml
> > +++ b/convert/linux_bootloaders.ml
> > @@ -345,7 +345,14 @@ object (self)
> >method remove_console = self#grub2_update_console ~remove:true
> >  
> >method update () =
> > -ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |])
> > +ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |]);
> > +
> > +(* Grub2 runs osprober which sometimes leaves around read-only
> > + * device-mapper maps covering existing filesystems.  These
> > + * confuse later steps (especially fstrim).  So just delete
> > + * any if found. (RHBZ#2003503).
> > + *)
> > +ignore (g#command [| "bash"; "-c"; "rm -f /dev/mapper/osprober-*" |])
> >  
> >method get_config_file () =
> >  grub_config
> > 

I'd like to make this change (on top):

-ignore (g#command [| "bash"; "-c"; "rm -f /dev/mapper/osprober-*" |])
+ignore (g#command [| "sh"; "-c"; "rm -f /dev/mapper/osprober-linux-*" |])

Bash might in theory not exist. I guess it's unlikely, but POSIX
defines 'sh' so we know that must exist.  It's not as if we're using
any bash features here.

The other part of the change is that the device nodes created by the
old & buggy osprober were always called "osprober-linux-*", so we may
as well be more specific about that.

I still cannot reproduce this bug locally.  I can force grub2-mkconfig
to run -- creating a guest like this is sufficient:

$ virt-builder rhel-8.4 --edit '/etc/sysconfig/grub: 
s|^GRUB_CMDLINE_LINUX="|GRUB_CMDLINE_LINUX="resume=/dev/sda1 |'

However it doesn't reproduce the bug observed.

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

Re: [Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

On Tue, May 17, 2022 at 11:59:26AM +0100, Richard W.M. Jones wrote:
> Still testing this one as well ...

The version (with rm) fixes the bug for me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH] daemon: In list_filesystems ignore /dev/mapper/osprober-* devices

[Richard W.M. Jones]

On Tue, May 17, 2022 at 11:20:32AM +0100, Richard W.M. Jones wrote:
> Still running virt-v2v to test this one ...

Whether or not we still want to go with this, I have now tested this
version and it works for me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

On Tue, May 17, 2022 at 01:10:32PM +0200, Laszlo Ersek wrote:
> On 05/17/22 12:59, Richard W.M. Jones wrote:
> > These devices can be left around by grub2 when it runs the osprober
> > tool after we run “/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg”.
> > They are read-only mirrors of existing filesystems.  These confuse
> > later steps in conversion, specifically fstrim.
> > 
> > Reported-by: Ming Xie
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> > ---
> >  convert/linux_bootloaders.ml | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml
> > index a70b65a41c..7c5fb0be3f 100644
> > --- a/convert/linux_bootloaders.ml
> > +++ b/convert/linux_bootloaders.ml
> > @@ -345,7 +345,14 @@ object (self)
> >method remove_console = self#grub2_update_console ~remove:true
> >  
> >method update () =
> > -ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |])
> > +ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |]);
> > +
> > +(* Grub2 runs osprober which sometimes leaves around read-only
> > + * device-mapper maps covering existing filesystems.  These
> > + * confuse later steps (especially fstrim).  So just delete
> > + * any if found. (RHBZ#2003503).
> > + *)
> > +ignore (g#command [| "bash"; "-c"; "rm -f /dev/mapper/osprober-*" |])
> >  
> >method get_config_file () =
> >  grub_config
> > 
> 
> Hmm... I'm not sure why this is better than "dmsetup remove", but it
> does not really matter. We need these nodes to disappear only for the
> remainder of the conversion -- so command that effects that suffices!
> 
> Reviewed-by: Laszlo Ersek 

Yeah, I'm not really sure what I was thinking there.  I certainly
*meant* to use dmsetup remove, but maybe ... it's accidentally better?

The problem with running dmsetup is that we have to run the guest
command, which might not exist.

Fun fact I found when investigating this.  Upstream os-prober dropped
the whole device mapper / dmsetup / osprober-linux-* stuff back in
2017.  The current code doesn't mention osprober-linux-* at all.
So I guess the version in this RHEL 8 guest is quite old.

Since I'm still testing the (above) version of this patch, let's see
if "rm" actually works before doing any further work on this.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit] s3: Only run the tests if boto3 is installed

[Richard W.M. Jones]

On Tue, May 17, 2022 at 12:50:53PM +0200, Laszlo Ersek wrote:
> On 05/17/22 12:11, Richard W.M. Jones wrote:
> > 
> > OK I see what's going on.
> > 
> > test-S3.sh uses the mocked boto3 in tests/test-S3/ to do an end-to-end
> > test (nbdcopy).
> > 
> > test-S3-unit.sh runs the unit tests within the plugin.  It's basically
> > testing the plugin as if it was a standalone Python script (without
> > nbdkit being involved).  This uses an internal MockS3Client class also
> > inside the plugin.
> > 
> > The second one does actually need real boto3, just because there's an
> > "import boto3" at the top and we don't set up PYTHONPATH to pick up
> > the tests/test-S3/ subdirectory.
> > 
> > We could either fix the second one by skipping it if (real) boto3
> > isn't installed, or by making it use the mocked boto3 from tests/test-S3/
> > 
> > Either approach would work, but the second one (below) seems maybe
> > better because we'd get more test coverage this way?
> 
> I think so, yes: from your description, "test-S3-unit.sh" seems "less
> demanding" than "test-S3.sh", so if the mocked module satisfies the
> latter, it should be good enough for the former too.
> 
> > 
> > Rich.
> > 
> > 
> > diff --git a/tests/test-S3-unit.sh b/tests/test-S3-unit.sh
> > index 6b6adf02..11718be3 100755
> > --- a/tests/test-S3-unit.sh
> > +++ b/tests/test-S3-unit.sh
> > @@ -44,6 +44,6 @@ if [ "$NBDKIT_VALGRIND" = "1" ]; then
> >  exit 77
> >  fi
> >  
> > -export PYTHONPATH=$srcdir/../plugins/S3:$PYTHONPATH
> > +export PYTHONPATH=$srcdir/../plugins/S3:$srcdir/test-S3:$PYTHONPATH
> >  
> >  $PYTHON -m unittest S3
> > 
> > 
> 
> For this hunk above:
> 
> Acked-by: Laszlo Ersek 

Thanks - committed as:

https://gitlab.com/nbdkit/nbdkit/-/commit/306f94a3c54a7a697e56978dee09d2f3816c2dc4

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

These devices can be left around by grub2 when it runs the osprober
tool after we run “/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg”.
They are read-only mirrors of existing filesystems.  These confuse
later steps in conversion, specifically fstrim.

Reported-by: Ming Xie
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
---
 convert/linux_bootloaders.ml | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml
index a70b65a41c..7c5fb0be3f 100644
--- a/convert/linux_bootloaders.ml
+++ b/convert/linux_bootloaders.ml
@@ -345,7 +345,14 @@ object (self)
   method remove_console = self#grub2_update_console ~remove:true
 
   method update () =
-ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |])
+ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |]);
+
+(* Grub2 runs osprober which sometimes leaves around read-only
+ * device-mapper maps covering existing filesystems.  These
+ * confuse later steps (especially fstrim).  So just delete
+ * any if found. (RHBZ#2003503).
+ *)
+ignore (g#command [| "bash"; "-c"; "rm -f /dev/mapper/osprober-*" |])
 
   method get_config_file () =
 grub_config
-- 
2.35.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

[Richard W.M. Jones]

Still testing this one as well ...

Rich.


___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs