Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Changlong Xie]

On 06/02/2016 06:16 PM, Dr. David Alan Gilbert wrote:

* Stefano Stabellini (sstabell...@kernel.org) wrote:

On Thu, 2 Jun 2016, Stefano Stabellini wrote:

On Wed, 1 Jun 2016, Dr. David Alan Gilbert wrote:

* Amit Shah (amit.s...@redhat.com) wrote:

Dave, can you take a look?


Yes, I think I'm happy with it;

Reviewed-by: Dr. David Alan Gilbert 

I think people might try interesting non-Xen things with it; for
example I think if you backed QEMU's main RAM by a /tmp file
perhaps you could start a guest up even under KVM just
by loading the devices.


I added this patch to the Xen queue.


I spoke too quickly. I get:

/local/qemu/migration/savevm.c: In function 'qmp_xen_load_devices_state':
/local/qemu/migration/savevm.c:2089:5: error: implicit declaration of function 
'qemu_fopen' [-Werror=implicit-function-declaration]
/local/qemu/migration/savevm.c:2089:5: error: nested extern declaration of 
'qemu_fopen' [-Werror=nested-externs]
/local/qemu/migration/savevm.c:2089:7: error: assignment makes pointer from 
integer without a cast [-Werror]


Ah, yes, your problem is that Dan's crypto changes have probably just removed 
that;
see qmp_xen_save_devices_state and 8925839f  that changed it to use the new 
qio_channel_file_new_path.



Yes, i just send another patchset.


Dave


   CCi386-softmmu/hw/display/virtio-vga.o
cc1: all warnings being treated as errors

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK


.

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Dr. David Alan Gilbert]

* Stefano Stabellini (sstabell...@kernel.org) wrote:
> On Thu, 2 Jun 2016, Stefano Stabellini wrote:
> > On Wed, 1 Jun 2016, Dr. David Alan Gilbert wrote:
> > > * Amit Shah (amit.s...@redhat.com) wrote:
> > > > Dave, can you take a look?
> > > 
> > > Yes, I think I'm happy with it;
> > > 
> > > Reviewed-by: Dr. David Alan Gilbert 
> > > 
> > > I think people might try interesting non-Xen things with it; for
> > > example I think if you backed QEMU's main RAM by a /tmp file
> > > perhaps you could start a guest up even under KVM just
> > > by loading the devices.
> > 
> > I added this patch to the Xen queue.
> 
> I spoke too quickly. I get:
> 
> /local/qemu/migration/savevm.c: In function 'qmp_xen_load_devices_state':
> /local/qemu/migration/savevm.c:2089:5: error: implicit declaration of 
> function 'qemu_fopen' [-Werror=implicit-function-declaration]
> /local/qemu/migration/savevm.c:2089:5: error: nested extern declaration of 
> 'qemu_fopen' [-Werror=nested-externs]
> /local/qemu/migration/savevm.c:2089:7: error: assignment makes pointer from 
> integer without a cast [-Werror]

Ah, yes, your problem is that Dan's crypto changes have probably just removed 
that;
see qmp_xen_save_devices_state and 8925839f  that changed it to use the new 
qio_channel_file_new_path.

Dave

>   CCi386-softmmu/hw/display/virtio-vga.o
> cc1: all warnings being treated as errors
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Stefano Stabellini]

On Thu, 2 Jun 2016, Stefano Stabellini wrote:
> On Wed, 1 Jun 2016, Dr. David Alan Gilbert wrote:
> > * Amit Shah (amit.s...@redhat.com) wrote:
> > > Dave, can you take a look?
> > 
> > Yes, I think I'm happy with it;
> > 
> > Reviewed-by: Dr. David Alan Gilbert 
> > 
> > I think people might try interesting non-Xen things with it; for
> > example I think if you backed QEMU's main RAM by a /tmp file
> > perhaps you could start a guest up even under KVM just
> > by loading the devices.
> 
> I added this patch to the Xen queue.

I spoke too quickly. I get:

/local/qemu/migration/savevm.c: In function 'qmp_xen_load_devices_state':
/local/qemu/migration/savevm.c:2089:5: error: implicit declaration of function 
'qemu_fopen' [-Werror=implicit-function-declaration]
/local/qemu/migration/savevm.c:2089:5: error: nested extern declaration of 
'qemu_fopen' [-Werror=nested-externs]
/local/qemu/migration/savevm.c:2089:7: error: assignment makes pointer from 
integer without a cast [-Werror]
  CCi386-softmmu/hw/display/virtio-vga.o
cc1: all warnings being treated as errors

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Stefano Stabellini]

On Wed, 1 Jun 2016, Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.s...@redhat.com) wrote:
> > Dave, can you take a look?
> 
> Yes, I think I'm happy with it;
> 
> Reviewed-by: Dr. David Alan Gilbert 
> 
> I think people might try interesting non-Xen things with it; for
> example I think if you backed QEMU's main RAM by a /tmp file
> perhaps you could start a guest up even under KVM just
> by loading the devices.

I added this patch to the Xen queue.

Thanks,

Stefano

> > 
> > Thanks,
> > 
> > On (Mon) 11 Apr 2016 [11:56:02], Changlong Xie wrote:
> > > From: Wen Congyang 
> > > 
> > > Introduce a "xen-load-devices-state" QAPI command that can be used to
> > > load the state of all devices, but not the RAM or the block devices of
> > > the VM.
> > > 
> > > We only have hmp commands savevm/loadvm, and qmp commands
> > > xen-save-devices-state.
> > > 
> > > We use this new command for COLO:
> > > 1. suspend both primary vm and secondary vm
> > > 2. sync the state
> > > 3. resume both primary vm and secondary vm
> > > 
> > > In such case, we need to update all devices' state in any time.
> > > 
> > > Signed-off-by: Wen Congyang 
> > > Signed-off-by: Changlong Xie 
> > > ---
> > >  migration/savevm.c | 36 
> > >  qapi-schema.json   | 14 ++
> > >  qmp-commands.hx| 27 +++
> > >  3 files changed, 77 insertions(+)
> > > 
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 16ba443..d361a29 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -30,6 +30,7 @@
> > >  #include "hw/boards.h"
> > >  #include "hw/hw.h"
> > >  #include "hw/qdev.h"
> > > +#include "hw/xen/xen.h"
> > >  #include "net/net.h"
> > >  #include "monitor/monitor.h"
> > >  #include "sysemu/sysemu.h"
> > > @@ -1775,6 +1776,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, 
> > > MigrationIncomingState *mis)
> > >  return -EINVAL;
> > >  }
> > >  
> > > +/* Validate if it is a device's state */
> > > +if (xen_enabled() && se->is_ram) {
> > > +error_report("loadvm: %s RAM loading not allowed on Xen", idstr);
> > > +return -EINVAL;
> > > +}
> > > +
> > >  /* Add entry */
> > >  le = g_malloc0(sizeof(*le));
> > >  
> > > @@ -2084,6 +2091,35 @@ void qmp_xen_save_devices_state(const char 
> > > *filename, Error **errp)
> > >  }
> > >  }
> > >  
> > > +void qmp_xen_load_devices_state(const char *filename, Error **errp)
> > > +{
> > > +QEMUFile *f;
> > > +int ret;
> > > +
> > > +/* Guest must be paused before loading the device state; the RAM 
> > > state
> > > + * will already have been loaded by xc
> > > + */
> > > +if (runstate_is_running()) {
> > > +error_setg(errp, "Cannot update device state while vm is 
> > > running");
> > > +return;
> > > +}
> > > +vm_stop(RUN_STATE_RESTORE_VM);
> > > +
> > > +f = qemu_fopen(filename, "rb");
> > > +if (!f) {
> > > +error_setg_file_open(errp, errno, filename);
> > > +return;
> > > +}
> > > +
> > > +migration_incoming_state_new(f);
> > > +ret = qemu_loadvm_state(f);
> > > +qemu_fclose(f);
> > > +if (ret < 0) {
> > > +error_setg(errp, QERR_IO_ERROR);
> > > +}
> > > +migration_incoming_state_destroy();
> > > +}
> > > +
> > >  int load_vmstate(const char *name)
> > >  {
> > >  BlockDriverState *bs, *bs_vm_state;
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 54634c4..132264f 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -4144,6 +4144,20 @@
> > >'data': [ 'none', 'record', 'play' ] }
> > >  
> > >  ##
> > > +# @xen-load-devices-state:
> > > +#
> > > +# Load the state of all devices from file. The RAM and the block devices
> > > +# of the VM are not loaded by this command.
> > > +#
> > > +# @filename: the file to load the state of the devices from as binary
> > > +# data. See xen-save-devices-state.txt for a description of the binary
> > > +# format.
> > > +#
> > > +# Since: 2.7
> > > +##
> > > +{ 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} }
> > > +
> > > +##
> > >  # @GICCapability:
> > >  #
> > >  # The struct describes capability for a specific GIC (Generic
> > > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > > index de896a5..68620f6 100644
> > > --- a/qmp-commands.hx
> > > +++ b/qmp-commands.hx
> > > @@ -587,6 +587,33 @@ Example:
> > >  EQMP
> > >  
> > >  {
> > > +.name   = "xen-load-devices-state",
> > > +.args_type  = "filename:F",
> > > +.mhandler.cmd_new = qmp_marshal_xen_load_devices_state,
> > > +},
> > > +
> > > +SQMP
> > > +xen-load-devices-state
> > > +--
> > > +
> > > +Load the state of all devices from file. The RAM and the block devices
> > > +of the VM are not loaded by this command.
> > > +
> > > 

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Dr. David Alan Gilbert]

* Amit Shah (amit.s...@redhat.com) wrote:
> Dave, can you take a look?

Yes, I think I'm happy with it;

Reviewed-by: Dr. David Alan Gilbert 

I think people might try interesting non-Xen things with it; for
example I think if you backed QEMU's main RAM by a /tmp file
perhaps you could start a guest up even under KVM just
by loading the devices.

Dave

> 
> Thanks,
> 
> On (Mon) 11 Apr 2016 [11:56:02], Changlong Xie wrote:
> > From: Wen Congyang 
> > 
> > Introduce a "xen-load-devices-state" QAPI command that can be used to
> > load the state of all devices, but not the RAM or the block devices of
> > the VM.
> > 
> > We only have hmp commands savevm/loadvm, and qmp commands
> > xen-save-devices-state.
> > 
> > We use this new command for COLO:
> > 1. suspend both primary vm and secondary vm
> > 2. sync the state
> > 3. resume both primary vm and secondary vm
> > 
> > In such case, we need to update all devices' state in any time.
> > 
> > Signed-off-by: Wen Congyang 
> > Signed-off-by: Changlong Xie 
> > ---
> >  migration/savevm.c | 36 
> >  qapi-schema.json   | 14 ++
> >  qmp-commands.hx| 27 +++
> >  3 files changed, 77 insertions(+)
> > 
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 16ba443..d361a29 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -30,6 +30,7 @@
> >  #include "hw/boards.h"
> >  #include "hw/hw.h"
> >  #include "hw/qdev.h"
> > +#include "hw/xen/xen.h"
> >  #include "net/net.h"
> >  #include "monitor/monitor.h"
> >  #include "sysemu/sysemu.h"
> > @@ -1775,6 +1776,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, 
> > MigrationIncomingState *mis)
> >  return -EINVAL;
> >  }
> >  
> > +/* Validate if it is a device's state */
> > +if (xen_enabled() && se->is_ram) {
> > +error_report("loadvm: %s RAM loading not allowed on Xen", idstr);
> > +return -EINVAL;
> > +}
> > +
> >  /* Add entry */
> >  le = g_malloc0(sizeof(*le));
> >  
> > @@ -2084,6 +2091,35 @@ void qmp_xen_save_devices_state(const char 
> > *filename, Error **errp)
> >  }
> >  }
> >  
> > +void qmp_xen_load_devices_state(const char *filename, Error **errp)
> > +{
> > +QEMUFile *f;
> > +int ret;
> > +
> > +/* Guest must be paused before loading the device state; the RAM state
> > + * will already have been loaded by xc
> > + */
> > +if (runstate_is_running()) {
> > +error_setg(errp, "Cannot update device state while vm is running");
> > +return;
> > +}
> > +vm_stop(RUN_STATE_RESTORE_VM);
> > +
> > +f = qemu_fopen(filename, "rb");
> > +if (!f) {
> > +error_setg_file_open(errp, errno, filename);
> > +return;
> > +}
> > +
> > +migration_incoming_state_new(f);
> > +ret = qemu_loadvm_state(f);
> > +qemu_fclose(f);
> > +if (ret < 0) {
> > +error_setg(errp, QERR_IO_ERROR);
> > +}
> > +migration_incoming_state_destroy();
> > +}
> > +
> >  int load_vmstate(const char *name)
> >  {
> >  BlockDriverState *bs, *bs_vm_state;
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 54634c4..132264f 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4144,6 +4144,20 @@
> >'data': [ 'none', 'record', 'play' ] }
> >  
> >  ##
> > +# @xen-load-devices-state:
> > +#
> > +# Load the state of all devices from file. The RAM and the block devices
> > +# of the VM are not loaded by this command.
> > +#
> > +# @filename: the file to load the state of the devices from as binary
> > +# data. See xen-save-devices-state.txt for a description of the binary
> > +# format.
> > +#
> > +# Since: 2.7
> > +##
> > +{ 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} }
> > +
> > +##
> >  # @GICCapability:
> >  #
> >  # The struct describes capability for a specific GIC (Generic
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index de896a5..68620f6 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -587,6 +587,33 @@ Example:
> >  EQMP
> >  
> >  {
> > +.name   = "xen-load-devices-state",
> > +.args_type  = "filename:F",
> > +.mhandler.cmd_new = qmp_marshal_xen_load_devices_state,
> > +},
> > +
> > +SQMP
> > +xen-load-devices-state
> > +--
> > +
> > +Load the state of all devices from file. The RAM and the block devices
> > +of the VM are not loaded by this command.
> > +
> > +Arguments:
> > +
> > +- "filename": the file to load the state of the devices from as binary
> > +data. See xen-save-devices-state.txt for a description of the binary
> > +format.
> > +
> > +Example:
> > +
> > +-> { "execute": "xen-load-devices-state",
> > + "arguments": { "filename": "/tmp/resume" } }
> > +<- { "return": {} }
> > +
> > +EQMP
> > +
> > +{
> >  .name   = 

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Eric Blake]

On 05/30/2016 09:17 AM, Stefano Stabellini wrote:
> On Fri, 27 May 2016, Anthony PERARD wrote:
>> On Mon, Apr 11, 2016 at 11:56:02AM +0800, Changlong Xie wrote:
>>> From: Wen Congyang 
>>>
>>> Introduce a "xen-load-devices-state" QAPI command that can be used to
>>> load the state of all devices, but not the RAM or the block devices of
>>> the VM.
>>>
>>> We only have hmp commands savevm/loadvm, and qmp commands
>>> xen-save-devices-state.
>>>
>>> We use this new command for COLO:
>>> 1. suspend both primary vm and secondary vm
>>> 2. sync the state
>>> 3. resume both primary vm and secondary vm
>>>
>>> In such case, we need to update all devices' state in any time.
>>>
>>> Signed-off-by: Wen Congyang 
>>> Signed-off-by: Changlong Xie 
>>
>> This patch looks good to me.
>>
>> Reviewed-by: Anthony PERARD 
> 
> It would be nicer (and less problematic) to load the state from a file
> descriptor, but given that we still saving the state to file, it would
> be unfair to ask to use file descriptors here.

If you use qemu_open(), then you can pass the magic "/dev/fdset/1"
notation to use a file descriptor previously added to an fdset.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Stefano Stabellini]

On Fri, 27 May 2016, Anthony PERARD wrote:
> On Mon, Apr 11, 2016 at 11:56:02AM +0800, Changlong Xie wrote:
> > From: Wen Congyang 
> > 
> > Introduce a "xen-load-devices-state" QAPI command that can be used to
> > load the state of all devices, but not the RAM or the block devices of
> > the VM.
> > 
> > We only have hmp commands savevm/loadvm, and qmp commands
> > xen-save-devices-state.
> > 
> > We use this new command for COLO:
> > 1. suspend both primary vm and secondary vm
> > 2. sync the state
> > 3. resume both primary vm and secondary vm
> > 
> > In such case, we need to update all devices' state in any time.
> > 
> > Signed-off-by: Wen Congyang 
> > Signed-off-by: Changlong Xie 
> 
> This patch looks good to me.
> 
> Reviewed-by: Anthony PERARD 

It would be nicer (and less problematic) to load the state from a file
descriptor, but given that we still saving the state to file, it would
be unfair to ask to use file descriptors here.

Acked-by: Stefano Stabellini 

Given that this is migration code, it still needs an ack from Juan or
Amit.

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Amit Shah]

Dave, can you take a look?

Thanks,

On (Mon) 11 Apr 2016 [11:56:02], Changlong Xie wrote:
> From: Wen Congyang 
> 
> Introduce a "xen-load-devices-state" QAPI command that can be used to
> load the state of all devices, but not the RAM or the block devices of
> the VM.
> 
> We only have hmp commands savevm/loadvm, and qmp commands
> xen-save-devices-state.
> 
> We use this new command for COLO:
> 1. suspend both primary vm and secondary vm
> 2. sync the state
> 3. resume both primary vm and secondary vm
> 
> In such case, we need to update all devices' state in any time.
> 
> Signed-off-by: Wen Congyang 
> Signed-off-by: Changlong Xie 
> ---
>  migration/savevm.c | 36 
>  qapi-schema.json   | 14 ++
>  qmp-commands.hx| 27 +++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 16ba443..d361a29 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -30,6 +30,7 @@
>  #include "hw/boards.h"
>  #include "hw/hw.h"
>  #include "hw/qdev.h"
> +#include "hw/xen/xen.h"
>  #include "net/net.h"
>  #include "monitor/monitor.h"
>  #include "sysemu/sysemu.h"
> @@ -1775,6 +1776,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, 
> MigrationIncomingState *mis)
>  return -EINVAL;
>  }
>  
> +/* Validate if it is a device's state */
> +if (xen_enabled() && se->is_ram) {
> +error_report("loadvm: %s RAM loading not allowed on Xen", idstr);
> +return -EINVAL;
> +}
> +
>  /* Add entry */
>  le = g_malloc0(sizeof(*le));
>  
> @@ -2084,6 +2091,35 @@ void qmp_xen_save_devices_state(const char *filename, 
> Error **errp)
>  }
>  }
>  
> +void qmp_xen_load_devices_state(const char *filename, Error **errp)
> +{
> +QEMUFile *f;
> +int ret;
> +
> +/* Guest must be paused before loading the device state; the RAM state
> + * will already have been loaded by xc
> + */
> +if (runstate_is_running()) {
> +error_setg(errp, "Cannot update device state while vm is running");
> +return;
> +}
> +vm_stop(RUN_STATE_RESTORE_VM);
> +
> +f = qemu_fopen(filename, "rb");
> +if (!f) {
> +error_setg_file_open(errp, errno, filename);
> +return;
> +}
> +
> +migration_incoming_state_new(f);
> +ret = qemu_loadvm_state(f);
> +qemu_fclose(f);
> +if (ret < 0) {
> +error_setg(errp, QERR_IO_ERROR);
> +}
> +migration_incoming_state_destroy();
> +}
> +
>  int load_vmstate(const char *name)
>  {
>  BlockDriverState *bs, *bs_vm_state;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..132264f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4144,6 +4144,20 @@
>'data': [ 'none', 'record', 'play' ] }
>  
>  ##
> +# @xen-load-devices-state:
> +#
> +# Load the state of all devices from file. The RAM and the block devices
> +# of the VM are not loaded by this command.
> +#
> +# @filename: the file to load the state of the devices from as binary
> +# data. See xen-save-devices-state.txt for a description of the binary
> +# format.
> +#
> +# Since: 2.7
> +##
> +{ 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} }
> +
> +##
>  # @GICCapability:
>  #
>  # The struct describes capability for a specific GIC (Generic
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index de896a5..68620f6 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -587,6 +587,33 @@ Example:
>  EQMP
>  
>  {
> +.name   = "xen-load-devices-state",
> +.args_type  = "filename:F",
> +.mhandler.cmd_new = qmp_marshal_xen_load_devices_state,
> +},
> +
> +SQMP
> +xen-load-devices-state
> +--
> +
> +Load the state of all devices from file. The RAM and the block devices
> +of the VM are not loaded by this command.
> +
> +Arguments:
> +
> +- "filename": the file to load the state of the devices from as binary
> +data. See xen-save-devices-state.txt for a description of the binary
> +format.
> +
> +Example:
> +
> +-> { "execute": "xen-load-devices-state",
> + "arguments": { "filename": "/tmp/resume" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +{
>  .name   = "xen-set-global-dirty-log",
>  .args_type  = "enable:b",
>  .mhandler.cmd_new = qmp_marshal_xen_set_global_dirty_log,
> -- 
> 1.9.3
> 
> 
> 

Amit

Re: [Qemu-devel] [PATCH v4 1/1] Introduce "xen-load-devices-state"

[Anthony PERARD]

On Mon, Apr 11, 2016 at 11:56:02AM +0800, Changlong Xie wrote:
> From: Wen Congyang 
> 
> Introduce a "xen-load-devices-state" QAPI command that can be used to
> load the state of all devices, but not the RAM or the block devices of
> the VM.
> 
> We only have hmp commands savevm/loadvm, and qmp commands
> xen-save-devices-state.
> 
> We use this new command for COLO:
> 1. suspend both primary vm and secondary vm
> 2. sync the state
> 3. resume both primary vm and secondary vm
> 
> In such case, we need to update all devices' state in any time.
> 
> Signed-off-by: Wen Congyang 
> Signed-off-by: Changlong Xie 

This patch looks good to me.

Reviewed-by: Anthony PERARD 

-- 
Anthony PERARD