* [PATCH] makedumpfile: Check dump file early
@ 2013-04-19 15:36 Michael Holzheu
2013-04-22 0:01 ` HATAYAMA Daisuke
0 siblings, 1 reply; 4+ messages in thread
From: Michael Holzheu @ 2013-04-19 15:36 UTC (permalink / raw)
To: kexec
On s390 the makedumpfile tool sometimes is used directly by
users on the command line. Currently the check if the dump
file already exists is done after the filtering function has
been called. Therefore, for large dumps the user has to wait
for filtering and after some time he gets the error message
"open_dump_file: Can't open the dump file(out). File exists".
This patch improves this by adding an early check for the
existence of the dump file. In case the -f (force) option has
been specified it is checked that an existing file is writable.
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
makedumpfile.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -730,6 +730,24 @@ open_dump_file(void)
}
int
+check_dump_file(const char *path)
+{
+ char *err_str;
+
+ if (access(path, F_OK) != 0)
+ return TRUE; /* File does not exist */
+ if (info->flag_force) {
+ if (access(path, W_OK) == 0)
+ return TRUE; /* We have write permission */
+ err_str = strerror(errno);
+ } else {
+ err_str = "File exists";
+ }
+ ERRMSG("Can't open the dump file (%s). %s\n", path, err_str);
+ return FALSE;
+}
+
+int
open_dump_bitmap(void)
{
int i, fd;
@@ -8609,6 +8627,9 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
+
if (!open_files_for_rearranging_dumpdata())
goto out;
@@ -8626,9 +8647,11 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
- if (!reassemble_dumpfile())
+ if (!check_dump_file(info->name_dumpfile))
goto out;
+ if (!reassemble_dumpfile())
+ goto out;
MSG("\n");
MSG("The dumpfile is saved to %s.\n", info->name_dumpfile);
} else if (info->flag_dmesg) {
@@ -8637,6 +8660,8 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
if (!dump_dmesg())
goto out;
@@ -8648,6 +8673,16 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->flag_split) {
+ for (i = 0; i < info->num_dumpfile; i++) {
+ if (!check_dump_file(SPLITTING_DUMPFILE(i)))
+ goto out;
+ }
+ } else {
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
+ }
+
if (!create_dumpfile())
goto out;
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] makedumpfile: Check dump file early
2013-04-19 15:36 [PATCH] makedumpfile: Check dump file early Michael Holzheu
@ 2013-04-22 0:01 ` HATAYAMA Daisuke
2013-04-22 9:11 ` Michael Holzheu
0 siblings, 1 reply; 4+ messages in thread
From: HATAYAMA Daisuke @ 2013-04-22 0:01 UTC (permalink / raw)
To: Michael Holzheu; +Cc: kexec
(2013/04/20 0:36), Michael Holzheu wrote:
> On s390 the makedumpfile tool sometimes is used directly by
> users on the command line. Currently the check if the dump
> file already exists is done after the filtering function has
> been called. Therefore, for large dumps the user has to wait
> for filtering and after some time he gets the error message
> "open_dump_file: Can't open the dump file(out). File exists".
>
> This patch improves this by adding an early check for the
> existence of the dump file. In case the -f (force) option has
> been specified it is checked that an existing file is writable.
>
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
> makedumpfile.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -730,6 +730,24 @@ open_dump_file(void)
> }
>
> int
> +check_dump_file(const char *path)
> +{
> + char *err_str;
> +
> + if (access(path, F_OK) != 0)
> + return TRUE; /* File does not exist */
> + if (info->flag_force) {
> + if (access(path, W_OK) == 0)
> + return TRUE; /* We have write permission */
> + err_str = strerror(errno);
> + } else {
> + err_str = "File exists";
How about strerror(EEXIST)? It's better to avoid hard code to use the
same string as what libc returns.
--
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] makedumpfile: Check dump file early
2013-04-22 0:01 ` HATAYAMA Daisuke
@ 2013-04-22 9:11 ` Michael Holzheu
2013-04-24 1:33 ` Atsushi Kumagai
0 siblings, 1 reply; 4+ messages in thread
From: Michael Holzheu @ 2013-04-22 9:11 UTC (permalink / raw)
To: HATAYAMA Daisuke; +Cc: kexec
On Mon, 22 Apr 2013 09:01:23 +0900
HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
> > + if (info->flag_force) {
> > + if (access(path, W_OK) == 0)
> > + return TRUE; /* We have write permission */
> > + err_str = strerror(errno);
> > + } else {
> > + err_str = "File exists";
>
> How about strerror(EEXIST)? It's better to avoid hard code to use the
> same string as what libc returns.
Yes, this solution is better.
Here the updated patch:
---
makedumpfile.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -730,6 +730,24 @@ open_dump_file(void)
}
int
+check_dump_file(const char *path)
+{
+ char *err_str;
+
+ if (access(path, F_OK) != 0)
+ return TRUE; /* File does not exist */
+ if (info->flag_force) {
+ if (access(path, W_OK) == 0)
+ return TRUE; /* We have write permission */
+ err_str = strerror(errno);
+ } else {
+ err_str = strerror(EEXIST);
+ }
+ ERRMSG("Can't open the dump file (%s). %s\n", path, err_str);
+ return FALSE;
+}
+
+int
open_dump_bitmap(void)
{
int i, fd;
@@ -8609,6 +8627,9 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
+
if (!open_files_for_rearranging_dumpdata())
goto out;
@@ -8626,9 +8647,11 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
- if (!reassemble_dumpfile())
+ if (!check_dump_file(info->name_dumpfile))
goto out;
+ if (!reassemble_dumpfile())
+ goto out;
MSG("\n");
MSG("The dumpfile is saved to %s.\n", info->name_dumpfile);
} else if (info->flag_dmesg) {
@@ -8637,6 +8660,8 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
if (!dump_dmesg())
goto out;
@@ -8648,6 +8673,16 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->flag_split) {
+ for (i = 0; i < info->num_dumpfile; i++) {
+ if (!check_dump_file(SPLITTING_DUMPFILE(i)))
+ goto out;
+ }
+ } else {
+ if (!check_dump_file(info->name_dumpfile))
+ goto out;
+ }
+
if (!create_dumpfile())
goto out;
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] makedumpfile: Check dump file early
2013-04-22 9:11 ` Michael Holzheu
@ 2013-04-24 1:33 ` Atsushi Kumagai
0 siblings, 0 replies; 4+ messages in thread
From: Atsushi Kumagai @ 2013-04-24 1:33 UTC (permalink / raw)
To: holzheu; +Cc: d.hatayama, kexec
Hello Michael,
On Mon, 22 Apr 2013 11:11:17 +0200
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:
> On Mon, 22 Apr 2013 09:01:23 +0900
> HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
> > > + if (info->flag_force) {
> > > + if (access(path, W_OK) == 0)
> > > + return TRUE; /* We have write permission */
> > > + err_str = strerror(errno);
> > > + } else {
> > > + err_str = "File exists";
> >
> > How about strerror(EEXIST)? It's better to avoid hard code to use the
> > same string as what libc returns.
>
> Yes, this solution is better.
>
> Here the updated patch:
Nice work!
I'll merge this patch into v1.5.4.
Thanks
Atsushi Kumagai
> ---
> makedumpfile.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -730,6 +730,24 @@ open_dump_file(void)
> }
>
> int
> +check_dump_file(const char *path)
> +{
> + char *err_str;
> +
> + if (access(path, F_OK) != 0)
> + return TRUE; /* File does not exist */
> + if (info->flag_force) {
> + if (access(path, W_OK) == 0)
> + return TRUE; /* We have write permission */
> + err_str = strerror(errno);
> + } else {
> + err_str = strerror(EEXIST);
> + }
> + ERRMSG("Can't open the dump file (%s). %s\n", path, err_str);
> + return FALSE;
> +}
> +
> +int
> open_dump_bitmap(void)
> {
> int i, fd;
> @@ -8609,6 +8627,9 @@ main(int argc, char *argv[])
> MSG("Try `makedumpfile --help' for more information.\n");
> goto out;
> }
> + if (!check_dump_file(info->name_dumpfile))
> + goto out;
> +
> if (!open_files_for_rearranging_dumpdata())
> goto out;
>
> @@ -8626,9 +8647,11 @@ main(int argc, char *argv[])
> MSG("Try `makedumpfile --help' for more information.\n");
> goto out;
> }
> - if (!reassemble_dumpfile())
> + if (!check_dump_file(info->name_dumpfile))
> goto out;
>
> + if (!reassemble_dumpfile())
> + goto out;
> MSG("\n");
> MSG("The dumpfile is saved to %s.\n", info->name_dumpfile);
> } else if (info->flag_dmesg) {
> @@ -8637,6 +8660,8 @@ main(int argc, char *argv[])
> MSG("Try `makedumpfile --help' for more information.\n");
> goto out;
> }
> + if (!check_dump_file(info->name_dumpfile))
> + goto out;
> if (!dump_dmesg())
> goto out;
>
> @@ -8648,6 +8673,16 @@ main(int argc, char *argv[])
> MSG("Try `makedumpfile --help' for more information.\n");
> goto out;
> }
> + if (info->flag_split) {
> + for (i = 0; i < info->num_dumpfile; i++) {
> + if (!check_dump_file(SPLITTING_DUMPFILE(i)))
> + goto out;
> + }
> + } else {
> + if (!check_dump_file(info->name_dumpfile))
> + goto out;
> + }
> +
> if (!create_dumpfile())
> goto out;
>
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-24 1:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-19 15:36 [PATCH] makedumpfile: Check dump file early Michael Holzheu
2013-04-22 0:01 ` HATAYAMA Daisuke
2013-04-22 9:11 ` Michael Holzheu
2013-04-24 1:33 ` Atsushi Kumagai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.