[libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported

Pavel Hrdina posted 4 patches 8 years, 10 months ago
[libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
Posted by Pavel Hrdina 8 years, 10 months ago
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_capabilities.c                                 | 4 +++-
 tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 -
 tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 -
 tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            | 1 -
 tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            | 1 -
 tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              | 1 -
 tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              | 1 -
 7 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ba5cb19427..5197090825 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5547,8 +5547,10 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
     if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch))
         VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY);
 
+    if (qemuDomainMachineHasBuiltinIDE(machine))
+        VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_IDE);
+
     VIR_DOMAIN_CAPS_ENUM_SET(disk->bus,
-                             VIR_DOMAIN_DISK_BUS_IDE,
                              VIR_DOMAIN_DISK_BUS_SCSI,
                              VIR_DOMAIN_DISK_BUS_VIRTIO,
                              /* VIR_DOMAIN_DISK_BUS_SD */);
diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
index 1fa7f6dff8..54b89dc72b 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml
@@ -63,7 +63,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>fdc</value>
         <value>scsi</value>
         <value>virtio</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
index d60fc1df98..60bf2f54f7 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml
@@ -63,7 +63,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>fdc</value>
         <value>scsi</value>
         <value>virtio</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
index fcc6f50e0e..1a980927cf 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml
@@ -63,7 +63,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>fdc</value>
         <value>scsi</value>
         <value>virtio</value>
diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
index 755c4f4475..4ecf8651b4 100644
--- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
+++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml
@@ -37,7 +37,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>scsi</value>
         <value>virtio</value>
         <value>usb</value>
diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
index 999e2795d8..dc6d2d8f0c 100644
--- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
@@ -32,7 +32,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>fdc</value>
         <value>scsi</value>
         <value>virtio</value>
diff --git a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
index 0b8135bc5c..53c3190f20 100644
--- a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml
@@ -113,7 +113,6 @@
         <value>lun</value>
       </enum>
       <enum name='bus'>
-        <value>ide</value>
         <value>fdc</value>
         <value>scsi</value>
         <value>virtio</value>
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
Posted by Peter Krempa 8 years, 9 months ago
On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:

Was this the problem that virt-manager allowed to use the IDE bus on
Q35?

> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c                                 | 4 +++-
>  tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 -
>  tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 -
>  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            | 1 -
>  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            | 1 -
>  tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              | 1 -
>  tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              | 1 -
>  7 files changed, 3 insertions(+), 7 deletions(-)

ACK,

but I think you should follow up and do the same for the floppy device
which AFAIK does not exist on aarch64 and others. There was an attempt
to do this but only for ppc in 020a178318

Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
Posted by Pavel Hrdina 8 years, 9 months ago
On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
> On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
> 
> Was this the problem that virt-manager allowed to use the IDE bus on
> Q35?

Yes :)

> 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c                                 | 4 +++-
> >  tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 -
> >  tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 -
> >  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            | 1 -
> >  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            | 1 -
> >  tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              | 1 -
> >  tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              | 1 -
> >  7 files changed, 3 insertions(+), 7 deletions(-)
> 
> ACK,
> 
> but I think you should follow up and do the same for the floppy device
> which AFAIK does not exist on aarch64 and others. There was an attempt
> to do this but only for ppc in 020a178318

There is a lot of space for improvement, I'll add it to my todo list :)

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
Posted by Peter Krempa 8 years, 9 months ago
On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote:
> On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
> > On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
> > 
> > Was this the problem that virt-manager allowed to use the IDE bus on
> > Q35?
> 
> Yes :)

Then please add something like "Since virt-manager (virt-install?) uses
this data it would allow to create configuration which would be rejected
by libvirt." or something like that ...

> 
> > 
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964

Since this BZ does not clearly state that fact.

> > > 
> > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > ---
> > >  src/qemu/qemu_capabilities.c                                 | 4 +++-
> > >  tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 -
> > >  tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 -
> > >  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            | 1 -
> > >  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            | 1 -
> > >  tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              | 1 -
> > >  tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              | 1 -
> > >  7 files changed, 3 insertions(+), 7 deletions(-)
> > 
> > ACK,
> > 
> > but I think you should follow up and do the same for the floppy device
> > which AFAIK does not exist on aarch64 and others. There was an attempt
> > to do this but only for ppc in 020a178318
> 
> There is a lot of space for improvement, I'll add it to my todo list :)

Yes. There is. As in every other part of libvirt :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
Posted by Pavel Hrdina 8 years, 9 months ago
On Tue, Apr 18, 2017 at 12:08:30PM +0200, Peter Krempa wrote:
> On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote:
> > On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
> > > On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
> > > 
> > > Was this the problem that virt-manager allowed to use the IDE bus on
> > > Q35?
> > 
> > Yes :)
> 
> Then please add something like "Since virt-manager (virt-install?) uses
> this data it would allow to create configuration which would be rejected
> by libvirt." or something like that ...

I don't see any value in mentioning virt-manager in commit message,
the domain capability output is wrong regardless of who uses it.

> > 
> > > 
> > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
> 
> Since this BZ does not clearly state that fact.
> 
> > > > 
> > > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > > ---
> > > >  src/qemu/qemu_capabilities.c                                 | 4 +++-
> > > >  tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 -
> > > >  tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 -
> > > >  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml            | 1 -
> > > >  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml            | 1 -
> > > >  tests/domaincapsschemadata/qemu_2.7.0.s390x.xml              | 1 -
> > > >  tests/domaincapsschemadata/qemu_2.8.0.s390x.xml              | 1 -
> > > >  7 files changed, 3 insertions(+), 7 deletions(-)
> > > 
> > > ACK,
> > > 
> > > but I think you should follow up and do the same for the floppy device
> > > which AFAIK does not exist on aarch64 and others. There was an attempt
> > > to do this but only for ppc in 020a178318
> > 
> > There is a lot of space for improvement, I'll add it to my todo list :)
> 
> Yes. There is. As in every other part of libvirt :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list