| Message ID | 20211230160525.462185-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Don't return for postcopy_send_discard_bm_ram() | expand |
On Thursday, 2021-12-30 at 17:05:25 +01, Philippe Mathieu-Daudé wrote: > postcopy_send_discard_bm_ram() always return zero. Since it can't > fail, simplify and do not return anything. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: David Edmondson <david.edmondson@oracle.com> > --- > Based-on: <20211224065000.97572-1-peterx@redhat.com> > --- > migration/ram.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 5234d1ece11..e241ce98461 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2433,14 +2433,12 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms) > /** > * postcopy_send_discard_bm_ram: discard a RAMBlock > * > - * Returns zero on success > - * > * Callback from postcopy_each_ram_send_discard for each RAMBlock > * > * @ms: current migration state > * @block: RAMBlock to discard > */ > -static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > +static void postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > { > unsigned long end = block->used_length >> TARGET_PAGE_BITS; > unsigned long current; > @@ -2464,8 +2462,6 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > postcopy_discard_send_range(ms, one, discard_length); > current = one + discard_length; > } > - > - return 0; > } > > static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block); dme.
Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > postcopy_send_discard_bm_ram() always return zero. Since it can't > fail, simplify and do not return anything. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > Based-on: <20211224065000.97572-1-peterx@redhat.com> > --- > migration/ram.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 5234d1ece11..e241ce98461 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2433,14 +2433,12 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms) > /** > * postcopy_send_discard_bm_ram: discard a RAMBlock > * > - * Returns zero on success > - * > * Callback from postcopy_each_ram_send_discard for each RAMBlock > * > * @ms: current migration state > * @block: RAMBlock to discard > */ > -static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > +static void postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > { > unsigned long end = block->used_length >> TARGET_PAGE_BITS; > unsigned long current; > @@ -2464,8 +2462,6 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > postcopy_discard_send_range(ms, one, discard_length); > current = one + discard_length; > } > - > - return 0; > } > > static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block); Nack. You need to change the only caller (postcopy_each_send_discard) also. ret = postcopy_send_discard_bm_ram(ms, block); postcopy_discard_send_finish(ms); if (ret) { return ret; } Not sure if doing the same operation with postcopy_each_send_discard/ram_postcopy_send_discard_bitmap() and postcopy_chunk_hugepages makes sense. Later, Juan.
On Tue, Jan 04, 2022 at 10:15:16AM +0100, Juan Quintela wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > postcopy_send_discard_bm_ram() always return zero. Since it can't > > fail, simplify and do not return anything. > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > --- > > Based-on: <20211224065000.97572-1-peterx@redhat.com> > > --- > > migration/ram.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/migration/ram.c b/migration/ram.c > > index 5234d1ece11..e241ce98461 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -2433,14 +2433,12 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms) > > /** > > * postcopy_send_discard_bm_ram: discard a RAMBlock > > * > > - * Returns zero on success > > - * > > * Callback from postcopy_each_ram_send_discard for each RAMBlock > > * > > * @ms: current migration state > > * @block: RAMBlock to discard > > */ > > -static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > > +static void postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > > { > > unsigned long end = block->used_length >> TARGET_PAGE_BITS; > > unsigned long current; > > @@ -2464,8 +2462,6 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) > > postcopy_discard_send_range(ms, one, discard_length); > > current = one + discard_length; > > } > > - > > - return 0; > > } > > > > static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block); > > Nack. > > You need to change the only caller (postcopy_each_send_discard) also. > > ret = postcopy_send_discard_bm_ram(ms, block); > postcopy_discard_send_finish(ms); > if (ret) { > return ret; > } > > Not sure if doing the same operation with > postcopy_each_send_discard/ram_postcopy_send_discard_bitmap() and > postcopy_chunk_hugepages makes sense. Juan, Phil's patch has a based-on dependency with the other patch: https://lore.kernel.org/qemu-devel/20211224065000.97572-6-peterx@redhat.com/ Thanks,
Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > postcopy_send_discard_bm_ram() always return zero. Since it can't > fail, simplify and do not return anything. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > Based-on: <20211224065000.97572-1-peterx@redhat.com> And here, I learn to read Based-on: Reviewed-by: Juan Quintela <quintela@redhat.com> queued.
diff --git a/migration/ram.c b/migration/ram.c index 5234d1ece11..e241ce98461 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2433,14 +2433,12 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms) /** * postcopy_send_discard_bm_ram: discard a RAMBlock * - * Returns zero on success - * * Callback from postcopy_each_ram_send_discard for each RAMBlock * * @ms: current migration state * @block: RAMBlock to discard */ -static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) +static void postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) { unsigned long end = block->used_length >> TARGET_PAGE_BITS; unsigned long current; @@ -2464,8 +2462,6 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms, RAMBlock *block) postcopy_discard_send_range(ms, one, discard_length); current = one + discard_length; } - - return 0; } static void postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block);
postcopy_send_discard_bm_ram() always return zero. Since it can't fail, simplify and do not return anything. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- Based-on: <20211224065000.97572-1-peterx@redhat.com> --- migration/ram.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)