[gPXE-devel] [PATCH] hierarchal friendly settings_ui

Stefan Hajnoczi stefanha at gmail.com
Fri Jul 9 01:22:40 EDT 2010


Thanks for resending it!

Stefan

On Fri, Jul 9, 2010 at 6:20 AM, Glenn Brown <glenn at myri.com> wrote:
>> Did patch 1/1 tag_type make it to the mailing list? I don't seem to have
>> it.
>
> I received a copy via CC, and sent the patch set with 'git send-email' so I
> don't think the loss was sender error, for what it's worth. Anyway, I've
> included the patch below, inline, and also attached a copy in case email
> formatting or cut-and-paste corrupts the inline version.
>
> --Glenn
>
> -------- Original Message --------
> From: - Tue Jul 06 16:19:45 2010
> X-Mozilla-Status: 0001
> X-Mozilla-Status2: 00000000
> Received: from glenn (glenn.sw.myri.com [172.31.129.6]) by myri.com
> (8.13.7+Sun/8.13.7) with ESMTP id o66NI0QW005219;   Tue, 6 Jul 2010 16:18:00
> -0700 (PDT)
> Received: by glenn (Postfix, from userid 8091)  id 2853CC0157; Tue,  6 Jul
> 2010 16:17:59 -0700 (PDT)
> From: Glenn Brown <glenn at myri.com>
> To: gpxe-devel at etherboot.org
> Cc: Glenn Brown <glenn at myri.com>
> Subject: [PATCH 1/6] [settings] Introduce TAG_TYPE().
> Date: Tue,  6 Jul 2010 16:17:37 -0700
> Message-Id: <1278458262-7478-2-git-send-email-glenn at myri.com>
> X-Mailer: git-send-email 1.7.0.4
> In-Reply-To: <1278458262-7478-1-git-send-email-glenn at myri.com>
> References: <1278458262-7478-1-git-send-email-glenn at myri.com>
>
> TAG_TYPE() is a new macro to extract a type from the tag field of each
> setting and from the tag_magic field of each settings block.  A Setting
> and a settings block are compatible if and only if their TAG_TYPE()s
> match.  TAG_TYPE() makes the compatibility testing independent of the
> particular module, while maintaining setting numbering for backwards
> compatibility.
>
> For clarity and consistency:
>  Make SMBIOS_TAG_MAGIC be 32 bits, the size of settings.tag_magic.
>  Explicitly name DHCP_TAG_MAGIC, replacing 0's found in code.
>
> TAG_READONLY() is included in this patch to show why TAG_TYPE() uses
> "0xfd" instead of "0xff".  These definitions were chosen for backwards
> compatibility with the previous tag_magic values, which were
>        0x00000000 for DHCP settings
>        0x5b000000 for SMBIOS settings
>        0xc19c1900 for Phantom CLP settings.
> TAG_READONLY() indicates which settings cannot be set by users and will
> be used to clean up the settings_ui in a forthcoming patch.
>
> Signed-off-by: Glenn Brown <glenn at myri.com>
> ---
>  src/core/nvo.c                         |    2 +-
>  src/include/gpxe/dhcp.h                |    3 +++
>  src/include/gpxe/settings.h            |   22 +++++++++++++++++++++-
>  src/interface/smbios/smbios_settings.c |   19 +++++--------------
>  src/net/dhcppkt.c                      |    2 +-
>  5 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/src/core/nvo.c b/src/core/nvo.c
> index 3dbf51d..53dd215 100644
> --- a/src/core/nvo.c
> +++ b/src/core/nvo.c
> @@ -205,7 +205,7 @@ void nvo_init ( struct nvo_block *nvo, struct nvs_device
> *nvs,
>        nvo->nvs = nvs;
>        nvo->fragments = fragments;
>        settings_init ( &nvo->settings, &nvo_settings_operations, refcnt,
> -                       "nvo", 0 );
> +                       "nvo", DHCP_TAG_MAGIC );
>  }
>
>  /**
> diff --git a/src/include/gpxe/dhcp.h b/src/include/gpxe/dhcp.h
> index c747948..1b72980 100644
> --- a/src/include/gpxe/dhcp.h
> +++ b/src/include/gpxe/dhcp.h
> @@ -612,6 +612,9 @@ struct dhcphdr {
>  /** Setting block name used for BootServerDHCP responses */
>  #define PXEBS_SETTINGS_NAME "pxebs"
>
> +/** Settings block tag_magic used for blocks holding DHCP settings. */
> +#define DHCP_TAG_MAGIC 0x00000000
> +
>  extern void * dhcp_chaddr ( struct net_device *netdev, uint8_t *hlen,
>                            uint16_t *flags );
>  extern int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
> diff --git a/src/include/gpxe/settings.h b/src/include/gpxe/settings.h
> index efefe73..17c9b49 100644
> --- a/src/include/gpxe/settings.h
> +++ b/src/include/gpxe/settings.h
> @@ -14,6 +14,18 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  #include <gpxe/list.h>
>  #include <gpxe/refcnt.h>
>
> +/** Type of a setting or settings block
> + *
> + * The type is encoded in the tag field of each setting and the
> + * tag_magic field of each settings block.  Settings are applicable to
> + * the settings block if and only iff the tag type of the setting
> + * matches the tag type of the settings block.
> + */
> +#define TAG_TYPE( tag ) ( ( ( tag ) >> 24 ) & 0xfd )
> +
> +/** Readonly status of a setting. */
> +#define TAG_READONLY( tag ) ( ( ( tag ) >> 25 ) & 1 )
> +
>  struct settings;
>  struct in_addr;
>  union uuid;
> @@ -33,7 +45,12 @@ struct setting {
>         * address, etc.).
>         */
>        struct setting_type *type;
> -       /** DHCP option number, if applicable */
> +       /** DHCP option number, if applicable
> +        *
> +        * This field may also hold non-DHCP setting identifiers, such
> +        * as for SMBIOS or Phantom CLP, and includes a 'readonly' bit.
> +        * See TAG_TYPE() and TAG_READONLY().
> +        */
>        unsigned int tag;
>  };
>
> @@ -87,6 +104,9 @@ struct settings {
>         * constructed by parse_setting_name(), and can be used to
>         * avoid e.g. attempting to retrieve the subnet mask from
>         * SMBIOS, or the system UUID from DHCP.
> +        *
> +        * The TAG_TYPE() of this field identifies which setting
> +        * structures are relevant to this settings block.
>         */
>        unsigned int tag_magic;
>        /** Parent settings block */
> diff --git a/src/interface/smbios/smbios_settings.c
> b/src/interface/smbios/smbios_settings.c
> index 1c96564..aa8a6b3 100644
> --- a/src/interface/smbios/smbios_settings.c
> +++ b/src/interface/smbios/smbios_settings.c
> @@ -27,14 +27,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  #include <gpxe/smbios.h>
>
>  /** SMBIOS settings tag magic number */
> -#define SMBIOS_TAG_MAGIC 0x5B /* "SmBios" */
> -
> -/**
> - * Construct SMBIOS empty tag
> - *
> - * @ret tag            SMBIOS setting tag
> - */
> -#define SMBIOS_EMPTY_TAG ( SMBIOS_TAG_MAGIC << 24 )
> +#define SMBIOS_TAG_MAGIC 0x5B000000 /* "SmBios" */
>
>  /**
>  * Construct SMBIOS raw-data tag
> @@ -45,7 +38,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  * @ret tag            SMBIOS setting tag
>  */
>  #define SMBIOS_RAW_TAG( _type, _structure, _field )            \
> -       ( ( SMBIOS_TAG_MAGIC << 24 ) |                          \
> +       ( SMBIOS_TAG_MAGIC |                                    \
>          ( (_type) << 16 ) |                                   \
>          ( offsetof ( _structure, _field ) << 8 ) |            \
>          ( sizeof ( ( ( _structure * ) 0 )->_field ) ) )
> @@ -59,7 +52,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  * @ret tag            SMBIOS setting tag
>  */
>  #define SMBIOS_STRING_TAG( _type, _structure, _field )         \
> -       ( ( SMBIOS_TAG_MAGIC << 24 ) |                          \
> +       ( SMBIOS_TAG_MAGIC |                                    \
>          ( (_type) << 16 ) |                                   \
>          ( offsetof ( _structure, _field ) << 8 ) )
>
> @@ -76,18 +69,16 @@ static int smbios_fetch ( struct settings *settings
> __unused,
>                          struct setting *setting,
>                          void *data, size_t len ) {
>        struct smbios_structure structure;
> -       unsigned int tag_magic;
>        unsigned int tag_type;
>        unsigned int tag_offset;
>        unsigned int tag_len;
>        int rc;
>
>        /* Split tag into type, offset and length */
> -       tag_magic = ( setting->tag >> 24 );
>        tag_type = ( ( setting->tag >> 16 ) & 0xff );
>        tag_offset = ( ( setting->tag >> 8 ) & 0xff );
>        tag_len = ( setting->tag & 0xff );
> -       if ( tag_magic != SMBIOS_TAG_MAGIC )
> +       if ( TAG_TYPE ( setting->tag ) != TAG_TYPE (SMBIOS_TAG_MAGIC) )
>                return -ENOENT;
>
>        /* Find SMBIOS structure */
> @@ -126,7 +117,7 @@ static struct settings_operations
> smbios_settings_operations = {
>  static struct settings smbios_settings = {
>        .refcnt = NULL,
>        .name = "smbios",
> -       .tag_magic = SMBIOS_EMPTY_TAG,
> +       .tag_magic = SMBIOS_TAG_MAGIC,
>        .siblings = LIST_HEAD_INIT ( smbios_settings.siblings ),
>        .children = LIST_HEAD_INIT ( smbios_settings.children ),
>        .op = &smbios_settings_operations,
> diff --git a/src/net/dhcppkt.c b/src/net/dhcppkt.c
> index 20a0e66..1938a66 100644
> --- a/src/net/dhcppkt.c
> +++ b/src/net/dhcppkt.c
> @@ -279,5 +279,5 @@ void dhcppkt_init ( struct dhcp_packet *dhcppkt, struct
> dhcphdr *data,
>                         dhcppkt->options.len );
>        settings_init ( &dhcppkt->settings,
>                        &dhcppkt_settings_operations, &dhcppkt->refcnt,
> -                       DHCP_SETTINGS_NAME, 0 );
> +                       DHCP_SETTINGS_NAME, DHCP_TAG_MAGIC );
>  }
> --
> 1.7.0.4
>
>


More information about the gPXE-devel mailing list