Re: [Xen-devel] [PATCH v4] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Wed, Jan 25, 2017 at 09:31:15AM -0600, Eric DeVolder wrote:

[...]

> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index 500e5a9..ec16247 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -51,6 +51,9 @@
>  #include "kexec-lzma.h"
>  #include <arch/options.h>
>
> +#define KEXEC_LOADED_PATH "/sys/kernel/kexec_loaded"
> +#define KEXEC_CRASH_LOADED_PATH "/sys/kernel/kexec_crash_loaded"
> +
>  unsigned long long mem_min = 0;
>  unsigned long long mem_max = ULONG_MAX;
>  static unsigned long kexec_flags = 0;
> @@ -890,8 +893,6 @@ static int my_exec(void)
>       return -1;
>  }
>
> -static int kexec_loaded(void);
> -
>  static int load_jump_back_helper_image(unsigned long kexec_flags, void 
> *entry)
>  {
>       int result;
> @@ -902,6 +903,40 @@ static int load_jump_back_helper_image(unsigned long 
> kexec_flags, void *entry)
>       return result;
>  }
>
> +static int kexec_loaded(const char *file)
> +{
> +     long ret = -1;
> +     FILE *fp;
> +     char *p;
> +     char line[3];
> +
> +     /* No way to tell if an image is loaded under Xen, assume it is. */
> +     if (xen_present())
> +             return 1;
> +
> +     fp = fopen(file, "r");
> +     if (fp == NULL)
> +             return -1;
> +
> +     p = fgets(line, sizeof(line), fp);
> +     fclose(fp);
> +
> +     if (p == NULL)
> +             return -1;
> +
> +     ret = strtol(line, &p, 10);
> +
> +     /* Too long */
> +     if (ret > INT_MAX)
> +             return -1;
> +
> +     /* No digits were found */
> +     if (p == line)
> +             return -1;
> +
> +     return (int)ret;
> +}
> +
>  /*
>   *   Jump back to the original kernel
>   */
> @@ -909,7 +944,7 @@ static int my_load_jump_back_helper(unsigned long 
> kexec_flags, void *entry)
>  {
>       int result;
>
> -     if (kexec_loaded()) {
> +     if (kexec_loaded(KEXEC_LOADED_PATH)) {
>               fprintf(stderr, "There is kexec kernel loaded, make sure "
>                       "you are in kexeced kernel.\n");
>               return -1;
> @@ -970,6 +1005,7 @@ void usage(void)
>              "                      to original kernel.\n"
>              " -s, --kexec-file-syscall Use file based syscall for kexec 
> operation\n"
>              " -d, --debug           Enable debugging to help spot a 
> failure.\n"
> +            " -S, --status         Return 0 if the type (by default crash) 
> is loaded.\n"
>              "\n"
>              "Supported kernel file types and options: \n");
>       for (i = 0; i < file_types; i++) {
> @@ -981,40 +1017,30 @@ void usage(void)
>       printf("\n");
>  }
>
> -static int kexec_loaded(void)
> +static int k_status(unsigned long kexec_flags)
>  {
> -     long ret = -1;
> -     FILE *fp;
> -     char *p;
> -     char line[3];
> +     int result;
> +     long native_arch;
> +
> +     /* set the arch */
> +     native_arch = physical_arch();
> +     if (native_arch < 0) {
> +             return -1;
> +     }
> +     kexec_flags |= native_arch;
>
> -     /* No way to tell if an image is loaded under Xen, assume it is. */
>       if (xen_present())
> -             return 1;
> -
> -     fp = fopen("/sys/kernel/kexec_loaded", "r");
> -     if (fp == NULL)
> -             return -1;
> -
> -     p = fgets(line, sizeof(line), fp);
> -     fclose(fp);
> -
> -     if (p == NULL)
> -             return -1;
> -
> -     ret = strtol(line, &p, 10);
> -
> -     /* Too long */
> -     if (ret > INT_MAX)
> -             return -1;
> -
> -     /* No digits were found */
> -     if (p == line)
> -             return -1;
> -
> -     return (int)ret;
> +             result = xen_kexec_status(kexec_flags);
> +     else {
> +             if (kexec_flags & KEXEC_ON_CRASH)
> +                     result = kexec_loaded(KEXEC_CRASH_LOADED_PATH);
> +             else
> +                     result = kexec_loaded(KEXEC_LOADED_PATH);
> +     }
> +     return result;
>  }

Ohhh... This is awful. Have you tried --patience option for "git format-patch"?
Does it help? If yes please repost. If it does not let's wait for maintainers
opinion about that. Maybe we should leave forward declaration in first patch
as is and then move kexec_loaded() in second (as a cleanup).

Though otherwise LGTM.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel