| Message ID | 20231214003117.43960-1-dongli.zhang@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] vhost-scsi: fix usage of error_reportf_err() | expand |
> On Dec 14, 2023, at 08:31, Dongli Zhang <dongli.zhang@oracle.com> wrote: > > It is required to use error_report() instead of error_reportf_err(), if the > prior function does not take local_err as the argument. As a result, the > local_err is always NULL and segment fault may happen. > > vhost_scsi_start() > -> vhost_scsi_set_endpoint(s) --> does not allocate local_err > -> error_reportf_err() > -> error_vprepend() > -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL > > In addition, add ": " at the end of other error_reportf_err() logs. > > Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend") > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> > --- > hw/scsi/vhost-scsi.c | 4 ++-- > hw/scsi/vhost-user-scsi.c | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index 3126df9e1d..9929c0d14b 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s) > > ret = vhost_scsi_common_start(vsc, &local_err); > if (ret < 0) { > - error_reportf_err(local_err, "Error starting vhost-scsi"); > + error_reportf_err(local_err, "Error starting vhost-scsi: "); > return ret; > } > > ret = vhost_scsi_set_endpoint(s); > if (ret < 0) { > - error_reportf_err(local_err, "Error setting vhost-scsi endpoint"); > + error_report("Error setting vhost-scsi endpoint"); > vhost_scsi_common_stop(vsc); > } > > diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c > index 780f10559d..af18c4f3d3 100644 > --- a/hw/scsi/vhost-user-scsi.c > +++ b/hw/scsi/vhost-user-scsi.c > @@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) > if (should_start) { > ret = vhost_user_scsi_start(s, &local_err); > if (ret < 0) { > - error_reportf_err(local_err, "unable to start vhost-user-scsi: %s", > + error_reportf_err(local_err, > + "unable to start vhost-user-scsi: %s: ", > strerror(-ret)); > qemu_chr_fe_disconnect(&vs->conf.chardev); > } > -- > 2.34.1 > Looks good. Reviewed-by: Feng Li <fengli@smartx.com> Thanks.
> On Dec 13, 2023, at 7:31 PM, Dongli Zhang <dongli.zhang@oracle.com> wrote: > > It is required to use error_report() instead of error_reportf_err(), if the > prior function does not take local_err as the argument. As a result, the > local_err is always NULL and segment fault may happen. > > vhost_scsi_start() > -> vhost_scsi_set_endpoint(s) --> does not allocate local_err > -> error_reportf_err() > -> error_vprepend() > -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL > > In addition, add ": " at the end of other error_reportf_err() logs. > > Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend") > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > --- > hw/scsi/vhost-scsi.c | 4 ++-- > hw/scsi/vhost-user-scsi.c | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index 3126df9e1d..9929c0d14b 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s) > > ret = vhost_scsi_common_start(vsc, &local_err); > if (ret < 0) { > - error_reportf_err(local_err, "Error starting vhost-scsi"); > + error_reportf_err(local_err, "Error starting vhost-scsi: "); > return ret; > } > > ret = vhost_scsi_set_endpoint(s); > if (ret < 0) { > - error_reportf_err(local_err, "Error setting vhost-scsi endpoint"); > + error_report("Error setting vhost-scsi endpoint"); > vhost_scsi_common_stop(vsc); > } > > diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c > index 780f10559d..af18c4f3d3 100644 > --- a/hw/scsi/vhost-user-scsi.c > +++ b/hw/scsi/vhost-user-scsi.c > @@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) > if (should_start) { > ret = vhost_user_scsi_start(s, &local_err); > if (ret < 0) { > - error_reportf_err(local_err, "unable to start vhost-user-scsi: %s", > + error_reportf_err(local_err, > + "unable to start vhost-user-scsi: %s: ", > strerror(-ret)); > qemu_chr_fe_disconnect(&vs->conf.chardev); > } > -- > 2.34.1 >
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 3126df9e1d..9929c0d14b 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s) ret = vhost_scsi_common_start(vsc, &local_err); if (ret < 0) { - error_reportf_err(local_err, "Error starting vhost-scsi"); + error_reportf_err(local_err, "Error starting vhost-scsi: "); return ret; } ret = vhost_scsi_set_endpoint(s); if (ret < 0) { - error_reportf_err(local_err, "Error setting vhost-scsi endpoint"); + error_report("Error setting vhost-scsi endpoint"); vhost_scsi_common_stop(vsc); } diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 780f10559d..af18c4f3d3 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) if (should_start) { ret = vhost_user_scsi_start(s, &local_err); if (ret < 0) { - error_reportf_err(local_err, "unable to start vhost-user-scsi: %s", + error_reportf_err(local_err, + "unable to start vhost-user-scsi: %s: ", strerror(-ret)); qemu_chr_fe_disconnect(&vs->conf.chardev); }
It is required to use error_report() instead of error_reportf_err(), if the prior function does not take local_err as the argument. As a result, the local_err is always NULL and segment fault may happen. vhost_scsi_start() -> vhost_scsi_set_endpoint(s) --> does not allocate local_err -> error_reportf_err() -> error_vprepend() -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL In addition, add ": " at the end of other error_reportf_err() logs. Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend") Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> --- hw/scsi/vhost-scsi.c | 4 ++-- hw/scsi/vhost-user-scsi.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-)