mbox series

[PATCHv3,0/3] btrfs-progs: Auto resize fs after device replace

Message ID 20200416004642.9941-1-marcos@mpdesouza.com (mailing list archive)
Headers show
Series btrfs-progs: Auto resize fs after device replace | expand

Message

Marcos Paulo de Souza April 16, 2020, 12:46 a.m. UTC
From: Marcos Paulo de Souza <mpdesouza@suse.com>

Changes from v2:
* Fixed the code format after moving function resize_filesystem in patch 0001
  (suggested by David)
* Sorted the resize_filesystem function prototype in patch 0001 (suggested by
  David)
* Changed the -a into long argument --autoresize in patch 0002 (suggested by
  David)
* Translate srcdev if the argument is not a devid (suggested by David)
* This also changes the way we use the ioctl, now only passing devid to the
  kernel, instead of passing the path and letting the kernel to translate
* Add tests to check the autoresize functionality

Changes from v1:
* Reworded the help message and the docs telling the user that the fs will be
  resized to it's max size (suggested by Qu)
* Added a warning message saying that the resize failed, asking the user to
  resize manually. (suggested by Qu)

Both changes were done only in patch 0002.

Anand suggested this job to be done in kernel side, atomically, but as I
received a good review from Qu I decided to send a v3 of this patchset.

Please review, thanks!

Original cover-letter[1]:
These two patches make possible to resize the fs after a successful replace
finishes. The flag -a is responsible for doing it (-r is already use, so -a in
this context means "automatically").

The first patch just moves the resize rationale to utils.c and the second patch
adds the flag an calls resize if -a is informed replace finishes successfully.

Please review!

Marcos Paulo de Souza (3):
  btrfs-progs: Move resize into functionaly into utils.c
  btrfs-progs: replace: New argument to resize the fs after replace
  btrfs-progs: tests: misc: Add some replace tests

 Documentation/btrfs-replace.asciidoc        |   5 +-
 cmds/filesystem.c                           |  58 +----------
 cmds/replace.c                              | 105 +++++++++++++-------
 common/utils.c                              |  60 +++++++++++
 common/utils.h                              |   2 +
 tests/misc-tests/039-replace-device/test.sh |  56 +++++++++++
 6 files changed, 192 insertions(+), 94 deletions(-)
 create mode 100755 tests/misc-tests/039-replace-device/test.sh

Comments

Marcos Paulo de Souza April 17, 2020, 2:15 a.m. UTC | #1
On Thu, 2020-04-16 at 07:18 +0100, devel@roosoftl.ltd.uk wrote:
> On 16/04/2020 01:46, Marcos Paulo de Souza wrote:
> > From: Marcos Paulo de Souza <mpdesouza@suse.com>
> >
> > Changes from v2:
> > * Fixed the code format after moving function resize_filesystem in
> patch 0001
> >   (suggested by David)
> > * Sorted the resize_filesystem function prototype in patch 0001
> (suggested by
> >   David)
> > * Changed the -a into long argument --autoresize in patch 0002
> (suggested by
> >   David)
> > * Translate srcdev if the argument is not a devid (suggested by
> David)
> > * This also changes the way we use the ioctl, now only passing
> devid to the
> >   kernel, instead of passing the path and letting the kernel to
> translate
> > * Add tests to check the autoresize functionality
> >
> > Changes from v1:
> > * Reworded the help message and the docs telling the user that the
> fs will be
> >   resized to it's max size (suggested by Qu)
> > * Added a warning message saying that the resize failed, asking the
> user to
> >   resize manually. (suggested by Qu)
> >
> > Both changes were done only in patch 0002.
> >
> > Anand suggested this job to be done in kernel side, atomically, but
> as I
> > received a good review from Qu I decided to send a v3 of this
> patchset.
> >
> > Please review, thanks!
> >
> > Original cover-letter[1]:
> > These two patches make possible to resize the fs after a successful
> replace
> > finishes. The flag -a is responsible for doing it (-r is already
> use, so -a in
> > this context means "automatically").
> >
> > The first patch just moves the resize rationale to utils.c and the
> second patch
> > adds the flag an calls resize if -a is informed replace finishes
> successfully.
> >
> > Please review!
> >
> > Marcos Paulo de Souza (3):
> >   btrfs-progs: Move resize into functionaly into utils.c
> >   btrfs-progs: replace: New argument to resize the fs after replace
> >   btrfs-progs: tests: misc: Add some replace tests
> >
> >  Documentation/btrfs-replace.asciidoc        |   5 +-
> >  cmds/filesystem.c                           |  58 +----------
> >  cmds/replace.c                              | 105 +++++++++++++---
> ----
> >  common/utils.c                              |  60 +++++++++++
> >  common/utils.h                              |   2 +
> >  tests/misc-tests/039-replace-device/test.sh |  56 +++++++++++
> >  6 files changed, 192 insertions(+), 94 deletions(-)
> >  create mode 100755 tests/misc-tests/039-replace-device/test.sh
> >
> Hmm,
> 
> 
> Does this take into account if different raid levels are used for
> data
> and meta data. When I last was upgrading a fs with larger drive it
> was
> not just a case of making the over all file size different I had to
> also
> change the ratio of metadata to data. And then I had to balance a
> whole
> lot (which found a bug in memory at the time)

What do you mean about "ratio of metadata to data"? The new disk will
receive the same content of the old disk, so, it the metadata wasn't
balanced before the replace, it might be a good idea to execute balance
after adding a maybe larger disk.

Can you please describe in more details you problem, or the problem you
had at the time you did the replace last time? It seems it's not
related to the resize itself, so these patches do not change the
current behavior.

> 
> 
> How do you deal with these edge cases ? Especially if you replacing
> with
> a smaller disk and then have to shrink the metadata ratio?

Replace does not accept a smaller disk to replace a larger one.

> 
> 
> Just curious.

Me too. These patches were meant to help people to avoid executing two
different command for a pretty common action, all details behind
replace and resize are the same, it's just a "helper" to make users to
use the full capacity of a newly added disk.
Marcos Paulo de Souza May 31, 2020, 7:11 p.m. UTC | #2
Humble ping :)

On Wed, 2020-04-15 at 21:46 -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Changes from v2:
> * Fixed the code format after moving function resize_filesystem in
> patch 0001
>   (suggested by David)
> * Sorted the resize_filesystem function prototype in patch 0001
> (suggested by
>   David)
> * Changed the -a into long argument --autoresize in patch 0002
> (suggested by
>   David)
> * Translate srcdev if the argument is not a devid (suggested by
> David)
> * This also changes the way we use the ioctl, now only passing devid
> to the
>   kernel, instead of passing the path and letting the kernel to
> translate
> * Add tests to check the autoresize functionality
> 
> Changes from v1:
> * Reworded the help message and the docs telling the user that the fs
> will be
>   resized to it's max size (suggested by Qu)
> * Added a warning message saying that the resize failed, asking the
> user to
>   resize manually. (suggested by Qu)
> 
> Both changes were done only in patch 0002.
> 
> Anand suggested this job to be done in kernel side, atomically, but
> as I
> received a good review from Qu I decided to send a v3 of this
> patchset.
> 
> Please review, thanks!
> 
> Original cover-letter[1]:
> These two patches make possible to resize the fs after a successful
> replace
> finishes. The flag -a is responsible for doing it (-r is already use,
> so -a in
> this context means "automatically").
> 
> The first patch just moves the resize rationale to utils.c and the
> second patch
> adds the flag an calls resize if -a is informed replace finishes
> successfully.
> 
> Please review!
> 
> Marcos Paulo de Souza (3):
>   btrfs-progs: Move resize into functionaly into utils.c
>   btrfs-progs: replace: New argument to resize the fs after replace
>   btrfs-progs: tests: misc: Add some replace tests
> 
>  Documentation/btrfs-replace.asciidoc        |   5 +-
>  cmds/filesystem.c                           |  58 +----------
>  cmds/replace.c                              | 105 +++++++++++++-----
> --
>  common/utils.c                              |  60 +++++++++++
>  common/utils.h                              |   2 +
>  tests/misc-tests/039-replace-device/test.sh |  56 +++++++++++
>  6 files changed, 192 insertions(+), 94 deletions(-)
>  create mode 100755 tests/misc-tests/039-replace-device/test.sh
>