Re: [Qemu-devel] [PATCH] Fix legacy device aliases for s390

[Anthony Liguori]

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:

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index dc4e4e1..f90966f 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -29,16 +29,17 @@ 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" },
     { }
@@ -110,6 +111,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].mask &&
+            !(qdev_alias_table[i].mask & arch_type)) {
+            continue;
+        }
+
         if (strcmp(qdev_alias_table[i].alias, alias) == 0) {
             return qdev_alias_table[i].typename;
         }


Not tested at all as I'm on holiday but if you wanted to take this patch and run 
with it, I think that would be a reasonable approach.

Regards,

Anthony Liguori

> Alex