Re: [Qemu-devel] savevm/loadvm

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 10/16/2013 05:51 PM, Alexey Kardashevskiy wrote:
> 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?


Ping? Or it is all fixed now?



> 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