7:23 a.m.
Fixes the following error when attempting to add a disk with bus='virtio':
virtio only support device address type 'PCI'
Signed-off-by: Pavel Fedin <p.fedin(a)samsung.com>
---
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 38104da..408b249 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2189,12 +2189,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
continue;
- /* don't touch s390 devices */
+ /* don't touch s390 and virtio-mmio devices */
if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
def->disks[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 ||
def->disks[i]->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+ def->disks[i]->info.type ==
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
continue;
if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
--
1.9.5.msysgit.0
8:27 a.m.
[Please don't Cc random people when they did not request it, all
developers are subscribed to the list]
On Tue, Sep 08, 2015 at 03:23:27PM +0300, Pavel Fedin wrote:
Fixes the following error when attempting to add a disk with
bus='virtio':
virtio only support device address type 'PCI'
How did you manage to do that? I think we are not handlind
virtio-mmio addressing properly as we won't add some controller qemu
will then be missing. Shouldn't that be fixed as well? And isn't
virtio-mmio only supported on some platforms?
Otherwise the change makes sense.
Signed-off-by: Pavel Fedin <p.fedin(a)samsung.com>
---
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 38104da..408b249 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2189,12 +2189,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
continue;
- /* don't touch s390 devices */
+ /* don't touch s390 and virtio-mmio devices */
if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI ||
def->disks[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 ||
def->disks[i]->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+ def->disks[i]->info.type ==
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
continue;
if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
--
1.9.5.msysgit.0
1:38 a.m.
New subject: [libvirt] [PATCH] Ignore virtio-mmio disks in qemuAssignDevicePCISlots()
Hello!
How did you manage to do that?
Probably i forgot to explicitly specify that it affects ARM 'virt' machine.
Before we added PCIe
support we were able to use only virtio-mmio there. Now we can use both, because
virtio-mmio support
in qemu didn't go anywhere.
It worked before PCIe support introduction just because qemuAssignDevicePCISlots() was
never
called.
I think we are not handlind virtio-mmio addressing properly as we
won't add some controller qemu
will then be missing. Shouldn't that be fixed as well?
Didn't understand exactly what you mean, but <address
type='virtio-mmio'/> perfectly works. Even
more, this is the default on ARM for backwards compatibility, as we decided before.
virtio-mmio is
assigned either manually or automatically for certain device kinds by
qemuDomainAssignARMVirtioMMIOAddresses(). After it, qemuDomainAssignPCIAddresses() is
called, which
before PCIe introduction just did nothing. Now it tries to assign PCI addresses to
anything which
still doesn't have addresses at all. For the majority of devices it checks only for
info.type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE, but for VirtIO disks it is a bit more picky for some
reason.
The problem is easy to reproduce with virt-manager. Just try to add a disk (or modify
existing
disk) and specify bus = virtio. By default, since we now have PCIe, it will add
virtio-scsi disk on
top of SCSI controller, so the default works flawlessly. Because virtio-scsi !=
virtio-block.
And isn't virtio-mmio only supported on some platforms?
Yes. But for platforms where virtio-mmio is not supported we can not get
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO addresses, because
qemuDomainAssignARMVirtioMMIOAddresses() will not do anything, because
QEMU_CAPS_DEVICE_VIRTIO_MMIO
will not be set.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
2:29 a.m.
On Wed, Sep 09, 2015 at 09:38:44AM +0300, Pavel Fedin wrote:
Hello!
> How did you manage to do that?
Probably i forgot to explicitly specify that it affects ARM 'virt' machine.
Before we added PCIe
support we were able to use only virtio-mmio there. Now we can use both, because
virtio-mmio support
in qemu didn't go anywhere.
It worked before PCIe support introduction just because qemuAssignDevicePCISlots() was
never
called.
OK, good that I tried it on an arm machine :)
> I think we are not handlind virtio-mmio addressing properly as
we won't add some controller qemu
> will then be missing. Shouldn't that be fixed as well?
Didn't understand exactly what you mean, but <address
type='virtio-mmio'/> perfectly works. Even
more, this is the default on ARM for backwards compatibility, as we decided before.
virtio-mmio is
assigned either manually or automatically for certain device kinds by
qemuDomainAssignARMVirtioMMIOAddresses(). After it, qemuDomainAssignPCIAddresses() is
called, which
before PCIe introduction just did nothing. Now it tries to assign PCI addresses to
anything which
still doesn't have addresses at all. For the majority of devices it checks only for
info.type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE, but for VirtIO disks it is a bit more picky for some
reason.
The problem is easy to reproduce with virt-manager. Just try to add a disk (or modify
existing
disk) and specify bus = virtio. By default, since we now have PCIe, it will add
virtio-scsi disk on
top of SCSI controller, so the default works flawlessly. Because virtio-scsi !=
virtio-block.
So that's what I was missing. virt-manager probably automatically
adds the controller, but libvirt itself doesn't. That's what I meant
that we should probably add that too (see *Add*Maybe() functions), but
that's not to be done in this patch, though.
> And isn't virtio-mmio only supported on some platforms?
Yes. But for platforms where virtio-mmio is not supported we can not get
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO addresses, because
qemuDomainAssignARMVirtioMMIOAddresses() will not do anything, because
QEMU_CAPS_DEVICE_VIRTIO_MMIO
will not be set.
Well, without your patch, I get a clear message for non-arm
(e.g. x86_64) machine that says "virtio disk cannot have an address of
type 'virtio-mmio'" (even though I have en_GB locale in which the
string should be very badly translated, but that's another problem).
And that is what we should keep for architectures without virtio-mmio
support, shouldn't we?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
2:59 a.m.
Hello!
Well, without your patch, I get a clear message for non-arm
(e.g. x86_64) machine that says "virtio disk cannot have an address of
type 'virtio-mmio'" (even though I have en_GB locale in which the
string should be very badly translated, but that's another problem).
And that is what we should keep for architectures without virtio-mmio
support, shouldn't we?
Agree, thanks for testing. I don't have x86 installation here. I will make v2 which
will check for
QEMU_CAPS_DEVICE_VIRTIO_MMIO capability.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
3398
days inactive
3399
days old
4 comments
2 participants
participants (2)
-
Martin Kletzander -
Pavel Fedin