[libvirt] [PATCH 0/9] qemu: Support qemu-system-arm vexpress-a9
This series adds the bits needed to kick of a qemu-system-arm -machine vexpress-a9 guest. Patches 1 and 8 are isolated bug fixes Patches 2-5 and 7 are mostly about fixing CLI generation. Unfortunately qemu ARM boards don't quite work like x86 where we can mix and match devices, so -device is out of the picture, and the boards depend on old CLI infrastrucure like -net nic and -serial. Patch 6 adds disk bus=sd, which is often the only way to specify storage for ARM boards. Patch 9 adds virtio-mmio address support, which enables virtio for vexpress-a9. Cole Robinson (9): qemu_command: Drop incorrect ATTRIBUTE_UNUSED annotation domain_conf: Set QEMU ARM default USB model to 'none' domain_conf: Don't add default memballoon device on ARM qemu: Fix adding specifying char devs for ARM qemu: Don't try to allocate PCI addresses for ARM domain_conf: Add disk bus=sd, wire it up for qemu qemu: Fix networking for ARM guests qemu: Only setup vhost if virtType == "kvm" qemu: Support virtio-mmio transport for virtio on ARM docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 20 ++++ src/conf/domain_conf.c | 72 +++++++++--- src/conf/domain_conf.h | 2 + src/qemu/qemu_capabilities.c | 22 ++++ src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_command.c | 126 +++++++++++++++++---- src/qemu/qemu_domain.c | 20 +++- src/qemu/qemu_process.c | 37 +++--- .../qemuxml2argv-arm-vexpressa9-basic.args | 1 + .../qemuxml2argv-arm-vexpressa9-basic.xml | 34 ++++++ .../qemuxml2argv-arm-vexpressa9-nodevs.args | 1 + .../qemuxml2argv-arm-vexpressa9-nodevs.xml | 26 +++++ .../qemuxml2argv-arm-vexpressa9-virtio.args | 1 + .../qemuxml2argv-arm-vexpressa9-virtio.xml | 45 ++++++++ tests/qemuxml2argvtest.c | 10 ++ tests/testutilsqemu.c | 32 ++++++ 17 files changed, 397 insertions(+), 60 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml -- 1.8.3.1
--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa3a2fd..d924110 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1439,7 +1439,7 @@ struct _qemuDomainPCIAddressSet { * with the specified PCI address set. */ static bool -qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, +qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, qemuDomainPCIConnectFlags flags) { -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa3a2fd..d924110 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1439,7 +1439,7 @@ struct _qemuDomainPCIAddressSet { * with the specified PCI address set. */ static bool -qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, +qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, qemuDomainPCIConnectFlags flags) {
This is already included in a larger patch that refactors this function (as well as renaming it) which I'm about to post. I'd rather we pushed mine.
On 08/01/2013 02:19 AM, Laine Stump wrote:
On 07/31/2013 10:14 PM, Cole Robinson wrote:
--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa3a2fd..d924110 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1439,7 +1439,7 @@ struct _qemuDomainPCIAddressSet { * with the specified PCI address set. */ static bool -qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, +qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, qemuDomainPCIConnectFlags flags) { This is already included in a larger patch that refactors this function (as well as renaming it) which I'm about to post. I'd rather we pushed mine.
Here is the larger patch I was referring to: https://www.redhat.com/archives/libvir-list/2013-August/msg00142.html
Preferably what we'd do is not add any USB controller by default, but that goes against how the QEMU driver has historically acted for other architectures, so let's be consistent. --- src/conf/domain_conf.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3aec69..2308580 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8801,6 +8801,16 @@ virDomainVideoDefaultRAM(virDomainDefPtr def, } } +static int +virDomainDefaultUSBControllerModel(virDomainDefPtr def) +{ + /* Not entirely true, some ARM boards actually can support USB, + * and even versatilepb supports the pci-ohci + */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + return -1; +} int virDomainVideoDefaultType(virDomainDefPtr def) @@ -11720,9 +11730,13 @@ virDomainDefParseXML(xmlDocPtr xml, if (def->virtType == VIR_DOMAIN_VIRT_QEMU || def->virtType == VIR_DOMAIN_VIRT_KQEMU || - def->virtType == VIR_DOMAIN_VIRT_KVM) - if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0) + def->virtType == VIR_DOMAIN_VIRT_KVM) { + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_USB, + 0, + virDomainDefaultUSBControllerModel(def)) < 0) goto error; + } /* analysis of the resource leases */ if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) { -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
Preferably what we'd do is not add any USB controller by default, but that goes against how the QEMU driver has historically acted for other architectures, so let's be consistent. --- src/conf/domain_conf.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3aec69..2308580 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8801,6 +8801,16 @@ virDomainVideoDefaultRAM(virDomainDefPtr def, } }
+static int +virDomainDefaultUSBControllerModel(virDomainDefPtr def) +{ + /* Not entirely true, some ARM boards actually can support USB, + * and even versatilepb supports the pci-ohci + */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + return -1; +}
We've been trying to put arch-specific stuff like this in the post-parse callbacks that are defined in the hypervisors (e.g. qemuDomainDefPostParse(), qemuDomainDeviceDefPostParse(), and bits in qemu_command.c) rather than in the parser itself.
int virDomainVideoDefaultType(virDomainDefPtr def) @@ -11720,9 +11730,13 @@ virDomainDefParseXML(xmlDocPtr xml,
if (def->virtType == VIR_DOMAIN_VIRT_QEMU || def->virtType == VIR_DOMAIN_VIRT_KQEMU || - def->virtType == VIR_DOMAIN_VIRT_KVM) - if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0)
I'm thinking that if this needs to be different for different arches, then it shouldn't be done here, it should be done in the hypervisor's post-parse callback.
+ def->virtType == VIR_DOMAIN_VIRT_KVM) { + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_USB, + 0, + virDomainDefaultUSBControllerModel(def)) < 0) goto error; + }
/* analysis of the resource leases */ if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) {
On 08/01/2013 02:30 AM, Laine Stump wrote:
On 07/31/2013 10:14 PM, Cole Robinson wrote:
Preferably what we'd do is not add any USB controller by default, but that goes against how the QEMU driver has historically acted for other architectures, so let's be consistent. --- src/conf/domain_conf.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3aec69..2308580 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8801,6 +8801,16 @@ virDomainVideoDefaultRAM(virDomainDefPtr def, } }
+static int +virDomainDefaultUSBControllerModel(virDomainDefPtr def) +{ + /* Not entirely true, some ARM boards actually can support USB, + * and even versatilepb supports the pci-ohci + */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; + return -1; +}
We've been trying to put arch-specific stuff like this in the post-parse callbacks that are defined in the hypervisors (e.g. qemuDomainDefPostParse(), qemuDomainDeviceDefPostParse(), and bits in qemu_command.c) rather than in the parser itself.
int virDomainVideoDefaultType(virDomainDefPtr def) @@ -11720,9 +11730,13 @@ virDomainDefParseXML(xmlDocPtr xml,
if (def->virtType == VIR_DOMAIN_VIRT_QEMU || def->virtType == VIR_DOMAIN_VIRT_KQEMU || - def->virtType == VIR_DOMAIN_VIRT_KVM) - if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0) I'm thinking that if this needs to be different for different arches, then it shouldn't be done here, it should be done in the hypervisor's post-parse callback.
As done in this patch: https://www.redhat.com/archives/libvir-list/2013-August/msg00141.html It accomplishes the same thing (I added in armvl7 to it so you don't have to add anything on).
Unlike USB, qemu_command can handle an empty memballoon device, so just don't add it. And add test cases for a basic working ARM guest. --- docs/schemas/domaincommon.rng | 19 +++++++++++ src/conf/domain_conf.c | 38 ++++++++++++++-------- .../qemuxml2argv-arm-vexpressa9-nodevs.args | 1 + .../qemuxml2argv-arm-vexpressa9-nodevs.xml | 26 +++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ tests/testutilsqemu.c | 32 ++++++++++++++++++ 6 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 745b959..781ecfd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -303,6 +303,7 @@ <ref name="hvmppc"/> <ref name="hvmppc64"/> <ref name="hvms390"/> + <ref name="hvmarm"/> </choice> </optional> <value>hvm</value> @@ -412,6 +413,24 @@ </optional> </group> </define> + <define name="hvmarm"> + <group> + <optional> + <attribute name="arch"> + <choice> + <value>armv7l</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="machine"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9_\.\-]+</param> + </data> + </attribute> + </optional> + </group> + </define> <define name="osexe"> <element name="os"> <element name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2308580..9d903f4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8812,6 +8812,23 @@ virDomainDefaultUSBControllerModel(virDomainDefPtr def) return -1; } +static bool +virDomainNeedDefaultMemballoonDevice(virDomainDefPtr def) +{ + if (def->virtType == VIR_DOMAIN_VIRT_XEN) + return true; + + if (def->virtType != VIR_DOMAIN_VIRT_QEMU && + def->virtType != VIR_DOMAIN_VIRT_KQEMU && + def->virtType != VIR_DOMAIN_VIRT_KVM) + return false; + + if (def->os.arch == VIR_ARCH_ARMV7L) + return false; + + return true; +} + int virDomainVideoDefaultType(virDomainDefPtr def) { @@ -12156,19 +12173,14 @@ virDomainDefParseXML(xmlDocPtr xml, def->memballoon = memballoon; VIR_FREE(nodes); - } else { - if (def->virtType == VIR_DOMAIN_VIRT_XEN || - def->virtType == VIR_DOMAIN_VIRT_QEMU || - def->virtType == VIR_DOMAIN_VIRT_KQEMU || - def->virtType == VIR_DOMAIN_VIRT_KVM) { - virDomainMemballoonDefPtr memballoon; - if (VIR_ALLOC(memballoon) < 0) - goto error; - memballoon->model = def->virtType == VIR_DOMAIN_VIRT_XEN ? - VIR_DOMAIN_MEMBALLOON_MODEL_XEN : - VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; - def->memballoon = memballoon; - } + } else if (virDomainNeedDefaultMemballoonDevice(def)) { + virDomainMemballoonDefPtr memballoon; + if (VIR_ALLOC(memballoon) < 0) + goto error; + memballoon->model = def->virtType == VIR_DOMAIN_VIRT_XEN ? + VIR_DOMAIN_MEMBALLOON_MODEL_XEN : + VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; + def->memballoon = memballoon; } /* Parse the RNG device */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args new file mode 100644 index 0000000..129f2ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append console=ttyAMA0,115200n8 -dtb /arm-dtb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml new file mode 100644 index 0000000..6a97e98 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml @@ -0,0 +1,26 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/arm-dtb</dtb> + <cmdline>console=ttyAMA0,115200n8</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b7485fc..361ddb8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1027,6 +1027,9 @@ mymain(void) DO_TEST_PARSE_ERROR("pci-root-address", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("arm-vexpressa9-nodevs", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index fac83b2..3907683 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -145,6 +145,35 @@ error: return -1; } +static int testQemuAddArmGuest(virCapsPtr caps) +{ + static const char *machines[] = { "vexpress-a9", + "versatilepb" }; + virCapsGuestMachinePtr *capsmachines = NULL; + virCapsGuestPtr guest; + + capsmachines = virCapabilitiesAllocMachines(machines, + ARRAY_CARDINALITY(machines)); + if (!capsmachines) + goto error; + + guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_ARMV7L, + "/usr/bin/qemu-system-arm", NULL, + ARRAY_CARDINALITY(machines), + capsmachines); + if (!guest) + goto error; + + if (!virCapabilitiesAddGuestDomain(guest, "qemu", NULL, NULL, 0, NULL)) + goto error; + + return 0; + +error: + virCapabilitiesFreeMachines(capsmachines, ARRAY_CARDINALITY(machines)); + return -1; +} + virCapsPtr testQemuCapsInit(void) { virCapsPtr caps; @@ -270,6 +299,9 @@ virCapsPtr testQemuCapsInit(void) { if (testQemuAddS390Guest(caps)) goto cleanup; + if (testQemuAddArmGuest(caps)) + goto cleanup; + if (virTestGetDebug()) { char *caps_str; -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
Unlike USB, qemu_command can handle an empty memballoon device, so just don't add it.
And add test cases for a basic working ARM guest. --- docs/schemas/domaincommon.rng | 19 +++++++++++ src/conf/domain_conf.c | 38 ++++++++++++++-------- .../qemuxml2argv-arm-vexpressa9-nodevs.args | 1 + .../qemuxml2argv-arm-vexpressa9-nodevs.xml | 26 +++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ tests/testutilsqemu.c | 32 ++++++++++++++++++ 6 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 745b959..781ecfd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -303,6 +303,7 @@ <ref name="hvmppc"/> <ref name="hvmppc64"/> <ref name="hvms390"/> + <ref name="hvmarm"/> </choice> </optional> <value>hvm</value> @@ -412,6 +413,24 @@ </optional> </group> </define> + <define name="hvmarm"> + <group> + <optional> + <attribute name="arch"> + <choice> + <value>armv7l</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="machine"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9_\.\-]+</param> + </data> + </attribute> + </optional> + </group> + </define> <define name="osexe"> <element name="os"> <element name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2308580..9d903f4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8812,6 +8812,23 @@ virDomainDefaultUSBControllerModel(virDomainDefPtr def) return -1; }
+static bool +virDomainNeedDefaultMemballoonDevice(virDomainDefPtr def) +{ + if (def->virtType == VIR_DOMAIN_VIRT_XEN) + return true; + + if (def->virtType != VIR_DOMAIN_VIRT_QEMU && + def->virtType != VIR_DOMAIN_VIRT_KQEMU && + def->virtType != VIR_DOMAIN_VIRT_KVM) + return false; + + if (def->os.arch == VIR_ARCH_ARMV7L) + return false; + + return true; +} +
Same comment applies to this as to 2/9 - arch-specific differences should go in the post-parse callback rather than in the parser itself.
int virDomainVideoDefaultType(virDomainDefPtr def) { @@ -12156,19 +12173,14 @@ virDomainDefParseXML(xmlDocPtr xml,
def->memballoon = memballoon; VIR_FREE(nodes); - } else { - if (def->virtType == VIR_DOMAIN_VIRT_XEN || - def->virtType == VIR_DOMAIN_VIRT_QEMU || - def->virtType == VIR_DOMAIN_VIRT_KQEMU || - def->virtType == VIR_DOMAIN_VIRT_KVM) { - virDomainMemballoonDefPtr memballoon; - if (VIR_ALLOC(memballoon) < 0) - goto error; - memballoon->model = def->virtType == VIR_DOMAIN_VIRT_XEN ? - VIR_DOMAIN_MEMBALLOON_MODEL_XEN : - VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; - def->memballoon = memballoon; - } + } else if (virDomainNeedDefaultMemballoonDevice(def)) { + virDomainMemballoonDefPtr memballoon; + if (VIR_ALLOC(memballoon) < 0) + goto error; + memballoon->model = def->virtType == VIR_DOMAIN_VIRT_XEN ? + VIR_DOMAIN_MEMBALLOON_MODEL_XEN : + VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO; + def->memballoon = memballoon;
Yeah, someone else may disagree, but I think all of this should be in the hypervisor. The parser should only translate what's in the XML directly into the config object without trying to infer things into it based on the hypervisor type or arch. I realize that you're just modifying similar code that was already there, but we've been trying to move this type of code out of the parser rather than add more of it.
}
/* Parse the RNG device */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args new file mode 100644 index 0000000..129f2ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append console=ttyAMA0,115200n8 -dtb /arm-dtb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml new file mode 100644 index 0000000..6a97e98 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-nodevs.xml @@ -0,0 +1,26 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/arm-dtb</dtb> + <cmdline>console=ttyAMA0,115200n8</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b7485fc..361ddb8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1027,6 +1027,9 @@ mymain(void) DO_TEST_PARSE_ERROR("pci-root-address", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
+ DO_TEST("arm-vexpressa9-nodevs", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index fac83b2..3907683 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -145,6 +145,35 @@ error: return -1; }
+static int testQemuAddArmGuest(virCapsPtr caps) +{ + static const char *machines[] = { "vexpress-a9", + "versatilepb" }; + virCapsGuestMachinePtr *capsmachines = NULL; + virCapsGuestPtr guest; + + capsmachines = virCapabilitiesAllocMachines(machines, + ARRAY_CARDINALITY(machines)); + if (!capsmachines) + goto error; + + guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_ARMV7L, + "/usr/bin/qemu-system-arm", NULL, + ARRAY_CARDINALITY(machines), + capsmachines); + if (!guest) + goto error; + + if (!virCapabilitiesAddGuestDomain(guest, "qemu", NULL, NULL, 0, NULL)) + goto error; + + return 0; + +error: + virCapabilitiesFreeMachines(capsmachines, ARRAY_CARDINALITY(machines)); + return -1; +} +
virCapsPtr testQemuCapsInit(void) { virCapsPtr caps; @@ -270,6 +299,9 @@ virCapsPtr testQemuCapsInit(void) { if (testQemuAddS390Guest(caps)) goto cleanup;
+ if (testQemuAddArmGuest(caps)) + goto cleanup; + if (virTestGetDebug()) { char *caps_str;
QEMU ARM boards don't give us any way to explicitly wire in a -chardev, so use the old style -serial options. --- src/qemu/qemu_capabilities.c | 18 ++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_process.c | 37 ++++++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08406b8..51064e8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2810,3 +2810,21 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps) { return qemuCaps->usedQMP; } + +bool +virQEMUCapsSupportsChardev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainChrDefPtr chr ATTRIBUTE_UNUSED) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + return false; + + /* This may not be true for all machine types, but at least + * the only supported serial devices of vexpress-a9 and versatilepb + * don't have the chardev property wired up */ + if (def->os.arch != VIR_ARCH_ARMV7L) + return false; + + return true; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f5f685d..56f8405 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -272,4 +272,8 @@ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); VIR_ENUM_DECL(virQEMUCaps); bool virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps); +bool virQEMUCapsSupportsChardev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainChrDefPtr chr); + #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d924110..4209e5e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7972,8 +7972,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *devstr; /* Use -chardev with -device if they are available */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&serial->source, serial->info.alias, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d631a6f..57036e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1583,22 +1583,25 @@ qemuProcessExtractTTYPath(const char *haystack, } static int -qemuProcessLookupPTYs(virDomainChrDefPtr *devices, +qemuProcessLookupPTYs(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainChrDefPtr *devices, int count, - virHashTablePtr paths, - bool chardevfmt) + virHashTablePtr paths) { size_t i; - const char *prefix = chardevfmt ? "char" : ""; for (i = 0; i < count; i++) { virDomainChrDefPtr chr = devices[i]; + bool chardevfmt = virQEMUCapsSupportsChardev(def, qemuCaps, chr); + if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { char id[32]; const char *path; if (snprintf(id, sizeof(id), "%s%s", - prefix, chr->info.alias) >= sizeof(id)) + chardevfmt ? "char" : "", + chr->info.alias) >= sizeof(id)) return -1; path = (const char *) virHashLookup(paths, id); @@ -1632,19 +1635,21 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, virHashTablePtr paths) { - bool chardevfmt = virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV); size_t i = 0; - if (qemuProcessLookupPTYs(vm->def->serials, vm->def->nserials, - paths, chardevfmt) < 0) + if (qemuProcessLookupPTYs(vm->def, qemuCaps, + vm->def->serials, vm->def->nserials, + paths) < 0) return -1; - if (qemuProcessLookupPTYs(vm->def->parallels, vm->def->nparallels, - paths, chardevfmt) < 0) + if (qemuProcessLookupPTYs(vm->def, qemuCaps, + vm->def->parallels, vm->def->nparallels, + paths) < 0) return -1; - if (qemuProcessLookupPTYs(vm->def->channels, vm->def->nchannels, - paths, chardevfmt) < 0) + if (qemuProcessLookupPTYs(vm->def, qemuCaps, + vm->def->channels, vm->def->nchannels, + paths) < 0) return -1; /* For historical reasons, console[0] can be just an alias * for serial[0]. That's why we need to update it as well. */ @@ -1662,8 +1667,9 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, } } - if (qemuProcessLookupPTYs(vm->def->consoles + i, vm->def->nconsoles - i, - paths, chardevfmt) < 0) + if (qemuProcessLookupPTYs(vm->def, qemuCaps, + vm->def->consoles + i, vm->def->nconsoles - i, + paths) < 0) return -1; return 0; @@ -1753,7 +1759,8 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, virHashTablePtr paths = NULL; qemuDomainObjPrivatePtr priv; - if (!virQEMUCapsUsedQMP(qemuCaps) && pos != -1) { + if (!virQEMUCapsUsedQMP(qemuCaps) + && pos != -1) { if ((logfd = qemuDomainOpenLog(driver, vm, pos)) < 0) return -1; -- 1.8.3.1
On Wed, Jul 31, 2013 at 10:14:34PM -0400, Cole Robinson wrote:
QEMU ARM boards don't give us any way to explicitly wire in a -chardev, so use the old style -serial options. --- src/qemu/qemu_capabilities.c | 18 ++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_process.c | 37 ++++++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08406b8..51064e8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2810,3 +2810,21 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps) { return qemuCaps->usedQMP; } + +bool +virQEMUCapsSupportsChardev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainChrDefPtr chr ATTRIBUTE_UNUSED) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + return false; + + /* This may not be true for all machine types, but at least + * the only supported serial devices of vexpress-a9 and versatilepb + * don't have the chardev property wired up */ + if (def->os.arch != VIR_ARCH_ARMV7L) + return false; + + return true; +}
Shouldn't we just avoid setting QEMU_CAPS_CHARDEV when probing capabilities for the QEMU ARM binary, so we don't need to special case the rest of the code. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/qemu/qemu_command.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4209e5e..6da35d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1692,6 +1692,16 @@ cleanup: return ret; } +static bool +qemuDomainSupportsPCI(virDomainDefPtr def) { + if (def->os.arch != VIR_ARCH_ARMV7L) + return true; + + if (STREQ(def->os.machine, "versatilepb")) + return true; + + return false; +} int qemuDomainAssignPCIAddresses(virDomainDefPtr def, @@ -1757,8 +1767,10 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) goto cleanup; - if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) - goto cleanup; + if (qemuDomainSupportsPCI(def)) { + if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) + goto cleanup; + } } if (obj && obj->privateData) { -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
--- src/qemu/qemu_command.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4209e5e..6da35d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1692,6 +1692,16 @@ cleanup: return ret; }
+static bool +qemuDomainSupportsPCI(virDomainDefPtr def) { + if (def->os.arch != VIR_ARCH_ARMV7L) + return true; + + if (STREQ(def->os.machine, "versatilepb")) + return true; + + return false; +}
int qemuDomainAssignPCIAddresses(virDomainDefPtr def, @@ -1757,8 +1767,10 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) goto cleanup;
- if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) - goto cleanup; + if (qemuDomainSupportsPCI(def)) { + if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
(Just combine those in the same if using &&)
+ goto cleanup; + }
I think what may be a more general solution would be to note for each device what kind of bus it needs to be attached for each machine type (if any). Still, as long as completely skipping the attempt to add PCI addresses to various devices will still result in a reasonable error message when someone tries to add a device that wouldn't work on this arch due to it requiring a PCI address, this patch seems to be a good "whack it off at the roots" type of solution. ACK (but it would be nice to flesh out the list of machines with no PCI bus, e.g. I believe the s390 has no PCI bus. yet we even have a unit test for the s390 that includes a PCI address in the XML !)
}
if (obj && obj->privateData) {
This corresponds to '-sd' and '-drive if=sd' on the qemu command line. Needed for many ARM boards which don't provide any other way to pass in storage. --- docs/formatdomain.html.in | 3 ++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 4 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 20 +++++++++++++++----- 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 49c7c8d..c7ea808 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1660,7 +1660,8 @@ as a device ordering hint. The optional <code>bus</code> attribute specifies the type of disk device to emulate; possible values are driver specific, with typical values being - "ide", "scsi", "virtio", "xen", "usb" or "sata". If omitted, the bus + "ide", "scsi", "virtio", "xen", "usb", "sata", or + "sd" <span class="since">"sd" since 1.1.2</span>. If omitted, the bus type is inferred from the style of the device name (e.g. a device named 'sda' will typically be exported using a SCSI bus). The optional attribute <code>tray</code> indicates the tray status of the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 781ecfd..4f4564b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1286,6 +1286,7 @@ <value>usb</value> <value>uml</value> <value>sata</value> + <value>sd</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d903f4..5e423cc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -239,7 +239,8 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST, "xen", "usb", "uml", - "sata") + "sata", + "sd") VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, "default", @@ -17273,6 +17274,7 @@ virDiskNameToBusDeviceIndex(const virDomainDiskDefPtr disk, case VIR_DOMAIN_DISK_BUS_USB: case VIR_DOMAIN_DISK_BUS_VIRTIO: case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_SD: default: *busIdx = 0; *devIdx = idx; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index de3b59c..978be16 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -508,6 +508,7 @@ enum virDomainDiskBus { VIR_DOMAIN_DISK_BUS_USB, VIR_DOMAIN_DISK_BUS_UML, VIR_DOMAIN_DISK_BUS_SATA, + VIR_DOMAIN_DISK_BUS_SD, VIR_DOMAIN_DISK_BUS_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6da35d0..248e4b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -73,7 +73,8 @@ VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, "xen", "usb", "uml", - "sata") + "sata", + "sd") VIR_ENUM_DECL(qemuDiskCacheV1) @@ -644,6 +645,9 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) case VIR_DOMAIN_DISK_BUS_XEN: ret = virAsprintf(&dev_name, "xenblk%d", devid); break; + case VIR_DOMAIN_DISK_BUS_SD: + ret = virAsprintf(&dev_name, "sd%d", devid); + break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported disk name mapping for bus '%s'"), @@ -3386,7 +3390,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, break; case VIR_DOMAIN_DISK_BUS_XEN: - /* Xen has no address type currently, so assign based on index */ + case VIR_DOMAIN_DISK_BUS_SD: + /* Xen and SD have no address type currently, so assign + * based on index */ break; } @@ -7669,12 +7675,13 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-drive"); /* Unfortunately it is not possible to use - -device for floppies, or Xen paravirt + -device for floppies, xen PV, or SD devices. Fortunately, those don't need static PCI addresses, so we don't really care that we can't use -device */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN && + disk->bus != VIR_DOMAIN_DISK_BUS_SD) { withDeviceArg = true; } else { virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); @@ -9358,6 +9365,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; else if (STREQ(values[i], "xen")) def->bus = VIR_DOMAIN_DISK_BUS_XEN; + else if (STREQ(values[i], "sd")) + def->bus = VIR_DOMAIN_DISK_BUS_SD; } else if (STREQ(keywords[i], "media")) { if (STREQ(values[i], "cdrom")) { def->device = VIR_DOMAIN_DISK_DEVICE_CDROM; @@ -9507,7 +9516,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (def->bus == VIR_DOMAIN_DISK_BUS_IDE) { ignore_value(VIR_STRDUP(def->dst, "hda")); - } else if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + } else if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI || + def->bus == VIR_DOMAIN_DISK_BUS_SD) { ignore_value(VIR_STRDUP(def->dst, "sda")); } else if (def->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { ignore_value(VIR_STRDUP(def->dst, "vda")); -- 1.8.3.1
Similar to the chardev bit, ARM boards depend on the old style '-net nic' for actually instantiating net devices. And add tests for working ARM XML with console, disk, and networking. --- src/qemu/qemu_command.c | 34 ++++++++++++++++------ src/qemu/qemu_domain.c | 20 +++++++++++-- .../qemuxml2argv-arm-vexpressa9-basic.args | 1 + .../qemuxml2argv-arm-vexpressa9-basic.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 248e4b4..3b275e3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -417,6 +417,26 @@ cleanup: return ret; } +static bool +qemuDomainSupportsNicdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + return false; + + /* arm boards require legacy -net nic */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return false; + + return true; +} + +static bool +qemuDomainSupportsNetdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!qemuDomainSupportsNicdev(def, qemuCaps)) + return false; + return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); +} /** * qemuOpenVhostNet: @@ -452,8 +472,7 @@ qemuOpenVhostNet(virDomainDefPtr def, * option), don't try to open the device. */ if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOST_NET) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { + qemuDomainSupportsNetdev(def, qemuCaps))) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with " @@ -6852,8 +6871,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, * * NB, no support for -netdev without use of -device */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuDomainSupportsNetdev(def, qemuCaps)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -6861,7 +6879,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuDomainSupportsNicdev(def, qemuCaps)) { if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); @@ -6870,8 +6888,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-net", nic, NULL); } - if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { + if (!qemuDomainSupportsNetdev(def, qemuCaps)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -7864,8 +7881,7 @@ qemuBuildCommandLine(virConnectPtr conn, int vlan; /* VLANs are not used with -netdev, so don't record them */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + if (qemuDomainSupportsNetdev(def, qemuCaps)) vlan = -1; else vlan = i; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da3b768..c868dc1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -746,6 +746,23 @@ qemuDomainDefPostParse(virDomainDefPtr def, return 0; } +static const char * +qemuDomainDefaultNetModel(virDomainDefPtr def) { + if (def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) + return "virtio"; + + if (def->os.arch == VIR_ARCH_ARMV7L) { + if (STREQ(def->os.machine, "versatilepb")) + return "smc91c111"; + + /* Incomplete. vexpress-a9 (and a few others) use this, but not all + * arm boards */ + return "lan9118"; + } + + return "rtl8139"; +} static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, @@ -761,8 +778,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && !dev->data.net->model) { if (VIR_STRDUP(dev->data.net->model, - def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X ? "virtio" : "rtl8139") < 0) + qemuDomainDefaultNetModel(def)) < 0) goto cleanup; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args new file mode 100644 index 0000000..98ca5c9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append 'console=ttyAMA0,115200n8 rw root=/dev/mmcblk0p3 rootwait physmap.enabled=0' -dtb /arm-dtb -drive file=/arm.raw,if=sd,index=0 -net nic,macaddr=52:54:00:09:a4:37,vlan=0,model=lan9118,name=net0 -net user,vlan=0,name=hostnet0 -serial pty diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml new file mode 100644 index 0000000..7b846c6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml @@ -0,0 +1,34 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/arm-dtb</dtb> + <cmdline>console=ttyAMA0,115200n8 rw root=/dev/mmcblk0p3 rootwait physmap.enabled=0</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + <disk type='file' device='disk'> + <source file='/arm.raw'/> + <target dev='sda' bus='sd'/> + </disk> + <interface type='user'> + <mac address='52:54:00:09:a4:37'/> + </interface> + <console type='pty'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 361ddb8..0bf2724 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1029,6 +1029,9 @@ mymain(void) DO_TEST("arm-vexpressa9-nodevs", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB); + DO_TEST("arm-vexpressa9-basic", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_DRIVE); virObjectUnref(driver.config); virObjectUnref(driver.caps); -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
Similar to the chardev bit, ARM boards depend on the old style '-net nic' for actually instantiating net devices.
And add tests for working ARM XML with console, disk, and networking. --- src/qemu/qemu_command.c | 34 ++++++++++++++++------ src/qemu/qemu_domain.c | 20 +++++++++++-- .../qemuxml2argv-arm-vexpressa9-basic.args | 1 + .../qemuxml2argv-arm-vexpressa9-basic.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 248e4b4..3b275e3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -417,6 +417,26 @@ cleanup: return ret; }
+static bool +qemuDomainSupportsNicdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + return false;
So they support -device, but not for network devices? sigh. Should we maybe try to encode this into the capabilities bits directly (and put this code in qemu_capabilities.c)? I don't think I have an opinion, just putting the question out... Aside from that question, this all looks good to me. ACK.
+ + /* arm boards require legacy -net nic */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return false; + + return true; +} + +static bool +qemuDomainSupportsNetdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!qemuDomainSupportsNicdev(def, qemuCaps)) + return false; + return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); +}
/** * qemuOpenVhostNet: @@ -452,8 +472,7 @@ qemuOpenVhostNet(virDomainDefPtr def, * option), don't try to open the device. */ if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOST_NET) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { + qemuDomainSupportsNetdev(def, qemuCaps))) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with " @@ -6852,8 +6871,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, * * NB, no support for -netdev without use of -device */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuDomainSupportsNetdev(def, qemuCaps)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -6861,7 +6879,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (qemuDomainSupportsNicdev(def, qemuCaps)) { if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); @@ -6870,8 +6888,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-net", nic, NULL); } - if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { + if (!qemuDomainSupportsNetdev(def, qemuCaps)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -7864,8 +7881,7 @@ qemuBuildCommandLine(virConnectPtr conn, int vlan;
/* VLANs are not used with -netdev, so don't record them */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + if (qemuDomainSupportsNetdev(def, qemuCaps)) vlan = -1; else vlan = i; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da3b768..c868dc1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -746,6 +746,23 @@ qemuDomainDefPostParse(virDomainDefPtr def, return 0; }
+static const char * +qemuDomainDefaultNetModel(virDomainDefPtr def) { + if (def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) + return "virtio"; + + if (def->os.arch == VIR_ARCH_ARMV7L) { + if (STREQ(def->os.machine, "versatilepb")) + return "smc91c111"; + + /* Incomplete. vexpress-a9 (and a few others) use this, but not all + * arm boards */ + return "lan9118"; + } + + return "rtl8139"; +}
static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, @@ -761,8 +778,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && !dev->data.net->model) { if (VIR_STRDUP(dev->data.net->model, - def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X ? "virtio" : "rtl8139") < 0) + qemuDomainDefaultNetModel(def)) < 0) goto cleanup; }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args new file mode 100644 index 0000000..98ca5c9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append 'console=ttyAMA0,115200n8 rw root=/dev/mmcblk0p3 rootwait physmap.enabled=0' -dtb /arm-dtb -drive file=/arm.raw,if=sd,index=0 -net nic,macaddr=52:54:00:09:a4:37,vlan=0,model=lan9118,name=net0 -net user,vlan=0,name=hostnet0 -serial pty diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml new file mode 100644 index 0000000..7b846c6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml @@ -0,0 +1,34 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/arm-dtb</dtb> + <cmdline>console=ttyAMA0,115200n8 rw root=/dev/mmcblk0p3 rootwait physmap.enabled=0</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + <disk type='file' device='disk'> + <source file='/arm.raw'/> + <target dev='sda' bus='sd'/> + </disk> + <interface type='user'> + <mac address='52:54:00:09:a4:37'/> + </interface> + <console type='pty'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 361ddb8..0bf2724 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1029,6 +1029,9 @@ mymain(void)
DO_TEST("arm-vexpressa9-nodevs", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB); + DO_TEST("arm-vexpressa9-basic", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_DRIVE);
virObjectUnref(driver.config); virObjectUnref(driver.caps);
On Wed, Jul 31, 2013 at 10:14:37PM -0400, Cole Robinson wrote:
Similar to the chardev bit, ARM boards depend on the old style '-net nic' for actually instantiating net devices.
And add tests for working ARM XML with console, disk, and networking. --- src/qemu/qemu_command.c | 34 ++++++++++++++++------ src/qemu/qemu_domain.c | 20 +++++++++++-- .../qemuxml2argv-arm-vexpressa9-basic.args | 1 + .../qemuxml2argv-arm-vexpressa9-basic.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 5 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-basic.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 248e4b4..3b275e3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -417,6 +417,26 @@ cleanup: return ret; }
+static bool +qemuDomainSupportsNicdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + return false; + + /* arm boards require legacy -net nic */ + if (def->os.arch == VIR_ARCH_ARMV7L) + return false; + + return true; +} + +static bool +qemuDomainSupportsNetdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + if (!qemuDomainSupportsNicdev(def, qemuCaps)) + return false; + return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); +}
Again, I think we should just not set the NETDEV capability for ARM There's once place in the code below that doesn't currnetly check NETDEV, but that seems like a bug. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
vhost only works in KVM mode AIUI, and is infact compiled out if the emulator is built for non-native architecture. --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b275e3..f611940 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -462,8 +462,10 @@ qemuOpenVhostNet(virDomainDefPtr def, { size_t i; - /* If the config says explicitly to not use vhost, return now */ - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { + /* If running a plain QEMU guest, or + * if the config says explicitly to not use vhost, return now*/ + if (def->virtType != VIR_DOMAIN_VIRT_KVM || + net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { *vhostfdSize = 0; return 0; } -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
vhost only works in KVM mode AIUI,
What's your definition of "AIUI"? :-) That makes sense, but in case you haven't yet asked MST, I'm CC'ing him to get it straight from the source. MST? If that's true, then ACK (and we might want to backport this to some older releases)
and is infact compiled out if the emulator is built for non-native architecture. --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b275e3..f611940 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -462,8 +462,10 @@ qemuOpenVhostNet(virDomainDefPtr def, { size_t i;
- /* If the config says explicitly to not use vhost, return now */ - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { + /* If running a plain QEMU guest, or + * if the config says explicitly to not use vhost, return now*/ + if (def->virtType != VIR_DOMAIN_VIRT_KVM || + net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { *vhostfdSize = 0; return 0; }
On Thu, Aug 01, 2013 at 03:06:41AM -0400, Laine Stump wrote:
On 07/31/2013 10:14 PM, Cole Robinson wrote:
vhost only works in KVM mode AIUI,
What's your definition of "AIUI"? :-) That makes sense, but in case you haven't yet asked MST, I'm CC'ing him to get it straight from the source. MST?
If that's true, then ACK (and we might want to backport this to some older releases)
That's true at the moment but I'm not sure about the future: fundamentally there's nothing kvm specific in vhost, it's just that qemu doesn't yet support ioeventfd and irqfd without kvm. So I think we really should design something to detect whether vhost-net is supported, I will look into this. It's up to you guys whether to wait for that and look into how to do this properly, or apply this patch and re-do the change again once qemu has this support.
and is infact compiled out if the emulator is built for non-native architecture. --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b275e3..f611940 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -462,8 +462,10 @@ qemuOpenVhostNet(virDomainDefPtr def, { size_t i;
- /* If the config says explicitly to not use vhost, return now */ - if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { + /* If running a plain QEMU guest, or + * if the config says explicitly to not use vhost, return now*/ + if (def->virtType != VIR_DOMAIN_VIRT_KVM || + net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { *vhostfdSize = 0; return 0; }
Starting with qemu 1.6, the qemu-system-arm vexpress-a9 model has a hardcoded virtio-mmio transport which enables attaching all virtio devices. On the command line, we have to use virtio-XXX-device rather than virtio-XXX-pci, thankfully s390 already set the precedent here so it's fairly straight forward. At the XML level, this adds a new device address type virtio-mmio. The controller and addressing don't have any subelements at the moment because we they aren't needed for this usecase, but could be added later if needed. Add a test case for an ARM guest with one of every virtio device enabled. --- src/conf/domain_conf.c | 12 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 16 ++++-- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 65 +++++++++++++++++----- .../qemuxml2argv-arm-vexpressa9-virtio.args | 1 + .../qemuxml2argv-arm-vexpressa9-virtio.xml | 45 +++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ 8 files changed, 124 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5e423cc..25d356b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -210,7 +210,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "usb", "spapr-vio", "virtio-s390", - "ccw") + "ccw", + "virtio-mmio") VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", @@ -2386,6 +2387,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 1; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: return 1; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: @@ -3027,6 +3029,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.ccw.devno); break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown address type '%d'"), info->type); @@ -3491,6 +3496,9 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, goto cleanup; break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + break; + default: /* Should not happen */ virReportError(VIR_ERR_INTERNAL_ERROR, @@ -5738,6 +5746,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Controllers must use the 'pci' address type")); @@ -6349,6 +6358,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Network interfaces must use 'pci' address type")); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 978be16..4d90f84 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -207,6 +207,7 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 51064e8..2b0e9fc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -234,6 +234,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vnc-share-policy", /* 150 */ "device-del-event", + "virtio-mmio", ); struct _virQEMUCaps { @@ -1381,6 +1382,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE }, { "vfio-pci", QEMU_CAPS_DEVICE_VFIO_PCI }, { "scsi-generic", QEMU_CAPS_DEVICE_SCSI_GENERIC }, + { "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { @@ -2814,17 +2816,19 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps) bool virQEMUCapsSupportsChardev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainChrDefPtr chr ATTRIBUTE_UNUSED) + virDomainChrDefPtr chr) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; - /* This may not be true for all machine types, but at least - * the only supported serial devices of vexpress-a9 and versatilepb - * don't have the chardev property wired up */ if (def->os.arch != VIR_ARCH_ARMV7L) - return false; + return true; - return true; + /* This may not be true for all ARM machine types, but at least + * the non-virtio serial devices of vexpress-a9 and versatilepb + * don't have the chardev property wired up */ + return (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 56f8405..fdb61b0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -190,6 +190,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_MLOCK = 149, /* -realtime mlock=on|off */ QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ + QEMU_CAPS_DEVICE_VIRTIO_MMIO = 152, /* -device virtio-mmio */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f611940..9b037f5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -418,22 +418,27 @@ cleanup: } static bool -qemuDomainSupportsNicdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuDomainSupportsNicdev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainNetDefPtr net) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; - /* arm boards require legacy -net nic */ - if (def->os.arch == VIR_ARCH_ARMV7L) + /* non-virtio ARM nics require legacy -net nic */ + if (def->os.arch == VIR_ARCH_ARMV7L && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) return false; return true; } static bool -qemuDomainSupportsNetdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuDomainSupportsNetdev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainNetDefPtr net) { - if (!qemuDomainSupportsNicdev(def, qemuCaps)) + if (!qemuDomainSupportsNicdev(def, qemuCaps, net)) return false; return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); } @@ -474,7 +479,7 @@ qemuOpenVhostNet(virDomainDefPtr def, * option), don't try to open the device. */ if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOST_NET) && - qemuDomainSupportsNetdev(def, qemuCaps))) { + qemuDomainSupportsNetdev(def, qemuCaps, net))) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with " @@ -1146,8 +1151,8 @@ cleanup: } static void -qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, - enum virDomainDeviceAddressType type) +qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, + enum virDomainDeviceAddressType type) { /* declare address-less virtio devices to be of address type 'type' @@ -1281,7 +1286,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, if (STREQLEN(def->os.machine, "s390-ccw", 8) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - qemuDomainPrimeS390VirtioDevices( + qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW); if (!(addrs = qemuDomainCCWAddressSetCreate())) @@ -1296,7 +1301,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, goto cleanup; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { /* deal with legacy virtio-s390 */ - qemuDomainPrimeS390VirtioDevices( + qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390); } @@ -1319,6 +1324,18 @@ cleanup: return ret; } +static int +qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + if (def->os.arch == VIR_ARCH_ARMV7L && + STREQ(def->os.machine, "vexpress-a9") && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { + qemuDomainPrimeVirtioDeviceAddresses( + def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); + } + return 0; +} static int qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED, @@ -1833,6 +1850,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, if (rc) return rc; + rc = qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps); + if (rc) + return rc; + return qemuDomainAssignPCIAddresses(def, qemuCaps, obj); } @@ -3939,6 +3960,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } else if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&opt, "virtio-blk-s390"); + } else if (disk->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + virBufferAddLit(&opt, "virtio-blk-device"); } else { virBufferAddLit(&opt, "virtio-blk-pci"); } @@ -4216,6 +4240,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, else if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) virBufferAddLit(&buf, "virtio-scsi-s390"); + else if (def->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + virBufferAddLit(&buf, "virtio-scsi-device"); else virBufferAddLit(&buf, "virtio-scsi-pci"); break; @@ -4245,6 +4272,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } else if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&buf, "virtio-serial-s390"); + } else if (def->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + virBufferAddLit(&buf, "virtio-serial-device"); } else { virBufferAddLit(&buf, "virtio-serial"); } @@ -4360,6 +4390,8 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, nic = "virtio-net-ccw"; else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) nic = "virtio-net-s390"; + else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + nic = "virtio-net-device"; else nic = "virtio-net-pci"; @@ -4604,6 +4636,9 @@ qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: virBufferAddLit(&buf, "virtio-balloon-ccw"); break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + virBufferAddLit(&buf, "virtio-balloon-device"); + break; default: virReportError(VIR_ERR_XML_ERROR, _("memballoon unsupported with address type '%s'"), @@ -5597,6 +5632,8 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd, virBufferAsprintf(&buf, "virtio-rng-ccw,rng=%s", dev->info.alias); else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) virBufferAsprintf(&buf, "virtio-rng-s390,rng=%s", dev->info.alias); + else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + virBufferAsprintf(&buf, "virtio-rng-device,rng=%s", dev->info.alias); else virBufferAsprintf(&buf, "virtio-rng-pci,rng=%s", dev->info.alias); @@ -6873,7 +6910,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, * * NB, no support for -netdev without use of -device */ - if (qemuDomainSupportsNetdev(def, qemuCaps)) { + if (qemuDomainSupportsNetdev(def, qemuCaps, net)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -6881,7 +6918,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); } - if (qemuDomainSupportsNicdev(def, qemuCaps)) { + if (qemuDomainSupportsNicdev(def, qemuCaps, net)) { if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); @@ -6890,7 +6927,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-net", nic, NULL); } - if (!qemuDomainSupportsNetdev(def, qemuCaps)) { + if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -7883,7 +7920,7 @@ qemuBuildCommandLine(virConnectPtr conn, int vlan; /* VLANs are not used with -netdev, so don't record them */ - if (qemuDomainSupportsNetdev(def, qemuCaps)) + if (qemuDomainSupportsNetdev(def, qemuCaps, net)) vlan = -1; else vlan = i; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args new file mode 100644 index 0000000..349c758 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append 'console=ttyAMA0,115200n8 rw root=/dev/vda3 rootwait physmap.enabled=0' -dtb /f19-arm.dtb -device virtio-serial-device,id=virtio-serial0 -drive file=/mnt/data/devel/images/f19-arm.raw,if=none,id=drive-virtio-disk0 -device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 -device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 -net user,vlan=0,name=hostnet0 -serial pty -chardev pty,id=charconsole1 -device virtconsole,chardev=charconsole1,id=console1 -device virtio-balloon-device,id=balloon0 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-device,rng=rng0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml new file mode 100644 index 0000000..7709db9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml @@ -0,0 +1,45 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/f19-arm.dtb</dtb> + <cmdline>console=ttyAMA0,115200n8 rw root=/dev/vda3 rootwait physmap.enabled=0</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + <disk type='file' device='disk'> + <source file='/mnt/data/devel/images/f19-arm.raw'/> + <target dev='vda' bus='virtio'/> + </disk> + <interface type='user'> + <mac address='52:54:00:09:a4:37'/> + <model type='virtio'/> + </interface> + <console type='pty'/> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='virtio'/> + <!-- + This actually doesn't work in practice because vexpress only has + 4 virtio slots available, rng makes 5 --> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0bf2724..2bdd18e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1032,6 +1032,10 @@ mymain(void) DO_TEST("arm-vexpressa9-basic", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, QEMU_CAPS_DRIVE); + DO_TEST("arm-vexpressa9-virtio", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_VIRTIO_MMIO, + QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); virObjectUnref(driver.config); virObjectUnref(driver.caps); -- 1.8.3.1
On 07/31/2013 10:14 PM, Cole Robinson wrote:
Starting with qemu 1.6, the qemu-system-arm vexpress-a9 model has a hardcoded virtio-mmio transport which enables attaching all virtio devices.
Okay, so there is something named "virtio-mmio" that is visible in the qemu capabilities; it isn't added to the commandline anywhere, but its presence means that all the virtio devices are available with the suffix ...-device rather than ...-device-whatever. A bit strange, but if that's the way it works, that's the way it works... This all looks okay to me, but somebody else should look too, as it's getting pretty late here and I'm not certain I'm completely alert :-)
On the command line, we have to use virtio-XXX-device rather than virtio-XXX-pci, thankfully s390 already set the precedent here so it's fairly straight forward.
At the XML level, this adds a new device address type virtio-mmio. The controller and addressing don't have any subelements at the moment because we they aren't needed for this usecase, but could be added later if needed.
Add a test case for an ARM guest with one of every virtio device enabled. --- src/conf/domain_conf.c | 12 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 16 ++++-- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 65 +++++++++++++++++----- .../qemuxml2argv-arm-vexpressa9-virtio.args | 1 + .../qemuxml2argv-arm-vexpressa9-virtio.xml | 45 +++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ 8 files changed, 124 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5e423cc..25d356b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -210,7 +210,8 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "usb", "spapr-vio", "virtio-s390", - "ccw") + "ccw", + "virtio-mmio")
VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", @@ -2386,6 +2387,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 1;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390: + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: return 1;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: @@ -3027,6 +3029,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.ccw.devno); break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown address type '%d'"), info->type); @@ -3491,6 +3496,9 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, goto cleanup; break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + break; + default: /* Should not happen */ virReportError(VIR_ERR_INTERNAL_ERROR, @@ -5738,6 +5746,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Controllers must use the 'pci' address type")); @@ -6349,6 +6358,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Network interfaces must use 'pci' address type")); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 978be16..4d90f84 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -207,6 +207,7 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 51064e8..2b0e9fc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -234,6 +234,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"vnc-share-policy", /* 150 */ "device-del-event", + "virtio-mmio", );
struct _virQEMUCaps { @@ -1381,6 +1382,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE }, { "vfio-pci", QEMU_CAPS_DEVICE_VFIO_PCI }, { "scsi-generic", QEMU_CAPS_DEVICE_SCSI_GENERIC }, + { "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { @@ -2814,17 +2816,19 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps) bool virQEMUCapsSupportsChardev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainChrDefPtr chr ATTRIBUTE_UNUSED) + virDomainChrDefPtr chr) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false;
- /* This may not be true for all machine types, but at least - * the only supported serial devices of vexpress-a9 and versatilepb - * don't have the chardev property wired up */ if (def->os.arch != VIR_ARCH_ARMV7L) - return false; + return true;
- return true; + /* This may not be true for all ARM machine types, but at least + * the non-virtio serial devices of vexpress-a9 and versatilepb + * don't have the chardev property wired up */ + return (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 56f8405..fdb61b0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -190,6 +190,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_MLOCK = 149, /* -realtime mlock=on|off */ QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ + QEMU_CAPS_DEVICE_VIRTIO_MMIO = 152, /* -device virtio-mmio */
QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f611940..9b037f5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -418,22 +418,27 @@ cleanup: }
static bool -qemuDomainSupportsNicdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuDomainSupportsNicdev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainNetDefPtr net) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false;
- /* arm boards require legacy -net nic */ - if (def->os.arch == VIR_ARCH_ARMV7L) + /* non-virtio ARM nics require legacy -net nic */ + if (def->os.arch == VIR_ARCH_ARMV7L && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) return false;
Urgh. Now I see why this shouldn't be put in qemu_capabilities.c :-/
return true; }
static bool -qemuDomainSupportsNetdev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuDomainSupportsNetdev(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainNetDefPtr net) { - if (!qemuDomainSupportsNicdev(def, qemuCaps)) + if (!qemuDomainSupportsNicdev(def, qemuCaps, net)) return false; return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); } @@ -474,7 +479,7 @@ qemuOpenVhostNet(virDomainDefPtr def, * option), don't try to open the device. */ if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOST_NET) && - qemuDomainSupportsNetdev(def, qemuCaps))) { + qemuDomainSupportsNetdev(def, qemuCaps, net))) { if (net->driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost-net is not supported with " @@ -1146,8 +1151,8 @@ cleanup: }
static void -qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, - enum virDomainDeviceAddressType type) +qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, + enum virDomainDeviceAddressType type) { /* declare address-less virtio devices to be of address type 'type' @@ -1281,7 +1286,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
if (STREQLEN(def->os.machine, "s390-ccw", 8) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - qemuDomainPrimeS390VirtioDevices( + qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
if (!(addrs = qemuDomainCCWAddressSetCreate())) @@ -1296,7 +1301,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, goto cleanup; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { /* deal with legacy virtio-s390 */ - qemuDomainPrimeS390VirtioDevices( + qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390); }
@@ -1319,6 +1324,18 @@ cleanup:
return ret; } +static int +qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps)
Does this really need to be arm-specific? Isn't is possible that there would be other machines/arches that also have a virtio-mmio controller built-in, and user the virtio-xxx-device devices? Maybe this function needs a more generic name, and to just check QEMU_CAPS_DEVICE_VIRTIO_MMIO, rather than adding in the check for os.arch and os.machine.
+{ + if (def->os.arch == VIR_ARCH_ARMV7L && + STREQ(def->os.machine, "vexpress-a9") && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { + qemuDomainPrimeVirtioDeviceAddresses( + def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); + } + return 0; +}
static int qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED, @@ -1833,6 +1850,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, if (rc) return rc;
+ rc = qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps); + if (rc) + return rc; + return qemuDomainAssignPCIAddresses(def, qemuCaps, obj); }
@@ -3939,6 +3960,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } else if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&opt, "virtio-blk-s390"); + } else if (disk->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + virBufferAddLit(&opt, "virtio-blk-device"); } else { virBufferAddLit(&opt, "virtio-blk-pci"); } @@ -4216,6 +4240,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, else if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) virBufferAddLit(&buf, "virtio-scsi-s390"); + else if (def->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + virBufferAddLit(&buf, "virtio-scsi-device"); else virBufferAddLit(&buf, "virtio-scsi-pci"); break; @@ -4245,6 +4272,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } else if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&buf, "virtio-serial-s390"); + } else if (def->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + virBufferAddLit(&buf, "virtio-serial-device"); } else { virBufferAddLit(&buf, "virtio-serial"); } @@ -4360,6 +4390,8 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, nic = "virtio-net-ccw"; else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) nic = "virtio-net-s390"; + else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + nic = "virtio-net-device"; else nic = "virtio-net-pci";
@@ -4604,6 +4636,9 @@ qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: virBufferAddLit(&buf, "virtio-balloon-ccw"); break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO: + virBufferAddLit(&buf, "virtio-balloon-device"); + break; default: virReportError(VIR_ERR_XML_ERROR, _("memballoon unsupported with address type '%s'"), @@ -5597,6 +5632,8 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd, virBufferAsprintf(&buf, "virtio-rng-ccw,rng=%s", dev->info.alias); else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) virBufferAsprintf(&buf, "virtio-rng-s390,rng=%s", dev->info.alias); + else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + virBufferAsprintf(&buf, "virtio-rng-device,rng=%s", dev->info.alias); else virBufferAsprintf(&buf, "virtio-rng-pci,rng=%s", dev->info.alias);
@@ -6873,7 +6910,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, * * NB, no support for -netdev without use of -device */ - if (qemuDomainSupportsNetdev(def, qemuCaps)) { + if (qemuDomainSupportsNetdev(def, qemuCaps, net)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -6881,7 +6918,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); } - if (qemuDomainSupportsNicdev(def, qemuCaps)) { + if (qemuDomainSupportsNicdev(def, qemuCaps, net)) { if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); @@ -6890,7 +6927,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, goto cleanup; virCommandAddArgList(cmd, "-net", nic, NULL); } - if (!qemuDomainSupportsNetdev(def, qemuCaps)) { + if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) { if (!(host = qemuBuildHostNetStr(net, driver, ',', vlan, tapfdName, tapfdSize, @@ -7883,7 +7920,7 @@ qemuBuildCommandLine(virConnectPtr conn, int vlan;
/* VLANs are not used with -netdev, so don't record them */ - if (qemuDomainSupportsNetdev(def, qemuCaps)) + if (qemuDomainSupportsNetdev(def, qemuCaps, net)) vlan = -1; else vlan = i; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args new file mode 100644 index 0000000..349c758 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-arm -S -M vexpress-a9 -m 1024 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -boot c -kernel /arm-kernel -initrd /arm-initrd -append 'console=ttyAMA0,115200n8 rw root=/dev/vda3 rootwait physmap.enabled=0' -dtb /f19-arm.dtb -device virtio-serial-device,id=virtio-serial0 -drive file=/mnt/data/devel/images/f19-arm.raw,if=none,id=drive-virtio-disk0 -device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 -device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 -net user,vlan=0,name=hostnet0 -serial pty -chardev pty,id=charconsole1 -device virtconsole,chardev=charconsole1,id=console1 -device virtio-balloon-device,id=balloon0 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-device,rng=rng0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml new file mode 100644 index 0000000..7709db9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.xml @@ -0,0 +1,45 @@ +<domain type="qemu"> + <name>armtest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e6a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch="armv7l" machine="vexpress-a9">hvm</type> + <kernel>/arm-kernel</kernel> + <initrd>/arm-initrd</initrd> + <dtb>/f19-arm.dtb</dtb> + <cmdline>console=ttyAMA0,115200n8 rw root=/dev/vda3 rootwait physmap.enabled=0</cmdline> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-arm</emulator> + <disk type='file' device='disk'> + <source file='/mnt/data/devel/images/f19-arm.raw'/> + <target dev='vda' bus='virtio'/> + </disk> + <interface type='user'> + <mac address='52:54:00:09:a4:37'/> + <model type='virtio'/> + </interface> + <console type='pty'/> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='virtio'/> + <!-- + This actually doesn't work in practice because vexpress only has + 4 virtio slots available, rng makes 5 --> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0bf2724..2bdd18e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1032,6 +1032,10 @@ mymain(void) DO_TEST("arm-vexpressa9-basic", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, QEMU_CAPS_DRIVE); + DO_TEST("arm-vexpressa9-virtio", + QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_VIRTIO_MMIO, + QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
virObjectUnref(driver.config); virObjectUnref(driver.caps);
On Wed, Jul 31, 2013 at 10:14:39PM -0400, Cole Robinson wrote:
Starting with qemu 1.6, the qemu-system-arm vexpress-a9 model has a hardcoded virtio-mmio transport which enables attaching all virtio devices.
On the command line, we have to use virtio-XXX-device rather than virtio-XXX-pci, thankfully s390 already set the precedent here so it's fairly straight forward.
At the XML level, this adds a new device address type virtio-mmio. The controller and addressing don't have any subelements at the moment because we they aren't needed for this usecase, but could be added later if needed.
Add a test case for an ARM guest with one of every virtio device enabled.
@@ -2814,17 +2816,19 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps) bool virQEMUCapsSupportsChardev(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainChrDefPtr chr ATTRIBUTE_UNUSED) + virDomainChrDefPtr chr) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false;
- /* This may not be true for all machine types, but at least - * the only supported serial devices of vexpress-a9 and versatilepb - * don't have the chardev property wired up */ if (def->os.arch != VIR_ARCH_ARMV7L) - return false; + return true;
- return true; + /* This may not be true for all ARM machine types, but at least + * the non-virtio serial devices of vexpress-a9 and versatilepb + * don't have the chardev property wired up */ + return (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); }
Urgh. Now I see why you didn't just clear the CHARDEV capability for ARM - it isn't a simple as blacklisting the whole architecture. This is one of those times I really hate QEMU :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Laine Stump -
Michael S. Tsirkin