[libvirt] [PATCH] Remove phyp driver

[Cole Robinson]

On 12/18/19 2:50 PM, Michal Prívozník wrote:
> [I'm CCing Eduardo, who worked a lot on this driver in the past. Maybe
> he knows if this hypervisor is still alive.]
> 
> On 12/18/19 4:43 PM, Cole Robinson wrote:
>> The phyp driver was added in 2009 and does not appear to have had any
>> real feature change since 2011. There's virtually no evidence online
>> of users actually using it. IMO it's time to kill it.
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>> I raised this in 3.5 years ago:
>> https://www.redhat.com/archives/libvir-list/2016-April/msg01060.html
>>
>> Not much on phyp/ side has changed since then, except dozens of dev
>> patches transitioning the code forward.
>>
>> That mail also mentions xenapi and hyperv. hyperv saw signs of life
>> afterwards and is still around. xenapi has been removed, along with uml.
>>
>> Considering the amount of code transitions we are currently undergoing
>> (gnulib, glib, memory auto cleanup, etc), phyp/ will probably have an
>> uptick of dev energy in the medium term. Let's bite the bullet and
>> remove it!
>>
>>  docs/aclpolkit.html.in        |    4 -
>>  docs/api.html.in              |    2 +-
>>  docs/drivers.html.in          |    1 -
>>  docs/drvphyp.html.in          |   50 -
>>  docs/schemas/capability.rng   |    3 +-
>>  docs/schemas/domaincommon.rng |    2 +-
>>  libvirt.spec.in               |   11 +-
>>  m4/virt-driver-phyp.m4        |   48 -
>>  mingw-libvirt.spec.in         |    7 -
>>  po/POTFILES.in                |    1 -
>>  src/Makefile.am               |    1 -
>>  src/README                    |    1 -
>>  src/libvirt.c                 |   10 -
>>  src/phyp/Makefile.inc.am      |   21 -
>>  src/phyp/phyp_driver.c        | 3739 ---------------------------------
>>  src/phyp/phyp_driver.h        |   24 -
>>  16 files changed, 4 insertions(+), 3921 deletions(-)
>>  delete mode 100644 docs/drvphyp.html.in
>>  delete mode 100644 m4/virt-driver-phyp.m4
>>  delete mode 100644 src/phyp/Makefile.inc.am
>>  delete mode 100644 src/phyp/phyp_driver.c
>>  delete mode 100644 src/phyp/phyp_driver.h
> 
> Apart from what you proposed to squash in, I'd add/change the following:
> 
> diff --git i/include/libvirt/virterror.h w/include/libvirt/virterror.h
> index 685e171235..7c7e5fd145 100644
> --- i/include/libvirt/virterror.h
> +++ w/include/libvirt/virterror.h
> @@ -84,7 +84,7 @@ typedef enum {
>      VIR_FROM_ONE = 27,          /* The OpenNebula driver no longer exists.
>                                     Retained for ABI/API compat only */
>      VIR_FROM_ESX = 28,          /* Error from ESX driver */
> -    VIR_FROM_PHYP = 29,         /* Error from IBM power hypervisor */
> +    VIR_FROM_PHYP = 29,         /* Error from IBM power hypervisor;
> unused since 6.0.0 */
> 

I'll add this, thanks

>      VIR_FROM_SECRET = 30,       /* Error from secret storage */
>      VIR_FROM_CPU = 31,          /* Error from CPU driver */
> diff --git i/src/util/virhostcpu.c w/src/util/virhostcpu.c
> index 22102f2c75..f17a888638 100644
> --- i/src/util/virhostcpu.c
> +++ w/src/util/virhostcpu.c
> @@ -650,7 +650,7 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
>       * If the user tampers the cpu online/offline states using chcpu or
> other
>       * means, then it is an unsupported configuration for kvm.
>       * The code below tries to keep in mind
> -     *  - when the libvirtd is run inside a KVM guest or Phyp based guest.
> +     *  - when the libvirtd is run inside a KVM guest.
>       *  - Or on the kvm host where user manually tampers the cpu states to
>       *    offline/online randomly.
>       * On hosts other than POWER this will be 0, in which case a simpler
> @@ -1133,7 +1133,7 @@ virHostCPUGetThreadsPerSubcore(virArch arch)
>              goto out;
>          }
> 
> -        /* For Phyp and KVM based guests the ioctl for KVM_CAP_PPC_SMT
> +        /* For KVM based guests the ioctl for KVM_CAP_PPC_SMT
>           * returns zero and both primary and secondary threads will be
>           * online */
>          threads_per_subcore = ioctl(kvmfd,
> 

These references look to be about CPU topology inside a Phyp VM, but
independent of libvirt phyp driver. So they are still relevant. I'll
leave them, but we can remove in a follow up patch if I'm wrong

> 
> And also, I'd mention in news.xml that the driver was removed.
> 

I'll send a separate patch for that

Thanks,
Cole