Re: [PATCH v3 3/6] ieee1275: implement FCP methods for WWPN and LUNs
On Thu, Jun 06, 2024 at 06:07:24PM GMT, Avnish Chouhan wrote:
> This patch enables the fcp-targets and fcp-luns methods which are
> responsible to get WWPNs and LUNs for fibre channel devices.
>
> Those methods are specially necessary if the boot directory and grub
> installation are in different FCP disks, allowing the dev_iterate()
> to find the WWPNs and LUNs when called by searchfs.uuid tool.
>
> Signed-off-by: Diego Domingos
> Signed-off-by: Avnish Chouhan
> ---
> grub-core/disk/ieee1275/ofdisk.c | 111
> ++-
> 1 file changed, 110 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c
> b/grub-core/disk/ieee1275/ofdisk.c
> index 5534684..5958e5e 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -209,7 +209,116 @@ dev_iterate_real (const char *name, const char *path)
> static void
> dev_iterate (const struct grub_ieee1275_devalias *alias)
> {
> - if (grub_strcmp (alias->type, "vscsi") == 0)
> + if (grub_strcmp (alias->type, "fcp") == 0)
> +{
> + /*
> + * If we are dealing with fcp devices, we need
> + * to find the WWPNs and LUNs to iterate them
> + */
> + grub_ieee1275_ihandle_t ihandle;
> + grub_uint64_t *ptr_targets, *ptr_luns, k, l;
> + unsigned int i, j, pos;
> + char *buf, *bufptr;
> + struct set_fcp_targets_args
> + {
> +struct grub_ieee1275_common_hdr common;
> +grub_ieee1275_cell_t method;
> +grub_ieee1275_cell_t ihandle;
> +grub_ieee1275_cell_t catch_result;
> +grub_ieee1275_cell_t nentries;
> +grub_ieee1275_cell_t table;
> + } args_targets;
> +
> + struct set_fcp_luns_args
> + {
> +struct grub_ieee1275_common_hdr common;
> +grub_ieee1275_cell_t method;
> +grub_ieee1275_cell_t ihandle;
> +grub_ieee1275_cell_t wwpn_h;
> +grub_ieee1275_cell_t wwpn_l;
> +grub_ieee1275_cell_t catch_result;
> +grub_ieee1275_cell_t nentries;
> +grub_ieee1275_cell_t table;
> + } args_luns;
> +
> + struct args_ret
> + {
> +grub_uint64_t addr;
> +grub_uint64_t len;
> + };
> +
> + if (grub_ieee1275_open (alias->path, &ihandle))
> +{
> + grub_dprintf ("disk", "failed to open the disk while iterating FCP
> disk path=%s\n", alias->path);
> + return;
> +}
> +
> + /* Setup the fcp-targets method to call via pfw*/
> + INIT_IEEE1275_COMMON (&args_targets.common, "call-method", 2, 3);
> + args_targets.method = (grub_ieee1275_cell_t) "fcp-targets";
> + args_targets.ihandle = ihandle;
> +
> + /* Setup the fcp-luns method to call via pfw */
> + INIT_IEEE1275_COMMON (&args_luns.common, "call-method", 4, 3);
> + args_luns.method = (grub_ieee1275_cell_t) "fcp-luns";
> + args_luns.ihandle = ihandle;
> + if (IEEE1275_CALL_ENTRY_FN (&args_targets) == -1)
> +{
> + grub_dprintf ("disk", "failed to get the targets while iterating
> FCP disk path=%s\n", alias->path);
> + grub_ieee1275_close (ihandle);
> + return;
> +}
> + buf = grub_malloc (grub_strlen (alias->path) + 32 + 32);
> + if (!buf)
> +{
> + grub_ieee1275_close (ihandle);
> + return;
> +}
> + bufptr = grub_stpcpy (buf, alias->path);
> +
> + /*
> + * Iterate over entries returned by pfw. Each entry contains a
> + * pointer to wwpn table and his length.
> + */
> + struct args_ret *targets_table = (struct args_ret *)
> (args_targets.table);
> + for (i = 0; i < args_targets.nentries; i++)
> +{
> + ptr_targets = (grub_uint64_t*) (grub_uint32_t)
> targets_table[i].addr;
I believe a comment is needed here to explain why it is necessary to
truncate the 64-bit address into 32-bit. It appears that losing the
higher 32 bits is intentional.
> + /* Iterate over all wwpns in given table */
> + for(k = 0; k < targets_table[i].len; k++)
> +{
> + args_luns.wwpn_l = (grub_ieee1275_cell_t) (*ptr_targets);
> + args_luns.wwpn_h = (grub_ieee1275_cell_t) (*ptr_targets >> 32);
> + pos = grub_snprintf (bufptr, 32, "/disk@%" PRIxGRUB_UINT64_T,
> + *ptr_targets++);
As long as ptr_targets holds a pointer external to grub, the alignment
is neither validated nor guaranteed. IMHO it is safer to use
grub_get_unaligned64(ptr_targets++).
> + /* Get the luns for given wwpn target */
> + if (IEEE1275_CALL_ENTRY_FN (&args_luns) == -1)
> +{
> + grub_dprintf ("disk", "failed to get the LUNS while
> iterating FCP disk path=%s\n", buf);
> + grub_ieee1275_close (ihandle);
> + grub_free (buf);
> + return;
> +}
> +
Re: [PATCH v3 3/6] ieee1275: implement FCP methods for WWPN and LUNs
On 2024-07-01 13:04, Michael Chang wrote:
On Thu, Jun 06, 2024 at 06:07:24PM GMT, Avnish Chouhan wrote:
This patch enables the fcp-targets and fcp-luns methods which are
responsible to get WWPNs and LUNs for fibre channel devices.
Those methods are specially necessary if the boot directory and grub
installation are in different FCP disks, allowing the dev_iterate()
to find the WWPNs and LUNs when called by searchfs.uuid tool.
Signed-off-by: Diego Domingos
Signed-off-by: Avnish Chouhan
---
grub-core/disk/ieee1275/ofdisk.c | 111
++-
1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/grub-core/disk/ieee1275/ofdisk.c
b/grub-core/disk/ieee1275/ofdisk.c
index 5534684..5958e5e 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c
@@ -209,7 +209,116 @@ dev_iterate_real (const char *name, const char
*path)
static void
dev_iterate (const struct grub_ieee1275_devalias *alias)
{
- if (grub_strcmp (alias->type, "vscsi") == 0)
+ if (grub_strcmp (alias->type, "fcp") == 0)
+{
+ /*
+ * If we are dealing with fcp devices, we need
+ * to find the WWPNs and LUNs to iterate them
+ */
+ grub_ieee1275_ihandle_t ihandle;
+ grub_uint64_t *ptr_targets, *ptr_luns, k, l;
+ unsigned int i, j, pos;
+ char *buf, *bufptr;
+ struct set_fcp_targets_args
+ {
+struct grub_ieee1275_common_hdr common;
+grub_ieee1275_cell_t method;
+grub_ieee1275_cell_t ihandle;
+grub_ieee1275_cell_t catch_result;
+grub_ieee1275_cell_t nentries;
+grub_ieee1275_cell_t table;
+ } args_targets;
+
+ struct set_fcp_luns_args
+ {
+struct grub_ieee1275_common_hdr common;
+grub_ieee1275_cell_t method;
+grub_ieee1275_cell_t ihandle;
+grub_ieee1275_cell_t wwpn_h;
+grub_ieee1275_cell_t wwpn_l;
+grub_ieee1275_cell_t catch_result;
+grub_ieee1275_cell_t nentries;
+grub_ieee1275_cell_t table;
+ } args_luns;
+
+ struct args_ret
+ {
+grub_uint64_t addr;
+grub_uint64_t len;
+ };
+
+ if (grub_ieee1275_open (alias->path, &ihandle))
+{
+ grub_dprintf ("disk", "failed to open the disk while
iterating FCP disk path=%s\n", alias->path);
+ return;
+}
+
+ /* Setup the fcp-targets method to call via pfw*/
+ INIT_IEEE1275_COMMON (&args_targets.common, "call-method", 2,
3);
+ args_targets.method = (grub_ieee1275_cell_t) "fcp-targets";
+ args_targets.ihandle = ihandle;
+
+ /* Setup the fcp-luns method to call via pfw */
+ INIT_IEEE1275_COMMON (&args_luns.common, "call-method", 4, 3);
+ args_luns.method = (grub_ieee1275_cell_t) "fcp-luns";
+ args_luns.ihandle = ihandle;
+ if (IEEE1275_CALL_ENTRY_FN (&args_targets) == -1)
+{
+ grub_dprintf ("disk", "failed to get the targets while
iterating FCP disk path=%s\n", alias->path);
+ grub_ieee1275_close (ihandle);
+ return;
+}
+ buf = grub_malloc (grub_strlen (alias->path) + 32 + 32);
+ if (!buf)
+{
+ grub_ieee1275_close (ihandle);
+ return;
+}
+ bufptr = grub_stpcpy (buf, alias->path);
+
+ /*
+ * Iterate over entries returned by pfw. Each entry contains a
+ * pointer to wwpn table and his length.
+ */
+ struct args_ret *targets_table = (struct args_ret *)
(args_targets.table);
+ for (i = 0; i < args_targets.nentries; i++)
+{
+ ptr_targets = (grub_uint64_t*) (grub_uint32_t)
targets_table[i].addr;
I believe a comment is needed here to explain why it is necessary to
truncate the 64-bit address into 32-bit. It appears that losing the
higher 32 bits is intentional.
Hi Michael,
Thank you so much for your reviews!
Sure, we'll add a comment here.
+ /* Iterate over all wwpns in given table */
+ for(k = 0; k < targets_table[i].len; k++)
+{
+ args_luns.wwpn_l = (grub_ieee1275_cell_t)
(*ptr_targets);
+ args_luns.wwpn_h = (grub_ieee1275_cell_t) (*ptr_targets
>> 32);
+ pos = grub_snprintf (bufptr, 32, "/disk@%"
PRIxGRUB_UINT64_T,
+ *ptr_targets++);
As long as ptr_targets holds a pointer external to grub, the alignment
is neither validated nor guaranteed. IMHO it is safer to use
grub_get_unaligned64(ptr_targets++).
Sure, we can change this as suggested.
Regards,
Avnish Chouhan
+ /* Get the luns for given wwpn target */
+ if (IEEE1275_CALL_ENTRY_FN (&args_luns) == -1)
+{
+ grub_dprintf ("disk", "failed to get the LUNS while
iterating FCP disk path=%s\n", buf);
+ grub_ieee1275_close (ihandle);
+ grub_free (buf);
+ return;
+}
+ struct args_ret *luns
How to test grub_iso9660_uuid from userland ?
Hi, i found a wrong word in an error message of grub-core/fs/iso9660.c function grub_iso9660_uuid(): grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem to generate UUID"); The missing entity would be the modification date data->voldesc.modified rather than the creation date data->voldesc.created The fix is obviously simple. But i have no idea how to get the code execution to this function for testing. An ISO with 0 date would be easy to fake. A use case known to me is in GRUB configuration with command "search" and option "--fs-uuid". In grub-fstest.c i see no opportunity to perform "search". Any idea how to test grub_iso9660_uuid() from userland of a running GNU/Linux ? Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: How to test grub_iso9660_uuid from userland ?
Le lun. 1 juil. 2024, 17:12, Thomas Schmitt via Grub-devel < grub-devel@gnu.org> a écrit : > Hi, > > i found a wrong word in an error message of grub-core/fs/iso9660.c > function grub_iso9660_uuid(): > > grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem > to generate UUID"); > > The missing entity would be the modification date > data->voldesc.modified > rather than the creation date > data->voldesc.created > > The fix is obviously simple. But i have no idea how to get the code > execution to this function for testing. > An ISO with 0 date would be easy to fake. A use case known to me is in > GRUB configuration with command "search" and option "--fs-uuid". > In grub-fstest.c i see no opportunity to perform "search". > > Any idea how to test grub_iso9660_uuid() from userland of a running > GNU/Linux ? > grub-fstest IMAGE ls -- -l > > > Have a nice day :) > > Thomas > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: How to test grub_iso9660_uuid from userland ?
Hi, the question is now: Is it worth to correct an error text which i cannot get to show up ? Vladimir 'phcoder' Serbinenko wrote: > grub-fstest IMAGE ls -- -l $ gunzip /tmp/iso9660_early_ce.iso $ ./grub-fstest /tmp/iso9660_early_ce.iso ls -- -l ... Device loop0: Filesystem type iso9660 - Label `ISOIMAGE' - Last modification time 2023-03-04 16:28:35 Saturday, UUID 2023-03-04-16-28-35-00 - Sector size 512B - Total size 68KiB ... Now zeroizing the modification date (offset 830 in PVD at offset 32768): $ dd if=/dev/zero of=/tmp/iso9660_early_ce.iso conv=notrunc bs=1 count=17 seek=33598 $ ./grub-fstest /tmp/iso9660_early_ce.iso ls -- -l ... Device loop0: Filesystem type iso9660 - Label `ISOIMAGE' - Sector size 512B - Total size 68KiB ... $ Well, it did not crash and there is no UUID. But i don't see the error message either. Switching to the branch with my proposed error text improvements of grub-fstest ... Still nothing to see. Obviously grub_errno is not set after function execute_command of grub-fstest.c. I checked that a bad ISO still causes a visible error message $ /grub-fstest /tmp/iso9660_ce_loop2.iso ls / -- -l ls : GRUB error 9 with message 'suspecting endless CE loop' ./grub-fstest: error: encountered 1 error during command execution. $ I also zeroized creation date at offset 32768+813 of the original ISO and verified that grub-fstest still shows an UUID. So the only thing i could prove by my test run is that indeed GRUB depends on ISO 9660 modification time for its UUID production. I failed to hack the function grub_error in grub-core/kern/err.c so that it shows grub_errmsg to me. Neither fprintf(3) nor write(2) seem in reach. The attempt to #include /usr/include/stdio.h or /usr/include/unistd.h fails with not found sub-header files. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] fs/iso9660.c: Correct error message for missing UUID
The UUID emitted by function grub_iso9660_uuid is derived from the
ISO 9660 Primary Volume Descriptor field "Volume Modification Date and
Time". But the error message about possible invalid content of this
field talks of "creation date" rather than of "modification date".
Signed-off-by: Thomas Schmitt
---
grub-core/fs/iso9660.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
index 8c348b59a..a40c0fc0e 100644
--- a/grub-core/fs/iso9660.c
+++ b/grub-core/fs/iso9660.c
@@ -1166,7 +1166,7 @@ grub_iso9660_uuid (grub_device_t device, char **uuid)
&& ! data->voldesc.modified.second[0] && !
data->voldesc.modified.second[1]
&& ! data->voldesc.modified.hundredth[0] && !
data->voldesc.modified.hundredth[1])
{
- grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem to
generate UUID");
+ grub_error (GRUB_ERR_BAD_NUMBER, "no modification date in filesystem
to generate UUID");
*uuid = NULL;
}
else
--
2.39.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] fs/iso9660.c: Correct error message for missing UUID
The UUID emitted by function grub_iso9660_uuid is derived from the
ISO 9660 Primary Volume Descriptor field "Volume Modification Date and
Time". But the error message about possible invalid content of this
field talks of "creation date" rather than of "modification date".
Signed-off-by: Thomas Schmitt
---
grub-core/fs/iso9660.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
index 8c348b59a..a40c0fc0e 100644
--- a/grub-core/fs/iso9660.c
+++ b/grub-core/fs/iso9660.c
@@ -1166,7 +1166,7 @@ grub_iso9660_uuid (grub_device_t device, char **uuid)
&& ! data->voldesc.modified.second[0] && !
data->voldesc.modified.second[1]
&& ! data->voldesc.modified.hundredth[0] && !
data->voldesc.modified.hundredth[1])
{
- grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem to
generate UUID");
+ grub_error (GRUB_ERR_BAD_NUMBER, "no modification date in filesystem
to generate UUID");
*uuid = NULL;
}
else
--
2.39.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] fs/iso9660.c: Correct error message for missing UUID
Hi, sorry for goofing up "git send-email" again. This time not by omitting the empty line after the subject of a cover letter but probably by leaving the commit id of the previous "git format-patch" in the "git send-email" command line. git send-email --to=grub-devel@gnu.org $directory $surplus_id Shall i post the patch again as single message ? Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
