Re: [PATCH 00/26] Btrfs: Add device replace code
On Tue, 13 Nov 2012 17:25:46 +0100, Bart Noordervliet wrote: Hi Stefan, I gave your patchset a whirl and it worked like a charm. Thanks a lot for your work. I was confused for a moment by the fact that the operation doesn't immediately resize btrfs to use all of the new device's available space. But after a bit of thought I suppose that is intentional to make it easy to return to a device of similar size as the original. Yes, replace and resize is an operation that intentionally requires two manual steps, since the step backwards, to shrink a device, can take a long time. One suggestion remains: I would prefer the operation to leave some record of what has happened in the system log. Other operations like device add and device delete do and I think it makes sense to have a log trail of such invasive operations on the filesystem. That's a good idea. I have now added such KERN_INFO log messages. Tested-by: Bart Noordervliet b...@noordervliet.net Thanks for testing and for your feedback :) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
Hi Stefan, I gave your patchset a whirl and it worked like a charm. Thanks a lot for your work. I was confused for a moment by the fact that the operation doesn't immediately resize btrfs to use all of the new device's available space. But after a bit of thought I suppose that is intentional to make it easy to return to a device of similar size as the original. One suggestion remains: I would prefer the operation to leave some record of what has happened in the system log. Other operations like device add and device delete do and I think it makes sense to have a log trail of such invasive operations on the filesystem. Tested-by: Bart Noordervliet b...@noordervliet.net Best regards, Bart -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On 8 Nov 2012 18:31 +0100, from sbehr...@giantdisaster.de (Stefan Behrens): btrfs device replace cancel path was the point when I gave up putting it below the device commands. IMO that's just too long, too much to type. How often is one going to type that? I like the idea of consistency with add/delete, here, since it really amounts to largely the same thing. -- Michael Kjörling • http://michael.kjorling.se • mich...@kjorling.se “People who think they know everything really annoy those of us who know we don’t.” (Bjarne Stroustrup) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
Hi Stefan, great work. However I have a suggestion: what about putting all the command under 'device' sub commands: something like: - btrfs device replace old new /path - btrfs device status /path Where btrfs device status would show only the status of the replacing operation; but in the future it could show also the status of the delete command (which it is the only other device sub-command which needs time to complete). Of course I am not asking to complete the btrfs device status part for the btrfs device delete command. This could be implemented in a second time. I think that so replace would be the natral extension to the add and delete subcommands. BR G.Baroncelli On Wed, Nov 7, 2012 at 2:12 PM, Stefan Behrens sbehr...@giantdisaster.de wrote: On Wed, 07 Nov 2012 11:14:36 +0900, Tsutomu Itoh wrote: (2012/11/07 1:38), Stefan Behrens wrote: replace start [-Bfr] path srcdev|devid targetdev I think that btrfs replace start [-Bfr] srcdev|devid targetdev path of the same synopsis as other subcommands is better. You are right. 'btrfs device add' and 'btrfs device delete' have the path at the end and the device in front, and there is no good reason to not make it look equally. I will change it accordingly and post a V2 of the Btrfs-progs patch within the next few days. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On 11/08/2012 06:31 PM, Stefan Behrens wrote: On Thu, 8 Nov 2012 13:50:19 +0100, Goffredo Baroncelli wrote: [...] I think that so replace would be the natural extension to the add and delete subcommands. btrfs device replace old new path was also my first idea. It used to be like this initially. btrfs device replace cancel path was the point when I gave up putting it below the device commands. IMO that's just too long, too much to type. Now it has the same look and feel as the scrub commands (scrub start, scrub status and scrub cancel). Yes, but scrub was a a new command. Instead I see replace as an extension of btrfs device add/del (from an user interface POV). If someone would extend the btrfs device delete command to support status/pause/resume, how could do it ? May be we need a new series of command which handle the background process (like btrfs replace, btrfs device delete, btrfs subvolume delete) to status/stop/suspend/resume these processes ? I am doing a bit of brain-storming... -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On Tue, 6 Nov 2012 14:48:36 -0800, Zach Brown wrote:
Yes, this happens on 32 bit builds, not on 64 bit builds. If you
look at the source code, the compiler is obviously wrong (or I am
blind).
ret = btrfs_dev_replace_find_srcdev(root, args-start.srcdevid,
args-start.srcdev_name,
src_device);
mutex_unlock(fs_info-volume_mutex);
if (ret) { -- this is line 344
But thanks for the feedback anyway!
This usually means that somewhere in the call tree under
btrfs_dev_replace_find_srcdev(), there's a way that the function can
return without returning a value
Indeed, and that's obviously the case in 15/26 with
btrfs_dev_replace_find_srcdev() when btrfs_find_device() returns a
device.
*mumbles something about the reason for ERR_PTR semantics*
Thanks Bart, Hugo and Zach!
I'll fix it in
git://btrfs.giantdisaster.de/git/btrfs device-replace
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On Wed, 07 Nov 2012 11:14:36 +0900, Tsutomu Itoh wrote: (2012/11/07 1:38), Stefan Behrens wrote: replace start [-Bfr] path srcdev|devid targetdev I think that btrfs replace start [-Bfr] srcdev|devid targetdev path of the same synopsis as other subcommands is better. You are right. 'btrfs device add' and 'btrfs device delete' have the path at the end and the device in front, and there is no good reason to not make it look equally. I will change it accordingly and post a V2 of the Btrfs-progs patch within the next few days. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On Tue, Nov 06, 2012 at 09:38:18AM -0700, Stefan Behrens wrote: This patch series adds support for replacing disks at runtime. It replaces the following steps in case a disk was lost: mount ... -o degraded btrfs device add new_disk btrfs device delete missing Or in case a disk just needs to be replaced because the error rate is increasing: btrfs device add new_disk btrfs device delete old_disk Instead just run: btrfs replace mountpoint old_disk new_disk This is just fantastic. I'm pulling it down and doing a test integration with RAID56. More comments as I hash through things. -chris -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On 11/06/2012 19:17, Bart Noordervliet wrote:
Great new feature Stefan, thanks a lot! Going to give it a spin right
away. While compiling I got this (probably superfluous) error that you
might want to prevent:
fs/btrfs/dev-replace.c: In function ‘btrfs_dev_replace_start’:
fs/btrfs/dev-replace.c:344:5: warning: ‘ret’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
Regards,
Bart
Yes, this happens on 32 bit builds, not on 64 bit builds. If you look at
the source code, the compiler is obviously wrong (or I am blind).
ret = btrfs_dev_replace_find_srcdev(root, args-start.srcdevid,
args-start.srcdev_name,
src_device);
mutex_unlock(fs_info-volume_mutex);
if (ret) { -- this is line 344
But thanks for the feedback anyway!
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
On Tue, Nov 06, 2012 at 07:57:47PM +0100, Stefan Behrens wrote:
On 11/06/2012 19:17, Bart Noordervliet wrote:
Great new feature Stefan, thanks a lot! Going to give it a spin right
away. While compiling I got this (probably superfluous) error that you
might want to prevent:
fs/btrfs/dev-replace.c: In function ‘btrfs_dev_replace_start’:
fs/btrfs/dev-replace.c:344:5: warning: ‘ret’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
Regards,
Bart
Yes, this happens on 32 bit builds, not on 64 bit builds. If you
look at the source code, the compiler is obviously wrong (or I am
blind).
ret = btrfs_dev_replace_find_srcdev(root, args-start.srcdevid,
args-start.srcdev_name,
src_device);
mutex_unlock(fs_info-volume_mutex);
if (ret) { -- this is line 344
But thanks for the feedback anyway!
This usually means that somewhere in the call tree under
btrfs_dev_replace_find_srcdev(), there's a way that the function can
return without returning a value, or returning an unintialised
variable. Not quite sure why it's only on 32 bits, though.
Hugo.
--
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- Computer Science is not about computers, any more than ---
astronomy is about telescopes.
signature.asc
Description: Digital signature
Re: [PATCH 00/26] Btrfs: Add device replace code
Yes, this happens on 32 bit builds, not on 64 bit builds. If you
look at the source code, the compiler is obviously wrong (or I am
blind).
ret = btrfs_dev_replace_find_srcdev(root, args-start.srcdevid,
args-start.srcdev_name,
src_device);
mutex_unlock(fs_info-volume_mutex);
if (ret) { -- this is line 344
But thanks for the feedback anyway!
This usually means that somewhere in the call tree under
btrfs_dev_replace_find_srcdev(), there's a way that the function can
return without returning a value
Indeed, and that's obviously the case in 15/26 with
btrfs_dev_replace_find_srcdev() when btrfs_find_device() returns a
device.
*mumbles something about the reason for ERR_PTR semantics*
- z
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Btrfs: Add device replace code
(2012/11/07 1:38), Stefan Behrens wrote: This patch series adds support for replacing disks at runtime. It replaces the following steps in case a disk was lost: mount ... -o degraded btrfs device add new_disk btrfs device delete missing Or in case a disk just needs to be replaced because the error rate is increasing: btrfs device add new_disk btrfs device delete old_disk Instead just run: btrfs replace mountpoint old_disk new_disk The device replace operation takes place at runtime on a live filesystem, you don't need to unmount it or stop active tasks. It is safe to crash or lose power during the operation, the process resumes with the next mount. The copy usually takes place at 90% of the available platter speed if no additional disk I/O is ongoing during the copy operation, thus the degraded state without redundancy can be left quickly. The copy process is started manually. It is a different project to react on an increased device I/O error rate with an automatic start of this procedure. The patch series is based on btrfs-next and also available here: git://btrfs.giantdisaster.de/git/btrfs device-replace The user mode part is the top commit of git://btrfs.giantdisaster.de/git/btrfs-progs master replace start [-Bfr] path srcdev|devid targetdev I think that btrfs replace start [-Bfr] srcdev|devid targetdev path of the same synopsis as other subcommands is better. - Tsutomu Replace device of a btrfs filesystem. On a live filesystem, duplicate the data to the target device which is currently stored on the source device. If the source device is not avail- able anymore, or if the -r option is set, the data is built only using the RAID redundancy mechanisms. After completion of the operation, the source device is removed from the filesystem. If the srcdev is a numerical value, it is assumed to be the device id of the filesystem which is mounted at mount_point, otherwise is is the path to the source device. If the source device is disconnected, from the system, you have to use the devid parame- ter format. The targetdev needs to be same size or larger than the srcdev. Options -r only read from srcdev if no other zero-defect mirror exists (enable this if your drive has lots of read errors, the access would be very slow) -f force using and overwriting targetdev even if it looks like containing a valid btrfs filesystem. A valid filesystem is assumed if a btrfs superblock is found which contains a correct checksum. Devices which are cur- rently mounted are never allowed to be used as the tar- getdev -B do not background replace status [-1] path Print status and progress information of a running device replace operation. Options -1 print once instead of print continously until the replace operation finishes (or is canceled) replace cancel path Cancel a running device replace operation. Stefan Behrens (26): Btrfs: rename the scrub context structure Btrfs: remove the block device pointer from the scrub context struct Btrfs: make the scrub page array dynamically allocated Btrfs: in scrub repair code, optimize the reading of mirrors Btrfs: in scrub repair code, simplify alloc error handling Btrfs: cleanup scrub bio and worker wait code Btrfs: add two more find_device() methods Btrfs: Pass fs_info to btrfs_num_copies() instead of mapping_tree Btrfs: pass fs_info to btrfs_map_block() instead of mapping_tree Btrfs: add btrfs_scratch_superblock() function Btrfs: pass fs_info instead of root Btrfs: avoid risk of a deadlock in btrfs_handle_error Btrfs: enhance btrfs structures for device replace support Btrfs: introduce a btrfs_dev_replace_item type Btrfs: add a new source file with device replace code Btrfs: disallow mutually exclusiv admin operations from user mode Btrfs: disallow some operations on the device replace target device Btrfs: handle errors from btrfs_map_bio() everywhere Btrfs: add code to scrub to copy read data to another disk Btrfs: change core code of btrfs to support the device replace operations Btrfs: introduce GET_READ_MIRRORS functionality for btrfs_map_block() Btrfs: changes to live filesystem are also written to replacement disk Btrfs: optionally avoid reads from device replace source drive Btrfs: increase BTRFS_MAX_MIRRORS by one for dev replace Btrfs: allow repair code to include target disk when searching mirrors Btrfs: add support for device replace ioctls fs/btrfs/Makefile
