[virt-tools-list] [virt-manager PATCH 4/5] virt-install: add a new guest feature GIC for ARM guests
Pavel Hrdina
phrdina at redhat.com
Sat Jun 11 15:36:16 UTC 2016
On Sat, Jun 11, 2016 at 10:58:17AM -0400, Cole Robinson wrote:
> On 06/10/2016 01:30 PM, Pavel Hrdina wrote:
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1334857
> >
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > man/virt-install.pod | 5 +++
> > .../compare/virt-install-aarch64-kvm-gic.xml | 33 ++++++++++++++++++
> > tests/clitest.py | 1 +
> > virtinst/cli.py | 39 ++++++++++++----------
> > virtinst/guest.py | 16 +++++++++
> > 5 files changed, 77 insertions(+), 17 deletions(-)
> > create mode 100644 tests/cli-test-xml/compare/virt-install-aarch64-kvm-gic.xml
> >
> > diff --git a/man/virt-install.pod b/man/virt-install.pod
> > index 0537693..8054f68 100644
> > --- a/man/virt-install.pod
> > +++ b/man/virt-install.pod
> > @@ -254,6 +254,11 @@ Allow the KVM hypervisor signature to be hidden from the guest
> >
> > Notify the guest that the host supports paravirtual spinlocks for example by exposing the pvticketlocks mechanism.
> >
> > +=item B<--features gic=2>
> > +
> > +This is relevant only for aarch64 architecture. Possible values are "host" or
> > +version number.
> > +
> > =back
> >
> > Use --features=? to see a list of all available sub options. Complete details at L<http://libvirt.org/formatdomain.html#elementsFeatures>
> > diff --git a/tests/cli-test-xml/compare/virt-install-aarch64-kvm-gic.xml b/tests/cli-test-xml/compare/virt-install-aarch64-kvm-gic.xml
> > new file mode 100644
> > index 0000000..189373d
> > --- /dev/null
> > +++ b/tests/cli-test-xml/compare/virt-install-aarch64-kvm-gic.xml
> > @@ -0,0 +1,33 @@
> > +<domain type="kvm">
> > + <name>foobar</name>
> > + <uuid>00000000-1111-2222-3333-444444444444</uuid>
> > + <memory>65536</memory>
> > + <currentMemory>65536</currentMemory>
> > + <vcpu>1</vcpu>
> > + <os>
> > + <type arch="aarch64" machine="virt">hvm</type>
> > + <loader readonly="yes" type="pflash">/usr/share/AAVMF/AAVMF_CODE.fd</loader>
> > + <boot dev="hd"/>
> > + </os>
> > + <features>
> > + <gic version="host"/>
> > + </features>
> > + <cpu mode="host-passthrough"/>
> > + <clock offset="utc"/>
> > + <on_poweroff>destroy</on_poweroff>
> > + <on_reboot>restart</on_reboot>
> > + <on_crash>restart</on_crash>
> > + <devices>
> > + <emulator>/usr/bin/qemu-system-aarch64</emulator>
> > + <interface type="bridge">
> > + <source bridge="eth0"/>
> > + <mac address="00:11:22:33:44:55"/>
> > + <model type="virtio"/>
> > + </interface>
> > + <console type="pty"/>
> > + <channel type="unix">
> > + <source mode="bind"/>
> > + <target type="virtio" name="org.qemu.guest_agent.0"/>
> > + </channel>
> > + </devices>
> > +</domain>
> > diff --git a/tests/clitest.py b/tests/clitest.py
> > index 5d26078..376a17e 100644
> > --- a/tests/clitest.py
> > +++ b/tests/clitest.py
> > @@ -711,6 +711,7 @@ c.add_compare("--arch aarch64 --machine virt --boot kernel=/f19-arm.kernel,initr
> > c.add_compare("--arch aarch64 --boot kernel=/f19-arm.kernel,initrd=/f19-arm.initrd,kernel_args=\"console=ttyAMA0,1234 rw root=/dev/vda3\",extra_args=foo --disk %(EXISTIMG1)s", "aarch64-machdefault")
> > c.add_compare("--arch aarch64 --cdrom %(EXISTIMG2)s --boot loader=CODE.fd,nvram_template=VARS.fd --disk %(EXISTIMG1)s --cpu none --events on_crash=preserve,on_reboot=destroy,on_poweroff=restart", "aarch64-cdrom")
> > c.add_compare("--connect %(URI-KVM-AARCH64)s --disk %(EXISTIMG1)s --import --os-variant fedora21", "aarch64-kvm-import")
> > +c.add_compare("--connect %(URI-KVM-AARCH64)s --disk none --os-variant fedora23 --features gic=host", "aarch64-kvm-gic")
> >
> > # ppc64 tests
> > c.add_compare("--arch ppc64 --machine pseries --boot network --disk %(EXISTIMG1)s --disk device=cdrom --os-variant fedora20 --network none", "ppc64-pseries-f20")
> > diff --git a/virtinst/cli.py b/virtinst/cli.py
> > index 7924b14..1f486b2 100644
> > --- a/virtinst/cli.py
> > +++ b/virtinst/cli.py
> > @@ -54,7 +54,6 @@ from .devicetpm import VirtualTPMDevice
> > from .devicevideo import VirtualVideoDevice
> > from .devicewatchdog import VirtualWatchdog
> > from .domainblkiotune import DomainBlkiotune
> > -from .domainfeatures import DomainFeatures
> > from .domainmemorybacking import DomainMemorybacking
> > from .domainmemorytune import DomainMemorytune
> > from .domainnumatune import DomainNumatune
> > @@ -1518,32 +1517,38 @@ class ParserSecurity(VirtCLIParser):
> >
> > class ParserFeatures(VirtCLIParser):
> > def _init_params(self):
> > - self.objclass = DomainFeatures
> >
>
> This has some subtle side effects, basically disabling virt-xml --features
> clearxml=off, but that could be worked around.
So that's why ParserBoot has 'self.clear_attr = "os"'. I'll add the same for
features:
self.clear_attr = "features"
> > - self.set_param("acpi", "acpi", is_onoff=True)
> > - self.set_param("apic", "apic", is_onoff=True)
> > - self.set_param("pae", "pae", is_onoff=True)
> > - self.set_param("privnet", "privnet",
> > + self.set_param("features.acpi", "acpi", is_onoff=True)
> > + self.set_param("features.apic", "apic", is_onoff=True)
> > + self.set_param("features.pae", "pae", is_onoff=True)
> > + self.set_param("features.privnet", "privnet",
> > is_onoff=True)
> > - self.set_param("hap", "hap",
> > + self.set_param("features.hap", "hap",
> > is_onoff=True)
> > - self.set_param("viridian", "viridian",
> > + self.set_param("features.viridian", "viridian",
> > is_onoff=True)
> > - self.set_param("eoi", "eoi", is_onoff=True)
> > - self.set_param("pmu", "pmu", is_onoff=True)
> > + self.set_param("features.eoi", "eoi", is_onoff=True)
> > + self.set_param("features.pmu", "pmu", is_onoff=True)
> >
> > - self.set_param("hyperv_vapic", "hyperv_vapic",
> > + self.set_param("features.hyperv_vapic", "hyperv_vapic",
> > is_onoff=True)
> > - self.set_param("hyperv_relaxed", "hyperv_relaxed",
> > + self.set_param("features.hyperv_relaxed", "hyperv_relaxed",
> > is_onoff=True)
> > - self.set_param("hyperv_spinlocks", "hyperv_spinlocks",
> > + self.set_param("features.hyperv_spinlocks", "hyperv_spinlocks",
> > is_onoff=True)
> > - self.set_param("hyperv_spinlocks_retries",
> > + self.set_param("features.hyperv_spinlocks_retries",
> > "hyperv_spinlocks_retries")
> >
> > - self.set_param("vmport", "vmport", is_onoff=True)
> > - self.set_param("kvm_hidden", "kvm_hidden", is_onoff=True)
> > - self.set_param("pvspinlock", "pvspinlock", is_onoff=True)
> > + self.set_param("features.vmport", "vmport", is_onoff=True)
> > + self.set_param("features.kvm_hidden", "kvm_hidden", is_onoff=True)
> > + self.set_param("features.pvspinlock", "pvspinlock", is_onoff=True)
> > +
> > + def set_gic(opts, inst, cliname, val):
> > + ignore = opts
> > + ignore = cliname
> > + inst.set_gic_version(val)
> > +
> > + self.set_param(None, "gic", setter_cb=set_gic)
> >
> >
> > ###################
> > diff --git a/virtinst/guest.py b/virtinst/guest.py
> > index 490b90b..544a65a 100644
> > --- a/virtinst/guest.py
> > +++ b/virtinst/guest.py
> > @@ -587,6 +587,22 @@ class Guest(XMLBuilder):
> > self.os.loader = path
> >
> >
> > + def set_gic_version(self, val):
> > + """
> > + Checks and sets GIC version for ARM guest.
> > + """
> > +
> > + domcaps = DomainCapabilities.build_from_guest(self)
> > +
> > + if not domcaps.supports_gic(self):
> > + raise RuntimeError(_("GIC is not supported."))
> > +
> > + if val != "host" and val not in domcaps.get_gic_versions():
> > + raise RuntimeError(_("GIC version '%s' is not supported") % val)
> > +
> > + self.features.gic_version = val
> > +
> > +
>
> Do we actually need this validation here? Isn't libvirt going to throw us
> these same errors once the XML is defined?
Unfortunately not, libvirt allows to create a domain with invalid gic version
but fails to start such domain. The only validation what libvirt does is
against XML schema where are all known gic versions, but that doesn't validate
whether the host supports that gic version or not.
Thanks,
Pavel
More information about the virt-tools-list
mailing list