Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 10 Dec 2022 10:50:59 +0700
From:   Bagas Sanjaya <bagasdotme@gmail.com>
To:     Sergei Shtepa <sergei.shtepa@veeam.com>
Cc:     axboe@kernel.dk, corbet@lwn.net, linux-block@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 04/21] documentation, blksnap:  Block Devices
 Snapshots Module
Message-ID: <Y5QCI9rXE+HW03ci@debian.me>
References: <20221209142331.26395-1-sergei.shtepa@veeam.com>
 <20221209142331.26395-5-sergei.shtepa@veeam.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="740pxZ5UMCIx1ZPc"
Content-Disposition: inline
In-Reply-To: <20221209142331.26395-5-sergei.shtepa@veeam.com>
Precedence: bulk


--740pxZ5UMCIx1ZPc
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Dec 09, 2022 at 03:23:14PM +0100, Sergei Shtepa wrote:
> The document contains:
> * Describes the purpose of the mechanism
> * Description of features
> * Description of algorithms
> * Recommendations about using the module from the user-space side
> * Reference to module interface description

The patch subject should be "Documentation: document Block Devices
Snapshots Module".

Don't forget to write the description in imperative mood instead.

> +Introduction
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> +
> +At first glance, there is no novelty in the idea of creating snapshots f=
or
> +block devices. The Linux kernel already has mechanisms for creating snap=
shots.
> +Device Mapper includes dm-snap, which allows to create snapshots of block
> +devices. BTRFS supports snapshots at the file system level. However, both
> +of these options have flaws that do not allow to use them as a universal
> +tool for creating backups.
> +
> +Device Mapper flaws:
> +
> +- Block devices must have LVM markup.
> +  If no logical volumes were created during system installation, then dm=
-snap
> +  cannot be applied.
> +- To store snapshot differences of one logical volume, it is necessary to
> +  reserve a fixed range of sectors on a reserved empty logical volume.
> +  Firstly, it is required that the system has enough space unoccupied by=
 the
> +  file system, which rarely occurs on real servers. Secondly, as a rule,
> +  it is necessary to create snapshots for all logical volumes at once, w=
hich
> +  requires dividing this reserved space between several logical volumes.
> +  This space can be divided equally or proportionally to the size. But
> +  the load on different disks is usually uneven. As a result, a snapshot
> +  overflow may occur for one of the block devices, while for others all
> +  the reserved space may remain free. This complicates management of the
> +  difference storage and makes it almost impossible to create a coherent
> +  snapshot of multiple logical volumes.
> +
> +BTRFS flaws:
> +
> +- Snapshots create a persistent image of the file system, not a block de=
vice.
> +  Such a snapshot is only applicable for a file backup.
> +- When synchronizing the snapshot subvolume with the backup subvolume, r=
eading
> +  the differences leads to random access to the block device, which leads
> +  to decrease in efficiency compared to direct copying of the block devi=
ce.
> +- BTRFS allows to get an incremental backup [#btrfs_increment]_, but it =
is
> +  necessary to keep a snapshot of the previous backup cycle on the syste=
m,
> +  which leads to excessive consumption of disk space.
> +- If there is not enough free space on the file system while holding the
> +  snapshot, new data cannot be saved, which leads to a server malfunctio=
n.
> +
> +Features of the blksnap module:
> +
> +- Change tracker
> +- Snapshots at the block device level
> +- Dynamic allocation of space for storing differences
> +- Snapshot overflow resistance
> +- Coherent snapshot of multiple block devices

There is a jump from drawbacks of existing method to blksnap features.
Before introducing the latter, there should have been blksnap
description and how it solves the drawbacks. Also, the features list
above is redundant due to features section which describes each of them.

> +Static C++ library
> +------------------
> +
> +The [#userspace_libs]_ library was created primarily to simplify creatio=
n of

"The libblksnap [#userspace_libs]_ library was ..."


> +Module interface description
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> +
> +.. kernel-doc:: include/uapi/linux/blksnap.h

IMO this section should be placed after "How to use" section, because
it's quite odd to add another info after references section.

Thanks.=20

--=20
An old man doll... just what I always wanted! - Clara

--740pxZ5UMCIx1ZPc
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQSSYQ6Cy7oyFNCHrUH2uYlJVVFOowUCY5QCHwAKCRD2uYlJVVFO
o28kAP9cdVAiAeRGozPZ57xf/LM6z/ASMVfrquTf9kSnycL3NAEAoW6WPu+U1KsR
azbIOnLYwKahdIXUoQ0AkDbSqBO5Vgw=
=L60b
-----END PGP SIGNATURE-----

--740pxZ5UMCIx1ZPc--