Re: [Libguestfs] nbdkit background threads

2020-02-11 Thread Richard W.M. Jones
On Mon, Feb 10, 2020 at 01:52:25PM -0600, Eric Blake wrote: > On 2/10/20 1:39 PM, Richard W.M. Jones wrote: > >https://github.com/libguestfs/nbdkit/blob/ecef5b16359fb5af7e7abf4fd2fb3ad5438b16be/TODO#L76 > > > >Already existing filters (readahead, cache) could be improved if > >filters could open a

Re: [Libguestfs] [nbdkit PATCH 03/10] filters: Wire up filter support for NBD_INFO_INIT_STATE

2020-02-11 Thread Richard W.M. Jones
On Mon, Feb 10, 2020 at 03:43:56PM -0600, Eric Blake wrote: > diff --git a/filters/noextents/noextents.c b/filters/noextents/noextents.c > index e6ac33b..091f30b 100644 > --- a/filters/noextents/noextents.c > +++ b/filters/noextents/noextents.c > @@ -1,5 +1,5 @@ > /* nbdkit > - * Copyright (C) 201

Re: [Libguestfs] [nbdkit PATCH 04/10] plugins: Wire up in-memory plugin support for NBD_INFO_INIT_STATE

2020-02-11 Thread Richard W.M. Jones
On Mon, Feb 10, 2020 at 03:43:57PM -0600, Eric Blake wrote: > +/* Does current client start with a sparse image. */ > +static int > +memory_init_sparse (void *handle) > +{ > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); > + return sparse_array_is_sparse (sa); > +} > + > +/* Does current client start

Re: [Libguestfs] [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE

2020-02-11 Thread Richard W.M. Jones
On Mon, Feb 10, 2020 at 03:43:58PM -0600, Eric Blake wrote: > @@ -214,6 +217,52 @@ file_open (int readonly) >h->can_fallocate = true; >h->can_zeroout = h->is_block_device; > > + h->can_extents = false; > + h->init_sparse = false; > + h->init_zero = false; > +#ifdef SEEK_HOLE > + if (!h

Re: [Libguestfs] [nbdkit PATCH 10/10] plugins: Wire up nbd plugin support for NBD_INFO_INIT_STATE

2020-02-11 Thread Richard W.M. Jones
Aside from my comments made on particular patches, this patch series is fine. I do think we should split up the libnbd get_init_flags into two separate calls though, and that changes some of the nbdkit tests a little. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.

Re: [Libguestfs] [PATCH] lib: allow to specify physical/logical block size for disks

2020-02-11 Thread Richard W.M. Jones
On Tue, Feb 11, 2020 at 01:36:16AM +0200, Mykola Ivanets wrote: > +=item C > + > +The integer parameter C allows specifying both physical and > logical > +block size the disk will report to the libguestfs appliance. > + > +Physical block size would be the value returned by the C ioctl > and > +de

[Libguestfs] [nbdkit PATCH] filters: Make nxdata persistent

2020-02-11 Thread Eric Blake
Future patches want to allow a filter to pass a single opaque parameter into another framework (such as ext2fs_open) or even spawn a helper thread, which requires that nxdata be stable for the entire life of the connection between .open and .close. Our current approach of creating a stack-allocate

Re: [Libguestfs] nbdkit background threads

2020-02-11 Thread Eric Blake
On 2/11/20 2:41 AM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 01:52:25PM -0600, Eric Blake wrote: On 2/10/20 1:39 PM, Richard W.M. Jones wrote: https://github.com/libguestfs/nbdkit/blob/ecef5b16359fb5af7e7abf4fd2fb3ad5438b16be/TODO#L76 Already existing filters (readahead, cache) could

[Libguestfs] [PATCH v2] lib: add support for disks with 4096 bytes sector size

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets Nowadays there are hard drives and operating systems which support "4K native" sector size. In this mode physical and logical block size exposed to the operating system is equal to 4096 bytes. GPT partition table (as a known example) being created in this mode will place G

Re: [Libguestfs] [nbdkit PATCH 03/10] filters: Wire up filter support for NBD_INFO_INIT_STATE

2020-02-11 Thread Eric Blake
On 2/11/20 4:40 AM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 03:43:56PM -0600, Eric Blake wrote: extentlist (answer based on the extents we are advertising), noextents (suppress the advertisement), and truncate (advertise sparse support diff --git a/filters/noextents/noextents.c b

Re: [Libguestfs] [nbdkit PATCH 04/10] plugins: Wire up in-memory plugin support for NBD_INFO_INIT_STATE

2020-02-11 Thread Eric Blake
On 2/11/20 4:48 AM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 03:43:57PM -0600, Eric Blake wrote: +/* Does current client start with a sparse image. */ +static int +memory_init_sparse (void *handle) +{ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + return sparse_array_is_sparse (sa); +

Re: [Libguestfs] [nbdkit PATCH] filters: Make nxdata persistent

2020-02-11 Thread Richard W.M. Jones
Thanks - I pushed this, with a single line adjustment to get around a conflict with the whitespace change in commit 9414a16ba68a. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora

Re: [Libguestfs] [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE

2020-02-11 Thread Eric Blake
On 2/11/20 4:52 AM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 03:43:58PM -0600, Eric Blake wrote: @@ -214,6 +217,52 @@ file_open (int readonly) h->can_fallocate = true; h->can_zeroout = h->is_block_device; + h->can_extents = false; + h->init_sparse = false; + h->init_zero = f

Re: [Libguestfs] [PATCH 1/2] firstboot: use absolute path to rhsrvany

2020-02-11 Thread Richard W.M. Jones
On Thu, Nov 21, 2019 at 12:04:17PM +0100, Tomáš Golembiovský wrote: > Signed-off-by: Tomáš Golembiovský > --- > mlcustomize/firstboot.ml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mlcustomize/firstboot.ml b/mlcustomize/firstboot.ml > index a8f4ef7..c3ebfd9 100644 >

Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE

2020-02-11 Thread Eric Blake
On 2/10/20 4:52 PM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 04:29:53PM -0600, Eric Blake wrote: On 2/10/20 4:12 PM, Richard W.M. Jones wrote: On Mon, Feb 10, 2020 at 03:37:20PM -0600, Eric Blake wrote: For now, only 2 of those 16 bits are defined: NBD_INIT_SPARSE (the image has at le

Re: [Libguestfs] [PATCH 2/2] firstboot: schedule firstboot as delayed task

2020-02-11 Thread Richard W.M. Jones
On Thu, Nov 21, 2019 at 12:04:18PM +0100, Tomáš Golembiovský wrote: > Instead of running firstboot scripts during early boot schedule a task > delayed for 1-2 minute. > > During the first boot, after virt-v2v conversion, Windows installs the > drivers injected by virit-v2v. When this installation

Re: [Libguestfs] [PATCH v2] lib: add support for disks with 4096 bytes sector size

2020-02-11 Thread Richard W.M. Jones
I pushed this with some trailing whitespace fixes, and I dropped the change to tmp/.gitignore since the test does clean up after itself. I also fixed test-qemu-drive-with-blocksize-libvirt.sh so it doesn't actually open /dev/sda etc on the host (don't run tests as root!) However ... We already

Re: [Libguestfs] [PATCH v2] lib: add support for disks with 4096 bytes sector size

2020-02-11 Thread Nikolay Ivanets
вт, 11 лют. 2020 о 17:20 Richard W.M. Jones пише: > > I pushed this with some trailing whitespace fixes, and I dropped the > change to tmp/.gitignore since the test does clean up after itself. I > also fixed test-qemu-drive-with-blocksize-libvirt.sh so it doesn't > actually open /dev/sda etc on t

[Libguestfs] [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

2020-02-11 Thread Richard W.M. Jones
Since we're going to be calling this function a lot, add a short alias for it. --- server/internal.h | 1 + server/public.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/internal.h b/server/internal.h index a1fa7309..1e7b4cf0 100644 --- a/server/internal.h +++

[Libguestfs] [PATCH nbdkit 2/3] server: connections: Don't free TLS storage of conn until end of function.

2020-02-11 Thread Richard W.M. Jones
We want to do this as late as possible. In particular, backend_close may call plugin.close which may (although it's unlikely) call one of the functions which relies on the implicit connection pointer fetched through thread-local storage (TLS) which would then fail. --- server/connections.c | 2 +-

[Libguestfs] [PATCH nbdkit 0/3] server: Remove explicit connection parameter.

2020-02-11 Thread Richard W.M. Jones
The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich. _

Re: [Libguestfs] [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

2020-02-11 Thread Eric Blake
On 2/11/20 11:15 AM, Richard W.M. Jones wrote: Since we're going to be calling this function a lot, add a short alias for it. --- server/internal.h | 1 + server/public.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/internal.h b/server/internal.h index a

Re: [Libguestfs] [PATCH v2] lib: add support for disks with 4096 bytes sector size

2020-02-11 Thread Richard W.M. Jones
On Tue, Feb 11, 2020 at 06:16:25PM +0200, Nikolay Ivanets wrote: > вт, 11 лют. 2020 о 17:20 Richard W.M. Jones пише: > > > > I pushed this with some trailing whitespace fixes, and I dropped the > > change to tmp/.gitignore since the test does clean up after itself. I > > also fixed test-qemu-driv

Re: [Libguestfs] [PATCH nbdkit 1/3] server: Add GET_CONN macro, alias for threadlocal_get_conn ().

2020-02-11 Thread Richard W.M. Jones
On Tue, Feb 11, 2020 at 11:25:49AM -0600, Eric Blake wrote: > On 2/11/20 11:15 AM, Richard W.M. Jones wrote: > >Since we're going to be calling this function a lot, add a short alias > >for it. > >--- > > server/internal.h | 1 + > > server/public.c | 6 +++--- > > 2 files changed, 4 insertions(

[Libguestfs] [PATCH nbdkit v2 2/3] server: connections: Don't free TLS storage of conn until end of function.

2020-02-11 Thread Richard W.M. Jones
We want to do this as late as possible. In particular, backend_close may call plugin.close which may (although it's unlikely) call one of the functions which relies on the implicit connection pointer fetched through thread-local storage (TLS) which would then fail. --- server/connections.c | 2 +-

[Libguestfs] [PATCH nbdkit v2 0/3] server: Remove explicit connection parameter.

2020-02-11 Thread Richard W.M. Jones
v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must us

[Libguestfs] [PATCH nbdkit v2 1/3] server: Add GET_CONN macro around threadlocal_get_conn ().

2020-02-11 Thread Richard W.M. Jones
Since we're going to be calling this function a lot, add a short alias for setting 'struct connection *conn' within a function. --- server/internal.h | 9 + 1 file changed, 9 insertions(+) diff --git a/server/internal.h b/server/internal.h index a1fa7309..52c16a4e 100644 --- a/server/inte

Re: [Libguestfs] [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.

2020-02-11 Thread Eric Blake
On 2/11/20 11:15 AM, Richard W.M. Jones wrote: Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage. In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value

Re: [Libguestfs] [PATCH nbdkit 3/3] server: Remove explicit connection parameter, use TLS instead.

2020-02-11 Thread Richard W.M. Jones
On Tue, Feb 11, 2020 at 11:46:06AM -0600, Eric Blake wrote: > On 2/11/20 11:15 AM, Richard W.M. Jones wrote: > >Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored > >the connection object in thread-local storage. > > > >In this very large, but mostly mechanical change we stop pass

Re: [Libguestfs] [PATCH nbdkit v2 0/3] server: Remove explicit connection parameter.

2020-02-11 Thread Eric Blake
On 2/11/20 11:39 AM, Richard W.M. Jones wrote: v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn

Re: [Libguestfs] [PATCH 2/2] firstboot: schedule firstboot as delayed task

2020-02-11 Thread Nikolay Ivanets
> What is "~dpnx0"? That is a series of substitutions of batch parameter. Here is Microsoft doc: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/call And here is more clear step-by-step explanation: https://stackoverflow.com/a/3679781/716075 -- Mykola Ivanets

[Libguestfs] [common PATCH] options: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch adds '--blocksize' command line option parsing and handling for guestfish and other C-based tools which share the same code. --- options/options.c | 13 - options/options.h | 125 +++--- 2 files changed, 86 insertions(

[Libguestfs] [PATCH 1/1] tools: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch adds '--blocksize' command line option for guestfish and other C-based tools. This option allows specifying disk sector size. --- align/scan.c | 8 align/virt-alignment-scan.pod | 12 cat/cat.c | 8 +++

[Libguestfs] [PATCH 0/1] tools: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch depends on changes in 'common' sub-module posted here: https://www.redhat.com/archives/libguestfs/2020-February/msg00096.html Nikolay Ivanets (1): tools: add '--blocksize' option for C-based tools align/scan.c | 8 align/virt-alignm

[Libguestfs] [common PATCH v2 1/1] options: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch adds '--blocksize' command line option parsing and handling for guestfish and other C-based tools which share the same code from this sub-module. '--blocksize' will be a common for almost all libguestfs-based tools and thus parameter description will be repeated

[Libguestfs] [common PATCH v2 0/1] options: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets In v2 I've moved '--blocksize' parameter description into the separate file called blocksize-option.pod so we can include it everywhere we need similar to key-option.pod. v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00096.html Nikolay Ivanets (1

[Libguestfs] [PATCH v2 1/1] tools: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch adds '--blocksize' command line option for guestfish and other C-based tools. This option allows specifying disk sector size. --- align/Makefile.am | 1 + align/scan.c | 8 align/virt-alignment-scan.pod | 2 ++ cat/Makefile.

[Libguestfs] [PATCH v2 0/1] tools: add '--blocksize' option for C-based tools

2020-02-11 Thread Mykola Ivanets
From: Nikolay Ivanets This patch depends on changes in 'common' sub-module posted here: https://www.redhat.com/archives/libguestfs/2020-February/msg00099.html v2: Almost the same as v1 except '--blocksize' option description is moved into a common submodule (similar to key-option.pod). v1 was h

[Libguestfs] [nbdkit PATCH] server: Correct logic when filter fails .prepare

2020-02-11 Thread Eric Blake
If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepared'), which led to an assertion failure.

Re: [Libguestfs] [common PATCH v2 1/1] options: add '--blocksize' option for C-based tools

2020-02-11 Thread Eric Blake
On 2/11/20 6:54 PM, Mykola Ivanets wrote: From: Nikolay Ivanets This patch adds '--blocksize' command line option parsing and handling for guestfish and other C-based tools which share the same code from this sub-module. '--blocksize' will be a common for almost all libguestfs-based tools and

Re: [Libguestfs] [nbdkit PATCH] server: Correct logic when filter fails .prepare

2020-02-11 Thread Eric Blake
On 2/11/20 7:24 PM, Eric Blake wrote: If .prepare fails, we do not want to call .finalize (similar to how if .open fails, we do not want to call .close). However, the logic in backend_finalize was checking on the wrong condition ('was the connection opened' rather than 'was the connection prepar

[Libguestfs] [nbdkit PATCH 2/3] ext2: Deprecate ext2 plugin

2020-02-11 Thread Eric Blake
Plan to remove it in the next-but-one stable release. Signed-off-by: Eric Blake --- plugins/ext2/nbdkit-ext2-plugin.pod | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/ext2/nbdkit-ext2-plugin.pod b/plugins/ext2/nbdkit-ext2-plugin.pod index b8d6c21e..5691

[Libguestfs] [nbdkit PATCH 0/3] Make ext2 a filter

2020-02-11 Thread Eric Blake
I'm impressed that I was able to whip this out in just one day of hacking. Below, I'll include a diff between the plugin and the filter as of patch 1, if it aids review. Eric Blake (3): filters: Add ext2 filter ext2: Deprecate ext2 plugin ext2: Add mode for letting client exportname choose

[Libguestfs] [nbdkit PATCH 3/3] ext2: Add mode for letting client exportname choose file from image

2020-02-11 Thread Eric Blake
Not enabled by default, because of the potential security concern that a client can use this to sniff what file names exist in the image; but for an opt-in setting in the server, this lets a single nbdkit process serve multiple files from an ext2 image (one at a time, unless we find a way to allow

[Libguestfs] [nbdkit PATCH 1/3] filters: Add ext2 filter

2020-02-11 Thread Eric Blake
The ext2 plugin does not work with partitions, and requires the disk image to be in the local filesystem. Better is to have a filter that lets any other plugin be the disk image (including with the partition filter), where the ext2 code then calls into our backend code. But since our backend code

Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE

2020-02-11 Thread Wouter Verhelst
Hi, On Mon, Feb 10, 2020 at 10:52:55PM +, Richard W.M. Jones wrote: > But anyway ... could a flag indicating that the whole image is sparse > be useful, either as well as NBD_INIT_SPARSE or instead of it? You > could use it to avoid an initial disk trim, which is something that > mke2fs does: