| Message ID | 1336034836-17892-1-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 03.05.2012, at 10:47, Christian Borntraeger wrote: > When qemu is called with -device virtio-serial/blk/net on s390, this alias > is translated to virtio-serial/blk/net-pci instead of s390, since these > drivers are first in the alias table. > Let the core code check if the driver exist, if not lets search further. > This fixes errors like: > > qemu-kvm: -device virtio-serial,id=virtio-serial0: Parameter 'driver' > expects device type > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> The usual old fix was to not even compile them in. Why are they in the alias list in the s390 build now? Alex
On 03/05/12 10:49, Alexander Graf wrote: > > On 03.05.2012, at 10:47, Christian Borntraeger wrote: > >> When qemu is called with -device virtio-serial/blk/net on s390, this alias >> is translated to virtio-serial/blk/net-pci instead of s390, since these >> drivers are first in the alias table. >> Let the core code check if the driver exist, if not lets search further. >> This fixes errors like: >> >> qemu-kvm: -device virtio-serial,id=virtio-serial0: Parameter 'driver' >> expects device type >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > > The usual old fix was to not even compile them in. Why are they in the alias list in the s390 build now? Huh? The aliases always worked for me when I compiled qemu myself for s390. This changed with commit 6acbe4c6f18e7de00481ff30574262b58526de45 Author: Anthony Liguori <aliguori@us.ibm.com> Date: Thu Dec 22 11:05:00 2011 -0600 qdev: remove baked in notion of aliases (v2) In other word v1.0 is fine, master is not. This looks like a regression, no? Christian
On 03.05.2012, at 10:53, Christian Borntraeger wrote: > On 03/05/12 10:49, Alexander Graf wrote: >> >> On 03.05.2012, at 10:47, Christian Borntraeger wrote: >> >>> When qemu is called with -device virtio-serial/blk/net on s390, this alias >>> is translated to virtio-serial/blk/net-pci instead of s390, since these >>> drivers are first in the alias table. >>> Let the core code check if the driver exist, if not lets search further. >>> This fixes errors like: >>> >>> qemu-kvm: -device virtio-serial,id=virtio-serial0: Parameter 'driver' >>> expects device type >>> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> >> The usual old fix was to not even compile them in. Why are they in the alias list in the s390 build now? > > Huh? The aliases always worked for me when I compiled qemu myself for s390. This changed with > > commit 6acbe4c6f18e7de00481ff30574262b58526de45 > Author: Anthony Liguori <aliguori@us.ibm.com> > Date: Thu Dec 22 11:05:00 2011 -0600 > > qdev: remove baked in notion of aliases (v2) > > In other word v1.0 is fine, master is not. This looks like a regression, no? Yeah. According to your code however, we have the "virtio-blk-pci" and "virtio-blk-s390" modules both in the alias list, regardless of the architecture. The reason it worked before was that the alias list was specifically built based on qdev devices for each target. Apparently that's changed now. Anthony, was this change on purpose? If so, Christian's patch is correct and should be applied. Alex
> 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. Paolo
> > 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. Paolo
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? Alex
> > 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? The alias list is used in other places than just instantiation (e.g. -device ?). If it were me I'd just go with Christian's patch; it is conceptually broken, but in the same way as QEMU 1.0 (i.e. relies on virtio-*-pci not being compiled into s390 binaries). A comment will do. Paolo
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c index dc4e4e1..8d1b5ba 100644 --- a/hw/qdev-monitor.c +++ b/hw/qdev-monitor.c @@ -110,7 +110,8 @@ static const char *find_typename_by_alias(const char *alias) int i; for (i = 0; qdev_alias_table[i].alias; i++) { - if (strcmp(qdev_alias_table[i].alias, alias) == 0) { + if (strcmp(qdev_alias_table[i].alias, alias) == 0 && + object_class_by_name(qdev_alias_table[i].typename)) { return qdev_alias_table[i].typename; } }
When qemu is called with -device virtio-serial/blk/net on s390, this alias is translated to virtio-serial/blk/net-pci instead of s390, since these drivers are first in the alias table. Let the core code check if the driver exist, if not lets search further. This fixes errors like: qemu-kvm: -device virtio-serial,id=virtio-serial0: Parameter 'driver' expects device type Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- hw/qdev-monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)