Re: [Qemu-devel] savevm/loadvm

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 10/10/2013 02:50 PM, Alexey Kardashevskiy wrote:
> On 10/09/2013 06:47 PM, Paolo Bonzini wrote:
>> Il 09/10/2013 09:15, Alexey Kardashevskiy ha scritto:
>>> Sorry for my ignorance (I never ever touched this part of qemu) but how can
>>> you possibly avoid block.c while doing savevm? The qcow2 driver must not
>>> use posix read()/write(), right? So no matter how, all writes end up in
>>> bdrv_co_do_writev() which changes blocks number. Or use
>>> raw_aio_readv()/raw_aio_writev() API directly? Please give some more hints.
>>> Thanks.
>>
>> I think Kevin was suggesting using qcow_aio_writev directly, or
>> something like that.  But it is not trivial, especially because
>> save_vm_state takes byte offsets instead of sectors.  So for now I'd
>> still go for the more hacky solution.
> 
> I failed to find qcow_aio_writev() or anything like that. qcow2_co_writev()
> uses block.c. And I tried this:
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 4a9888c..17faf8b 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1837,10 +1837,16 @@ static int qcow2_save_vmstate(BlockDriverState *bs,
> QEMUIOVector *qiov,
>      BDRVQcowState *s = bs->opaque;
>      int growable = bs->growable;
>      int ret;
> +    int64_t total_sectors = bs->total_sectors;
> 
>      BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
>      bs->growable = 1;
>      ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> +    /*
> +     * Setting @growable may cause underlying bdrv_co_do_writev()
> +     * to increase bs->total_sectors and we do not want this to happen.
> +     */
> +    bs->total_sectors = total_sectors;
>      bs->growable = growable;
> 
>      return ret;
> 
> 
> It breaks loadvm in a different (weird) way, the error is something like
> "ram" or "spapr/htab" (streams registered with register_savevm_live())
> chunk cannot be read. Need to debug more...


Just to keep the conversation going :) The patch below helps while the
patch above creates snapshots which cannot be loaded.

And there is no qcow_aio_writev-like API to fix it, what did you mean?

Why not just revert the breaking patch?

Thanks.

diff --git a/savevm.c b/savevm.c
index e0c8aee..aeda0d1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -42,6 +42,7 @@
 #include "qemu/iov.h"
 #include "block/snapshot.h"
 #include "block/qapi.h"
+#include "block/block_int.h"

 #define SELF_ANNOUNCE_ROUNDS 5

@@ -2389,6 +2390,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     qemu_timeval tv;
     struct tm tm;
     const char *name = qdict_get_try_str(qdict, "name");
+    int64_t total_sectors;

     /* Verify if there is a device that doesn't support snapshots and is
writable */
     bs = NULL;
@@ -2442,6 +2444,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     }

     /* save the VM state */
+    total_sectors = bs->total_sectors;
     f = qemu_fopen_bdrv(bs, 1);
     if (!f) {
         monitor_printf(mon, "Could not open VM state file\n");
@@ -2450,6 +2453,11 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     ret = qemu_savevm_state(f);
     vm_state_size = qemu_ftell(f);
     qemu_fclose(f);
+    /*
+     * Setting @growable may cause underlying bdrv_co_do_writev()
+     * to increase bs->total_sectors and we do not want this to happen.
+     */
+    bs->total_sectors = total_sectors;
     if (ret < 0) {
         monitor_printf(mon, "Error %d while writing VM\n", ret);
         goto the_end;




-- 
Alexey