diff mbox

[1/3] Start moving migration code into a migration directory

Message ID 1413446034-25167-2-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert Oct. 16, 2014, 7:53 a.m. UTC 
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The migration code now occupies a fair chunk of the top level .c
files, it seems time to give it it's own directory.

I've not touched:
   arch_init.c - that's mostly RAM migration but has a few random other
                 bits
   savevm.c    - because it's built target specific
   block-migration.c - should that go in block/ or migration/ ?

This is purely a code move; no code has changed.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 Makefile.objs                                    | 9 ++-------
 migration/Makefile.objs                          | 7 +++++++
 migration-exec.c => migration/migration-exec.c   | 0
 migration-fd.c => migration/migration-fd.c       | 0
 migration-rdma.c => migration/migration-rdma.c   | 0
 migration-tcp.c => migration/migration-tcp.c     | 0
 migration-unix.c => migration/migration-unix.c   | 0
 migration.c => migration/migration.c             | 0
 qemu-file-stdio.c => migration/qemu-file-stdio.c | 0
 qemu-file-unix.c => migration/qemu-file-unix.c   | 0
 qemu-file.c => migration/qemu-file.c             | 0
 vmstate.c => migration/vmstate.c                 | 0
 xbzrle.c => migration/xbzrle.c                   | 0
 tests/Makefile                                   | 6 +++---
 14 files changed, 12 insertions(+), 10 deletions(-)
 create mode 100644 migration/Makefile.objs
 rename migration-exec.c => migration/migration-exec.c (100%)
 rename migration-fd.c => migration/migration-fd.c (100%)
 rename migration-rdma.c => migration/migration-rdma.c (100%)
 rename migration-tcp.c => migration/migration-tcp.c (100%)
 rename migration-unix.c => migration/migration-unix.c (100%)
 rename migration.c => migration/migration.c (100%)
 rename qemu-file-stdio.c => migration/qemu-file-stdio.c (100%)
 rename qemu-file-unix.c => migration/qemu-file-unix.c (100%)
 rename qemu-file.c => migration/qemu-file.c (100%)
 rename vmstate.c => migration/vmstate.c (100%)
 rename xbzrle.c => migration/xbzrle.c (100%)

Comments

Juan Quintela Oct. 16, 2014, 8:08 a.m. UTC | #1 
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
G> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The migration code now occupies a fair chunk of the top level .c
> files, it seems time to give it it's own directory.
>
> I've not touched:
>    arch_init.c - that's mostly RAM migration but has a few random other
>                  bits

Will split the memory bits, and can go to migration.

>    savevm.c    - because it's built target specific

Damn, would have to look at it.

>    block-migration.c - should that go in block/ or migration/ ?

It is really on migration.  we have basically:

- ram-migration
- block-migration

We can call other names if people preffer.

>
> This is purely a code move; no code has changed.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks, Juan.
Juan Quintela Oct. 16, 2014, 8:08 a.m. UTC | #2 
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The migration code now occupies a fair chunk of the top level .c
> files, it seems time to give it it's own directory.
>
> I've not touched:
>    arch_init.c - that's mostly RAM migration but has a few random other
>                  bits
>    savevm.c    - because it's built target specific
>    block-migration.c - should that go in block/ or migration/ ?
>
> This is purely a code move; no code has changed.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
Dr. David Alan Gilbert Oct. 16, 2014, 8:12 a.m. UTC | #3 
* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> G> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The migration code now occupies a fair chunk of the top level .c
> > files, it seems time to give it it's own directory.
> >
> > I've not touched:
> >    arch_init.c - that's mostly RAM migration but has a few random other
> >                  bits
> 
> Will split the memory bits, and can go to migration.
> 
> >    savevm.c    - because it's built target specific
> 
> Damn, would have to look at it.

Yes; I suspect it's the vmstate_register_ram that uses TARGET_PAGE_MASK,
one of the patches in my postcopy world adds a function in exec.c that
returns TARGET_PAGE_BITS so that stuff that needs to know target page size
can make a call to find it out; that might solve that.

Dave

> 
> >    block-migration.c - should that go in block/ or migration/ ?
> 
> It is really on migration.  we have basically:
> 
> - ram-migration
> - block-migration
> 
> We can call other names if people preffer.
> 
> >
> > This is purely a code move; no code has changed.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Thanks, Juan.
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Paolo Bonzini Oct. 20, 2014, 6:34 p.m. UTC | #4 
On 10/16/2014 10:12 AM, Dr. David Alan Gilbert wrote:
> Yes; I suspect it's the vmstate_register_ram that uses TARGET_PAGE_MASK,
> one of the patches in my postcopy world adds a function in exec.c that
> returns TARGET_PAGE_BITS so that stuff that needs to know target page size
> can make a call to find it out; that might solve that.

Yeah, or you can just move vmstate_register_ram to migration/ram.c 
together with the arch_init.c bits.

Paolo
Amit Shah Oct. 30, 2014, 12:26 p.m. UTC | #5 
On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The migration code now occupies a fair chunk of the top level .c
> files, it seems time to give it it's own directory.

s/it's/its

> I've not touched:
>    arch_init.c - that's mostly RAM migration but has a few random other
>                  bits
>    savevm.c    - because it's built target specific
>    block-migration.c - should that go in block/ or migration/ ?
> 
> This is purely a code move; no code has changed.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>


		Amit
Amit Shah Oct. 30, 2014, 12:28 p.m. UTC | #6 
On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:

>  rename migration-exec.c => migration/migration-exec.c (100%)
>  rename migration-fd.c => migration/migration-fd.c (100%)
>  rename migration-rdma.c => migration/migration-rdma.c (100%)
>  rename migration-tcp.c => migration/migration-tcp.c (100%)
>  rename migration-unix.c => migration/migration-unix.c (100%)
>  rename migration.c => migration/migration.c (100%)

I'm wondering if we should also use the opportunity to cleanup the
file names:

migration.c => main.c
migration-X.c => X.c

?

		Amit
Dr. David Alan Gilbert Oct. 30, 2014, 12:37 p.m. UTC | #7 
* Amit Shah (amit.shah@redhat.com) wrote:
> On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
> 
> >  rename migration-exec.c => migration/migration-exec.c (100%)
> >  rename migration-fd.c => migration/migration-fd.c (100%)
> >  rename migration-rdma.c => migration/migration-rdma.c (100%)
> >  rename migration-tcp.c => migration/migration-tcp.c (100%)
> >  rename migration-unix.c => migration/migration-unix.c (100%)
> >  rename migration.c => migration/migration.c (100%)
> 
> I'm wondering if we should also use the opportunity to cleanup the
> file names:
> 
> migration.c => main.c
> migration-X.c => X.c
> 
> ?

I'd be OK with changing filenames, but they would have to
be fairly clear; I don't like having a 'main.c' because that's
where I'd expect to find a main() - but we do have a few core.c's
as the core of each of a few hw subdirs.

Dave

> 
> 		Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Amit Shah Oct. 30, 2014, 12:46 p.m. UTC | #8 
On (Thu) 30 Oct 2014 [12:37:27], Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.shah@redhat.com) wrote:
> > On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
> > 
> > >  rename migration-exec.c => migration/migration-exec.c (100%)
> > >  rename migration-fd.c => migration/migration-fd.c (100%)
> > >  rename migration-rdma.c => migration/migration-rdma.c (100%)
> > >  rename migration-tcp.c => migration/migration-tcp.c (100%)
> > >  rename migration-unix.c => migration/migration-unix.c (100%)
> > >  rename migration.c => migration/migration.c (100%)
> > 
> > I'm wondering if we should also use the opportunity to cleanup the
> > file names:
> > 
> > migration.c => main.c
> > migration-X.c => X.c
> > 
> > ?
> 
> I'd be OK with changing filenames, but they would have to
> be fairly clear; I don't like having a 'main.c' because that's
> where I'd expect to find a main() - but we do have a few core.c's
> as the core of each of a few hw subdirs.

I think main should be OK in a subdir, because it's the main file for
that subsystem (not necessarily the whole program).  Checking the src
for main.c, though, it looks like no one uses it in such a way (Linux
has lots of these, btw, hence my initial suggestion).

core.c is fine, too.

		Amit
Peter Maydell Oct. 31, 2014, 6:32 p.m. UTC | #9 
On 16 October 2014 08:53, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The migration code now occupies a fair chunk of the top level .c
> files, it seems time to give it it's own directory.

Missed opportunity to make the patch summary line
"Start migrating migration code into a migration directory" :-)

-- PMM
Dr. David Alan Gilbert Oct. 31, 2014, 6:58 p.m. UTC | #10 
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 16 October 2014 08:53, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The migration code now occupies a fair chunk of the top level .c
> > files, it seems time to give it it's own directory.
> 
> Missed opportunity to make the patch summary line
> "Start migrating migration code into a migration directory" :-)

I would certainly allow a committer to fix such a missed opportunity.

Dave

> 
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Gary Hook Oct. 31, 2014, 9:08 p.m. UTC | #11 
On 10/30/14, 7:26 AM, "Amit Shah" <amit.shah@redhat.com> wrote:

>On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> The migration code now occupies a fair chunk of the top level .c
>> files, it seems time to give it it's own directory.
>
>s/it's/its

6 out of 87 .c files, and approximately 370 blocks out of 2840 (based on
du output). 13% is a "fair chunk"?

But tidy organization is a good thing while needless renaming is not. The
only goal that the suggested renames would appear to accomplish is
additional obfuscation. How about just moving them into a subdirectory and
leave their names alone?
Peter Maydell Oct. 31, 2014, 11:31 p.m. UTC | #12 
On 30 October 2014 12:37, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Amit Shah (amit.shah@redhat.com) wrote:
>> On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
>>
>> >  rename migration-exec.c => migration/migration-exec.c (100%)
>> >  rename migration-fd.c => migration/migration-fd.c (100%)
>> >  rename migration-rdma.c => migration/migration-rdma.c (100%)
>> >  rename migration-tcp.c => migration/migration-tcp.c (100%)
>> >  rename migration-unix.c => migration/migration-unix.c (100%)
>> >  rename migration.c => migration/migration.c (100%)
>>
>> I'm wondering if we should also use the opportunity to cleanup the
>> file names:
>>
>> migration.c => main.c
>> migration-X.c => X.c
>>
>> ?
>
> I'd be OK with changing filenames, but they would have to
> be fairly clear; I don't like having a 'main.c' because that's
> where I'd expect to find a main() - but we do have a few core.c's
> as the core of each of a few hw subdirs.

Yeah, I think I agree that 'main.c' is not a great choice
(but core.c or migration.c would be fine); for the others I
agree with Amit that we should drop the migration- prefix
as we move them. (It's generally how we've handled other
moves-into-subdirectories in the past; various arm_* and
alpha_* files lost their prefixes when they moved into hw/arm
and hw/alpha, for instance.)

thanks
-- PMM
Dr. David Alan Gilbert Nov. 3, 2014, 8:53 a.m. UTC | #13 
* Gary Hook (gary.hook@nimboxx.com) wrote:
> 
> 
> On 10/30/14, 7:26 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
> 
> >On (Thu) 16 Oct 2014 [08:53:52], Dr. David Alan Gilbert (git) wrote:
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> 
> >> The migration code now occupies a fair chunk of the top level .c
> >> files, it seems time to give it it's own directory.
> >
> >s/it's/its
> 
> 6 out of 87 .c files, and approximately 370 blocks out of 2840 (based on
> du output). 13% is a "fair chunk"?

I'm not sure how you got 6;
  migration.c
  migration-exec.c
  migration-fd.c
  migration-rdma.c
  migration-tcp.c
  migration-unix.c
  qemu-file-buf.c
  qemu-file.c
  qemu-file-stdio.c
  qemu-file-unix.c
  vmstate.c
  xbzrle.c

so that's 12, and there are another 3 in the commit message saying they could
do with being moved.  That would be 15 files, or 17% - and so yes, I do call that
a fair chunk.

> But tidy organization is a good thing while needless renaming is not. The
> only goal that the suggested renames would appear to accomplish is
> additional obfuscation. How about just moving them into a subdirectory and
> leave their names alone?

Which is what I did; however I have sympathy with those that think that in a
directory called 'migration' starting a bunch of the files with 'migration-' is
excessive.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Eduardo Habkost Nov. 3, 2014, 12:32 p.m. UTC | #14 
On Mon, Nov 03, 2014 at 08:53:54AM +0000, Dr. David Alan Gilbert wrote:
[...]
> > But tidy organization is a good thing while needless renaming is not. The
> > only goal that the suggested renames would appear to accomplish is
> > additional obfuscation. How about just moving them into a subdirectory and
> > leave their names alone?
> 
> Which is what I did; however I have sympathy with those that think that in a
> directory called 'migration' starting a bunch of the files with 'migration-' is
> excessive.

I, for one, don't even see the difference between moving and renaming.  :)
diff mbox

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 18fd35c..71b4b79 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -48,15 +48,10 @@  common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
 
-common-obj-y += migration.o migration-tcp.o
-common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
-common-obj-$(CONFIG_RDMA) += migration-rdma.o
+common-obj-y += migration/
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
-common-obj-y += page_cache.o xbzrle.o
-
-common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
+common-obj-y += page_cache.o
 
 common-obj-$(CONFIG_SPICE) += spice-qemu-char.o
 
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
new file mode 100644
index 0000000..681bae9
--- /dev/null
+++ b/migration/Makefile.objs
@@ -0,0 +1,7 @@ 
+common-obj-y += migration.o migration-tcp.o
+common-obj-y += vmstate.o
+common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-$(CONFIG_RDMA) += migration-rdma.o
+common-obj-y += xbzrle.o
+
+common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/migration-exec.c b/migration/migration-exec.c
similarity index 100%
rename from migration-exec.c
rename to migration/migration-exec.c
diff --git a/migration-fd.c b/migration/migration-fd.c
similarity index 100%
rename from migration-fd.c
rename to migration/migration-fd.c
diff --git a/migration-rdma.c b/migration/migration-rdma.c
similarity index 100%
rename from migration-rdma.c
rename to migration/migration-rdma.c
diff --git a/migration-tcp.c b/migration/migration-tcp.c
similarity index 100%
rename from migration-tcp.c
rename to migration/migration-tcp.c
diff --git a/migration-unix.c b/migration/migration-unix.c
similarity index 100%
rename from migration-unix.c
rename to migration/migration-unix.c
diff --git a/migration.c b/migration/migration.c
similarity index 100%
rename from migration.c
rename to migration/migration.c
diff --git a/qemu-file-stdio.c b/migration/qemu-file-stdio.c
similarity index 100%
rename from qemu-file-stdio.c
rename to migration/qemu-file-stdio.c
diff --git a/qemu-file-unix.c b/migration/qemu-file-unix.c
similarity index 100%
rename from qemu-file-unix.c
rename to migration/qemu-file-unix.c
diff --git a/qemu-file.c b/migration/qemu-file.c
similarity index 100%
rename from qemu-file.c
rename to migration/qemu-file.c
diff --git a/vmstate.c b/migration/vmstate.c
similarity index 100%
rename from vmstate.c
rename to migration/vmstate.c
diff --git a/xbzrle.c b/migration/xbzrle.c
similarity index 100%
rename from xbzrle.c
rename to migration/xbzrle.c
diff --git a/tests/Makefile b/tests/Makefile
index 16f0e4c..3a03979 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -49,7 +49,7 @@  check-unit-y += tests/test-x86-cpuid$(EXESUF)
 # all code tested by test-x86-cpuid is inside topology.h
 gcov-files-test-x86-cpuid-y =
 check-unit-y += tests/test-xbzrle$(EXESUF)
-gcov-files-test-xbzrle-y = xbzrle.c
+gcov-files-test-xbzrle-y = migration/xbzrle.c
 check-unit-y += tests/test-cutils$(EXESUF)
 gcov-files-test-cutils-y += util/cutils.c
 check-unit-y += tests/test-mul64$(EXESUF)
@@ -247,7 +247,7 @@  tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(block-obj-y) libqemu
 tests/test-iov$(EXESUF): tests/test-iov.o libqemuutil.a
 tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o libqemuutil.a libqemustub.a
 tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
-tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuutil.a
+tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o page_cache.o libqemuutil.a
 tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
 tests/test-int128$(EXESUF): tests/test-int128.o
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
@@ -258,7 +258,7 @@  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	$(test-qapi-obj-y) \
 	libqemuutil.a libqemustub.a
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
-	vmstate.o qemu-file.o qemu-file-unix.o \
+	migration/vmstate.o migration/qemu-file.o migration/qemu-file-unix.o \
 	libqemuutil.a libqemustub.a
 
 tests/test-qapi-types.c tests/test-qapi-types.h :\