[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/11] migration: split use of MigrationState.to
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 04/11] migration: split use of MigrationState.total_time |
Date: |
Wed, 3 Jan 2018 17:29:37 +0800 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Jan 03, 2018 at 10:20:39AM +0100, Juan Quintela wrote:
> Peter Xu <address@hidden> wrote:
> > On Wed, Jan 03, 2018 at 09:58:10AM +0100, Juan Quintela wrote:
> >> Peter Xu <address@hidden> wrote:
> >> > It was used either to:
> >> >
> >> > 1. store initial timestamp of migration start, and
> >> > 2. store total time used by last migration
> >> >
> >> > Let's provide two parameters for each of them. Mix use of the two is
> >> > slightly misleading.
> >> >
> >> > Signed-off-by: Peter Xu <address@hidden>
> >>
> >> Reviewed-by: Juan Quintela <address@hidden>
> >
> > Thanks!
> >
> >>
> >> If you have to respin, I would like to use the names:
> >
> > (I think it very possible :-)
> >
> >>
> >> start_time and total_time, i.e. without the mig_ preffix, because they
> >> are in an struct that is clearly named migration O:-)
> >
> > Oh, it's my bad (or good?) habit of keeping some prefix so that cscope
> > won't mix these variables with others. I think the problem is that
> > cscope is always using a global namespace for variables. Considering
> > this do you still like me to change? :) Any suggestions on better
> > usage of cscope would be greatly welcomed too!
>
> I only use cscope very ocassionally, so I can't comment about its usage.
> As said, I put the reviewed-by anyways. But if you dont want to use
> generic names like start_time/total_time, then please use the full name:
>
> - migration_start_time
> - migration_total_time
>
> It is only used a couple of times, and clearer to read. I normally only
> put _prefixes_ if context don't make clear what the variable means. If
> I need *context* I tend to use the full name of things, not
> abbreviations. But yes, not all the code is coherent/consistent.
I'll use the shorter ones. Thanks!
--
Peter Xu
- Re: [Qemu-devel] [PATCH 03/11] migration: remove "enable_colo" var, (continued)
[Qemu-devel] [PATCH 04/11] migration: split use of MigrationState.total_time, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 06/11] migration: introduce vm_down_start_time, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 08/11] migration: use switch at the end of migration, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 07/11] migration: introduce migrate_calculate_complete, Peter Xu, 2018/01/03
[Qemu-devel] [PATCH 09/11] migration: cleanup stats update into function, Peter Xu, 2018/01/03