Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-23 Thread Juan Quintela
Peter Xu  wrote:
> On Thu, May 18, 2017 at 03:26:23PM +0200, Juan Quintela wrote:
>> Peter Xu  wrote:
>> > On Wed, May 17, 2017 at 05:47:50PM +0200, Juan Quintela wrote:
>> >> Signed-off-by: Juan Quintela 
>> >> ---
>> >>  include/migration/migration.h |  1 +
>> >>  include/migration/qemu-file.h |  4 
>> >>  migration/channel.c   |  1 +
>> >>  migration/colo.c  |  1 +
>> >>  migration/migration.c |  1 +
>> >>  migration/qemu-file-channel.c |  1 +
>> >>  migration/qemu-file-channel.h | 21 +
>> >>  migration/rdma.c  |  1 +
>> >>  migration/savevm.c|  1 +
>> >>  tests/test-vmstate.c  |  1 +
>> >>  10 files changed, 29 insertions(+), 4 deletions(-)
>> >>  create mode 100644 migration/qemu-file-channel.h
>> >> 
>> >> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> >> index e831259..8280df1 100644
>> >> --- a/include/migration/migration.h
>> >> +++ b/include/migration/migration.h
>> >> @@ -19,6 +19,7 @@
>> >>  #include "qemu/thread.h"
>> >>  #include "qemu/notify.h"
>> >>  #include "migration/vmstate.h"
>> >> +#include "io/channel.h"
>> >
>> > Could I ask why we add this line here? I thought one of the main goals
>> > of this series is removing things from migration.h...
>> 
>> 
>> 
>> I remove from include/migration/qemu-file.h
>> 
>> -#include "io/channel.h"
>> 
>> 
>> Because all the QIOChannel functions in qemu-file.h are moved to
>> qemu-file-channel.h.
>> 
>> Great!
>> 
>> But migration/vmstate.h includes qemu-file.h
>> 
>> And migration.h includes vmstate.h
>> 
>> And migration.h has this functions:
>> 
>> void qemu_start_incoming_migration(const char *uri, Error **errp);
>> QIOChannel *ioc,
>> Error **errp);
>
> (In my repo, it does not need QIOChannel, which looks like:
>  void qemu_start_incoming_migration(const char *uri, Error **errp)
>  but it does not really matter much...)
>
>> 
>> void migration_tls_channel_connect(MigrationState *s,
>>QIOChannel *ioc,
>>const char *hostname,
>>Error **errp);
>> 
>> And nothing else declares the QIOChannel.
>> 
>> So, the easy solution so far is to include this by now to maintain
>> compilation.
>
> I see. It's okay to me.
>
> (Another solution would be moving these functions outside of
>  migration/migration.h as well since they are used by migration
>  internally as well? Anyway we already have migration/tls.c to keep
>  migration_tls_* functions)
>
> I'll reply to the latest version of this patch for the r-b. Thanks.

They are moved there on the new cleanup series that I sent on Thrusday.
Internally the series were about 70 patches, so I tried to group the
patches in series by the way that they do similar things, so some things
like this one have not been handled "perfectly".  Grouping by this kind
of thing made other things less clean.  Compromise, as usual.

Thanks, Juan.




Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Peter Xu
On Thu, May 18, 2017 at 06:16:49PM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 

Reviewed-by: Peter Xu 

-- 
Peter Xu



Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Peter Xu
On Thu, May 18, 2017 at 03:26:23PM +0200, Juan Quintela wrote:
> Peter Xu  wrote:
> > On Wed, May 17, 2017 at 05:47:50PM +0200, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  include/migration/migration.h |  1 +
> >>  include/migration/qemu-file.h |  4 
> >>  migration/channel.c   |  1 +
> >>  migration/colo.c  |  1 +
> >>  migration/migration.c |  1 +
> >>  migration/qemu-file-channel.c |  1 +
> >>  migration/qemu-file-channel.h | 21 +
> >>  migration/rdma.c  |  1 +
> >>  migration/savevm.c|  1 +
> >>  tests/test-vmstate.c  |  1 +
> >>  10 files changed, 29 insertions(+), 4 deletions(-)
> >>  create mode 100644 migration/qemu-file-channel.h
> >> 
> >> diff --git a/include/migration/migration.h b/include/migration/migration.h
> >> index e831259..8280df1 100644
> >> --- a/include/migration/migration.h
> >> +++ b/include/migration/migration.h
> >> @@ -19,6 +19,7 @@
> >>  #include "qemu/thread.h"
> >>  #include "qemu/notify.h"
> >>  #include "migration/vmstate.h"
> >> +#include "io/channel.h"
> >
> > Could I ask why we add this line here? I thought one of the main goals
> > of this series is removing things from migration.h...
> 
> 
> 
> I remove from include/migration/qemu-file.h
> 
> -#include "io/channel.h"
> 
> 
> Because all the QIOChannel functions in qemu-file.h are moved to
> qemu-file-channel.h.
> 
> Great!
> 
> But migration/vmstate.h includes qemu-file.h
> 
> And migration.h includes vmstate.h
> 
> And migration.h has this functions:
> 
> void qemu_start_incoming_migration(const char *uri, Error **errp);
> QIOChannel *ioc,
> Error **errp);

(In my repo, it does not need QIOChannel, which looks like:
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 but it does not really matter much...)

> 
> void migration_tls_channel_connect(MigrationState *s,
>QIOChannel *ioc,
>const char *hostname,
>Error **errp);
> 
> And nothing else declares the QIOChannel.
> 
> So, the easy solution so far is to include this by now to maintain
> compilation.

I see. It's okay to me.

(Another solution would be moving these functions outside of
 migration/migration.h as well since they are used by migration
 internally as well? Anyway we already have migration/tls.c to keep
 migration_tls_* functions)

I'll reply to the latest version of this patch for the r-b. Thanks.

-- 
Peter Xu



Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h |  1 +
>  include/migration/qemu-file.h |  4 
>  migration/channel.c   |  1 +
>  migration/colo.c  |  1 +
>  migration/migration.c |  1 +
>  migration/qemu-file-channel.c |  1 +
>  migration/qemu-file-channel.h | 32 
>  migration/rdma.c  |  1 +
>  migration/savevm.c|  1 +
>  tests/test-vmstate.c  |  1 +
>  10 files changed, 40 insertions(+), 4 deletions(-)
>  create mode 100644 migration/qemu-file-channel.h
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e831259..8280df1 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -19,6 +19,7 @@
>  #include "qemu/thread.h"
>  #include "qemu/notify.h"
>  #include "migration/vmstate.h"
> +#include "io/channel.h"
>  #include "qapi-types.h"
>  #include "exec/cpu-common.h"
>  #include "qemu/coroutine_int.h"
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 0cd648a..b5ac800 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -27,8 +27,6 @@
>  
>  #include "qemu-common.h"
>  #include "exec/cpu-common.h"
> -#include "io/channel.h"
> -
>  
>  /* Read a chunk of data from a file at the given position.  The pos argument
>   * can be ignored if the file is only be used for streaming.  The number of
> @@ -119,8 +117,6 @@ typedef struct QEMUFileHooks {
>  } QEMUFileHooks;
>  
>  QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
> -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
>  void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
>  int qemu_get_fd(QEMUFile *f);
>  int qemu_fclose(QEMUFile *f);
> diff --git a/migration/channel.c b/migration/channel.c
> index f50267a..124857d 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -16,6 +16,7 @@
>  #include "qemu/osdep.h"
>  #include "channel.h"
>  #include "migration/migration.h"
> +#include "qemu-file-channel.h"
>  #include "trace.h"
>  #include "qapi/error.h"
>  #include "io/channel-tls.h"
> diff --git a/migration/colo.c b/migration/colo.c
> index dd38fed..9cd0250 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu-file-channel.h"
>  #include "migration/colo.h"
>  #include "migration/block.h"
>  #include "io/channel-buffer.h"
> diff --git a/migration/migration.c b/migration/migration.c
> index 217616d..2d9379c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -19,6 +19,7 @@
>  #include "qemu/main-loop.h"
>  #include "migration/blocker.h"
>  #include "migration/migration.h"
> +#include "qemu-file-channel.h"
>  #include "migration/qemu-file.h"
>  #include "sysemu/sysemu.h"
>  #include "block/block.h"
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 45c13f1..dc991c9 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu-file-channel.h"
>  #include "migration/qemu-file.h"
>  #include "io/channel-socket.h"
>  #include "qemu/iov.h"
> diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> new file mode 100644
> index 000..0028a09
> --- /dev/null
> +++ b/migration/qemu-file-channel.h
> @@ -0,0 +1,32 @@
> +/*
> + * QEMUFile backend for QIOChannel objects
> + *
> + * Copyright (c) 2015-2016 Red Hat, Inc
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

OK, I think that is correct, and we can change it to something else
later if we want, so:


Reviewed-by: Dr. David Alan Gilbert 

> +#ifndef QEMU

[Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 include/migration/migration.h |  1 +
 include/migration/qemu-file.h |  4 
 migration/channel.c   |  1 +
 migration/colo.c  |  1 +
 migration/migration.c |  1 +
 migration/qemu-file-channel.c |  1 +
 migration/qemu-file-channel.h | 32 
 migration/rdma.c  |  1 +
 migration/savevm.c|  1 +
 tests/test-vmstate.c  |  1 +
 10 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 migration/qemu-file-channel.h

diff --git a/include/migration/migration.h b/include/migration/migration.h
index e831259..8280df1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -19,6 +19,7 @@
 #include "qemu/thread.h"
 #include "qemu/notify.h"
 #include "migration/vmstate.h"
+#include "io/channel.h"
 #include "qapi-types.h"
 #include "exec/cpu-common.h"
 #include "qemu/coroutine_int.h"
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 0cd648a..b5ac800 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -27,8 +27,6 @@
 
 #include "qemu-common.h"
 #include "exec/cpu-common.h"
-#include "io/channel.h"
-
 
 /* Read a chunk of data from a file at the given position.  The pos argument
  * can be ignored if the file is only be used for streaming.  The number of
@@ -119,8 +117,6 @@ typedef struct QEMUFileHooks {
 } QEMUFileHooks;
 
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
-QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
-QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
diff --git a/migration/channel.c b/migration/channel.c
index f50267a..124857d 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "channel.h"
 #include "migration/migration.h"
+#include "qemu-file-channel.h"
 #include "trace.h"
 #include "qapi/error.h"
 #include "io/channel-tls.h"
diff --git a/migration/colo.c b/migration/colo.c
index dd38fed..9cd0250 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
+#include "qemu-file-channel.h"
 #include "migration/colo.h"
 #include "migration/block.h"
 #include "io/channel-buffer.h"
diff --git a/migration/migration.c b/migration/migration.c
index 217616d..2d9379c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -19,6 +19,7 @@
 #include "qemu/main-loop.h"
 #include "migration/blocker.h"
 #include "migration/migration.h"
+#include "qemu-file-channel.h"
 #include "migration/qemu-file.h"
 #include "sysemu/sysemu.h"
 #include "block/block.h"
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 45c13f1..dc991c9 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu-file-channel.h"
 #include "migration/qemu-file.h"
 #include "io/channel-socket.h"
 #include "qemu/iov.h"
diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
new file mode 100644
index 000..0028a09
--- /dev/null
+++ b/migration/qemu-file-channel.h
@@ -0,0 +1,32 @@
+/*
+ * QEMUFile backend for QIOChannel objects
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_FILE_CHANNEL_H
+#define QEMU_FILE_CHANNEL_H
+
+#include "io/channel.h"
+
+QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
+QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
+#endif
diff --git a/migration/rdma.c b/migration/rdma.c
index 7eaaf96..166cd60 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -20,6 +20,7 @@
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
 #include "exec/cpu-common.h"
+#incl

Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Juan Quintela
"Daniel P. Berrange"  wrote:
> On Thu, May 18, 2017 at 04:44:08PM +0100, Dr. David Alan Gilbert wrote:

>> OK, similar copyright - but slightly different problem.
>> These two lines come from include/migration/qemu-file.h;
>> they were both added by Daniel in a9cfeb33bb23a81.
>> The qemu-file.h header however has the old Fabrice copyright
>> with a 3 paragraph copyright rather than the GPLv2.
>> 
>> So, Authors, and copyright need changing - not sure what the
>> right way to do the copyright is.  Personally I'd ask Dan really
>> nicely to let it be GPL v2 or newer, but the quicker answer I think
>> is to take the header from qemu-file.h
>
> That is of course fine. Anything I contribute is under Red Hat copyright,
> and so as another Red Hat employee, you're free to change the license on
> any code I wrote [1]
>
> Regards,
> Daniel
>
> [1] unless I put a comment about having copied the code from another source

/*
 * QEMU live migration channel operations
 *
 * Copyright Red Hat, Inc. 2016
 *
 * Authors:
 *  Daniel P. Berrange 
 *
 * This work is licensed under the terms of the GNU GPL, version 2.  See
 * the COPYING file in the top-level directory.
 *
 * Contributions after 2012-01-13 are licensed under the terms of the
 * GNU GPL, version 2 or (at your option) any later version.
 */

Putting that one for channel operations and qemu-file-chanel operations
(changing the 1st line).

Later, Juan.



Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Daniel P. Berrange
On Thu, May 18, 2017 at 04:44:08PM +0100, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
> > Signed-off-by: Juan Quintela 
> > ---
> >  include/migration/migration.h |  1 +
> >  include/migration/qemu-file.h |  4 
> >  migration/channel.c   |  1 +
> >  migration/colo.c  |  1 +
> >  migration/migration.c |  1 +
> >  migration/qemu-file-channel.c |  1 +
> >  migration/qemu-file-channel.h | 21 +
> >  migration/rdma.c  |  1 +
> >  migration/savevm.c|  1 +
> >  tests/test-vmstate.c  |  1 +
> >  10 files changed, 29 insertions(+), 4 deletions(-)
> >  create mode 100644 migration/qemu-file-channel.h
> > 
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index e831259..8280df1 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -19,6 +19,7 @@
> >  #include "qemu/thread.h"
> >  #include "qemu/notify.h"
> >  #include "migration/vmstate.h"
> > +#include "io/channel.h"
> >  #include "qapi-types.h"
> >  #include "exec/cpu-common.h"
> >  #include "qemu/coroutine_int.h"
> > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> > index 0cd648a..b5ac800 100644
> > --- a/include/migration/qemu-file.h
> > +++ b/include/migration/qemu-file.h
> > @@ -27,8 +27,6 @@
> >  
> >  #include "qemu-common.h"
> >  #include "exec/cpu-common.h"
> > -#include "io/channel.h"
> > -
> >  
> >  /* Read a chunk of data from a file at the given position.  The pos 
> > argument
> >   * can be ignored if the file is only be used for streaming.  The number of
> > @@ -119,8 +117,6 @@ typedef struct QEMUFileHooks {
> >  } QEMUFileHooks;
> >  
> >  QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
> > -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> > -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> >  void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> >  int qemu_get_fd(QEMUFile *f);
> >  int qemu_fclose(QEMUFile *f);
> > diff --git a/migration/channel.c b/migration/channel.c
> > index 6104de5..fed8563 100644
> > --- a/migration/channel.c
> > +++ b/migration/channel.c
> > @@ -16,6 +16,7 @@
> >  #include "qemu/osdep.h"
> >  #include "channel.h"
> >  #include "migration/migration.h"
> > +#include "qemu-file-channel.h"
> >  #include "trace.h"
> >  #include "qapi/error.h"
> >  #include "io/channel-tls.h"
> > diff --git a/migration/colo.c b/migration/colo.c
> > index dd38fed..9cd0250 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -13,6 +13,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/timer.h"
> >  #include "sysemu/sysemu.h"
> > +#include "qemu-file-channel.h"
> >  #include "migration/colo.h"
> >  #include "migration/block.h"
> >  #include "io/channel-buffer.h"
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 3ba32eb..ff3f7aa 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -19,6 +19,7 @@
> >  #include "qemu/main-loop.h"
> >  #include "migration/blocker.h"
> >  #include "migration/migration.h"
> > +#include "qemu-file-channel.h"
> >  #include "migration/qemu-file.h"
> >  #include "sysemu/sysemu.h"
> >  #include "block/block.h"
> > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> > index 45c13f1..dc991c9 100644
> > --- a/migration/qemu-file-channel.c
> > +++ b/migration/qemu-file-channel.c
> > @@ -23,6 +23,7 @@
> >   */
> >  
> >  #include "qemu/osdep.h"
> > +#include "qemu-file-channel.h"
> >  #include "migration/qemu-file.h"
> >  #include "io/channel-socket.h"
> >  #include "qemu/iov.h"
> > diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> > new file mode 100644
> > index 000..d1bd5ff
> > --- /dev/null
> > +++ b/migration/qemu-file-channel.h
> > @@ -0,0 +1,21 @@
> > +/*
> > + * QEMU migration file channel operations
> > + *
> > + * Copyright IBM, Corp. 2008
> > + *
> > + * Authors:
> > + *  Anthony Liguori   
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef QEMU_FILE_CHANNEL_H
> > +#define QEMU_FILE_CHANNEL_H
> > +
> > +#include "io/channel.h"
> > +
> > +QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> > +QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> > +#endif
> 
> OK, similar copyright - but slightly different problem.
> These two lines come from include/migration/qemu-file.h;
> they were both added by Daniel in a9cfeb33bb23a81.
> The qemu-file.h header however has the old Fabrice copyright
> with a 3 paragraph copyright rather than the GPLv2.
> 
> So, Authors, and copyright need changing - not sure what the
> right way to do the copyright is.  Personally I'd ask Dan really
> nicely to let it be GPL v2 or newer, but the quicker answer I think
> is to take the header from qemu-file.h

That is of course fine. Anything I con

Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h |  1 +
>  include/migration/qemu-file.h |  4 
>  migration/channel.c   |  1 +
>  migration/colo.c  |  1 +
>  migration/migration.c |  1 +
>  migration/qemu-file-channel.c |  1 +
>  migration/qemu-file-channel.h | 21 +
>  migration/rdma.c  |  1 +
>  migration/savevm.c|  1 +
>  tests/test-vmstate.c  |  1 +
>  10 files changed, 29 insertions(+), 4 deletions(-)
>  create mode 100644 migration/qemu-file-channel.h
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e831259..8280df1 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -19,6 +19,7 @@
>  #include "qemu/thread.h"
>  #include "qemu/notify.h"
>  #include "migration/vmstate.h"
> +#include "io/channel.h"
>  #include "qapi-types.h"
>  #include "exec/cpu-common.h"
>  #include "qemu/coroutine_int.h"
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 0cd648a..b5ac800 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -27,8 +27,6 @@
>  
>  #include "qemu-common.h"
>  #include "exec/cpu-common.h"
> -#include "io/channel.h"
> -
>  
>  /* Read a chunk of data from a file at the given position.  The pos argument
>   * can be ignored if the file is only be used for streaming.  The number of
> @@ -119,8 +117,6 @@ typedef struct QEMUFileHooks {
>  } QEMUFileHooks;
>  
>  QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
> -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
>  void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
>  int qemu_get_fd(QEMUFile *f);
>  int qemu_fclose(QEMUFile *f);
> diff --git a/migration/channel.c b/migration/channel.c
> index 6104de5..fed8563 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -16,6 +16,7 @@
>  #include "qemu/osdep.h"
>  #include "channel.h"
>  #include "migration/migration.h"
> +#include "qemu-file-channel.h"
>  #include "trace.h"
>  #include "qapi/error.h"
>  #include "io/channel-tls.h"
> diff --git a/migration/colo.c b/migration/colo.c
> index dd38fed..9cd0250 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu-file-channel.h"
>  #include "migration/colo.h"
>  #include "migration/block.h"
>  #include "io/channel-buffer.h"
> diff --git a/migration/migration.c b/migration/migration.c
> index 3ba32eb..ff3f7aa 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -19,6 +19,7 @@
>  #include "qemu/main-loop.h"
>  #include "migration/blocker.h"
>  #include "migration/migration.h"
> +#include "qemu-file-channel.h"
>  #include "migration/qemu-file.h"
>  #include "sysemu/sysemu.h"
>  #include "block/block.h"
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 45c13f1..dc991c9 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu-file-channel.h"
>  #include "migration/qemu-file.h"
>  #include "io/channel-socket.h"
>  #include "qemu/iov.h"
> diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> new file mode 100644
> index 000..d1bd5ff
> --- /dev/null
> +++ b/migration/qemu-file-channel.h
> @@ -0,0 +1,21 @@
> +/*
> + * QEMU migration file channel operations
> + *
> + * Copyright IBM, Corp. 2008
> + *
> + * Authors:
> + *  Anthony Liguori   
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_FILE_CHANNEL_H
> +#define QEMU_FILE_CHANNEL_H
> +
> +#include "io/channel.h"
> +
> +QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> +QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> +#endif

OK, similar copyright - but slightly different problem.
These two lines come from include/migration/qemu-file.h;
they were both added by Daniel in a9cfeb33bb23a81.
The qemu-file.h header however has the old Fabrice copyright
with a 3 paragraph copyright rather than the GPLv2.

So, Authors, and copyright need changing - not sure what the
right way to do the copyright is.  Personally I'd ask Dan really
nicely to let it be GPL v2 or newer, but the quicker answer I think
is to take the header from qemu-file.h



> diff --git a/migration/rdma.c b/migration/rdma.c
> index 7eaaf96..166cd60 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -20,6 +20,7 @@
>  #include "migration/migration.h"
>  #include "migration/qemu-file.h"
>  #include "exec/cpu-common.h"
> +#include "qemu-file-channel.h"
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/sockets.h"

Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Juan Quintela
Peter Xu  wrote:
> On Wed, May 17, 2017 at 05:47:50PM +0200, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela 
>> ---
>>  include/migration/migration.h |  1 +
>>  include/migration/qemu-file.h |  4 
>>  migration/channel.c   |  1 +
>>  migration/colo.c  |  1 +
>>  migration/migration.c |  1 +
>>  migration/qemu-file-channel.c |  1 +
>>  migration/qemu-file-channel.h | 21 +
>>  migration/rdma.c  |  1 +
>>  migration/savevm.c|  1 +
>>  tests/test-vmstate.c  |  1 +
>>  10 files changed, 29 insertions(+), 4 deletions(-)
>>  create mode 100644 migration/qemu-file-channel.h
>> 
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index e831259..8280df1 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -19,6 +19,7 @@
>>  #include "qemu/thread.h"
>>  #include "qemu/notify.h"
>>  #include "migration/vmstate.h"
>> +#include "io/channel.h"
>
> Could I ask why we add this line here? I thought one of the main goals
> of this series is removing things from migration.h...



I remove from include/migration/qemu-file.h

-#include "io/channel.h"


Because all the QIOChannel functions in qemu-file.h are moved to
qemu-file-channel.h.

Great!

But migration/vmstate.h includes qemu-file.h

And migration.h includes vmstate.h

And migration.h has this functions:

void qemu_start_incoming_migration(const char *uri, Error **errp);
QIOChannel *ioc,
Error **errp);

void migration_tls_channel_connect(MigrationState *s,
   QIOChannel *ioc,
   const char *hostname,
   Error **errp);

And nothing else declares the QIOChannel.

So, the easy solution so far is to include this by now to maintain
compilation.

BTW, good catch.

So, I think that there is no other good solution to untangle the mess.

Later, Juan.






Re: [Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-18 Thread Peter Xu
On Wed, May 17, 2017 at 05:47:50PM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h |  1 +
>  include/migration/qemu-file.h |  4 
>  migration/channel.c   |  1 +
>  migration/colo.c  |  1 +
>  migration/migration.c |  1 +
>  migration/qemu-file-channel.c |  1 +
>  migration/qemu-file-channel.h | 21 +
>  migration/rdma.c  |  1 +
>  migration/savevm.c|  1 +
>  tests/test-vmstate.c  |  1 +
>  10 files changed, 29 insertions(+), 4 deletions(-)
>  create mode 100644 migration/qemu-file-channel.h
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e831259..8280df1 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -19,6 +19,7 @@
>  #include "qemu/thread.h"
>  #include "qemu/notify.h"
>  #include "migration/vmstate.h"
> +#include "io/channel.h"

Could I ask why we add this line here? I thought one of the main goals
of this series is removing things from migration.h...

>  #include "qapi-types.h"
>  #include "exec/cpu-common.h"
>  #include "qemu/coroutine_int.h"

Thanks,

-- 
Peter Xu



[Qemu-devel] [PATCH 3/9] migration: Export qemu-file-channel.c functions in its own file

2017-05-17 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 include/migration/migration.h |  1 +
 include/migration/qemu-file.h |  4 
 migration/channel.c   |  1 +
 migration/colo.c  |  1 +
 migration/migration.c |  1 +
 migration/qemu-file-channel.c |  1 +
 migration/qemu-file-channel.h | 21 +
 migration/rdma.c  |  1 +
 migration/savevm.c|  1 +
 tests/test-vmstate.c  |  1 +
 10 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 migration/qemu-file-channel.h

diff --git a/include/migration/migration.h b/include/migration/migration.h
index e831259..8280df1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -19,6 +19,7 @@
 #include "qemu/thread.h"
 #include "qemu/notify.h"
 #include "migration/vmstate.h"
+#include "io/channel.h"
 #include "qapi-types.h"
 #include "exec/cpu-common.h"
 #include "qemu/coroutine_int.h"
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 0cd648a..b5ac800 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -27,8 +27,6 @@
 
 #include "qemu-common.h"
 #include "exec/cpu-common.h"
-#include "io/channel.h"
-
 
 /* Read a chunk of data from a file at the given position.  The pos argument
  * can be ignored if the file is only be used for streaming.  The number of
@@ -119,8 +117,6 @@ typedef struct QEMUFileHooks {
 } QEMUFileHooks;
 
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
-QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
-QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
diff --git a/migration/channel.c b/migration/channel.c
index 6104de5..fed8563 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "channel.h"
 #include "migration/migration.h"
+#include "qemu-file-channel.h"
 #include "trace.h"
 #include "qapi/error.h"
 #include "io/channel-tls.h"
diff --git a/migration/colo.c b/migration/colo.c
index dd38fed..9cd0250 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
+#include "qemu-file-channel.h"
 #include "migration/colo.h"
 #include "migration/block.h"
 #include "io/channel-buffer.h"
diff --git a/migration/migration.c b/migration/migration.c
index 3ba32eb..ff3f7aa 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -19,6 +19,7 @@
 #include "qemu/main-loop.h"
 #include "migration/blocker.h"
 #include "migration/migration.h"
+#include "qemu-file-channel.h"
 #include "migration/qemu-file.h"
 #include "sysemu/sysemu.h"
 #include "block/block.h"
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 45c13f1..dc991c9 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu-file-channel.h"
 #include "migration/qemu-file.h"
 #include "io/channel-socket.h"
 #include "qemu/iov.h"
diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
new file mode 100644
index 000..d1bd5ff
--- /dev/null
+++ b/migration/qemu-file-channel.h
@@ -0,0 +1,21 @@
+/*
+ * QEMU migration file channel operations
+ *
+ * Copyright IBM, Corp. 2008
+ *
+ * Authors:
+ *  Anthony Liguori   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_FILE_CHANNEL_H
+#define QEMU_FILE_CHANNEL_H
+
+#include "io/channel.h"
+
+QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
+QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
+#endif
diff --git a/migration/rdma.c b/migration/rdma.c
index 7eaaf96..166cd60 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -20,6 +20,7 @@
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
 #include "exec/cpu-common.h"
+#include "qemu-file-channel.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/sockets.h"
diff --git a/migration/savevm.c b/migration/savevm.c
index a728414..8565103 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -36,6 +36,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/timer.h"
 #include "migration/migration.h"
+#include "qemu-file-channel.h"
 #include "postcopy-ram.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/error-report.h"
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index f694a89..1c13570 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -27,6 +27,7 @@
 #include "qemu-common.h"
 #include "migration/migration.h"
 #include "migration/vmstate.h"
+#include "../migration/qemu-file-channel.h"
 #include "qemu/coroutine.h"
 #include "io/channel-file.h"
 
-- 
2.9.3