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. > >+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