Re: [PATCH 2/3] qcow2: refactor handle_dependencies() loop body

2021-08-20 Thread Hanna Reitz

On 24.07.21 15:38, Vladimir Sementsov-Ogievskiy wrote:

No logic change, just prepare for the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/qcow2-cluster.c | 49 ---
  1 file changed, 28 insertions(+), 21 deletions(-)


Reviewed-by: Hanna Reitz 




Re: [PATCH 2/3] qcow2: refactor handle_dependencies() loop body

2021-08-19 Thread Eric Blake
On Sat, Jul 24, 2021 at 04:38:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> No logic change, just prepare for the following commit.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-cluster.c | 49 ---
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index bd0597842f..967121c7e6 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1400,29 +1400,36 @@ static int handle_dependencies(BlockDriverState *bs, 
> uint64_t guest_offset,
>  
>  if (end <= old_start || start >= old_end) {
>  /* No intersection */
> -} else {
> -if (start < old_start) {
> -/* Stop at the start of a running allocation */
> -bytes = old_start - start;
> -} else {
> -bytes = 0;
> -}
> +continue;
> +}
>  
> -/* Stop if already an l2meta exists. After yielding, it wouldn't

Pre-existing, but...

> - * be valid any more, so we'd have to clean up the old L2Metas
> - * and deal with requests depending on them before starting to
> - * gather new ones. Not worth the trouble. */
> -if (bytes == 0 && *m) {
> -*cur_bytes = 0;
> -return 0;
> -}
> +/* Conflict */
>  
> -if (bytes == 0) {
> -/* Wait for the dependency to complete. We need to recheck
> - * the free/allocated clusters when we continue. */
> -qemu_co_queue_wait(_alloc->dependent_requests, >lock);
> -return -EAGAIN;
> -}
> +if (start < old_start) {
> +/* Stop at the start of a running allocation */
> +bytes = old_start - start;
> +} else {
> +bytes = 0;
> +}
> +
> +/*
> + * Stop if already an l2meta exists. After yielding, it wouldn't

...might as well fix the grammar:  Stop if an l2meta already exists.

> + * be valid any more, so we'd have to clean up the old L2Metas
> + * and deal with requests depending on them before starting to
> + * gather new ones. Not worth the trouble.
> + */
> +if (bytes == 0 && *m) {
> +*cur_bytes = 0;
> +return 0;
> +}
> +
> +if (bytes == 0) {
> +/*
> + * Wait for the dependency to complete. We need to recheck
> + * the free/allocated clusters when we continue.
> + */
> +qemu_co_queue_wait(_alloc->dependent_requests, >lock);
> +return -EAGAIN;

So the change adds a short-circuiting 'continue', then reduces the
indentation of the rest of the loop body.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




[PATCH 2/3] qcow2: refactor handle_dependencies() loop body

2021-07-24 Thread Vladimir Sementsov-Ogievskiy
No logic change, just prepare for the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-cluster.c | 49 ---
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index bd0597842f..967121c7e6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1400,29 +1400,36 @@ static int handle_dependencies(BlockDriverState *bs, 
uint64_t guest_offset,
 
 if (end <= old_start || start >= old_end) {
 /* No intersection */
-} else {
-if (start < old_start) {
-/* Stop at the start of a running allocation */
-bytes = old_start - start;
-} else {
-bytes = 0;
-}
+continue;
+}
 
-/* Stop if already an l2meta exists. After yielding, it wouldn't
- * be valid any more, so we'd have to clean up the old L2Metas
- * and deal with requests depending on them before starting to
- * gather new ones. Not worth the trouble. */
-if (bytes == 0 && *m) {
-*cur_bytes = 0;
-return 0;
-}
+/* Conflict */
 
-if (bytes == 0) {
-/* Wait for the dependency to complete. We need to recheck
- * the free/allocated clusters when we continue. */
-qemu_co_queue_wait(_alloc->dependent_requests, >lock);
-return -EAGAIN;
-}
+if (start < old_start) {
+/* Stop at the start of a running allocation */
+bytes = old_start - start;
+} else {
+bytes = 0;
+}
+
+/*
+ * Stop if already an l2meta exists. After yielding, it wouldn't
+ * be valid any more, so we'd have to clean up the old L2Metas
+ * and deal with requests depending on them before starting to
+ * gather new ones. Not worth the trouble.
+ */
+if (bytes == 0 && *m) {
+*cur_bytes = 0;
+return 0;
+}
+
+if (bytes == 0) {
+/*
+ * Wait for the dependency to complete. We need to recheck
+ * the free/allocated clusters when we continue.
+ */
+qemu_co_queue_wait(_alloc->dependent_requests, >lock);
+return -EAGAIN;
 }
 }
 
-- 
2.29.2