| Message ID | 1371472182-10074-2-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
Hi, Am 17.06.2013 14:29, schrieb Christian Borntraeger: > The latest ipl code adoptions collided with some of the virtio "adaptions"? > refactoring rework. This resulted in always booting the first > disk. Lets fix booting from a given ID. "Let's"? > The new code also checks for command lines without bootindex to > avoid random behaviour when accessing dev_st (==0). > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > CC: qemu-stable@nongnu.org > --- > hw/s390x/ipl.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 0aeb003..8b25b1c 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -156,13 +156,15 @@ static void s390_ipl_reset(DeviceState *dev) > if (!ipl->kernel) { > /* booting firmware, tell what device to boot from */ > DeviceState *dev_st = get_boot_device(0); > - VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast( > - OBJECT(&(dev_st->parent_obj)), "virtio-blk-ccw"); This should've never accessed parent_obj but simply use OBJECT(dev_st). I would expect object_dynamic_cast() to return NULL on NULL input then, but it seems the issue is rather that dev_st will be the VirtioDevice and not the VirtIOS390Device. > - > - if (ccw_dev) { > - env->regs[7] = ccw_dev->sch->cssid << 24 | > - ccw_dev->sch->ssid << 16 | > - ccw_dev->sch->devno; > + if (dev_st) { > + VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast( > + OBJECT((dev_st->parent_obj.parent)), "virtio-blk-ccw"); This is worse and equivalent to OBJECT(OBJECT(dev_st)->parent), with the outer cast superfluous and parent being an Object-private field according to include/qom/object.h. IIRC we had once suggested to introduce an object_get_parent() accessor, but Anthony was against it for some reason...? CC'ing. Instead, I believe it would be permissible to access the device's bus, which in turn has a pointer to its parent device: OBJECT(qdev_get_parent_bus(dev_st)->parent) > + > + if (ccw_dev) { > + env->regs[7] = ccw_dev->sch->cssid << 24 | > + ccw_dev->sch->ssid << 16 | > + ccw_dev->sch->devno; > + } Previously, env->regs[7] would've been assigned -1 for !ccw_dev. Functional change intentional? Cheers, Andreas > } else { > env->regs[7] = -1; > }
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 0aeb003..8b25b1c 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -156,13 +156,15 @@ static void s390_ipl_reset(DeviceState *dev) if (!ipl->kernel) { /* booting firmware, tell what device to boot from */ DeviceState *dev_st = get_boot_device(0); - VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast( - OBJECT(&(dev_st->parent_obj)), "virtio-blk-ccw"); - - if (ccw_dev) { - env->regs[7] = ccw_dev->sch->cssid << 24 | - ccw_dev->sch->ssid << 16 | - ccw_dev->sch->devno; + if (dev_st) { + VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast( + OBJECT((dev_st->parent_obj.parent)), "virtio-blk-ccw"); + + if (ccw_dev) { + env->regs[7] = ccw_dev->sch->cssid << 24 | + ccw_dev->sch->ssid << 16 | + ccw_dev->sch->devno; + } } else { env->regs[7] = -1; }
The latest ipl code adoptions collided with some of the virtio refactoring rework. This resulted in always booting the first disk. Lets fix booting from a given ID. The new code also checks for command lines without bootindex to avoid random behaviour when accessing dev_st (==0). Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> CC: qemu-stable@nongnu.org --- hw/s390x/ipl.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)