Re: [Qemu-devel] [Qemu-stable] [QEMU PATCH v3] qdev: fix get_fw_dev_path to support to add nothing to fw_dev_path

[Amos Kong]

On Thu, May 30, 2013 at 02:09:25PM +0200, Stefan Priebe - Profihost AG wrote:
> Am 29.05.2013 09:56, schrieb Amos Kong:
> > Recent virtio refactoring in QEMU made virtio-bus become the parent bus
> > of scsi-bus, and virtio-bus doesn't have get_fw_dev_path implementation,
> > typename will be added to fw_dev_path by default, the new fw_dev_path
> > could not be identified by seabios. It causes that bootindex parameter
> > of scsi device doesn't work.
> > 
> > This patch implements get_fw_dev_path() in BusClass, it will be called
> > if bus doesn't implement the method, tyename will be added to
> > fw_dev_path. If the implemented method returns NULL, nothing will be
> > added to fw_dev_path.
> > 
> > It also implements virtio_bus_get_fw_dev_path() to return NULL. Then
> > QEMU will still pass original style of fw_dev_path to seabios.
> > 
> > Signed-off-by: Amos Kong <address@hidden>
> > --
> > v2: only add nothing to fw_dev_path when get_fw_dev_path() is
> >     implemented and returns NULL. then it will not effect other devices
> >     don't have get_fw_dev_path() implementation.
> > v3: implement default get_fw_dev_path() in BusClass
> > ---
> >  hw/core/qdev.c         | 10 +++++++++-
> >  hw/virtio/virtio-bus.c |  6 ++++++
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index 6985ad8..9190a7e 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -515,7 +515,7 @@ static int qdev_get_fw_dev_path_helper(DeviceState 
> > *dev, char *p, int size)
> >              l += snprintf(p + l, size - l, "%s", d);
> >              g_free(d);
> >          } else {
> > -            l += snprintf(p + l, size - l, "%s", 
> > object_get_typename(OBJECT(dev)));
> > +            return l;
> >          }
> >      }
> >      l += snprintf(p + l , size - l, "/");
> > @@ -867,9 +867,17 @@ static void qbus_initfn(Object *obj)
> >      QTAILQ_INIT(&bus->children);
> >  }
> >  
> > +static char *default_bus_get_fw_dev_path(DeviceState *dev)
> > +{
> > +    return g_strdup(object_get_typename(OBJECT(dev)));
> > +}
> > +
> >  static void bus_class_init(ObjectClass *class, void *data)
> >  {
> > +    BusClass *bc = BUS_CLASS(class);
> > +
> >      class->unparent = bus_unparent;
> > +    bc->get_fw_dev_path = default_bus_get_fw_dev_path;
> >  }
> >  
> >  static void qbus_finalize(Object *obj)
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index ea2e11a..6849a01 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -161,10 +161,16 @@ static char *virtio_bus_get_dev_path(DeviceState *dev)
> >      return qdev_get_dev_path(proxy);
> >  }
> >  
> > +static char *virtio_bus_get_fw_dev_path(DeviceState *dev)
> > +{
> > +    return NULL;
> > +}
> > +
> >  static void virtio_bus_class_init(ObjectClass *klass, void *data)
> >  {
> >      BusClass *bus_class = BUS_CLASS(klass);
> >      bus_class->get_dev_path = virtio_bus_get_dev_path;
> > +    bus_class->get_fw_dev_path = virtio_bus_get_fw_dev_path;
> >  }
> >  
> >  static const TypeInfo virtio_bus_info = {
> > 
> 
> To me booting VMs with more than one SCSI disk does still not work.

Hi Stefan,

Can you provide your full command-lines ?
 
> net: bootindex=100
> scsi0: bootindex=201
> 
> does not work
> 
> this one works fine:
> net: bootindex=200
> scsi0: bootindex=101

For me, they all work, (1. check the bootindex string, 2. check boot menu by 
entering F12, 3. check by waiting ).

address@hidden qemu]# qemu-upstream -drive 
file=/images/RHEL-Server-6.4-64-virtio.qcow2,if=none,id=drive-system-disk \
 -device virtio-scsi-pci,bus=pci.0,addr=0x4,id=scsi0 -device 
scsi-hd,drive=drive-system-disk,bus=scsi0.0,bootindex=201 \
 -device virtio-net-pci,netdev=h1,bootindex=100 \
 -netdev tap,id=h1 -monitor stdio -boot menu=on \
 -drive 
file=/images/RHEL-Server-6.4-64-virtio.qcow2,if=none,id=drive-system-disk2 \
 -device virtio-scsi-pci,bus=pci.0,addr=0x5,id=scsi02 \
 -device scsi-hd,drive=drive-system-disk2,bus=scsi0.0,bootindex=202
/address@hidden/address@hidden/address@hidden
/address@hidden/address@hidden/address@hidden/address@hidden,0
/address@hidden/address@hidden/address@hidden/address@hidden,0

address@hidden qemu]# qemu-upstream -drive 
file=/images/RHEL-Server-6.4-64-virtio.qcow2,if=none,id=drive-system-disk \
 -device virtio-scsi-pci,bus=pci.0,addr=0x4,id=scsi0 -device 
scsi-hd,drive=drive-system-disk,bus=scsi0.0,bootindex=200 \
 -device virtio-net-pci,netdev=h1,bootindex=101 \
 -netdev tap,id=h1 -monitor stdio -boot menu=on \
 -drive 
file=/images/RHEL-Server-6.4-64-virtio.qcow2,if=none,id=drive-system-disk2 \
 -device virtio-scsi-pci,bus=pci.0,addr=0x5,id=scsi02 \
 -device scsi-hd,drive=drive-system-disk2,bus=scsi0.0,bootindex=202
/address@hidden/address@hidden/address@hidden
/address@hidden/address@hidden/address@hidden/address@hidden,0
/address@hidden/address@hidden/address@hidden/address@hidden,0


 (I outputed the bootindex string)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 479113b..3f42660 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -476,6 +476,7 @@ static void fw_cfg_machine_ready(struct Notifier *n, void 
*data)
     FWCfgState *s = container_of(n, FWCfgState, machine_ready);
     char *bootindex = get_boot_devices_list(&len);
 
+    printf("%s\n", bootindex);
     fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
 }


-- 
                        Amos.