* [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
@ 2017-09-25 8:41 Chen Qi
2017-09-25 8:41 ` [PATCH V2 2/3] linux-yocto: make bbappend have effect conditionally Chen Qi
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Chen Qi @ 2017-09-25 8:41 UTC (permalink / raw)
To: meta-virtualization
Add a new class, sanity-bbappend.bbclass, to check for whether necessary
settings are available for bbappend files in this layer to be effective,
and warn users if not.
In addition, a variable SKIP_SANITY_BBAPPEND_CHECK is added to enable users
to explicitly skip the checking to avoid unwanted warnings.
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
classes/sanity-bbappend.bbclass | 10 ++++++++++
conf/layer.conf | 4 ++++
2 files changed, 14 insertions(+)
create mode 100644 classes/sanity-bbappend.bbclass
diff --git a/classes/sanity-bbappend.bbclass b/classes/sanity-bbappend.bbclass
new file mode 100644
index 0000000..bd2b717
--- /dev/null
+++ b/classes/sanity-bbappend.bbclass
@@ -0,0 +1,10 @@
+addhandler virt_bbappend_distrocheck
+virt_bbappend_distrocheck[eventmask] = "bb.event.ConfigParsed"
+python virt_bbappend_distrocheck() {
+ skip_check = e.data.getVar('SKIP_SANITY_BBAPPEND_CHECK') == "1"
+ if 'virtualization' not in e.data.getVar('DISTRO_FEATURES').split() and not skip_check:
+ bb.warn("You have included the meta-virtualization layer, but \
+'virtualization' has not been enabled in your DISTRO_FEATURES. Some bbappend files \
+may not take effect. See the meta-virtualization README for details on enabling \
+virtualization support.")
+}
diff --git a/conf/layer.conf b/conf/layer.conf
index be08a98..341ad38 100644
--- a/conf/layer.conf
+++ b/conf/layer.conf
@@ -22,3 +22,7 @@ require conf/distro/include/virt_security_flags.inc
PREFERRED_PROVIDER_virtual/runc ?= "runc-docker"
PREFERRED_PROVIDER_virtual/containerd ?= "containerd-docker"
+
+# Sanity check for application of bbappend files.
+# Setting SKIP_SANITY_BBAPPEND_CHECK to "1" would skip the check.
+INHERIT += "sanity-bbappend"
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V2 2/3] linux-yocto: make bbappend have effect conditionally
2017-09-25 8:41 [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Chen Qi
@ 2017-09-25 8:41 ` Chen Qi
2017-09-25 8:41 ` [PATCH V2 3/3] README: update to include information about bbappend inclusion Chen Qi
2017-09-25 10:40 ` [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Richard Purdie
2 siblings, 0 replies; 9+ messages in thread
From: Chen Qi @ 2017-09-25 8:41 UTC (permalink / raw)
To: meta-virtualization
Make these bbappend files to take effect only when DISTRO_FEATURES
contain 'virtualization'. Otherwise, we would meet failure failure
at system booting up qemux86. Related logs are as below.
systemd-modules-load[113]: Failed to insert 'kvm_amd': Operation not supported
systemd-modules-load[113]: Failed to insert 'kvm_intel': Operation not supported
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
recipes-kernel/linux/linux-yocto_4.10.bbappend | 21 +--------------------
recipes-kernel/linux/linux-yocto_4.12.bbappend | 21 +--------------------
recipes-kernel/linux/linux-yocto_4.4.bbappend | 21 +--------------------
recipes-kernel/linux/linux-yocto_4.9.bbappend | 21 +--------------------
recipes-kernel/linux/linux-yocto_virtualization.inc | 20 ++++++++++++++++++++
5 files changed, 24 insertions(+), 80 deletions(-)
create mode 100644 recipes-kernel/linux/linux-yocto_virtualization.inc
diff --git a/recipes-kernel/linux/linux-yocto_4.10.bbappend b/recipes-kernel/linux/linux-yocto_4.10.bbappend
index f163fdf..617cacc 100644
--- a/recipes-kernel/linux/linux-yocto_4.10.bbappend
+++ b/recipes-kernel/linux/linux-yocto_4.10.bbappend
@@ -1,20 +1 @@
-FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-
-SRC_URI += "file://xt-checksum.scc \
- file://ebtables.scc \
- file://vswitch.scc \
- file://lxc.scc \
- file://docker.scc \
- "
-KERNEL_FEATURES_append = " features/kvm/qemu-kvm-enable.scc"
-
-KERNEL_MODULE_AUTOLOAD += "openvswitch"
-KERNEL_MODULE_AUTOLOAD += "kvm"
-KERNEL_MODULE_AUTOLOAD += "kvm-amd"
-KERNEL_MODULE_AUTOLOAD += "kvm-intel"
-
-# aufs kernel support required for xen-image-minimal
-KERNEL_FEATURES_append += "${@bb.utils.contains('DISTRO_FEATURES', 'aufs', ' features/aufs/aufs-enable.scc', '', d)}"
-
-# xen kernel support
-SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'xen', ' file://xen.scc', '', d)}"
+require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', '${BPN}_virtualization.inc', '', d)}
diff --git a/recipes-kernel/linux/linux-yocto_4.12.bbappend b/recipes-kernel/linux/linux-yocto_4.12.bbappend
index f163fdf..617cacc 100644
--- a/recipes-kernel/linux/linux-yocto_4.12.bbappend
+++ b/recipes-kernel/linux/linux-yocto_4.12.bbappend
@@ -1,20 +1 @@
-FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-
-SRC_URI += "file://xt-checksum.scc \
- file://ebtables.scc \
- file://vswitch.scc \
- file://lxc.scc \
- file://docker.scc \
- "
-KERNEL_FEATURES_append = " features/kvm/qemu-kvm-enable.scc"
-
-KERNEL_MODULE_AUTOLOAD += "openvswitch"
-KERNEL_MODULE_AUTOLOAD += "kvm"
-KERNEL_MODULE_AUTOLOAD += "kvm-amd"
-KERNEL_MODULE_AUTOLOAD += "kvm-intel"
-
-# aufs kernel support required for xen-image-minimal
-KERNEL_FEATURES_append += "${@bb.utils.contains('DISTRO_FEATURES', 'aufs', ' features/aufs/aufs-enable.scc', '', d)}"
-
-# xen kernel support
-SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'xen', ' file://xen.scc', '', d)}"
+require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', '${BPN}_virtualization.inc', '', d)}
diff --git a/recipes-kernel/linux/linux-yocto_4.4.bbappend b/recipes-kernel/linux/linux-yocto_4.4.bbappend
index f163fdf..617cacc 100644
--- a/recipes-kernel/linux/linux-yocto_4.4.bbappend
+++ b/recipes-kernel/linux/linux-yocto_4.4.bbappend
@@ -1,20 +1 @@
-FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-
-SRC_URI += "file://xt-checksum.scc \
- file://ebtables.scc \
- file://vswitch.scc \
- file://lxc.scc \
- file://docker.scc \
- "
-KERNEL_FEATURES_append = " features/kvm/qemu-kvm-enable.scc"
-
-KERNEL_MODULE_AUTOLOAD += "openvswitch"
-KERNEL_MODULE_AUTOLOAD += "kvm"
-KERNEL_MODULE_AUTOLOAD += "kvm-amd"
-KERNEL_MODULE_AUTOLOAD += "kvm-intel"
-
-# aufs kernel support required for xen-image-minimal
-KERNEL_FEATURES_append += "${@bb.utils.contains('DISTRO_FEATURES', 'aufs', ' features/aufs/aufs-enable.scc', '', d)}"
-
-# xen kernel support
-SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'xen', ' file://xen.scc', '', d)}"
+require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', '${BPN}_virtualization.inc', '', d)}
diff --git a/recipes-kernel/linux/linux-yocto_4.9.bbappend b/recipes-kernel/linux/linux-yocto_4.9.bbappend
index f163fdf..617cacc 100644
--- a/recipes-kernel/linux/linux-yocto_4.9.bbappend
+++ b/recipes-kernel/linux/linux-yocto_4.9.bbappend
@@ -1,20 +1 @@
-FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-
-SRC_URI += "file://xt-checksum.scc \
- file://ebtables.scc \
- file://vswitch.scc \
- file://lxc.scc \
- file://docker.scc \
- "
-KERNEL_FEATURES_append = " features/kvm/qemu-kvm-enable.scc"
-
-KERNEL_MODULE_AUTOLOAD += "openvswitch"
-KERNEL_MODULE_AUTOLOAD += "kvm"
-KERNEL_MODULE_AUTOLOAD += "kvm-amd"
-KERNEL_MODULE_AUTOLOAD += "kvm-intel"
-
-# aufs kernel support required for xen-image-minimal
-KERNEL_FEATURES_append += "${@bb.utils.contains('DISTRO_FEATURES', 'aufs', ' features/aufs/aufs-enable.scc', '', d)}"
-
-# xen kernel support
-SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'xen', ' file://xen.scc', '', d)}"
+require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', '${BPN}_virtualization.inc', '', d)}
diff --git a/recipes-kernel/linux/linux-yocto_virtualization.inc b/recipes-kernel/linux/linux-yocto_virtualization.inc
new file mode 100644
index 0000000..8b296d7
--- /dev/null
+++ b/recipes-kernel/linux/linux-yocto_virtualization.inc
@@ -0,0 +1,20 @@
+FILESEXTRAPATHS_prepend := "${THISDIR}/linux-yocto:"
+
+SRC_URI += "file://xt-checksum.scc \
+ file://ebtables.scc \
+ file://vswitch.scc \
+ file://lxc.scc \
+ file://docker.scc \
+ "
+KERNEL_FEATURES_append = " features/kvm/qemu-kvm-enable.scc"
+
+KERNEL_MODULE_AUTOLOAD += "openvswitch"
+KERNEL_MODULE_AUTOLOAD += "kvm"
+KERNEL_MODULE_AUTOLOAD += "kvm-amd"
+KERNEL_MODULE_AUTOLOAD += "kvm-intel"
+
+# aufs kernel support required for xen-image-minimal
+KERNEL_FEATURES_append += "${@bb.utils.contains('DISTRO_FEATURES', 'aufs', ' features/aufs/aufs-enable.scc', '', d)}"
+
+# xen kernel support
+SRC_URI += "${@bb.utils.contains('DISTRO_FEATURES', 'xen', ' file://xen.scc', '', d)}"
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V2 3/3] README: update to include information about bbappend inclusion
2017-09-25 8:41 [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Chen Qi
2017-09-25 8:41 ` [PATCH V2 2/3] linux-yocto: make bbappend have effect conditionally Chen Qi
@ 2017-09-25 8:41 ` Chen Qi
2017-09-25 10:40 ` [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Richard Purdie
2 siblings, 0 replies; 9+ messages in thread
From: Chen Qi @ 2017-09-25 8:41 UTC (permalink / raw)
To: meta-virtualization
Update README file to include information about bbappend file inclusion,
telling the users that 'virtualization' needs to be in DISTRO_FEATURES
to make some bbappend files to be effective.
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
README | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/README b/README
index 2578f90..49fda10 100644
--- a/README
+++ b/README
@@ -4,6 +4,11 @@ meta-virtualization
This layer provides support for building Xen, KVM, Libvirt, and associated
packages necessary for constructing OE-based virtualized solutions.
+The bbappend files for some recipe (e.g. linux-yocto) in this layer need to
+have 'virtualization' in DISTRO_FEATURES to have effect. To enable them, add
+in configuration file the following line.
+DISTRO_FEATURES_append = " virtualization"
+
Dependencies
------------
This layer depends on:
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 8:41 [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Chen Qi
2017-09-25 8:41 ` [PATCH V2 2/3] linux-yocto: make bbappend have effect conditionally Chen Qi
2017-09-25 8:41 ` [PATCH V2 3/3] README: update to include information about bbappend inclusion Chen Qi
@ 2017-09-25 10:40 ` Richard Purdie
2017-09-25 12:00 ` Rich Persaud
2017-09-25 12:49 ` Bruce Ashfield
2 siblings, 2 replies; 9+ messages in thread
From: Richard Purdie @ 2017-09-25 10:40 UTC (permalink / raw)
To: Chen Qi, meta-virtualization
On Mon, 2017-09-25 at 16:41 +0800, Chen Qi wrote:
> Add a new class, sanity-bbappend.bbclass, to check for whether
> necessary
> settings are available for bbappend files in this layer to be
> effective,
> and warn users if not.
>
> In addition, a variable SKIP_SANITY_BBAPPEND_CHECK is added to enable
> users
> to explicitly skip the checking to avoid unwanted warnings.
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
> classes/sanity-bbappend.bbclass | 10 ++++++++++
That is a horrible name for a bbclass, could easily clash with another
layer.
> conf/layer.conf | 4 ++++
> 2 files changed, 14 insertions(+)
> create mode 100644 classes/sanity-bbappend.bbclass
Looking at this patch series, I suspect this is partly related to YP
Compatibility v2.
I do think that its highlighting an issue here in that we have several
very similar bbappends where the functionality would likely be better
in a common file and secondly, better in the core recipes.
This layer would then just need to configure it rather than adding the
config fragements and so on as well.
So yes, I think there is a valid issue here and in many senses, YP
Compat v2 is probably highlighting a real problem but I suspect this
patch series is not the way to fix it...
Is there a good reason we wouldn't want "virt" markup in the main
recipes and it being a configuration option?
Cheers,
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 10:40 ` [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Richard Purdie
@ 2017-09-25 12:00 ` Rich Persaud
2017-09-25 12:45 ` Richard Purdie
2017-09-25 12:49 ` Bruce Ashfield
1 sibling, 1 reply; 9+ messages in thread
From: Rich Persaud @ 2017-09-25 12:00 UTC (permalink / raw)
To: Richard Purdie; +Cc: meta-virtualization
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On Sep 25, 2017, at 06:40, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> ...
>
> So yes, I think there is a valid issue here and in many senses, YP
> Compat v2 is probably highlighting a real problem but I suspect this
> patch series is not the way to fix it...
Are there docs on YP Compat v2? A web search turned up this RFC:
https://lists.yoctoproject.org/pipermail/yocto/2017-February/034489.html
Rich
[-- Attachment #2: Type: text/html, Size: 903 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 12:00 ` Rich Persaud
@ 2017-09-25 12:45 ` Richard Purdie
0 siblings, 0 replies; 9+ messages in thread
From: Richard Purdie @ 2017-09-25 12:45 UTC (permalink / raw)
To: Rich Persaud; +Cc: meta-virtualization
On Mon, 2017-09-25 at 08:00 -0400, Rich Persaud wrote:
> On Sep 25, 2017, at 06:40, Richard Purdie <richard.purdie@linuxfounda
> tion.org> wrote:
> ...
> >
> > So yes, I think there is a valid issue here and in many senses, YP
> > Compat v2 is probably highlighting a real problem but I suspect
> > this
> > patch series is not the way to fix it...
> Are there docs on YP Compat v2? A web search turned up this RFC:
>
> https://lists.yoctoproject.org/pipermail/yocto/2017-February/034489.h
> tml
There is the yocto-layer-check script which basically embodies the
planned changes. I do need to write up a lot of the information but am
struggling to find the time to do that the justice it needs...
Cheers,
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 10:40 ` [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Richard Purdie
2017-09-25 12:00 ` Rich Persaud
@ 2017-09-25 12:49 ` Bruce Ashfield
2017-09-25 13:10 ` Richard Purdie
1 sibling, 1 reply; 9+ messages in thread
From: Bruce Ashfield @ 2017-09-25 12:49 UTC (permalink / raw)
To: Richard Purdie; +Cc: meta-virtualization@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]
On Mon, Sep 25, 2017 at 6:40 AM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2017-09-25 at 16:41 +0800, Chen Qi wrote:
> > Add a new class, sanity-bbappend.bbclass, to check for whether
> > necessary
> > settings are available for bbappend files in this layer to be
> > effective,
> > and warn users if not.
> >
> > In addition, a variable SKIP_SANITY_BBAPPEND_CHECK is added to enable
> > users
> > to explicitly skip the checking to avoid unwanted warnings.
> >
> > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > ---
> > classes/sanity-bbappend.bbclass | 10 ++++++++++
>
> That is a horrible name for a bbclass, could easily clash with another
> layer.
>
> > conf/layer.conf | 4 ++++
> > 2 files changed, 14 insertions(+)
> > create mode 100644 classes/sanity-bbappend.bbclass
>
> Looking at this patch series, I suspect this is partly related to YP
> Compatibility v2.
>
It is.
The first series arrived that created a magic distro variable
'virtualization'
that no one would know about, and have no idea how to use it .. as a result
existing configurations would have a silent and drastic change on the
behaviour.
I wasn't about to merge that change, so I suggested that if everything was
to be triggered off a variable, that variable needed to be advertised by
more
than just its use in the code, and more than a mention in a README.
That led to this first effort at making the new distro variable visible.
>
> I do think that its highlighting an issue here in that we have several
> very similar bbappends where the functionality would likely be better
> in a common file and secondly, better in the core recipes.
>
> This layer would then just need to configure it rather than adding the
> config fragements and so on as well.
>
> So yes, I think there is a valid issue here and in many senses, YP
> Compat v2 is probably highlighting a real problem but I suspect this
> patch series is not the way to fix it...
>
> Is there a good reason we wouldn't want "virt" markup in the main
> recipes and it being a configuration option?
>
I'm not following the configuration option 100%. Do you just mean setting
'virtualization' in distro features or some other similar configuration
variable ? If that had to be manually done via distro or local.conf that
would
leave my original concern outstanding.
Bruce
>
> Cheers,
>
> Richard
>
>
> --
> _______________________________________________
> meta-virtualization mailing list
> meta-virtualization@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/meta-virtualization
>
--
"Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end"
[-- Attachment #2: Type: text/html, Size: 4202 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 12:49 ` Bruce Ashfield
@ 2017-09-25 13:10 ` Richard Purdie
2017-09-25 14:02 ` Bruce Ashfield
0 siblings, 1 reply; 9+ messages in thread
From: Richard Purdie @ 2017-09-25 13:10 UTC (permalink / raw)
To: Bruce Ashfield; +Cc: meta-virtualization@yoctoproject.org
On Mon, 2017-09-25 at 08:49 -0400, Bruce Ashfield wrote:
> On Mon, Sep 25, 2017 at 6:40 AM, Richard Purdie <richard.purdie@linux
> foundation.org> wrote:
> >
> > > conf/layer.conf | 4 ++++
> > > 2 files changed, 14 insertions(+)
> > > create mode 100644 classes/sanity-bbappend.bbclass
> >
> > Looking at this patch series, I suspect this is partly related to
> > YP Compatibility v2.
> It is.
>
> The first series arrived that created a magic distro variable
> 'virtualization' that no one would know about, and have no idea how
> to use it .. as a result existing configurations would have a silent
> and drastic change on the behaviour.
>
> I wasn't about to merge that change, so I suggested that if
> everything was to be triggered off a variable, that variable needed
> to be advertised by more than just its use in the code, and more than
> a mention in a README.
>
> That led to this first effort at making the new distro variable
> visible.
>
> >
> > I do think that its highlighting an issue here in that we have
> > several
> > very similar bbappends where the functionality would likely be
> > better
> > in a common file and secondly, better in the core recipes.
> >
> > This layer would then just need to configure it rather than adding
> > the
> > config fragements and so on as well.
> >
> > So yes, I think there is a valid issue here and in many senses, YP
> > Compat v2 is probably highlighting a real problem but I suspect
> > this
> > patch series is not the way to fix it...
> >
> > Is there a good reason we wouldn't want "virt" markup in the main
> > recipes and it being a configuration option?
>
> I'm not following the configuration option 100%. Do you just mean
> setting 'virtualization' in distro features or some other similar
> configuration variable ? If that had to be manually done via distro
> or local.conf that would leave my original concern outstanding.
I'll spell this out from the ground up since I know you understand this
but I think others reading might not have thought about all of this
quite as much.
One of the bigger problems we have today is that as you add layers,
your build subtly changes and most users have any idea that it happens.
This means sstate becomes invalid, things rebuild and people don't
really understand or control what they're building.
In this case, changing the kernel to enable virtualisation when adding
a virtualisation layer isn't totally unexpected but there are other
cases which are more subtle and problematic. Even here, spelling it out
as a configuration option which users (or distro) opt into is a good
thing.
I'm fine with the warning in this patch series, that makes a lot of
sense. The naming of the class is horrible IMO though.
What I am a little more worried about is that we have to bbappend
recipes like linux-yocto. I have to ask the question, why not put the
configuration markup into the upstream recipe?
There still may be something in meta-virt that warns if key
configuration isn't enabled but the actual code that handles the option
perhaps shouldn't be there now its established?
In new layers pushing the boundaries of technology with things that are
rapidly changing, bbappends in layers make sense. In this case I have
to wonder why we don't just put some configuration options in linux-
yocto in OE-Core. Its not rapid changing, its well established and
fairly universally useful.
We could consider whether it should be based on DISTRO_FEATURE or we
whether we add 'PACKAGECONFIG' for the kernel?
Whilst there are easy ways to make the YP Compat tests pass, I do want
us to think about what actually makes sense...
Cheers,
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking
2017-09-25 13:10 ` Richard Purdie
@ 2017-09-25 14:02 ` Bruce Ashfield
0 siblings, 0 replies; 9+ messages in thread
From: Bruce Ashfield @ 2017-09-25 14:02 UTC (permalink / raw)
To: Richard Purdie; +Cc: meta-virtualization@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]
On Mon, Sep 25, 2017 at 9:10 AM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2017-09-25 at 08:49 -0400, Bruce Ashfield wrote:
> > On Mon, Sep 25, 2017 at 6:40 AM, Richard Purdie <richard.purdie@linux
> > foundation.org> wrote:
> > >
> > > > conf/layer.conf | 4 ++++
> > > > 2 files changed, 14 insertions(+)
> > > > create mode 100644 classes/sanity-bbappend.bbclass
> > >
> > > Looking at this patch series, I suspect this is partly related to
> > > YP Compatibility v2.
> > It is.
> >
> > The first series arrived that created a magic distro variable
> > 'virtualization' that no one would know about, and have no idea how
> > to use it .. as a result existing configurations would have a silent
> > and drastic change on the behaviour.
> >
> > I wasn't about to merge that change, so I suggested that if
> > everything was to be triggered off a variable, that variable needed
> > to be advertised by more than just its use in the code, and more than
> > a mention in a README.
> >
> > That led to this first effort at making the new distro variable
> > visible.
> >
> > >
> > > I do think that its highlighting an issue here in that we have
> > > several
> > > very similar bbappends where the functionality would likely be
> > > better
> > > in a common file and secondly, better in the core recipes.
> > >
> > > This layer would then just need to configure it rather than adding
> > > the
> > > config fragements and so on as well.
> > >
> > > So yes, I think there is a valid issue here and in many senses, YP
> > > Compat v2 is probably highlighting a real problem but I suspect
> > > this
> > > patch series is not the way to fix it...
> > >
> > > Is there a good reason we wouldn't want "virt" markup in the main
> > > recipes and it being a configuration option?
> >
> > I'm not following the configuration option 100%. Do you just mean
> > setting 'virtualization' in distro features or some other similar
> > configuration variable ? If that had to be manually done via distro
> > or local.conf that would leave my original concern outstanding.
>
> I'll spell this out from the ground up since I know you understand this
> but I think others reading might not have thought about all of this
> quite as much.
>
> One of the bigger problems we have today is that as you add layers,
> your build subtly changes and most users have any idea that it happens.
> This means sstate becomes invalid, things rebuild and people don't
> really understand or control what they're building.
>
> In this case, changing the kernel to enable virtualisation when adding
> a virtualisation layer isn't totally unexpected but there are other
> cases which are more subtle and problematic. Even here, spelling it out
> as a configuration option which users (or distro) opt into is a good
> thing.
>
Agreed. I don't object to this being controlled by a variable, I object to
a silent change to require that variable.
>
> I'm fine with the warning in this patch series, that makes a lot of
> sense. The naming of the class is horrible IMO though.
>
I don't disagree on that point. A better namespace-safe name for the
class could be used. As long as the functionality is there, I'm happy.
>
> What I am a little more worried about is that we have to bbappend
> recipes like linux-yocto. I have to ask the question, why not put the
> configuration markup into the upstream recipe?
>
I'm not following on this one. You mean have the relevant recipes be
able to inject changes that impact how the kernel is configured or
built ? i.e. pkg-config or something similar ?
>
> There still may be something in meta-virt that warns if key
> configuration isn't enabled but the actual code that handles the option
> perhaps shouldn't be there now its established?
>
> In new layers pushing the boundaries of technology with things that are
> rapidly changing, bbappends in layers make sense. In this case I have
> to wonder why we don't just put some configuration options in linux-
> yocto in OE-Core. Its not rapid changing, its well established and
> fairly universally useful.
>
We've been using fragments to that end, and with my code that makes the
fragments available to any kernel, that should help on this front.
>
> We could consider whether it should be based on DISTRO_FEATURE or we
> whether we add 'PACKAGECONFIG' for the kernel?
>
I have lots of thoughts on this topic, but I want to understand before I
head
off on a tangent. The kernel is a royal pain in this area given the drastic
difference in capabilities, versions, h/w, etc. I have old bugzilla entries
on this
topic, and have the first stage of code ready to make available early in the
next release cycle.
>
> Whilst there are easy ways to make the YP Compat tests pass, I do want
> us to think about what actually makes sense...
>
Agreed.
Bruce
>
> Cheers,
>
> Richard
>
>
>
--
"Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end"
[-- Attachment #2: Type: text/html, Size: 7191 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-25 14:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-25 8:41 [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Chen Qi
2017-09-25 8:41 ` [PATCH V2 2/3] linux-yocto: make bbappend have effect conditionally Chen Qi
2017-09-25 8:41 ` [PATCH V2 3/3] README: update to include information about bbappend inclusion Chen Qi
2017-09-25 10:40 ` [PATCH V2 1/3] sanity-bbappend.bbclass: add class for bbappend files checking Richard Purdie
2017-09-25 12:00 ` Rich Persaud
2017-09-25 12:45 ` Richard Purdie
2017-09-25 12:49 ` Bruce Ashfield
2017-09-25 13:10 ` Richard Purdie
2017-09-25 14:02 ` Bruce Ashfield
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.