Re: [Xen-devel] [PATCH 3/8] kexec: add public interface for improved load/unload sub-ops

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Feb 21, 2013 at 05:48:09PM +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@xxxxxxxxxx>
>
> Add replacement KEXEC_CMD_load and KEXEC_CMD_unload sub-ops to the
> kexec hypercall.  These new sub-ops allow a priviledged guest to
> provide the image data to be loaded into Xen memory or the crash
> region instead of guests loading the image data themselves and
> providing the relocation code and metadata.
>
> The old interface is provided to guests requesting an interface
> version prior to 4.3.
>
> Signed-off: David Vrabel <david.vrabel@xxxxxxxxxx>

[...]

> diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
> index 61a8d7d..5259446 100644
> --- a/xen/include/public/kexec.h
> +++ b/xen/include/public/kexec.h
> @@ -116,12 +116,12 @@ typedef struct xen_kexec_exec {
>   * type  == KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH [in]
>   * image == relocation information for kexec (ignored for unload) [in]
>   */
> -#define KEXEC_CMD_kexec_load            1
> -#define KEXEC_CMD_kexec_unload          2
> -typedef struct xen_kexec_load {
> +#define KEXEC_CMD_kexec_load_v1         1 /* obsolete since 0x00040300 */
> +#define KEXEC_CMD_kexec_unload_v1       2 /* obsolete since 0x00040300 */
> +typedef struct xen_kexec_load_v1 {
>      int type;
>      xen_kexec_image_t image;
> -} xen_kexec_load_t;
> +} xen_kexec_load_v1_t;

I think that this is not good idea to redefine meaning of constants,
types, structures, etc. IMO it is comparable to redefining meaning
of words in any laguage (e.g. English). It will be very confusing
and may easily lead to stupid bugs. I think that old interface should
stay as is (with its bad behavior). New interface should be introduced
with "_v2" suffix, e.g. KEXEC_CMD_kexec_load_v2, ...
This would not confuse our descendants.

Daniel

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