diff mbox

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

Message ID 1293310330-12067-1-git-send-email-Pierre.Riteau@irisa.fr
State New
Headers show

Commit Message

Pierre Riteau Dec. 25, 2010, 8:52 p.m. UTC
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(-)

Comments

Pierre Riteau Jan. 9, 2011, 7:37 p.m. UTC | #1
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. UTC | #2
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. UTC | #3
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. UTC | #4
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
diff mbox

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)