* [RFC][PATCH] wic: use part_name when defined
@ 2023-03-22 15:54 dit.kozmaj
2023-03-22 16:27 ` Dit Kozmaj
2023-03-29 15:55 ` [OE-core] " Richard Purdie
0 siblings, 2 replies; 4+ messages in thread
From: dit.kozmaj @ 2023-03-22 15:54 UTC (permalink / raw)
To: openembedded-core; +Cc: Dit Kozmaj, Diego Rondini
From: Dit Kozmaj <dit.kozmaj@kynetics.com>
So far part.label has been used to define GPT partition label even if
part.part_name was defined.
Fix the code to use part.part_name whenever available, as it makes sense
to have a GPT partition label which is different from the contained
filesystem label.
Signed-off-by: Dit Kozmaj <dit.kozmaj@kynetics.com>
Signed-off-by: Diego Rondini <diego.rondini@kynetics.com>
---
scripts/lib/wic/plugins/imager/direct.py | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index dfaa901567..1f06b89f8e 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -571,11 +571,12 @@ class PartitionedImage():
self._create_partition(self.path, part.type,
parted_fs_type, part.start, part.size_sec)
- if part.part_name:
+ if part.part_name or part.label:
+ partition_label = part.part_name if part.part_name else part.label
logger.debug("partition %d: set name to %s",
- part.num, part.part_name)
+ part.num, partition_label)
exec_native_cmd("sgdisk --change-name=%d:%s %s" % \
- (part.num, part.part_name,
+ (part.num, partition_label,
self.path), self.native_sysroot)
if part.part_type:
@@ -592,13 +593,6 @@ class PartitionedImage():
(part.num, part.uuid, self.path),
self.native_sysroot)
- if part.label and self.ptable_format == "gpt":
- logger.debug("partition %d: set name to %s",
- part.num, part.label)
- exec_native_cmd("parted -s %s name %d %s" % \
- (self.path, part.num, part.label),
- self.native_sysroot)
-
if part.active:
flag_name = "legacy_boot" if self.ptable_format == 'gpt' else "boot"
logger.debug("Set '%s' flag for partition '%s' on disk '%s'",
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC][PATCH] wic: use part_name when defined
2023-03-22 15:54 [RFC][PATCH] wic: use part_name when defined dit.kozmaj
@ 2023-03-22 16:27 ` Dit Kozmaj
2023-03-29 15:55 ` [OE-core] " Richard Purdie
1 sibling, 0 replies; 4+ messages in thread
From: Dit Kozmaj @ 2023-03-22 16:27 UTC (permalink / raw)
To: openembedded-core; +Cc: Diego Rondini
[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]
Hi,
We tested a few combinations of --part-name and --label parameters when
creating a WIC image.
When only the --part-name is defined, the GPT partition label is correctly
set, and filesystem label is empty.
If both are defined, or only --label is defined, both the filesystem label
and the GPT partition label are set to the value of --label.
As it makes sense to have a GPT partition label which is different from the
contained filesystem label, our proposal is to fix the case when both are
set, and keep the current behavior when only --label is set for
compatibility reasons.
If this proposal is ok, we will patch the documentation to document that
the GPT partition label is set even when only --label is defined.
Regards
On Wed, Mar 22, 2023 at 4:54 PM <dit.kozmaj@kynetics.com> wrote:
> From: Dit Kozmaj <dit.kozmaj@kynetics.com>
>
> So far part.label has been used to define GPT partition label even if
> part.part_name was defined.
> Fix the code to use part.part_name whenever available, as it makes sense
> to have a GPT partition label which is different from the contained
> filesystem label.
>
> Signed-off-by: Dit Kozmaj <dit.kozmaj@kynetics.com>
> Signed-off-by: Diego Rondini <diego.rondini@kynetics.com>
> ---
> scripts/lib/wic/plugins/imager/direct.py | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/lib/wic/plugins/imager/direct.py
> b/scripts/lib/wic/plugins/imager/direct.py
> index dfaa901567..1f06b89f8e 100644
> --- a/scripts/lib/wic/plugins/imager/direct.py
> +++ b/scripts/lib/wic/plugins/imager/direct.py
> @@ -571,11 +571,12 @@ class PartitionedImage():
> self._create_partition(self.path, part.type,
> parted_fs_type, part.start,
> part.size_sec)
>
> - if part.part_name:
> + if part.part_name or part.label:
> + partition_label = part.part_name if part.part_name else
> part.label
> logger.debug("partition %d: set name to %s",
> - part.num, part.part_name)
> + part.num, partition_label)
> exec_native_cmd("sgdisk --change-name=%d:%s %s" % \
> - (part.num, part.part_name,
> + (part.num, partition_label,
> self.path), self.native_sysroot)
>
> if part.part_type:
> @@ -592,13 +593,6 @@ class PartitionedImage():
> (part.num, part.uuid, self.path),
> self.native_sysroot)
>
> - if part.label and self.ptable_format == "gpt":
> - logger.debug("partition %d: set name to %s",
> - part.num, part.label)
> - exec_native_cmd("parted -s %s name %d %s" % \
> - (self.path, part.num, part.label),
> - self.native_sysroot)
> -
> if part.active:
> flag_name = "legacy_boot" if self.ptable_format == 'gpt'
> else "boot"
> logger.debug("Set '%s' flag for partition '%s' on disk
> '%s'",
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 4321 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [OE-core] [RFC][PATCH] wic: use part_name when defined
2023-03-22 15:54 [RFC][PATCH] wic: use part_name when defined dit.kozmaj
2023-03-22 16:27 ` Dit Kozmaj
@ 2023-03-29 15:55 ` Richard Purdie
2023-04-06 9:08 ` Diego Rondini
1 sibling, 1 reply; 4+ messages in thread
From: Richard Purdie @ 2023-03-29 15:55 UTC (permalink / raw)
To: dit.kozmaj, openembedded-core; +Cc: Diego Rondini
On Wed, 2023-03-22 at 16:54 +0100, dit.kozmaj@kynetics.com wrote:
> From: Dit Kozmaj <dit.kozmaj@kynetics.com>
>
> So far part.label has been used to define GPT partition label even if
> part.part_name was defined.
> Fix the code to use part.part_name whenever available, as it makes sense
> to have a GPT partition label which is different from the contained
> filesystem label.
>
> Signed-off-by: Dit Kozmaj <dit.kozmaj@kynetics.com>
> Signed-off-by: Diego Rondini <diego.rondini@kynetics.com>
> ---
> scripts/lib/wic/plugins/imager/direct.py | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
> index dfaa901567..1f06b89f8e 100644
> --- a/scripts/lib/wic/plugins/imager/direct.py
> +++ b/scripts/lib/wic/plugins/imager/direct.py
> @@ -571,11 +571,12 @@ class PartitionedImage():
> self._create_partition(self.path, part.type,
> parted_fs_type, part.start, part.size_sec)
>
> - if part.part_name:
> + if part.part_name or part.label:
> + partition_label = part.part_name if part.part_name else part.label
> logger.debug("partition %d: set name to %s",
> - part.num, part.part_name)
> + part.num, partition_label)
> exec_native_cmd("sgdisk --change-name=%d:%s %s" % \
> - (part.num, part.part_name,
> + (part.num, partition_label,
> self.path), self.native_sysroot)
>
> if part.part_type:
> @@ -592,13 +593,6 @@ class PartitionedImage():
> (part.num, part.uuid, self.path),
> self.native_sysroot)
>
> - if part.label and self.ptable_format == "gpt":
> - logger.debug("partition %d: set name to %s",
> - part.num, part.label)
> - exec_native_cmd("parted -s %s name %d %s" % \
> - (self.path, part.num, part.label),
> - self.native_sysroot)
> -
> if part.active:
> flag_name = "legacy_boot" if self.ptable_format == 'gpt' else "boot"
> logger.debug("Set '%s' flag for partition '%s' on disk '%s'",
This was flagged on irc as not having a response. We're struggling with
a lack of a maintainer for wic at the moment so review is slow and I'm
not really a wic expert but I'm all we have :/.
As mentioned, changes to code like this are much more likely if there
is a test case included. The existing tests can be run with "oe-
selftest -r wic".
Can you confirm this patch doesn't regress the current tests?
Would you be able to add a test so this change doesn't regress in
future?
Cheers,
Richard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [OE-core] [RFC][PATCH] wic: use part_name when defined
2023-03-29 15:55 ` [OE-core] " Richard Purdie
@ 2023-04-06 9:08 ` Diego Rondini
0 siblings, 0 replies; 4+ messages in thread
From: Diego Rondini @ 2023-04-06 9:08 UTC (permalink / raw)
To: Richard Purdie; +Cc: dit.kozmaj, openembedded-core
[-- Attachment #1: Type: text/plain, Size: 3727 bytes --]
Hi,
On Wed, Mar 29, 2023 at 5:55 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2023-03-22 at 16:54 +0100, dit.kozmaj@kynetics.com wrote:
> > From: Dit Kozmaj <dit.kozmaj@kynetics.com>
> >
> > So far part.label has been used to define GPT partition label even if
> > part.part_name was defined.
> > Fix the code to use part.part_name whenever available, as it makes sense
> > to have a GPT partition label which is different from the contained
> > filesystem label.
> >
> > Signed-off-by: Dit Kozmaj <dit.kozmaj@kynetics.com>
> > Signed-off-by: Diego Rondini <diego.rondini@kynetics.com>
> > ---
> > scripts/lib/wic/plugins/imager/direct.py | 14 ++++----------
> > 1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/scripts/lib/wic/plugins/imager/direct.py
> b/scripts/lib/wic/plugins/imager/direct.py
> > index dfaa901567..1f06b89f8e 100644
> > --- a/scripts/lib/wic/plugins/imager/direct.py
> > +++ b/scripts/lib/wic/plugins/imager/direct.py
> > @@ -571,11 +571,12 @@ class PartitionedImage():
> > self._create_partition(self.path, part.type,
> > parted_fs_type, part.start,
> part.size_sec)
> >
> > - if part.part_name:
> > + if part.part_name or part.label:
> > + partition_label = part.part_name if part.part_name else
> part.label
> > logger.debug("partition %d: set name to %s",
> > - part.num, part.part_name)
> > + part.num, partition_label)
> > exec_native_cmd("sgdisk --change-name=%d:%s %s" % \
> > - (part.num, part.part_name,
> > + (part.num, partition_label,
> > self.path),
> self.native_sysroot)
> >
> > if part.part_type:
> > @@ -592,13 +593,6 @@ class PartitionedImage():
> > (part.num, part.uuid, self.path),
> > self.native_sysroot)
> >
> > - if part.label and self.ptable_format == "gpt":
> > - logger.debug("partition %d: set name to %s",
> > - part.num, part.label)
> > - exec_native_cmd("parted -s %s name %d %s" % \
> > - (self.path, part.num, part.label),
> > - self.native_sysroot)
> > -
> > if part.active:
> > flag_name = "legacy_boot" if self.ptable_format ==
> 'gpt' else "boot"
> > logger.debug("Set '%s' flag for partition '%s' on disk
> '%s'",
>
> This was flagged on irc as not having a response. We're struggling with
> a lack of a maintainer for wic at the moment so review is slow and I'm
> not really a wic expert but I'm all we have :/.
>
> As mentioned, changes to code like this are much more likely if there
> is a test case included. The existing tests can be run with "oe-
> selftest -r wic".
>
> Can you confirm this patch doesn't regress the current tests?
>
We've sent a new patchset that doesn't regress the current tests:
https://patchwork.yoctoproject.org/project/oe-core/list/?series=11822
> Would you be able to add a test so this change doesn't regress in
> future?
>
The aforementioned patchset includes a new test for that purpose and
updates the documentation to reflect the changes and explain what happens
in each case.
This RFC is superseded by the patchset.
Cheers,
>
> Richard
>
>
Cheers,
Diego Rondini
Sr. Embedded Engineer
Kynetics
www.kynetics.com
[-- Attachment #2: Type: text/html, Size: 5644 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-06 9:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 15:54 [RFC][PATCH] wic: use part_name when defined dit.kozmaj
2023-03-22 16:27 ` Dit Kozmaj
2023-03-29 15:55 ` [OE-core] " Richard Purdie
2023-04-06 9:08 ` Diego Rondini
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.