* [PATCH 0/3] Search for specific config files using UUID, MAC and IP
@ 2019-10-04 22:44 Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-04 22:44 UTC (permalink / raw)
To: grub-devel
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
Javier Martinez Canillas
Hello,
This series contains some patches that have been carried in Fedora (and
other distros) for a long time. It allows to fetch GRUB configurations
that are customized for each machine by using the client UUID, MAC and
IP addresses as suffixes for the grub.cfg file.
This procedure is similar to what is used by pxelinux and yaboot:
http://www.syslinux.org/wiki/index.php/PXELINUX#config
Patch #1 sets net_<interface>_client{id,uuid} environment variables by
using the DHCP client ID and UUID options if these are found.
Patch #2 adds a %X format specifier to make the grub_printf() function
more similar to printf() and not require to use %x plus grub_toupper().
Finally Patch #3 adds the support to fetch the config files using the
UUID, MAC and IP as suffixes. If these aren't found, the default GRUB
config filename is used as a fallback.
Best regards,
Javier
Paulo Flabiano Smorigo (3):
Set net_<interface>_client{id,uuid} variables from DHCP options
Add %X option to printf functions
Search for specific config files for netboot
grub-core/kern/misc.c | 7 ++-
grub-core/net/bootp.c | 85 ++++++++++++++++++++++++++---
grub-core/net/net.c | 117 ++++++++++++++++++++++++++++++++++++++++
grub-core/normal/main.c | 17 ++++--
include/grub/net.h | 5 ++
5 files changed, 217 insertions(+), 14 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options
2019-10-04 22:44 [PATCH 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
@ 2019-10-04 22:44 ` Javier Martinez Canillas
2019-10-15 14:19 ` Daniel Kiper
2019-10-04 22:44 ` [PATCH 2/3] Add %X option to printf functions Javier Martinez Canillas
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-04 22:44 UTC (permalink / raw)
To: grub-devel
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
Javier Martinez Canillas
From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
GRUB environment variables, using the DHCP client ID and UUID options if
these are found.
In the same way than net_<interface>_<option> variables are set for other
options such domain name, boot file, next server, etc.
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/net/bootp.c | 85 +++++++++++++++++++++++++++++++++++++++----
include/grub/net.h | 2 +
2 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 04cfbb04504..0e6e41a1699 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -95,6 +95,49 @@ enum
/* Max timeout when waiting for BOOTP/DHCP reply */
#define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
+static char *
+grub_env_write_readonly (struct grub_env_var *var __attribute__ ((unused)),
+ const char *val __attribute__ ((unused)))
+{
+ return NULL;
+}
+
+static void
+set_env_limn_ro (const char *intername, const char *suffix,
+ const char *value, grub_size_t len)
+{
+ char *varname, *varvalue;
+ char *ptr;
+ varname = grub_xasprintf ("net_%s_%s", intername, suffix);
+ if (!varname)
+ return;
+ for (ptr = varname; *ptr; ptr++)
+ if (*ptr == ':')
+ *ptr = '_';
+ varvalue = grub_malloc (len + 1);
+ if (!varvalue)
+ {
+ grub_free (varname);
+ return;
+ }
+
+ grub_memcpy (varvalue, value, len);
+ varvalue[len] = 0;
+ grub_env_set (varname, varvalue);
+ grub_register_variable_hook (varname, 0, grub_env_write_readonly);
+ grub_env_export (varname);
+ grub_free (varname);
+ grub_free (varvalue);
+}
+
+static char
+hexdigit (grub_uint8_t val)
+{
+ if (val < 10)
+ return val + '0';
+ return val + 'a' - 10;
+}
+
static const void *
find_dhcp_option (const struct grub_net_bootp_packet *bp, grub_size_t size,
grub_uint8_t opt_code, grub_uint8_t *opt_len)
@@ -152,6 +195,9 @@ again:
if (i + taglength >= size)
return NULL;
+ grub_dprintf("net", "DHCP option %u (0x%02x) found with length %u.\n",
+ tagtype, tagtype, taglength);
+
/* FIXME RFC 3396 options concatentation */
if (tagtype == opt_code)
{
@@ -354,6 +400,37 @@ grub_net_configure_by_dhcp_ack (const char *name,
}
grub_net_add_ipv4_local (inter, mask);
+ opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_ID, &opt_len);
+ if (opt)
+ {
+ set_env_limn_ro (name, "clientid", (char *) opt, opt_len);
+ }
+
+ opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_UUID, &opt_len);
+ if (opt && opt_len == 17)
+ {
+ /* The format is 9cfe245e-d0c8-bd45-a79f-54ea5fbd3d97 */
+
+ opt += 1;
+ opt_len -= 1;
+
+ char *val = grub_malloc (2 * opt_len + 4 + 1);
+ int i = 0;
+ int j = 0;
+ for (i = 0; i < opt_len; i++)
+ {
+ val[2 * i + j] = hexdigit (opt[i] >> 4);
+ val[2 * i + 1 + j] = hexdigit (opt[i] & 0xf);
+
+ if ((i == 3) || (i == 5) || (i == 7) || (i == 9))
+ {
+ j++;
+ val[2 * i + 1+ j] = '-';
+ }
+ }
+ set_env_limn_ro (name, "clientuuid", (char *) val, 2 * opt_len + 4);
+ }
+
/* We do not implement dead gateway detection and the first entry SHOULD
be preferred one */
opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_ROUTER, &opt_len);
@@ -631,14 +708,6 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
}
}
-static char
-hexdigit (grub_uint8_t val)
-{
- if (val < 10)
- return val + '0';
- return val + 'a' - 10;
-}
-
static grub_err_t
grub_cmd_dhcpopt (struct grub_command *cmd __attribute__ ((unused)),
int argc, char **args)
diff --git a/include/grub/net.h b/include/grub/net.h
index 4a9069a1474..556c54e579f 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -462,6 +462,8 @@ enum
GRUB_NET_BOOTP_DOMAIN = 0x0f,
GRUB_NET_BOOTP_ROOT_PATH = 0x11,
GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12,
+ GRUB_NET_BOOTP_CLIENT_ID = 0x3d,
+ GRUB_NET_BOOTP_CLIENT_UUID = 0x61,
GRUB_NET_DHCP_REQUESTED_IP_ADDRESS = 50,
GRUB_NET_DHCP_OVERLOAD = 52,
GRUB_NET_DHCP_MESSAGE_TYPE = 53,
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] Add %X option to printf functions
2019-10-04 22:44 [PATCH 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
@ 2019-10-04 22:44 ` Javier Martinez Canillas
2019-10-05 1:47 ` Steve McIntyre
2019-10-15 14:24 ` Daniel Kiper
2019-10-04 22:44 ` [PATCH 3/3] Search for specific config files for netboot Javier Martinez Canillas
2019-10-15 14:37 ` [PATCH 0/3] Search for specific config files using UUID, MAC and IP Daniel Kiper
3 siblings, 2 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-04 22:44 UTC (permalink / raw)
To: grub-devel
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
Javier Martinez Canillas
From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
The printf(3) function has support for the %X format specifier, to output
an unsigned hexadecimal integer in uppercase.
This can be achived in GRUB using the %x format specifier in grub_printf()
and calling grub_toupper(), but it is more convenient if there is support
for %X in grub_printf().
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/kern/misc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 3b633d51f4c..76e7fb22872 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d, grub_uint64_t *r)
static inline char *
grub_lltoa (char *str, int c, unsigned long long n)
{
- unsigned base = (c == 'x') ? 16 : 10;
+ unsigned base = ((c == 'x') || (c == 'X')) ? 16 : 10;
char *p;
if ((long long) n < 0 && c == 'd')
@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
do
{
unsigned d = (unsigned) (n & 0xf);
- *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
+ *p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';
}
while (n >>= 4);
else
@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
{
case 'p':
case 'x':
+ case 'X':
case 'u':
case 'd':
case 'c':
@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
switch (c)
{
case 'x':
+ case 'X':
case 'u':
args->ptr[curn].type = UNSIGNED_INT + longfmt;
break;
@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const char *fmt0,
c = 'x';
/* Fall through. */
case 'x':
+ case 'X':
case 'u':
case 'd':
{
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] Search for specific config files for netboot
2019-10-04 22:44 [PATCH 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 2/3] Add %X option to printf functions Javier Martinez Canillas
@ 2019-10-04 22:44 ` Javier Martinez Canillas
2019-10-15 14:35 ` Daniel Kiper
2019-10-15 14:37 ` [PATCH 0/3] Search for specific config files using UUID, MAC and IP Daniel Kiper
3 siblings, 1 reply; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-04 22:44 UTC (permalink / raw)
To: grub-devel
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
Javier Martinez Canillas
From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
This patch implements a search for a specific configuration when the config
file is on a remoteserver. It uses the following order:
1) DHCP client UUID option.
2) MAC address (in lower case hexadecimal with dash separators);
3) IP (in upper case hexadecimal) or IPv6;
4) The original grub.cfg file.
This procedure is similar to what is used by pxelinux and yaboot:
http://www.syslinux.org/wiki/index.php/PXELINUX#config
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/net/net.c | 117 ++++++++++++++++++++++++++++++++++++++++
grub-core/normal/main.c | 17 ++++--
include/grub/net.h | 3 ++
3 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index d5d726a315e..6492bb74274 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1735,6 +1735,123 @@ grub_net_restore_hw (void)
return GRUB_ERR_NONE;
}
+grub_err_t
+grub_net_search_configfile (char *config)
+{
+ grub_size_t config_len;
+ char *suffix;
+
+ auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
+ int search_through (grub_size_t num_tries, grub_size_t slice_size)
+ {
+ while (num_tries-- > 0)
+ {
+ grub_dprintf ("net", "attempt to fetch config %s\n", config);
+
+ grub_file_t file;
+ file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
+
+ if (file)
+ {
+ grub_file_close (file);
+ return 0;
+ }
+ else
+ {
+ if (grub_errno == GRUB_ERR_IO)
+ grub_errno = GRUB_ERR_NONE;
+ }
+
+ if (grub_strlen (suffix) < slice_size)
+ break;
+
+ config[grub_strlen (config) - slice_size] = '\0';
+ }
+
+ return 1;
+ }
+
+ config_len = grub_strlen (config);
+ config[config_len] = '-';
+ suffix = config + config_len + 1;
+
+ struct grub_net_network_level_interface *inf;
+ FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
+ {
+ /* By the Client UUID. */
+
+ char client_uuid_var[sizeof ("net_") + grub_strlen (inf->name) +
+ sizeof ("_clientuuid") + 1];
+ grub_snprintf (client_uuid_var, sizeof (client_uuid_var),
+ "net_%s_clientuuid", inf->name);
+
+ const char *client_uuid;
+ client_uuid = grub_env_get (client_uuid_var);
+
+ if (client_uuid)
+ {
+ grub_strcpy (suffix, client_uuid);
+ if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+ }
+
+ /* By the MAC address. */
+
+ /* Add ethernet type */
+ grub_strcpy (suffix, "01-");
+
+ grub_net_hwaddr_to_str (&inf->hwaddress, suffix + 3);
+
+ char *ptr;
+ for (ptr = suffix; *ptr; ptr++)
+ if (*ptr == ':')
+ *ptr = '-';
+
+ if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+
+ /* By IP address */
+
+ switch ((&inf->address)->type)
+ {
+ case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4:
+ {
+ grub_uint32_t n = grub_be_to_cpu32 ((&inf->address)->ipv4);
+ grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%02X%02X%02X%02X", \
+ ((n >> 24) & 0xff), ((n >> 16) & 0xff), \
+ ((n >> 8) & 0xff), ((n >> 0) & 0xff));
+
+ if (search_through (8, 1) == 0) return GRUB_ERR_NONE;
+ break;
+ }
+ case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
+ {
+ char buf[GRUB_NET_MAX_STR_ADDR_LEN];
+ struct grub_net_network_level_address base;
+ base.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
+ grub_memcpy (&base.ipv6, ((&inf->address)->ipv6), 16);
+ grub_net_addr_to_str (&base, buf);
+
+ for (ptr = buf; *ptr; ptr++)
+ if (*ptr == ':')
+ *ptr = '-';
+
+ grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
+ if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
+ break;
+ }
+ case GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV:
+ return grub_error (GRUB_ERR_BUG, "shouldn't reach here");
+ default:
+ return grub_error (GRUB_ERR_BUG,
+ "unsupported address type %d", (&inf->address)->type);
+ }
+ }
+
+ /* Remove the remaining minus sign at the end. */
+ config[config_len] = '\0';
+
+ return GRUB_ERR_NONE;
+}
+
static struct grub_preboot *fini_hnd;
static grub_command_t cmd_addaddr, cmd_deladdr, cmd_addroute, cmd_delroute;
diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index 1b03dfd57b9..62ad546f4f7 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -323,10 +323,19 @@ grub_cmd_normal (struct grub_command *cmd __attribute__ ((unused)),
prefix = grub_env_get ("prefix");
if (prefix)
- {
- config = grub_xasprintf ("%s/grub.cfg", prefix);
- if (! config)
- goto quit;
+ {
+ grub_size_t config_len;
+ config_len = grub_strlen (prefix) +
+ sizeof ("/grub.cfg-XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX");
+ config = grub_malloc (config_len);
+
+ if (! config)
+ goto quit;
+
+ grub_snprintf (config, config_len, "%s/grub.cfg", prefix);
+
+ if (grub_strncmp (prefix + 1, "tftp", sizeof ("tftp") - 1) == 0)
+ grub_net_search_configfile (config);
grub_enter_normal_mode (config);
grub_free (config);
diff --git a/include/grub/net.h b/include/grub/net.h
index 556c54e579f..ff6d347f7da 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -578,4 +578,7 @@ extern char *grub_net_default_server;
#define VLANTAG_IDENTIFIER 0x8100
+grub_err_t
+grub_net_search_configfile (char *config);
+
#endif /* ! GRUB_NET_HEADER */
--
2.21.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] Add %X option to printf functions
2019-10-04 22:44 ` [PATCH 2/3] Add %X option to printf functions Javier Martinez Canillas
@ 2019-10-05 1:47 ` Steve McIntyre
2019-10-05 7:15 ` Javier Martinez Canillas
2019-10-15 14:26 ` Daniel Kiper
2019-10-15 14:24 ` Daniel Kiper
1 sibling, 2 replies; 13+ messages in thread
From: Steve McIntyre @ 2019-10-05 1:47 UTC (permalink / raw)
To: The development of GNU GRUB
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo,
Javier Martinez Canillas
On Sat, Oct 05, 2019 at 12:44:26AM +0200, Javier Martinez Canillas wrote:
>From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>
>The printf(3) function has support for the %X format specifier, to output
>an unsigned hexadecimal integer in uppercase.
>
>This can be achived in GRUB using the %x format specifier in grub_printf()
>and calling grub_toupper(), but it is more convenient if there is support
>for %X in grub_printf().
>
>Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>---
>
> grub-core/kern/misc.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>index 3b633d51f4c..76e7fb22872 100644
>--- a/grub-core/kern/misc.c
>+++ b/grub-core/kern/misc.c
>@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d, grub_uint64_t *r)
> static inline char *
> grub_lltoa (char *str, int c, unsigned long long n)
> {
>- unsigned base = (c == 'x') ? 16 : 10;
>+ unsigned base = ((c == 'x') || (c == 'X')) ? 16 : 10;
> char *p;
>
> if ((long long) n < 0 && c == 'd')
>@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
> do
> {
> unsigned d = (unsigned) (n & 0xf);
>- *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
>+ *p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';
> }
> while (n >>= 4);
> else
>@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
> {
> case 'p':
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> case 'c':
>@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
> switch (c)
> {
> case 'x':
>+ case 'X':
> case 'u':
> args->ptr[curn].type = UNSIGNED_INT + longfmt;
> break;
>@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const char *fmt0,
> c = 'x';
> /* Fall through. */
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> {
Ummm, isn't this exactly the same patch that Colin Watson proposed and
I reviewed back in March, as part of his changeset for UEFI?
https://lists.gnu.org/archive/html/grub-devel/2019-03/msg00121.html
Oh, that never got applied. :-(
--
Steve McIntyre, Cambridge, UK. steve@einval.com
Mature Sporty Personal
More Innovation More Adult
A Man in Dandism
Powered Midship Specialty
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] Add %X option to printf functions
2019-10-05 1:47 ` Steve McIntyre
@ 2019-10-05 7:15 ` Javier Martinez Canillas
2019-10-15 14:26 ` Daniel Kiper
1 sibling, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-05 7:15 UTC (permalink / raw)
To: Steve McIntyre, The development of GNU GRUB
Cc: Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
Hello Steve,
On 10/5/19 3:47 AM, Steve McIntyre wrote:
> On Sat, Oct 05, 2019 at 12:44:26AM +0200, Javier Martinez Canillas wrote:
[snip]
>> c = 'x';
>> /* Fall through. */
>> case 'x':
>> + case 'X':
>> case 'u':
>> case 'd':
>> {
>
> Ummm, isn't this exactly the same patch that Colin Watson proposed and
> I reviewed back in March, as part of his changeset for UEFI?
>
> https://lists.gnu.org/archive/html/grub-devel/2019-03/msg00121.html
>
> Oh, that never got applied. :-(
>
That patch does the same indeed. If that gets applied then
I could just drop this patch from the series.
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options
2019-10-04 22:44 ` [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
@ 2019-10-15 14:19 ` Daniel Kiper
2019-10-18 7:46 ` Javier Martinez Canillas
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Kiper @ 2019-10-15 14:19 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
On Sat, Oct 05, 2019 at 12:44:25AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>
> This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
> GRUB environment variables, using the DHCP client ID and UUID options if
> these are found.
>
> In the same way than net_<interface>_<option> variables are set for other
> options such domain name, boot file, next server, etc.
>
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
In general "Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>"
but a few nit picks below...
> ---
>
> grub-core/net/bootp.c | 85 +++++++++++++++++++++++++++++++++++++++----
> include/grub/net.h | 2 +
> 2 files changed, 79 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index 04cfbb04504..0e6e41a1699 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -95,6 +95,49 @@ enum
> /* Max timeout when waiting for BOOTP/DHCP reply */
> #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>
> +static char *
> +grub_env_write_readonly (struct grub_env_var *var __attribute__ ((unused)),
> + const char *val __attribute__ ((unused)))
s/grub_env_write_readonly/grub_env_ro_write/?
> +{
> + return NULL;
> +}
> +
> +static void
> +set_env_limn_ro (const char *intername, const char *suffix,
> + const char *value, grub_size_t len)
I have hard time reading this. What does "limn" stand for?
Could we make the name of this function less cryptic?
> +{
> + char *varname, *varvalue;
> + char *ptr;
Please add empty line here.
> + varname = grub_xasprintf ("net_%s_%s", intername, suffix);
> + if (!varname)
> + return;
> + for (ptr = varname; *ptr; ptr++)
> + if (*ptr == ':')
> + *ptr = '_';
> + varvalue = grub_malloc (len + 1);
grub_zalloc() and then you can...
> + if (!varvalue)
> + {
> + grub_free (varname);
> + return;
> + }
> +
> + grub_memcpy (varvalue, value, len);
> + varvalue[len] = 0;
...drop this...
Daniel
> + grub_env_set (varname, varvalue);
> + grub_register_variable_hook (varname, 0, grub_env_write_readonly);
> + grub_env_export (varname);
> + grub_free (varname);
> + grub_free (varvalue);
> +}
> +
> +static char
> +hexdigit (grub_uint8_t val)
> +{
> + if (val < 10)
> + return val + '0';
> + return val + 'a' - 10;
> +}
> +
> static const void *
> find_dhcp_option (const struct grub_net_bootp_packet *bp, grub_size_t size,
> grub_uint8_t opt_code, grub_uint8_t *opt_len)
> @@ -152,6 +195,9 @@ again:
> if (i + taglength >= size)
> return NULL;
>
> + grub_dprintf("net", "DHCP option %u (0x%02x) found with length %u.\n",
> + tagtype, tagtype, taglength);
> +
> /* FIXME RFC 3396 options concatentation */
> if (tagtype == opt_code)
> {
> @@ -354,6 +400,37 @@ grub_net_configure_by_dhcp_ack (const char *name,
> }
> grub_net_add_ipv4_local (inter, mask);
>
> + opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_ID, &opt_len);
> + if (opt)
> + {
> + set_env_limn_ro (name, "clientid", (char *) opt, opt_len);
> + }
> +
> + opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_CLIENT_UUID, &opt_len);
> + if (opt && opt_len == 17)
> + {
> + /* The format is 9cfe245e-d0c8-bd45-a79f-54ea5fbd3d97 */
> +
> + opt += 1;
> + opt_len -= 1;
> +
> + char *val = grub_malloc (2 * opt_len + 4 + 1);
> + int i = 0;
> + int j = 0;
> + for (i = 0; i < opt_len; i++)
> + {
> + val[2 * i + j] = hexdigit (opt[i] >> 4);
> + val[2 * i + 1 + j] = hexdigit (opt[i] & 0xf);
> +
> + if ((i == 3) || (i == 5) || (i == 7) || (i == 9))
> + {
> + j++;
> + val[2 * i + 1+ j] = '-';
> + }
> + }
> + set_env_limn_ro (name, "clientuuid", (char *) val, 2 * opt_len + 4);
> + }
> +
> /* We do not implement dead gateway detection and the first entry SHOULD
> be preferred one */
> opt = find_dhcp_option (bp, size, GRUB_NET_BOOTP_ROUTER, &opt_len);
> @@ -631,14 +708,6 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
> }
> }
>
> -static char
> -hexdigit (grub_uint8_t val)
> -{
> - if (val < 10)
> - return val + '0';
> - return val + 'a' - 10;
> -}
> -
> static grub_err_t
> grub_cmd_dhcpopt (struct grub_command *cmd __attribute__ ((unused)),
> int argc, char **args)
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 4a9069a1474..556c54e579f 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -462,6 +462,8 @@ enum
> GRUB_NET_BOOTP_DOMAIN = 0x0f,
> GRUB_NET_BOOTP_ROOT_PATH = 0x11,
> GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12,
> + GRUB_NET_BOOTP_CLIENT_ID = 0x3d,
> + GRUB_NET_BOOTP_CLIENT_UUID = 0x61,
> GRUB_NET_DHCP_REQUESTED_IP_ADDRESS = 50,
> GRUB_NET_DHCP_OVERLOAD = 52,
> GRUB_NET_DHCP_MESSAGE_TYPE = 53,
> --
> 2.21.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] Add %X option to printf functions
2019-10-04 22:44 ` [PATCH 2/3] Add %X option to printf functions Javier Martinez Canillas
2019-10-05 1:47 ` Steve McIntyre
@ 2019-10-15 14:24 ` Daniel Kiper
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Kiper @ 2019-10-15 14:24 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
On Sat, Oct 05, 2019 at 12:44:26AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>
> The printf(3) function has support for the %X format specifier, to output
> an unsigned hexadecimal integer in uppercase.
>
> This can be achived in GRUB using the %x format specifier in grub_printf()
> and calling grub_toupper(), but it is more convenient if there is support
> for %X in grub_printf().
>
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
But I would like to see this as a first patch...
Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] Add %X option to printf functions
2019-10-05 1:47 ` Steve McIntyre
2019-10-05 7:15 ` Javier Martinez Canillas
@ 2019-10-15 14:26 ` Daniel Kiper
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Kiper @ 2019-10-15 14:26 UTC (permalink / raw)
To: Steve McIntyre
Cc: The development of GNU GRUB, Vladimir Serbinenko, Daniel Kiper,
Paulo Flabiano Smorigo, Javier Martinez Canillas
On Sat, Oct 05, 2019 at 02:47:23AM +0100, Steve McIntyre wrote:
> On Sat, Oct 05, 2019 at 12:44:26AM +0200, Javier Martinez Canillas wrote:
> >From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> >
> >The printf(3) function has support for the %X format specifier, to output
> >an unsigned hexadecimal integer in uppercase.
> >
> >This can be achived in GRUB using the %x format specifier in grub_printf()
> >and calling grub_toupper(), but it is more convenient if there is support
> >for %X in grub_printf().
> >
> >Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> >Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> >---
> >
> > grub-core/kern/misc.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> >diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> >index 3b633d51f4c..76e7fb22872 100644
> >--- a/grub-core/kern/misc.c
> >+++ b/grub-core/kern/misc.c
> >@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d, grub_uint64_t *r)
> > static inline char *
> > grub_lltoa (char *str, int c, unsigned long long n)
> > {
> >- unsigned base = (c == 'x') ? 16 : 10;
> >+ unsigned base = ((c == 'x') || (c == 'X')) ? 16 : 10;
> > char *p;
> >
> > if ((long long) n < 0 && c == 'd')
> >@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
> > do
> > {
> > unsigned d = (unsigned) (n & 0xf);
> >- *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
> >+ *p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';
> > }
> > while (n >>= 4);
> > else
> >@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
> > {
> > case 'p':
> > case 'x':
> >+ case 'X':
> > case 'u':
> > case 'd':
> > case 'c':
> >@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args *args,
> > switch (c)
> > {
> > case 'x':
> >+ case 'X':
> > case 'u':
> > args->ptr[curn].type = UNSIGNED_INT + longfmt;
> > break;
> >@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const char *fmt0,
> > c = 'x';
> > /* Fall through. */
> > case 'x':
> >+ case 'X':
> > case 'u':
> > case 'd':
> > {
>
> Ummm, isn't this exactly the same patch that Colin Watson proposed and
> I reviewed back in March, as part of his changeset for UEFI?
>
> https://lists.gnu.org/archive/html/grub-devel/2019-03/msg00121.html
>
> Oh, that never got applied. :-(
Well, IIRC because the whole patch series was not taken... :-(((
I hope that this time we will take it.
Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] Search for specific config files for netboot
2019-10-04 22:44 ` [PATCH 3/3] Search for specific config files for netboot Javier Martinez Canillas
@ 2019-10-15 14:35 ` Daniel Kiper
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Kiper @ 2019-10-15 14:35 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
On Sat, Oct 05, 2019 at 12:44:27AM +0200, Javier Martinez Canillas wrote:
> From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>
> This patch implements a search for a specific configuration when the config
> file is on a remoteserver. It uses the following order:
> 1) DHCP client UUID option.
> 2) MAC address (in lower case hexadecimal with dash separators);
> 3) IP (in upper case hexadecimal) or IPv6;
> 4) The original grub.cfg file.
>
> This procedure is similar to what is used by pxelinux and yaboot:
> http://www.syslinux.org/wiki/index.php/PXELINUX#config
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406
>
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
However, one nitpick below...
> ---
>
> grub-core/net/net.c | 117 ++++++++++++++++++++++++++++++++++++++++
> grub-core/normal/main.c | 17 ++++--
> include/grub/net.h | 3 ++
> 3 files changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index d5d726a315e..6492bb74274 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1735,6 +1735,123 @@ grub_net_restore_hw (void)
> return GRUB_ERR_NONE;
> }
>
> +grub_err_t
> +grub_net_search_configfile (char *config)
s/grub_net_search_configfile/grub_net_search_config_file/
Daniel
> +{
> + grub_size_t config_len;
> + char *suffix;
> +
> + auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
> + int search_through (grub_size_t num_tries, grub_size_t slice_size)
> + {
> + while (num_tries-- > 0)
> + {
> + grub_dprintf ("net", "attempt to fetch config %s\n", config);
> +
> + grub_file_t file;
> + file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
> +
> + if (file)
> + {
> + grub_file_close (file);
> + return 0;
> + }
> + else
> + {
> + if (grub_errno == GRUB_ERR_IO)
> + grub_errno = GRUB_ERR_NONE;
> + }
> +
> + if (grub_strlen (suffix) < slice_size)
> + break;
> +
> + config[grub_strlen (config) - slice_size] = '\0';
> + }
> +
> + return 1;
> + }
> +
> + config_len = grub_strlen (config);
> + config[config_len] = '-';
> + suffix = config + config_len + 1;
> +
> + struct grub_net_network_level_interface *inf;
> + FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
> + {
> + /* By the Client UUID. */
> +
> + char client_uuid_var[sizeof ("net_") + grub_strlen (inf->name) +
> + sizeof ("_clientuuid") + 1];
> + grub_snprintf (client_uuid_var, sizeof (client_uuid_var),
> + "net_%s_clientuuid", inf->name);
> +
> + const char *client_uuid;
> + client_uuid = grub_env_get (client_uuid_var);
> +
> + if (client_uuid)
> + {
> + grub_strcpy (suffix, client_uuid);
> + if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
> + }
> +
> + /* By the MAC address. */
> +
> + /* Add ethernet type */
> + grub_strcpy (suffix, "01-");
> +
> + grub_net_hwaddr_to_str (&inf->hwaddress, suffix + 3);
> +
> + char *ptr;
> + for (ptr = suffix; *ptr; ptr++)
> + if (*ptr == ':')
> + *ptr = '-';
> +
> + if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
> +
> + /* By IP address */
> +
> + switch ((&inf->address)->type)
> + {
> + case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4:
> + {
> + grub_uint32_t n = grub_be_to_cpu32 ((&inf->address)->ipv4);
> + grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%02X%02X%02X%02X", \
> + ((n >> 24) & 0xff), ((n >> 16) & 0xff), \
> + ((n >> 8) & 0xff), ((n >> 0) & 0xff));
> +
> + if (search_through (8, 1) == 0) return GRUB_ERR_NONE;
> + break;
> + }
> + case GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6:
> + {
> + char buf[GRUB_NET_MAX_STR_ADDR_LEN];
> + struct grub_net_network_level_address base;
> + base.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> + grub_memcpy (&base.ipv6, ((&inf->address)->ipv6), 16);
> + grub_net_addr_to_str (&base, buf);
> +
> + for (ptr = buf; *ptr; ptr++)
> + if (*ptr == ':')
> + *ptr = '-';
> +
> + grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
> + if (search_through (1, 0) == 0) return GRUB_ERR_NONE;
> + break;
> + }
> + case GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV:
> + return grub_error (GRUB_ERR_BUG, "shouldn't reach here");
> + default:
> + return grub_error (GRUB_ERR_BUG,
> + "unsupported address type %d", (&inf->address)->type);
> + }
> + }
> +
> + /* Remove the remaining minus sign at the end. */
> + config[config_len] = '\0';
> +
> + return GRUB_ERR_NONE;
> +}
> +
> static struct grub_preboot *fini_hnd;
>
> static grub_command_t cmd_addaddr, cmd_deladdr, cmd_addroute, cmd_delroute;
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
> index 1b03dfd57b9..62ad546f4f7 100644
> --- a/grub-core/normal/main.c
> +++ b/grub-core/normal/main.c
> @@ -323,10 +323,19 @@ grub_cmd_normal (struct grub_command *cmd __attribute__ ((unused)),
>
> prefix = grub_env_get ("prefix");
> if (prefix)
> - {
> - config = grub_xasprintf ("%s/grub.cfg", prefix);
> - if (! config)
> - goto quit;
> + {
> + grub_size_t config_len;
> + config_len = grub_strlen (prefix) +
> + sizeof ("/grub.cfg-XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX");
> + config = grub_malloc (config_len);
> +
> + if (! config)
> + goto quit;
> +
> + grub_snprintf (config, config_len, "%s/grub.cfg", prefix);
> +
> + if (grub_strncmp (prefix + 1, "tftp", sizeof ("tftp") - 1) == 0)
> + grub_net_search_configfile (config);
>
> grub_enter_normal_mode (config);
> grub_free (config);
> diff --git a/include/grub/net.h b/include/grub/net.h
> index 556c54e579f..ff6d347f7da 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -578,4 +578,7 @@ extern char *grub_net_default_server;
>
> #define VLANTAG_IDENTIFIER 0x8100
>
> +grub_err_t
> +grub_net_search_configfile (char *config);
> +
> #endif /* ! GRUB_NET_HEADER */
> --
> 2.21.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Search for specific config files using UUID, MAC and IP
2019-10-04 22:44 [PATCH 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
` (2 preceding siblings ...)
2019-10-04 22:44 ` [PATCH 3/3] Search for specific config files for netboot Javier Martinez Canillas
@ 2019-10-15 14:37 ` Daniel Kiper
2019-10-18 8:00 ` Javier Martinez Canillas
3 siblings, 1 reply; 13+ messages in thread
From: Daniel Kiper @ 2019-10-15 14:37 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
On Sat, Oct 05, 2019 at 12:44:24AM +0200, Javier Martinez Canillas wrote:
> Hello,
>
> This series contains some patches that have been carried in Fedora (and
> other distros) for a long time. It allows to fetch GRUB configurations
> that are customized for each machine by using the client UUID, MAC and
> IP addresses as suffixes for the grub.cfg file.
>
> This procedure is similar to what is used by pxelinux and yaboot:
> http://www.syslinux.org/wiki/index.php/PXELINUX#config
>
> Patch #1 sets net_<interface>_client{id,uuid} environment variables by
> using the DHCP client ID and UUID options if these are found.
>
> Patch #2 adds a %X format specifier to make the grub_printf() function
> more similar to printf() and not require to use %x plus grub_toupper().
>
> Finally Patch #3 adds the support to fetch the config files using the
> UUID, MAC and IP as suffixes. If these aren't found, the default GRUB
> config filename is used as a fallback.
Except some nitpicks LGTM. You can repost the patch series or I can
fix these minor things before applying.
Daniel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options
2019-10-15 14:19 ` Daniel Kiper
@ 2019-10-18 7:46 ` Javier Martinez Canillas
0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18 7:46 UTC (permalink / raw)
To: Daniel Kiper
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
Hello Daniel,
On 10/15/19 4:19 PM, Daniel Kiper wrote:
> On Sat, Oct 05, 2019 at 12:44:25AM +0200, Javier Martinez Canillas wrote:
>> From: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>>
>> This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
>> GRUB environment variables, using the DHCP client ID and UUID options if
>> these are found.
>>
>> In the same way than net_<interface>_<option> variables are set for other
>> options such domain name, boot file, next server, etc.
>>
>> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> In general "Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>"
> but a few nit picks below...
>
Thanks for the review.
>> ---
>>
>> grub-core/net/bootp.c | 85 +++++++++++++++++++++++++++++++++++++++----
>> include/grub/net.h | 2 +
>> 2 files changed, 79 insertions(+), 8 deletions(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index 04cfbb04504..0e6e41a1699 100644
>> --- a/grub-core/net/bootp.c
>> +++ b/grub-core/net/bootp.c
>> @@ -95,6 +95,49 @@ enum
>> /* Max timeout when waiting for BOOTP/DHCP reply */
>> #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>>
>> +static char *
>> +grub_env_write_readonly (struct grub_env_var *var __attribute__ ((unused)),
>> + const char *val __attribute__ ((unused)))
>
> s/grub_env_write_readonly/grub_env_ro_write/?
>
I noticed that this function already exists in grub-core/net/net.c, take a
look to commit e4dbf247b65 ("add grub_env_set_net_property function").
So I'll just remove this and the cryptic set_env_limn_ro() function.
>> +{
>> + return NULL;
>> +}
>> +
>> +static void
>> +set_env_limn_ro (const char *intername, const char *suffix,
>> + const char *value, grub_size_t len)
>
> I have hard time reading this. What does "limn" stand for?
> Could we make the name of this function less cryptic?
>
Yes, same. But this function also already exists in grub-core/net/net.c as
mentioned. It's called grub_env_set_net_property() and I'll use that.
Sorry for missing that this patch could be much more simpler after a rebase
to the latest GRUB version.
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Search for specific config files using UUID, MAC and IP
2019-10-15 14:37 ` [PATCH 0/3] Search for specific config files using UUID, MAC and IP Daniel Kiper
@ 2019-10-18 8:00 ` Javier Martinez Canillas
0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2019-10-18 8:00 UTC (permalink / raw)
To: Daniel Kiper
Cc: grub-devel, Vladimir Serbinenko, Daniel Kiper, Paulo Flabiano Smorigo
Hello Daniel,
On 10/15/19 4:37 PM, Daniel Kiper wrote:
> On Sat, Oct 05, 2019 at 12:44:24AM +0200, Javier Martinez Canillas wrote:
>> Hello,
>>
>> This series contains some patches that have been carried in Fedora (and
>> other distros) for a long time. It allows to fetch GRUB configurations
>> that are customized for each machine by using the client UUID, MAC and
>> IP addresses as suffixes for the grub.cfg file.
>>
>> This procedure is similar to what is used by pxelinux and yaboot:
>> http://www.syslinux.org/wiki/index.php/PXELINUX#config
>>
>> Patch #1 sets net_<interface>_client{id,uuid} environment variables by
>> using the DHCP client ID and UUID options if these are found.
>>
>> Patch #2 adds a %X format specifier to make the grub_printf() function
>> more similar to printf() and not require to use %x plus grub_toupper().
>>
>> Finally Patch #3 adds the support to fetch the config files using the
>> UUID, MAC and IP as suffixes. If these aren't found, the default GRUB
>> config filename is used as a fallback.
>
> Except some nitpicks LGTM. You can repost the patch series or I can
> fix these minor things before applying.
>
Thanks for the feedback. I've posted a v2 that addresses all the issues
you pointed out. Please let me know if anything else is missing.
> Daniel
>
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-18 8:00 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 22:44 [PATCH 0/3] Search for specific config files using UUID, MAC and IP Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 1/3] Set net_<interface>_client{id, uuid} variables from DHCP options Javier Martinez Canillas
2019-10-15 14:19 ` Daniel Kiper
2019-10-18 7:46 ` Javier Martinez Canillas
2019-10-04 22:44 ` [PATCH 2/3] Add %X option to printf functions Javier Martinez Canillas
2019-10-05 1:47 ` Steve McIntyre
2019-10-05 7:15 ` Javier Martinez Canillas
2019-10-15 14:26 ` Daniel Kiper
2019-10-15 14:24 ` Daniel Kiper
2019-10-04 22:44 ` [PATCH 3/3] Search for specific config files for netboot Javier Martinez Canillas
2019-10-15 14:35 ` Daniel Kiper
2019-10-15 14:37 ` [PATCH 0/3] Search for specific config files using UUID, MAC and IP Daniel Kiper
2019-10-18 8:00 ` Javier Martinez Canillas
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.