[Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When scsi_bus_legacy_add_drive() produces an error,
we will lose the error message. Using error_report
to report it.

Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
v4:
 fix typo in the subject (Eric), Thanks.
v3:
 fix some typos/grammar issues (Eric) and add 'R-by' tag
v2: 
 using original condition instead of local_err (Markus)
---
 hw/pci/pci-hotplug-old.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index cf2caeb..d87c469 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -107,6 +107,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
 {
     SCSIBus *scsibus;
     SCSIDevice *scsidev;
+    Error *local_err = NULL;
 
     scsibus = (SCSIBus *)
         object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
@@ -127,8 +128,10 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
     dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
     dinfo->bus = scsibus->busnr;
     scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
-                                        false, -1, NULL, NULL);
+                                        false, -1, NULL, &local_err);
     if (!scsidev) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
         return -1;
     }
     dinfo->unit = scsidev->id;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Gonglei (Arei)]

Hi, Michael

Would you like to pick up this patch, which had
been reviewed by Markus and Eric? 

Thanks. :)


Best regards,
-Gonglei

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Thursday, September 18, 2014 9:06 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; Huangweidong (C); armbru@redhat.com;
> eblake@redhat.com; Gonglei (Arei)
> Subject: [PATCH v4] pci-hotplug-old: avoid losing error message
> 
> From: Gonglei <arei.gonglei@huawei.com>
> 
> When scsi_bus_legacy_add_drive() produces an error,
> we will lose the error message. Using error_report
> to report it.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> v4:
>  fix typo in the subject (Eric), Thanks.
> v3:
>  fix some typos/grammar issues (Eric) and add 'R-by' tag
> v2:
>  using original condition instead of local_err (Markus)
> ---
>  hw/pci/pci-hotplug-old.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index cf2caeb..d87c469 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -107,6 +107,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState
> *adapter,
>  {
>      SCSIBus *scsibus;
>      SCSIDevice *scsidev;
> +    Error *local_err = NULL;
> 
>      scsibus = (SCSIBus *)
>          object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)),
> @@ -127,8 +128,10 @@ static int scsi_hot_add(Monitor *mon, DeviceState
> *adapter,
>      dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
>      dinfo->bus = scsibus->busnr;
>      scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> -                                        false, -1, NULL, NULL);
> +                                        false, -1, NULL, &local_err);
>      if (!scsidev) {
> +        error_report("%s", error_get_pretty(local_err));
> +        error_free(local_err);
>          return -1;
>      }
>      dinfo->unit = scsidev->id;
> --
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Paolo Bonzini]

Il 18/09/2014 15:06, arei.gonglei@huawei.com ha scritto:
> we will lose the error message. Using error_report
> to report it.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> v4:
>  fix typo in the subject (Eric), Thanks.
> v3:
>  fix some typos/grammar issues (Eric) and add 'R-by' tag
> v2: 
>  using original condition instead of local_err (Markus)
> ---
>  hw/pci/pci-hotplug-old.c | 5 ++++-

Hi Gonglei, since this is for an HMP-only command, please use
qerror_report_err like qemu_pci_hot_add_nic does.

Sorry for not noticing this patch earlier.

Paolo

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Gonglei (Arei)]

Hi,

> Subject: Re: [PATCH v4] pci-hotplug-old: avoid losing error message
> 
> Il 18/09/2014 15:06, arei.gonglei@huawei.com ha scritto:
> > we will lose the error message. Using error_report
> > to report it.
> >
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > ---
> > v4:
> >  fix typo in the subject (Eric), Thanks.
> > v3:
> >  fix some typos/grammar issues (Eric) and add 'R-by' tag
> > v2:
> >  using original condition instead of local_err (Markus)
> > ---
> >  hw/pci/pci-hotplug-old.c | 5 ++++-
> 
> Hi Gonglei, since this is for an HMP-only command, please use
> qerror_report_err like qemu_pci_hot_add_nic does.
> 
OK. Thanks. v5 will be posted.

> Sorry for not noticing this patch earlier.
> 
That's fine. :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 18/09/2014 15:06, arei.gonglei@huawei.com ha scritto:
>> we will lose the error message. Using error_report
>> to report it.
>> 
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> v4:
>>  fix typo in the subject (Eric), Thanks.
>> v3:
>>  fix some typos/grammar issues (Eric) and add 'R-by' tag
>> v2: 
>>  using original condition instead of local_err (Markus)
>> ---
>>  hw/pci/pci-hotplug-old.c | 5 ++++-
>
> Hi Gonglei, since this is for an HMP-only command, please use
> qerror_report_err like qemu_pci_hot_add_nic does.
>
> Sorry for not noticing this patch earlier.

error_report() is just fine here.  qerror_report() is never necessary in
HMP-only code.  It makes a difference only in QMP context, and even
there it should only be used in code that hasn't been converted to the
Error API.  For an example, see commit b1422f2.

Eventually, qerror.h should just die.  I've been chipping away at it
when I have nothing better to do.  Slow progress, better than no
progress.

Let's commit v4.

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Paolo Bonzini]

Il 30/09/2014 10:12, Markus Armbruster ha scritto:
> error_report() is just fine here.  qerror_report() is never necessary in
> HMP-only code.  It makes a difference only in QMP context, and even
> there it should only be used in code that hasn't been converted to the
> Error API.  For an example, see commit b1422f2.
> 
> Eventually, qerror.h should just die.  I've been chipping away at it
> when I have nothing better to do.  Slow progress, better than no
> progress.
> 
> Let's commit v4.

Yeah, I've seen now that error_report ends up in monitor_vprintf.

v4 is okay, sorry Gonglei.

Paolo

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Gonglei (Arei)]

> Subject: Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error
> message
> 
> Il 30/09/2014 10:12, Markus Armbruster ha scritto:
> > error_report() is just fine here.  qerror_report() is never necessary in
> > HMP-only code.  It makes a difference only in QMP context, and even
> > there it should only be used in code that hasn't been converted to the
> > Error API.  For an example, see commit b1422f2.
> >
> > Eventually, qerror.h should just die.  I've been chipping away at it
> > when I have nothing better to do.  Slow progress, better than no
> > progress.
> >
> > Let's commit v4.
> 
> Yeah, I've seen now that error_report ends up in monitor_vprintf.
> 
> v4 is okay, sorry Gonglei.
> 
> Paolo

I'm fine, I'm a learner in some area. :)

Who can merge this patch? Paolo, can you? Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error message

[Michael S. Tsirkin]

On Tue, Sep 30, 2014 at 08:23:05AM +0000, Gonglei (Arei) wrote:
> > Subject: Re: [Qemu-devel] [PATCH v4] pci-hotplug-old: avoid losing error
> > message
> > 
> > Il 30/09/2014 10:12, Markus Armbruster ha scritto:
> > > error_report() is just fine here.  qerror_report() is never necessary in
> > > HMP-only code.  It makes a difference only in QMP context, and even
> > > there it should only be used in code that hasn't been converted to the
> > > Error API.  For an example, see commit b1422f2.
> > >
> > > Eventually, qerror.h should just die.  I've been chipping away at it
> > > when I have nothing better to do.  Slow progress, better than no
> > > progress.
> > >
> > > Let's commit v4.
> > 
> > Yeah, I've seen now that error_report ends up in monitor_vprintf.
> > 
> > v4 is okay, sorry Gonglei.
> > 
> > Paolo
> 
> I'm fine, I'm a learner in some area. :)
> 
> Who can merge this patch? Paolo, can you? Thanks!
> 
> Best regards,
> -Gonglei


v4 is already applied in my tree, thanks everyone.