[libvirt PATCH] news: Mention Apple Silicon support

Andrea Bolognani posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210215095947.22829-1-abologna@redhat.com
NEWS.rst | 6 ++++++
1 file changed, 6 insertions(+)
[libvirt PATCH] news: Mention Apple Silicon support
Posted by Andrea Bolognani 3 years, 2 months ago
After the recent fixes, it's now confirmed to work.

https://gitlab.com/libvirt/libvirt/-/issues/121

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 NEWS.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index a9a1a61119..440dbf2601 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -11,6 +11,12 @@ For a more fine-grained view, use the `git log`_.
 v7.1.0 (unreleased)
 ===================
 
+* **Portability**
+
+  * Implement Apple Silicon support
+
+    libvirt now runs on the ARM-based Apple Silicon Macs.
+
 * **New features**
 
   * Introduce virtio-pmem ``<memory/>`` model
-- 
2.26.2

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Peter Krempa 3 years, 2 months ago
On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> After the recent fixes, it's now confirmed to work.
> 
> https://gitlab.com/libvirt/libvirt/-/issues/121

Should it be closed then?

> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  NEWS.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/NEWS.rst b/NEWS.rst
> index a9a1a61119..440dbf2601 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -11,6 +11,12 @@ For a more fine-grained view, use the `git log`_.
>  v7.1.0 (unreleased)
>  ===================
>  
> +* **Portability**
> +
> +  * Implement Apple Silicon support

Just semantics, wasn't the problem just in the test suite? In which case
I'd consider it more of a fix than implementation.

> +
> +    libvirt now runs on the ARM-based Apple Silicon Macs.

Regardless of the above,

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Erik Skultety 3 years, 2 months ago
On Mon, Feb 15, 2021 at 11:10:43AM +0100, Peter Krempa wrote:
> On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > After the recent fixes, it's now confirmed to work.
> > 
> > https://gitlab.com/libvirt/libvirt/-/issues/121
> 
> Should it be closed then?

Good point, we should start using the "magic" keywords that GitLab recognizes
an closes issues automatically.

Erik

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Peter Krempa 3 years, 2 months ago
On Mon, Feb 15, 2021 at 11:19:12 +0100, Erik Skultety wrote:
> On Mon, Feb 15, 2021 at 11:10:43AM +0100, Peter Krempa wrote:
> > On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > > After the recent fixes, it's now confirmed to work.
> > > 
> > > https://gitlab.com/libvirt/libvirt/-/issues/121
> > 
> > Should it be closed then?
> 
> Good point, we should start using the "magic" keywords that GitLab recognizes
> an closes issues automatically.

Or just close it manually after the person confirms that it's fixed.

In cases such as this you don't want to close the issue with a
push-event if you can't test that the patches indeed fix the issue.

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Erik Skultety 3 years, 2 months ago
On Mon, Feb 15, 2021 at 11:22:33AM +0100, Peter Krempa wrote:
> On Mon, Feb 15, 2021 at 11:19:12 +0100, Erik Skultety wrote:
> > On Mon, Feb 15, 2021 at 11:10:43AM +0100, Peter Krempa wrote:
> > > On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > > > After the recent fixes, it's now confirmed to work.
> > > > 
> > > > https://gitlab.com/libvirt/libvirt/-/issues/121
> > > 
> > > Should it be closed then?
> > 
> > Good point, we should start using the "magic" keywords that GitLab recognizes
> > an closes issues automatically.
> 
> Or just close it manually after the person confirms that it's fixed.
> 
> In cases such as this you don't want to close the issue with a
> push-event if you can't test that the patches indeed fix the issue.

Wait, what? If that were the case and we could have not verified reliably that
the patches indeed fixed the issue, such patches should have never been
accepted in the first place, this is a retroactive effort, the work has been
done already, so you DO want to close it automatically ideally. Of course, no
argument against good old manual handling :).

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Andrea Bolognani 3 years, 2 months ago
On Mon, 2021-02-15 at 11:10 +0100, Peter Krempa wrote:
> On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > After the recent fixes, it's now confirmed to work.
> > 
> > https://gitlab.com/libvirt/libvirt/-/issues/121
> 
> Should it be closed then?

I will close it as soon as this is pushed :)

> > +* **Portability**
> > +
> > +  * Implement Apple Silicon support
> 
> Just semantics, wasn't the problem just in the test suite? In which case
> I'd consider it more of a fix than implementation.

Most of the changes indeed happened in the test suite, but some
tweaks to the aarch64 CPU driver were necessary as well,
specifically:

  https://gitlab.com/libvirt/libvirt/-/commit/82ffb81c9cafbcdf7b1f56f9644883fe8398faa5
  https://gitlab.com/libvirt/libvirt/-/commit/03af15c0242fdb485fc639f24b9acef9ac21599d
  https://gitlab.com/libvirt/libvirt/-/commit/f834c341fbec94ead3671931c58db9c0488b76db

As for whether this should be considered a bug fix, of course the
line between the various release notes sections is not entirely well
defined and there's always some overlap / leeway...

I would consider this a bug fix if we had Apple Silicon support in
the past and broke it, but since the hardware literally didn't exist
until a few months ago, I think it qualifies as a new feature - it
just so happens to be one where we could piggy-back on existing
features almost completely ;)

> Regardless of the above,
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Thanks! Pushed now.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Daniel P. Berrangé 3 years, 2 months ago
On Mon, Feb 15, 2021 at 11:22:18AM +0100, Andrea Bolognani wrote:
> On Mon, 2021-02-15 at 11:10 +0100, Peter Krempa wrote:
> > On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > > After the recent fixes, it's now confirmed to work.
> > > 
> > > https://gitlab.com/libvirt/libvirt/-/issues/121
> > 
> > Should it be closed then?
> 
> I will close it as soon as this is pushed :)
> 
> > > +* **Portability**
> > > +
> > > +  * Implement Apple Silicon support
> > 
> > Just semantics, wasn't the problem just in the test suite? In which case
> > I'd consider it more of a fix than implementation.
> 
> Most of the changes indeed happened in the test suite, but some
> tweaks to the aarch64 CPU driver were necessary as well,
> specifically:
> 
>   https://gitlab.com/libvirt/libvirt/-/commit/82ffb81c9cafbcdf7b1f56f9644883fe8398faa5
>   https://gitlab.com/libvirt/libvirt/-/commit/03af15c0242fdb485fc639f24b9acef9ac21599d
>   https://gitlab.com/libvirt/libvirt/-/commit/f834c341fbec94ead3671931c58db9c0488b76db
> 
> As for whether this should be considered a bug fix, of course the
> line between the various release notes sections is not entirely well
> defined and there's always some overlap / leeway...
> 
> I would consider this a bug fix if we had Apple Silicon support in
> the past and broke it, but since the hardware literally didn't exist
> until a few months ago, I think it qualifies as a new feature - it
> just so happens to be one where we could piggy-back on existing
> features almost completely ;)

Nitpick, for our supported platforms we generally aim to have CI coverage
and that isn't the case for Apple Silicon. So it may or may not work when
the release comes - we could well break it between now and release day.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH] news: Mention Apple Silicon support
Posted by Peter Krempa 3 years, 2 months ago
On Mon, Feb 15, 2021 at 11:22:18 +0100, Andrea Bolognani wrote:
> On Mon, 2021-02-15 at 11:10 +0100, Peter Krempa wrote:
> > On Mon, Feb 15, 2021 at 10:59:47 +0100, Andrea Bolognani wrote:
> > > After the recent fixes, it's now confirmed to work.
> > > 
> > > https://gitlab.com/libvirt/libvirt/-/issues/121
> > 
> > Should it be closed then?
> 
> I will close it as soon as this is pushed :)
> > > +* **Portability**
> > > +
> > > +  * Implement Apple Silicon support
> > 
> > Just semantics, wasn't the problem just in the test suite? In which case
> > I'd consider it more of a fix than implementation.
> 
> Most of the changes indeed happened in the test suite, but some
> tweaks to the aarch64 CPU driver were necessary as well,
> specifically:
> 
>   https://gitlab.com/libvirt/libvirt/-/commit/82ffb81c9cafbcdf7b1f56f9644883fe8398faa5
>   https://gitlab.com/libvirt/libvirt/-/commit/03af15c0242fdb485fc639f24b9acef9ac21599d
>   https://gitlab.com/libvirt/libvirt/-/commit/f834c341fbec94ead3671931c58db9c0488b76db
> 
> As for whether this should be considered a bug fix, of course the
> line between the various release notes sections is not entirely well
> defined and there's always some overlap / leeway...
> 
> I would consider this a bug fix if we had Apple Silicon support in
> the past and broke it, but since the hardware literally didn't exist
> until a few months ago, I think it qualifies as a new feature - it
> just so happens to be one where we could piggy-back on existing
> features almost completely ;)

Yeah, the semantics are very cloudy in this case. The patches clearly
have nothing to do with "implementation" but you are right that we are
making the code support something which it didn't before.