[Qemu-devel] [PATCH] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly

Philippe Mathieu-Daudé posted 1 patch 4 years, 10 months ago
Test s390x failed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190528140750.28036-1-philmd@redhat.com
Maintainers: Fam Zheng <fam@euphon.net>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>
hw/scsi/vmw_pvscsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago
Since the BusState is accesible from the SCSIBus object,
it is pointless to use qbus_reset_all_fn.
Use qbus_reset_all() directly.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
One step toward removing qbus_reset_all_fn()
---
 hw/scsi/vmw_pvscsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 584b4be07e..6f571d0d19 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -440,7 +440,7 @@ static void
 pvscsi_reset_adapter(PVSCSIState *s)
 {
     s->resetting++;
-    qbus_reset_all_fn(&s->bus);
+    qbus_reset_all(&s->bus.qbus);
     s->resetting--;
     pvscsi_process_completion_queue(s);
     assert(QTAILQ_EMPTY(&s->pending_queue));
@@ -848,7 +848,7 @@ pvscsi_on_cmd_reset_bus(PVSCSIState *s)
     trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_RESET_BUS");
 
     s->resetting++;
-    qbus_reset_all_fn(&s->bus);
+    qbus_reset_all(&s->bus.qbus);
     s->resetting--;
     return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
 }
-- 
2.20.1


Re: [Qemu-devel] [PATCH] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly
Posted by Peter Maydell 4 years, 10 months ago
On Tue, 28 May 2019 at 15:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Since the BusState is accesible from the SCSIBus object,
> it is pointless to use qbus_reset_all_fn.
> Use qbus_reset_all() directly.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> One step toward removing qbus_reset_all_fn()
> ---
>  hw/scsi/vmw_pvscsi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 584b4be07e..6f571d0d19 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -440,7 +440,7 @@ static void
>  pvscsi_reset_adapter(PVSCSIState *s)
>  {
>      s->resetting++;
> -    qbus_reset_all_fn(&s->bus);
> +    qbus_reset_all(&s->bus.qbus);

I thought our QOM style prefers using "BUS(&s->bus)"
rather than looking inside the definition of the
SCSIBus struct with "s->bus.qbus" ? (Compare preferring
"DEVICE(s)" to "s->qdev".)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago
On 5/28/19 4:12 PM, Peter Maydell wrote:
> On Tue, 28 May 2019 at 15:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Since the BusState is accesible from the SCSIBus object,
>> it is pointless to use qbus_reset_all_fn.
>> Use qbus_reset_all() directly.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> One step toward removing qbus_reset_all_fn()
>> ---
>>  hw/scsi/vmw_pvscsi.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
>> index 584b4be07e..6f571d0d19 100644
>> --- a/hw/scsi/vmw_pvscsi.c
>> +++ b/hw/scsi/vmw_pvscsi.c
>> @@ -440,7 +440,7 @@ static void
>>  pvscsi_reset_adapter(PVSCSIState *s)
>>  {
>>      s->resetting++;
>> -    qbus_reset_all_fn(&s->bus);
>> +    qbus_reset_all(&s->bus.qbus);
> 
> I thought our QOM style prefers using "BUS(&s->bus)"
> rather than looking inside the definition of the
> SCSIBus struct with "s->bus.qbus" ? (Compare preferring
> "DEVICE(s)" to "s->qdev".)

This is not explicit in HACKING/CODING_STYLE, but I agree this is
cleaner and better to treat the generic QOM objects as opaque, and use
the macros to check for the QOM type.

I'll respin.