[Libguestfs] [PATCH 0/2] tests/mount-local: fix relative pathname regression
Commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18)
missed updating a relative pathname in "tests/mount-local". This causes
the test case to hang. Furthermore, due to an earlier error handling
bug, the symptoms of the hang are somewhat chaotic. This small series
intends to fix both issues.
Thanks,
Laszlo
Laszlo Ersek (2):
tests/mount-local: exit child immediately when exec fails
tests/mount-local: fix relative pathname of FUSE client executable
tests/mount-local/test-parallel-mount-local.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 2/2] tests/mount-local: fix relative pathname of FUSE client executable
In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
the working directory relative to which "test-parallel-mount-local" would
be launched (by the test machinery) changed from "tests/mount-local" to
just "tests".
While the relative pathname of the "guestunmount" executable was updated
inside "test-parallel-mount-local" accordingly, the relative pathname of
the FUSE client ("test-parallel-mount-local" itself, just invoked with
"--test") was not. This issue guarantees that the exec call fails in the
child, and so the test case always hangs.
Because we had removed "mount-local" from the end of the working
directory, prepend it now to the relative pathname of the FUSE client
executable.
Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
Signed-off-by: Laszlo Ersek
---
tests/mount-local/test-parallel-mount-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index 5f00e328a39c..c33ecf5b2680 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -220,7 +220,7 @@ start_thread (void *statevp)
if (pid == 0) { /* child */
setpgid (0, 0); /* so we don't get ^C from parent */
- execlp ("./test-parallel-mount-local",
+ execlp ("mount-local/test-parallel-mount-local",
"test-parallel-mount-local", "--test", state->mp, NULL);
perror ("execlp");
_exit (EXIT_FAILURE);
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 1/2] tests/mount-local: exit child immediately when exec fails
Each worker thread of "test-parallel-mount-local" performs the following
steps (among others):
(1) it starts an appliance dedicated to that thread, using a private
scratch disk image,
(2) exports a dedicated FUSE mount point on the host, exposing the file
system on the appliance's disk,
(3) launches a child process for manipulating the particular FUSE mount
point on the host,
(4) enters a FUSE request processing loop, translating requests between
the host kernel (coming in via the FUSE mount point) and the
appliance.
Items to note:
- The child process from step (3) consists of a single thread of execution
(see fork() in POSIX): a duplicate of the parent process's respective
worker thread.
- The child process from step (3) blocks on any FUSE mount point access on
the host until the worker thread in the parent process starts processing
FUSE requests, in step (4).
- The FUSE request processing in step (4), in the worker thread living in
the parent process, terminates if and only if the child process unmounts
the FUSE mount point originating from (2).
Should the exec call in step (3) fail for any reason, the child currently
jumps to the "error" label. This is wrong: under the error label, we call
guestfs_close() on the appliance -- but the appliance is owned by the
parent process's worker thread, not the child. What happens is that the
child kills off the appliance while the parent's worker thread is in the
FUSE request processing loop (4).
The "error" label was never meant to be reached by the child process -- if
exec fails for any reason, exit the child immediately. The parent will
remain in the FUSE request processing loop (4) forever, but no state will
be corrupted. For example, using another (interactive) session on the
host, the FUSE mount points can be interacted with, and if all of them are
manually unmounted, the FUSE request processing (4) completes in every
worker thread.
This patch does not fix the primary issue with
"test-parallel-mount-local", but removes "chaos" from the symptoms. The
next patch will fix the actual regression in this test case.
Signed-off-by: Laszlo Ersek
---
tests/mount-local/test-parallel-mount-local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/mount-local/test-parallel-mount-local.c
b/tests/mount-local/test-parallel-mount-local.c
index d3db6914fc4c..5f00e328a39c 100644
--- a/tests/mount-local/test-parallel-mount-local.c
+++ b/tests/mount-local/test-parallel-mount-local.c
@@ -223,7 +223,7 @@ start_thread (void *statevp)
execlp ("./test-parallel-mount-local",
"test-parallel-mount-local", "--test", state->mp, NULL);
perror ("execlp");
- goto error;
+ _exit (EXIT_FAILURE);
}
/* Run the FUSE main loop. We don't really want to see libguestfs
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] fixes for building guestfs-tools and virt-v2v against a fresh libguestfs
Hi, I'll post two small patch sets as followups to this "higher order" cover letter. The goal is to build guestfs-tools and virt-v2v against a freshly built libguestfs (git checkout), using libguestfs's "run" script. The modifications affect two projects, libguestfs-common and guestfs-tools, hence the two (upcoming) patch series. libguestfs-common is tricky because it is consumed as a submodule by three git superprojects (libguestfs itself, then guestfs-tools and virt-v2v). Yet more trickily, the three superprojects don't all seem to consume libguestfs-common at the same submodule git commit, at the moment. (libguestfs and guestfs-tools consume the submodule at older commit 74bc5c5c5cb4, while virt-v2v consumes the submodule at current HEAD commit 6d26b6eac9da.) For verifying the libguestfs-common updates, I first attempted various git-pushes into the submodule checkouts of the superprojects. That proved super unwieldy, as it would require a whole lot of git massaging just to (incrementally) rebuild all three superprojects after each libguestfs-common update. Instead, in each superproject, I created a development branch, and as first commit on that branch, I removed the submodule altogether, and replaced it with a directory tree of symbolic links into my stand-alone libguestfs-common worktree -- refer to the "lndir" command. I didn't expect creating new files in libguestfs-common, so once established, the symlink set could be considered final. Furthermore, "lndir" was required for symlinking individual regular files: symlinking subdirectories from libguestfs-common into the superprojects' "common" directories doesn't work, as the "common" content refers back to the superproject, via relative pathnames such as "../blah". For such pathnames to work, the deep "common" directory structure actually needs to exist within each superproject, only the leaves (the regular files) can be replaced with symlinks. Thankfully, "lndir" implements just that. This effectively bumped libguestfs and guestfs-tools to libguestfs-common HEAD commit 6d26b6eac9da, as a basis for the needed libguestfs-common fixes. With the "libguestfs-common" patch set applied (on top of commit 6d26b6eac9da), libguestfs can be built and checked without regressions. Furthermore, virt-v2v too can be built and checked against the just-built libguestfs (using the latter's "run" script) without regressions. The same applies to "guestfs-tools", assuming the other patch set is applied to it (on top of commit 9ba463545fa0). Once these patch sets are up-stream, the three superprojects should advance their submodule references to the new libguestfs-common head commit. Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] lib: direct: Remove use of sga
On 09/08/21 17:42, Richard W.M. Jones wrote:
> sga (or "sgabios" or "Serial Graphics Adapter") is an option ROM for
> seabios which directs output to the serial adapter. This is very
> useful for debugging BIOS problems during boot.
>
> RHEL wants to deprecate this feature (in fact, they just deprecated it
> without telling us). However there is an equivalent feature now in
> seabios which can be enabled using either -nographic or
> -machine graphics=off
(
That's from SeaBIOS commit 0ebc29f9c4db ("paravirt: serial console
configuration.", 2017-09-22), as far as I can tell. The containing
series is:
1 44270bc1d285 std: add cp437 to unicode map
2 90fa51152714 kbd: make enqueue_key public, add ascii_to_keycode
3 1bda724cc567 romfile: add support for constant files.
4 0ebc29f9c4db paravirt: serial console configuration.
5 d6728f301d7e add serial console support
Part of "rel-1.11.0".
)
>
> This commit removes sga and enables -machine graphics=off in the
> direct backend.
>
> (We cannot do the same for the libvirt backend because libvirt has no
> feature to implement this yet).
> ---
> lib/launch-direct.c | 19 +++
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/lib/launch-direct.c b/lib/launch-direct.c
> index 972e77e13..e5b9a5611 100644
> --- a/lib/launch-direct.c
> +++ b/lib/launch-direct.c
> @@ -544,6 +544,13 @@ launch_direct (guestfs_h *g, void *datav, const char
> *arg)
>append_list ("gic-version=host");
> #endif
> append_list_format ("accel=%s", accel_val);
> +#if defined(__i386__) || defined(__x86_64__)
> +/* Tell seabios to send debug messages to the serial port.
> + * This used to be done by sgabios.
> + */
> +if (g->verbose)
> + append_list ("graphics=off");
> +#endif
>} end_list ();
>
>cpu_model = guestfs_int_get_cpu_model (has_kvm && !force_tcg);
> @@ -665,18 +672,6 @@ launch_direct (guestfs_h *g, void *datav, const char
> *arg)
>} end_list ();
> #endif
>
> - if (g->verbose &&
> - guestfs_int_qemu_supports_device (g, data->qemu_data,
> -"Serial Graphics Adapter")) {
> -/* Use sgabios instead of vgabios. This means we'll see BIOS
> - * messages on the serial port, and also works around this bug
> - * in qemu 1.1.0:
> - * https://bugs.launchpad.net/qemu/+bug/1021649
> - * QEmu has included sgabios upstream since just before 1.0.
> - */
> -arg ("-device", "sga");
> - }
> -
>/* Set up virtio-serial for the communications channel. */
>start_list ("-chardev") {
> append_list ("socket");
>
So, I'm asking this question mainly for my own education:
How does libguestfs deal with different QEMU versions / QEMU feature
deprecation? Is there some kind of feature detection?
For example, why would it be less good to implement the change as follows:
if (g->verbose) {
if (guestfs_int_qemu_supports_device(...) {
arg ("-device", "sga");
} else {
#if defined(__i386__) || defined(__x86_64__)
arg ("-machine", "graphics=off");
#endif
}
This would continue working with SeaBIOS preceding "rel-1.11.0", if at
the same time, the QEMU board had SGA support.
But: do we care? (That's what I'd like to learn about libguestfs,
primarily.)
The patch looks good to me, otherwise.
Reviewed-by: Laszlo Ersek
Thanks
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [hivex PATCH 0/8] add a pkg-config descriptor for the local (not installed) build tree
On 09/08/21 15:27, Laszlo Ersek wrote: > This series introduces the "lib/local/hivex.pc" pkg-config file to > hivex, permitting, through the "run" script, other programs and > libraries to be built against the just-built (not installed) hivex tree. > A few small details in the hivex build machinery that I understood to be > warts are cleaned up, and parts of the libguestfs tree *layout* are > adopted. > > Thanks, > Laszlo > > Laszlo Ersek (8): > build: do not look for headers in "$(top_builddir)/lib" > build: expose public library header "hivex.h" without "lib" contents > build: remove "hivex.pc" from EXTRA_DIST > build: move "hivex.pc.in" to the "lib" subdirectory > run: use 'prepend' function to build paths > build: allow C programs using hivex to be compiled against build dir > build: link hivex statically into C programs compiled against build > dir > build: allow OCaml programs using hivex to be compiled against build > dir > > configure.ac | 4 ++- > Makefile.am| 8 ++--- > images/Makefile.am | 2 +- > include/Makefile.am| 20 + > lib/Makefile.am| 16 +++--- > ocaml/Makefile.am | 21 -- > perl/Makefile.PL.in| 2 +- > python/Makefile.am | 2 +- > ruby/Rakefile.in | 2 +- > sh/Makefile.am | 3 +- > xml/Makefile.am| 1 + > .gitignore | 6 ++-- > generator/generator.ml | 2 +- > hivex.pc.in => lib/hivex.pc.in | 0 > lib/local/hivex.pc.in | 35 ++ > run.in | 53 +- > 16 files changed, 129 insertions(+), 48 deletions(-) > create mode 100644 include/Makefile.am > rename hivex.pc.in => lib/hivex.pc.in (100%) > create mode 100644 lib/local/hivex.pc.in > Merged as commit range ace0be49b529..34c6d42a9b0e. Thanks Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [hivex PATCH 7/8] build: link hivex statically into C programs compiled against build dir
On 09/08/21 17:48, Richard W.M. Jones wrote:
> On Wed, Sep 08, 2021 at 03:27:57PM +0200, Laszlo Ersek wrote:
>> This static-only linking allows the libguestfs daemon ("guestfsd") to
>> launch in the appliance, without unresolved hivex symbols, when libguestfs
>> is built against the hivex build dir. (Libguestfs's automatic *package*
>> collection for the appliance, based on the shared library requirements of
>> "guestfsd", cannot cover hivex when hivex is provided by a local build
>> directory.)
>>
>> Linking hivex statically into *host-side* C programs that are compiled
>> against the hivex build dir is only a small penalty.
>>
>> Signed-off-by: Laszlo Ersek
>> ---
>> lib/local/hivex.pc.in | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/local/hivex.pc.in b/lib/local/hivex.pc.in
>> index d899cb01a282..6f0b5bb44237 100644
>> --- a/lib/local/hivex.pc.in
>> +++ b/lib/local/hivex.pc.in
>> @@ -32,4 +32,4 @@ Version: @VERSION@
>> Description: Read and write Windows Registry Hive files.
>> Requires:
>> Cflags: -I${includedir}
>> -Libs: -L${libdir} -lhivex
>> +Libs: -L${libdir} -l:libhivex.a
>
> Worth a comment here in the .pc.in file? I was previously not aware
> this magic was possible.
How about squashing this:
> diff --git a/lib/local/hivex.pc.in b/lib/local/hivex.pc.in
> index 6f0b5bb44237..941638939fa3 100644
> --- a/lib/local/hivex.pc.in
> +++ b/lib/local/hivex.pc.in
> @@ -32,4 +32,8 @@ Version: @VERSION@
> Description: Read and write Windows Registry Hive files.
> Requires:
> Cflags: -I${includedir}
> +# The colon notation forces an exact filename search when linking; here
> +# effectively disabling shared library lookup. (Refer to "--library" in
> ld(1).)
> +# Statically linking hivex matters mainly for "guestfsd", which runs in the
> +# appliance.
> Libs: -L${libdir} -l:libhivex.a
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [hivex PATCH 8/8] build: allow OCaml programs using hivex to be compiled against build dir
On 09/08/21 17:49, Richard W.M. Jones wrote: > > Yes the series looks fine, ACK series. Thanks! > I think the presence of $(top_builddir)/lib to look for the header > file was likely a copy-paste thing from some other project. The > generator always writes files into the source directory (or if you > build from the tarball, everything is already in the source directory.) Right, I'd not been familiar with autotools internals, so I first needed to understand that the generator ran during autogen, and (therefore) its output would be included by "make dist" in the *source* tree. Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [hivex PATCH 8/8] build: allow OCaml programs using hivex to be compiled against build dir
On 09/09/21 13:08, Richard W.M. Jones wrote: > On Thu, Sep 09, 2021 at 12:42:45PM +0200, Laszlo Ersek wrote: >> On 09/08/21 17:49, Richard W.M. Jones wrote: >>> >>> Yes the series looks fine, ACK series. >> >> Thanks! >> >>> I think the presence of $(top_builddir)/lib to look for the header >>> file was likely a copy-paste thing from some other project. The >>> generator always writes files into the source directory (or if you >>> build from the tarball, everything is already in the source directory.) >> >> Right, I'd not been familiar with autotools internals, so I first needed >> to understand that the generator ran during autogen, and (therefore) its >> output would be included by "make dist" in the *source* tree. > > It's because we want to allow people to build from tarballs without > needing the ocamlc dependency. Ah, good point! I didn't realize there was a specific incentive. > It may not be necessary any longer, but it's what we're doing now. Oh I didn't mean it as criticism at all -- it makes perfect sense to me, it was just something new I had to learn :) Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] lib: direct: Remove use of sga
On 09/09/21 13:35, Richard W.M. Jones wrote:
> On Thu, Sep 09, 2021 at 01:11:09PM +0200, Laszlo Ersek wrote:
>> How does libguestfs deal with different QEMU versions / QEMU feature
>> deprecation? Is there some kind of feature detection?
>
> Kind of, but it's only grepping the output of -help, -device \?,
> and/or looking at QMP output of "query-qmp-schema" and "query-kvm".
>
> The code is in:
>
> https://github.com/libguestfs/libguestfs/blob/master/lib/qemu.c
>
> As qemu has settled down over time and libguestfs uses only a
> well-established set of features, this kind of feature detection has
> become less and less interesting. (So quite unlike libvirt where
> they're always trying to catch up with the latest qemu features). At
> some point we might just say "use qemu >= VERSION" and have done with
> it.
OK!
>
>> For example, why would it be less good to implement the change as
>> follows:
>>
>> if (g->verbose) {
>> if (guestfs_int_qemu_supports_device(...) {
>> arg ("-device", "sga");
>> } else {
>> #if defined(__i386__) || defined(__x86_64__)
>> arg ("-machine", "graphics=off");
>> #endif
>> }
>
> I think in this case it's my understanding that the "new" (from RHEL
> 7!)
Ah, OK! I didn't check.
> seabios graphics feature is just better than SGA,
Agreed.
> and so we might as well use it all the time even if SGA is available.
> That was my understanding from talking to Gerd anyhow.
So yes, my R-b stands. Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libguestfs PATCH 0/2] generalize ocaml-hivex[-devel] lookup
On 09/08/21 15:35, Laszlo Ersek wrote: > "daemon/Makefile.am" needs some tweaks for finding such OCaml bindings > for hivex that are not installed system-wide. > > Thanks, > Laszlo > > Laszlo Ersek (2): > daemon: generalize ocaml-hivex[-devel] lookup > daemon_utils_tests: generalize ocaml-hivex[-devel] lookup > > daemon/Makefile.am | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > Merged as commit range ceb034c92cbb..523b0180d8cf. Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [guestfs-tools PATCH 3/3] Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml programs
On 09/06/21 16:40, Richard W.M. Jones wrote: > > Yes this all seems like obvious stuff. ACK both series + the update > commits to bring the common submodules up to date. Thank you for the quick review! - libguestfs-common: commit range 6d26b6eac9da..19302b64c5f1 - guestfs-tools: commit range 9ba463545fa0..9b0f2e2d5893 - libguestfs:commit 81ad309d47e2 - virt-v2v: commit 10af3dee6a45 Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs-common PATCH 1/2] Makefile.am: supply missing $(LIBGUESTFS_CFLAGS)
C source code in the "edit", "mlvisit", "parallel", "progress", "structs", "visit", and "windows" modules includes "guestfs.h", but the makefiles of those modules do not add $(LIBGUESTFS_CFLAGS) to the respective module CFLAGS macros. This is a problem when these modules are built as a part of guestfs-tools, against a just-built libguestfs tree, as follows: > guestfs-tools$ ../libguestfs/run ./configure > guestfs-tools$ ../libguestfs/run make Example error: > CC libmlvisit_a-visit-c.o > visit-c.c:33:10: fatal error: guestfs.h: No such file or directory >33 | #include "guestfs.h" > | ^~~ Add the missing $(LIBGUESTFS_CFLAGS) instances. Signed-off-by: Laszlo Ersek --- edit/Makefile.am | 3 ++- mlvisit/Makefile.am | 1 + parallel/Makefile.am | 1 + progress/Makefile.am | 1 + structs/Makefile.am | 1 + visit/Makefile.am| 1 + windows/Makefile.am | 3 ++- 7 files changed, 9 insertions(+), 2 deletions(-) diff --git a/edit/Makefile.am b/edit/Makefile.am index e45c9bacead8..dff8b8e8c709 100644 --- a/edit/Makefile.am +++ b/edit/Makefile.am @@ -30,6 +30,7 @@ libedit_la_CPPFLAGS = \ -I$(top_srcdir)/lib -I$(top_builddir)/lib \ $(INCLUDE_DIRECTORY) libedit_la_CFLAGS = \ - $(WARN_CFLAGS) $(WERROR_CFLAGS) + $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) libedit_la_LIBADD = \ $(top_builddir)/common/utils/libutils.la diff --git a/mlvisit/Makefile.am b/mlvisit/Makefile.am index a727f4fe0459..64108ebfbbc7 100644 --- a/mlvisit/Makefile.am +++ b/mlvisit/Makefile.am @@ -60,6 +60,7 @@ libmlvisit_a_CPPFLAGS = \ -I$(top_srcdir)/common/visit libmlvisit_a_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(LIBVIRT_CFLAGS) $(LIBXML2_CFLAGS) \ -fPIC diff --git a/parallel/Makefile.am b/parallel/Makefile.am index 73d31494f4ee..8cf35424c1ef 100644 --- a/parallel/Makefile.am +++ b/parallel/Makefile.am @@ -39,6 +39,7 @@ libparallel_la_CPPFLAGS = \ libparallel_la_CFLAGS = \ -pthread \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(LIBXML2_CFLAGS) \ $(LIBVIRT_CFLAGS) libparallel_la_LIBADD = \ diff --git a/progress/Makefile.am b/progress/Makefile.am index 1a5d11996483..beea95a5e655 100644 --- a/progress/Makefile.am +++ b/progress/Makefile.am @@ -31,6 +31,7 @@ libprogress_la_CPPFLAGS = \ $(INCLUDE_DIRECTORY) libprogress_la_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(LIBTINFO_CFLAGS) libprogress_la_LIBADD = \ $(top_builddir)/common/utils/libutils.la \ diff --git a/structs/Makefile.am b/structs/Makefile.am index 32fd91c36bc8..5881a1c9bd32 100644 --- a/structs/Makefile.am +++ b/structs/Makefile.am @@ -40,4 +40,5 @@ libstructs_la_CPPFLAGS = \ $(INCLUDE_DIRECTORY) libstructs_la_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(GCC_VISIBILITY_HIDDEN) diff --git a/visit/Makefile.am b/visit/Makefile.am index e57cdfa5be86..7a4be2978e60 100644 --- a/visit/Makefile.am +++ b/visit/Makefile.am @@ -32,4 +32,5 @@ libvisit_la_CPPFLAGS = \ -I$(top_srcdir)/common/structs -I$(top_builddir)/common/structs libvisit_la_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(GCC_VISIBILITY_HIDDEN) diff --git a/windows/Makefile.am b/windows/Makefile.am index 3f38ca67ddfb..7bdb7b862162 100644 --- a/windows/Makefile.am +++ b/windows/Makefile.am @@ -31,7 +31,8 @@ libwindows_la_CPPFLAGS = \ -I$(top_srcdir)/lib -I$(top_builddir)/lib \ $(INCLUDE_DIRECTORY) libwindows_la_CFLAGS = \ - $(WARN_CFLAGS) $(WERROR_CFLAGS) + $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) libwindows_la_LIBADD = \ $(top_builddir)/common/utils/libutils.la \ $(LTLIBINTL) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs-common PATCH 2/2] Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml test programs
Otherwise > guestfs-tools$ ../libguestfs/run make check produces errors like > GEN tools_utils_tests > /usr/bin/ld: cannot find -lguestfs > /usr/bin/ld: cannot find -lguestfs > collect2: error: ld returned 1 exit status > File "caml_startup", line 1: > Error: Error during linking (exit code 1) in the "mltools" and "mlvisit" modules. Signed-off-by: Laszlo Ersek --- mltools/Makefile.am | 24 ++-- mlvisit/Makefile.am | 9 - 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/mltools/Makefile.am b/mltools/Makefile.am index 5cec178e4a78..26e466835732 100644 --- a/mltools/Makefile.am +++ b/mltools/Makefile.am @@ -262,9 +262,15 @@ tools_utils_tests_DEPENDENCIES = \ ../mlpcre/mlpcre.$(MLARCHIVE) \ $(MLTOOLS_CMA) \ $(top_srcdir)/ocaml-link.sh +tools_utils_tests_OCAMLCLIBS = \ + -pthread -lpthread \ + -lutils \ + $(LIBXML2_LIBS) \ + -lgnu \ + $(LIBGUESTFS_LIBS) tools_utils_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread -lutils $(LIBXML2_LIBS) -lgnu' -- \ + -cclib '$(tools_utils_tests_OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(tools_utils_tests_THEOBJECTS) -o $@ @@ -276,9 +282,15 @@ getopt_tests_DEPENDENCIES = \ ../mlpcre/mlpcre.$(MLARCHIVE) \ $(MLTOOLS_CMA) \ $(top_srcdir)/ocaml-link.sh +getopt_tests_OCAMLCLIBS = \ + -pthread -lpthread \ + -lutils \ + $(LIBXML2_LIBS) \ + -lgnu \ + $(LIBGUESTFS_LIBS) getopt_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread -lutils $(LIBXML2_LIBS) -lgnu' -- \ + -cclib '$(getopt_tests_OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(getopt_tests_THEOBJECTS) -o $@ @@ -292,7 +304,7 @@ JSON_tests_DEPENDENCIES = \ $(top_srcdir)/ocaml-link.sh JSON_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread' -- \ + -cclib '-pthread -lpthread $(LIBGUESTFS_LIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(JSON_tests_THEOBJECTS) -o $@ @@ -306,7 +318,7 @@ JSON_parser_tests_DEPENDENCIES = \ $(top_srcdir)/ocaml-link.sh JSON_parser_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread $(OCAMLCLIBS)' -- \ + -cclib '-pthread -lpthread $(OCAMLCLIBS) $(LIBGUESTFS_LIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(JSON_parser_tests_THEOBJECTS) -o $@ @@ -320,7 +332,7 @@ machine_readable_tests_DEPENDENCIES = \ $(top_srcdir)/ocaml-link.sh machine_readable_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread -lutils -lgnu' -- \ + -cclib '-pthread -lpthread -lutils -lgnu $(LIBGUESTFS_LIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(machine_readable_tests_THEOBJECTS) -o $@ @@ -334,7 +346,7 @@ tools_messages_tests_DEPENDENCIES = \ $(top_srcdir)/ocaml-link.sh tools_messages_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread -lutils -lgnu' -- \ + -cclib '-pthread -lpthread -lutils -lgnu $(LIBGUESTFS_LIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(tools_messages_tests_THEOBJECTS) -o $@ diff --git a/mlvisit/Makefile.am b/mlvisit/Makefile.am index 64108ebfbbc7..beff0bc64cdc 100644 --- a/mlvisit/Makefile.am +++ b/mlvisit/Makefile.am @@ -115,9 +115,16 @@ visit_tests_DEPENDENCIES = \ ../mlutils/mlcutils.$(MLARCHIVE) \ $(MLVISIT_CMA) \ $(top_srcdir)/ocaml-link.sh +visit_tests_OCAMLCLIBS = \ + -pthread -lpthread \ + -lvisit \ + -lstructs \ + -lutils \ + $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) visit_tests_LINK = \ $(top_srcdir)/ocaml-link.sh \ - -cclib '-pthread -lpthread -lvisit -lstructs -lutils $(LIBXML2_LIBS)' -- \ + -cclib '$(visit_tests_OCAMLCLIBS)' -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(visit_tests_THEOBJECTS) -o $@ -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs-common PATCH 0/2] build guestfs-tools and virt-v2v against locally built libguestfs
This series lets (a) virt-v2v, and (b) in combination with the sibling series for guestfs-tools, guestfs-tools, be built against a just-built libguestfs (e.g. with the libguest-devel package absent), using the commands $ git submodule update --init --force $ autoreconf -i $ ../libguestfs/run ./configure CFLAGS=-fPIC $ ../libguestfs/run make -j $(getconf _NPROCESSORS_ONLN) $ ../libguestfs/run make -j $(getconf _NPROCESSORS_ONLN) check Thanks, Laszlo Laszlo Ersek (2): Makefile.am: supply missing $(LIBGUESTFS_CFLAGS) Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml test programs edit/Makefile.am | 3 ++- mltools/Makefile.am | 24 ++-- mlvisit/Makefile.am | 10 +- parallel/Makefile.am | 1 + progress/Makefile.am | 1 + structs/Makefile.am | 1 + visit/Makefile.am| 1 + windows/Makefile.am | 3 ++- 8 files changed, 35 insertions(+), 9 deletions(-) base-commit: 6d26b6eac9dab6bc8fddf9f6564a13f39c51defe -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 0/3] build against locally built libguestfs
This series (in combination with the sibling series for libguestfs-common) allows guestfs-tools to be built against a just-built libguestfs (e.g. with the libguest-devel package absent), using the commands $ git submodule update --init --force $ autoreconf -i $ ../libguestfs/run ./configure CFLAGS=-fPIC $ ../libguestfs/run make -j $(getconf _NPROCESSORS_ONLN) $ ../libguestfs/run make -j $(getconf _NPROCESSORS_ONLN) check Thanks, Laszlo Laszlo Ersek (3): Makefile.am: fix $(LIBGUESTFS_CFLAGS) typo for virt-ls Makefile.am: supply missing $(LIBGUESTFS_CFLAGS) Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml programs builder/Makefile.am| 5 - cat/Makefile.am| 2 +- customize/Makefile.am | 2 ++ dib/Makefile.am| 1 + get-kernel/Makefile.am | 1 + resize/Makefile.am | 1 + sparsify/Makefile.am | 1 + sysprep/Makefile.am| 1 + 8 files changed, 12 insertions(+), 2 deletions(-) base-commit: 9ba463545fa017910a5077434f87ae67e146c9e0 -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 1/3] Makefile.am: fix $(LIBGUESTFS_CFLAGS) typo for virt-ls
Commit 4354a3126152 ("Add build system.", 2021-03-11) added
$(LIBGUESTFS_LIBS) to "virt_ls_CFLAGS" and "virt_ls_LDADD" both.
Correct the former; it should carry $(LIBGUESTFS_CFLAGS). Currently, the
command
> $ ../libguestfs/run make
produces the error
> CC virt_ls-ls.o
> ls.c:47:10: fatal error: guestfs.h: No such file or directory
>47 | #include "guestfs.h"
> | ^~~
Fixes: 4354a3126152a2748cc9097cba139b3908ccc342
Signed-off-by: Laszlo Ersek
---
cat/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cat/Makefile.am b/cat/Makefile.am
index 39d0d25576eb..f1de5c866987 100644
--- a/cat/Makefile.am
+++ b/cat/Makefile.am
@@ -138,9 +138,9 @@ virt_ls_CPPFLAGS = \
virt_ls_CFLAGS = \
$(WARN_CFLAGS) $(WERROR_CFLAGS) \
$(LIBXML2_CFLAGS) \
- $(LIBGUESTFS_LIBS)
+ $(LIBGUESTFS_CFLAGS)
virt_ls_LDADD = \
$(top_builddir)/common/options/liboptions.la \
$(top_builddir)/common/visit/libvisit.la \
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 2/3] Makefile.am: supply missing $(LIBGUESTFS_CFLAGS)
Common C source code pulled into virt-builder, virt-index-validate and virt-customize #includes "guestfs.h", but the guestfs-tools makefiles of those modules do not add $(LIBGUESTFS_CFLAGS) to the respective module CFLAGS macros. The command > $ ../libguestfs/run make produces errors such as > CC ../common/edit/libcustomize_a-file-edit.o > In file included from ../common/edit/file-edit.c:46: > ../common/edit/file-edit.h:22:10: fatal error: guestfs.h: No such file or > directory >22 | #include > | ^~~ Add the missing $(LIBGUESTFS_CFLAGS) instances. Signed-off-by: Laszlo Ersek --- builder/Makefile.am | 4 +++- customize/Makefile.am | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/builder/Makefile.am b/builder/Makefile.am index f49a3b0aaa12..9c155c7f29ef 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -148,8 +148,9 @@ virt_builder_CPPFLAGS = \ virt_builder_CFLAGS = \ -pthread \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ -Wno-unused-macros \ + $(LIBGUESTFS_CFLAGS) \ $(LIBLZMA_CFLAGS) \ $(LIBTINFO_CFLAGS) \ $(LIBXML2_CFLAGS) @@ -446,9 +447,10 @@ virt_index_validate_CPPFLAGS = \ -I$(top_srcdir)/lib \ -I$(top_srcdir)/include virt_index_validate_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ - -Wno-unused-macros + -Wno-unused-macros \ + $(LIBGUESTFS_CFLAGS) virt_index_validate_LDADD = \ $(LTLIBINTL) \ ../gnulib/lib/libgnu.la diff --git a/customize/Makefile.am b/customize/Makefile.am index d3f07d723c4a..7bf6af444b67 100644 --- a/customize/Makefile.am +++ b/customize/Makefile.am @@ -102,8 +102,9 @@ libcustomize_a_CPPFLAGS = \ -I$(top_srcdir)/include \ -I$(top_srcdir)/common/edit libcustomize_a_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBGUESTFS_CFLAGS) \ $(LIBCRYPT_CFLAGS) \ $(LIBVIRT_CFLAGS) \ $(LIBXML2_CFLAGS) \ -fPIC -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 3/3] Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml programs
Otherwise the command > $ ../libguestfs/run make produces errors like > GEN virt-customize > /usr/bin/ld: cannot find -lguestfs > /usr/bin/ld: cannot find -lguestfs > collect2: error: ld returned 1 exit status > File "caml_startup", line 1: > Error: Error during linking (exit code 1) in the "builder", "customize", "dib", "get-kernel", "resize", "sparsify" and "sysprep" modules. Signed-off-by: Laszlo Ersek --- builder/Makefile.am| 1 + customize/Makefile.am | 1 + dib/Makefile.am| 1 + get-kernel/Makefile.am | 1 + resize/Makefile.am | 1 + sparsify/Makefile.am | 1 + sysprep/Makefile.am| 1 + 7 files changed, 7 insertions(+) diff --git a/builder/Makefile.am b/builder/Makefile.am index 9c155c7f29ef..78274426aec5 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -200,8 +200,9 @@ OCAMLCLIBS = \ -pthread -lpthread \ -lutils \ $(LIBTINFO_LIBS) \ $(LIBCRYPT_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBLZMA_LIBS) \ $(LIBXML2_LIBS) \ $(JANSSON_LIBS) \ $(LIBINTL) \ diff --git a/customize/Makefile.am b/customize/Makefile.am index 7bf6af444b67..6bdd5fae91e6 100644 --- a/customize/Makefile.am +++ b/customize/Makefile.am @@ -168,8 +168,9 @@ OCAMLLINKFLAGS = \ OCAMLCLIBS = \ -pthread -lpthread \ -lutils \ $(LIBTINFO_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBCRYPT_LIBS) \ $(LIBVIRT_LIBS) \ $(LIBXML2_LIBS) \ $(LIBINTL) \ diff --git a/dib/Makefile.am b/dib/Makefile.am index f607449f249a..7581feb787ec 100644 --- a/dib/Makefile.am +++ b/dib/Makefile.am @@ -91,8 +91,9 @@ endif OCAMLCLIBS = \ -pthread -lpthread \ -lutils \ $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBINTL) \ -lgnu OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' diff --git a/get-kernel/Makefile.am b/get-kernel/Makefile.am index e0df00ec06a7..c81cb8ba8cb4 100644 --- a/get-kernel/Makefile.am +++ b/get-kernel/Makefile.am @@ -73,8 +73,9 @@ endif OCAMLCLIBS = \ -pthread -lpthread \ -lutils \ $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBINTL) \ -lgnu OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' diff --git a/resize/Makefile.am b/resize/Makefile.am index 606fcecb6ebc..f8aa4c46bfcf 100644 --- a/resize/Makefile.am +++ b/resize/Makefile.am @@ -73,8 +73,9 @@ OCAMLCLIBS = \ -lprogress \ -lutils \ $(LIBTINFO_LIBS) \ $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBINTL) \ -lgnu OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am index d5c8cd3367ae..0eca849da46b 100644 --- a/sparsify/Makefile.am +++ b/sparsify/Makefile.am @@ -80,8 +80,9 @@ OCAMLCLIBS = \ -lprogress \ -lutils \ $(LIBTINFO_LIBS) \ $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBINTL) \ -lgnu OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index d32ab20e57e9..e213244bc17a 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -131,8 +131,9 @@ OCAMLCLIBS = \ -lutils \ $(LIBTINFO_LIBS) \ $(LIBCRYPT_LIBS) \ $(LIBXML2_LIBS) \ + $(LIBGUESTFS_LIBS) \ $(LIBINTL) \ -lgnu OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)' -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] fixes for building libguestfs against a fresh hivex
Hi, the hivex tree provides a "run" script in its root directory, but it does not suffice for building / checking libguestfs, like this: libguestfs$ autoreconf -i libguestfs$ ../hivex/run ./configure CFLAGS=-fPIC libguestfs$ ../hivex/run make -j4 libguestfs$ ../hivex/run make -j4 check I'll post two small patch sets in-reply-to this cover letter, one for hivex and another for libguestfs, to enable the above usage. I regression-tested the patch sets in the following scenarios: - build & check libguestfs against the system-wide installed hivex packages - build & check guestfs-tools, and virt-v2v, against a libguestfs that was locally built against either a local hivex or a system-wide hivex. - did a bunch of "make dist"-based tests as well, such as out-of-source-tree builds and "make check"s of tar.gz-provided source trees. There were two major head-aches: - "daemon_utils_tests" is linked with a manually written LINK target that assumes hivex is in a system-wide location. It was difficult to recognize that the way to pass $(HIVEX_LIBS) to it was to extend the -cclib switch. (The OCaml documentation I managed to find wasn't stellar.) - A problem more at the design level: guestfsd links against hivex, but runs in the appliance. The daemon Makefile deals with this by translating the shared object dependencies of the executable to package (RPM) names, and then installing those RPMs in the appliance. This does not (and cannot) work when an RPM simply does not *exist* that could provide the locally-built hivex inside the appliance. The approach I chose was to link hivex statically into guestfsd, not dynamically. With that, a new set of challenges applied, as it is apparently autotools's mission to prevent statically linking *just one library* into an otherwise dynamic executable. I've read what there's to read about this on stackoverflow and elsewhere; automake *rejects* -Wl,-Bstatic / -Wl,-Bdynamic tricks in guestfsd_LDADD. Automake suggests guestfsd_LDFLAGS, but that is useless, as these flags are position-dependent, and they should only bracket $(HIVEX_LIBS) in guestfsd_LDADD. In the end, it wouldn't be right to modify guestfsd_LDADD anyway, as we don't *unconditionally* want hivex to be linked statically into guestfsd. So the next (and successful) idea was to modify the (new) "lib/local/hivex.pc" pkg-config descriptor in hivex with one more step: replace "-lhivex" with "-l:libhivex.a" on the "Libs:" line. This gets picked up in libguestfs's HIVEX_LIBS, and causes the linker to look specifically for "libhivex.a", not "libhivex.so". (Refer to the "-l" flags documentation in ld(1) -- I found the trick somewhere on stackoverflow.) A side effect is that *all* binaries built with "../hivex/run" will then get a static copy of the hivex library -- but that's a small price to pay. After all, such builds -- against a local (and not installed) hivex tree -- are always development builds. I've verified that the guestfsd executable still depends on the hivex *shared* object (and so the system-wide RPM) when "../hivex/run" is not in use. Thanks, Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 3/8] build: remove "hivex.pc" from EXTRA_DIST
The "configure" script creates "hivex.pc" in the build directory from "hivex.pc.in"; there's no need to package "hivex.pc" in a tarball distribution (created by "make dist"). Remove "hivex.pc" from EXTRA_DIST. Signed-off-by: Laszlo Ersek --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index d217ea52fcb2..f69d8cfb8e67 100644 --- a/Makefile.am +++ b/Makefile.am @@ -52,7 +52,7 @@ if ALWAYS_FALSE SUBDIRS += extra-tests endif -EXTRA_DIST = hivex.pc hivex.pc.in LICENSE README run.in \ +EXTRA_DIST = hivex.pc.in LICENSE README run.in \ po/zanata-pull.sh po/zanata.xml # Generate the ChangeLog automatically from the gitlog. -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 5/8] run: use 'prepend' function to build paths
Port libguestfs commit cae7909f5ed8 ("./run: Use 'prepend' function to
build paths.", 2015-02-13) to hivex:
Add a bash function 'prepend' for intelligently prepending elements to
paths. eg:
prepend PYTHONPATH "/foo"
would set PYTHONPATH to "/foo" or
"/foo:".
Signed-off-by: Laszlo Ersek
---
run.in | 47 ---
1 file changed, 20 insertions(+), 27 deletions(-)
diff --git a/run.in b/run.in
index 0f23a7c638d4..4adb2039c6fe 100755
--- a/run.in
+++ b/run.in
@@ -29,29 +29,31 @@
#--
+# Function to intelligently prepend a path to an environment variable.
+# See http://stackoverflow.com/a/9631350
+prepend()
+{
+eval $1="$2\${$1:+:\$$1}"
+}
+
# Source and build directories (absolute paths so this works from any
# directory).
s="$(cd @abs_srcdir@ && pwd)"
b="$(cd @abs_builddir@ && pwd)"
# Set PATH to contain all local programs.
-PATH="$b/sh:$b/regedit:$b/xml:$PATH"
+prepend PATH "$b/xml"
+prepend PATH "$b/regedit"
+prepend PATH "$b/sh"
export PATH
# Set LD_LIBRARY_PATH to contain library.
-if [ -z "$LD_LIBRARY_PATH" ]; then
-LD_LIBRARY_PATH="$b/lib/.libs"
-else
-LD_LIBRARY_PATH="$b/lib/.libs:$LD_LIBRARY_PATH"
-fi
+prepend LD_LIBRARY_PATH "$b/lib/.libs"
export LD_LIBRARY_PATH
# For Perl.
-if [ -z "$PERL5LIB" ]; then
-PERL5LIB="$b/perl/blib/lib:$b/perl/blib/arch"
-else
-PERL5LIB="$b/perl/blib/lib:$b/perl/blib/arch:$PERL5LIB"
-fi
+prepend PERL5LIB "$b/perl/blib/arch"
+prepend PERL5LIB "$b/perl/blib/lib"
export PERL5LIB
# Enable Perl valgrinding.
@@ -63,30 +65,21 @@ export PERL_DESTRUCT_LEVEL=2
# For Python.
export PYTHON="@PYTHON@"
-if [ -z "$PYTHONPATH" ]; then
-PYTHONPATH="$s/python:$b/python:$b/python/.libs"
-else
-PYTHONPATH="$s/python:$b/python:$b/python/.libs:$PYTHONPATH"
-fi
+prepend PYTHONPATH "$b/python/.libs"
+prepend PYTHONPATH "$b/python"
+prepend PYTHONPATH "$s/python"
export PYTHONPATH
# For Ruby.
export RUBY="@RUBY@"
export RAKE="@RAKE@"
-if [ -z "$RUBYLIB" ]; then
-RUBYLIB="$s/ruby/lib:$b/ruby/ext/hivex"
-else
-RUBYLIB="$s/ruby/lib:$b/ruby/ext/hivex:$RUBYLIB"
-fi
+prepend RUBYLIB "$b/ruby/ext/hivex"
+prepend RUBYLIB "$s/ruby/lib"
export RUBYLIB
-export LD_LIBRARY_PATH="$b/ruby/ext/hivex:$LD_LIBRARY_PATH"
+prepend LD_LIBRARY_PATH "$b/ruby/ext/hivex"
# For OCaml.
-if [ -z "$CAML_LD_LIBRARY_PATH" ]; then
-CAML_LD_LIBRARY_PATH="$b/ocaml"
-else
-CAML_LD_LIBRARY_PATH="$b/ocaml:$CAML_LD_LIBRARY_PATH"
-fi
+prepend CAML_LD_LIBRARY_PATH "$b/ocaml"
export CAML_LD_LIBRARY_PATH
# This is a cheap way to find some use-after-free and uninitialized
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 1/8] build: do not look for headers in "$(top_builddir)/lib"
The "hivex.h" public library header is generated when "autogen.sh" is run.
Thus, for example, "hivex.h" is part of the tarball distribution that is
created by "make dist". When plain "make" is invoked (for building the
project), "hivex.h" already exists, namely in "$(top_srcdir)/lib".
"make" does not generate *any* header files in "$(top_builddir)/lib"
(which, for out-of-tree builds, stands for a different pathname than
"$(top_srcdir)/lib" does). Remove the useless "-I$(top_builddir)/lib"
options from the lib, perl, and python makefiles, preserving the proper
"-I$(top_srcdir)/lib" ones.
Signed-off-by: Laszlo Ersek
---
lib/Makefile.am | 2 +-
perl/Makefile.PL.in | 2 +-
python/Makefile.am | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 4a7cea1752e6..1a1e2bc2e55d 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -84,7 +84,7 @@ TESTS = test-just-header
test_just_header_SOURCES = test-just-header.c
test_just_header_CFLAGS = \
- -I$(top_srcdir)/lib -I$(top_builddir)/lib \
+ -I$(top_srcdir)/lib \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
test_just_header_LDADD = \
$(top_builddir)/lib/libhivex.la
diff --git a/perl/Makefile.PL.in b/perl/Makefile.PL.in
index 77b4a0e4dd59..92e298b9a889 100644
--- a/perl/Makefile.PL.in
+++ b/perl/Makefile.PL.in
@@ -25,7 +25,7 @@ WriteMakefile (
VERSION => '@PACKAGE_VERSION@',
LIBS => '-L@top_builddir@/lib/.libs -lhivex',
-INC => '-I@top_builddir@/lib -I@top_srcdir@/lib',
+INC => '-I@top_srcdir@/lib',
TYPEMAPS => [ '@srcdir@/typemap' ],
CCFLAGS => $Config{ccflags} . ' @CFLAGS@',
);
diff --git a/python/Makefile.am b/python/Makefile.am
index 1117ab942edc..5bac34b95fc7 100644
--- a/python/Makefile.am
+++ b/python/Makefile.am
@@ -38,7 +38,7 @@ python_LTLIBRARIES = libhivexmod.la
libhivexmod_la_SOURCES = hivex-py.c
libhivexmod_la_CFLAGS = -Wall $(PYTHON_CFLAGS) \
- -I$(top_srcdir)/lib -I$(top_builddir)/lib
+ -I$(top_srcdir)/lib
libhivexmod_la_LIBADD = $(top_builddir)/lib/libhivex.la
libhivexmod_la_LDFLAGS = -avoid-version -shared -module -shrext
$(PYTHON_EXT_SUFFIX)
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 6/8] build: allow C programs using hivex to be compiled against build dir
Port libguestfs commits e33b3c83a02c ("build: Allow C programs using
libguestfs to be compiled against build dir.", 2020-03-12) and
dbfab7d3b283 ("build: fix includedir in uninstalled libguestfs.pc",
2020-09-22) to hivex.
This allows C programs to find a just built, but not installed, hivex
tree. The "run" script points PKG_CONFIG_PATH to the new file
"lib/local/hivex.pc", which describes the just-built hivex worktree.
Signed-off-by: Laszlo Ersek
---
configure.ac | 1 +
lib/Makefile.am | 3 ++-
.gitignore| 1 +
lib/local/hivex.pc.in | 35 +++
run.in| 4
5 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 lib/local/hivex.pc.in
diff --git a/configure.ac b/configure.ac
index 3c79842ee5c6..dd4a31b619e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -574,6 +574,7 @@ AC_CONFIG_FILES([Makefile
include/Makefile
lib/Makefile
lib/hivex.pc
+ lib/local/hivex.pc
lib/tools/Makefile
ocaml/Makefile ocaml/META
perl/Makefile perl/Makefile.PL
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 581a0112d8df..f385e769e1fb 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -20,7 +20,8 @@ SUBDIRS = tools
EXTRA_DIST = \
hivex.pc.in \
hivex.pod \
- hivex.syms
+ hivex.syms \
+ local/hivex.pc.in
lib_LTLIBRARIES = libhivex.la
diff --git a/.gitignore b/.gitignore
index 10154711f56c..596030afe5cc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -58,6 +58,7 @@ Makefile.in
/lib/hivex.pc
/lib/hivex.pod
/lib/hivex.syms
+/lib/local/hivex.pc
/lib/test-just-header
/lib/tools/*.opt
/libtool
diff --git a/lib/local/hivex.pc.in b/lib/local/hivex.pc.in
new file mode 100644
index ..d899cb01a282
--- /dev/null
+++ b/lib/local/hivex.pc.in
@@ -0,0 +1,35 @@
+# @configure_input@
+# Copyright (C) 2020-2021 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.
+
+
+# Dummy pkg-config file which is used to allow out of tree packages to
+# be configured against the hivex tree without hivex needing to be installed.
+#
+# Note if you are using the ./run script then you don't need to worry
+# about this because the script sets PKG_CONFIG_PATH correctly.
+
+prefix=@abs_top_builddir@
+exec_prefix=@abs_top_builddir@
+libdir=@abs_top_builddir@/lib/.libs
+includedir=@abs_top_srcdir@/include
+
+Name: hivex
+Version: @VERSION@
+Description: Read and write Windows Registry Hive files.
+Requires:
+Cflags: -I${includedir}
+Libs: -L${libdir} -lhivex
diff --git a/run.in b/run.in
index 4adb2039c6fe..e2563845f673 100755
--- a/run.in
+++ b/run.in
@@ -87,6 +87,10 @@ export CAML_LD_LIBRARY_PATH
random_val="$(awk 'BEGIN{srand(); print 1+int(255*rand())}' < /dev/null)"
export MALLOC_PERTURB_=$random_val
+# Allow dependent packages like libguestfs to be compiled against local hivex.
+prepend PKG_CONFIG_PATH "$b/lib/local"
+export PKG_CONFIG_PATH
+
# Do we have libtool? If we have it then we can use it to make
# running valgrind simpler. However don't depend on it.
if libtool --help >/dev/null 2>&1; then
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 7/8] build: link hivex statically into C programs compiled against build dir
This static-only linking allows the libguestfs daemon ("guestfsd") to
launch in the appliance, without unresolved hivex symbols, when libguestfs
is built against the hivex build dir. (Libguestfs's automatic *package*
collection for the appliance, based on the shared library requirements of
"guestfsd", cannot cover hivex when hivex is provided by a local build
directory.)
Linking hivex statically into *host-side* C programs that are compiled
against the hivex build dir is only a small penalty.
Signed-off-by: Laszlo Ersek
---
lib/local/hivex.pc.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/local/hivex.pc.in b/lib/local/hivex.pc.in
index d899cb01a282..6f0b5bb44237 100644
--- a/lib/local/hivex.pc.in
+++ b/lib/local/hivex.pc.in
@@ -32,4 +32,4 @@ Version: @VERSION@
Description: Read and write Windows Registry Hive files.
Requires:
Cflags: -I${includedir}
-Libs: -L${libdir} -lhivex
+Libs: -L${libdir} -l:libhivex.a
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 8/8] build: allow OCaml programs using hivex to be compiled against build dir
Port libguestfs commit bf61bf7355d3 ("build: Allow OCaml programs using
libguestfs to be compiled against build dir.", 2020-03-12) to hivex.
This allows C+OCaml programs, such as libguestfs itself, to find a just
built, but not installed, hivex tree:
> libguestfs$ ../hivex/run ./configure CFLAGS=-fPIC
> libguestfs$ ../hivex/run make -j $(getconf _NPROCESSORS_ONLN)
> libguestfs$ ../hivex/run make -j $(getconf _NPROCESSORS_ONLN) check
Signed-off-by: Laszlo Ersek
---
ocaml/Makefile.am | 19 ++-
.gitignore| 1 +
run.in| 2 ++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
index 1ef0c194e212..ace3208ea309 100644
--- a/ocaml/Makefile.am
+++ b/ocaml/Makefile.am
@@ -112,7 +112,24 @@ install-data-hook:
CLEANFILES += $(noinst_DATA)
-endif
+# This "tricks" ocamlfind into allowing us to compile other OCaml
+# programs against a locally compiled copy of the hivex sources.
+# ocamlfind needs to see a directory called ‘hivex’ which contains
+# ‘META’. The current directory is called ‘ocaml’, but if we make
+# this symlink then we can create the required directory structure.
+#
+# Note if you just want to use this, make sure you use
+# ‘../hivex/run make’ in your other program and everything should
+# just work.
+CLEANFILES += hivex
+
+all-local: hivex
+
+hivex:
+ rm -f $@
+ $(LN_S) . $@
+
+endif HAVE_OCAML
# Tell version 3.79 and up of GNU make to not build goals in this
# directory in parallel. (See RHBZ#502309).
diff --git a/.gitignore b/.gitignore
index 596030afe5cc..b856922c917b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -72,6 +72,7 @@ Makefile.in
/m4/ltversion.m4
/maint.mk
/missing
+/ocaml/hivex
/ocaml/hivex.ml
/ocaml/hivex.mli
/ocaml/hivex_c.c
diff --git a/run.in b/run.in
index e2563845f673..e166a9202248 100755
--- a/run.in
+++ b/run.in
@@ -90,6 +90,8 @@ export MALLOC_PERTURB_=$random_val
# Allow dependent packages like libguestfs to be compiled against local hivex.
prepend PKG_CONFIG_PATH "$b/lib/local"
export PKG_CONFIG_PATH
+prepend OCAMLPATH "$b/ocaml"
+export OCAMLPATH
# Do we have libtool? If we have it then we can use it to make
# running valgrind simpler. However don't depend on it.
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 0/8] add a pkg-config descriptor for the local (not installed) build tree
This series introduces the "lib/local/hivex.pc" pkg-config file to hivex, permitting, through the "run" script, other programs and libraries to be built against the just-built (not installed) hivex tree. A few small details in the hivex build machinery that I understood to be warts are cleaned up, and parts of the libguestfs tree *layout* are adopted. Thanks, Laszlo Laszlo Ersek (8): build: do not look for headers in "$(top_builddir)/lib" build: expose public library header "hivex.h" without "lib" contents build: remove "hivex.pc" from EXTRA_DIST build: move "hivex.pc.in" to the "lib" subdirectory run: use 'prepend' function to build paths build: allow C programs using hivex to be compiled against build dir build: link hivex statically into C programs compiled against build dir build: allow OCaml programs using hivex to be compiled against build dir configure.ac | 4 ++- Makefile.am| 8 ++--- images/Makefile.am | 2 +- include/Makefile.am| 20 + lib/Makefile.am| 16 +++--- ocaml/Makefile.am | 21 -- perl/Makefile.PL.in| 2 +- python/Makefile.am | 2 +- ruby/Rakefile.in | 2 +- sh/Makefile.am | 3 +- xml/Makefile.am| 1 + .gitignore | 6 ++-- generator/generator.ml | 2 +- hivex.pc.in => lib/hivex.pc.in | 0 lib/local/hivex.pc.in | 35 ++ run.in | 53 +- 16 files changed, 129 insertions(+), 48 deletions(-) create mode 100644 include/Makefile.am rename hivex.pc.in => lib/hivex.pc.in (100%) create mode 100644 lib/local/hivex.pc.in -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH 2/8] build: expose public library header "hivex.h" without "lib" contents
Move the public library header "hivex.h" from "lib" to "include".
In the "lib", "sh" and "xml" subdirectories, some of the library-internal
header files ("byte_conversions.h", "gettext.h", "hivex-internal.h",
"mmap.h") are consumed. There, let the new "include" directory extend --
and not replace -- the existent "lib" directory.
Signed-off-by: Laszlo Ersek
---
configure.ac | 1 +
Makefile.am| 2 +-
images/Makefile.am | 2 +-
include/Makefile.am| 20
lib/Makefile.am| 7 +++
ocaml/Makefile.am | 2 +-
perl/Makefile.PL.in| 2 +-
python/Makefile.am | 2 +-
ruby/Rakefile.in | 2 +-
sh/Makefile.am | 3 ++-
xml/Makefile.am| 1 +
.gitignore | 2 +-
generator/generator.ml | 2 +-
13 files changed, 35 insertions(+), 13 deletions(-)
create mode 100644 include/Makefile.am
diff --git a/configure.ac b/configure.ac
index 6180195ec4be..8adcb8512e66 100644
--- a/configure.ac
+++ b/configure.ac
@@ -572,6 +572,7 @@ AC_CONFIG_FILES([Makefile
gnulib/tests/Makefile
hivex.pc
images/Makefile
+ include/Makefile
lib/Makefile
lib/tools/Makefile
ocaml/Makefile ocaml/META
diff --git a/Makefile.am b/Makefile.am
index 48611a0f929e..d217ea52fcb2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -20,7 +20,7 @@ ACLOCAL_AMFLAGS = -I m4
# Work around broken libtool.
export to_tool_file_cmd=func_convert_file_noop
-SUBDIRS = gnulib/lib generator lib images gnulib/tests
+SUBDIRS = gnulib/lib generator lib images include gnulib/tests
if HAVE_LIBXML2
SUBDIRS += xml
diff --git a/images/Makefile.am b/images/Makefile.am
index cdf3edc4c27d..41a0c0c06935 100644
--- a/images/Makefile.am
+++ b/images/Makefile.am
@@ -26,7 +26,7 @@ EXTRA_DIST = minimal rlenvalue_test_hive special
mkzero/Makefile mkzero/mkzero.c
noinst_PROGRAMS = mklarge
mklarge_SOURCES = mklarge.c
mklarge_CFLAGS = \
- -I$(srcdir)/../lib \
+ -I$(srcdir)/../include \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
mklarge_LDADD = ../lib/libhivex.la
diff --git a/include/Makefile.am b/include/Makefile.am
new file mode 100644
index ..edb02fe0a972
--- /dev/null
+++ b/include/Makefile.am
@@ -0,0 +1,20 @@
+# hivex
+# Copyright (C) 2009-2021 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., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+EXTRA_DIST = hivex.h
+
+include_HEADERS = hivex.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 1a1e2bc2e55d..9bdf494c7742 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -27,7 +27,7 @@ libhivex_la_SOURCES = \
byte_conversions.h \
gettext.h \
handle.c \
- hivex.h \
+ ../include/hivex.h \
hivex-internal.h \
mmap.h \
node.c \
@@ -48,11 +48,10 @@ libhivex_la_LDFLAGS = \
libhivex_la_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
libhivex_la_CPPFLAGS = \
-I$(top_srcdir)/gnulib/lib \
+ -I$(top_srcdir)/include \
-I$(top_builddir)/gnulib/lib \
-I$(srcdir)
-include_HEADERS = hivex.h
-
man_MANS = hivex.3
hivex.3: hivex.pod
@@ -84,7 +83,7 @@ TESTS = test-just-header
test_just_header_SOURCES = test-just-header.c
test_just_header_CFLAGS = \
- -I$(top_srcdir)/lib \
+ -I$(top_srcdir)/include \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
test_just_header_LDADD = \
$(top_builddir)/lib/libhivex.la
diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
index 85655b6f727d..1ef0c194e212 100644
--- a/ocaml/Makefile.am
+++ b/ocaml/Makefile.am
@@ -26,7 +26,7 @@ CLEANFILES += t/*.cmi t/*.cmo t/*.cmx t/*.o t/*.a t/*.so
AM_CPPFLAGS = \
-I$(top_builddir) -I$(OCAMLLIB) -I$(top_srcdir)/ocaml \
- -I$(top_srcdir)/lib \
+ -I$(top_srcdir)/include \
$(WARN_CFLAGS) $(WERROR_CFLAGS)
if HAVE_OCAML
diff --git a/perl/Makefile.PL.in b/perl/Makefile.PL.in
index 92e298b9a889..49d79d1a1c29 100644
--- a/perl/Makefile.PL.in
+++ b/perl/Makefile.PL.in
@@ -25,7 +25,7 @@ WriteMakefile (
VERSION => '@PACKAGE_VERSION@',
LIBS => '-L@top_builddir@/lib/.libs -lhivex',
-INC => '-I@top_srcdir@/lib',
+INC => '-I@top_srcdir@/include',
TYPEMAPS => [ '@srcdir@/type
[Libguestfs] [hivex PATCH 4/8] build: move "hivex.pc.in" to the "lib" subdirectory
Bring hivex's worktree structure closer to libguestfs's, in preparation for introducing "lib/local/hivex.pc.in" in a subsequent patch. Move "hivex.pc.in" to the "lib" subdirectory. Signed-off-by: Laszlo Ersek --- configure.ac | 2 +- Makefile.am| 6 +- lib/Makefile.am| 8 .gitignore | 2 +- hivex.pc.in => lib/hivex.pc.in | 0 5 files changed, 11 insertions(+), 7 deletions(-) rename hivex.pc.in => lib/hivex.pc.in (100%) diff --git a/configure.ac b/configure.ac index 8adcb8512e66..3c79842ee5c6 100644 --- a/configure.ac +++ b/configure.ac @@ -570,10 +570,10 @@ AC_CONFIG_FILES([Makefile generator/Makefile gnulib/lib/Makefile gnulib/tests/Makefile - hivex.pc images/Makefile include/Makefile lib/Makefile + lib/hivex.pc lib/tools/Makefile ocaml/Makefile ocaml/META perl/Makefile perl/Makefile.PL diff --git a/Makefile.am b/Makefile.am index f69d8cfb8e67..221eebb75298 100644 --- a/Makefile.am +++ b/Makefile.am @@ -52,7 +52,7 @@ if ALWAYS_FALSE SUBDIRS += extra-tests endif -EXTRA_DIST = hivex.pc.in LICENSE README run.in \ +EXTRA_DIST = LICENSE README run.in \ po/zanata-pull.sh po/zanata.xml # Generate the ChangeLog automatically from the gitlog. @@ -60,10 +60,6 @@ dist-hook: $(top_srcdir)/build-aux/gitlog-to-changelog > ChangeLog cp ChangeLog $(distdir)/ChangeLog -# Pkgconfig. -pkgconfigdir = $(libdir)/pkgconfig -pkgconfig_DATA = hivex.pc - # Maintainer website update. HTMLFILES = \ html/hivex.3.html \ diff --git a/lib/Makefile.am b/lib/Makefile.am index 9bdf494c7742..581a0112d8df 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -18,6 +18,7 @@ SUBDIRS = tools EXTRA_DIST = \ + hivex.pc.in \ hivex.pod \ hivex.syms @@ -52,6 +53,13 @@ libhivex_la_CPPFLAGS = \ -I$(top_builddir)/gnulib/lib \ -I$(srcdir) +# Pkgconfig. + +pkgconfigdir = $(libdir)/pkgconfig +pkgconfig_DATA = hivex.pc + +# Manual page. + man_MANS = hivex.3 hivex.3: hivex.pod diff --git a/.gitignore b/.gitignore index af256d59030e..10154711f56c 100644 --- a/.gitignore +++ b/.gitignore @@ -44,7 +44,6 @@ Makefile.in /generator/stamp-generator /GNUmakefile /gnulib -/hivex.pc /hivex-*.tar.gz /html/hivex.3.html /html/hivexget.1.html @@ -56,6 +55,7 @@ Makefile.in /include/hivex.h /install-sh /lib/*.3 +/lib/hivex.pc /lib/hivex.pod /lib/hivex.syms /lib/test-just-header diff --git a/hivex.pc.in b/lib/hivex.pc.in similarity index 100% rename from hivex.pc.in rename to lib/hivex.pc.in -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs PATCH 1/2] daemon: generalize ocaml-hivex[-devel] lookup
"ocamlc -where" is supposed to "print the location of the standard library and exit". While this directory contains core OCaml C header files, it does not contain hivex-related C header files. Trim "guestfsd_CPPFLAGS" accordingly. Furthermore, the hivex module for OCaml may exist elsewhere than under the OCaml standard library directory. Invoke "ocamlfind query hivex" to find this module. This is what AC_CHECK_OCAML_PKG(hivex) does too, in "m4/guestfs-ocaml.m4" and "m4/ocaml.m4". Signed-off-by: Laszlo Ersek --- daemon/Makefile.am | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 6f13bd43cf55..83bf39975e04 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -215,7 +215,7 @@ guestfsd_SOURCES = \ guestfsd_LDFLAGS = \ -L$(shell $(OCAMLC) -where) \ - -L$(shell $(OCAMLC) -where)/hivex \ + -L$(shell $(OCAMLFIND) query hivex) \ -L../common/mlutils \ -L../common/mlstdutils \ -L../bundled/ocaml-augeas \ @@ -246,7 +246,6 @@ guestfsd_LDADD = \ guestfsd_CPPFLAGS = \ -DCAML_NAME_SPACE \ -I$(shell $(OCAMLC) -where) \ - -I$(shell $(OCAMLC) -where)/hivex \ -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/gnulib/lib \ -I$(top_srcdir)/lib \ -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs PATCH 0/2] generalize ocaml-hivex[-devel] lookup
"daemon/Makefile.am" needs some tweaks for finding such OCaml bindings for hivex that are not installed system-wide. Thanks, Laszlo Laszlo Ersek (2): daemon: generalize ocaml-hivex[-devel] lookup daemon_utils_tests: generalize ocaml-hivex[-devel] lookup daemon/Makefile.am | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libguestfs PATCH 2/2] daemon_utils_tests: generalize ocaml-hivex[-devel] lookup
Pass $(HIVEX_LIBS) with -cclib under the "daemon_utils_tests_LINK" target; otherwise the OCaml compiler does not tell the linker where "-lhivex" can be found, and the linking step fails if "-lhivex" is not on a system library path. Signed-off-by: Laszlo Ersek --- daemon/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 83bf39975e04..7322bfa5d765 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -453,7 +453,7 @@ daemon_utils_tests_DEPENDENCIES = \ $(top_builddir)/ocaml-link.sh daemon_utils_tests_LINK = \ $(top_builddir)/ocaml-link.sh \ - -cclib '-lutils -lgnu' \ + -cclib '-lutils -lgnu $(HIVEX_LIBS)' \ -- \ $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ $(OCAMLPACKAGES) \ -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 3/4] tests: xfs: remove lazy-counter disablement test
According to xfs_admin(8): >-c 0|1 Enable (1) or disable (0) lazy-counters in the filesys‐ > tem. > > Lazy-counters may not be disabled on Version 5 su‐ > perblock filesystems (i.e. those with metadata CRCs en‐ > abled). > > [...] According to mkfs.xfs(1): >-m global_metadata_options >Section Name: [metadata] > These options specify metadata format options that ei‐ > ther apply to the entire filesystem or aren't easily > characterised by a specific functionality group. The > valid global_metadata_options are: > > [...] > >crc=value > This is used to create a filesystem which > maintains and checks CRC information in all > metadata objects on disk. The value is ei‐ > ther 0 to disable the feature, or 1 to en‐ > able the use of CRCs. > > [...] > > By default, mkfs.xfs will enable metadata > CRCs. Consistently with the above, the first "xfs_admin" test case in "generator/actions_core.ml", which attempts to disable lazy counters, always fails: > 534/550 test_xfs_admin_0 > libguestfs: error: xfs_admin: /dev/sda1: Cannot disable lazy-counters on V5 fs We can resolve this test failure in three ways: (1) Extend do_mkfs() [daemon/mkfs.c], possibly even introduce do_mkfs_xfs(), and permit the caller to specify "-m crc=0" for mkfs.xfs. Then use this option when the temporary filesystem is created in the XFS test that disables lazy counters. (2) Extend the "guestfs_int_xfsinfo" structure in the libguestfs-common project, with an "xfs_crc" field. Extend parse_xfs_info() [daemon/xfs.c] to populate the field from "meta-data=...crc=[01]". Modify the test case to check the following post-condition: xfs_crc || xfs_lazycount == 0 instead of the current xfs_lazycount == 0 effectively ignoring "xfs_lazycount" when "xfs_crc" is set. (3) Remove the test altogether that attempts to disable lazy counters after filesystem creation. Given that new XFS filesystems are created with metadata CRCs enabled by default, and several XFS features depend on metadata CRCs being enabled, this patch implements option (3). Signed-off-by: Laszlo Ersek --- generator/actions_core.ml | 6 -- 1 file changed, 6 deletions(-) diff --git a/generator/actions_core.ml b/generator/actions_core.ml index bb602ee02dfc..5933282dcf90 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -7644,12 +7644,6 @@ with zeroes)." }; style = RErr, [String (Device, "device")], [OBool "extunwritten"; OBool "imgfile"; OBool "v2log"; OBool "projid32bit"; OBool "lazycounter"; OString "label"; OString "uuid"]; optional = Some "xfs"; tests = [ -InitEmpty, Always, TestResult ( - [["part_disk"; "/dev/sda"; "mbr"]; - ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; - ["xfs_admin"; "/dev/sda1"; ""; ""; ""; ""; "false"; "NOARG"; "NOARG"]; - ["mount"; "/dev/sda1"; "/"]; - ["xfs_info"; "/"]], "ret->xfs_lazycount == 0"), []; InitEmpty, Always, TestResultString ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
The current "arg_string_list" and "free_string_list" implementations go
back to commit b6f01f32606d ("Add Go (language) bindings.", 2013-07-03).
There are two problems with them:
- "free_string_list" doesn't actually free anything,
- at the *first* such g.Internal_test() call site that passes an
Ostringlist member inside the Optargs argument, namely:
> g.Internal_test ("abc",
> string_addr ("def"),
> []string{},
> false,
> 0,
> 0,
> "123",
> "456",
> []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
> _test{Ostringlist_is_set: true,
>Ostringlist: []string{}
> }
> )
the "golang/run-bindtests" case crashes:
> panic: runtime error: cgo argument has Go pointer to Go pointer
>
> goroutine 1 [running]:
> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc18180,
> 0xadfb60, 0xadfb80, 0xc10048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
> 0xade3a0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc18180, 0x4ee3a5,
> 0x3, 0xc61be8, 0xc61af8, 0x0, 0x0, 0xc61a00, 0x0, 0x0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
> main.main()
> golang/bindtests/bindtests.go:77 +0x151e
> exit status 2
> FAIL run-bindtests (exit status: 1)
It turns out that a C-language array containing C-language pointers can
only be allocated in C:
https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
Rewrite the "arg_string_list" and "free_string_list" functions almost
entirely in C.
Signed-off-by: Laszlo Ersek
---
generator/golang.ml | 46 -
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/generator/golang.ml b/generator/golang.ml
index d328abe4ed08..5db478e8a494 100644
--- a/generator/golang.ml
+++ b/generator/golang.ml
@@ -52,6 +52,39 @@ _go_guestfs_create_flags (unsigned flags)
{
return guestfs_create_flags (flags);
}
+
+//
+// A C-language array of C-language pointers needs to be allocated in C.
+//
https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
+//
+typedef char *pChar;
+
+pChar *allocStringArray (size_t nmemb)
+{
+pChar *array;
+
+array = malloc (sizeof (pChar) * (nmemb + 1));
+array[nmemb] = NULL;
+return array;
+}
+
+void storeString (pChar *array, size_t idx, pChar string)
+{
+array[idx] = string;
+}
+
+void freeStringArray (pChar *array)
+{
+pChar *position;
+pChar string;
+
+position = array;
+while ((string = *position) != NULL) {
+ free (string);
+ position++;
+}
+free (array);
+}
*/
import \"C\"
@@ -148,12 +181,11 @@ func (g *Guestfs) Close () *GuestfsError {
* C strings and golang []string.
*/
func arg_string_list (xs []string) **C.char {
-r := make ([]*C.char, 1 + len (xs))
+r := C.allocStringArray (C.size_t (len (xs)))
for i, x := range xs {
-r[i] = C.CString (x)
+C.storeString (r, C.size_t (i), C.CString (x));
}
-r[len (xs)] = nil
-return [0]
+return (**C.char) (r)
}
func count_string_list (argv **C.char) int {
@@ -167,11 +199,7 @@ func count_string_list (argv **C.char) int {
}
func free_string_list (argv **C.char) {
-for *argv != nil {
-//C.free (*argv)
-argv = (**C.char) (unsafe.Pointer (uintptr (unsafe.Pointer (argv)) +
- unsafe.Sizeof (*argv)))
-}
+C.freeStringArray ((*C.pChar) (argv))
}
func return_string_list (argv **C.char) []string {
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 2/4] test-md-and-lvm-devices: work around RAID0 regression in Linux v3.14/v5.4
The "test-md-and-lvm-devices" test case creates, among other things, a RAID0 array (md127) that spans two *differently sized* block devices (sda1: 20MB, lv0: 16MB). In Linux v3.14, the layout of such arrays was changed incompatibly and undetectably. If an array were created with a pre-v3.14 kernel and assembled on a v3.14+ kernel, or vice versa, data could be corrupted. In Linux v5.4, a mitigation was added, requiring the user to specify the layout version of such RAID0 arrays explicitly, as a module parameter. If the user fails to specify a layout version, the v5.4+ kernel refuses to assemble such arrays. This is why "test-md-and-lvm-devices" currently fails, with any v5.4+ appliance kernel. Until we implement a more general solution (see the bugzilla link below), work around the issue by sizing sda1 and lv0 identically. For this, increase the size of sdb1 to 24MB: when one 4MB extent is spent on LVM metadata, the resultant lv0 size (20MB) will precisely match the size of sda1. This workaround only affects sizes, and does not interfere with the original purpose of this test case, which is to test various *stackings* between disk partitions, software RAID (md), and LVM logical volumes. Related: https://bugzilla.redhat.com/show_bug.cgi?id=2005485 Signed-off-by: Laszlo Ersek --- tests/md/test-md-and-lvm-devices.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/md/test-md-and-lvm-devices.sh b/tests/md/test-md-and-lvm-devices.sh index 5e82e3a4ff69..54f1c3bfb3f5 100755 --- a/tests/md/test-md-and-lvm-devices.sh +++ b/tests/md/test-md-and-lvm-devices.sh @@ -53,7 +53,7 @@ trap cleanup INT QUIT TERM EXIT # sda2: 20M PV (vg1) # sda3: 20M MD (md125) # -# sdb1: 20M PV (vg0) +# sdb1: 24M PV (vg0) [*] # sdb2: 20M PV (vg2) # sdb3: 20M MD (md125) # @@ -66,6 +66,9 @@ trap cleanup INT QUIT TERM EXIT # vg3 : VG (md125) # lv3 : LV (vg3) # +# [*] The reason for making sdb1 4M larger than sda1 is that the LVM metadata +# will consume one 4MB extent, and we need lv0 to offer exactly as much space +# as sda1 does, for combining them in md127. Refer to RHBZ#2005485. guestfish <https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 0/4] libguestfs modifications in order to pass "make check"
In my Fedora 34 environment, with all libguestfs features enabled except the Erlang and Haskell bindings, "make check" passes if: - these patches are applied on top of current master (f47e0bb67254), and - the btrfs-progs package is downgraded to 5.13.1-1.fc34.x86_64, according to <https://bugzilla.redhat.com/show_bug.cgi?id=2005529>. Thanks, Laszlo Laszlo Ersek (4): test-9p: fix the base directory that's exported to the guest test-md-and-lvm-devices: work around RAID0 regression in Linux v3.14/v5.4 tests: xfs: remove lazy-counter disablement test Go bindings: fix "C array of strings" -- char** -- allocation generator/actions_core.ml | 6 generator/golang.ml | 46 +++-- tests/9p/test-9p.sh | 2 +- tests/md/test-md-and-lvm-devices.sh | 12 +--- 4 files changed, 46 insertions(+), 20 deletions(-) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 1/4] test-9p: fix the base directory that's exported to the guest
In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
the "abs_srcdir" macro value that the 9p test would see changed from
".../tests/9p" to just ".../tests" -- the last component got dropped.
(Said commit updated some "abs_srcdir"-based references accordingly, for
example under "tests/disks", but "tests/9p/test-9p.sh" was missed.)
Therefore, the guest-visible location of the "/test-9p.sh" file changed to
"/9p/test-9p.sh", and a non-recursive listing of the guest-visible root
directory would not return the file. Thus, the test fails now.
Restore the host-side base directory to ".../tests/9p".
Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
Signed-off-by: Laszlo Ersek
---
tests/9p/test-9p.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/9p/test-9p.sh b/tests/9p/test-9p.sh
index b4bdbe56e077..4fd5de7fdafe 100755
--- a/tests/9p/test-9p.sh
+++ b/tests/9p/test-9p.sh
@@ -45,7 +45,7 @@ guestfish <https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [hivex PATCH] lib: write: improve key collation compatibility with Windows
On 09/10/21 09:48, Richard W.M. Jones wrote:
> On Fri, Sep 10, 2021 at 01:06:17AM +0200, Laszlo Ersek wrote:
>> There are multiple problems with using strcasecmp() for ordering registry
>> keys:
>>
>> (1) strcasecmp() is influenced by LC_CTYPE.
>>
>> (2) strcasecmp() cannot implement case conversion for multibyte UTF-8
>> sequences.
>>
>> (3) Even with LC_CTYPE=POSIX and key names consisting solely of ASCII
>> characters, strcasecmp() converts characters to lowercase, for
>> comparison. But on Windows, the CompareStringOrdinal() function
>> converts characters to uppercase. This makes a difference when
>> comparing a letter to one of the characters that fall between 'Z'
>> (0x5A) and 'a' (0x61), namely {'[', '\\', ']', '^', '_', '`'}. For
>> example,
>>
>> 'c' (0x63) > '_' (0x5F)
>> 'C' (0x43) < '_' (0x5F)
>>
>> Compare key names byte for byte, eliminating problems (1) and (3).
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1648520
>> Signed-off-by: Laszlo Ersek
>> ---
>> lib/write.c | 32 +++-
>> 1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/write.c b/lib/write.c
>> index 70105c9d9907..d9a13a3c18b6 100644
>> --- a/lib/write.c
>> +++ b/lib/write.c
>> @@ -462,7 +462,37 @@ compare_name_with_nk_name (hive_h *h, const char *name,
>> hive_node_h nk_offs)
>> return 0;
>>}
>>
>> - int r = strcasecmp (name, nname);
>> + /* Perform a limited case-insensitive comparison. ASCII letters will be
>> + * *upper-cased*. Multibyte sequences will produce nonsensical orderings.
>> + */
>> + int r = 0;
>> + const char *s1 = name;
>> + const char *s2 = nname;
>> +
>> + for (;;) {
>> +unsigned char c1 = *(s1++);
>> +unsigned char c2 = *(s2++);
>> +
>> +if (c1 >= 'a' && c1 <= 'z')
>> + c1 = 'A' + (c1 - 'a');
>> +if (c2 >= 'a' && c2 <= 'z')
>> + c2 = 'A' + (c2 - 'a');
>> +if (c1 < c2) {
>> + /* Also covers the case when "name" is a prefix of "nname". */
>> + r = -1;
>> + break;
>> +}
>> +if (c1 > c2) {
>> + /* Also covers the case when "nname" is a prefix of "name". */
>> + r = 1;
>> + break;
>> +}
>> +if (c1 == '\0') {
>> + /* Both strings end. */
>> + break;
>> +}
>> + }
>> +
>>free (nname);
>>
>>return r;
>
> Thanks for the detailed analysis on the BZ.
>
> ACK - since it's an incremental improvement over what we have now and
> fixes the bug.
Thank you! Commit d5a522c0bb73.
> There may be registries with multibyte keys (nothing surprises me
> about the Windows registry), but as this only affects the ability to
> insert new keys into a node that has such keys, the problem that we
> don't handle those is limited in practice.
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [hivex PATCH] lib: write: improve key collation compatibility with Windows
There are multiple problems with using strcasecmp() for ordering registry
keys:
(1) strcasecmp() is influenced by LC_CTYPE.
(2) strcasecmp() cannot implement case conversion for multibyte UTF-8
sequences.
(3) Even with LC_CTYPE=POSIX and key names consisting solely of ASCII
characters, strcasecmp() converts characters to lowercase, for
comparison. But on Windows, the CompareStringOrdinal() function
converts characters to uppercase. This makes a difference when
comparing a letter to one of the characters that fall between 'Z'
(0x5A) and 'a' (0x61), namely {'[', '\\', ']', '^', '_', '`'}. For
example,
'c' (0x63) > '_' (0x5F)
'C' (0x43) < '_' (0x5F)
Compare key names byte for byte, eliminating problems (1) and (3).
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1648520
Signed-off-by: Laszlo Ersek
---
lib/write.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/lib/write.c b/lib/write.c
index 70105c9d9907..d9a13a3c18b6 100644
--- a/lib/write.c
+++ b/lib/write.c
@@ -462,7 +462,37 @@ compare_name_with_nk_name (hive_h *h, const char *name,
hive_node_h nk_offs)
return 0;
}
- int r = strcasecmp (name, nname);
+ /* Perform a limited case-insensitive comparison. ASCII letters will be
+ * *upper-cased*. Multibyte sequences will produce nonsensical orderings.
+ */
+ int r = 0;
+ const char *s1 = name;
+ const char *s2 = nname;
+
+ for (;;) {
+unsigned char c1 = *(s1++);
+unsigned char c2 = *(s2++);
+
+if (c1 >= 'a' && c1 <= 'z')
+ c1 = 'A' + (c1 - 'a');
+if (c2 >= 'a' && c2 <= 'z')
+ c2 = 'A' + (c2 - 'a');
+if (c1 < c2) {
+ /* Also covers the case when "name" is a prefix of "nname". */
+ r = -1;
+ break;
+}
+if (c1 > c2) {
+ /* Also covers the case when "nname" is a prefix of "name". */
+ r = 1;
+ break;
+}
+if (c1 == '\0') {
+ /* Both strings end. */
+ break;
+}
+ }
+
free (nname);
return r;
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 2/2] tests/mount-local: fix relative pathname of FUSE client executable
On 09/02/21 15:56, Richard W.M. Jones wrote:
> On Thu, Sep 02, 2021 at 03:51:24PM +0200, Laszlo Ersek wrote:
>> In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
>> the working directory relative to which "test-parallel-mount-local" would
>> be launched (by the test machinery) changed from "tests/mount-local" to
>> just "tests".
>>
>> While the relative pathname of the "guestunmount" executable was updated
>> inside "test-parallel-mount-local" accordingly, the relative pathname of
>> the FUSE client ("test-parallel-mount-local" itself, just invoked with
>> "--test") was not. This issue guarantees that the exec call fails in the
>> child, and so the test case always hangs.
>>
>> Because we had removed "mount-local" from the end of the working
>> directory, prepend it now to the relative pathname of the FUSE client
>> executable.
>>
>> Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
>> Signed-off-by: Laszlo Ersek
>> ---
>> tests/mount-local/test-parallel-mount-local.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/mount-local/test-parallel-mount-local.c
>> b/tests/mount-local/test-parallel-mount-local.c
>> index 5f00e328a39c..c33ecf5b2680 100644
>> --- a/tests/mount-local/test-parallel-mount-local.c
>> +++ b/tests/mount-local/test-parallel-mount-local.c
>> @@ -220,7 +220,7 @@ start_thread (void *statevp)
>>
>> if (pid == 0) { /* child */
>>setpgid (0, 0); /* so we don't get ^C from parent */
>> - execlp ("./test-parallel-mount-local",
>> + execlp ("mount-local/test-parallel-mount-local",
>>"test-parallel-mount-local", "--test", state->mp, NULL);
>>perror ("execlp");
>>_exit (EXIT_FAILURE);
>
> ACK series.
Pushed as commit range 13a1ae6da6e8..0a2f7621a089.
I added "Acked-by: Richard W.M. Jones " to both
commit messages, with Rich's permission.
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v2] Go bindings: fix "C array of strings" -- char** -- allocation
The current "arg_string_list" and "free_string_list" implementations go
back to commit b6f01f32606d ("Add Go (language) bindings.", 2013-07-03).
There are two problems with them:
- "free_string_list" doesn't actually free anything,
- at the *first* such g.Internal_test() call site that passes an
Ostringlist member inside the Optargs argument, namely:
> g.Internal_test ("abc",
> string_addr ("def"),
> []string{},
> false,
> 0,
> 0,
> "123",
> "456",
> []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
> _test{Ostringlist_is_set: true,
>Ostringlist: []string{}
> }
> )
the "golang/run-bindtests" case crashes:
> panic: runtime error: cgo argument has Go pointer to Go pointer
>
> goroutine 1 [running]:
> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc18180,
> 0xadfb60, 0xadfb80, 0xc10048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
> 0xade3a0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc18180, 0x4ee3a5,
> 0x3, 0xc61be8, 0xc61af8, 0x0, 0x0, 0xc61a00, 0x0, 0x0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
> main.main()
> golang/bindtests/bindtests.go:77 +0x151e
> exit status 2
> FAIL run-bindtests (exit status: 1)
In Daniel P. Berrangé's words [1],
> You're allowed to pass a Go pointer to C via CGo, but the memory that
> points to is not allowed to contained further Go pointers. So the struct
> fields must strictly use a C pointer.
One pattern to solve the problem has been shown on stackoverflow [2].
Thus, rewrite the "arg_string_list" and "free_string_list" functions
almost entirely in C, following that example.
While this approach is not the most idiomatic Go, as a solution exists
without C helper functions [3], it should still be acceptable, at least as
an incremental improvement, for letting "golang/run-bindtests" pass.
[1] https://listman.redhat.com/archives/libguestfs/2021-September/msg00118.html
[2]
https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
[3] https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html
Cc: "Daniel P. Berrangé"
Cc: "Richard W.M. Jones"
Signed-off-by: Laszlo Ersek
---
Notes:
v2:
- update commit message [Daniel, Rich]
- update code comment [Daniel, Rich]
- no code changes
- retested
generator/golang.ml | 47 -
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/generator/golang.ml b/generator/golang.ml
index d328abe4ed08..0d6a92367f9b 100644
--- a/generator/golang.ml
+++ b/generator/golang.ml
@@ -52,6 +52,40 @@ _go_guestfs_create_flags (unsigned flags)
{
return guestfs_create_flags (flags);
}
+
+// Passing a Go pointer to C via CGo is allowed, but the memory that points to
+// is not allowed to contain further Go pointers. The helper functions below
+// are one way to implement this, although the same can be achieved purely in
+// Go as well. See the discussion here:
+//
<https://listman.redhat.com/archives/libguestfs/2021-September/msg00101.html>.
+typedef char *pChar;
+
+pChar *allocStringArray (size_t nmemb)
+{
+pChar *array;
+
+array = malloc (sizeof (pChar) * (nmemb + 1));
+array[nmemb] = NULL;
+return array;
+}
+
+void storeString (pChar *array, size_t idx, pChar string)
+{
+array[idx] = string;
+}
+
+void freeStringArray (pChar *array)
+{
+pChar *position;
+pChar string;
+
+position = array;
+while ((string = *position) != NULL) {
+ free (string);
+ position++;
+}
+free (array);
+}
*/
import \"C\"
@@ -148,12 +182,11 @@ func (g *Guestfs) Close () *GuestfsError {
* C strings and golang []string.
*/
func arg_string_list (xs []string) **C.char {
-r := make ([]*C.char, 1 + len (xs))
+r := C.allocStringArray (C.size_t (len (xs)))
for i, x := range xs {
-r[i] = C.CString (x)
+C.storeString (r, C.size_t (i), C.CString (x));
}
-r[len (xs)] = nil
-return [0]
+return (**C.char) (r)
}
func count_string_list (argv **C.char) int {
@@ -167,11 +200,7 @@ func count_string_list (argv **C.char) int {
}
func free_string_list (argv **C.char) {
-for *argv != nil {
-//C.free (*argv)
-argv = (**C.char) (unsafe.Pointer (uintptr (unsafe.Pointer (argv)) +
- unsafe.Sizeof (*argv)))
-}
+C.freeStringArr
Re: [Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
On 09/20/21 12:37, Daniel P. Berrangé wrote:
> On Mon, Sep 20, 2021 at 07:23:35AM +0200, Laszlo Ersek wrote:
>> The current "arg_string_list" and "free_string_list" implementations go
>> back to commit b6f01f32606d ("Add Go (language) bindings.", 2013-07-03).
>> There are two problems with them:
>>
>> - "free_string_list" doesn't actually free anything,
>
> Doh.
In fact I do not think that was an oversight (i.e. that C.free() was
commented out). I didn't try, but I suspect that calling C.free()
triggered the same issue when Rich originally worked on the go bindings,
so C.free() was postponed. And then forgotten. (This is just my
speculation, again.)
>
>>
>> - at the *first* such g.Internal_test() call site that passes an
>> Ostringlist member inside the Optargs argument, namely:
>>
>>> g.Internal_test ("abc",
>>> string_addr ("def"),
>>> []string{},
>>> false,
>>> 0,
>>> 0,
>>> "123",
>>> "456",
>>> []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
>>> _test{Ostringlist_is_set: true,
>>>Ostringlist: []string{}
>>> }
>>> )
>>
>> the "golang/run-bindtests" case crashes:
>>
>>> panic: runtime error: cgo argument has Go pointer to Go pointer
>>>
>>> goroutine 1 [running]:
>>> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc18180,
>>> 0xadfb60, 0xadfb80, 0xc10048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
>>> 0xade3a0, ...)
>>> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
>>> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc18180, 0x4ee3a5,
>>> 0x3, 0xc61be8, 0xc61af8, 0x0, 0x0, 0xc61a00, 0x0, 0x0, ...)
>>> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
>>> main.main()
>>> golang/bindtests/bindtests.go:77 +0x151e
>>> exit status 2
>>> FAIL run-bindtests (exit status: 1)
>
>
> What distro / go version do you see this on, as I can't reproduce
> this pointer problem with a standalone demo app ?
Stock Fedora 34:
golang-bin-1.16.6-2.fc34.x86_64
It's reproduced reliably by
$ make -C golang V=1 check
in libguestfs.
Interestingly, this kind of "stringlist" is used in two distinct
scenarios in "golang/bindtests/bindtests.go". The first one is when such
a stringlist is passed to the generated Internal_test() function
[golang/src/libguestfs.org/guestfs/guestfs.go] through the "strlist"
parameter. There are no problems then. But when a similar object is
embedded in the guestfs.OptargsInternal_test argument, that's when
things break.
>> It turns out that a C-language array containing C-language pointers can
>> only be allocated in C:
>>
>> https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
>>
>> Rewrite the "arg_string_list" and "free_string_list" functions almost
>> entirely in C.
>>
>> Signed-off-by: Laszlo Ersek
>> ---
>> generator/golang.ml | 46 -
>> 1 file changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/generator/golang.ml b/generator/golang.ml
>> index d328abe4ed08..5db478e8a494 100644
>> --- a/generator/golang.ml
>> +++ b/generator/golang.ml
>> @@ -52,6 +52,39 @@ _go_guestfs_create_flags (unsigned flags)
>> {
>> return guestfs_create_flags (flags);
>> }
>> +
>> +//
>> +// A C-language array of C-language pointers needs to be allocated in C.
>> +//
>> https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
>> +//
>> +typedef char *pChar;
>> +
>> +pChar *allocStringArray (size_t nmemb)
>> +{
>> +pChar *array;
>> +
>> +array = malloc (sizeof (pChar) * (nmemb + 1));
>> +array[nmemb] = NULL;
>> +return array;
>> +}
>> +
>> +void storeString (pChar *array, size_t idx, pChar string)
>> +{
>> +array[idx] = string;
>> +}
>> +
>> +void freeStringArray (pChar *array)
>> +{
>> +pChar *position;
>> +pChar string;
>> +
>> +position = array;
>> +while ((string = *position) != NULL)
Re: [Libguestfs] [virt-v2v PATCH] build: fix typo in "--enable-werror" help string
On 10/12/21 19:54, Richard W.M. Jones wrote: > On Tue, Oct 12, 2021 at 06:08:24PM +0200, Laszlo Ersek wrote: >> Fixes: af9251086030886580cd5243afabf726d5e50fdc >> Signed-off-by: Laszlo Ersek >> --- >> m4/guestfs-c.m4 | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/m4/guestfs-c.m4 b/m4/guestfs-c.m4 >> index 6b417c091c91..67a7f276df74 100644 >> --- a/m4/guestfs-c.m4 >> +++ b/m4/guestfs-c.m4 >> @@ -30,7 +30,7 @@ test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI >> compliant]) >> AM_PROG_CC_C_O >> >> AC_ARG_ENABLE([werror], >> -[AS_HELP_STRING([--enable-error], >> +[AS_HELP_STRING([--enable-werror], >> [turn on lots of GCC warnings (for developers)])], >> [case $enableval in >>yes|no) ;; > > ACK Commit 7ce5df273be3. Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 2/2] virt-inspector.rng: Add support for Kylin (RHBZ#1995391).
Similar-to: b8bc491ff59cc1cc24a1935be99cee0c5edfb5be Signed-off-by: Laszlo Ersek --- inspector/virt-inspector.rng | 1 + 1 file changed, 1 insertion(+) diff --git a/inspector/virt-inspector.rng b/inspector/virt-inspector.rng index 0a81538e7c49..5b460b364593 100644 --- a/inspector/virt-inspector.rng +++ b/inspector/virt-inspector.rng @@ -88,6 +88,7 @@ frugalware gentoo kalilinux +kylin linuxmint mageia mandriva -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH] daemon: inspection: Add support for Kylin (RHBZ#1995391).
Similar-to: cd08039d2427b584237265237c713d8cf46536a0
Signed-off-by: Laszlo Ersek
---
daemon/inspect_fs.ml| 2 ++
daemon/inspect_fs_unix.ml | 1 +
daemon/inspect_types.ml | 2 ++
daemon/inspect_types.mli| 1 +
generator/actions_inspection.ml | 4
5 files changed, 10 insertions(+)
diff --git a/daemon/inspect_fs.ml b/daemon/inspect_fs.ml
index 02b5a0470930..77f0f6aea6dc 100644
--- a/daemon/inspect_fs.ml
+++ b/daemon/inspect_fs.ml
@@ -275,6 +275,7 @@ and check_package_format { distro } =
Some PACKAGE_FORMAT_RPM
| Some DISTRO_DEBIAN
| Some DISTRO_KALI_LINUX
+ | Some DISTRO_KYLIN (* supposedly another Ubuntu derivative *)
| Some DISTRO_LINUX_MINT
| Some DISTRO_UBUNTU ->
Some PACKAGE_FORMAT_DEB
@@ -345,6 +346,7 @@ and check_package_management { distro; version } =
| Some DISTRO_ALTLINUX
| Some DISTRO_DEBIAN
| Some DISTRO_KALI_LINUX
+ | Some DISTRO_KYLIN (* supposedly another Ubuntu derivative *)
| Some DISTRO_LINUX_MINT
| Some DISTRO_UBUNTU ->
Some PACKAGE_MANAGEMENT_APT
diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
index 652bacc0fc18..7f6eb92e93c0 100644
--- a/daemon/inspect_fs_unix.ml
+++ b/daemon/inspect_fs_unix.ml
@@ -146,6 +146,7 @@ and distro_of_os_release_id = function
| "frugalware" -> Some DISTRO_FRUGALWARE
| "gentoo" -> Some DISTRO_GENTOO
| "kali" -> Some DISTRO_KALI_LINUX
+ | "kylin" -> Some DISTRO_KYLIN
| "mageia" -> Some DISTRO_MAGEIA
| "neokylin" -> Some DISTRO_NEOKYLIN
| "openmandriva" -> Some DISTRO_OPENMANDRIVA
diff --git a/daemon/inspect_types.ml b/daemon/inspect_types.ml
index 18e410ce0309..e2bc7165c283 100644
--- a/daemon/inspect_types.ml
+++ b/daemon/inspect_types.ml
@@ -79,6 +79,7 @@ and distro =
| DISTRO_FRUGALWARE
| DISTRO_GENTOO
| DISTRO_KALI_LINUX
+ | DISTRO_KYLIN
| DISTRO_LINUX_MINT
| DISTRO_MAGEIA
| DISTRO_MANDRIVA
@@ -211,6 +212,7 @@ and string_of_distro = function
| DISTRO_FRUGALWARE -> "frugalware"
| DISTRO_GENTOO -> "gentoo"
| DISTRO_KALI_LINUX -> "kalilinux"
+ | DISTRO_KYLIN -> "kylin"
| DISTRO_LINUX_MINT -> "linuxmint"
| DISTRO_MAGEIA -> "mageia"
| DISTRO_MANDRIVA -> "mandriva"
diff --git a/daemon/inspect_types.mli b/daemon/inspect_types.mli
index d12f7a61aa99..43c79818ff23 100644
--- a/daemon/inspect_types.mli
+++ b/daemon/inspect_types.mli
@@ -86,6 +86,7 @@ and distro =
| DISTRO_FRUGALWARE
| DISTRO_GENTOO
| DISTRO_KALI_LINUX
+ | DISTRO_KYLIN
| DISTRO_LINUX_MINT
| DISTRO_MAGEIA
| DISTRO_MANDRIVA
diff --git a/generator/actions_inspection.ml b/generator/actions_inspection.ml
index 690afd460be4..0c6d39b43786 100644
--- a/generator/actions_inspection.ml
+++ b/generator/actions_inspection.ml
@@ -214,6 +214,10 @@ Gentoo.
Kali Linux.
+=item \"kylin\"
+
+Kylin.
+
=item \"linuxmint\"
Linux Mint.
base-commit: e597fc5317e018c259c75eb475cf6668e07236d1
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 0/2] refresh "virt-inspector.rng", recognize kylin
These patches (for guestfs-tools) are not strictly necessary for solving RHBZ#1995391. However, "virt-inspector.rng" is supposed to validate virt-inspector output, and so, when virt-inspector learns about a new distro (via libguestfs) and its output broadens, we might want to refresh "virt-inspector.rng" too. Thanks, Laszlo Laszlo Ersek (2): virt-inspector.rng: recognize "kalilinux" and "msdos" distros virt-inspector.rng: Add support for Kylin (RHBZ#1995391). inspector/virt-inspector.rng | 3 +++ 1 file changed, 3 insertions(+) base-commit: a4930f5fad82e5358d565b8cf3610970e9646259 -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [guestfs-tools PATCH 1/2] virt-inspector.rng: recognize "kalilinux" and "msdos" distros
As of libguestfs @ e597fc5317e0, the "string_of_distro" function [daemon/inspect_types.ml] may output "kalilinux" and "msdos" beyond what "virt-inspector.rng" currently accepts. Add these distro identifiers to "virt-inspector.rng" now. Signed-off-by: Laszlo Ersek --- inspector/virt-inspector.rng | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inspector/virt-inspector.rng b/inspector/virt-inspector.rng index 7807e4d7c4e3..0a81538e7c49 100644 --- a/inspector/virt-inspector.rng +++ b/inspector/virt-inspector.rng @@ -87,10 +87,12 @@ freedos frugalware gentoo +kalilinux linuxmint mageia mandriva meego +msdos neokylin netbsd openbsd -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 0/3] clean up Yara (version) dependency
Libguestfs now depends (optionally) on Yara >= 4.0.0. Make this explicit in the build configuration and the build documentation. Thanks, Laszlo Laszlo Ersek (3): build: fix the pkg-config identifier of the (optional) Yara library build: eliminate the AC_CHECK_LIB / AC_CHECK_HEADER tests for Yara build, docs: spell out minimum version (4.0.0) for the (optional) Yara lib docs/guestfs-building.pod | 2 +- m4/guestfs-daemon.m4 | 11 ++- 2 files changed, 3 insertions(+), 10 deletions(-) base-commit: e597fc5317e018c259c75eb475cf6668e07236d1 -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 1/3] build: fix the pkg-config identifier of the (optional) Yara library
The upstream Yara project has always provided its "libyara/yara.pc.in"
file with "Name: yara" -- ever since its introduction in commit
334bd1a671ca ("Add support for pkg-config", 2015-01-08).
In spite of this, when the (optional) Yara dependency was added to
libguestfs in commit 2e24129da376 ("appliance: add yara dependency",
2017-05-02), PKG_CHECK_MODULES was invoked with [libyara] as
list-of-modules.
(That was clearly a bug. I'm unsure what Debian calls their Yara
pkg-config module, but calling it anything else than "yara" would be a
distribution bug: upstream projects provide pkg-config files specifically
so that dependent projects can find their dependencies *regardless of
distribution*.)
As a consequence, on Fedora today, the PKG_CHECK_MODULES macro always
fails, and only the AC_CHECK_LIB / AC_CHECK_HEADER branch finds Yara.
In a subsequent patch, we'll want to add a version requirement to the
PKG_CHECK_MODULES macro invocation, so at first, fix the pkg-config
identifier of Yara.
Fixes: 2e24129da37656706b5a1d3b6bc83d4c089740bb
Signed-off-by: Laszlo Ersek
---
m4/guestfs-daemon.m4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/m4/guestfs-daemon.m4 b/m4/guestfs-daemon.m4
index 1728249a5fb7..0790f8848249 100644
--- a/m4/guestfs-daemon.m4
+++ b/m4/guestfs-daemon.m4
@@ -134,7 +134,7 @@ AC_CHECK_LIB([tsk],[tsk_version_print],[
],[AC_MSG_WARN([The Sleuth Kit library (libtsk) not found])])
dnl yara library (optional)
-PKG_CHECK_MODULES([YARA], [libyara],[
+PKG_CHECK_MODULES([YARA], [yara],[
AC_SUBST([YARA_CFLAGS])
AC_SUBST([YARA_LIBS])
AC_DEFINE([HAVE_YARA],[1],[yara library found at compile time.])
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 3/3] build, docs: spell out minimum version (4.0.0) for the (optional) Yara lib
Commit e597fc5317e0 ("daemon/yara: fix undefined behavior due to Yara 4.0
API changes", 2021-10-12) prevents the daemon from using such a Yara
version that precedes 4.0.0.
If only yara < 4 is found, treat the library as absent, rather than
attempting and failing to compile the yara module of the daemon. Note the
version requirement in the documentation too.
Suggested-by: Eric Blake
Suggested-by: Richard W.M. Jones
Signed-off-by: Laszlo Ersek
---
docs/guestfs-building.pod | 2 +-
m4/guestfs-daemon.m4 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index 1f872700e367..803b89fd5ea6 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -387,7 +387,7 @@ Optional. For tab-completion of commands in bash.
Optional. Library for filesystem forensics analysis.
-=item yara
+=item yara E 4.0.0
Optional. Tool for categorizing files based on their content.
diff --git a/m4/guestfs-daemon.m4 b/m4/guestfs-daemon.m4
index 316a811d30a9..4dec4fa67995 100644
--- a/m4/guestfs-daemon.m4
+++ b/m4/guestfs-daemon.m4
@@ -134,7 +134,7 @@ AC_CHECK_LIB([tsk],[tsk_version_print],[
],[AC_MSG_WARN([The Sleuth Kit library (libtsk) not found])])
dnl yara library (optional)
-PKG_CHECK_MODULES([YARA], [yara],[
+PKG_CHECK_MODULES([YARA], [yara >= 4.0.0],[
AC_SUBST([YARA_CFLAGS])
AC_SUBST([YARA_LIBS])
AC_DEFINE([HAVE_YARA],[1],[yara library found at compile time.])
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 2/3] build: eliminate the AC_CHECK_LIB / AC_CHECK_HEADER tests for Yara
Eliminate the AC_CHECK_LIB / AC_CHECK_HEADER tests for Yara, for the following reasons: - Upstream Yara has provided a pkg-config file since 2015, so the (now-fixed) pkg-config check should always find it, without the AC_CHECK_LIB / AC_CHECK_HEADER fallback branch. - In a subsequent patch, we'll want to test for the incompatible Yara API changes described at <https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API>. That's easy to do with pkg-config, but impossible with AC_CHECK_*, without a custom test. Namely, both AC_CHECK_DECLS and AC_CHECK_TYPES appear unable to check the parameter list of a function pointer typedef (namely YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC). And writing a dedicated test for this is overkill. Signed-off-by: Laszlo Ersek --- m4/guestfs-daemon.m4 | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/m4/guestfs-daemon.m4 b/m4/guestfs-daemon.m4 index 0790f8848249..316a811d30a9 100644 --- a/m4/guestfs-daemon.m4 +++ b/m4/guestfs-daemon.m4 @@ -138,11 +138,4 @@ PKG_CHECK_MODULES([YARA], [yara],[ AC_SUBST([YARA_CFLAGS]) AC_SUBST([YARA_LIBS]) AC_DEFINE([HAVE_YARA],[1],[yara library found at compile time.]) -],[ -AC_CHECK_LIB([yara],[yr_initialize],[ -AC_CHECK_HEADER([yara.h],[ -AC_SUBST([YARA_LIBS], [-lyara]) -AC_DEFINE([HAVE_YARA], [1], [Define to 1 if Yara library is available.]) -], []) -],[AC_MSG_WARN([Yara library not found])]) -]) +],[AC_MSG_WARN([Yara library not found])]) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
On 10/12/21 17:27, Richard W.M. Jones wrote:
> On Tue, Oct 12, 2021 at 09:16:10AM -0500, Eric Blake wrote:
>> On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
>>> The prototype of yara_rules_callback() is:
>>>
>>>> static int
>>>> yara_rules_callback (int code, void *message, void *data)
>>>
>>> however, in Yara commit 2b121b166d25 ("Track string matches using
>>> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
>>> release, the rules callback prototype was changed as follows:
>>>
>>>> diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
>>>> index cad095cd70c2..f415033c4aa6 100644
>>>> --- a/libyara/include/yara/types.h
>>>> +++ b/libyara/include/yara/types.h
>>>> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>>>>
>>>>
>>>> typedef int (*YR_CALLBACK_FUNC)(
>>>> +YR_SCAN_CONTEXT* context,
>>>> int message,
>>>> void* message_data,
>>>> void* user_data);
>>
>> Do we intend to compile against both older and newer versions of Yara,
>> in which case we'd need a configure-time probe of which variant we
>> must compile against? I could not quickly find documentation of a
>> minimum version of Yara that we are willing to support, at least not
>> in README or HACKING.
>
> FWIW as Yara is a very niche feature for the digital forensics people
> I'm fine with supporting only the latest or only the most convenient
> version. Good idea to document which version we support though.
I avoided spelling out a Yara version in either documentation or in
build configuration -- even if we restrict our dependency now to >= 4.0,
if the Yara authors break the public APIs again, e.g. in 5.0 or
whatever, our dependency check will be useless, and we'll only be able
to rely on the compilation failure again. I thought we could just put
pre-4.0 Yara versions to the sword with the exact same effort --
compilation failure.
Now, requiring precisely Yara 4.0 or 4.1 would be a different matter,
but that would turn into an unnecessary annoyance once upstream Yara
advanced *without* breaking the APIs...
I can follow up with a patch for the Yara dependency description in the
"docs/guestfs-building.pod" file (plus a dozen *.po files I guess? not
sure), but I'm unsure what to say. "We currently support >= 4.0
versions, unless upstream has broken their public API since 4.0"?
(... And so we've arrived at interface introspection here too. :))
Thanks
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 2/3] lib/proto: suppress "may be used uninitialized" in send_file_complete()
On 10/12/21 16:03, Eric Blake wrote:
> On Tue, Oct 12, 2021 at 12:36:26AM +0200, Laszlo Ersek wrote:
>> In order to shut up the compiler, just zero-initialize the buffer --
>> that's simpler than adding diagnostics pragmas. The "maybe-uninitialized"
>> warning is otherwise very useful, so keep it globally enabled (per
>> WARN_CFLAGS / WERROR_CFLAGS).
> ...
>
>> +++ b/lib/proto.c
>> @@ -433,7 +433,7 @@ send_file_cancellation (guestfs_h *g)
>> static int
>> send_file_complete (guestfs_h *g)
>> {
>> - char buf[1];
>> + char buf[1] = { '\0' };
>>return send_file_chunk (g, 0, buf, 0);
>
> If it were me writing this, I would have done this to shave typing:
>
> char c = '\0';
> return send_file_chunk (g, 0, , 0);
>
> or even abbreviated with:
>
> char c = 0;
>
> In fact, since send_file_chunk takes a const char *, we could get away
> with:
>
>
> return send_file_chunk (g, 0, "", 0);
>
> But your way is fine, too, and we aren't in a code golf competition.
>
I wanted to be as surgical as possible; the best would be to just pass
NULL (assuming xdr_bytes() coped with that, due to size==0 -- cue the
earlier debate whether memcpy() should accept NULL with n==0, given that
NULL is not a "pointer to an object").
Thanks
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
Currently, the Yara test case ("yara/test-yara-scan.sh") fails, with the
following obscure error message:
> > yara-scan /text.txt
> libguestfs: error: deserialise_yara_detection_list:
Namely, the Yara rule match list serialization / de-serialization, between
the daemon and the library, is broken. It is caused by the following
incompatible pointer passing (i.e., undefined behavior), in function
do_internal_yara_scan(), file "daemon/yara.c":
> r = yr_rules_scan_fd (rules, fd, 0, yara_rules_callback, (void *) path, 0);
^^^
The prototype of yara_rules_callback() is:
> static int
> yara_rules_callback (int code, void *message, void *data)
however, in Yara commit 2b121b166d25 ("Track string matches using
YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
release, the rules callback prototype was changed as follows:
> diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
> index cad095cd70c2..f415033c4aa6 100644
> --- a/libyara/include/yara/types.h
> +++ b/libyara/include/yara/types.h
> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>
>
> typedef int (*YR_CALLBACK_FUNC)(
> +YR_SCAN_CONTEXT* context,
> int message,
> void* message_data,
> void* user_data);
Therefore, the yara_rules_callback() function is entered with a mismatched
parameter list in the daemon, and so it passes garbage to
send_detection_info(), for serializing the match list.
This incompatible change was in fact documented by the Yara project:
https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#scanning-callback
Gcc too warns about the incompatible pointer type, under
"-Wincompatible-pointer-types". However, libguestfs is built without
"-Werror" by default, so the warning is easy to miss, and the bug only
manifests at runtime.
(The same problem exists for yr_compiler_set_callback() /
compile_error_callback():
https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#compiler-callback
except that this instance of the problem is not triggered by the test
case, as the rule list always compiles.)
Rather than simply fixing the parameter lists, consider the following
approach.
If Yara's YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC typedefs were not
for *pointer* types but actual *function* prototypes, then we could use
them directly in the declarations of our callback functions. Then any
future changes in the param lists would force a "conflicting types"
*compilation error* (not a warning). Illustration:
/* this is *not* a pointer type */
typedef int HELLO_FUNC (void);
/* function declarations */
static HELLO_FUNC my_hello_good;
static HELLO_FUNC my_hello_bad;
/* function definition, with explicit parameter list */
static int my_hello_good (void) { return 1; }
/* function definition with wrong param list -> compilation error */
static int my_hello_bad (int i) { return i; }
Unfortunately, given that the Yara-provided typedefs are already pointers,
we can't do this, in standard C anyway. Type derivation only allows for
array, structure, union, function, and pointer type derivation; it does
not allow "undoing" previous derivations.
However, using gcc's "typeof" keyword, the idea is possible. Given
YR_CALLBACK_FUNC, the expression
(YR_CALLBACK_FUNC)NULL
is a well-defined null pointer, and the function designator expression
*(YR_CALLBACK_FUNC)NULL
has the desired function type.
Of course, evaluating this expression would be undefined behavior, but in
the GCC extension expression
typeof (*(YR_CALLBACK_FUNC)NULL)
the operand of the "typeof" operator is never evaluated, as it does not
have a variably modified type. We can therefore use this "typeof" in the
same role as HELLO_FUNC had in the above example.
Signed-off-by: Laszlo Ersek
---
daemon/yara.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/daemon/yara.c b/daemon/yara.c
index 9e7bc9414957..e5f5b89eb6d9 100644
--- a/daemon/yara.c
+++ b/daemon/yara.c
@@ -56,11 +56,22 @@ static YR_RULES *rules = NULL;
static bool initialized = false;
static int compile_rules_file (const char *);
-static void compile_error_callback (int, const char *, int, const char *, void
*);
static void cleanup_destroy_yara_compiler (void *ptr);
-static int yara_rules_callback (int , void *, void *);
static int send_detection_info (const char *, YR_RULE *);
+/* Typedefs that effectively strip the pointer derivation from Yara's
+ * YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC types, using GCC's "typeof"
+ * extension.
+ */
+typedef typeof (*(YR_CALLBACK_FUNC)NULL) guestfs_yr_callback;
+typedef typeof (*(YR_COMPILER_CALLBACK_FUNC)NULL) guestfs_yr_compiler_call
[Libguestfs] [PATCH 2/3] lib/proto: suppress "may be used uninitialized" in send_file_complete()
gcc emits the following warning:
> proto.c: In function ‘send_file_complete’:
> proto.c:437:10: error: ‘buf’ may be used uninitialized
> [-Werror=maybe-uninitialized]
> 437 | return send_file_chunk (g, 0, buf, 0);
> | ^~
In theory, passing the 1-byte array "buf", with indeterminate contents, to
xdr_bytes() ultimately, could be fine -- assuming xdr_bytes() never reads
the contents of the buffer, due to the buffer size being zero. However,
the xdr_bytes() manual does not seem to guarantee this (it also does not
explicitly permit passing a NULL buffer alongside size=0, which would be
even simpler for the caller).
In order to shut up the compiler, just zero-initialize the buffer --
that's simpler than adding diagnostics pragmas. The "maybe-uninitialized"
warning is otherwise very useful, so keep it globally enabled (per
WARN_CFLAGS / WERROR_CFLAGS).
Signed-off-by: Laszlo Ersek
---
lib/proto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/proto.c b/lib/proto.c
index 3976e98b56d0..f798ece05e32 100644
--- a/lib/proto.c
+++ b/lib/proto.c
@@ -433,7 +433,7 @@ send_file_cancellation (guestfs_h *g)
static int
send_file_complete (guestfs_h *g)
{
- char buf[1];
+ char buf[1] = { '\0' };
return send_file_chunk (g, 0, buf, 0);
}
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 1/3] build: fix typo in "--enable-werror" help string
While <https://libguestfs.org/guestfs-building.1.html> correctly documents the "--enable-werror" option, the "./configure" help text itself doesn't. Replace "--enable-error" with "--enable-werror" now. Fixes: 0f54df53d26e4c293871fb30bce88511e1d61d6c Signed-off-by: Laszlo Ersek --- m4/guestfs-c.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m4/guestfs-c.m4 b/m4/guestfs-c.m4 index 08fd4e1b5067..95eddb231957 100644 --- a/m4/guestfs-c.m4 +++ b/m4/guestfs-c.m4 @@ -30,7 +30,7 @@ test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant]) AM_PROG_CC_C_O AC_ARG_ENABLE([werror], -[AS_HELP_STRING([--enable-error], +[AS_HELP_STRING([--enable-werror], [turn on lots of GCC warnings (for developers)])], [case $enableval in yes|no) ;; -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH 0/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
I *almost* got around looking at RHBZ#1995391 again, but then I pulled master to build a new baseline, and then: - the Yara test case failed (I had recently installed the Yara devel package), - it turns out the test case failure comes from a genuine bug, - it turns out that gcc warns about the bug, but I don't notice gcc warnings unless I ask for "-Werror" (I don't really "watch" the build), - it turns out the "./configure --help" hint on "-Werror" is inexact :) Thanks, Laszlo Laszlo Ersek (3): build: fix typo in "--enable-werror" help string lib/proto: suppress "may be used uninitialized" in send_file_complete() daemon/yara: fix undefined behavior due to Yara 4.0 API changes daemon/yara.c | 20 lib/proto.c | 2 +- m4/guestfs-c.m4 | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
On 10/12/21 12:04, Richard W.M. Jones wrote:
>
> Thanks - ACK series.
Thanks! Merged as commit range 63c9cd933af7..e597fc5317e0.
> It may be that the bad help string ("error" instead of "werror") is
> present in other projects since I just copied the configure.ac when
> splitting libguestfs up.
I've found it now in one other project: virt-v2v, from commit
af9251086030 ("build: Remove gnulib submodule.", 2021-04-14). I'll send
a patch.
Thanks
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
On 10/12/21 00:36, Laszlo Ersek wrote:
> Currently, the Yara test case ("yara/test-yara-scan.sh") fails, with the
> following obscure error message:
>
>>> yara-scan /text.txt
>> libguestfs: error: deserialise_yara_detection_list:
>
> Namely, the Yara rule match list serialization / de-serialization, between
> the daemon and the library, is broken. It is caused by the following
> incompatible pointer passing (i.e., undefined behavior), in function
> do_internal_yara_scan(), file "daemon/yara.c":
>
>> r = yr_rules_scan_fd (rules, fd, 0, yara_rules_callback, (void *) path, 0);
> ^^^
>
> The prototype of yara_rules_callback() is:
>
>> static int
>> yara_rules_callback (int code, void *message, void *data)
>
> however, in Yara commit 2b121b166d25 ("Track string matches using
> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
> release, the rules callback prototype was changed as follows:
>
>> diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
>> index cad095cd70c2..f415033c4aa6 100644
>> --- a/libyara/include/yara/types.h
>> +++ b/libyara/include/yara/types.h
>> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>>
>>
>> typedef int (*YR_CALLBACK_FUNC)(
>> +YR_SCAN_CONTEXT* context,
>> int message,
>> void* message_data,
>> void* user_data);
>
> Therefore, the yara_rules_callback() function is entered with a mismatched
> parameter list in the daemon, and so it passes garbage to
> send_detection_info(), for serializing the match list.
>
> This incompatible change was in fact documented by the Yara project:
>
>
> https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#scanning-callback
>
> Gcc too warns about the incompatible pointer type, under
> "-Wincompatible-pointer-types". However, libguestfs is built without
> "-Werror" by default, so the warning is easy to miss, and the bug only
> manifests at runtime.
>
> (The same problem exists for yr_compiler_set_callback() /
> compile_error_callback():
>
>
> https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#compiler-callback
>
> except that this instance of the problem is not triggered by the test
> case, as the rule list always compiles.)
>
> Rather than simply fixing the parameter lists, consider the following
> approach.
>
> If Yara's YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC typedefs were not
> for *pointer* types but actual *function* prototypes, then we could use
> them directly in the declarations of our callback functions. Then any
> future changes in the param lists would force a "conflicting types"
> *compilation error* (not a warning). Illustration:
>
> /* this is *not* a pointer type */
> typedef int HELLO_FUNC (void);
>
> /* function declarations */
> static HELLO_FUNC my_hello_good;
> static HELLO_FUNC my_hello_bad;
>
> /* function definition, with explicit parameter list */
> static int my_hello_good (void) { return 1; }
>
> /* function definition with wrong param list -> compilation error */
> static int my_hello_bad (int i) { return i; }
>
> Unfortunately, given that the Yara-provided typedefs are already pointers,
> we can't do this, in standard C anyway. Type derivation only allows for
> array, structure, union, function, and pointer type derivation; it does
> not allow "undoing" previous derivations.
>
> However, using gcc's "typeof" keyword, the idea is possible. Given
> YR_CALLBACK_FUNC, the expression
>
> (YR_CALLBACK_FUNC)NULL
>
> is a well-defined null pointer, and the function designator expression
>
> *(YR_CALLBACK_FUNC)NULL
>
> has the desired function type.
>
> Of course, evaluating this expression would be undefined behavior, but in
> the GCC extension expression
>
> typeof (*(YR_CALLBACK_FUNC)NULL)
>
> the operand of the "typeof" operator is never evaluated, as it does not
> have a variably modified type. We can therefore use this "typeof" in the
> same role as HELLO_FUNC had in the above example.
>
> Signed-off-by: Laszlo Ersek
> ---
> daemon/yara.c | 20
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/daemon/yara.c b/daemon/yara.c
> index 9e7bc9414957..e5f5b89eb6d9 100644
> --- a/daemon/yara.c
> +++ b/daemon/yara.c
> @@ -56,11 +56,22 @@ static YR_RULES *rules = NULL;
> static bool initialized = false;
>
> static int compile_rul
[Libguestfs] [virt-v2v PATCH] build: fix typo in "--enable-werror" help string
Fixes: af9251086030886580cd5243afabf726d5e50fdc Signed-off-by: Laszlo Ersek --- m4/guestfs-c.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m4/guestfs-c.m4 b/m4/guestfs-c.m4 index 6b417c091c91..67a7f276df74 100644 --- a/m4/guestfs-c.m4 +++ b/m4/guestfs-c.m4 @@ -30,7 +30,7 @@ test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant]) AM_PROG_CC_C_O AC_ARG_ENABLE([werror], -[AS_HELP_STRING([--enable-error], +[AS_HELP_STRING([--enable-werror], [turn on lots of GCC warnings (for developers)])], [case $enableval in yes|no) ;; -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH wave 1] lib/types: remove "source video: QXL" constructor
* Background:
We intend to stop virt-v2v from outputting converted domains with a QXL
display device [1]. The reason is that QXL is a complex device model with
a large security attack surface, while at the same time, better device
models exist with regard to either guest compatibility, or graphics
acceleration. Namely, if compatibility takes priority, standard VGA is at
least as widely supported by guests [2] [3] [4], with lesser security
risks. And if graphics performance takes priority, then 2D acceleration is
generally considered obsolete, and virtio-vga is where new 3D features are
being developed [5].
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c1
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c2
[3] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#qxl-vga
[4] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA
[5] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#virtio-vga
For virt-v2v in particular, compatibility is what matters, so standard VGA
will ultimately supplant QXL in newly converted domains.
* About this patch:
In order to get the ball rolling, remove the "Source_QXL" constructor of
the "source_video" union type.
"Source_QXL" was originally introduced in commit [6], and then put to use
in commit [7], specifically for the sake of in-place conversions, so that
the "input" QXL display device serve as a distinguished hint for the
"output" domain (after conversion).
With the modularization of virt-v2v in commit [8], in-place conversion has
been (temporarily) removed. Therefore, "Source_QXL" is functionally dead
code, at the time of writing.
[6] 04af2bc3cd2c ("v2v: collect source network and video adapter types",
2016-03-24)
[7] 3c21ad036296 ("v2v: in-place: request caps based on source config",
2016-03-24)
[8] 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07)
"Functionally dead" means that, mentions of the QXL display device in the
*source* domain descriptions of the test suite can be satisfied by the
parametric "Source_other_video of string" constructor just as well; there
is no actual functionality attached to "Source_QXL" at the moment. And
once we reimplement in-place conversion for modular virt-v2v, we'll still
want to flip domains that use QXL to standard VGA (see the background
above).
(Further patches are expected for RHBZ#1961107.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107
Signed-off-by: Laszlo Ersek
---
lib/types.ml | 4 +---
lib/types.mli | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/types.ml b/lib/types.ml
index 27f488255fd7..72d10d1ec774 100644
--- a/lib/types.ml
+++ b/lib/types.ml
@@ -90,7 +90,7 @@ and s_display_listen =
| LNone
and source_video = Source_other_video of string |
- Source_Cirrus | Source_QXL
+ Source_Cirrus
and source_sound = {
s_sound_model : source_sound_model;
@@ -260,12 +260,10 @@ and string_of_source_display { s_display_type = typ;
)
and string_of_source_video = function
- | Source_QXL -> "qxl"
| Source_Cirrus -> "cirrus"
| Source_other_video video -> video
and source_video_of_string = function
- | "qxl" -> Source_QXL
| "cirrus" -> Source_Cirrus
| video -> Source_other_video video
diff --git a/lib/types.mli b/lib/types.mli
index 2af5871fd54b..0bae445d8393 100644
--- a/lib/types.mli
+++ b/lib/types.mli
@@ -147,7 +147,7 @@ and s_display_listen =
(** Video adapter model. *)
and source_video = Source_other_video of string |
- Source_Cirrus | Source_QXL
+ Source_Cirrus
and source_sound = {
s_sound_model : source_sound_model; (** Sound model. *)
base-commit: 7ce5df273be304ec3ba87e46319cf78275eb4990
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [virt-v2v PATCH wave 1] lib/types: remove "source video: QXL" constructor
On 10/20/21 18:46, Richard W.M. Jones wrote:
> On Wed, Oct 20, 2021 at 06:35:03PM +0200, Laszlo Ersek wrote:
>> * Background:
>>
>> We intend to stop virt-v2v from outputting converted domains with a QXL
>> display device [1]. The reason is that QXL is a complex device model with
>> a large security attack surface, while at the same time, better device
>> models exist with regard to either guest compatibility, or graphics
>> acceleration. Namely, if compatibility takes priority, standard VGA is at
>> least as widely supported by guests [2] [3] [4], with lesser security
>> risks. And if graphics performance takes priority, then 2D acceleration is
>> generally considered obsolete, and virtio-vga is where new 3D features are
>> being developed [5].
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c1
>> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1961107#c2
>> [3] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#qxl-vga
>> [4] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#VGA
>> [5] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#virtio-vga
>>
>> For virt-v2v in particular, compatibility is what matters, so standard VGA
>> will ultimately supplant QXL in newly converted domains.
>>
>> * About this patch:
>>
>> In order to get the ball rolling, remove the "Source_QXL" constructor of
>> the "source_video" union type.
>>
>> "Source_QXL" was originally introduced in commit [6], and then put to use
>> in commit [7], specifically for the sake of in-place conversions, so that
>> the "input" QXL display device serve as a distinguished hint for the
>> "output" domain (after conversion).
>>
>> With the modularization of virt-v2v in commit [8], in-place conversion has
>> been (temporarily) removed. Therefore, "Source_QXL" is functionally dead
>> code, at the time of writing.
>>
>> [6] 04af2bc3cd2c ("v2v: collect source network and video adapter types",
>> 2016-03-24)
>> [7] 3c21ad036296 ("v2v: in-place: request caps based on source config",
>> 2016-03-24)
>> [8] 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07)
>>
>> "Functionally dead" means that, mentions of the QXL display device in the
>> *source* domain descriptions of the test suite can be satisfied by the
>> parametric "Source_other_video of string" constructor just as well; there
>> is no actual functionality attached to "Source_QXL" at the moment. And
>> once we reimplement in-place conversion for modular virt-v2v, we'll still
>> want to flip domains that use QXL to standard VGA (see the background
>> above).
>>
>> (Further patches are expected for RHBZ#1961107.)
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1961107
>> Signed-off-by: Laszlo Ersek
>> ---
>> lib/types.ml | 4 +---
>> lib/types.mli | 2 +-
>> 2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/types.ml b/lib/types.ml
>> index 27f488255fd7..72d10d1ec774 100644
>> --- a/lib/types.ml
>> +++ b/lib/types.ml
>> @@ -90,7 +90,7 @@ and s_display_listen =
>>| LNone
>>
>> and source_video = Source_other_video of string |
>> - Source_Cirrus | Source_QXL
>> + Source_Cirrus
>>
>> and source_sound = {
>>s_sound_model : source_sound_model;
>> @@ -260,12 +260,10 @@ and string_of_source_display { s_display_type = typ;
>> )
>>
>> and string_of_source_video = function
>> - | Source_QXL -> "qxl"
>>| Source_Cirrus -> "cirrus"
>>| Source_other_video video -> video
>>
>> and source_video_of_string = function
>> - | "qxl" -> Source_QXL
>>| "cirrus" -> Source_Cirrus
>>| video -> Source_other_video video
>>
>> diff --git a/lib/types.mli b/lib/types.mli
>> index 2af5871fd54b..0bae445d8393 100644
>> --- a/lib/types.mli
>> +++ b/lib/types.mli
>> @@ -147,7 +147,7 @@ and s_display_listen =
>>
>> (** Video adapter model. *)
>> and source_video = Source_other_video of string |
>> - Source_Cirrus | Source_QXL
>> + Source_Cirrus
>>
>> and source_sound = {
>>s_sound_model : source_sound_model; (** Sound model. *)
>>
>> base-commit: 7ce5df273be304ec3ba87e46319cf78275eb4990
>> --
>
> A neutral change that doesn't affect current virt-v2v at all, and
> won't affect in-place conversions in future because we won't want to
> install the QXL driver for the reasons outlined in the commit message.
> Therefore:
>
> ACK
Commit 17e7cbffb9e5.
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] curl: Add effective-url flag
On 10/13/21 12:06, Richard W.M. Jones wrote: > This commit adds a new command line option (effective-url=true) which > causes nbdkit-curl-plugin the first time it fetches a URL to update > its internal 'url' variable with the CURLINFO_EFFECTIVE_URL. That > means, the URL after all redirects have been done. Further > connections will be done using this post-redirect URL, ensuring that > those connections are stable. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2013000 > --- > plugins/curl/nbdkit-curl-plugin.pod | 35 > plugins/curl/curl.c | 42 ++--- > 2 files changed, 73 insertions(+), 4 deletions(-) > > diff --git a/plugins/curl/nbdkit-curl-plugin.pod > b/plugins/curl/nbdkit-curl-plugin.pod > index c7acf6225..673f1d327 100644 > --- a/plugins/curl/nbdkit-curl-plugin.pod > +++ b/plugins/curl/nbdkit-curl-plugin.pod > @@ -126,6 +126,40 @@ Run
Re: [Libguestfs] [PATCH nbdkit] curl: Add effective-url flag
On 10/13/21 12:06, Richard W.M. Jones wrote: > Probably best to read this first: > https://bugzilla.redhat.com/show_bug.cgi?id=2013000 > > This adds an effective-url=true|false flag to nbdkit-curl-plugin. If > true, the first time we fetch the URL, we fetch the "effective" URL > (ie. the URL after all redirects were resolved) and use that for all > future connections within the current nbdkit instance. > > The idea behind this patch is to do with the Fedora mirror system > which is flakey. Some mirrors return errors or 404s. And the mirror > system will give you a new redirect each time (within your > geographical region). This results in transfers sometimes failing > because a single read went to a bad mirror. > > After implementing this patch I'm not very happy with it. It is > incomplete because we pass the original URL to cookie-script and > header-script, so plugin/curl/scripts.c will also need to be modified. > Once those modifications are done the change becomes quite invasive. > Also I'm not convinced that it really solves any problem. In the > manual change I wrote: > > Note use of this feature in long-lived nbdkit instances can cause > subtle problems: > > • The effective URL persists across connections for the lifetime > of the nbdkit instance. If nbdkit is used for a long time then > it is possible for the redirected URL to become stale. > > • It will defeat some mirror load-balancing techniques. > > • If the mirror service sometimes redirects to a broken URL and > it happens that the URL you fetch first is broken then nbdkit > will no longer recover on subsequent connections (instead you > will need to restart nbdkit). > > I suggested another way to solve this by using curl APIs to fetch the > effective URL up front and passing that URL to nbdkit (see > https://bugzilla.redhat.com/show_bug.cgi?id=2013000#c1), but > apparently that solution isn't acceptable for unclear reasons. > > I can't think of any other way to solve this in the context of nbdkit > (maybe have it detect when redirection is happening and retry the > redirection on error?). So here's the patch. Given that I've been CC'd, here's my opinion: I strongly dislike transparent mirror selection (redirects) as a principle (based in experience). Precisely with the Fedora mirror system, I frequently see "dnf update" download some packages at lightning speed, then get stuck *completely* at some other package (potantially after spewing a bunch of "broken mirror" messages at me), so that I have to Ctrl-C the whole command, and re-issue it. Transparent redirects seem to want to hide the "mess" from the user, but they fail to do that quite frequently, IMO. The Cygwin experience is better, IMO. When you start the Cygwin package installer (whether you do it for initial installation or for updating packages), a fresh list of mirrors is fetched from some central location (I think?), but then you, the user, have to pick a *specific* mirror from that list. I always just go for fsn.hu, which I know to be a rock-stable mirror (for many OS distros, including Fedora) in my location. No bad surprises using that mirror, ever. With that in mind, I wouldn't complicate *any* application to deal with redirects / mirror selection transparently. Whatever the application does, the user will not be happy, and will want to tweak the logic. Just let the user pick an effective URL themselves, and stick with that forever. This may not be great for "load balancing", but AIUI the pain point here is failed *individual* imports. I think it should be OK to stick with a particular fixed URL for the duration of an import. (I should actually update my DNF repo files on Fedora to use fsn.hu as well, I just always get discouraged by the "metalink" stuff in there, and the hard-to-read variables (such as "$releasever", "$basearch", ...).) >From Alexander's description, it's clear that the reliability of the mirror network is the core issue here, and we're now pushing around the unwanted job of hiding it from the user, from one application to the other. I'd say let the *user* deal with it, *once*, in their configuration, and neither application (= neither nbdkit nor the app that starts nbdkit) should struggle with redirects. In particular, tolerating (following) redirects per every single Range request looks incredibly inefficient to me (not to mention, brittle). (Sorry if my opinion is too naive.) Thanks Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit v2 2/2] tests: Test retry-request + curl in a realistic mirroring situation
tart nbdkit. */
> + if (asprintf (_param, "unix-socket-path=%s", sockpath) == -1) {
> +perror ("asprintf");
> +exit (EXIT_FAILURE);
Is exit() safe here (with the web server running in another thread)?
Furthermore, should we do anything for (e.g.) deleting the pathname of
the unix domain socket? (I don't think it matters much; I'm asking this
is mostly for my own education.) And then that would apply more or less
to every error exit after this point.
> + }
> + if (test_start_nbdkit ("--filter=retry-request",
> + "curl", usp_param, "http://localhost/mirror;,
> + "retry-request-delay=1",
> + NULL) == -1)
> +exit (EXIT_FAILURE);
> +
> + nbd = nbd_create ();
> + if (nbd == NULL) {
> + nbd_error:
> +fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
> +exit (EXIT_FAILURE);
> + }
Can we put this block of code at the very end of the function, and here,
just say "goto nbd_error"?
Because right now we have gotos jumping backward, which I find awkward
even for (otherwise justified) "algorithmic reasons", and very awkward
for a usual error path.
... Either way:
Acked-by: Laszlo Ersek
Thanks!
Laszlo
> +
> + if (nbd_connect_unix (nbd, sock /* NBD socket */) == -1)
> +goto nbd_error;
> +
> + /* The way the test works is we fetch the magic "/mirror" path (see
> + * web-server.c). This redirects to /mirror1, /mirror2, /mirror3
> + * round robin on each request. /mirror1 returns all 1's, /mirror2
> + * returns all 2's, and /mirror3 returns a 404 error. The 404 error
> + * should be transparently skipped by the filter, so we should see
> + * alternating 1's and 2's buffers.
> + */
> + if (nbd_pread (nbd, buf, sizeof buf, 0, 0) == -1)
> +goto nbd_error;
> +
> + if (buf[0] == 1 && buf[1] == 1 && buf[511] == 1)
> +state = 2;
> + else if (buf[0] == 2 && buf[1] == 2 && buf[511] == 2)
> +state = 1;
> + else {
> +fprintf (stderr, "%s: unexpected start state\n", argv[0]);
> +exit (EXIT_FAILURE);
> + }
> +
> + for (i = 0; i < 10; ++i) {
> +if (nbd_pread (nbd, buf, sizeof buf, 0, 0) == -1)
> + goto nbd_error;
> +if (buf[0] != state || buf[1] != state || buf[511] != state) {
> + fprintf (stderr, "%s: unexpected state\n", argv[0]);
> + exit (EXIT_FAILURE);
> +}
> +state++;
> +if (state == 3)
> + state = 1;
> + }
> +
> + nbd_close (nbd);
> + exit (EXIT_SUCCESS);
> +}
> diff --git a/tests/web-server.c b/tests/web-server.c
> index ac44d16b2..07a2ec398 100644
> --- a/tests/web-server.c
> +++ b/tests/web-server.c
> @@ -57,6 +57,8 @@
> #define SOCK_CLOEXEC 0
> #endif
>
> +enum method { HEAD, GET };
> +
> static char tmpdir[] = "/tmp/wsXX";
> static char sockpath[] = "./sock";
> static int listen_sock = -1;
> @@ -67,7 +69,12 @@ static check_request_t check_request;
>
> static void *start_web_server (void *arg);
> static void handle_requests (int s);
> -static void handle_request (int s, bool headers_only);
> +static void handle_file_request (int s, enum method method);
> +static void handle_mirror_redirect_request (int s);
> +static void handle_mirror_data_request (int s, enum method method, char
> byte);
> +static void send_404_not_found (int s);
> +static void send_405_method_not_allowed (int s);
> +static void send_500_internal_server_error (int s);
> static void xwrite (int s, const char *buf, size_t len);
> static void xpread (char *buf, size_t count, off_t offset);
>
> @@ -177,12 +184,15 @@ start_web_server (void *arg)
> static void
> handle_requests (int s)
> {
> - size_t r, n, sz;
>bool eof = false;
>
>fprintf (stderr, "web server: accepted connection\n");
>
>while (!eof) {
> +size_t r, n, sz;
> +enum method method;
> +char path[128];
> +
> /* Read request until we see "\r\n\r\n" (end of headers) or EOF. */
> n = 0;
> for (;;) {
> @@ -214,30 +224,73 @@ handle_requests (int s)
> /* Call the optional user function to check the request. */
> if (check_request) check_request (request);
>
> -/* HEAD or GET request? */
> -if (strncmp (request, "HEAD ", 5) == 0)
> - handle_request (s, true);
> -else if (strncmp (request, "GET ", 4) == 0)
> - handle_request (s, false);
> +/* Get the method and path fields from the first line. */
> +if (strncmp (request, "HEAD ", 5) == 0) {
> + method = HEAD;
> +
Re: [Libguestfs] [PATCH nbdkit v2 1/2] New filter: nbdkit-retry-request-filter
r in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * * Neither the name of Red Hat nor the names of its contributors may be
> + * used to endorse or promote products derived from this software without
> + * specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "cleanup.h"
> +#include "windows-compat.h"
> +
> +static unsigned retries = 2;/* 0 = filter is disabled */
> +static unsigned delay = 2;
> +static bool retry_open_call = true;
> +
> +static int
> +retry_request_thread_model (void)
> +{
> + return NBDKIT_THREAD_MODEL_PARALLEL;
> +}
> +
> +static int
> +retry_request_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
> + const char *key, const char *value)
> +{
> + int r;
> +
> + if (strcmp (key, "retry-request-retries") == 0) {
> +if (nbdkit_parse_unsigned ("retry-request-retries", value, ) ==
> -1)
> + return -1;
> +if (retries > 1000) {
> + nbdkit_error ("retry-request-retries: value too large");
> + return -1;
> +}
> +return 0;
> + }
> + else if (strcmp (key, "retry-request-delay") == 0) {
> +if (nbdkit_parse_unsigned ("retry-request-delay", value, ) == -1)
> + return -1;
> +if (delay == 0) {
> + nbdkit_error ("retry-request-delay cannot be 0");
> + return -1;
> +}
> +return 0;
> + }
> + else if (strcmp (key, "retry-request-open") == 0) {
> +r = nbdkit_parse_bool (value);
> +if (r == -1)
> + return -1;
> +retry_open_call = r;
> +return 0;
> + }
> +
> + return next (nxdata, key, value);
> +}
> +
> +#define retry_request_config_help \
> + "retry-request-retries= Number of retries (default: 2).\n" \
> + "retry-request-delay= Seconds to wait before retry (default: 2).\n" \
> + "retry-request-open=false Do not retry opening the plugin (default:
> true).\n"
> +
> +/* These macros encapsulate the logic of retrying. The code between
> + * RETRY_START...RETRY_END must set r to 0 or -1 on success or
> + * failure.
> + */
> +#define RETRY_START \
> + { \
> +unsigned i; \
> +\
> +r = -1; \
> +for (i = 0; r == -1 && i <= retries; ++i) { \
> + if (i > 0) { \
> +nbdkit_debug ("retry %u: waiting %u seconds before retrying", \
> + i, delay);\
> +if (nbdkit_nanosleep (delay, 0) == -1) {\
> + if (*err == 0)\
> +*err = errno; \
> + break;\
> +} \
> + } \
> + do
> +#define RETRY_END \
> + while (0);\
> +} \
> + }
> +
> +static void *
> +retry_request_open (nbdkit_next_open *next, nbdkit_context *nxdata,
> +int readonly, con
Re: [Libguestfs] [PATCH nbdkit v2 2/2] tests: Test retry-request + curl in a realistic mirroring situation
On 10/18/21 23:30, Richard W.M. Jones wrote:
> On Mon, Oct 18, 2021 at 10:12:49PM +0100, Richard W.M. Jones wrote:
>> On Mon, Oct 18, 2021 at 08:27:36PM +0200, Laszlo Ersek wrote:
>>> On 10/15/21 16:17, Richard W.M. Jones wrote:
>>>> +#ifndef HAVE_CURLOPT_UNIX_SOCKET_PATH
>>>> + fprintf (stderr, "%s: curl does not support CURLOPT_UNIX_SOCKET_PATH\n",
>>>> + argv[0]);
>>>> + exit (77);
>>>> +#endif
>>>> +
>>>> + sockpath = web_server ("disk", NULL);
>>>> + if (sockpath == NULL) {
>>>> +fprintf (stderr, "%s: could not start web server thread\n", argv[0]);
>>>> +exit (EXIT_FAILURE);
>>>> + }
>>>> +
>>>> + /* Start nbdkit. */
>>>> + if (asprintf (_param, "unix-socket-path=%s", sockpath) == -1) {
>>>> +perror ("asprintf");
>>>> +exit (EXIT_FAILURE);
>>>
>>> Is exit() safe here (with the web server running in another thread)?
>>
>> I didn't know, but I temporarily modified the test changing #ifndef ->
>> #ifdef above and it did exit (skip) fine.
>
> Actually not a good test, because that happens before the web server
> launches. Inserting an exit later in the code works as expected.
Awesome, thanks. It's really thoughtful that web_server() "owns" the
unix domain socket, i.e., that it takes responsibility for removing the
pathname.
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/3] build, docs: spell out minimum version (4.0.0) for the (optional) Yara lib
On 10/14/21 13:54, Richard W.M. Jones wrote: > > Series looks good, thanks. > > ACK > > Rich. > Thank you both; merged as commit range e597fc5317e0..f34bd6b12f85. Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH] daemon: inspection: Add support for Kylin (RHBZ#1995391).
On 10/14/21 13:56, Richard W.M. Jones wrote: > ACK > > Thanks, > Rich. > Commit 305b02e7e74a. Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [guestfs-tools PATCH 2/2] virt-inspector.rng: Add support for Kylin (RHBZ#1995391).
On 10/14/21 13:57, Richard W.M. Jones wrote: > > ACK series > > It's kind of annoying that we need to keep this file up to date with > libguestfs (considering they are now separate projects). But here we are ... Merged as commit range a4930f5fad82..f041a5e24d8c. Thank you! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] curl: Add effective-url flag
tangent: On 10/14/21 19:12, Richard W.M. Jones wrote: > https://libguestfs.org/nbdkit-plugin.3.html#Callback-lifecycle Impressive. Both the particular diagram, and the general documentation quality in the v2v project set. Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] New filter: request-request
only mentioning things that I managed to think of in addition to Eric's comments (which is of course not to say I thought of *everything* that Eric pointed out): On 10/14/21 22:58, Richard W.M. Jones wrote: > Similar to nbdkit-retry-filter, but this only retries single failing > requests rather than reopening the whole plugin. This is intended to > be used with the curl filter to solve: > > https://bugzilla.redhat.com/show_bug.cgi?id=2013000 > --- > docs/nbdkit-captive.pod | 3 +- > .../nbdkit-retry-request-filter.pod | 67 + > filters/retry/nbdkit-retry-filter.pod | 7 +- > plugins/curl/nbdkit-curl-plugin.pod | 1 + > configure.ac | 2 + > filters/retry-request/Makefile.am | 68 ++ > filters/retry-request/retry-request.c | 229 ++ > 7 files changed, 375 insertions(+), 2 deletions(-) > > diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod > index d340b8ae0..eafe36d8b 100644 > --- a/docs/nbdkit-captive.pod > +++ b/docs/nbdkit-captive.pod > @@ -118,7 +118,8 @@ help performance: > --run 'qemu-img convert $nbd disk.img' > > If the source suffers from temporary network failures > -L may help. > +L or L may > +help. > > To overwrite a file inside an uncompressed tar file (the file being > overwritten must be the same size), use L like > diff --git a/filters/retry-request/nbdkit-retry-request-filter.pod > b/filters/retry-request/nbdkit-retry-request-filter.pod > new file mode 100644 > index 0..1216cf83b > --- /dev/null > +++ b/filters/retry-request/nbdkit-retry-request-filter.pod > @@ -0,0 +1,67 @@ > +=head1 NAME > + > +nbdkit-retry-request-filter - retry single requests on error > + > +=head1 SYNOPSIS > + > + nbdkit --filter=retry-request PLUGIN [retry-request-retries=N] > + > +=head1 DESCRIPTION > + > +C is a filter for nbdkit that > +transparently retries single requests if they fail. This is useful > +for plugins that are not completely reliable or have random behaviour. > +For example L might behave this way if pointed > +at a load balancer which sometimes redirects to web server that are > +not responsive. > + > +An alternative filter with different trade-offs is > +L. That filter is more heavyweight because it > +always reopens the whole plugin connection on failure. > + > +=head1 PARAMETERS > + > +=over 4 > + > +=item BN > + > +The number of times any single request will be retried before we give > +up and fail the operation. The default is 2. > + > +=item BN > + > +The number of seconds to wait before retrying. The default is 2 > +seconds. > + > +=back > + > +=head1 FILES > + > +=over 4 > + > +=item F<$filterdir/nbdkit-retry-request-filter.so> > + > +The filter. > + > +Use C to find the location of C<$filterdir>. > + > +=back > + > +=head1 VERSION > + > +C first appeared in nbdkit 1.30. > + > +=head1 SEE ALSO > + > +L, > +L, > +L, > +L. > + > +=head1 AUTHORS > + > +Richard W.M. Jones > + > +=head1 COPYRIGHT > + > +Copyright (C) 2019-2021 Red Hat Inc. > diff --git a/filters/retry/nbdkit-retry-filter.pod > b/filters/retry/nbdkit-retry-filter.pod > index fee4b7e64..babd82bbd 100644 > --- a/filters/retry/nbdkit-retry-filter.pod > +++ b/filters/retry/nbdkit-retry-filter.pod > @@ -16,6 +16,10 @@ make long-running copy operations reliable in the presence > of > temporary network failures, without requiring any changes to the > plugin or the NBD client. > > +An alternative and more fine-grained filter is > +L which will retry only single > +requests that fail. > + > Several optional parameters are available to control: > > =over 4 > @@ -113,7 +117,8 @@ C first appeared in nbdkit 1.16. > > L, > L, > -L. > +L, > +L. > > =head1 AUTHORS > > diff --git a/plugins/curl/nbdkit-curl-plugin.pod > b/plugins/curl/nbdkit-curl-plugin.pod > index c7acf6225..07f71b389 100644 > --- a/plugins/curl/nbdkit-curl-plugin.pod > +++ b/plugins/curl/nbdkit-curl-plugin.pod > @@ -503,6 +503,7 @@ L, > L, > L, > L, > +L, > L, > L, > L, > diff --git a/configure.ac b/configure.ac > index fd15e2960..3fd8a397e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -139,6 +139,7 @@ filters="\ > rate \ > readahead \ > retry \ > +retry-request \ > stats \ > swab \ > tar \ > @@ -1342,6 +1343,7 @@ AC_CONFIG_FILES([Makefile > filters/rate/Makefile > filters/readahead/Makefile > filters/retry/Makefile > + filters/retry-request/Makefile > filters/stats/Makefile > filters/swab/Makefile > filters/tar/Makefile > diff --git a/filters/retry-request/Makefile.am > b/filters/retry-request/Makefile.am > new file mode 100644 > index 0..95fdafdcd > --- /dev/null > +++ b/filters/retry-request/Makefile.am > @@ -0,0 +1,68 @@ > +# nbdkit > +# Copyright (C) 2019-2021 Red Hat
Re: [Libguestfs] [PATCH nbdkit] New filter: request-request
On 10/14/21 23:28, Richard W.M. Jones wrote: > On Thu, Oct 14, 2021 at 04:22:27PM -0500, Eric Blake wrote: >> I know this is just an RFC, but the idea looks like it may have merit. >> Would we want to merge this functionality into the 'retry' filter (a >> single filter, with a knob on whether to retry single requests >> vs. full connection), or would that be too heavyweight for one filter? > > I did look at that, but the current retry filter is complicated enough > and this would make it quite a lot more complicated (unless you can > see some easy way to do it). > > Also the two filters do have somewhat different behaviour and perhaps > use cases. > > I really need a way to test this with the curl plugin to check that it > really does what I think it does. In particular I'm not convinced > curl will actually go through the whole redirect phase in this case, > which would mean it doesn't actually solve Alexander's problem. I wonder if one of the commonly used proxies (squid perhaps?) can be configured to inject errors / fail requests intentionally with some user-specified frequency. Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] New filter: request-request
On 10/15/21 12:52, Richard W.M. Jones wrote: > nbdkit definitely abuses macros. I guess you've not seen > https://gitlab.com/nbdkit/nbdkit/-/blob/master/common/utils/vector.h > yet. > > Or for even more excitement, > https://github.com/libguestfs/libguestfs-common/blob/master/utils/libxml2-writer-macros.h Whoa. :) > > Lots of little callback functions would annoy me TBH. How about > Eric's idea of having the START_RETRY / END_RETRY macros instead? Sure! (Funnily enough, in edk2, there is DEBUG_CODE(...), and DEBUG_CODE_BEGIN + DEBUG_CODE_END :)) Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/3] daemon/yara: fix undefined behavior due to Yara 4.0 API changes
On 10/12/21 17:27, Richard W.M. Jones wrote:
> On Tue, Oct 12, 2021 at 09:16:10AM -0500, Eric Blake wrote:
>> On Tue, Oct 12, 2021 at 12:36:27AM +0200, Laszlo Ersek wrote:
>>> The prototype of yara_rules_callback() is:
>>>
>>>> static int
>>>> yara_rules_callback (int code, void *message, void *data)
>>>
>>> however, in Yara commit 2b121b166d25 ("Track string matches using
>>> YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
>>> release, the rules callback prototype was changed as follows:
>>>
>>>> diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
>>>> index cad095cd70c2..f415033c4aa6 100644
>>>> --- a/libyara/include/yara/types.h
>>>> +++ b/libyara/include/yara/types.h
>>>> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>>>>
>>>>
>>>> typedef int (*YR_CALLBACK_FUNC)(
>>>> +YR_SCAN_CONTEXT* context,
>>>> int message,
>>>> void* message_data,
>>>> void* user_data);
>>
>> Do we intend to compile against both older and newer versions of Yara,
>> in which case we'd need a configure-time probe of which variant we
>> must compile against? I could not quickly find documentation of a
>> minimum version of Yara that we are willing to support, at least not
>> in README or HACKING.
>
> FWIW as Yara is a very niche feature for the digital forensics people
> I'm fine with supporting only the latest or only the most convenient
> version. Good idea to document which version we support though.
I'd also like to clarify that I didn't ignore (part of) the feedback I
received between my posting the series and my merging it. Instead, I
simply didn't see it. I process emails in infrequent batches, in order
to improve my throughput (keep context switches as infrequent as
possible). Thus, I've not checked email between (approx.) Rich's "ACK
series" and now.
Thanks,
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [virt-v2v PATCH 2/4] test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain
On 09/27/21 10:30, Richard W.M. Jones wrote:
> On Mon, Sep 27, 2021 at 08:34:56AM +0200, Laszlo Ersek wrote:
>> Per commit ac39fa292c31 ("v2v: Set machine type explicitly for outputs
>> which support it (RHBZ#1581428).", 2020-12-04), Windows 7 guests (which
>> are "more modern" than Windows XP guests) are converted to Q35, not I440FX
>> boards.
>>
>> Per related commit d0267122e348 ("v2v: -o libvirt: Map IDE disks to
>> bus="sata" on Q35.", 2020-12-04), when a Windows 7 guest with an IDE
>> CD-ROM -- hence, an I440FX board -- is converted, the CD-ROM in the
>> converted domain will be on the SATA bus (Q35 does not have an IDE
>> controller, only a SATA controller).
>>
>> Because the Windows guest used in "test-v2v-cdrom" is "Windows 7 Phony
>> Edition", it triggers the above cascade, and currently fails with:
>>
>>> --- ./test-v2v-cdrom.expected 2021-09-03 22:51:31.026185527 +0200
>>> +++ test-v2v-cdrom.d/disks 2021-09-19 13:01:47.471745622 +0200
>>> @@ -4,5 +4,5 @@
>>>
>>>
>>>
>>> -
>>> +
>>>
>>> ./test-v2v-cdrom.sh: unexpected disk assignments
>>
>> The conversion seems correct, but the test expectation is stale. Most
>> probably, commit d0267122e348 missed updating the test data. Do it now.
>
> Actually the conversion is wrong - we need to update configuration
> inside the guest. Which is why I didn't fix this test - to remind
> myself that there's still a bug to be fixed.
Hmm, good point; now I remember that the i440fx->q35 machine type change
is basically a motherboard replacement for Windows.
So do we have to update some "windows device paths" (leading to the
CD-ROM) in the registry?
Thanks,
Laszlo
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1637857
>
> Rich.
>
>> Fixes: d0267122e348202f6fac4d833f7448409e7129c9
>> Signed-off-by: Laszlo Ersek
>> ---
>> tests/test-v2v-cdrom.expected | 2 +-
>> tests/test-v2v-cdrom.xml.in | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected
>> index 34d2bf5961b0..17bd152d8e64 100644
>> --- a/tests/test-v2v-cdrom.expected
>> +++ b/tests/test-v2v-cdrom.expected
>> @@ -4,5 +4,5 @@
>>
>>
>>
>> -
>> +
>>
>> diff --git a/tests/test-v2v-cdrom.xml.in b/tests/test-v2v-cdrom.xml.in
>> index 6bad5eab1cd4..a6e1e3f514d5 100644
>> --- a/tests/test-v2v-cdrom.xml.in
>> +++ b/tests/test-v2v-cdrom.xml.in
>> @@ -35,7 +35,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
>> MA 02110-1301 USA.
>>
>>
>> > file='@abs_top_builddir@/test-data/phony-guests/blank-disk.img'/>
>> -
>> +
>>
>>
>>
>> --
>> 2.19.1.3.g30247aa5d201
>>
>>
>> ___
>> Libguestfs mailing list
>> Libguestfs@redhat.com
>> https://listman.redhat.com/mailman/listinfo/libguestfs
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [virt-v2v PATCH 4/4] test-v2v-i-ova: spell out viosock element in output domain XML
On 09/27/21 10:32, Richard W.M. Jones wrote:
> On Mon, Sep 27, 2021 at 08:34:58AM +0200, Laszlo Ersek wrote:
>> This is likely an omission from commit 05f780c16f01 ("v2v: support
>> configuration of viosock driver", 2021-02-26) -- if the converted Windows
>> guest does not dispose over the virtio-sock guest drivers, an explicit
>> "none"-model element is now generated in the domain XML.
>>
>> Fixes: 05f780c16f0135c657615520c2245b42de1efc3e
>> Signed-off-by: Laszlo Ersek
>> ---
>> tests/test-v2v-i-ova.xml | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml
>> index a018fd44aee8..489c0ab09bcc 100644
>> --- a/tests/test-v2v-i-ova.xml
>> +++ b/tests/test-v2v-i-ova.xml
>> @@ -46,6 +46,7 @@
>>/dev/urandom
>>
>>
>> +
>>
>>
>>
>
> Patches 1 & 4 are good.
Thanks! I pushed them as commits 3767ebca30c5 and 5adf437bcc00 respectively.
I also synthesized an Acked-by in your name for each commit, from the
above "good". (If I shouldn't do that, please let me know.)
Thanks,
Laszlo
>
> Patches 2 & 3 are symptoms of the incomplete fix:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1581428
> https://bugzilla.redhat.com/show_bug.cgi?id=1637857
>
> so need some different work (although fixing the test will be part of
> the solution).
>
> Rich.
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libnbd PATCH] build: allow OCaml programs using libnbd to be compiled against build dir
On 09/27/21 10:00, Richard W.M. Jones wrote:
> On Mon, Sep 27, 2021 at 08:24:42AM +0200, Laszlo Ersek wrote:
>> Port libguestfs commit bf61bf7355d3 ("build: Allow OCaml programs using
>> libguestfs to be compiled against build dir.", 2020-03-12) to libnbd.
>>
>> This allows C+OCaml programs, such as virt-v2v, to find a just built, but
>> not installed, libnbd tree.
>>
>> Signed-off-by: Laszlo Ersek
>
> ACK
Pushed as commit bc408c8956b4.
Thanks!
Laszlo
>
> Thanks,
>
> Rich.
>
>> ocaml/Makefile.am | 17 +
>> .gitignore| 1 +
>> run.in| 2 ++
>> 3 files changed, 20 insertions(+)
>>
>> diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
>> index 1c7397a9da7a..b1ab80dd5fa5 100644
>> --- a/ocaml/Makefile.am
>> +++ b/ocaml/Makefile.am
>> @@ -161,4 +161,21 @@ install-data-hook:
>>$(data_hook_files)
>> rm $(DESTDIR)$(OCAMLLIB)/nbd/libnbdocaml.a
>>
>> +# This "tricks" ocamlfind into allowing us to compile other OCaml
>> +# programs against a locally compiled copy of the libnbd sources.
>> +# ocamlfind needs to see a directory called ‘nbd’ which contains
>> +# ‘META’. The current directory is called ‘ocaml’, but if we make
>> +# this symlink then we can create the required directory structure.
>> +#
>> +# Note if you just want to use this, make sure you use
>> +# ‘../libnbd/run make’ in your other program and everything should
>> +# just work.
>> +CLEANFILES += nbd
>> +
>> +all-local: nbd
>> +
>> +nbd:
>> +rm -f $@
>> +$(LN_S) . $@
>> +
>> endif HAVE_OCAML
>> diff --git a/.gitignore b/.gitignore
>> index 5fc596773ed4..55a5449a70e2 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -141,6 +141,7 @@ Makefile.in
>> /ocaml/examples/.depend
>> /ocaml/libnbd-ocaml.3
>> /ocaml/libnbdocaml.a
>> +/ocaml/nbd
>> /ocaml/nbd-c.c
>> /ocaml/stamp-manpages
>> /ocaml/stamp-mlnbd
>> diff --git a/run.in b/run.in
>> index 99c1f7ff0ff6..dda49228f949 100755
>> --- a/run.in
>> +++ b/run.in
>> @@ -100,6 +100,8 @@ export GODEBUG=cgocheck=2
>> # Allow dependent packages to be compiled against local libnbd.
>> prepend PKG_CONFIG_PATH "$b/lib/local"
>> export PKG_CONFIG_PATH
>> +prepend OCAMLPATH "$b/ocaml"
>> +export OCAMLPATH
>>
>> # Do we have libtool? If we have it then we can use it to make
>> # running valgrind simpler. However don't depend on it.
>> --
>> 2.19.1.3.g30247aa5d201
>>
>>
>> ___
>> Libguestfs mailing list
>> Libguestfs@redhat.com
>> https://listman.redhat.com/mailman/listinfo/libguestfs
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] translating CD-ROM device paths from i440fx to Q35 in virt-v2v (was: test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain)
(+libvirt-devel)
On 09/29/21 21:22, Richard W.M. Jones wrote:
> We currently partially install the virtio block drivers in the Windows
> guest (just enough to get the guest to boot on the target), and
> Windows itself re-installs the virtio block driver and other drivers
> it needs, and that's enough to get it to see C:
>
> As for other hard disk partitions, Windows does indeed contain a
> mapping to other drives in the Registry but IIRC it's not sensitive to
> the device driver (unlike Linux /dev/vdX vs /dev/sdX). If you're
> interested in that, see libguestfs.git/daemon/inspect_fs_windows.ml:
> get_drive_mappings. We never bothered with attempting to handle
> conversion of floppy drives or CD-ROMs for Windows.
OK. So AIUI, that means no work is needed here for Windows.
> On Linux we do better: We iterate over all the configuration files in
> /etc and change device paths. The significance of this bug is we need
> to change (eg) /dev/hdc to /dev/. The difficulty is
> working out where the device will appear on the target and not having
> it conflict with any hard disk, something we partly control (see
> virt-v2v.git/convert/target_bus_assignment.ml*)
AIUI the conflict avoidance logic ("no overlapping disks") is already in
place. The question is how to translate device paths in /etc/fstab and
similar.
Please correct me if I'm wrong: at the moment, I believe virt-v2v parses
and manipulates the following elements and attributes in the domain XML:
^^^ ^^^
My understanding is however that the target/@dev attribute is mostly
irrelevant:
https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
The dev attribute indicates the "logical" device name. The actual
device name specified is not guaranteed to map to the device name in
the guest OS. Treat it as a device ordering hint. [...]
What actually matters is the target/@bus attribute, in combination with
the sibling element . Such as:
^^^
^ ^ ^
^^^
^ ^ ^
^^^
^ ^ ^
^^^
^ ^ ^
So, target/@dev should be mostly ignored; what matters is the following
tuple:
(target/@bus, address/@controller, address/@bus, address/@unit)
Extracting just the tuples:
(ide, 0, 0, 0)
(ide, 0, 0, 1)
(ide, 0, 1, 0)
(ide, 0, 1, 1)
The first two components of each tuple -- i.e., (ide, 0) -- refer to the
following IDE controller:
^^ ^
and then the rest of the components, such as (0, 0), (0, 1), (1, 0), (1,
1), identify the disk on that IDE controller.
(Side comment: the PCI location of the (first) IDE controller is fixed
in QEMU; if one tries to change it, libvirt complains: "Primary IDE
controller must have PCI address 0:0:1.1".)
(Side comment: on the QEMU command line, this maps to
-device ide-cd,bus=ide.0,unit=0,... \
-device ide-cd,bus=ide.0,unit=1,... \
-device ide-cd,bus=ide.1,unit=0,... \
-device ide-cd,bus=ide.1,unit=1,... \
)
Inside the guest, /dev/hd* nodes don't even exist, so it's unlikely that
/etc/fstab would refer to them. /etc/fstab can however refer to symlinks
under "/dev/disk/by-id" (for example):
lrwxrwxrwx. 1 root root 9 Sep 30 11:54 ata-QEMU_DVD-ROM_QM1 -> ../../sr0
lrwxrwxrwx. 1 root root 9 Sep 30 11:54 ata-QEMU_DVD-ROM_QM2 -> ../../sr1
lrwxrwxrwx. 1 root root 9 Sep 30 11:54 ata-QEMU_DVD-ROM_QM3 -> ../../sr2
lrwxrwxrwx. 1 root root 9 Sep 30 11:54 ata-QEMU_DVD-ROM_QM4 -> ../../sr3
Furthermore, we have pseudo-files (directories) such as:
/sys/devices/pci:00/:00:01.1/ata1/host0/target0:0:0/0:0:0:0/block/sr0
/sys/devices/pci:00/:00:01.1/ata1/host0/target0:0:1/0:0:1:0/block/sr1
/sys/devices/pci:00/:00:01.1/ata2/host1/target1:0:0/1:0:0:0/block/sr2
/sys/devices/pci:00/:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sr3
^ ^
So in order to map a device path from the original guest's "/etc/fstab",
such as "/dev/disk/by-id/ata-QEMU_DVD-ROM_QM3", to the original
domain XML's element, we have to do the following in the "source"
appliance:
NODE=$(realpath /dev/disk/by-id/ata-QEMU_DVD-ROM_QM3)
# -> /dev/sr2
NODE=${NODE#/dev/}
# -> sr2
DEVPATH=$(ls -d
/sys/devices/pci:00/:00:01.1/ata?/host?/target?:0:?/?:0:?:0/block/$NODE)
# ->
/sys/devices/pci:00/:00:01.1/ata2/host1/target1:0:0/1:0:0:0/block/sr2
And then map the "1:0:0:0" pathname component from $DEVPATH to:
^^^
Re: [Libguestfs] [PATCH] daemon/inspect_fs_unix: recognize modern Pardus GNU/Linux releases
On 10/01/21 15:01, Richard W.M. Jones wrote:
> On Fri, Oct 01, 2021 at 02:53:38PM +0200, Laszlo Ersek wrote:
>> Recent Pardus releases seem to have abandoned the original
>> "/etc/pardus-release" file, which the current Pardus detection, from
>> commit 233530d3541d ("inspect: Add detection of Pardus.", 2010-10-29), is
>> based upon.
>>
>> Instead, Pardus apparently adopted the "/etc/os-release" specification
>> <https://www.freedesktop.org/software/systemd/man/os-release.html>, with
>> "ID=pardus". Extend the "distro_of_os_release_id" function accordingly.
>> Keep the original method for recognizing earlier releases.
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1993842
>> Signed-off-by: Laszlo Ersek
>> ---
>> daemon/inspect_fs_unix.ml | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
>> index 557f328333f0..652bacc0fc18 100644
>> --- a/daemon/inspect_fs_unix.ml
>> +++ b/daemon/inspect_fs_unix.ml
>> @@ -151,6 +151,7 @@ and distro_of_os_release_id = function
>>| "openmandriva" -> Some DISTRO_OPENMANDRIVA
>>| "opensuse" -> Some DISTRO_OPENSUSE
>>| s when String.is_prefix s "opensuse-" -> Some DISTRO_OPENSUSE
>> + | "pardus" -> Some DISTRO_PARDUS
>>| "pld" -> Some DISTRO_PLD_LINUX
>>| "rhel" -> Some DISTRO_RHEL
>>| "sles" | "sled" -> Some DISTRO_SLES
>
> Based on testing in the bug, ACK
Thank you, commit 3f6f2fb8f699.
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH] daemon/inspect_fs_unix: recognize modern Pardus GNU/Linux releases
Recent Pardus releases seem to have abandoned the original
"/etc/pardus-release" file, which the current Pardus detection, from
commit 233530d3541d ("inspect: Add detection of Pardus.", 2010-10-29), is
based upon.
Instead, Pardus apparently adopted the "/etc/os-release" specification
<https://www.freedesktop.org/software/systemd/man/os-release.html>, with
"ID=pardus". Extend the "distro_of_os_release_id" function accordingly.
Keep the original method for recognizing earlier releases.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1993842
Signed-off-by: Laszlo Ersek
---
daemon/inspect_fs_unix.ml | 1 +
1 file changed, 1 insertion(+)
diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml
index 557f328333f0..652bacc0fc18 100644
--- a/daemon/inspect_fs_unix.ml
+++ b/daemon/inspect_fs_unix.ml
@@ -151,6 +151,7 @@ and distro_of_os_release_id = function
| "openmandriva" -> Some DISTRO_OPENMANDRIVA
| "opensuse" -> Some DISTRO_OPENSUSE
| s when String.is_prefix s "opensuse-" -> Some DISTRO_OPENSUSE
+ | "pardus" -> Some DISTRO_PARDUS
| "pld" -> Some DISTRO_PLD_LINUX
| "rhel" -> Some DISTRO_RHEL
| "sles" | "sled" -> Some DISTRO_SLES
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] translating CD-ROM device paths from i440fx to Q35 in virt-v2v
Hi Rich,
(dropping libvirt-devel)
On 09/30/21 13:53, Richard W.M. Jones wrote:
> Also we don't currently try to find or rewrite /dev/disk/ paths in
> guest configuration files. The only rewriting that happens is for
> /dev/[hs]d* block device filenames and a few others. The actual code
> that does this is convert/convert_linux.ml:remap_block_devices
>
> So I wouldn't over-think this. It's likely fine to identify such
> devices and rewrite them as "/dev/cdrom", assuming that (I didn't
> check) udev creates that symlink for any reasonably modern Linux. And
> if there's more than one attached CD to the source, only convert the
> first one and warn about but drop the others.
So I've started reading "convert/convert_linux.ml" in parallel with the
OCaml manual. (I dabbled for a few weeks in Haskell when everyone else
did a few years ago, so it's not 100% unfamiliar.)
Question:
let family =
match inspect.i_distro with
| "fedora"
| "rhel" | "centos" | "scientificlinux" | "redhat-based"
| "oraclelinux" -> `RHEL_family
| "altlinux" -> `ALT_family
| "sles" | "suse-based" | "opensuse" -> `SUSE_family
| "debian" | "ubuntu" | "linuxmint" | "kalilinux" -> `Debian_family
Here, RHEL_family, ALT_family, SUSE_family, Debian_family are not plain
constructors ("fixed" variants) [1] but polymorphic ones [2].
Why?
Was this done *only* in order so that an explicit type definition such
as
type os_family = RHEL_family | ALT_family | SUSE_family | Debian_family
could be avoided?
Based on my (incomplete understanding) of the ocaml docs, this looks
like a bad idea. We need no polymorphism here, and using a static type
(a fixed variant) is generally beneficial [3].
The following (minimally modified) definition at the OCaml REPL:
let family1 distro =
match distro with
| "fedora"
| "rhel" | "centos" | "scientificlinux" | "redhat-based"
| "oraclelinux" -> `RHEL_family
| "altlinux" -> `ALT_family
| "sles" | "suse-based" | "opensuse" -> `SUSE_family
| "debian" | "ubuntu" | "linuxmint" | "kalilinux" -> `Debian_family
| _ -> assert false;;
deduces the following type:
string -> [> `ALT_family | `Debian_family | `RHEL_family | `SUSE_family ] =
with the nasty "[>" mark at the start (allowing for further type
refinement [2], which I think we don't need here). Conversely.
type os_family = RHEL_family | ALT_family | SUSE_family | Debian_family;;
let family2 distro =
match distro with
| "fedora"
| "rhel" | "centos" | "scientificlinux" | "redhat-based"
| "oraclelinux" -> RHEL_family
| "altlinux" -> ALT_family
| "sles" | "suse-based" | "opensuse" -> SUSE_family
| "debian" | "ubuntu" | "linuxmint" | "kalilinux" -> Debian_family
| _ -> assert false;;
(note the explicit os_family type definition, and the removal of the
backticks from the constructor names) comes back with the type:
val family2 : string -> os_family =
Basically converting a string to an enum constant.
So, what's the reason for the polymorphic variant?
[1] https://ocaml.org/manual/coreexamples.html#s%3Atut-recvariants
[2] https://ocaml.org/manual/polyvariant.html#sec48
[3] https://ocaml.org/manual/polyvariant.html#s%3Apolyvariant-weaknesses
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
On 09/20/21 14:33, Daniel P. Berrangé wrote:
> On Mon, Sep 20, 2021 at 12:03:51PM +0100, Richard W.M. Jones wrote:
>> On Mon, Sep 20, 2021 at 11:37:02AM +0100, Daniel P. Berrangé wrote:
>>> What distro / go version do you see this on, as I can't reproduce
>>> this pointer problem with a standalone demo app ?
>>
>> For me this started to happen after upgrading to
>> golang-bin-1.17-2.fc36.x86_64 in Rawhide. It also caused this error:
>
> Hmm, I still cant reproduce the problem that Laszlo is fixing
>
> $ cat str.c
>
> #include
>
> void foo(char **str) {
> for (int i = 0; str[i] != NULL; i++) {
> fprintf(stderr, "%d: %s (0x%p)\n", i, str[i], str[i]);
> }
> }
>
> $ cat str.go
> package main
>
> /*
> #cgo LDFLAGS: -L/home/berrange/t/lib -lstr
>
> #include
>
> extern void foo(char **str);
>
> */
> import "C"
>
> import (
> "fmt"
> "unsafe"
> )
>
> func array_elem(arr **C.char, idx int) **C.char {
> return (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(arr)) +
> (unsafe.Sizeof(arr) * uintptr(idx
> }
>
> func arg_string_list1(xs []string) **C.char {
> r := make([]*C.char, 1+len(xs))
> for i, x := range xs {
> r[i] = C.CString(x)
> }
> r[len(xs)] = nil
> return [0]
> }
>
> func arg_string_list2(xs []string) **C.char {
> var r **C.char
> r = (**C.char)(C.malloc(C.size_t(unsafe.Sizeof(*r) * (1 +
> uintptr(len(xs))
> for i, x := range xs {
> str := array_elem(r, i)
> *str = C.CString(x)
> }
> str := array_elem(r, len(xs))
> *str = nil
> return r
> }
>
> func free_string_list(argv **C.char) {
> for i := 0; ; i++ {
> str := (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(argv)) +
> (unsafe.Sizeof(*argv) * uintptr(i
> if *str == nil {
> break
> }
> fmt.Printf("%x\n", *str)
> C.free(unsafe.Pointer(*str))
> }
> }
>
> func bar(str []string) {
> cstr1 := arg_string_list1(str)
> defer free_string_list(cstr1)
> C.foo(cstr1)
> cstr2 := arg_string_list2(str)
> defer free_string_list(cstr2)
> C.foo(cstr2)
> }
>
> func main() {
> bar([]string{"hello", "world"})
> }
>
>
> My interpretation is that arg_string_list1 impl here should have
> raised the error that Laszlo reports, but both impls work fine
Can you create a new structure type, make the C function take the structure (or
a pointer to the structure), and in the structure, make the field have this
type:
char * const * str;
Because this is the scenario where the libguestfs test suite fails (panics).
The libguestfs test suite has a *different* case that does match your example
directly, and *that* case works in the libguestfs test suite flawlessly. The
panic surfaces only in the "char*const* embedded in struct" case. (I assume
"const" makes no difference, but who knows!)
>
> $ gcc -fPIC -c -o str.o str.c
> $ gcc -shared -o libstr.so str.o
>
> $ go version
> go version go1.17 linux/amd64
> $ go build -o str str.go
> $ LD_LIBRARY_PATH=/home/berrange/t/lib ./str
> 0: hello (0x0x1d68970)
> 1: world (0x0x1d68990)
> 0: hello (0x0x1d689d0)
> 1: world (0x0x1d689f0)
> 1d689d0
> 1d689f0
> 1d68970
> 1d68990
>
>
>
> Is my test scearnio there representative of what the failing test
> case is doing ?
No, your case represents the one that works fine in libguestfs too. From
"golang/bindtests/bindtests.go", generated at commit f47e0bb67254:
41 if err := g.Internal_test ("abc", nil, []string{}, false, 0, 0,
"123", "456", []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
_test{Oint64_is_set: true, Oint64: 1, Ostring_is_set:
true, Ostring: "string"}); err != nil {
The third argument is such a stringlist, and it works OK.
But this one panics due to "Ostringlist":
77 if err := g.Internal_test ("abc", string_addr ("def"), []string{},
false, 0, 0, "123", "456", []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
_test{Ostringlist_is_set: true, Ostringlist:
[]string{}}); err != nil {
The callee (Internal_test()) is in the also-generated source file
"golang/src/libguestfs.org/guestfs/guestfs.go".
Thanks!
Laszlo
> Or is perhaps the C function calling back into the Go code ?
>
> The reason I'm curious is that the current code for arrays here
> matches what libvirt-go-module currently uses in some places, so
> I'm wondering if that needs fixing too.
>
>
> Regards,
> Daniel
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdinfo v2 1/3] common/utils: Add function to convert sizes to human-readable
On 09/20/21 13:04, Richard W.M. Jones wrote:
> For example 1024 is returned as "1K".
>
> This does not attempt to handle decimals or SI units. If the number
> isn't some multiple of a power of 1024 then it is returned as bytes (a
> flag is available to indicate this).
>
> I looked at both the gnulib and qemu versions of this function. The
> gnulib version is not under a license which is compatible with libnbd
> and is also really complicated, although it does handle fractions and
> SI units. The qemu version is essentially just frexp + sprintf and
> doesn't attempt to convert to the human-readable version reversibly.
> ---
> .gitignore | 1 +
> common/utils/Makefile.am | 10 +++-
> common/utils/human-size.c | 54 +
> common/utils/human-size.h | 49 +++
> common/utils/test-human-size.c | 89 ++
> 5 files changed, 201 insertions(+), 2 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 2aa1fd99..5fc59677 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -31,6 +31,7 @@ Makefile.in
> /bash/nbdcopy
> /bash/nbdfuse
> /bash/nbdinfo
> +/common/utils/test-human-size
> /common/utils/test-vector
> /compile
> /config.cache
> diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
> index 1ca4a370..b273ada1 100644
> --- a/common/utils/Makefile.am
> +++ b/common/utils/Makefile.am
> @@ -34,6 +34,8 @@ include $(top_srcdir)/common-rules.mk
> noinst_LTLIBRARIES = libutils.la
>
> libutils_la_SOURCES = \
> + human-size.c \
> + human-size.h \
> vector.c \
> vector.h \
> version.c \
> @@ -50,8 +52,12 @@ libutils_la_LIBADD = \
>
> # Unit tests.
>
> -TESTS = test-vector
> -check_PROGRAMS = test-vector
> +TESTS = test-human-size test-vector
> +check_PROGRAMS = test-human-size test-vector
> +
> +test_human_size_SOURCES = test-human-size.c human-size.c human-size.h
> +test_human_size_CPPFLAGS = -I$(srcdir)
> +test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
>
> test_vector_SOURCES = test-vector.c vector.c vector.h
> test_vector_CPPFLAGS = -I$(srcdir)
> diff --git a/common/utils/human-size.c b/common/utils/human-size.c
> new file mode 100644
> index ..772f2489
> --- /dev/null
> +++ b/common/utils/human-size.c
> @@ -0,0 +1,54 @@
> +/* nbd client library in userspace
> + * Copyright (C) 2020-2021 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
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
(1) General question: should we adopt "SPDX-License-Identifier"s? They
are more succinct.
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "human-size.h"
> +
> +char *
> +human_size (char *buf, uint64_t bytes, bool *human)
> +{
> + static const char *ext[] = { "E", "P", "T", "G", "M", "K", "" };
(2) The following would be more frugal:
static const char ext[][2] = { "E", "P", "T", "G", "M", "K", "" };
(each pointer in the current array is 4 bytes or 8 bytes).
> + size_t i;
> +
> + if (buf == NULL) {
> +buf = malloc (HUMAN_SIZE_LONGEST);
> +if (buf == NULL)
> + return NULL;
> + }
> +
> + /* Work out which extension to use, if any. */
> + for (i = 6; i >= 0; --i) {
> +if (bytes == 0 || (bytes & 1023) != 0)
> + break;
> +bytes /= 1024;
> + }
(3) "size_t" is an unsigned type. When the loop is entered with i==0,
and the "break" is not taken, "i" will flip back to ((size_t)-1), a very
large positive integer. In other words, the controlling expression of
this loop is unfalsifiable.
Functionally, that's not a problem; when we enter the loop with i==0, we
will have shifted out 6*10 = 60 bits on the least significant end, we
only have 4 bits left, and so (bytes == 0 || (bytes & 1023) != 0) will
definitely fire. It's just that the controlling expression of the loop
is misleading (it suggests it has some role, when it doesn't).
(4) Style: I think bit-and (&) should be coupled with bit-shift (>>), or
else division (/) should be coupled with remainder (%).
(5) It is not necessary to keep the zero check in the loop body. Once we
determine (before the loop) that the input is nonzero, we know it must
have at least one bit that is set. So we can just continue shifting out
runs of ten zero bits, until
Re: [Libguestfs] [PATCH nbdinfo v2 1/3] common/utils: Add function to convert sizes to human-readable
On 09/20/21 18:25, Laszlo Ersek wrote: > On 09/20/21 13:04, Richard W.M. Jones wrote: >> +#define HUMAN_SIZE_LONGEST 64 > > (5) The integer constant expression > > ((sizeof (uint64_t) * 8 + 2) / 3 + 1) > > would be more frugal (but we might not care). > > If we take the number of three-bit groups in the word, and divide that > by three -- rounded up --, we get the number of necessary octal digits. > The number of decimal digits needed never exceeds the number of octal > digits needed, so this is safe. Add one character for the NUL terminator.) Typo, due to too much editing, of course. Please: s/number of three-bit groups/number of bits/ Thanks & sorry! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdinfo v2 2/3] info: Add human size to ordinary output
On 09/20/21 13:04, Richard W.M. Jones wrote:
> For example:
>
> $ nbdkit null 1G --run 'nbdinfo "$uri"'
> protocol: newstyle-fixed without TLS
> export="":
> export-size: 1073741824 (1G)
>
> If the value cannot be abbreviated then the output doesn't include the
> part in parentheses:
>
> $ nbdkit null 1023 --run 'nbdinfo "$uri"'
> protocol: newstyle-fixed without TLS
> export="":
> export-size: 1023
>
> The human-readable output changes, but callers shouldn't be parsing it
> (use --size and/or --json instead).
> ---
> info/nbdinfo.pod | 9 +
> info/show.c | 13 +++--
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod
> index fbd0ef20..f64fe213 100644
> --- a/info/nbdinfo.pod
> +++ b/info/nbdinfo.pod
> @@ -29,7 +29,7 @@
> L<https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md>):
> $ nbdinfo nbd://localhost
> protocol: newstyle-fixed without TLS
> export="":
> - export-size: 1048576
> + export-size: 1048576 (1M)
> content: data
> uri: nbd://localhost:10809/
> is_rotational: false
> @@ -78,7 +78,8 @@ the I<--json> parameter:
> "block_size_minimum": 1,
> "block_size_preferred": 4096,
> "block_size_maximum": 33554432,
> - "export-size": 2125119488
> + "export-size": 2125119488,
> + "export-size-str": "2075312K"
> }
> ]
> }
> @@ -222,11 +223,11 @@ For example:
> $ nbdkit file dir=. --run 'nbdinfo --list "$uri"'
> protocol: newstyle-fixed without TLS
> export="Fedora-Workstation-Live-x86_64-29-1.2.iso":
> - export-size: 1931476992
> + export-size: 1931476992 (1842M)
> uri: nbd://localhost:10809/Fedora-Workstation-Live-x86_64-29-1.2.iso
> [...]
> export="debian-10.4.0-amd64-DVD-1.iso":
> - export-size: 396352
> + export-size: 396352 (3862848K)
> uri: nbd://localhost:10809/debian-10.4.0-amd64-DVD-1.iso
> [...]
>
> diff --git a/info/show.c b/info/show.c
> index 893178cb..ff241a83 100644
> --- a/info/show.c
> +++ b/info/show.c
> @@ -28,6 +28,7 @@
>
> #include
>
> +#include "human-size.h"
> #include "vector.h"
>
> #include "nbdinfo.h"
> @@ -45,6 +46,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
> bool first, bool last)
> {
>int64_t i, size;
> + char size_str[HUMAN_SIZE_LONGEST];
> + bool human_size_flag;
>char *export_name = NULL;
>char *export_desc = NULL;
>char *content = NULL;
> @@ -75,6 +78,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
> exit (EXIT_FAILURE);
>}
>
> + human_size (size_str, size, _size_flag);
> +
>uri = nbd_get_uri (nbd);
>
>/* Prefer the server's version of the name, if available */
> @@ -116,7 +121,10 @@ show_one_export (struct nbd_handle *nbd, const char
> *desc,
> fprintf (fp, ":\n");
> if (desc && *desc)
>fprintf (fp, "\tdescription: %s\n", desc);
> -fprintf (fp, "\texport-size: %" PRIi64 "\n", size);
> +if (human_size_flag)
> + fprintf (fp, "\texport-size: %" PRIi64 " (%s)\n", size, size_str);
> +else
> + fprintf (fp, "\texport-size: %" PRIi64 "\n", size);
> if (content)
> fprintf (fp, "\tcontent: %s\n", content);
> if (uri)
> @@ -239,7 +247,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
> block_maximum);
>
> /* Put this one at the end because of the stupid comma thing in JSON. */
> -fprintf (fp, "\t\"export-size\": %" PRIi64 "\n", size);
> +fprintf (fp, "\t\"export-size\": %" PRIi64 ",\n", size);
> +fprintf (fp, "\t\"export-size-str\": \"%s\"\n", size_str);
>
> if (last)
>fprintf (fp, "\t} ]\n");
>
Assuming "size" (of type int64_t) is never negative here:
Acked-by: Laszlo Ersek
Thanks
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdinfo v2 1/3] common/utils: Add function to convert sizes to human-readable
On 09/20/21 18:39, Richard W.M. Jones wrote: > On Mon, Sep 20, 2021 at 06:25:34PM +0200, Laszlo Ersek wrote: >> On 09/20/21 13:04, Richard W.M. Jones wrote: >>> +/* If you allocate a buffer of at least this length in bytes and pass >>> + * it as the first parameter to human_size, then it will not overrun. >>> + */ >>> +#define HUMAN_SIZE_LONGEST 64 >> >> (5) The integer constant expression >> >> ((sizeof (uint64_t) * 8 + 2) / 3 + 1) >> >> would be more frugal (but we might not care). >> >> If we take the number of three-bit groups in the word, and divide that >> by three -- rounded up --, we get the number of necessary octal digits. >> The number of decimal digits needed never exceeds the number of octal >> digits needed, so this is safe. Add one character for the NUL terminator.) > > Yeah I was just guessing here. FWIW gnulib really does the hard work: > > https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/human.h;h=61f110fc0215205039ec9729ab34e68686c2d102;hb=HEAD#l30 I was prepared to mention log10(2) and that we could approximate it more closely than with 1/3; gnulib didn't disappoint this time either! :) Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
On 09/20/21 17:30, Daniel P. Berrangé wrote:
> On Mon, Sep 20, 2021 at 05:23:30PM +0200, Laszlo Ersek wrote:
>> On 09/20/21 14:33, Daniel P. Berrangé wrote:
>>> On Mon, Sep 20, 2021 at 12:03:51PM +0100, Richard W.M. Jones wrote:
>>>> On Mon, Sep 20, 2021 at 11:37:02AM +0100, Daniel P. Berrangé wrote:
>>>>> What distro / go version do you see this on, as I can't reproduce
>>>>> this pointer problem with a standalone demo app ?
>>>>
>>>> For me this started to happen after upgrading to
>>>> golang-bin-1.17-2.fc36.x86_64 in Rawhide. It also caused this error:
>>>
>>> Hmm, I still cant reproduce the problem that Laszlo is fixing
>>>
>>> $ cat str.c
>>>
>>> #include
>>>
>>> void foo(char **str) {
>>> for (int i = 0; str[i] != NULL; i++) {
>>> fprintf(stderr, "%d: %s (0x%p)\n", i, str[i], str[i]);
>>> }
>>> }
>>>
>>> $ cat str.go
>>> package main
>>>
>>> /*
>>> #cgo LDFLAGS: -L/home/berrange/t/lib -lstr
>>>
>>> #include
>>>
>>> extern void foo(char **str);
>>>
>>> */
>>> import "C"
>>>
>>> import (
>>> "fmt"
>>> "unsafe"
>>> )
>>>
>>> func array_elem(arr **C.char, idx int) **C.char {
>>> return (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(arr)) +
>>> (unsafe.Sizeof(arr) * uintptr(idx
>>> }
>>>
>>> func arg_string_list1(xs []string) **C.char {
>>> r := make([]*C.char, 1+len(xs))
>>> for i, x := range xs {
>>> r[i] = C.CString(x)
>>> }
>>> r[len(xs)] = nil
>>> return [0]
>>> }
>>>
>>> func arg_string_list2(xs []string) **C.char {
>>> var r **C.char
>>> r = (**C.char)(C.malloc(C.size_t(unsafe.Sizeof(*r) * (1 +
>>> uintptr(len(xs))
>>> for i, x := range xs {
>>> str := array_elem(r, i)
>>> *str = C.CString(x)
>>> }
>>> str := array_elem(r, len(xs))
>>> *str = nil
>>> return r
>>> }
>>>
>>> func free_string_list(argv **C.char) {
>>> for i := 0; ; i++ {
>>> str := (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(argv)) +
>>> (unsafe.Sizeof(*argv) * uintptr(i
>>> if *str == nil {
>>> break
>>> }
>>> fmt.Printf("%x\n", *str)
>>> C.free(unsafe.Pointer(*str))
>>> }
>>> }
>>>
>>> func bar(str []string) {
>>> cstr1 := arg_string_list1(str)
>>> defer free_string_list(cstr1)
>>> C.foo(cstr1)
>>> cstr2 := arg_string_list2(str)
>>> defer free_string_list(cstr2)
>>> C.foo(cstr2)
>>> }
>>>
>>> func main() {
>>> bar([]string{"hello", "world"})
>>> }
>>>
>>>
>>> My interpretation is that arg_string_list1 impl here should have
>>> raised the error that Laszlo reports, but both impls work fine
>>
>> Can you create a new structure type, make the C function take the structure
>> (or a pointer to the structure), and in the structure, make the field have
>> this type:
>>
>> char * const * str;
>>
>> Because this is the scenario where the libguestfs test suite fails (panics).
>> The libguestfs test suite has a *different* case that does match your
>> example directly, and *that* case works in the libguestfs test suite
>> flawlessly. The panic surfaces only in the "char*const* embedded in struct"
>> case. (I assume "const" makes no difference, but who knows!)
>
> Oh, that makes sense, because you have a Go pointer to the storage for
> the struct, and then the 'const *const *str' field is initialized with
> a Go pointer returned from the arg_string_list().
>
> You're allowed to pass a Go pointer to C via CGo, but the memory that
> points to is not allowed to contained further Go pointers. So the struct
> fields must strictly use a C pointer.
If I take your last paragraph here and work it into the commit message /
comment of this patch, will you accept the code in the patch?
I really don't insist on getting *this* particular patch in. What I'd
like to achieve is a clean "make check" baseline, so I can run the test
suite regularly, as I get into fixing other libguestfs BZs. I don't
intend to "maintain" Go bindings; I consider this a one-off fix. I'm
uncomfortable contributing any Go code that's not as "thin" as I can
possibly manage. (And honestly I think the swaths of "explicit unsafe"
just make things unreadable.)
So I could do two things:
- push patches #1-#3 and drop #4, allowing someone else to fix the issue
described in #4 in more idiomatic Go,
- post a v2 of #4 with updated comments / commit message.
I think a "less idiomatic but technically correct" Go binding is still
an improvement over a panicking Go runtime, but again, if someone can
fix this more idiomatically, I'll *gladly* defer to them.
Rich, what's your take?
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdinfo v2 3/3] copy: Print debug information with human sizes
On 09/20/21 18:41, Eric Blake wrote: > On Mon, Sep 20, 2021 at 06:32:29PM +0200, Laszlo Ersek wrote: >>> +++ b/copy/test-verbose.sh >>> @@ -28,11 +28,11 @@ requires nbdkit --version >>> file=test-verbose.out >>> cleanup_fn rm -f $file >>> >>> -$VG nbdcopy -v -- [ nbdkit null ] null: 2>$file >>> +$VG nbdcopy -v -- [ nbdkit memory 1M ] null: 2>$file >> >> (1) I don't understand this change. Why do we replace "null" with >> "memory 1M"? >> >> (Side question that I've been meaning to ask: what is this "$VG" magic?) > > Answering just the side question: > > When LIBNBD_VALGRIND is set to 1 in the environment, then $VG is set > in run.in to an invocation of valgrind, optionally further wrapped by > an invocation of libtool to see through any libtool wrapper script. > Otherwise $VG is empty, and you run the real binary without any outer > wrappers. It's actually a clever way of checking memory usage issues > during 'make check-valgrind' while probing the real binary rather than > accidentally running valgrind on a shell script. > Ah! So "VG" stands for "valgrind", which, in retrospec, probably stands for "value grind". (Never before have I dissected "valgrind" to "grinding values"; my non-native English parser doesn't work like that I guess.) Thanks! Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdinfo v2 3/3] copy: Print debug information with human sizes
On 09/20/21 13:04, Richard W.M. Jones wrote:
> ---
> copy/main.c | 6 +-
> copy/test-verbose.sh | 4 ++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/copy/main.c b/copy/main.c
> index 70534b5a..15a64544 100644
> --- a/copy/main.c
> +++ b/copy/main.c
> @@ -39,6 +39,7 @@
> #include
>
> #include "ispowerof2.h"
> +#include "human-size.h"
> #include "version.h"
> #include "nbdcopy.h"
>
> @@ -508,8 +509,11 @@ open_local (const char *filename, direction d)
> static void
> print_rw (struct rw *rw, const char *prefix, FILE *fp)
> {
> + char buf[HUMAN_SIZE_LONGEST];
> +
>fprintf (fp, "%s: %s \"%s\"\n", prefix, rw->ops->ops_name, rw->name);
> - fprintf (fp, "%s: size=%" PRIi64 "\n", prefix, rw->size);
> + fprintf (fp, "%s: size=%" PRIi64 " (%s)\n",
> + prefix, rw->size, human_size (buf, rw->size, NULL));
> }
>
> /* Default implementation of rw->ops->get_extents for backends which
Hopefully rw->size is never negative here...
> diff --git a/copy/test-verbose.sh b/copy/test-verbose.sh
> index afd57580..4cc67d37 100755
> --- a/copy/test-verbose.sh
> +++ b/copy/test-verbose.sh
> @@ -28,11 +28,11 @@ requires nbdkit --version
> file=test-verbose.out
> cleanup_fn rm -f $file
>
> -$VG nbdcopy -v -- [ nbdkit null ] null: 2>$file
> +$VG nbdcopy -v -- [ nbdkit memory 1M ] null: 2>$file
(1) I don't understand this change. Why do we replace "null" with
"memory 1M"?
(Side question that I've been meaning to ask: what is this "$VG" magic?)
>
> cat $file
>
> # Check some known strings appear in the output.
> grep '^nbdcopy: src: nbd_ops' $file
> -grep '^nbdcopy: src: size=0' $file
> +grep '^nbdcopy: src: size=1048576 (1M)' $file
> grep '^nbdcopy: dst: null_ops' $file
>
Ah, the test case is modified at once so it generate more interesting
output. Can you note that in the commit message please?
Acked-by: Laszlo Ersek
Thanks!
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH 2/4] test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain
Per commit ac39fa292c31 ("v2v: Set machine type explicitly for outputs
which support it (RHBZ#1581428).", 2020-12-04), Windows 7 guests (which
are "more modern" than Windows XP guests) are converted to Q35, not I440FX
boards.
Per related commit d0267122e348 ("v2v: -o libvirt: Map IDE disks to
bus="sata" on Q35.", 2020-12-04), when a Windows 7 guest with an IDE
CD-ROM -- hence, an I440FX board -- is converted, the CD-ROM in the
converted domain will be on the SATA bus (Q35 does not have an IDE
controller, only a SATA controller).
Because the Windows guest used in "test-v2v-cdrom" is "Windows 7 Phony
Edition", it triggers the above cascade, and currently fails with:
> --- ./test-v2v-cdrom.expected 2021-09-03 22:51:31.026185527 +0200
> +++ test-v2v-cdrom.d/disks 2021-09-19 13:01:47.471745622 +0200
> @@ -4,5 +4,5 @@
>
>
>
> -
> +
>
> ./test-v2v-cdrom.sh: unexpected disk assignments
The conversion seems correct, but the test expectation is stale. Most
probably, commit d0267122e348 missed updating the test data. Do it now.
Fixes: d0267122e348202f6fac4d833f7448409e7129c9
Signed-off-by: Laszlo Ersek
---
tests/test-v2v-cdrom.expected | 2 +-
tests/test-v2v-cdrom.xml.in | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected
index 34d2bf5961b0..17bd152d8e64 100644
--- a/tests/test-v2v-cdrom.expected
+++ b/tests/test-v2v-cdrom.expected
@@ -4,5 +4,5 @@
-
+
diff --git a/tests/test-v2v-cdrom.xml.in b/tests/test-v2v-cdrom.xml.in
index 6bad5eab1cd4..a6e1e3f514d5 100644
--- a/tests/test-v2v-cdrom.xml.in
+++ b/tests/test-v2v-cdrom.xml.in
@@ -35,7 +35,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA.
-
+
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH 3/4] test-v2v-i-ova: update the CD-ROM's bus to SATA in the converted domain
Using the argument from the "test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain" patch earlier in this series. Fixes: d0267122e348202f6fac4d833f7448409e7129c9 Signed-off-by: Laszlo Ersek --- tests/test-v2v-i-ova.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml index 671476b95d96..a018fd44aee8 100644 --- a/tests/test-v2v-i-ova.xml +++ b/tests/test-v2v-i-ova.xml @@ -28,7 +28,7 @@ - + -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v2] Go bindings: fix "C array of strings" -- char** -- allocation
On 09/21/21 23:01, Richard W.M. Jones wrote:
> On Tue, Sep 21, 2021 at 09:29:39PM +0200, Laszlo Ersek wrote:
>> The current "arg_string_list" and "free_string_list" implementations go
>> back to commit b6f01f32606d ("Add Go (language) bindings.", 2013-07-03).
>> There are two problems with them:
>>
>> - "free_string_list" doesn't actually free anything,
>>
>> - at the *first* such g.Internal_test() call site that passes an
>> Ostringlist member inside the Optargs argument, namely:
>>
>>> g.Internal_test ("abc",
>>> string_addr ("def"),
>>> []string{},
>>> false,
>>> 0,
>>> 0,
>>> "123",
>>> "456",
>>> []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
>>> _test{Ostringlist_is_set: true,
>>>Ostringlist: []string{}
>>> }
>>> )
>>
>> the "golang/run-bindtests" case crashes:
>>
>>> panic: runtime error: cgo argument has Go pointer to Go pointer
>>>
>>> goroutine 1 [running]:
>>> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc18180,
>>> 0xadfb60, 0xadfb80, 0xc10048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
>>> 0xade3a0, ...)
>>> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
>>> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc18180, 0x4ee3a5,
>>> 0x3, 0xc61be8, 0xc61af8, 0x0, 0x0, 0xc61a00, 0x0, 0x0, ...)
>>> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
>>> main.main()
>>> golang/bindtests/bindtests.go:77 +0x151e
>>> exit status 2
>>> FAIL run-bindtests (exit status: 1)
>>
>> In Daniel P. Berrangé's words [1],
>>
>>> You're allowed to pass a Go pointer to C via CGo, but the memory that
>>> points to is not allowed to contained further Go pointers. So the struct
>>> fields must strictly use a C pointer.
>>
>> One pattern to solve the problem has been shown on stackoverflow [2].
>> Thus, rewrite the "arg_string_list" and "free_string_list" functions
>> almost entirely in C, following that example.
>>
>> While this approach is not the most idiomatic Go, as a solution exists
>> without C helper functions [3], it should still be acceptable, at least as
>> an incremental improvement, for letting "golang/run-bindtests" pass.
>>
>> [1]
>> https://listman.redhat.com/archives/libguestfs/2021-September/msg00118.html
>> [2]
>> https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
>> [3]
>> https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html
>>
>> Cc: "Daniel P. Berrangé"
>> Cc: "Richard W.M. Jones"
>> Signed-off-by: Laszlo Ersek
>
> This time I enabled the golang bindings, reproduced the bug you
> reported, applied this patch, reran the tests and verified that it has
> been fixed, so:
>
> Tested-by: "Richard W.M. Jones"
> Acked-by: "Richard W.M. Jones"
Commit e2f8db27d0af.
Thank you!
Laszlo
>
> Rich.
>
>>
>> Notes:
>> v2:
>> - update commit message [Daniel, Rich]
>> - update code comment [Daniel, Rich]
>> - no code changes
>> - retested
>>
>> generator/golang.ml | 47 -
>> 1 file changed, 38 insertions(+), 9 deletions(-)
>>
>> diff --git a/generator/golang.ml b/generator/golang.ml
>> index d328abe4ed08..0d6a92367f9b 100644
>> --- a/generator/golang.ml
>> +++ b/generator/golang.ml
>> @@ -52,6 +52,40 @@ _go_guestfs_create_flags (unsigned flags)
>> {
>> return guestfs_create_flags (flags);
>> }
>> +
>> +// Passing a Go pointer to C via CGo is allowed, but the memory that points
>> to
>> +// is not allowed to contain further Go pointers. The helper functions below
>> +// are one way to implement this, although the same can be achieved purely
>> in
>> +// Go as well. See the discussion here:
>> +//
>> <https://listman.redhat.com/archives/libguestfs/2021-September/msg00101.html>.
>> +typedef char *pChar;
>> +
>> +pChar *allocStringArray (size_t nmemb)
>>
[Libguestfs] [libnbd PATCH] build: allow OCaml programs using libnbd to be compiled against build dir
Port libguestfs commit bf61bf7355d3 ("build: Allow OCaml programs using
libguestfs to be compiled against build dir.", 2020-03-12) to libnbd.
This allows C+OCaml programs, such as virt-v2v, to find a just built, but
not installed, libnbd tree.
Signed-off-by: Laszlo Ersek
---
ocaml/Makefile.am | 17 +
.gitignore| 1 +
run.in| 2 ++
3 files changed, 20 insertions(+)
diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
index 1c7397a9da7a..b1ab80dd5fa5 100644
--- a/ocaml/Makefile.am
+++ b/ocaml/Makefile.am
@@ -161,4 +161,21 @@ install-data-hook:
$(data_hook_files)
rm $(DESTDIR)$(OCAMLLIB)/nbd/libnbdocaml.a
+# This "tricks" ocamlfind into allowing us to compile other OCaml
+# programs against a locally compiled copy of the libnbd sources.
+# ocamlfind needs to see a directory called ‘nbd’ which contains
+# ‘META’. The current directory is called ‘ocaml’, but if we make
+# this symlink then we can create the required directory structure.
+#
+# Note if you just want to use this, make sure you use
+# ‘../libnbd/run make’ in your other program and everything should
+# just work.
+CLEANFILES += nbd
+
+all-local: nbd
+
+nbd:
+ rm -f $@
+ $(LN_S) . $@
+
endif HAVE_OCAML
diff --git a/.gitignore b/.gitignore
index 5fc596773ed4..55a5449a70e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -141,6 +141,7 @@ Makefile.in
/ocaml/examples/.depend
/ocaml/libnbd-ocaml.3
/ocaml/libnbdocaml.a
+/ocaml/nbd
/ocaml/nbd-c.c
/ocaml/stamp-manpages
/ocaml/stamp-mlnbd
diff --git a/run.in b/run.in
index 99c1f7ff0ff6..dda49228f949 100755
--- a/run.in
+++ b/run.in
@@ -100,6 +100,8 @@ export GODEBUG=cgocheck=2
# Allow dependent packages to be compiled against local libnbd.
prepend PKG_CONFIG_PATH "$b/lib/local"
export PKG_CONFIG_PATH
+prepend OCAMLPATH "$b/ocaml"
+export OCAMLPATH
# Do we have libtool? If we have it then we can use it to make
# running valgrind simpler. However don't depend on it.
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH 0/4] Build and test suite fixes
After (1) applying the libnbd patch at <https://listman.redhat.com/archives/libguestfs/2021-September/msg00158.html>, (2) creating a symlink to the just-built nbdkit binary under ~/bin, (3) applying these virt-v2v patches, and (4) masking the in-place v2v test temporarily with SKIP_TEST_V2V_IN_PLACE_SH=1, I can run "make" and "make check" successfully in the virt-v2v tree. Thanks, Laszlo Laszlo Ersek (4): Makefile.am: use $(LIBNBD_LIBS) for linking OCaml programs test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain test-v2v-i-ova: update the CD-ROM's bus to SATA in the converted domain test-v2v-i-ova: spell out viosock element in output domain XML convert/Makefile.am | 1 + output/Makefile.am| 1 + v2v/Makefile.am | 1 + tests/test-v2v-cdrom.expected | 2 +- tests/test-v2v-cdrom.xml.in | 4 +++- tests/test-v2v-i-ova.xml | 3 ++- 6 files changed, 9 insertions(+), 3 deletions(-) -- 2.19.1.3.g30247aa5d201 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH 1/4] Makefile.am: use $(LIBNBD_LIBS) for linking OCaml programs
Otherwise the build fails with errors such as
> GEN helper-v2v-output
> /usr/bin/ld: cannot find -lnbd
> collect2: error: ld returned 1 exit status
> File "caml_startup", line 1:
> Error: Error during linking (exit code 1)
in the "convert", "output" and "v2v" modules.
(This patch is similar to guestfs-tools commit 9b0f2e2d5893 ("Makefile.am:
use $(LIBGUESTFS_LIBS) for linking OCaml programs", 2021-09-06).)
Signed-off-by: Laszlo Ersek
---
convert/Makefile.am | 1 +
output/Makefile.am | 1 +
v2v/Makefile.am | 1 +
3 files changed, 3 insertions(+)
diff --git a/convert/Makefile.am b/convert/Makefile.am
index bb7c444f4aed..398bc1ed77a3 100644
--- a/convert/Makefile.am
+++ b/convert/Makefile.am
@@ -99,6 +99,7 @@ OCAMLCLIBS = \
$(LIBGUESTFS_LIBS) \
$(LIBXML2_LIBS) \
$(LIBOSINFO_LIBS) \
+ $(LIBNBD_LIBS) \
-lgnu
OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)'
diff --git a/output/Makefile.am b/output/Makefile.am
index 46e6560cb2bf..a63ad4c70d45 100644
--- a/output/Makefile.am
+++ b/output/Makefile.am
@@ -140,6 +140,7 @@ OCAMLCLIBS = \
$(LIBGUESTFS_LIBS) \
$(LIBXML2_LIBS) \
$(JANSSON_LIBS) \
+ $(LIBNBD_LIBS) \
-lgnu
OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)'
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 1ce11814e193..f1073d96399f 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -83,6 +83,7 @@ OCAMLCLIBS = \
$(JANSSON_LIBS) \
$(LIBOSINFO_LIBS) \
$(LIBINTL) \
+ $(LIBNBD_LIBS) \
-lgnu
OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) -ccopt '$(CFLAGS)'
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [virt-v2v PATCH 4/4] test-v2v-i-ova: spell out viosock element in output domain XML
This is likely an omission from commit 05f780c16f01 ("v2v: support
configuration of viosock driver", 2021-02-26) -- if the converted Windows
guest does not dispose over the virtio-sock guest drivers, an explicit
"none"-model element is now generated in the domain XML.
Fixes: 05f780c16f0135c657615520c2245b42de1efc3e
Signed-off-by: Laszlo Ersek
---
tests/test-v2v-i-ova.xml | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml
index a018fd44aee8..489c0ab09bcc 100644
--- a/tests/test-v2v-i-ova.xml
+++ b/tests/test-v2v-i-ova.xml
@@ -46,6 +46,7 @@
/dev/urandom
+
--
2.19.1.3.g30247aa5d201
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 2/4] test-md-and-lvm-devices: work around RAID0 regression in Linux v3.14/v5.4
On 09/20/21 07:23, Laszlo Ersek wrote: > The "test-md-and-lvm-devices" test case creates, among other things, a > RAID0 array (md127) that spans two *differently sized* block devices > (sda1: 20MB, lv0: 16MB). > > In Linux v3.14, the layout of such arrays was changed incompatibly and > undetectably. If an array were created with a pre-v3.14 kernel and > assembled on a v3.14+ kernel, or vice versa, data could be corrupted. > > In Linux v5.4, a mitigation was added, requiring the user to specify the > layout version of such RAID0 arrays explicitly, as a module parameter. If > the user fails to specify a layout version, the v5.4+ kernel refuses to > assemble such arrays. This is why "test-md-and-lvm-devices" currently > fails, with any v5.4+ appliance kernel. > > Until we implement a more general solution (see the bugzilla link below), > work around the issue by sizing sda1 and lv0 identically. For this, > increase the size of sdb1 to 24MB: when one 4MB extent is spent on LVM > metadata, the resultant lv0 size (20MB) will precisely match the size of > sda1. > > This workaround only affects sizes, and does not interfere with the > original purpose of this test case, which is to test various *stackings* > between disk partitions, software RAID (md), and LVM logical volumes. > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=2005485 > Signed-off-by: Laszlo Ersek > --- > tests/md/test-md-and-lvm-devices.sh | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tests/md/test-md-and-lvm-devices.sh > b/tests/md/test-md-and-lvm-devices.sh > index 5e82e3a4ff69..54f1c3bfb3f5 100755 > --- a/tests/md/test-md-and-lvm-devices.sh > +++ b/tests/md/test-md-and-lvm-devices.sh > @@ -53,7 +53,7 @@ trap cleanup INT QUIT TERM EXIT > # sda2: 20M PV (vg1) > # sda3: 20M MD (md125) > # > -# sdb1: 20M PV (vg0) > +# sdb1: 24M PV (vg0) [*] > # sdb2: 20M PV (vg2) > # sdb3: 20M MD (md125) > # > @@ -66,6 +66,9 @@ trap cleanup INT QUIT TERM EXIT > # vg3 : VG (md125) > # lv3 : LV (vg3) > # > +# [*] The reason for making sdb1 4M larger than sda1 is that the LVM metadata > +# will consume one 4MB extent, and we need lv0 to offer exactly as much space > +# as sda1 does, for combining them in md127. Refer to RHBZ#2005485. > > guestfish < # Add 2 empty disks > @@ -79,9 +82,10 @@ part-add /dev/sda p 64 41023 > part-add /dev/sda p 41024 81983 > part-add /dev/sda p 81984 122943 > part-init /dev/sdb mbr > -part-add /dev/sdb p 64 41023 > -part-add /dev/sdb p 41024 81983 > -part-add /dev/sdb p 81984 122943 > +part-add /dev/sdb p 64 49215 > +part-add /dev/sdb p 49216 90175 > +part-add /dev/sdb p 90176 131135 > + I've removed the stray empty line just above, and pushed the patch as commit 24f98939ae0c. Thanks Laszlo > > # Create volume group and logical volume on sdb1 > pvcreate /dev/sdb1 > ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 1/4] test-9p: fix the base directory that's exported to the guest
On 09/20/21 07:23, Laszlo Ersek wrote:
> In commit 6d32773e8118 ("tests: Run the tests in parallel.", 2021-03-18),
> the "abs_srcdir" macro value that the 9p test would see changed from
> ".../tests/9p" to just ".../tests" -- the last component got dropped.
>
> (Said commit updated some "abs_srcdir"-based references accordingly, for
> example under "tests/disks", but "tests/9p/test-9p.sh" was missed.)
>
> Therefore, the guest-visible location of the "/test-9p.sh" file changed to
> "/9p/test-9p.sh", and a non-recursive listing of the guest-visible root
> directory would not return the file. Thus, the test fails now.
>
> Restore the host-side base directory to ".../tests/9p".
>
> Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
> Signed-off-by: Laszlo Ersek
> ---
> tests/9p/test-9p.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/9p/test-9p.sh b/tests/9p/test-9p.sh
> index b4bdbe56e077..4fd5de7fdafe 100755
> --- a/tests/9p/test-9p.sh
> +++ b/tests/9p/test-9p.sh
> @@ -45,7 +45,7 @@ guestfish < sparse test-9p.img 1M
>
> config -device '$virtio_9p,fsdev=test9p,mount_tag=test9p'
> -config -fsdev 'local,id=test9p,path=${abs_srcdir},security_model=passthrough'
> +config -fsdev
> 'local,id=test9p,path=${abs_srcdir}/9p,security_model=passthrough'
>
> run
>
>
This patch has been pushed as commit 8156c1652062.
Thanks,
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH 3/4] tests: xfs: remove lazy-counter disablement test
On 09/20/21 07:23, Laszlo Ersek wrote: > According to xfs_admin(8): > >>-c 0|1 Enable (1) or disable (0) lazy-counters in the filesys‐ >> tem. >> >> Lazy-counters may not be disabled on Version 5 su‐ >> perblock filesystems (i.e. those with metadata CRCs en‐ >> abled). >> >> [...] > > According to mkfs.xfs(1): > >>-m global_metadata_options >>Section Name: [metadata] >> These options specify metadata format options that ei‐ >> ther apply to the entire filesystem or aren't easily >> characterised by a specific functionality group. The >> valid global_metadata_options are: >> >> [...] >> >>crc=value >> This is used to create a filesystem which >> maintains and checks CRC information in all >> metadata objects on disk. The value is ei‐ >> ther 0 to disable the feature, or 1 to en‐ >> able the use of CRCs. >> >> [...] >> >> By default, mkfs.xfs will enable metadata >> CRCs. > > Consistently with the above, the first "xfs_admin" test case in > "generator/actions_core.ml", which attempts to disable lazy counters, > always fails: > >> 534/550 test_xfs_admin_0 >> libguestfs: error: xfs_admin: /dev/sda1: Cannot disable lazy-counters on V5 >> fs > > We can resolve this test failure in three ways: > > (1) Extend do_mkfs() [daemon/mkfs.c], possibly even introduce > do_mkfs_xfs(), and permit the caller to specify "-m crc=0" for > mkfs.xfs. Then use this option when the temporary filesystem is > created in the XFS test that disables lazy counters. > > (2) Extend the "guestfs_int_xfsinfo" structure in the libguestfs-common > project, with an "xfs_crc" field. Extend parse_xfs_info() > [daemon/xfs.c] to populate the field from "meta-data=...crc=[01]". > Modify the test case to check the following post-condition: > > xfs_crc || xfs_lazycount == 0 > > instead of the current > > xfs_lazycount == 0 > > effectively ignoring "xfs_lazycount" when "xfs_crc" is set. > > (3) Remove the test altogether that attempts to disable lazy counters > after filesystem creation. > > Given that new XFS filesystems are created with metadata CRCs enabled by > default, and several XFS features depend on metadata CRCs being enabled, > this patch implements option (3). > > Signed-off-by: Laszlo Ersek > --- > generator/actions_core.ml | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/generator/actions_core.ml b/generator/actions_core.ml > index bb602ee02dfc..5933282dcf90 100644 > --- a/generator/actions_core.ml > +++ b/generator/actions_core.ml > @@ -7644,12 +7644,6 @@ with zeroes)." }; > style = RErr, [String (Device, "device")], [OBool "extunwritten"; OBool > "imgfile"; OBool "v2log"; OBool "projid32bit"; OBool "lazycounter"; OString > "label"; OString "uuid"]; > optional = Some "xfs"; > tests = [ > -InitEmpty, Always, TestResult ( > - [["part_disk"; "/dev/sda"; "mbr"]; > - ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; > - ["xfs_admin"; "/dev/sda1"; ""; ""; ""; ""; "false"; "NOARG"; > "NOARG"]; > - ["mount"; "/dev/sda1"; "/"]; > - ["xfs_info"; "/"]], "ret->xfs_lazycount == 0"), []; > InitEmpty, Always, TestResultString ( >[["part_disk"; "/dev/sda"; "mbr"]; > ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; > Patch pushed as commit 627f808e4b0a. Thanks Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
