On 2014-12-05 at 07:12, Fam Zheng wrote:
On Thu, 12/04 14:43, Max Reitz wrote:
+ if (!bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ target_bs = bdrv_find(target);
+ if (!target_bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, target);
+ return;
+ }
+
+ bdrv_ref(target_bs);
+ bdrv_set_aio_context(target_bs, bdrv_get_aio_context(bs));
In the cover letter you said you were acquiring the AIO context but you're
not. Something like the aio_context_acquire() call in qmp_drive_backup()
seems missing.
Yes. Adding.
+ backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+ if (local_err != NULL) {
+ bdrv_unref(target_bs);
Hm, as far as I can see, backup_complete() is always run, regardless of the
operation status. backup_complete() in turn calls bdrv_unref(s->target), so
this seems unnecessary to me.
In error case, backup_start does nothing. So we need to release for the
bdrv_ref two lines above.
Ah, right, I forgot about early errors. local_err is not even set if the
block job itself fails, because by the time the block job is started,
backup_start() will have already returned. My fault.
Max
+SQMP
+blockdev-backup
+------------
+
+The device version of drive-backup: this command takes an existing named device
+as backup target.
+
+Arguments:
+
+- "device": the name of the device which should be copied.
+ (json-string)
+- "target": the target of the new image. If the file exists, or if it is a
+ device, the existing file/device will be used as the new
+ destination. If it does not exist, a new file will be created.
+ (json-string)
+- "sync": what parts of the disk image should be copied to the destination;
+ possibilities include "full" for all the disk, "top" for only the
+ sectors allocated in the topmost image, or "none" to only replicate
+ new I/O (MirrorSyncMode).
+- "speed": the maximum speed, in bytes per second (json-int, optional)
+- "on-source-error": the action to take on an error on the source, default
+ 'report'. 'stop' and 'enospc' can only be used
+ if the block device supports io-status.
+ (BlockdevOnError, optional)
+- "on-target-error": the action to take on an error on the target, default
+ 'report' (no limitations, since this applies to
+ a different block device than device).
+ (BlockdevOnError, optional)
+
+Example:
+-> { "execute": "blockdev-backup", "arguments": { "device": "src-id",
+ "target": "tgt-id" } }
Isn't "sync" missing?
Yes.
Thanks,
Fam
