Re: [Qemu-devel] [PATCH v3 10/16] qmp: add drive-mirror command

[Kevin Wolf]

Am 18.10.2012 16:49, schrieb Paolo Bonzini:
> This adds the monitor commands that start the mirroring job.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>         v2->v3: bdrv_is_inserted pulled before dereference of bs->drv
> 
>  blockdev.c       | 124 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp-commands.hx  |  21 ++++++++++
>  hmp.c            |  28 +++++++++++++
>  hmp.h            |   1 +
>  qapi-schema.json |  34 +++++++++++++++
>  qmp-commands.hx  |  42 +++++++++++++++++++
>  6 file modificati, 250 inserzioni(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 1068960..53f5c54 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1180,6 +1180,130 @@ void qmp_block_commit(const char *device,
>      drive_get_ref(drive_get_by_blockdev(bs));
>  }
>  
> +void qmp_drive_mirror(const char *device, const char *target,
> +                      bool has_format, const char *format,
> +                      enum MirrorSyncMode sync,
> +                      bool has_mode, enum NewImageMode mode,
> +                      bool has_speed, int64_t speed, Error **errp)
> +{
> +    BlockDriverInfo bdi;
> +    BlockDriverState *bs;
> +    BlockDriverState *source, *target_bs;
> +    BlockDriver *proto_drv;
> +    BlockDriver *drv = NULL;
> +    Error *local_err = NULL;
> +    int flags;
> +    uint64_t size;
> +    int ret;
> +
> +    if (!has_speed) {
> +        speed = 0;
> +    }
> +    if (!has_mode) {
> +        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
> +    }
> +
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    if (!bdrv_is_inserted(bs)) {
> +        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> +        return;
> +    }
> +
> +    if (!has_format) {
> +        format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : 
> bs->drv->format_name;
> +    }
> +    if (format) {
> +        drv = bdrv_find_format(format);
> +        if (!drv) {
> +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> +            return;
> +        }
> +    }
> +
> +    if (bdrv_in_use(bs)) {
> +        error_set(errp, QERR_DEVICE_IN_USE, device);
> +        return;
> +    }
> +
> +    flags = bs->open_flags | BDRV_O_RDWR;
> +    source = bs->backing_hd;
> +    if (!source && sync == MIRROR_SYNC_MODE_TOP) {
> +        sync = MIRROR_SYNC_MODE_FULL;
> +    }
> +
> +    proto_drv = bdrv_find_protocol(target);
> +    if (!proto_drv) {
> +        error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);

This error message is still not fixed, and totally confusing, pointing
at the wrong cause. No matter what changes to the error reporting we do
later, we should add a better error string right now.

Kevin