* [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
@ 2013-05-28 7:40 Amos Kong
2013-05-28 8:06 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Amos Kong @ 2013-05-28 7:40 UTC (permalink / raw)
To: seabios; +Cc: pbonzini, kevin, qemu-devel
bootindex parameter of scsi device doesn't work, it causes
by wrong pattern in seabios.
qemu passes the following firmware dev_path to seabios:
/pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
Signed-off-by: Amos Kong <akong@redhat.com>
---
src/boot.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/boot.c b/src/boot.c
index c308602..cd9d784 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
if (!pci)
// support only pci machine for now
return -1;
- // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
+ /* Find scsi drive - for example:
+ /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
char desc[256], *p;
p = build_pci_path(desc, sizeof(desc), "*", pci);
- snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
+ snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
return find_prio(desc);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
2013-05-28 7:40 [Qemu-devel] [PATCH] boot: fix path pattern of scsi device Amos Kong
@ 2013-05-28 8:06 ` Paolo Bonzini
2013-05-28 9:35 ` Amos Kong
2013-05-28 10:26 ` Laszlo Ersek
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-05-28 8:06 UTC (permalink / raw)
To: Amos Kong; +Cc: kevin, seabios, qemu-devel, KONRAD Frédéric
Il 28/05/2013 09:40, Amos Kong ha scritto:
> bootindex parameter of scsi device doesn't work, it causes
> by wrong pattern in seabios.
>
> qemu passes the following firmware dev_path to seabios:
> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
No, this is another unexpected change due to the virtio refactoring in
QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
implementation in hw/virtio/virtio-bus.c.
We fixed it already for migration paths, it should be easy to do the
same for this.
Please Cc qemu-stable@nongnu.org when sending the QEMU patch.
Thanks,
Paolo
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> src/boot.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index c308602..cd9d784 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
> if (!pci)
> // support only pci machine for now
> return -1;
> - // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
> + /* Find scsi drive - for example:
> + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
> char desc[256], *p;
> p = build_pci_path(desc, sizeof(desc), "*", pci);
> - snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
> + snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
> return find_prio(desc);
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
2013-05-28 8:06 ` Paolo Bonzini
@ 2013-05-28 9:35 ` Amos Kong
2013-05-28 9:45 ` Paolo Bonzini
2013-05-28 10:26 ` Laszlo Ersek
1 sibling, 1 reply; 6+ messages in thread
From: Amos Kong @ 2013-05-28 9:35 UTC (permalink / raw)
To: Paolo Bonzini, sluo; +Cc: kevin, seabios, qemu-devel, KONRAD Frédéric
On Tue, May 28, 2013 at 10:06:51AM +0200, Paolo Bonzini wrote:
> Il 28/05/2013 09:40, Amos Kong ha scritto:
> > bootindex parameter of scsi device doesn't work, it causes
> > by wrong pattern in seabios.
> >
> > qemu passes the following firmware dev_path to seabios:
> > /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
>
> No, this is another unexpected change due to the virtio refactoring in
> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
> implementation in hw/virtio/virtio-bus.c.
Hi Paolo,
We could not fix this by implementing get_fw_dev_path in
hw/virtio/virtio-bus.c
virtio-bus is the parent bus of scsi-bus, scsibus_get_fw_dev_path()
will be called after called virtio_bus_get_fw_dev_path().
str0 = returns of parents of virtio-bus /* eg: /pci@i0cf8/scsi@4 */
str1 = return of virtio_bus_get_fw_dev_path() /* eg: .. */
str2 = scsibus_get_fw_dev_path() /* eg: channel@0/disk@0,0 */
The final fw_dev_path should be $str0/$str1/$str2
| static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
| {
| int l = 0;
|
| if (dev && dev->parent_bus) {
| char *d;
| l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
| d = bus_get_fw_dev_path(dev->parent_bus, dev);
| if (d) {
| l += snprintf(p + l, size - l, "%s", d);
| g_free(d);
if we implement virtio_bus_get_fw_dev_path(), the return value will be set to $str1
| } else {
| l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev)));
Currently we didn't implement virtio_bus_get_fw_dev_path(), so 'virtio-scsi-device' was set to $str1
| }
|
However, we need to change the pattern.
Amos.
> We fixed it already for migration paths, it should be easy to do the
> same for this.
>
> Please Cc qemu-stable@nongnu.org when sending the QEMU patch.
>
> Thanks,
>
> Paolo
>
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> > src/boot.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/boot.c b/src/boot.c
> > index c308602..cd9d784 100644
> > --- a/src/boot.c
> > +++ b/src/boot.c
> > @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
> > if (!pci)
> > // support only pci machine for now
> > return -1;
> > - // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
> > + /* Find scsi drive - for example:
> > + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
> > char desc[256], *p;
> > p = build_pci_path(desc, sizeof(desc), "*", pci);
> > - snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
> > + snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
> > return find_prio(desc);
> > }
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
2013-05-28 9:35 ` Amos Kong
@ 2013-05-28 9:45 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-05-28 9:45 UTC (permalink / raw)
To: Amos Kong; +Cc: sluo, kevin, seabios, qemu-devel, KONRAD Frédéric
Il 28/05/2013 11:35, Amos Kong ha scritto:
> On Tue, May 28, 2013 at 10:06:51AM +0200, Paolo Bonzini wrote:
>> Il 28/05/2013 09:40, Amos Kong ha scritto:
>>> bootindex parameter of scsi device doesn't work, it causes
>>> by wrong pattern in seabios.
>>>
>>> qemu passes the following firmware dev_path to seabios:
>>> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
>>
>> No, this is another unexpected change due to the virtio refactoring in
>> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
>> implementation in hw/virtio/virtio-bus.c.
>
> Hi Paolo,
>
> We could not fix this by implementing get_fw_dev_path in
> hw/virtio/virtio-bus.c
>
> virtio-bus is the parent bus of scsi-bus, scsibus_get_fw_dev_path()
> will be called after called virtio_bus_get_fw_dev_path().
>
> str0 = returns of parents of virtio-bus /* eg: /pci@i0cf8/scsi@4 */
> str1 = return of virtio_bus_get_fw_dev_path() /* eg: .. */
> str2 = scsibus_get_fw_dev_path() /* eg: channel@0/disk@0,0 */
>
> The final fw_dev_path should be $str0/$str1/$str2
>
> | static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> | {
> | int l = 0;
> |
> | if (dev && dev->parent_bus) {
> | char *d;
> | l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
> | d = bus_get_fw_dev_path(dev->parent_bus, dev);
> | if (d) {
> | l += snprintf(p + l, size - l, "%s", d);
> | g_free(d);
>
> if we implement virtio_bus_get_fw_dev_path(), the return value will be set to $str1
>
> | } else {
> | l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev)));
>
> Currently we didn't implement virtio_bus_get_fw_dev_path(), so 'virtio-scsi-device' was set to $str1
>
> | }
> |
>
> However, we need to change the pattern.
Turn this:
l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev)));
into the default implementation of get_fw_dev_path (in TYPE_BUS),
and change qdev_get_fw_dev_path_helper to
if (d) {
l += snprintf(p + l, size - l, "%s", d);
g_free(d);
} else {
return l;
}
Then virtio_bus_get_fw_dev_path can just return NULL.
Paolo
> Amos.
>
>> We fixed it already for migration paths, it should be easy to do the
>> same for this.
>>
>> Please Cc qemu-stable@nongnu.org when sending the QEMU patch.
>>
>> Thanks,
>>
>> Paolo
>>
>>> Signed-off-by: Amos Kong <akong@redhat.com>
>>> ---
>>> src/boot.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/boot.c b/src/boot.c
>>> index c308602..cd9d784 100644
>>> --- a/src/boot.c
>>> +++ b/src/boot.c
>>> @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci, int target, int lun)
>>> if (!pci)
>>> // support only pci machine for now
>>> return -1;
>>> - // Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
>>> + /* Find scsi drive - for example:
>>> + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
>>> char desc[256], *p;
>>> p = build_pci_path(desc, sizeof(desc), "*", pci);
>>> - snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
>>> + snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
>>> return find_prio(desc);
>>> }
>>>
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
2013-05-28 8:06 ` Paolo Bonzini
2013-05-28 9:35 ` Amos Kong
@ 2013-05-28 10:26 ` Laszlo Ersek
2013-05-28 10:31 ` Amos Kong
1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2013-05-28 10:26 UTC (permalink / raw)
To: Paolo Bonzini, Amos Kong
Cc: kevin, seabios, qemu-devel, KONRAD Frédéric
On 05/28/13 10:06, Paolo Bonzini wrote:
> Il 28/05/2013 09:40, Amos Kong ha scritto:
>> bootindex parameter of scsi device doesn't work, it causes
>> by wrong pattern in seabios.
>>
>> qemu passes the following firmware dev_path to seabios:
>> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
>
> No, this is another unexpected change due to the virtio refactoring in
> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
> implementation in hw/virtio/virtio-bus.c.
>
> We fixed it already for migration paths, it should be easy to do the
> same for this.
>
> Please Cc qemu-stable@nongnu.org when sending the QEMU patch.
>
> Thanks,
>
> Paolo
Ahhh. I was super confused by this patch initially.
Amos, when posting a patch to both lists, please add the project name to
the bracketed bag-of-tags in the subject, like
[SeaBIOS PATCH] boot: fix path pattern of scsi device
I saw this message first on qemu-devel, and until I noticed "src/boot.c"
I was kind of confused whom you want to adapt to whom, and in what
direction Paolo argues against it.
So, virtio refactoring in QEMU (care to name a commit or release?)
changed the OpenFirmware device path exported for virtio-scsi devices
under the boot order fw_cfg key. This patch intended to adapt SeaBIOS to
recognize the new OFW devpath. Under this approach I would have to
update QemuBootOrder.c in OVMF in parallel, so that it accepts both old
and new style OFW devpaths for virtio-scsi.
However Paolo says the new style OFW devpath should be fixed
(eliminated) in qemu, and consumers shouldn't notice any change in the
long term. And I won't have to change QemuBootOrder.c. Right?
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
2013-05-28 10:26 ` Laszlo Ersek
@ 2013-05-28 10:31 ` Amos Kong
0 siblings, 0 replies; 6+ messages in thread
From: Amos Kong @ 2013-05-28 10:31 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Paolo Bonzini, kevin, seabios, qemu-devel,
KONRAD Frédéric
On Tue, May 28, 2013 at 12:26:34PM +0200, Laszlo Ersek wrote:
> On 05/28/13 10:06, Paolo Bonzini wrote:
> > Il 28/05/2013 09:40, Amos Kong ha scritto:
> >> bootindex parameter of scsi device doesn't work, it causes
> >> by wrong pattern in seabios.
> >>
> >> qemu passes the following firmware dev_path to seabios:
> >> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
> >
> > No, this is another unexpected change due to the virtio refactoring in
> > QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
> > implementation in hw/virtio/virtio-bus.c.
> >
> > We fixed it already for migration paths, it should be easy to do the
> > same for this.
> >
> > Please Cc qemu-stable@nongnu.org when sending the QEMU patch.
> >
> > Thanks,
> >
> > Paolo
>
> Ahhh. I was super confused by this patch initially.
>
> Amos, when posting a patch to both lists, please add the project name to
> the bracketed bag-of-tags in the subject, like
>
> [SeaBIOS PATCH] boot: fix path pattern of scsi device
Sorry for the mistiness.
> I saw this message first on qemu-devel, and until I noticed "src/boot.c"
> I was kind of confused whom you want to adapt to whom, and in what
> direction Paolo argues against it.
>
> So, virtio refactoring in QEMU (care to name a commit or release?)
> changed the OpenFirmware device path exported for virtio-scsi devices
> under the boot order fw_cfg key. This patch intended to adapt SeaBIOS to
> recognize the new OFW devpath. Under this approach I would have to
> update QemuBootOrder.c in OVMF in parallel, so that it accepts both old
> and new style OFW devpaths for virtio-scsi.
>
> However Paolo says the new style OFW devpath should be fixed
> (eliminated) in qemu, and consumers shouldn't notice any change in the
> long term. And I won't have to change QemuBootOrder.c. Right?
Just sent a qemu patch to fix this problem. We will keep original
style devpath.
> Thanks!
> Laszlo
--
Amos.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-28 10:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-28 7:40 [Qemu-devel] [PATCH] boot: fix path pattern of scsi device Amos Kong
2013-05-28 8:06 ` Paolo Bonzini
2013-05-28 9:35 ` Amos Kong
2013-05-28 9:45 ` Paolo Bonzini
2013-05-28 10:26 ` Laszlo Ersek
2013-05-28 10:31 ` Amos Kong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).