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
