Patchwork Avoid divide by zero when there is no block device to migrate

login
register
mail settings
Submitter Pierre Riteau
Date Dec. 25, 2010, 8:52 p.m.
Message ID <1293310330-12067-1-git-send-email-Pierre.Riteau@irisa.fr>
Download mbox | patch
Permalink /patch/76701/
State New
Headers show

Comments

Pierre Riteau - Dec. 25, 2010, 8:52 p.m.
When block migration is requested and no read-write block device is
present, a divide by zero exception is triggered because
total_sector_sum equals zero.

Signed-off-by: Pierre Riteau <Pierre.Riteau@irisa.fr>
---
 block-migration.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Pierre Riteau - Jan. 9, 2011, 7:37 p.m.
On 25 déc. 2010, at 21:52, Pierre Riteau wrote:

> When block migration is requested and no read-write block device is
> present, a divide by zero exception is triggered because
> total_sector_sum equals zero.
> 
> Signed-off-by: Pierre Riteau <Pierre.Riteau@irisa.fr>
> ---
> block-migration.c |    6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/block-migration.c b/block-migration.c
> index 1475325..d62d63e 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -350,7 +350,11 @@ static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f)
>         }
>     }
> 
> -    progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
> +    if (block_mig_state.total_sector_sum != 0) {
> +        progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
> +    } else {
> +        progress = 100;
> +    }
>     if (progress != block_mig_state.prev_progress) {
>         block_mig_state.prev_progress = progress;
>         qemu_put_be64(f, (progress << BDRV_SECTOR_BITS)
> -- 
> 1.7.3.4
> 


Comments, anyone?
Kevin Wolf - Jan. 11, 2011, 11:54 a.m.
Am 25.12.2010 21:52, schrieb Pierre Riteau:
> When block migration is requested and no read-write block device is
> present, a divide by zero exception is triggered because
> total_sector_sum equals zero.
> 
> Signed-off-by: Pierre Riteau <Pierre.Riteau@irisa.fr>

Maybe in this case we should generate an error before actually starting
with block migration. If you bothered to request block migration you
certainly didn't have in mind to do nothing.

> ---
>  block-migration.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/block-migration.c b/block-migration.c
> index 1475325..d62d63e 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -350,7 +350,11 @@ static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f)
>          }
>      }
>  
> -    progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
> +    if (block_mig_state.total_sector_sum != 0) {
> +        progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;

This exceeds 80 characters per line.

> +    } else {
> +        progress = 100;
> +    }
>      if (progress != block_mig_state.prev_progress) {
>          block_mig_state.prev_progress = progress;
>          qemu_put_be64(f, (progress << BDRV_SECTOR_BITS)

Kevin
Pierre Riteau - Jan. 12, 2011, 11:01 a.m.
On 11 janv. 2011, at 12:54, Kevin Wolf wrote:

> Am 25.12.2010 21:52, schrieb Pierre Riteau:
>> When block migration is requested and no read-write block device is
>> present, a divide by zero exception is triggered because
>> total_sector_sum equals zero.
>> 
>> Signed-off-by: Pierre Riteau <Pierre.Riteau@irisa.fr>
> 
> Maybe in this case we should generate an error before actually starting
> with block migration. If you bothered to request block migration you
> certainly didn't have in mind to do nothing.

I had in mind a scenario where a management system would use block migration all the time, regardless of what kind of devices were plugged in the VM.
If we throw an error, should it be a fatal error? Or just a migration failed result given to the monitor?

>> ---
>> block-migration.c |    6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>> 
>> diff --git a/block-migration.c b/block-migration.c
>> index 1475325..d62d63e 100644
>> --- a/block-migration.c
>> +++ b/block-migration.c
>> @@ -350,7 +350,11 @@ static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f)
>>         }
>>     }
>> 
>> -    progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
>> +    if (block_mig_state.total_sector_sum != 0) {
>> +        progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
> 
> This exceeds 80 characters per line.
> 
>> +    } else {
>> +        progress = 100;
>> +    }
>>     if (progress != block_mig_state.prev_progress) {
>>         block_mig_state.prev_progress = progress;
>>         qemu_put_be64(f, (progress << BDRV_SECTOR_BITS)
> 
> Kevin
Kevin Wolf - Jan. 12, 2011, 11:40 a.m.
Am 12.01.2011 12:01, schrieb Pierre Riteau:
> On 11 janv. 2011, at 12:54, Kevin Wolf wrote:
> 
>> Am 25.12.2010 21:52, schrieb Pierre Riteau:
>>> When block migration is requested and no read-write block device is
>>> present, a divide by zero exception is triggered because
>>> total_sector_sum equals zero.
>>>
>>> Signed-off-by: Pierre Riteau <Pierre.Riteau@irisa.fr>
>>
>> Maybe in this case we should generate an error before actually starting
>> with block migration. If you bothered to request block migration you
>> certainly didn't have in mind to do nothing.
> 
> I had in mind a scenario where a management system would use block migration all the time, regardless of what kind of devices were plugged in the VM.
> If we throw an error, should it be a fatal error? Or just a migration failed result given to the monitor?

Okay, that makes sense.

We could still check for this condition before really starting block
migration and complete it immediately (successfully, though). I'm also
okay with your patch as it's a very simple solution to the problem. If
you resend it with the > 80 characters line fixed, I'll apply it.

Kevin

Patch

diff --git a/block-migration.c b/block-migration.c
index 1475325..d62d63e 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -350,7 +350,11 @@  static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f)
         }
     }
 
-    progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
+    if (block_mig_state.total_sector_sum != 0) {
+        progress = completed_sector_sum * 100 / block_mig_state.total_sector_sum;
+    } else {
+        progress = 100;
+    }
     if (progress != block_mig_state.prev_progress) {
         block_mig_state.prev_progress = progress;
         qemu_put_be64(f, (progress << BDRV_SECTOR_BITS)