diff mbox

Fix legacy device aliases for s390

Message ID 59ABF71D-B380-40F0-B00E-1F60BBB05B93@suse.de
State New
Headers show

Commit Message

Alexander Graf May 3, 2012, 1:34 p.m. UTC 
On 03.05.2012, at 14:32, Anthony Liguori wrote:

> On 05/03/2012 04:07 AM, Alexander Graf wrote:
>> 
>> On 03.05.2012, at 11:05, Paolo Bonzini wrote:
>> 
>>> 
>>>>> The usual old fix was to not even compile them in. Why are they in
>>>>> the alias list in the s390 build now?
>>>> 
>>>> Because the alias list is in target-independent code.
>>>> 
>>>> The old fix was brittle anyway, it dependent on the fact that
>>>> virtio-blk-pci was not part of libhw.  A similar trick broke for
>>>> cirrus-vga when it became part of libhw.
>>>> 
>>>> Christian fix is correct.
>>> 
>>> Uhm, Christian fix would have the same problem actually if
>>> virtio-*-pci were to be moved in libhw.  IIRC I proposed the
>>> same change on review and Anthony nacked it on these grounds.
>>> You could move the alias list to target-dependent code, though.
>> 
>> Can't we just make the virtio-*-pci variants fail instantiation and based on that search the list on?
> 
> No, but you could do:

Ah, nice. Here is a fixed (and tested) version:

Comments

Alexander Graf May 17, 2012, 10:57 p.m. UTC | #1 
On 03.05.2012, at 15:34, Alexander Graf wrote:

> 
> On 03.05.2012, at 14:32, Anthony Liguori wrote:
> 
>> On 05/03/2012 04:07 AM, Alexander Graf wrote:
>>> 
>>> On 03.05.2012, at 11:05, Paolo Bonzini wrote:
>>> 
>>>> 
>>>>>> The usual old fix was to not even compile them in. Why are they in
>>>>>> the alias list in the s390 build now?
>>>>> 
>>>>> Because the alias list is in target-independent code.
>>>>> 
>>>>> The old fix was brittle anyway, it dependent on the fact that
>>>>> virtio-blk-pci was not part of libhw.  A similar trick broke for
>>>>> cirrus-vga when it became part of libhw.
>>>>> 
>>>>> Christian fix is correct.
>>>> 
>>>> Uhm, Christian fix would have the same problem actually if
>>>> virtio-*-pci were to be moved in libhw.  IIRC I proposed the
>>>> same change on review and Anthony nacked it on these grounds.
>>>> You could move the alias list to target-dependent code, though.
>>> 
>>> Can't we just make the virtio-*-pci variants fail instantiation and based on that search the list on?
>> 
>> No, but you could do:
> 
> Ah, nice. Here is a fixed (and tested) version:

Ping? What do we do about this one?


Alex
Anthony Liguori May 18, 2012, 12:28 a.m. UTC | #2 
On 05/17/2012 05:57 PM, Alexander Graf wrote:
>
> On 03.05.2012, at 15:34, Alexander Graf wrote:
>
>>
>> On 03.05.2012, at 14:32, Anthony Liguori wrote:
>>
>>> On 05/03/2012 04:07 AM, Alexander Graf wrote:
>>>>
>>>> On 03.05.2012, at 11:05, Paolo Bonzini wrote:
>>>>
>>>>>
>>>>>>> The usual old fix was to not even compile them in. Why are they in
>>>>>>> the alias list in the s390 build now?
>>>>>>
>>>>>> Because the alias list is in target-independent code.
>>>>>>
>>>>>> The old fix was brittle anyway, it dependent on the fact that
>>>>>> virtio-blk-pci was not part of libhw.  A similar trick broke for
>>>>>> cirrus-vga when it became part of libhw.
>>>>>>
>>>>>> Christian fix is correct.
>>>>>
>>>>> Uhm, Christian fix would have the same problem actually if
>>>>> virtio-*-pci were to be moved in libhw.  IIRC I proposed the
>>>>> same change on review and Anthony nacked it on these grounds.
>>>>> You could move the alias list to target-dependent code, though.
>>>>
>>>> Can't we just make the virtio-*-pci variants fail instantiation and based on that search the list on?
>>>
>>> No, but you could do:
>>
>> Ah, nice. Here is a fixed (and tested) version:
>
> Ping? What do we do about this one?

Are you going to submit your fixed and tested version as a proper patch?

You can add my SoB if you need to.

Regards,

Anthony Liguori

>
>
> Alex
>
diff mbox

Patch

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index dc4e4e1..f7a03fe 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -20,6 +20,7 @@ 
 #include "qdev.h"
 #include "monitor.h"
 #include "qmp-commands.h"
+#include "arch_init.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -29,16 +30,18 @@  typedef struct QDevAlias
 {
     const char *typename;
     const char *alias;
+    uint32_t arch_mask;
 } QDevAlias;
 
 static const QDevAlias qdev_alias_table[] = {
-    { "virtio-blk-pci", "virtio-blk" },
-    { "virtio-net-pci", "virtio-net" },
-    { "virtio-serial-pci", "virtio-serial" },
-    { "virtio-balloon-pci", "virtio-balloon" },
-    { "virtio-blk-s390", "virtio-blk" },
-    { "virtio-net-s390", "virtio-net" },
-    { "virtio-serial-s390", "virtio-serial" },
+    { "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-balloon-pci", "virtio-balloon",
+            QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-blk-s390", "virtio-blk", QEMU_ARCH_S390X },
+    { "virtio-net-s390", "virtio-net", QEMU_ARCH_S390X },
+    { "virtio-serial-s390", "virtio-serial", QEMU_ARCH_S390X },
     { "lsi53c895a", "lsi" },
     { "ich9-ahci", "ahci" },
     { }
@@ -50,6 +53,11 @@  static const char *qdev_class_get_alias(DeviceClass *dc)
     int i;
 
     for (i = 0; qdev_alias_table[i].typename; i++) {
+        if (qdev_alias_table[i].arch_mask &&
+            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            continue;
+        }
+
         if (strcmp(qdev_alias_table[i].typename, typename) == 0) {
             return qdev_alias_table[i].alias;
         }
@@ -110,6 +118,11 @@  static const char *find_typename_by_alias(const char *alias)
     int i;
 
     for (i = 0; qdev_alias_table[i].alias; i++) {
+        if (qdev_alias_table[i].arch_mask &&
+            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            continue;
+        }
+
         if (strcmp(qdev_alias_table[i].alias, alias) == 0) {
             return qdev_alias_table[i].typename;
         }