Eric Blake
2020-Feb-10 22:29 UTC
Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
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 least one hole) and NBD_INIT_ZERO (the image reads >> completely as zero); the two bits are orthogonal and can be set >> independently, although it is easy enough to see completely sparse >> files with both bits set. > > I think I'm confused about the exact meaning of NBD_INIT_SPARSE. Do > you really mean the whole image is sparse; or (as you seem to have > said above) that there exists a hole somewhere in the image but we're > not saying where it is and there can be non-sparse parts of the image?As implemented: NBD_INIT_SPARSE - there is at least one hole somewhere (allocation would be required to write to that part of the file), but there may b allocated data elsewhere in the image. Most disk images will fit this definition (for example, it is very common to have a hole between the MBR or GPT and the first partition containing a file system, or for file systems themselves to be sparse within the larger block device). NBD_INIT_ZERO - all bytes read as zero. The combination NBD_INIT_SPARSE|NBD_INIT_ZERO is common (generally, if you use lseek(SEEK_DATA) to prove the entire image reads as zeroes, you also know the entire image is sparse), but NBD_INIT_ZERO in isolation is also possible (especially with the qcow2 proposal of a persistent autoclear bit, where even with a fully preallocated qcow2 image you still know it reads as zeroes but there are no holes). But you are also right that for servers that can advertise both bits efficiently, NBD_INIT_SPARSE in isolation may be more common than NBD_INIT_SPARSE|NBD_INIT_ZERO (the former for most disk images, the latter only for a freshly-created image that happens to create with zero initialization). What's more, in my patches, I did NOT patch qemu to set or consume INIT_SPARSE; so far, it only sets/consumes INIT_ZERO. Of course, if we can find a reason WHY qemu should track whether a qcow2 image is fully-allocated, by demonstrating a qemu-img algorithm that becomes easier for knowing if an image is sparse (even if our justification is: "when copying an image, I want to know if the _source_ is sparse, to know whether I have to bend over backwards to preallocate the destination"), then using that in qemu makes sense for my v2 patches. But for v1, my only justification was "when copying an image, I can skip holes in the source if I know the _destination_ already reads as zeroes", which only needed INIT_ZERO. Some of the nbdkit patches demonstrate the some-vs.-all nature of the two bits; for example, in the split plugin, I initialize h->init_sparse = false; h->init_zero = true; then in a loop over each file change h->init_sparse to true if at least one file was sparse, and change h->init_zero to false if at least one file had non-zero contents. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2020-Feb-10 22:52 UTC
Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
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 least one hole) and NBD_INIT_ZERO (the image reads > >>completely as zero); the two bits are orthogonal and can be set > >>independently, although it is easy enough to see completely sparse > >>files with both bits set. > > > >I think I'm confused about the exact meaning of NBD_INIT_SPARSE. Do > >you really mean the whole image is sparse; or (as you seem to have > >said above) that there exists a hole somewhere in the image but we're > >not saying where it is and there can be non-sparse parts of the image? > > As implemented: > > NBD_INIT_SPARSE - there is at least one hole somewhere (allocation > would be required to write to that part of the file), but there may > b allocated data elsewhere in the image. Most disk images will fit > this definition (for example, it is very common to have a hole > between the MBR or GPT and the first partition containing a file > system, or for file systems themselves to be sparse within the > larger block device).I think I'm still confused about why this particular flag would be useful for clients (I can completely understand why clients need NBD_INIT_ZERO). 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: https://github.com/tytso/e2fsprogs/blob/0670fc20df4a4bbbeb0edb30d82628ea30a80598/misc/mke2fs.c#L2768 and which is painfully slow over NBD for very large devices because of the 32 bit limit on request sizes - try doing mke2fs on a 1E nbdkit memory disk some time.> NBD_INIT_ZERO - all bytes read as zero. > > The combination NBD_INIT_SPARSE|NBD_INIT_ZERO is common (generally, > if you use lseek(SEEK_DATA) to prove the entire image reads as > zeroes, you also know the entire image is sparse), but NBD_INIT_ZERO > in isolation is also possible (especially with the qcow2 proposal of > a persistent autoclear bit, where even with a fully preallocated > qcow2 image you still know it reads as zeroes but there are no > holes). But you are also right that for servers that can advertise > both bits efficiently, NBD_INIT_SPARSE in isolation may be more > common than NBD_INIT_SPARSE|NBD_INIT_ZERO (the former for most disk > images, the latter only for a freshly-created image that happens to > create with zero initialization). > > What's more, in my patches, I did NOT patch qemu to set or consume > INIT_SPARSE; so far, it only sets/consumes INIT_ZERO. Of course, if > we can find a reason WHY qemu should track whether a qcow2 image is > fully-allocated, by demonstrating a qemu-img algorithm that becomes > easier for knowing if an image is sparse (even if our justification > is: "when copying an image, I want to know if the _source_ is > sparse, to know whether I have to bend over backwards to preallocate > the destination"), then using that in qemu makes sense for my v2 > patches. But for v1, my only justification was "when copying an > image, I can skip holes in the source if I know the _destination_ > already reads as zeroes", which only needed INIT_ZERO. > > Some of the nbdkit patches demonstrate the some-vs.-all nature of > the two bits; for example, in the split plugin, I initialize > h->init_sparse = false; h->init_zero = true; then in a loop over > each file change h->init_sparse to true if at least one file was > sparse, and change h->init_zero to false if at least one file had > non-zero contents.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
Eric Blake
2020-Feb-11 14:33 UTC
Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
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 least one hole) and NBD_INIT_ZERO (the image reads >>>> completely as zero); the two bits are orthogonal and can be set >>>> independently, although it is easy enough to see completely sparse >>>> files with both bits set. >>> >>> I think I'm confused about the exact meaning of NBD_INIT_SPARSE. Do >>> you really mean the whole image is sparse; or (as you seem to have >>> said above) that there exists a hole somewhere in the image but we're >>> not saying where it is and there can be non-sparse parts of the image? >> >> As implemented: >> >> NBD_INIT_SPARSE - there is at least one hole somewhere (allocation >> would be required to write to that part of the file), but there may >> b allocated data elsewhere in the image. Most disk images will fit >> this definition (for example, it is very common to have a hole >> between the MBR or GPT and the first partition containing a file >> system, or for file systems themselves to be sparse within the >> larger block device). > > I think I'm still confused about why this particular flag would be > useful for clients (I can completely understand why clients need > NBD_INIT_ZERO). > > 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: > > https://github.com/tytso/e2fsprogs/blob/0670fc20df4a4bbbeb0edb30d82628ea30a80598/misc/mke2fs.c#L2768I'm open to suggestions on how many initial bits should be provided. In fact, if we wanted, we could have a pair mutually-exclusive bits, advertising: 00 - no information known 01 - image is completely sparse 10 - image is completely allocated 11 - error The goal of providing a 16-bit answer (or we could mandate 32 or 64 bits, if we think we will ever want to extend that far) was to make it easier to add in whatever other initial-state extensions that someone could find useful. Until we're happy with the design, the size or any given bit assignment is not locked down; once we do start committing any of this series, we've locked in what interoperability will demand but still have spare bits as future extensions. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Wouter Verhelst
2020-Feb-12 07:27 UTC
Re: [Libguestfs] Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
Hi, On Mon, Feb 10, 2020 at 10:52:55PM +0000, 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:Yeah, I think that could definitely be useful. I honestly can't see a use for NBD_INIT_SPARSE as defined in this proposal; and I don't think it's generally useful to have a feature if we can't think of a use case for it (that creates added complexity for no benefit). If we can find a reasonable use case for NBD_INIT_SPARSE as defined in this proposal, then just add a third bit (NBD_INIT_ALL_SPARSE or something) that says "the whole image is sparse". Otherwise, I think we should redefine NBD_INIT_SPARSE to say that. -- To the thief who stole my anti-depressants: I hope you're happy -- seen somewhere on the Internet on a photo of a billboard
Maybe Matching Threads
- Re: Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
- Re: Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
- Re: Cross-project NBD extension proposal: NBD_INFO_INIT_STATE
- [nbdkit PATCH 04/10] plugins: Wire up in-memory plugin support for NBD_INFO_INIT_STATE
- [nbdkit PATCH 05/10] plugins: Wire up file-based plugin support for NBD_INFO_INIT_STATE