H. Peter Anvin
2016-Jul-14 23:28 UTC
[syslinux] [PATCH] : Adding dlabel option to chain.c32
On 07/14/16 05:56, Ady Ady via Syslinux wrote:> > @Peter, Erwan, Gene, Michal, (and anyone else interested)... > > Although I haven't actually tested Erwan's patch, would it be > acceptable by you (all) if I were to send a patch to this Syslinux > Mailing List with the same code while changing the wording as I > previously suggested in a prior email? Would it be OK to use > "diskbypartitionlabel" as name of the option? At any rate, the credit > should go to Erwan; I am just trying to help so to actually make the > push for it. >You're right, in fact, I misinterpreted it exactly this way! I would suggest "diskwithpartlabel" as a reasonable contraction of "disk with partition label". And yes, let's push it out. -hpa
Looking at the UEFI specs, and _if_ I understand Erwan's patch correctly,
we are talking about searching for a "Partition Name". Avoiding the
"label" term for this new option should help in reducing some
potential
confusions (e.g. BSD's "disklabel", fdisk's "partition
label (type)",
filesystem's "volume label"...).
So, IMO, "diskbypartname" should be a reasonable contraction of
"disk
being selected by searching for a GPT Partition Name".
If it has not been clear enough in prior emails, this new
"diskbypartname" option could be applicable for users with a
(Protective) MBR in a GPT scheme booting in BIOS / CSM mode.
I am basing this patch on Syslinux 6.04-pre1. At the time of writing
this email, it is also equivalent to basing it on the current git
master head.
If the patch is accepted, it would be helpful for users to also see
'doc/chain.txt' being patched. Additional potential rewording in
'com32/chain/{chain.c,options.c} might be helpful too in future
separated patches.
I took Erwan's patch and slightly modified it with some rewording and
by changing "dlabel" to "diskbypartname". I hope I've
done it
correctly; please evaluate it with careful attention. I have not
compiled any binaries with this patch.
The following is the relevant text for the patch and the patching code
for 'com32/chain/chain.c' and for 'com32/chain/options.c.
The existing "label:" and "guid:" options select a specified
partition
and jump to it.
The new "diskbypartname" option searches in disks for a GPT partition
with the specified Partition Name, and jumps to the first _disk_ that
matches the search.
The specified partition might not be related to the boot process; its
Name is just used for the detection / selection of the disk.
A possible use case is for making a localboot like:
label localboot
com32 chain.c32
append diskbypartname=datapartition
This option allows booting a disk that contains at least one GPT
partition named "datapartition".
The "diskbypartname:" option can be considered as doing almost what
the
"mbr:" option does, but by searching for a specified Partition Name in
the GPT partitions.
diff U3 syslinux-6.04-pre1/com32/chain/chain.c
diskbypartname/com32/chain/chain.c
--- syslinux-6.04-pre1/com32/chain/chain.c Fri Jul 15 01:27:23 2016
+++ diskbypartname/com32/chain/chain.c Fri Jul 15 01:00:03 2016
@@ -143,6 +143,38 @@
return drive;
}
+/*
+ * Search for a disk having a GPT partition with a specified Partition Name.
+ * Return disk and iterator at proper position.
+ */
+static int find_disk_by_partition_name(const char *label, struct part_iter
**_boot_part)
+{
+ struct part_iter *iter = NULL;
+ struct disk_info diskinfo;
+ int drive;
+
+ for (drive = 0x80; drive < 0x80 + fixed_cnt; drive++) {
+ if (disk_get_params(drive, &diskinfo))
+ continue; /* Drive doesn't exist */
+ if (!(iter = pi_begin(&diskinfo, opt.piflags)))
+ continue;
+ /* Check for a matching GPT Partition Name */
+ if (iter->type == typegpt)
+ while (!pi_next(iter)) {
+ if (!strcmp(label, iter->gpt.part_label))
+ // We don't care about the actual partition that matched
+ pi_del(&iter);
+ // Let's return the disk itself instead
+ iter = pi_begin(&diskinfo, opt.piflags);
+ goto ok;
+ }
+ }
+ drive = -1;
+ok:
+ *_boot_part = iter;
+ return drive;
+}
+
static void do_boot(struct data_area *data, int ndata)
{
struct syslinux_memmap *mmap;
@@ -300,6 +332,15 @@
}
if (find_by_label(opt.drivename + 6, &iter) < 0) {
error("Unable to find requested GPT partition by label.");
+ goto bail;
+ }
+ } else if (!strncmp(opt.drivename, "diskbypartname", 14)) {
+ if (!opt.drivename[15]) {
+ error("No partition name specified.");
+ goto bail;
+ }
+ if (find_disk_by_partition_name(opt.drivename + 15, &iter) < 0) {
+ error("Unable to find requested GPT partition by name.");
goto bail;
}
} else if ((opt.drivename[0] == 'h' || opt.drivename[0] ==
'f') &&
diff U3 syslinux-6.04-pre1/com32/chain/options.c
diskbypartname/com32/chain/options.c
--- syslinux-6.04-pre1/com32/chain/options.c Fri Jul 15 01:28:29 2016
+++ diskbypartname/com32/chain/options.c Fri Jul 15 00:56:26 2016
@@ -107,6 +107,7 @@
" direct partition selection:",
" chain.c32 guid=<guid> [options]",
" chain.c32 label=<label> [options]",
+" chain.c32 diskbypartname=<partition_name> [options]",
" chain.c32 fs [options]",
"",
"You can use ':' instead of '=' and ' ' instead of
','.",
@@ -396,6 +397,8 @@
|| !strncmp(argv[i], "guid=", 5)
|| !strncmp(argv[i], "label:", 6)
|| !strncmp(argv[i], "label=", 6)
+ || !strncmp(argv[i], "diskbypartname:", 15)
+ || !strncmp(argv[i], "diskbypartname=", 15)
|| !strcmp(argv[i], "boot")
|| !strncmp(argv[i], "boot,", 5)
|| !strcmp(argv[i], "fs")) {
--
On 15.07.2016 03:50, Ady Ady via Syslinux wrote:> Looking at the UEFI specs, and _if_ I understand Erwan's patch correctly, > we are talking about searching for a "Partition Name". Avoiding the > "label" term for this new option should help in reducing some potential > confusions (e.g. BSD's "disklabel", fdisk's "partition label (type)", > filesystem's "volume label"...). > > So, IMO, "diskbypartname" should be a reasonable contraction of "disk > being selected by searching for a GPT Partition Name". > > If it has not been clear enough in prior emails, this new > "diskbypartname" option could be applicable for users with a > (Protective) MBR in a GPT scheme booting in BIOS / CSM mode. > > I am basing this patch on Syslinux 6.04-pre1. At the time of writing > this email, it is also equivalent to basing it on the current git > master head. > > If the patch is accepted, it would be helpful for users to also see > 'doc/chain.txt' being patched. Additional potential rewording in > 'com32/chain/{chain.c,options.c} might be helpful too in future > separated patches. > > I took Erwan's patch and slightly modified it with some rewording and > by changing "dlabel" to "diskbypartname". I hope I've done it > correctly; please evaluate it with careful attention. I have not > compiled any binaries with this patch. > > The following is the relevant text for the patch and the patching code > for 'com32/chain/chain.c' and for 'com32/chain/options.c. > > > > > The existing "label:" and "guid:" options select a specified partition > and jump to it. > > The new "diskbypartname" option searches in disks for a GPT partition > with the specified Partition Name, and jumps to the first _disk_ that > matches the search. > > The specified partition might not be related to the boot process; its > Name is just used for the detection / selection of the disk. > > A possible use case is for making a localboot like: > > label localboot > com32 chain.c32 > append diskbypartname=datapartition > > This option allows booting a disk that contains at least one GPT > partition named "datapartition". > > The "diskbypartname:" option can be considered as doing almost what the > "mbr:" option does, but by searching for a specified Partition Name in > the GPT partitions. > > > diff U3 syslinux-6.04-pre1/com32/chain/chain.c diskbypartname/com32/chain/chain.c > --- syslinux-6.04-pre1/com32/chain/chain.c Fri Jul 15 01:27:23 2016 > +++ diskbypartname/com32/chain/chain.c Fri Jul 15 01:00:03 2016 > @@ -143,6 +143,38 @@ > return drive; > } > > +/* > + * Search for a disk having a GPT partition with a specified Partition Name. > + * Return disk and iterator at proper position. > + */ > +static int find_disk_by_partition_name(const char *label, struct part_iter **_boot_part) > +{ > + struct part_iter *iter = NULL; > + struct disk_info diskinfo; > + int drive; > + > + for (drive = 0x80; drive < 0x80 + fixed_cnt; drive++) { > + if (disk_get_params(drive, &diskinfo)) > + continue; /* Drive doesn't exist */ > + if (!(iter = pi_begin(&diskinfo, opt.piflags))) > + continue; > + /* Check for a matching GPT Partition Name */ > + if (iter->type == typegpt) > + while (!pi_next(iter)) { > + if (!strcmp(label, iter->gpt.part_label)) > + // We don't care about the actual partition that matched > + pi_del(&iter); > + // Let's return the disk itself instead > + iter = pi_begin(&diskinfo, opt.piflags); > + goto ok; > + } > + } > + drive = -1; > +ok: > + *_boot_part = iter; > + return drive; > +} > + > static void do_boot(struct data_area *data, int ndata) > { > struct syslinux_memmap *mmap; > @@ -300,6 +332,15 @@ > } > if (find_by_label(opt.drivename + 6, &iter) < 0) { > error("Unable to find requested GPT partition by label."); > + goto bail; > + } > + } else if (!strncmp(opt.drivename, "diskbypartname", 14)) { > + if (!opt.drivename[15]) { > + error("No partition name specified."); > + goto bail; > + } > + if (find_disk_by_partition_name(opt.drivename + 15, &iter) < 0) { > + error("Unable to find requested GPT partition by name."); > goto bail; > } > } else if ((opt.drivename[0] == 'h' || opt.drivename[0] == 'f') && > > > > diff U3 syslinux-6.04-pre1/com32/chain/options.c diskbypartname/com32/chain/options.c > --- syslinux-6.04-pre1/com32/chain/options.c Fri Jul 15 01:28:29 2016 > +++ diskbypartname/com32/chain/options.c Fri Jul 15 00:56:26 2016 > @@ -107,6 +107,7 @@ > " direct partition selection:", > " chain.c32 guid=<guid> [options]", > " chain.c32 label=<label> [options]", > +" chain.c32 diskbypartname=<partition_name> [options]", > " chain.c32 fs [options]", > "", > "You can use ':' instead of '=' and ' ' instead of ','.", > @@ -396,6 +397,8 @@ > || !strncmp(argv[i], "guid=", 5) > || !strncmp(argv[i], "label:", 6) > || !strncmp(argv[i], "label=", 6) > + || !strncmp(argv[i], "diskbypartname:", 15) > + || !strncmp(argv[i], "diskbypartname=", 15) > || !strcmp(argv[i], "boot") > || !strncmp(argv[i], "boot,", 5) > || !strcmp(argv[i], "fs")) { > > -- > >dlabel -> diskbypartname +1
Ady Ady
2016-Jul-28 17:18 UTC
[syslinux] [PATCH] : Add diskbypartname option to chain.c32 (v.2)
Based on:
http://www.syslinux.org/archives/2016-July/025329.html
I am resending my original email with the slightly-modified _updated_ code,
explanations and
rewording of the commit's text.
I have successfully tested this patch by replacing the two relevant files,
"chain.c" and
"options.c", over the respective ones as of Syslinux 6.03, then
rebuilding and testing the
resulting binaries in VMs. The reason for me not to test / build ATM by basing
on 6.04-pre1 nor on
current git master head is to avoid some building issues (which have been
discussed in bug 71 of
Syslinux's Bugzilla).
Once again, I'd like to clearly state that credit and thanks should go to
Erwan; I am just using
"diskbypartname" and rewording (for clarity, I hope) so to actually
push this out.
***
Looking at the UEFI specs, and _if_ I understand Erwan's patch correctly,
we are talking about searching for a "Partition Name". Avoiding the
"label" term for this new
option should help in reducing some potential
confusions (e.g. BSD's "disklabel", fdisk's "partition
label (type)", filesystem's "volume
label"...).
So, IMO, "diskbypartname" should be a reasonable contraction of
"disk being selected by searching
for a GPT Partition Name".
If it has not been clear enough in prior emails, this new
"diskbypartname" option could be
applicable for users with a (Protective) MBR in a GPT scheme booting in BIOS /
CSM mode.
I am basing this patch on Syslinux 6.04-pre1. At the time of writing this email,
it is also
equivalent to basing it on the current git master head.
If the patch is accepted, it would be helpful for users to also see
'doc/chain.txt' being patched.
Additional potential rewording in 'com32/chain/{chain.c,options.c} might be
helpful too in future
separated patches.
I took Erwan's patch and slightly modified it with some rewording and by
changing "dlabel" to
"diskbypartname". I hope I've done it correctly; please evaluate
it with careful attention. I have
not compiled any binaries with this patch (UPDATE: as of 2016Jul28, this patch
has been
successfully tested).
The following is the relevant text for the patch and the patching code for
'com32/chain/chain.c'
and for 'com32/chain/options.c.
***
The existing "label:" and "guid:" options select a specified
partition
and jump to it.
The new "diskbypartname" option searches in disks for a GPT partition
with the specified Partition Name, and jumps to the first _disk_ that
matches the search.
The specified partition might not be related to the boot process; its
Name is just used for the detection / selection of the disk.
A possible use case is for making a localboot like:
label localboot
com32 chain.c32
append diskbypartname=datapartition
This option allows booting a disk that contains at least one GPT
partition named "datapartition".
The "diskbypartname:" option can be considered as doing almost what
the
"mbr:" option does, but by searching for a specified Partition Name in
the GPT partitions.
diff U3 syslinux-6.04-pre1/com32/chain/chain.c
diskbypartname/com32/chain/chain.c
--- syslinux-6.04-pre1/com32/chain/chain.c Fri Jul 15 03:27:23 2016
+++ diskbypartname/com32/chain/chain.c Fri Jul 15 03:00:03 2016
@@ -143,6 +143,39 @@
return drive;
}
+/*
+ * Search for a disk having a GPT partition with a specified Partition Name.
+ * Return disk and iterator at proper position.
+ */
+static int find_disk_by_partition_name(const char *label, struct part_iter
**_boot_part)
+{
+ struct part_iter *iter = NULL;
+ struct disk_info diskinfo;
+ int drive;
+
+ for (drive = 0x80; drive < 0x80 + fixed_cnt; drive++) {
+ if (disk_get_params(drive, &diskinfo))
+ continue; /* Drive doesn't exist */
+ if (!(iter = pi_begin(&diskinfo, opt.piflags)))
+ continue;
+ /* Check for a matching GPT Partition Name */
+ if (iter->type == typegpt)
+ while (!pi_next(iter)) {
+ if (!strncmp(label, iter->gpt.part_label, strlen(label))) {
+ // We don't care about the actual partition that matched
+ pi_del(&iter);
+ // Let's return the disk itself instead
+ iter = pi_begin(&diskinfo, opt.piflags);
+ goto ok;
+ }
+ }
+ }
+ drive = -1;
+ok:
+ *_boot_part = iter;
+ return drive;
+}
+
static void do_boot(struct data_area *data, int ndata)
{
struct syslinux_memmap *mmap;
@@ -300,6 +333,15 @@
}
if (find_by_label(opt.drivename + 6, &iter) < 0) {
error("Unable to find requested GPT partition by label.");
+ goto bail;
+ }
+ } else if (!strncmp(opt.drivename, "diskbypartname", 14)) {
+ if (!opt.drivename[15]) {
+ error("No partition name specified.");
+ goto bail;
+ }
+ if (find_disk_by_partition_name(opt.drivename + 15, &iter) < 0) {
+ error("Unable to find requested GPT partition by name.");
goto bail;
}
} else if ((opt.drivename[0] == 'h' || opt.drivename[0] ==
'f') &&
diff U3 syslinux-6.04-pre1/com32/chain/options.c
diskbypartname/com32/chain/options.c
--- syslinux-6.04-pre1/com32/chain/options.c Fri Jul 15 01:28:29 2016
+++ diskbypartname/com32/chain/options.c Fri Jul 15 00:56:26 2016
@@ -107,6 +107,7 @@
" direct partition selection:",
" chain.c32 guid=<guid> [options]",
" chain.c32 label=<label> [options]",
+" chain.c32 diskbypartname=<partition_name> [options]",
" chain.c32 fs [options]",
"",
"You can use ':' instead of '=' and ' ' instead of
','.",
@@ -396,6 +397,8 @@
|| !strncmp(argv[i], "guid=", 5)
|| !strncmp(argv[i], "label:", 6)
|| !strncmp(argv[i], "label=", 6)
+ || !strncmp(argv[i], "diskbypartname:", 15)
+ || !strncmp(argv[i], "diskbypartname=", 15)
|| !strcmp(argv[i], "boot")
|| !strncmp(argv[i], "boot,", 5)
|| !strcmp(argv[i], "fs")) {
--