Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two
17.03.2020 22:03, Markus Armbruster wrote:
Uh, I failed to actually send this. It's in my pull request now.
Vladimir Sementsov-Ogievskiy writes:
13.03.2020 20:05, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster
---
hw/block/xen-block.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..7b3b6dee97 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,14 +998,13 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
@@ -1015,8 +1014,6 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
Hmm, no "return;" statement after this propagation. It's OK, as there no more code in the
function after this "if", but I'd add it to be consistent and to avoid forgetting to add
a return here when add more code to the function.
(and if you do this, you may also fix indentation of string paramter of
error_propagate_prepend...)
Anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy
Like this, I guess:
xen-block: Use one Error * variable instead of two
While there, tidy up indentation, and add return just for consistency
and robustness.
Signed-off-by: Markus Armbruster
Message-Id: <20200313170517.22480-4-arm...@redhat.com>
Reviewed-by: Peter Maydell
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Vladimir Sementsov-Ogievskiy
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..07bb32e22b 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,29 +998,27 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
-"failed to destroy iothread: ");
+"failed to destroy iothread: ");
return;
}
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
-"failed to destroy drive: ");
+"failed to destroy drive: ");
+return;
}
}
}
Yes, that's what I meant, thanks!
--
Best regards,
Vladimir
Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two
Uh, I failed to actually send this. It's in my pull request now.
Vladimir Sementsov-Ogievskiy writes:
> 13.03.2020 20:05, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster
>> ---
>> hw/block/xen-block.c | 5 +
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
>> index 3885464513..7b3b6dee97 100644
>> --- a/hw/block/xen-block.c
>> +++ b/hw/block/xen-block.c
>> @@ -998,14 +998,13 @@ static void
>> xen_block_device_destroy(XenBackendInstance *backend,
>> XenBlockVdev *vdev = >props.vdev;
>> XenBlockDrive *drive = blockdev->drive;
>> XenBlockIOThread *iothread = blockdev->iothread;
>> +Error *local_err = NULL;
>> trace_xen_block_device_destroy(vdev->number);
>> object_unparent(OBJECT(xendev));
>> if (iothread) {
>> -Error *local_err = NULL;
>> -
>> xen_block_iothread_destroy(iothread, _err);
>> if (local_err) {
>> error_propagate_prepend(errp, local_err,
>> @@ -1015,8 +1014,6 @@ static void
>> xen_block_device_destroy(XenBackendInstance *backend,
>> }
>> if (drive) {
>> -Error *local_err = NULL;
>> -
>> xen_block_drive_destroy(drive, _err);
>> if (local_err) {
>> error_propagate_prepend(errp, local_err,
>
> Hmm, no "return;" statement after this propagation. It's OK, as there no more
> code in the function after this "if", but I'd add it to be consistent and to
> avoid forgetting to add a return here when add more code to the function.
>
> (and if you do this, you may also fix indentation of string paramter of
> error_propagate_prepend...)
>
>
>
> Anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy
Like this, I guess:
xen-block: Use one Error * variable instead of two
While there, tidy up indentation, and add return just for consistency
and robustness.
Signed-off-by: Markus Armbruster
Message-Id: <20200313170517.22480-4-arm...@redhat.com>
Reviewed-by: Peter Maydell
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Vladimir Sementsov-Ogievskiy
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..07bb32e22b 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,29 +998,27 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
-"failed to destroy iothread: ");
+"failed to destroy iothread: ");
return;
}
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
-"failed to destroy drive: ");
+"failed to destroy drive: ");
+return;
}
}
}
Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two
13.03.2020 20:05, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster
---
hw/block/xen-block.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..7b3b6dee97 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,14 +998,13 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
@@ -1015,8 +1014,6 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
Hmm, no "return;" statement after this propagation. It's OK, as there no more code in the
function after this "if", but I'd add it to be consistent and to avoid forgetting to add
a return here when add more code to the function.
(and if you do this, you may also fix indentation of string paramter of
error_propagate_prepend...)
Anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two
On 3/13/20 6:05 PM, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster
---
hw/block/xen-block.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..7b3b6dee97 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,14 +998,13 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
@@ -1015,8 +1014,6 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two
On 3/13/20 12:05 PM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- hw/block/xen-block.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH 3/3] xen-block: Use one Error * variable instead of two
Signed-off-by: Markus Armbruster
---
hw/block/xen-block.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..7b3b6dee97 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,14 +998,13 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
XenBlockVdev *vdev = >props.vdev;
XenBlockDrive *drive = blockdev->drive;
XenBlockIOThread *iothread = blockdev->iothread;
+Error *local_err = NULL;
trace_xen_block_device_destroy(vdev->number);
object_unparent(OBJECT(xendev));
if (iothread) {
-Error *local_err = NULL;
-
xen_block_iothread_destroy(iothread, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
@@ -1015,8 +1014,6 @@ static void xen_block_device_destroy(XenBackendInstance
*backend,
}
if (drive) {
-Error *local_err = NULL;
-
xen_block_drive_destroy(drive, _err);
if (local_err) {
error_propagate_prepend(errp, local_err,
--
2.21.1
