Re: [Qemu-devel] [PULL 04/14] migration: let MigrationState be a qdev

2017-07-02 Thread Peter Xu 
On Fri, Jun 30, 2017 at 04:27:46PM -0500, Eric Blake wrote:
> On 06/30/2017 04:18 PM, Philippe Mathieu-Daudé wrote:
> > Hi Peter, Juan,
> > 
> > On 06/28/2017 08:30 AM, Juan Quintela wrote:
> >> From: Peter Xu 
> >>
> >> Let the old man "MigrationState" join the object family. Direct benefit
> >> is that we can start to use all the property features derived from
> >> current QDev, like: HW_COMPAT_* bits, command line setup for migration
> >> parameters (so will never need to set them up each time using HMP/QMP,
> >> this is really, really attractive for test writters), etc.
> >>
> >> I see no reason to disallow this happen yet. So let's start from this
> >> one, to see whether it would be anything good.
> >>
> >> Now we init the MigrationState struct statically in main() to make sure
> >> it's initialized after global properties are applied, since we'll use
> >> them during creation of the object.
> >>
> >> No functional change at all.
> >>
> 
> > qemu-system-arm: migration/migration.c:127: migrate_get_current:
> > Assertion `current_migration' failed.
> > 
> > I'v bisected to this commit using the following script:
> 
> Known issue;
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06958.html

Yeah, I'll post the fix today. Really sorry for the inconvenience!

-- 
Peter Xu

Re: [Qemu-devel] [PULL 04/14] migration: let MigrationState be a qdev

2017-06-30 Thread Eric Blake 
On 06/30/2017 04:18 PM, Philippe Mathieu-Daudé wrote:
> Hi Peter, Juan,
> 
> On 06/28/2017 08:30 AM, Juan Quintela wrote:
>> From: Peter Xu 
>>
>> Let the old man "MigrationState" join the object family. Direct benefit
>> is that we can start to use all the property features derived from
>> current QDev, like: HW_COMPAT_* bits, command line setup for migration
>> parameters (so will never need to set them up each time using HMP/QMP,
>> this is really, really attractive for test writters), etc.
>>
>> I see no reason to disallow this happen yet. So let's start from this
>> one, to see whether it would be anything good.
>>
>> Now we init the MigrationState struct statically in main() to make sure
>> it's initialized after global properties are applied, since we'll use
>> them during creation of the object.
>>
>> No functional change at all.
>>

> qemu-system-arm: migration/migration.c:127: migrate_get_current:
> Assertion `current_migration' failed.
> 
> I'v bisected to this commit using the following script:

Known issue;

https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06958.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 04/14] migration: let MigrationState be a qdev

2017-06-30 Thread Philippe Mathieu-Daudé 

Hi Peter, Juan,

On 06/28/2017 08:30 AM, Juan Quintela wrote:

From: Peter Xu 

Let the old man "MigrationState" join the object family. Direct benefit
is that we can start to use all the property features derived from
current QDev, like: HW_COMPAT_* bits, command line setup for migration
parameters (so will never need to set them up each time using HMP/QMP,
this is really, really attractive for test writters), etc.

I see no reason to disallow this happen yet. So let's start from this
one, to see whether it would be anything good.

Now we init the MigrationState struct statically in main() to make sure
it's initialized after global properties are applied, since we'll use
them during creation of the object.

No functional change at all.

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
Message-Id: <1498536619-14548-5-git-send-email-pet...@redhat.com>
Reviewed-by: Eduardo Habkost 
Signed-off-by: Juan Quintela 
---
  include/migration/misc.h |  1 +
  migration/migration.c| 78 ++--
  migration/migration.h| 19 
  vl.c |  6 
  4 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 65c7070..2d36cf5 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -45,6 +45,7 @@ void savevm_skip_section_footers(void);
  void savevm_skip_configuration(void);
  
  /* migration/migration.c */

+void migration_object_init(void);
  void qemu_start_incoming_migration(const char *uri, Error **errp);
  bool migration_is_idle(void);
  void add_migration_state_change_notifier(Notifier *notify);
diff --git a/migration/migration.c b/migration/migration.c
index f588329..2c25927 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -98,32 +98,21 @@ enum mig_rp_message_type {
 migrations at once.  For now we don't need to add
 dynamic creation of migration */
  
+static MigrationState *current_migration;

+
+void migration_object_init(void)
+{
+/* This can only be called once. */
+assert(!current_migration);
+current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION));
+}
+
  /* For outgoing */
  MigrationState *migrate_get_current(void)
  {
-static bool once;
-static MigrationState current_migration = {
-.state = MIGRATION_STATUS_NONE,
-.xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
-.mbps = -1,
-.parameters = {
-.compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
-.compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
-.decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
-.cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
-.cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
-.max_bandwidth = MAX_THROTTLE,
-.downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME,
-.x_checkpoint_delay = DEFAULT_MIGRATE_X_CHECKPOINT_DELAY,
-},
-};
-
-if (!once) {
-current_migration.parameters.tls_creds = g_strdup("");
-current_migration.parameters.tls_hostname = g_strdup("");
-once = true;
-}
-return _migration;
+/* This can only be called after the object created. */
+assert(current_migration);


This this pull I'v been unable to run qemu:

qemu-system-arm: migration/migration.c:127: migrate_get_current: 
Assertion `current_migration' failed.


I'v bisected to this commit using the following script:

#! /usr/bin/env bash
test -f test.qcow2 || qemu-img create -f qcow test.qcow2 1G
make -C build/system-arm subdir-arm-softmmu -j4 || exit 125
echo q | build/system-arm/arm-softmmu/qemu-system-arm -M virt \
  -drive if=none,file=test.qcow2,format=qcow,id=hd \
  -device virtio-blk-device,drive=hd \
  -nographic -serial null -monitor stdio
test $? -eq 0 || exit 1

Regards,

Phil.


+return current_migration;
  }
  
  MigrationIncomingState *migration_incoming_get_current(void)

@@ -1987,3 +1976,46 @@ void migrate_fd_connect(MigrationState *s)
  s->migration_thread_running = true;
  }
  
+static void migration_class_init(ObjectClass *klass, void *data)

+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->user_creatable = false;
+}
+
+static void migration_instance_init(Object *obj)
+{
+MigrationState *ms = MIGRATION_OBJ(obj);
+
+ms->state = MIGRATION_STATUS_NONE;
+ms->xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE;
+ms->mbps = -1;
+ms->parameters = (MigrationParameters) {
+.compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
+.compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
+.decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
+.cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
+.cpu_throttle_increment =