[PATCH] e2image: Print a warning if running over a mounted filesystem

[PATCH] e2image: Print a warning if running over a mounted filesystem

[Carlos Maiolino]

Several users use to run e2image over a mounted filesystem, providing
inconsistent, useless e2images.
This patch adds a warning in such cases, notifying the user and also adds a
force option making e2image able to run over Read-only filesystems.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 misc/Makefile.in |  2 +-
 misc/e2image.c   | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/misc/Makefile.in b/misc/Makefile.in
index 8a69855..2e20b25 100644
--- a/misc/Makefile.in
+++ b/misc/Makefile.in
@@ -49,7 +49,7 @@ UUIDGEN_OBJS=	uuidgen.o
 UUIDD_OBJS=	uuidd.o
 DUMPE2FS_OBJS=	dumpe2fs.o
 BADBLOCKS_OBJS=	badblocks.o
-E2IMAGE_OBJS=	e2image.o
+E2IMAGE_OBJS=	e2image.o ismounted.o
 FSCK_OBJS=	fsck.o base_device.o ismounted.o
 BLKID_OBJS=	blkid.o
 FILEFRAG_OBJS=	filefrag.o
diff --git a/misc/e2image.c b/misc/e2image.c
index 885a794..6f6329a 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -49,6 +49,8 @@ extern int optind;
 
 #define QCOW_OFLAG_COPIED     (1LL << 63)
 
+/* ismounted.h */
+extern int is_mounted(const char *file);
 
 const char * program_name = "e2image";
 char * device_name = NULL;
@@ -87,7 +89,7 @@ static int get_bits_from_size(size_t size)
 
 static void usage(void)
 {
-	fprintf(stderr, _("Usage: %s [-rsIQa] device image_file\n"),
+	fprintf(stderr, _("Usage: %s [-rsIQaf] device image_file\n"),
 		program_name);
 	exit (1);
 }
@@ -1255,6 +1257,7 @@ int main (int argc, char ** argv)
 	int qcow2_fd = 0;
 	int fd = 0;
 	int ret = 0;
+	int ignore_mounted = 0;
 	struct stat st;
 
 #ifdef ENABLE_NLS
@@ -1269,7 +1272,7 @@ int main (int argc, char ** argv)
 	if (argc && *argv)
 		program_name = *argv;
 	add_error_table(&et_ext2_error_table);
-	while ((c = getopt(argc, argv, "rsIQa")) != EOF)
+	while ((c = getopt(argc, argv, "rsIQaf")) != EOF)
 		switch (c) {
 		case 'I':
 			flags |= E2IMAGE_INSTALL_FLAG;
@@ -1290,6 +1293,9 @@ int main (int argc, char ** argv)
 		case 'a':
 			all_data = 1;
 			break;
+		case 'f':
+			ignore_mounted = 1;
+			break;
 		default:
 			usage();
 		}
@@ -1305,6 +1311,14 @@ int main (int argc, char ** argv)
 	device_name = argv[optind];
 	image_fn = argv[optind+1];
 
+	if (is_mounted(device_name) && !ignore_mounted) {
+		fprintf(stderr, "\nWarning: Running e2image on a mounted "
+				"filesystem can result in an inconsistent "
+				"image which will not be useful. Use -f "
+				"option if you really want to do that.\n");
+		exit(1);
+	}
+
 	if (flags & E2IMAGE_INSTALL_FLAG) {
 		install_image(device_name, image_fn, img_type);
 		exit (0);
-- 
1.8.1.4

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Eric Sandeen]

On 9/26/13 4:00 PM, Carlos Maiolino wrote:
> Several users use to run e2image over a mounted filesystem, providing
> inconsistent, useless e2images.
> This patch adds a warning in such cases, notifying the user and also adds a
> force option making e2image able to run over Read-only filesystems.

Good idea.  ;)  But I think it needs a manpage update as well.

If you use ext2fs_check_if_mounted() can you find out whether it's
mounted readonly, and allow it to proceed in that case?

(But I suppose that only checks one mount point, I'm not sure?)

-Eric

> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  misc/Makefile.in |  2 +-
>  misc/e2image.c   | 18 ++++++++++++++++--
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/misc/Makefile.in b/misc/Makefile.in
> index 8a69855..2e20b25 100644
> --- a/misc/Makefile.in
> +++ b/misc/Makefile.in
> @@ -49,7 +49,7 @@ UUIDGEN_OBJS=	uuidgen.o
>  UUIDD_OBJS=	uuidd.o
>  DUMPE2FS_OBJS=	dumpe2fs.o
>  BADBLOCKS_OBJS=	badblocks.o
> -E2IMAGE_OBJS=	e2image.o
> +E2IMAGE_OBJS=	e2image.o ismounted.o
>  FSCK_OBJS=	fsck.o base_device.o ismounted.o
>  BLKID_OBJS=	blkid.o
>  FILEFRAG_OBJS=	filefrag.o
> diff --git a/misc/e2image.c b/misc/e2image.c
> index 885a794..6f6329a 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -49,6 +49,8 @@ extern int optind;
>  
>  #define QCOW_OFLAG_COPIED     (1LL << 63)
>  
> +/* ismounted.h */
> +extern int is_mounted(const char *file);
>  
>  const char * program_name = "e2image";
>  char * device_name = NULL;
> @@ -87,7 +89,7 @@ static int get_bits_from_size(size_t size)
>  
>  static void usage(void)
>  {
> -	fprintf(stderr, _("Usage: %s [-rsIQa] device image_file\n"),
> +	fprintf(stderr, _("Usage: %s [-rsIQaf] device image_file\n"),
>  		program_name);
>  	exit (1);
>  }
> @@ -1255,6 +1257,7 @@ int main (int argc, char ** argv)
>  	int qcow2_fd = 0;
>  	int fd = 0;
>  	int ret = 0;
> +	int ignore_mounted = 0;
>  	struct stat st;
>  
>  #ifdef ENABLE_NLS
> @@ -1269,7 +1272,7 @@ int main (int argc, char ** argv)
>  	if (argc && *argv)
>  		program_name = *argv;
>  	add_error_table(&et_ext2_error_table);
> -	while ((c = getopt(argc, argv, "rsIQa")) != EOF)
> +	while ((c = getopt(argc, argv, "rsIQaf")) != EOF)
>  		switch (c) {
>  		case 'I':
>  			flags |= E2IMAGE_INSTALL_FLAG;
> @@ -1290,6 +1293,9 @@ int main (int argc, char ** argv)
>  		case 'a':
>  			all_data = 1;
>  			break;
> +		case 'f':
> +			ignore_mounted = 1;
> +			break;
>  		default:
>  			usage();
>  		}
> @@ -1305,6 +1311,14 @@ int main (int argc, char ** argv)
>  	device_name = argv[optind];
>  	image_fn = argv[optind+1];
>  
> +	if (is_mounted(device_name) && !ignore_mounted) {
> +		fprintf(stderr, "\nWarning: Running e2image on a mounted "
> +				"filesystem can result in an inconsistent "
> +				"image which will not be useful. Use -f "
> +				"option if you really want to do that.\n");
> +		exit(1);
> +	}
> +
>  	if (flags & E2IMAGE_INSTALL_FLAG) {
>  		install_image(device_name, image_fn, img_type);
>  		exit (0);
> 

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Theodore Ts'o]

On Thu, Sep 26, 2013 at 06:00:04PM -0300, Carlos Maiolino wrote:
> Several users use to run e2image over a mounted filesystem, providing
> inconsistent, useless e2images.
> This patch adds a warning in such cases, notifying the user and also adds a
> force option making e2image able to run over Read-only filesystems.

It should be perfectly safe to run e2image on a read-only mounted file
system option, so it's not obvious to me why the force option would be
needed in that case.

Also, if we are saving a "normal" (not a raw or qcow) e2image file, we
are only backing up the statically located metadata blocks (i.e.,
superblock, block group descriptors, inode table, and allocation
bitmaps).  If we do this on a mounted file system, the e2image file is
less useful, but I'm not sure I'd call it completely useless.  If the
goal is to backup critical metadata, it will do that just fine.  So
maybe it's worthy of a warning, but I'm not sure it should require a
force option.

If the user is trying to capture a raw or qcow image file, I agree
that requiring that the file systme either be mounted read-only, not
mounted at all, or that a force option be specified, makes sense.

	   	   	  	       	  - Ted

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Eric Sandeen]

On 9/26/13 6:56 PM, Theodore Ts'o wrote:
> On Thu, Sep 26, 2013 at 06:00:04PM -0300, Carlos Maiolino wrote:
>> Several users use to run e2image over a mounted filesystem, providing
>> inconsistent, useless e2images.
>> This patch adds a warning in such cases, notifying the user and also adds a
>> force option making e2image able to run over Read-only filesystems.
> 
> It should be perfectly safe to run e2image on a read-only mounted file
> system option, so it's not obvious to me why the force option would be
> needed in that case.

right now Carlos' test isn't checking for readonly; just mounted, right?

But I think it should check for ro, and allow it by default in that case,
I agree.

> Also, if we are saving a "normal" (not a raw or qcow) e2image file, we
> are only backing up the statically located metadata blocks (i.e.,
> superblock, block group descriptors, inode table, and allocation
> bitmaps).  If we do this on a mounted file system, the e2image file is
> less useful, but I'm not sure I'd call it completely useless.  If the
> goal is to backup critical metadata, it will do that just fine.  So
> maybe it's worthy of a warning, but I'm not sure it should require a
> force option.

I asked Carlos to do this after getting the 2nd customer filesystem
image in a week which was useless for triage due to having been run
on a live, mounted fs...

I fear that a warning would be ignored, but *shrug* - at least something
so we have some hope of getting something useful out the other end
of e2image -r or -Q...

TBH I've never used a "normal" image; if you want to allow it to
run on an RW filesystem that won't bother me at all.  ;)

> If the user is trying to capture a raw or qcow image file, I agree
> that requiring that the file systme either be mounted read-only, not
> mounted at all, or that a force option be specified, makes sense.

cool.  :)

-Eric

> 	   	   	  	       	  - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Carlos Maiolino]

Hey, sorry my late answer, I'm experimenting some new mutt configuration.


> > If the user is trying to capture a raw or qcow image file, I agree
> > that requiring that the file systme either be mounted read-only, not
> > mounted at all, or that a force option be specified, makes sense.
> 
> cool.  :)
> 
> -Eric

I'd suggest then to need a force when using -r or -q on a RW image. In case of a
normal image, just a warning but continue.

Ted, as Eric said, I've got some useless images in the past too and just a
warning won't really avoid the problem IMHO since, most of users don't even read
what's being printed :-) so, request a force when getting at least a raw or
QCOW2 image is very useful IMHO.

any comments?
> 
> > 	   	   	  	       	  - Ted

-- 
Carlos

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Theodore Ts'o]

On Thu, Sep 26, 2013 at 09:48:33PM -0300, Carlos Maiolino wrote:
> 
> I'd suggest then to need a force when using -r or -q on a RW image. In case of a
> normal image, just a warning but continue.

(Sorry for not repsonding earlier; I was on vacation in at Yellowstone
/ Grand Teton National Park from Thursday through today.)

Yes, that's what I was suggesting.  Requiring force for the case of -r
or -q, but just a warning otherwise.

> Ted, as Eric said, I've got some useless images in the past too and just a
> warning won't really avoid the problem IMHO since, most of users don't even read
> what's being printed :-) so, request a force when getting at least a raw or
> QCOW2 image is very useful IMHO.

The useless images are primarily when we are trying to use -r or -q to
get the dynamic metadata (i.e., the directory blocks and extent tree
blocks).  That's what we generally need when are doing debugging, and
so yes, I'm fully in agreement with requiring force in the case of
e2image -r and e2image -q.

Regards,

					- Ted

[PATCH] e2image: Print a warning if running over a mounted filesystem

[Carlos Maiolino]

Several users use to run e2image over a mounted RW filesystem, providing
inconsistent, useless e2images for debugging purposes.
This patch adds a warning in such cases, notifying the user and also adds a
force option making e2image able to run in such cases.
Also print a warning when not using qcow or raw formats but allows image
creation to proceed, since, such images might be used for metadata backup
purposes.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 misc/e2image.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/misc/e2image.c b/misc/e2image.c
index 885a794..e467301 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -87,7 +87,7 @@ static int get_bits_from_size(size_t size)
 
 static void usage(void)
 {
-	fprintf(stderr, _("Usage: %s [-rsIQa] device image_file\n"),
+	fprintf(stderr, _("Usage: %s [-rsIQaf] device image_file\n"),
 		program_name);
 	exit (1);
 }
@@ -1252,9 +1252,11 @@ int main (int argc, char ** argv)
 	int open_flag = EXT2_FLAG_64BITS;
 	int img_type = 0;
 	int flags = 0;
+	int mount_flags = 0;
 	int qcow2_fd = 0;
 	int fd = 0;
 	int ret = 0;
+	int ignore_rw_mount = 0;
 	struct stat st;
 
 #ifdef ENABLE_NLS
@@ -1269,7 +1271,7 @@ int main (int argc, char ** argv)
 	if (argc && *argv)
 		program_name = *argv;
 	add_error_table(&et_ext2_error_table);
-	while ((c = getopt(argc, argv, "rsIQa")) != EOF)
+	while ((c = getopt(argc, argv, "rsIQaf")) != EOF)
 		switch (c) {
 		case 'I':
 			flags |= E2IMAGE_INSTALL_FLAG;
@@ -1290,6 +1292,9 @@ int main (int argc, char ** argv)
 		case 'a':
 			all_data = 1;
 			break;
+		case 'f':
+			ignore_rw_mount = 1;
+			break;
 		default:
 			usage();
 		}
@@ -1305,6 +1310,31 @@ int main (int argc, char ** argv)
 	device_name = argv[optind];
 	image_fn = argv[optind+1];
 
+	ext2fs_check_if_mounted(device_name, &mount_flags);
+
+	if ((mount_flags & EXT2_MF_MOUNTED) &&
+	   !(mount_flags & EXT2_MF_READONLY)) {
+		fprintf(stderr, "\nWarning: Running e2image on a mounted "
+				"RW filesystem can result in an inconsistent "
+				"image which will not be useful for "
+				"debugging purposes.\n");
+		/*
+		 * A force is required here once qcow and raw image
+		 * formats (img_type is set) are mostly used for
+		 * debugging purposes, while, a "normal" image is
+		 * usually collected for metadata backup.
+		 *
+		 * An inconsistent image is almost useless for
+		 * debugging, so we ensure the filesystem is read-only
+		 * before continue (unless the user force the image creation)
+		 */
+		if (img_type && !ignore_rw_mount) {
+			fprintf(stderr, "Use -f option if you really want to "
+					"do that.\n");
+			exit(1);
+		}
+	}
+
 	if (flags & E2IMAGE_INSTALL_FLAG) {
 		install_image(device_name, image_fn, img_type);
 		exit (0);
-- 
1.8.1.4

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Carlos Maiolino]

I'm so Sorry, I forgot to add a  V3 tag in the subject

On Fri, Sep 27, 2013 at 04:01:35PM -0300, Carlos Maiolino wrote:
> Several users use to run e2image over a mounted RW filesystem, providing
> inconsistent, useless e2images for debugging purposes.
> This patch adds a warning in such cases, notifying the user and also adds a
> force option making e2image able to run in such cases.
> Also print a warning when not using qcow or raw formats but allows image
> creation to proceed, since, such images might be used for metadata backup
> purposes.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  misc/e2image.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/e2image.c b/misc/e2image.c
> index 885a794..e467301 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -87,7 +87,7 @@ static int get_bits_from_size(size_t size)
>  
>  static void usage(void)
>  {
> -	fprintf(stderr, _("Usage: %s [-rsIQa] device image_file\n"),
> +	fprintf(stderr, _("Usage: %s [-rsIQaf] device image_file\n"),
>  		program_name);
>  	exit (1);
>  }
> @@ -1252,9 +1252,11 @@ int main (int argc, char ** argv)
>  	int open_flag = EXT2_FLAG_64BITS;
>  	int img_type = 0;
>  	int flags = 0;
> +	int mount_flags = 0;
>  	int qcow2_fd = 0;
>  	int fd = 0;
>  	int ret = 0;
> +	int ignore_rw_mount = 0;
>  	struct stat st;
>  
>  #ifdef ENABLE_NLS
> @@ -1269,7 +1271,7 @@ int main (int argc, char ** argv)
>  	if (argc && *argv)
>  		program_name = *argv;
>  	add_error_table(&et_ext2_error_table);
> -	while ((c = getopt(argc, argv, "rsIQa")) != EOF)
> +	while ((c = getopt(argc, argv, "rsIQaf")) != EOF)
>  		switch (c) {
>  		case 'I':
>  			flags |= E2IMAGE_INSTALL_FLAG;
> @@ -1290,6 +1292,9 @@ int main (int argc, char ** argv)
>  		case 'a':
>  			all_data = 1;
>  			break;
> +		case 'f':
> +			ignore_rw_mount = 1;
> +			break;
>  		default:
>  			usage();
>  		}
> @@ -1305,6 +1310,31 @@ int main (int argc, char ** argv)
>  	device_name = argv[optind];
>  	image_fn = argv[optind+1];
>  
> +	ext2fs_check_if_mounted(device_name, &mount_flags);
> +
> +	if ((mount_flags & EXT2_MF_MOUNTED) &&
> +	   !(mount_flags & EXT2_MF_READONLY)) {
> +		fprintf(stderr, "\nWarning: Running e2image on a mounted "
> +				"RW filesystem can result in an inconsistent "
> +				"image which will not be useful for "
> +				"debugging purposes.\n");
> +		/*
> +		 * A force is required here once qcow and raw image
> +		 * formats (img_type is set) are mostly used for
> +		 * debugging purposes, while, a "normal" image is
> +		 * usually collected for metadata backup.
> +		 *
> +		 * An inconsistent image is almost useless for
> +		 * debugging, so we ensure the filesystem is read-only
> +		 * before continue (unless the user force the image creation)
> +		 */
> +		if (img_type && !ignore_rw_mount) {
> +			fprintf(stderr, "Use -f option if you really want to "
> +					"do that.\n");
> +			exit(1);
> +		}
> +	}
> +
>  	if (flags & E2IMAGE_INSTALL_FLAG) {
>  		install_image(device_name, image_fn, img_type);
>  		exit (0);
> -- 
> 1.8.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Theodore Ts'o]

On Fri, Sep 27, 2013 at 04:01:35PM -0300, Carlos Maiolino wrote:
> Several users use to run e2image over a mounted RW filesystem, providing
> inconsistent, useless e2images for debugging purposes.
> This patch adds a warning in such cases, notifying the user and also adds a
> force option making e2image able to run in such cases.
> Also print a warning when not using qcow or raw formats but allows image
> creation to proceed, since, such images might be used for metadata backup
> purposes.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

This looks good to me; go ahead and update the man page.  :-)

     	   	       	  	    	   - Ted

Re: [PATCH] e2image: Print a warning if running over a mounted filesystem

[Carlos Maiolino]

Roger,

should send a manpage update today yet

Cheers, and thanks for the review eric and ted
On Mon, Sep 30, 2013 at 04:24:39PM -0400, Theodore Ts'o wrote:
> On Fri, Sep 27, 2013 at 04:01:35PM -0300, Carlos Maiolino wrote:
> > Several users use to run e2image over a mounted RW filesystem, providing
> > inconsistent, useless e2images for debugging purposes.
> > This patch adds a warning in such cases, notifying the user and also adds a
> > force option making e2image able to run in such cases.
> > Also print a warning when not using qcow or raw formats but allows image
> > creation to proceed, since, such images might be used for metadata backup
> > purposes.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> This looks good to me; go ahead and update the man page.  :-)
> 
>      	   	       	  	    	   - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos