Re: [Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
On Mon, Feb 21, 2022 at 04:00:11PM -0600, Eric Blake wrote:
> We were previously enforcing minimum block size with EINVAL for
> too-small requests. Advertise this to the client.
> ---
> filters/swab/nbdkit-swab-filter.pod | 6 ++
> filters/swab/swab.c | 24 +++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/filters/swab/nbdkit-swab-filter.pod
> b/filters/swab/nbdkit-swab-filter.pod
> index f8500150..030a0852 100644
> --- a/filters/swab/nbdkit-swab-filter.pod
> +++ b/filters/swab/nbdkit-swab-filter.pod
> @@ -34,6 +34,11 @@ the last few bytes, combine this filter with
> L; fortunately, sector-based disk images
> are already suitably sized.
>
> +Note that this filter fails operations that are not aligned to the
> +swab-bits boundaries; if you need byte-level access, apply the
> +L before this one, to get
> +read-modify-write access to individual bytes.
> +
> =head1 PARAMETERS
>
> =over 4
> @@ -90,6 +95,7 @@ L,
> L,
> L,
> L,
> +L.
> L.
>
> =head1 AUTHORS
> diff --git a/filters/swab/swab.c b/filters/swab/swab.c
> index 68776eee..6e8dc981 100644
> --- a/filters/swab/swab.c
> +++ b/filters/swab/swab.c
> @@ -44,6 +44,7 @@
> #include "byte-swapping.h"
> #include "isaligned.h"
> #include "cleanup.h"
> +#include "minmax.h"
> #include "rounding.h"
>
> /* Can only be 8 (filter disabled), 16, 32 or 64. */
> @@ -85,8 +86,28 @@ swab_get_size (nbdkit_next *next,
>return ROUND_DOWN (size, bits/8);
> }
>
> +/* Block size constraints. */
> +static int
> +swab_block_size (nbdkit_next *next, void *handle,
> + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*minimum == 0) { /* No constraints set by the plugin. */
> +*minimum = bits/8;
> +*preferred = 512;
> +*maximum = 0x;
> + }
> + else {
> +*minimum = MAX (*minimum, bits/8);
> + }
> +
> + return 0;
> +}
> +
> /* The request must be aligned.
> - * XXX We could lift this restriction with more work.
> + * If you want finer alignment, use the blocksize filter.
> */
> static bool
> is_aligned (uint32_t count, uint64_t offset, int *err)
> @@ -220,6 +241,7 @@ static struct nbdkit_filter filter = {
>.config= swab_config,
>.config_help = swab_config_help,
>.get_size = swab_get_size,
> + .block_size= swab_block_size,
>.pread = swab_pread,
>.pwrite= swab_pwrite,
>.trim = swab_trim,
Looks sensible, ACK
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [nbdkit PATCH] swab: Implement .block_size callback
We were previously enforcing minimum block size with EINVAL for
too-small requests. Advertise this to the client.
---
filters/swab/nbdkit-swab-filter.pod | 6 ++
filters/swab/swab.c | 24 +++-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/filters/swab/nbdkit-swab-filter.pod
b/filters/swab/nbdkit-swab-filter.pod
index f8500150..030a0852 100644
--- a/filters/swab/nbdkit-swab-filter.pod
+++ b/filters/swab/nbdkit-swab-filter.pod
@@ -34,6 +34,11 @@ the last few bytes, combine this filter with
L; fortunately, sector-based disk images
are already suitably sized.
+Note that this filter fails operations that are not aligned to the
+swab-bits boundaries; if you need byte-level access, apply the
+L before this one, to get
+read-modify-write access to individual bytes.
+
=head1 PARAMETERS
=over 4
@@ -90,6 +95,7 @@ L,
L,
L,
L,
+L.
L.
=head1 AUTHORS
diff --git a/filters/swab/swab.c b/filters/swab/swab.c
index 68776eee..6e8dc981 100644
--- a/filters/swab/swab.c
+++ b/filters/swab/swab.c
@@ -44,6 +44,7 @@
#include "byte-swapping.h"
#include "isaligned.h"
#include "cleanup.h"
+#include "minmax.h"
#include "rounding.h"
/* Can only be 8 (filter disabled), 16, 32 or 64. */
@@ -85,8 +86,28 @@ swab_get_size (nbdkit_next *next,
return ROUND_DOWN (size, bits/8);
}
+/* Block size constraints. */
+static int
+swab_block_size (nbdkit_next *next, void *handle,
+ uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
+{
+ if (next->block_size (next, minimum, preferred, maximum) == -1)
+return -1;
+
+ if (*minimum == 0) { /* No constraints set by the plugin. */
+*minimum = bits/8;
+*preferred = 512;
+*maximum = 0x;
+ }
+ else {
+*minimum = MAX (*minimum, bits/8);
+ }
+
+ return 0;
+}
+
/* The request must be aligned.
- * XXX We could lift this restriction with more work.
+ * If you want finer alignment, use the blocksize filter.
*/
static bool
is_aligned (uint32_t count, uint64_t offset, int *err)
@@ -220,6 +241,7 @@ static struct nbdkit_filter filter = {
.config= swab_config,
.config_help = swab_config_help,
.get_size = swab_get_size,
+ .block_size= swab_block_size,
.pread = swab_pread,
.pwrite= swab_pwrite,
.trim = swab_trim,
--
2.35.1
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit PATCH] log: Implement .block_size callback
On Mon, Feb 21, 2022 at 07:32:41PM +, Richard W.M. Jones wrote: > On Mon, Feb 21, 2022 at 10:59:39AM -0600, Eric Blake wrote: > > Enhance the amount of information logged during connection. > > --- > > filters/log/nbdkit-log-filter.pod | 2 +- > > filters/log/log.c | 12 > > 2 files changed, 9 insertions(+), 5 deletions(-) > > > > ACK > > Rich. Now in as 842bf64122 The swab filter should probably also consider a .block_size; there, we can still support unaligned reads, but due to the overhead of byte swapping, we want to ensure preferred size is at least our swap alignment (but then again, the NBD spec says a preferred size has to be at least 512, which is larger than our swap alignment. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] blocksize: Export block size constraints
On Sun, Feb 20, 2022 at 08:56:37PM +, Richard W.M. Jones wrote:
> This filter is a little unusual because it allows clients to send a
> wider range of request sizes than the underlying plugin allows.
> Therefore we advertise the widest possible minimum and maximum block
> size to clients.
>
> We still need to pick a suitable preferred block size assuming the
> plugin itself doesn't advertise one.
> ---
> filters/blocksize/blocksize.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
> index a6fa00cb..46e759b0 100644
> --- a/filters/blocksize/blocksize.c
> +++ b/filters/blocksize/blocksize.c
> @@ -156,6 +156,29 @@ blocksize_get_size (nbdkit_next *next,
>return ROUND_DOWN (size, minblock);
> }
>
> +/* Block size constraints.
> + *
> + * Note that the purpose of this filter is to allow clients to send a
> + * wider range of request sizes than the underlying plugin permits,
> + * and therefore this callback advertises the full availability of the
> + * filter, likely widening the constraints from the plugin.
> + */
> +static int
> +blocksize_block_size (nbdkit_next *next, void *handle,
> + uint32_t *minimum, uint32_t *preferred, uint32_t
> *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*preferred == 0)
> +*preferred = MAX (4096, minblock);
Hmm. Even if minblock > *maximum reported by the plugin,...
> +
> + *minimum = 1;
> + *maximum = 0x;
...our advertisement to the client does not trigger any asserts.
ACK.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints
On Sun, Feb 20, 2022 at 08:49:08PM +, Richard W.M. Jones wrote:
> Because these filters perform a read-modify-write cycle for requests
> which are smaller than the block size of the filter, we can adjust or
> set the preferred block size to the block size of the filter or the
> preferred block size of the underlying plugin, whichever is larger.
>
> We're careful not to set a preferred block size which is larger than
> the maximum block size.
>
> diff --git a/filters/cache/cache.c b/filters/cache/cache.c
> index c912c5fb..f0ee42f1 100644
> --- a/filters/cache/cache.c
> +++ b/filters/cache/cache.c
> @@ -260,6 +260,26 @@ cache_get_size (nbdkit_next *next,
>return size;
> }
>
> +/* Block size constraints. */
> +static int
> +cache_block_size (nbdkit_next *next, void *handle,
> + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*minimum == 0) { /* No constraints set by the plugin. */
> +*minimum = 1;
> +*preferred = blksize;
> +*maximum = 0x;
> + }
> + else if (*maximum >= blksize) {
> +*preferred = MAX (*preferred, blksize);
> + }
We're not changing the minimum or maximum (use blocksize-policy filter
if we want to enforce that before it reaches the plugin), but what you
are changing here makes sense to me - we really are running a
potentially larger preferred I/O size than the plugin based on how our
cache blocks work.
ACK.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit PATCH] log: Implement .block_size callback
On Mon, Feb 21, 2022 at 10:59:39AM -0600, Eric Blake wrote:
> Enhance the amount of information logged during connection.
> ---
> filters/log/nbdkit-log-filter.pod | 2 +-
> filters/log/log.c | 12
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/filters/log/nbdkit-log-filter.pod
> b/filters/log/nbdkit-log-filter.pod
> index dd04f1ab..e0a0c209 100644
> --- a/filters/log/nbdkit-log-filter.pod
> +++ b/filters/log/nbdkit-log-filter.pod
> @@ -81,7 +81,7 @@ before performing a single successful read is:
>
> 2020-08-06 02:07:23.080415 ListExports id=1 readonly=0 tls=0 ...
> 2020-08-06 02:07:23.080502 ...ListExports id=1 exports=("") return=0
> - 2020-08-06 02:07:23.080712 connection=1 Connect export="" tls=0 size=0x400
> write=1 flush=1 rotational=0 trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1
> + 2020-08-06 02:07:23.080712 connection=1 Connect export="" tls=0 size=0x400
> minsize=0x1 prefsize=0x200 maxsize=0x write=1 flush=1 rotational=0
> trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1
> 2020-08-06 02:07:23.080907 connection=1 Read id=1 offset=0x0 count=0x200 ...
> 2020-08-06 02:07:23.080927 connection=1 ...Read id=1 return=0
> 2020-08-06 02:07:23.081255 connection=1 Disconnect transactions=1
> diff --git a/filters/log/log.c b/filters/log/log.c
> index acc9d77e..4255f85f 100644
> --- a/filters/log/log.c
> +++ b/filters/log/log.c
> @@ -1,5 +1,5 @@
> /* nbdkit
> - * Copyright (C) 2018-2020 Red Hat Inc.
> + * Copyright (C) 2018-2022 Red Hat Inc.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions are
> @@ -251,6 +251,7 @@ log_prepare (nbdkit_next *next, void *handle,
>struct handle *h = handle;
>const char *exportname = h->exportname;
>int64_t size = next->get_size (next);
> + uint32_t minsize, prefsize, maxsize;
>int w = next->can_write (next);
>int f = next->can_flush (next);
>int r = next->is_rotational (next);
> @@ -260,9 +261,10 @@ log_prepare (nbdkit_next *next, void *handle,
>int e = next->can_extents (next);
>int c = next->can_cache (next);
>int Z = next->can_fast_zero (next);
> + int s = next->block_size (next, , , );
>
>if (size < 0 || w < 0 || f < 0 || r < 0 || t < 0 || z < 0 || F < 0 ||
> - e < 0 || c < 0 || Z < 0)
> + e < 0 || c < 0 || Z < 0 || s < 0)
> return -1;
>
>fp = open_memstream (, );
> @@ -270,10 +272,12 @@ log_prepare (nbdkit_next *next, void *handle,
> fprintf (fp, "export=");
> shell_quote (exportname, fp);
> fprintf (fp,
> - " tls=%d size=0x%" PRIx64 " write=%d "
> + " tls=%d size=0x%" PRIx64 " minsize=0x%" PRIx32 " prefsize=0x%"
> + PRIx32 " maxsize=0x%" PRIx32 " write=%d "
> "flush=%d rotational=%d trim=%d zero=%d fua=%d extents=%d "
> "cache=%d fast_zero=%d",
> - h->tls, size, w, f, r, t, z, F, e, c, Z);
> + h->tls, size, minsize, prefsize, maxsize,
> + w, f, r, t, z, F, e, c, Z);
> fclose (fp);
> print (h, "Connect", "%s", str);
>}
ACK
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [nbdkit PATCH] log: Implement .block_size callback
Enhance the amount of information logged during connection.
---
filters/log/nbdkit-log-filter.pod | 2 +-
filters/log/log.c | 12
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/filters/log/nbdkit-log-filter.pod
b/filters/log/nbdkit-log-filter.pod
index dd04f1ab..e0a0c209 100644
--- a/filters/log/nbdkit-log-filter.pod
+++ b/filters/log/nbdkit-log-filter.pod
@@ -81,7 +81,7 @@ before performing a single successful read is:
2020-08-06 02:07:23.080415 ListExports id=1 readonly=0 tls=0 ...
2020-08-06 02:07:23.080502 ...ListExports id=1 exports=("") return=0
- 2020-08-06 02:07:23.080712 connection=1 Connect export="" tls=0 size=0x400
write=1 flush=1 rotational=0 trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1
+ 2020-08-06 02:07:23.080712 connection=1 Connect export="" tls=0 size=0x400
minsize=0x1 prefsize=0x200 maxsize=0x write=1 flush=1 rotational=0
trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1
2020-08-06 02:07:23.080907 connection=1 Read id=1 offset=0x0 count=0x200 ...
2020-08-06 02:07:23.080927 connection=1 ...Read id=1 return=0
2020-08-06 02:07:23.081255 connection=1 Disconnect transactions=1
diff --git a/filters/log/log.c b/filters/log/log.c
index acc9d77e..4255f85f 100644
--- a/filters/log/log.c
+++ b/filters/log/log.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018-2020 Red Hat Inc.
+ * Copyright (C) 2018-2022 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -251,6 +251,7 @@ log_prepare (nbdkit_next *next, void *handle,
struct handle *h = handle;
const char *exportname = h->exportname;
int64_t size = next->get_size (next);
+ uint32_t minsize, prefsize, maxsize;
int w = next->can_write (next);
int f = next->can_flush (next);
int r = next->is_rotational (next);
@@ -260,9 +261,10 @@ log_prepare (nbdkit_next *next, void *handle,
int e = next->can_extents (next);
int c = next->can_cache (next);
int Z = next->can_fast_zero (next);
+ int s = next->block_size (next, , , );
if (size < 0 || w < 0 || f < 0 || r < 0 || t < 0 || z < 0 || F < 0 ||
- e < 0 || c < 0 || Z < 0)
+ e < 0 || c < 0 || Z < 0 || s < 0)
return -1;
fp = open_memstream (, );
@@ -270,10 +272,12 @@ log_prepare (nbdkit_next *next, void *handle,
fprintf (fp, "export=");
shell_quote (exportname, fp);
fprintf (fp,
- " tls=%d size=0x%" PRIx64 " write=%d "
+ " tls=%d size=0x%" PRIx64 " minsize=0x%" PRIx32 " prefsize=0x%"
+ PRIx32 " maxsize=0x%" PRIx32 " write=%d "
"flush=%d rotational=%d trim=%d zero=%d fua=%d extents=%d "
"cache=%d fast_zero=%d",
- h->tls, size, w, f, r, t, z, F, e, c, Z);
+ h->tls, size, minsize, prefsize, maxsize,
+ w, f, r, t, z, F, e, c, Z);
fclose (fp);
print (h, "Connect", "%s", str);
}
--
2.35.1
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] how can I run a subset of the tests?
On Mon, Feb 21, 2022 at 03:10:02PM +0100, Laszlo Ersek wrote: > Hi, > > "make check" in libguestfs takes very long (especially when it's run > after every patch in a series). > > How can I run only those tests that are, for example, in "tests/luks/"? This used to be possible before: commit 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c Author: Richard W.M. Jones Date: Thu Mar 18 11:15:06 2021 + tests: Run the tests in parallel. As far as I know it's no longer possible to run just tests from a single directory. However - a bit clumsy - if you know the exact list of tests you want to run then this works: $ make -C tests check TESTS=" luks/test-luks.sh luks/test-luks-list.sh luks/test-key-option.sh luks/test-key-option-inspect.sh " ... make[3]: Entering directory '/home/rjones/d/libguestfs/tests' SKIP: luks/test-key-option-inspect.sh PASS: luks/test-luks-list.sh PASS: luks/test-key-option.sh PASS: luks/test-luks.sh Testsuite summary for libguestfs 1.47.2 # TOTAL: 4 # PASS: 3 # SKIP: 1 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 make[3]: Leaving directory '/home/rjones/d/libguestfs/tests' make[2]: Leaving directory '/home/rjones/d/libguestfs/tests' make[1]: Leaving directory '/home/rjones/d/libguestfs/tests' make: Leaving directory '/home/rjones/d/libguestfs/tests' Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] how can I run a subset of the tests?
On Mon, Feb 21, 2022 at 03:10:02PM +0100, Laszlo Ersek wrote: > Hi, > > "make check" in libguestfs takes very long (especially when it's run > after every patch in a series). > > How can I run only those tests that are, for example, in "tests/luks/"? Disclaimer: The following advice works for libnbd and nbdkit, but I have not tested it on libguestfs: For automake-based projects that still use directory recursion, 'make -C tests/luks check' will run the tests in just one subdirectory, and 'make -C path/to/dir check TESTS=name_of_test' will run just one test. If we ever switch to non-recursive automake, or even to a different build system (qemu and libvirt both switched to meson), then I'll have to relearn tricks. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 8/8] copy: Adaptive queue size
On Sun, Feb 20, 2022 at 02:14:03PM +0200, Nir Soffer wrote:
> Limit the size of the copy queue also by the number of queued bytes.
> This allows using many concurrent small requests, required to get good
> performance, but limiting the number of large requests that are actually
> faster with lower concurrency.
>
> New --queue-size option added to control the maximum queue size. With 2
> MiB we can have 8 256 KiB requests per connection. The default queue
> size is 16 MiB, to match the default --requests value (64) with the
> default --request-size (256 KiB). Testing show that using more than 16
> 256 KiB requests with one connection do not improve anything.
s/do/does/
>
> The new option will simplify limiting memory usage when using large
> requests, like this change in virt-v2v:
> https://github.com/libguestfs/virt-v2v/commit/c943420219fa0ee971fc228aa4d9127c5ce973f7
>
> I tested this change with 3 images:
>
> - Fedora 35 + 3g of random data - hopefully simulates a real image
> - Fully allocated image - the special case when every read command is
> converted to a write command.
> - Zero image - the special case when every read command is converted to
> a zero command.
>
> On 2 machines:
>
> - laptop: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz, 12 cpus,
> 1.5 MiB L2 cache per 2 cpus, 12 MiB L3 cache.
> - server: Intel(R) Xeon(R) Gold 5218R CPU @ 2.10GHz, 80 cpus,
> 1 MiB L2 cache per cpu, 27.5 MiB L3 cache.
>
> In all cases, both source and destination are served by qemu-nbd, using
> --cache=none --aio=native. Because qemu-nbd does not support MULTI_CON
MULTI_CONN
> for writing, we are testing a single connection when copying an to
Did you mean 'copying an image to'?
> qemu-nbd. I tested also copying to null: since in this case we use 4
> connections (these tests are marked with /ro).
>
> Results for copying all images on all machines with nbdcopy v1.11.0 and
> this change. "before" and "after" are average time of 10 runs.
>
> image machinebeforeafterqueue sizeimprovement
> ===
> fedora laptop 3.0442.129 2m +43%
> fulllaptop 4.9003.136 2m +56%
> zerolaptop 3.1472.624 2m +20%
> ---
> fedora server 2.3242.189 16m+6%
> fullserver 3.5213.380 8m+4%
> zeroserver 2.2972.338 16m-2%
> ---
> fedora/ro laptop 2.0401.663 1m +23%
> fedora/ro server 1.5851.393 2m +14%
>
> Signed-off-by: Nir Soffer
> ---
> copy/main.c | 52 -
> copy/multi-thread-copying.c | 18 +++--
> copy/nbdcopy.h | 1 +
> copy/nbdcopy.pod| 12 +++--
> 4 files changed, 55 insertions(+), 28 deletions(-)
>
> static void __attribute__((noreturn))
> usage (FILE *fp, int exitcode)
> {
>fprintf (fp,
> "\n"
> "Copy to and from an NBD server:\n"
> "\n"
> "nbdcopy [--allocated] [-C N|--connections=N]\n"
> "[--destination-is-zero|--target-is-zero] [--flush]\n"
> "[--no-extents] [-p|--progress|--progress=FD]\n"
> -"[--request-size=N] [-R N|--requests=N] [-S N|--sparse=N]\n"
> -"[--synchronous] [-T N|--threads=N] [-v|--verbose]\n"
> +"[--request-size=N] [--queue-size=N] [-R N|--requests=N]\n"
The options are listed in mostly alphabetic order already, so
--queue-size before --request-size makes more sense to me.
> @@ -104,33 +106,35 @@ main (int argc, char *argv[])
> {
>enum {
> HELP_OPTION = CHAR_MAX + 1,
> LONG_OPTIONS,
> SHORT_OPTIONS,
> ALLOCATED_OPTION,
> DESTINATION_IS_ZERO_OPTION,
> FLUSH_OPTION,
> NO_EXTENTS_OPTION,
> REQUEST_SIZE_OPTION,
> +QUEUE_SIZE_OPTION,
Likewise here.
> SYNCHRONOUS_OPTION,
>};
>const char *short_options = "C:pR:S:T:vV";
>const struct option long_options[] = {
> { "help", no_argument, NULL, HELP_OPTION },
> { "long-options", no_argument, NULL, LONG_OPTIONS },
> { "allocated", no_argument, NULL, ALLOCATED_OPTION },
> { "connections",required_argument, NULL, 'C' },
> { "destination-is-zero",no_argument, NULL,
> DESTINATION_IS_ZERO_OPTION },
> { "flush", no_argument, NULL, FLUSH_OPTION },
> { "no-extents", no_argument, NULL, NO_EXTENTS_OPTION },
> { "progress", optional_argument, NULL, 'p' },
> { "request-size", required_argument, NULL, REQUEST_SIZE_OPTION },
> +{ "queue-size", required_argument, NULL, QUEUE_SIZE_OPTION },
Re: [Libguestfs] [PATCH libnbd 6/8] copy: Keep worker pointer in command
On Sun, Feb 20, 2022 at 02:14:01PM +0200, Nir Soffer wrote:
> Replace the command index with a worker pointer. The nbd-ops access the
> index via the worker pointer. This allows commands to modify worker
> state during processing.
>
> Signed-off-by: Nir Soffer
> ---
> copy/multi-thread-copying.c | 12 ++--
> copy/nbd-ops.c | 6 +++---
> copy/nbdcopy.h | 2 +-
> 3 files changed, 10 insertions(+), 10 deletions(-)
> struct command {
>uint64_t offset; /* Offset relative to start of disk. */
>struct slice slice; /* Data slice. */
> - size_t index; /* Thread number. */
> + struct worker *worker;/* The worker owning this command. */
> };
ACK 5 and 6. Hmm, maybe scripts/git.orderfile needs a tweak to show
copy/*.h prior to copy/*.c, but that's a separate issue.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 4/8] copy: Separate finishing a command from freeing it
On Sun, Feb 20, 2022 at 02:13:59PM +0200, Nir Soffer wrote:
> free_command() was abused as a completion callback. Introduce
> finish_command() completion callback, so code that want to free a
> command does not have to add dummy errors.
>
> This will make it easier to manage worker state when a command
> completes.
>
> Signed-off-by: Nir Soffer
> ---
> copy/multi-thread-copying.c | 34 --
> 1 file changed, 20 insertions(+), 14 deletions(-)
In addition to Rich's review,
>
> static int
> -free_command (void *vp, int *error)
> +finished_command (void *vp, int *error)
> {
>struct command *command = vp;
> - struct buffer *buffer = command->slice.buffer;
>
>if (*error) {
> fprintf (stderr, "write at offset %" PRId64 " failed: %s\n",
> command->offset, strerror (*error));
> exit (EXIT_FAILURE);
>}
>
> + free_command (command);
> +
> + return 1; /* auto-retires the command */
> +}
> +
> +static void
> +free_command (struct command *command)
> +{
> + struct buffer *buffer = command->slice.buffer;
Do we want to allow 'free_command (NULL)', in which case this should
check if command is non-NULL before initializing buffer? Doing so may
make some other cleanup paths easier to write.
But for now, all callers pass in non-NULL, so ACK either way.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 3/8] copy: Extract create_command and create_buffer helpers
On Sun, Feb 20, 2022 at 02:13:58PM +0200, Nir Soffer wrote:
> Creating a new command requires lot of boilerplate that makes it harder
> to focus on the interesting code. Extract a helpers to create a command,
> and the command slice buffer.
>
> create_buffer is called only once, but the compiler is smart enough to
> inline it, and adding it makes the code much simpler.
>
> This change is a refactoring except fixing perror() message for calloc()
> failure.
>
> Signed-off-by: Nir Soffer
> ---
> copy/multi-thread-copying.c | 87 +++--
> 1 file changed, 54 insertions(+), 33 deletions(-)
>if (exts.ptr[i].zero) {
> /* The source is zero so we can proceed directly to skipping,
> * fast zeroing, or writing zeroes at the destination.
> */
> -command = calloc (1, sizeof *command);
> -if (command == NULL) {
> - perror ("malloc");
> - exit (EXIT_FAILURE);
> -}
> -command->offset = exts.ptr[i].offset;
> -command->slice.len = exts.ptr[i].length;
> -command->slice.base = 0;
This assignment is dead code after calloc,...
> -command->index = index;
> +command = create_command (exts.ptr[i].offset, exts.ptr[i].length,
> + true, index);
> fill_dst_range_with_zeroes (command);
>}
>
>else /* data */ {
> /* As the extent might be larger than permitted for a single
> * command, we may have to split this into multiple read
> * requests.
> */
> while (exts.ptr[i].length > 0) {
>len = exts.ptr[i].length;
>if (len > request_size)
> len = request_size;
> - data = malloc (len);
> - if (data == NULL) {
> -perror ("malloc");
> -exit (EXIT_FAILURE);
> - }
> - buffer = calloc (1, sizeof *buffer);
> - if (buffer == NULL) {
> -perror ("malloc");
> -exit (EXIT_FAILURE);
> - }
> - buffer->data = data;
> - buffer->refs = 1;
> - command = calloc (1, sizeof *command);
> - if (command == NULL) {
> -perror ("malloc");
> -exit (EXIT_FAILURE);
> - }
> - command->offset = exts.ptr[i].offset;
> - command->slice.len = len;
> - command->slice.base = 0;
...as was this,...
> - command->slice.buffer = buffer;
> - command->index = index;
> +
> + command = create_command (exts.ptr[i].offset, len,
> +false, index);
>
>wait_for_request_slots (index);
>
>/* Begin the asynch read operation. */
>src->ops->asynch_read (src, command,
> (nbd_completion_callback) {
> .callback = finished_read,
> .user_data = command,
> });
>
> @@ -331,20 +305,67 @@ poll_both_ends (uintptr_t index)
> else if ((fds[1].revents & POLLOUT) != 0)
>dst->ops->asynch_notify_write (dst, index);
> else if ((fds[1].revents & (POLLERR | POLLNVAL)) != 0) {
>errno = ENOTCONN;
>perror (dst->name);
>exit (EXIT_FAILURE);
> }
>}
> }
>
> +/* Create a new buffer. */
> +static struct buffer*
> +create_buffer (size_t len)
> +{
> + struct buffer *buffer;
> +
> + buffer = calloc (1, sizeof *buffer);
> + if (buffer == NULL) {
> +perror ("calloc");
> +exit (EXIT_FAILURE);
> + }
> +
> + buffer->data = malloc (len);
> + if (buffer->data == NULL) {
> +perror ("malloc");
> +exit (EXIT_FAILURE);
> + }
> +
> + buffer->refs = 1;
> +
> + return buffer;
> +}
> +
> +/* Create a new command for read or zero. */
> +static struct command *
> +create_command (uint64_t offset, size_t len, bool zero, uintptr_t index)
> +{
> + struct command *command;
> +
> + command = calloc (1, sizeof *command);
> + if (command == NULL) {
> +perror ("calloc");
> +exit (EXIT_FAILURE);
> + }
> +
> + command->offset = offset;
> + command->slice.len = len;
> + command->slice.base = 0;
...but keeping it here for the sake of refactoring is fine.
> +
> + if (!zero)
> +command->slice.buffer = create_buffer (len);
> +
> + command->index = index;
> +
> + return command;
> +}
ACK
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 1/8] copy: Remove wrong references to holes
On Sun, Feb 20, 2022 at 02:13:56PM +0200, Nir Soffer wrote:
> In the past nbdcopy was looking for hole extents instead of zero
> extents. When we fixed this, we forgot to update some comments and
> variable names referencing hole instead of zero.
Might be nice to add:
Fixes: d5f65e36 ("copy: Do not use trim for zeroing", v1.7.3)
or whatever commit you think would be better.
>
> Signed-off-by: Nir Soffer
> ---
> copy/multi-thread-copying.c | 34 +-
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
ACK.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 2/8] copy: Rename copy_subcommand to create_subcommand
On Sun, Feb 20, 2022 at 02:13:57PM +0200, Nir Soffer wrote:
> copy_subcommand creates a new command without copying the original
> command. Rename the function to make this more clear.
>
> Signed-off-by: Nir Soffer
> ---
> copy/multi-thread-copying.c | 29 ++---
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> if (!last_is_zero) {
>/* Write the last data (if any). */
>if (i - last_offset > 0) {
> -newcommand = copy_subcommand (command,
> +newcommand = create_subcommand (command,
>last_offset, i - last_offset,
>false);
Indentation needs updates here.
> dst->ops->asynch_write (dst, newcommand,
> (nbd_completion_callback) {
>.callback = free_command,
>.user_data = newcommand,
> });
>}
>/* Start the new zero range. */
>last_offset = i;
> @@ -431,55 +430,55 @@ finished_read (void *vp, int *error)
> }
>}
>else {
> /* It's data. If the last was data too, do nothing =>
> * coalesce. Otherwise write the last zero range and start a
> * new data.
> */
> if (last_is_zero) {
>/* Write the last zero range (if any). */
>if (i - last_offset > 0) {
> -newcommand = copy_subcommand (command,
> - last_offset, i - last_offset,
> - true);
> +newcommand = create_subcommand (command,
> +last_offset, i - last_offset,
> +true);
But you got it right elsewhere.
ACK.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints
On Mon, Feb 21, 2022 at 03:19:23PM +0100, Laszlo Ersek wrote:
> On 02/21/22 11:22, Richard W.M. Jones wrote:
> > On Mon, Feb 21, 2022 at 10:22:04AM +0100, Laszlo Ersek wrote:
> >>> +/* Block size constraints. */
> >>> +static int
> >>> +cache_block_size (nbdkit_next *next, void *handle,
> >>> + uint32_t *minimum, uint32_t *preferred, uint32_t
> >>> *maximum)
> >>> +{
> >>> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> >>> +return -1;
> >>> +
> >>> + if (*minimum == 0) { /* No constraints set by the plugin. */
> >>> +*minimum = 1;
> >>> +*preferred = blksize;
> >>> +*maximum = 0x;
> >>> + }
> >>> + else if (*maximum >= blksize) {
> >>
> >> Do we need braces here?
> >>
> >>> +*preferred = MAX (*preferred, blksize);
> >>> + }
> >
> > I don't think we need them, but it might be clearer with them.
>
> Sorry, I'm still a bit confused whether braces around single statements
> are *permitted* -- the rule even seems to vary across the various v2v
> projects.
Oh I got the wrong end of the stick. I thought you meant would the
code be clearer like this:
+ if (*minimum == 0) { /* No constraints set by the plugin. */
+*minimum = 1;
+*preferred = blksize;
+*maximum = 0x;
+ }
+ else {
+if (*maximum >= blksize)
+ *preferred = MAX (*preferred, blksize);
+ }
I think it would be clearer, since the else clause now refers to the
separate case where the plugin *did* set constraints. In fact I
changed my local copy already.
But I think you meant should I have used braces around the single line
statement (in the original code).
> My understanding has been that we forbid braces around single
> statements, at least in some projects. So what's the rule?
There's not really a rule. I tend to use whatever is clearer in the
particular case. ie. Trading off code density (being able to see more
code on the page helps me) vs clarity (braces can be used to make it
clearer what code is contained in a clause, but correct indentation
does that as well).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints
On 02/21/22 11:22, Richard W.M. Jones wrote:
> On Mon, Feb 21, 2022 at 10:22:04AM +0100, Laszlo Ersek wrote:
>>> +/* Block size constraints. */
>>> +static int
>>> +cache_block_size (nbdkit_next *next, void *handle,
>>> + uint32_t *minimum, uint32_t *preferred, uint32_t
>>> *maximum)
>>> +{
>>> + if (next->block_size (next, minimum, preferred, maximum) == -1)
>>> +return -1;
>>> +
>>> + if (*minimum == 0) { /* No constraints set by the plugin. */
>>> +*minimum = 1;
>>> +*preferred = blksize;
>>> +*maximum = 0x;
>>> + }
>>> + else if (*maximum >= blksize) {
>>
>> Do we need braces here?
>>
>>> +*preferred = MAX (*preferred, blksize);
>>> + }
>
> I don't think we need them, but it might be clearer with them.
Sorry, I'm still a bit confused whether braces around single statements
are *permitted* -- the rule even seems to vary across the various v2v
projects. My understanding has been that we forbid braces around single
statements, at least in some projects. So what's the rule?
Thanks,
Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [libnbd PATCH] RFC: api: Add set_request_block_size
On Sun, Feb 20, 2022 at 07:14:06PM +, Richard W.M. Jones wrote: > On Thu, Feb 17, 2022 at 05:04:13PM -0600, Eric Blake wrote: > > WIP: I need to finish writing the actual interop-qemu-block-size.sh to > > demonstrate scenarios where qemu-nbd advertises a block size > 1 to > > clients that request it, but sticks to 1 otherwise (see > > https://gitlab.com/qemu-project/qemu/-/blob/master/nbd/server.c#L660). > > > > Our testsuite coverage of nbd_get_block_size() is pretty sparse (a > > single line in tests/errors.c, which was skipping until patches to > > nbdkit finally made it possible to utilize). But in the process of > > adding an interop test with qemu-nbd, I also noticed that qemu-nbd (at > > least version 6.2) exposes different block sizes to older clients that > > don't request block size than it does to newer clients which promise > > to obey block sizes. We still want to request by default, but now we > > need a knob, similar to the existing set_full_info(), for overriding > > that default for testing purposes. > > The idea seems sensible. Okay, I'll polish it up with a complete test with another post later today. > > > @@ -20,6 +20,7 @@ include $(top_srcdir)/subdir-rules.mk > > EXTRA_DIST = \ > > dirty-bitmap.sh \ > > interop-qemu-storage-daemon.sh \ > > + interop-qemu-block-size.sh \ > > list-exports-nbd-config \ > > list-exports-test-dir/disk1 \ > > list-exports-test-dir/disk2 \ > > @@ -141,6 +142,7 @@ TESTS += \ > > socket-activation-qemu-nbd \ > > dirty-bitmap.sh \ > > structured-read.sh \ > > + interop-qemu-block-size.sh \ > > $(NULL) > > Is there an issue with the whitespace on these lines? The recent addition of .editorconfig is causing this, and this is the second time I've been hit by it. For some reason, emacs is now preferring the addition of 2 spaces rather than a TAB character, because the config is telling it to prefer showing TABs with a width of 2 spaces when editing .mk files. I will fix this instance as part of finalizing the test and posting v2, but I have no idea how to further tweak .editorconfig to not have emacs make the mistake in the first place. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] how can I run a subset of the tests?
Hi, "make check" in libguestfs takes very long (especially when it's run after every patch in a series). How can I run only those tests that are, for example, in "tests/luks/"? Thanks, Laszlo ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 7/8] copy: Track worker queue size
On Mon, Feb 21, 2022 at 12:17 PM Richard W.M. Jones wrote:
>
> On Mon, Feb 21, 2022 at 08:28:54AM +0200, Nir Soffer wrote:
> > On Sun, Feb 20, 2022 at 8:53 PM Richard W.M. Jones
> > wrote:
> > >
> > > On Sun, Feb 20, 2022 at 02:14:02PM +0200, Nir Soffer wrote:
> > > > +static inline void
> > > > +increase_queue_size(struct worker *worker, size_t len)
> > >
> > > ^ space
> > >
> > > and the same in the next function:
> >
> > Sure will fix before pushing.
> >
> > Do we have a way to format the source automatically with spaces
> > before ()?
>
> I don't think anyone was written GNU indent rules yet ..
Seems that it is supported:
-pcs, --space-after-procedure-calls
Insert a space between the name of the procedure being
called and the ‘(’.
See STATEMENTS.
>
> > > > +{
> > > > + worker->queue_size += len;
> > > > +}
> > > > +
> > > > +static inline void
> > > > +decrease_queue_size(struct worker *worker, size_t len)
> > > > +{
> > > > + assert (worker->queue_size >= len);
> > > > + worker->queue_size -= len;
> > > > +}
> > >
> > > Do we not need any locking here?
> >
> > Since every worker thread accesses only its data, no locking is needed.
>
> OK
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-df lists disk usage of guests without needing to install any
> software inside the virtual machine. Supports Linux and Windows.
> http://people.redhat.com/~rjones/virt-df/
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd 7/8] copy: Track worker queue size
On Mon, Feb 21, 2022 at 08:28:54AM +0200, Nir Soffer wrote:
> On Sun, Feb 20, 2022 at 8:53 PM Richard W.M. Jones wrote:
> >
> > On Sun, Feb 20, 2022 at 02:14:02PM +0200, Nir Soffer wrote:
> > > +static inline void
> > > +increase_queue_size(struct worker *worker, size_t len)
> >
> > ^ space
> >
> > and the same in the next function:
>
> Sure will fix before pushing.
>
> Do we have a way to format the source automatically with spaces
> before ()?
I don't think anyone was written GNU indent rules yet ..
> > > +{
> > > + worker->queue_size += len;
> > > +}
> > > +
> > > +static inline void
> > > +decrease_queue_size(struct worker *worker, size_t len)
> > > +{
> > > + assert (worker->queue_size >= len);
> > > + worker->queue_size -= len;
> > > +}
> >
> > Do we not need any locking here?
>
> Since every worker thread accesses only its data, no locking is needed.
OK
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints
On Mon, Feb 21, 2022 at 10:22:04AM +0100, Laszlo Ersek wrote:
> > +/* Block size constraints. */
> > +static int
> > +cache_block_size (nbdkit_next *next, void *handle,
> > + uint32_t *minimum, uint32_t *preferred, uint32_t
> > *maximum)
> > +{
> > + if (next->block_size (next, minimum, preferred, maximum) == -1)
> > +return -1;
> > +
> > + if (*minimum == 0) { /* No constraints set by the plugin. */
> > +*minimum = 1;
> > +*preferred = blksize;
> > +*maximum = 0x;
> > + }
> > + else if (*maximum >= blksize) {
>
> Do we need braces here?
>
> > +*preferred = MAX (*preferred, blksize);
> > + }
I don't think we need them, but it might be clearer with them.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] blocksize: Export block size constraints
On 02/20/22 21:56, Richard W.M. Jones wrote:
> This filter is a little unusual because it allows clients to send a
> wider range of request sizes than the underlying plugin allows.
> Therefore we advertise the widest possible minimum and maximum block
> size to clients.
>
> We still need to pick a suitable preferred block size assuming the
> plugin itself doesn't advertise one.
> ---
> filters/blocksize/blocksize.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
> index a6fa00cb..46e759b0 100644
> --- a/filters/blocksize/blocksize.c
> +++ b/filters/blocksize/blocksize.c
> @@ -156,6 +156,29 @@ blocksize_get_size (nbdkit_next *next,
>return ROUND_DOWN (size, minblock);
> }
>
> +/* Block size constraints.
> + *
> + * Note that the purpose of this filter is to allow clients to send a
> + * wider range of request sizes than the underlying plugin permits,
> + * and therefore this callback advertises the full availability of the
> + * filter, likely widening the constraints from the plugin.
> + */
> +static int
> +blocksize_block_size (nbdkit_next *next, void *handle,
> + uint32_t *minimum, uint32_t *preferred, uint32_t
> *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*preferred == 0)
> +*preferred = MAX (4096, minblock);
> +
> + *minimum = 1;
> + *maximum = 0x;
> +
> + return 0;
> +}
> +
> static int
> blocksize_pread (nbdkit_next *next,
> void *handle, void *b, uint32_t count, uint64_t offs,
> @@ -432,6 +455,7 @@ static struct nbdkit_filter filter = {
>.config_complete = blocksize_config_complete,
>.config_help = blocksize_config_help,
>.get_size = blocksize_get_size,
> + .block_size= blocksize_block_size,
>.pread = blocksize_pread,
>.pwrite= blocksize_pwrite,
>.trim = blocksize_trim,
>
After reading the manual...
Acked-by: Laszlo Ersek
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints
On 02/20/22 21:49, Richard W.M. Jones wrote:
> Because these filters perform a read-modify-write cycle for requests
> which are smaller than the block size of the filter, we can adjust or
> set the preferred block size to the block size of the filter or the
> preferred block size of the underlying plugin, whichever is larger.
>
> We're careful not to set a preferred block size which is larger than
> the maximum block size.
>
> After this change:
>
> $ ./nbdkit --filter=cow floppy /usr/share/doc/nbdkit-devel \
>cow-block-size=128k \
>--run 'nbdinfo $uri'
> protocol: newstyle-fixed without TLS
> export="":
> export-size: 1458176 (1424K)
> content: DOS/MBR boot sector; partition 1 : ID=0xc, start-CHS
> (0x3ff,254,63), end-CHS (0x3ff,254,63), startsector 2048, 800 sectors
> uri: nbd://localhost:10809/
> contexts:
> base:allocation
> is_rotational: false
> is_read_only: false
> can_cache: true
> can_df: true
> can_fast_zero: false
> can_flush: true
> can_fua: true
> can_multi_conn: true
> can_trim: true
> can_zero: false
> block_size_minimum: 1
> block_size_preferred: 131072 <--- note
> block_size_maximum: 4294967295
> ---
> filters/cache/cache.c | 21 +
> filters/cow/cow.c | 21 +
> 2 files changed, 42 insertions(+)
>
> diff --git a/filters/cache/cache.c b/filters/cache/cache.c
> index c912c5fb..f0ee42f1 100644
> --- a/filters/cache/cache.c
> +++ b/filters/cache/cache.c
> @@ -260,6 +260,26 @@ cache_get_size (nbdkit_next *next,
>return size;
> }
>
> +/* Block size constraints. */
> +static int
> +cache_block_size (nbdkit_next *next, void *handle,
> + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*minimum == 0) { /* No constraints set by the plugin. */
> +*minimum = 1;
> +*preferred = blksize;
> +*maximum = 0x;
> + }
> + else if (*maximum >= blksize) {
Do we need braces here?
> +*preferred = MAX (*preferred, blksize);
> + }
> +
> + return 0;
> +}
> +
> /* Force an early call to cache_get_size because we have to set the
> * backing file size and bitmap size before any other read or write
> * calls.
> @@ -716,6 +736,7 @@ static struct nbdkit_filter filter = {
>.get_ready = cache_get_ready,
>.prepare = cache_prepare,
>.get_size = cache_get_size,
> + .block_size= cache_block_size,
>.can_cache = cache_can_cache,
>.can_fast_zero = cache_can_fast_zero,
>.can_flush = cache_can_flush,
> diff --git a/filters/cow/cow.c b/filters/cow/cow.c
> index e111c60f..cdc5b44f 100644
> --- a/filters/cow/cow.c
> +++ b/filters/cow/cow.c
> @@ -182,6 +182,26 @@ cow_get_size (nbdkit_next *next,
>return size;
> }
>
> +/* Block size constraints. */
> +static int
> +cow_block_size (nbdkit_next *next, void *handle,
> +uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
> +{
> + if (next->block_size (next, minimum, preferred, maximum) == -1)
> +return -1;
> +
> + if (*minimum == 0) { /* No constraints set by the plugin. */
> +*minimum = 1;
> +*preferred = blksize;
> +*maximum = 0x;
> + }
> + else if (*maximum >= blksize) {
Same here.
Makes sense to me otherwise.
Acked-by: Laszlo Ersek
Thanks
Laszlo
> +*preferred = MAX (*preferred, blksize);
> + }
> +
> + return 0;
> +}
> +
> /* Force an early call to cow_get_size because we have to set the
> * backing file size and bitmap size before any other read or write
> * calls.
> @@ -762,6 +782,7 @@ static struct nbdkit_filter filter = {
>.get_ready = cow_get_ready,
>.prepare = cow_prepare,
>.get_size = cow_get_size,
> + .block_size= cow_block_size,
>.can_write = cow_can_write,
>.can_flush = cow_can_flush,
>.can_trim = cow_can_trim,
>
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
