Patchwork [2/2] Add the blockdev-reopen and blockdev-migrate commands

login
register
mail settings
Submitter Federico Simoncelli
Date Feb. 24, 2012, 11:37 a.m.
Message ID <1330083459-13583-2-git-send-email-fsimonce@redhat.com>
Download mbox | patch
Permalink /patch/142816/
State New
Headers show

Comments

Federico Simoncelli - Feb. 24, 2012, 11:37 a.m.
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
 block/blkmirror.c |    2 +-
 blockdev.c        |  109 +++++++++++++++++++++++++++++++++++++++++++++++------
 hmp-commands.hx   |   36 +++++++++++++++++
 hmp.c             |   30 ++++++++++++++
 hmp.h             |    2 +
 qapi-schema.json  |   63 ++++++++++++++++++++++++++++++
 6 files changed, 229 insertions(+), 13 deletions(-)
Kevin Wolf - Feb. 24, 2012, 12:03 p.m.
Am 24.02.2012 12:37, schrieb Federico Simoncelli:
> Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
> ---
>  block/blkmirror.c |    2 +-
>  blockdev.c        |  109 +++++++++++++++++++++++++++++++++++++++++++++++------
>  hmp-commands.hx   |   36 +++++++++++++++++
>  hmp.c             |   30 ++++++++++++++
>  hmp.h             |    2 +
>  qapi-schema.json  |   63 ++++++++++++++++++++++++++++++
>  6 files changed, 229 insertions(+), 13 deletions(-)
> 
> diff --git a/block/blkmirror.c b/block/blkmirror.c
> index 39927c8..49e3381 100644
> --- a/block/blkmirror.c
> +++ b/block/blkmirror.c
> @@ -81,7 +81,7 @@ static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
>          bdrv_delete(m->bs[0]);
>          return -ENOMEM;
>      }
> -    ret = bdrv_open(m->bs[1], filename, flags, NULL);
> +    ret = bdrv_open(m->bs[1], filename, flags | BDRV_O_NO_BACKING, NULL);
>      if (ret < 0) {
>          bdrv_delete(m->bs[0]);
>          return ret;

Was this hunk meant to be in patch 1?

> diff --git a/blockdev.c b/blockdev.c
> index 2c132a3..1df2542 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -646,9 +646,8 @@ void do_commit(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> -void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
> -                                bool has_format, const char *format,
> -                                Error **errp)
> +static void change_blockdev_image(const char *device, const char *new_image_file,
> +                                  const char *format, bool create, Error **errp)
>  {
>      BlockDriverState *bs;
>      BlockDriver *drv, *old_drv, *proto_drv;
> @@ -671,7 +670,7 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
>      old_drv = bs->drv;
>      flags = bs->open_flags;
>  
> -    if (!has_format) {
> +    if (!format) {
>          format = "qcow2";
>      }
>  
> @@ -681,24 +680,26 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
>          return;
>      }
>  
> -    proto_drv = bdrv_find_protocol(snapshot_file);
> +    proto_drv = bdrv_find_protocol(new_image_file);
>      if (!proto_drv) {
>          error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
>          return;
>      }
>  
> -    ret = bdrv_img_create(snapshot_file, format, bs->filename,
> -                          bs->drv->format_name, NULL, -1, flags);
> -    if (ret) {
> -        error_set(errp, QERR_UNDEFINED_ERROR);
> -        return;
> +    if (create) {
> +        ret = bdrv_img_create(new_image_file, format, bs->filename,
> +                              bs->drv->format_name, NULL, -1, flags);
> +        if (ret) {
> +            error_set(errp, QERR_UNDEFINED_ERROR);
> +            return;
> +        }
>      }
>  
>      bdrv_drain_all();
>      bdrv_flush(bs);
>  
>      bdrv_close(bs);
> -    ret = bdrv_open(bs, snapshot_file, flags, drv);
> +    ret = bdrv_open(bs, new_image_file, flags, drv);
>      /*
>       * If reopening the image file we just created fails, fall back
>       * and try to re-open the original image. If that fails too, we
> @@ -709,11 +710,95 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
>          if (ret != 0) {
>              error_set(errp, QERR_OPEN_FILE_FAILED, old_filename);
>          } else {
> -            error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file);
> +            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
>          }
>      }
>  }
>  
> +void qmp_blockdev_reopen(const char *device, const char *new_image_file,
> +                         bool has_format, const char *format, Error **errp)
> +{
> +    change_blockdev_image(device, new_image_file,
> +                          has_format ? format : NULL, false, errp);
> +}
> +
> +void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
> +                                bool has_format, const char *format,
> +                                Error **errp)
> +{
> +    change_blockdev_image(device, snapshot_file,
> +                          has_format ? format : NULL, true, errp);
> +}
> +
> +void qmp_blockdev_migrate(const char *device, BlockMigrateOp mode,
> +                          const char *destination, bool has_new_image_file,
> +                          const char *new_image_file, Error **errp)
> +{
> +    BlockDriverState *bs;
> +    BlockDriver *drv;
> +    int i, j, escape;
> +    char filename[2048];
> +
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +    if (bdrv_in_use(bs)) {
> +        error_set(errp, QERR_DEVICE_IN_USE, device);
> +        return;
> +    }
> +
> +    if (mode == BLOCK_MIGRATE_OP_MIRROR) {

Move into a separate function?

> +        drv = bdrv_find_format("blkmirror");
> +        if (!drv) {
> +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, "blkmirror");
> +            return;
> +        }
> +
> +        if (!has_new_image_file) {
> +            new_image_file = bs->filename;
> +        }
> +
> +        pstrcpy(filename, sizeof(filename), "blkmirror:");
> +        i = strlen("blkmirror:");
> +
> +        escape = 0;
> +        for (j = 0; j < strlen(new_image_file); j++) {
> + loop:
> +            if (!(i < sizeof(filename) - 2)) {
> +                error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> +                          "new-image-file", "shorter filename");
> +                return;
> +            }
> +
> +            if (new_image_file[j] == ':' || new_image_file[j] == '\\') {

Markus suggested that using comma for the separator is better even
though it requires escaping. It would allow to parse the option string
with QemuOpts.

> +                if (!escape) {
> +                    filename[i++] = '\\', escape = 1;
> +                    goto loop;
> +                } else {
> +                    escape = 0;
> +                }
> +            }
> +
> +            filename[i++] = new_image_file[j];
> +        }

Looks like a string helper for qemu-option.c (it contains the parser, so
keeping the escaping nearby would make sense).

> +
> +        if (i + strlen(destination) + 2 > sizeof(filename)) {
> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> +                      "destination", "shorter filename");
> +            return;
> +        }
> +
> +        filename[i++] = ':';
> +        pstrcpy(filename + i, sizeof(filename) - i - 2, destination);
> +
> +        change_blockdev_image(device, filename, "blkmirror", false, errp);
> +    } else if (mode == BLOCK_MIGRATE_OP_STREAM) {
> +        error_set(errp, QERR_NOT_SUPPORTED);

Why even define it then?

> +    }
> +}
> +
>  static void eject_device(BlockDriverState *bs, int force, Error **errp)
>  {
>      if (bdrv_in_use(bs)) {
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 573b823..ccb1f62 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -886,6 +886,42 @@ Snapshot device, using snapshot file as target if provided
>  ETEXI
>  
>      {
> +        .name       = "blkdev_reopen",

NACK on the name. Let's reserve blkdev_*/blockdev_* for the proper API
(that we'll release with the qemu version that comes with Hurd 1.0).

drive_* would align well with the existing drive_add/del commands.

> +        .args_type  = "device:B,new-image-file:s?,format:s?",
> +        .params     = "device [new-image-file] [format]",
> +        .help       = "Assigns a new image file to a device.\n\t\t\t"
> +                      "The image will be opened using the format\n\t\t\t"
> +                      "specified or 'qcow2' by default.",
> +        .mhandler.cmd = hmp_blkdev_reopen,
> +    },
> +
> +STEXI
> +@item blkdev_reopen
> +@findex blkdev_reopen
> +Assigns a new image file to a device.
> +ETEXI
> +
> +    {
> +        .name       = "blkdev_migrate",
> +        .args_type  = "mirror:-m,device:B,destination:s,new-image-file:s?",
> +        .params     = "device mode destination [new-image-file]",
> +        .help       = "Migrates the device to a new destination.\n\t\t\t"
> +                      "The default migration method is 'stream',\n\t\t\t"
> +                      "the option -m is used to select 'mirror'\n\t\t\t"
> +                      "(currently the only method supported).\n\t\t\t"
> +                      "An optional existing image file that will\n\t\t\t"
> +                      "replace the current one in the device\n\t\t\t"
> +                      "can be specified.",
> +        .mhandler.cmd = hmp_blkdev_migrate,
> +    },
> +
> +STEXI
> +@item blkdev_migrate
> +@findex blkdev_migrate
> +Migrates the device to a new destination.
> +ETEXI
> +
> +    {
>          .name       = "drive_add",
>          .args_type  = "pci_addr:s,opts:s",
>          .params     = "[[<domain>:]<bus>:]<slot>\n"
> diff --git a/hmp.c b/hmp.c
> index 8ff8c94..b687b8f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -701,6 +701,36 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &errp);
>  }
>  
> +void hmp_blkdev_reopen(Monitor *mon, const QDict *qdict)
> +{
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *filename = qdict_get_str(qdict, "new-image-file");
> +    const char *format = qdict_get_try_str(qdict, "format");
> +    Error *errp = NULL;
> +
> +    qmp_blockdev_reopen(device, filename, !!format, format, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict)
> +{
> +    bool mirror = qdict_get_try_bool(qdict, "mirror", 0);
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *destination = qdict_get_str(qdict, "destination");
> +    const char *new_image_file = qdict_get_try_str(qdict, "new-image-file");
> +    BlockMigrateOp mode;
> +    Error *errp = NULL;
> +
> +    if (mirror)
> +        mode = BLOCK_MIGRATE_OP_MIRROR;
> +    else
> +        mode = BLOCK_MIGRATE_OP_STREAM;
> +
> +    qmp_blockdev_migrate(device, mode, destination,
> +                         !!new_image_file, new_image_file, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> +
>  void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
>  {
>      qmp_migrate_cancel(NULL);
> diff --git a/hmp.h b/hmp.h
> index 18eecbd..f4e802b 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -47,6 +47,8 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict);
>  void hmp_balloon(Monitor *mon, const QDict *qdict);
>  void hmp_block_resize(Monitor *mon, const QDict *qdict);
>  void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
> +void hmp_blkdev_reopen(Monitor *mon, const QDict *qdict);
> +void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index d02ee86..c86796a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1136,6 +1136,69 @@
>    'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
>  
>  ##
> +# @blockdev-reopen
> +#
> +# Assigns a new image file to a device.
> +#
> +# @device: the name of the device for which we are chainging the image file.
> +#
> +# @new-image-file: the target of the new image. If the file doesn't exists the
> +#                  command will fail.
> +#
> +# @format: #optional the format of the new image, default is 'qcow2'.
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @new-image-file can't be opened, OpenFileFailed
> +#          If @format is invalid, InvalidBlockFormat
> +#
> +# Since 1.1
> +##
> +
> +{ 'command': 'blockdev-reopen',
> +  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }

Same consideration on the name.

Also I think we should immediately mark the command as deprecated (I
think there is precedence for it, though I can't remember which command
it was). This is not an interface we'll want to keep long term.

Kevin
Federico Simoncelli - Feb. 24, 2012, 12:12 p.m.
----- Original Message -----
> From: "Kevin Wolf" <kwolf@redhat.com>
> To: "Federico Simoncelli" <fsimonce@redhat.com>
> Cc: qemu-devel@nongnu.org, "Marcelo Tosatti" <mtosatti@redhat.com>, lcapitulino@redhat.com, "Paolo Bonzini"
> <pbonzini@redhat.com>, "Markus Armbruster" <armbru@redhat.com>
> Sent: Friday, February 24, 2012 1:03:08 PM
> Subject: Re: [PATCH 2/2] Add the blockdev-reopen and blockdev-migrate commands
> 
> Am 24.02.2012 12:37, schrieb Federico Simoncelli:
> > Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
> > ---
> >  block/blkmirror.c |    2 +-
> >  blockdev.c        |  109
> >  +++++++++++++++++++++++++++++++++++++++++++++++------
> >  hmp-commands.hx   |   36 +++++++++++++++++
> >  hmp.c             |   30 ++++++++++++++
> >  hmp.h             |    2 +
> >  qapi-schema.json  |   63 ++++++++++++++++++++++++++++++
> >  6 files changed, 229 insertions(+), 13 deletions(-)
> > 
> > diff --git a/block/blkmirror.c b/block/blkmirror.c
> > index 39927c8..49e3381 100644
> > --- a/block/blkmirror.c
> > +++ b/block/blkmirror.c
> > @@ -81,7 +81,7 @@ static int blkmirror_open(BlockDriverState *bs,
> > const char *filename, int flags)
> >          bdrv_delete(m->bs[0]);
> >          return -ENOMEM;
> >      }
> > -    ret = bdrv_open(m->bs[1], filename, flags, NULL);
> > +    ret = bdrv_open(m->bs[1], filename, flags | BDRV_O_NO_BACKING,
> > NULL);
> >      if (ret < 0) {
> >          bdrv_delete(m->bs[0]);
> >          return ret;
> 
> Was this hunk meant to be in patch 1?

Not necessarily, I thought a lot about it. I didn't want to modify Marcelo's
patch too much. This flag is actually mandatory only to make blockdev-migrate
work with a destination that doesn't have a backing file yet, so I thought it
was more appropriate to squash it here.
Paolo Bonzini - Feb. 24, 2012, 1:11 p.m.
On 02/24/2012 01:03 PM, Kevin Wolf wrote:
>> + loop:
>> +            if (!(i < sizeof(filename) - 2)) {
>> +                error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> +                          "new-image-file", "shorter filename");
>> +                return;
>> +            }
>> +
>> +            if (new_image_file[j] == ':' || new_image_file[j] == '\\') {
> 
> Markus suggested that using comma for the separator is better even
> though it requires escaping. It would allow to parse the option string
> with QemuOpts.

Isn't that a bit overengineering for an internal interface?

>> +                if (!escape) {
>> +                    filename[i++] = '\\', escape = 1;
>> +                    goto loop;
>> +                } else {
>> +                    escape = 0;
>> +                }
>> +            }
>> +
>> +            filename[i++] = new_image_file[j];
>> +        }
> 
> Looks like a string helper for qemu-option.c (it contains the parser, so
> keeping the escaping nearby would make sense).
> 
>> +
>> +        if (i + strlen(destination) + 2 > sizeof(filename)) {
>> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> +                      "destination", "shorter filename");
>> +            return;
>> +        }
>> +
>> +        filename[i++] = ':';
>> +        pstrcpy(filename + i, sizeof(filename) - i - 2, destination);
>> +
>> +        change_blockdev_image(device, filename, "blkmirror", false, errp);
>> +    } else if (mode == BLOCK_MIGRATE_OP_STREAM) {
>> +        error_set(errp, QERR_NOT_SUPPORTED);
> 
> Why even define it then?

Because it's the default for the HMP.  Until we have live merging,
mirror mode cannot be consider anything more than a hack.

>>  ##
>> +# @blockdev-reopen
>> +#
>> +# Assigns a new image file to a device.
>> +#
>> +# @device: the name of the device for which we are chainging the image file.
>> +#
>> +# @new-image-file: the target of the new image. If the file doesn't exists the
>> +#                  command will fail.
>> +#
>> +# @format: #optional the format of the new image, default is 'qcow2'.
>> +#
>> +# Returns: nothing on success
>> +#          If @device is not a valid block device, DeviceNotFound
>> +#          If @new-image-file can't be opened, OpenFileFailed
>> +#          If @format is invalid, InvalidBlockFormat
>> +#
>> +# Since 1.1
>> +##
>> +
>> +{ 'command': 'blockdev-reopen',
>> +  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }
> 
> Same consideration on the name.
> 
> Also I think we should immediately mark the command as deprecated (I
> think there is precedence for it, though I can't remember which command
> it was). This is not an interface we'll want to keep long term.

What about blockdev-snapshot-sync/snapshot_blkdev?

Paolo
Luiz Capitulino - Feb. 24, 2012, 5:04 p.m.
On Fri, 24 Feb 2012 13:03:08 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> >  static void eject_device(BlockDriverState *bs, int force, Error **errp)
> >  {
> >      if (bdrv_in_use(bs)) {
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 573b823..ccb1f62 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -886,6 +886,42 @@ Snapshot device, using snapshot file as target if provided
> >  ETEXI
> >  
> >      {
> > +        .name       = "blkdev_reopen",
> 
> NACK on the name. Let's reserve blkdev_*/blockdev_* for the proper API
> (that we'll release with the qemu version that comes with Hurd 1.0).

lol.

> >  ##
> > +# @blockdev-reopen
> > +#
> > +# Assigns a new image file to a device.
> > +#
> > +# @device: the name of the device for which we are chainging the image file.
> > +#
> > +# @new-image-file: the target of the new image. If the file doesn't exists the
> > +#                  command will fail.
> > +#
> > +# @format: #optional the format of the new image, default is 'qcow2'.
> > +#
> > +# Returns: nothing on success
> > +#          If @device is not a valid block device, DeviceNotFound
> > +#          If @new-image-file can't be opened, OpenFileFailed
> > +#          If @format is invalid, InvalidBlockFormat
> > +#
> > +# Since 1.1
> > +##
> > +
> > +{ 'command': 'blockdev-reopen',
> > +  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }
> 
> Same consideration on the name.
> 
> Also I think we should immediately mark the command as deprecated (I
> think there is precedence for it, though I can't remember which command
> it was). This is not an interface we'll want to keep long term.

We can only mark an interface as deprecated when there's a substitute for it.
Markus Armbruster - Feb. 27, 2012, 2:57 p.m.
Kevin Wolf <kwolf@redhat.com> writes:

> Am 24.02.2012 12:37, schrieb Federico Simoncelli:
[...]
>> diff --git a/blockdev.c b/blockdev.c
>> index 2c132a3..1df2542 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
[...]
>> +void qmp_blockdev_migrate(const char *device, BlockMigrateOp mode,
>> +                          const char *destination, bool has_new_image_file,
>> +                          const char *new_image_file, Error **errp)
>> +{
>> +    BlockDriverState *bs;
>> +    BlockDriver *drv;
>> +    int i, j, escape;
>> +    char filename[2048];
>> +
>> +    bs = bdrv_find(device);
>> +    if (!bs) {
>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>> +        return;
>> +    }
>> +    if (bdrv_in_use(bs)) {
>> +        error_set(errp, QERR_DEVICE_IN_USE, device);
>> +        return;
>> +    }
>> +
>> +    if (mode == BLOCK_MIGRATE_OP_MIRROR) {
>
> Move into a separate function?
>
>> +        drv = bdrv_find_format("blkmirror");
>> +        if (!drv) {
>> +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, "blkmirror");
>> +            return;
>> +        }
>> +
>> +        if (!has_new_image_file) {
>> +            new_image_file = bs->filename;
>> +        }
>> +
>> +        pstrcpy(filename, sizeof(filename), "blkmirror:");
>> +        i = strlen("blkmirror:");
>> +
>> +        escape = 0;
>> +        for (j = 0; j < strlen(new_image_file); j++) {
>> + loop:
>> +            if (!(i < sizeof(filename) - 2)) {
>> +                error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> +                          "new-image-file", "shorter filename");
>> +                return;
>> +            }
>> +
>> +            if (new_image_file[j] == ':' || new_image_file[j] == '\\') {
>
> Markus suggested that using comma for the separator is better even
> though it requires escaping. It would allow to parse the option string
> with QemuOpts.

Yes, I did.  While I'm no particular friend of QemuOpts, inventing more
ad hoc syntaxes is even worse.

On the other hand, many block drivers already do, often a colon syntax
similar to yours.  But not all.  I guess consistency is a lost cause
there, as is generality (support for arbitrary filenames).  Maybe it's
best to concede defeat, do another ad hoc syntax now, and hope we can
replace the whole -drive mess by something saner eventually.

>> +                if (!escape) {
>> +                    filename[i++] = '\\', escape = 1;
>> +                    goto loop;
>> +                } else {
>> +                    escape = 0;
>> +                }
>> +            }
>> +
>> +            filename[i++] = new_image_file[j];
>> +        }
>
> Looks like a string helper for qemu-option.c (it contains the parser, so
> keeping the escaping nearby would make sense).
>
>> +
>> +        if (i + strlen(destination) + 2 > sizeof(filename)) {
>> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> +                      "destination", "shorter filename");
>> +            return;
>> +        }
>> +
>> +        filename[i++] = ':';
>> +        pstrcpy(filename + i, sizeof(filename) - i - 2, destination);
>> +
>> +        change_blockdev_image(device, filename, "blkmirror", false, errp);
>> +    } else if (mode == BLOCK_MIGRATE_OP_STREAM) {
>> +        error_set(errp, QERR_NOT_SUPPORTED);
>
> Why even define it then?
>
>> +    }
>> +}
>> +
>>  static void eject_device(BlockDriverState *bs, int force, Error **errp)
>>  {
>>      if (bdrv_in_use(bs)) {
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 573b823..ccb1f62 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -886,6 +886,42 @@ Snapshot device, using snapshot file as target if provided
>>  ETEXI
>>  
>>      {
>> +        .name       = "blkdev_reopen",
>
> NACK on the name. Let's reserve blkdev_*/blockdev_* for the proper API

Yes, please.

> (that we'll release with the qemu version that comes with Hurd 1.0).

/me hides.

> drive_* would align well with the existing drive_add/del commands.

Agree.

[...]
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index d02ee86..c86796a 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1136,6 +1136,69 @@
>>    'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
>>  
>>  ##
>> +# @blockdev-reopen
>> +#
>> +# Assigns a new image file to a device.
>> +#
>> +# @device: the name of the device for which we are chainging the image file.
>> +#
>> +# @new-image-file: the target of the new image. If the file doesn't exists the
>> +#                  command will fail.
>> +#
>> +# @format: #optional the format of the new image, default is 'qcow2'.
>> +#
>> +# Returns: nothing on success
>> +#          If @device is not a valid block device, DeviceNotFound
>> +#          If @new-image-file can't be opened, OpenFileFailed
>> +#          If @format is invalid, InvalidBlockFormat
>> +#
>> +# Since 1.1
>> +##
>> +
>> +{ 'command': 'blockdev-reopen',
>> +  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }
>
> Same consideration on the name.
>
> Also I think we should immediately mark the command as deprecated (I
> think there is precedence for it, though I can't remember which command
> it was). This is not an interface we'll want to keep long term.

"Deprecated" isn't quite right without a replacement.  We could mark it
"experimental" :)

Patch

diff --git a/block/blkmirror.c b/block/blkmirror.c
index 39927c8..49e3381 100644
--- a/block/blkmirror.c
+++ b/block/blkmirror.c
@@ -81,7 +81,7 @@  static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
         bdrv_delete(m->bs[0]);
         return -ENOMEM;
     }
-    ret = bdrv_open(m->bs[1], filename, flags, NULL);
+    ret = bdrv_open(m->bs[1], filename, flags | BDRV_O_NO_BACKING, NULL);
     if (ret < 0) {
         bdrv_delete(m->bs[0]);
         return ret;
diff --git a/blockdev.c b/blockdev.c
index 2c132a3..1df2542 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -646,9 +646,8 @@  void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
-void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
-                                bool has_format, const char *format,
-                                Error **errp)
+static void change_blockdev_image(const char *device, const char *new_image_file,
+                                  const char *format, bool create, Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv, *old_drv, *proto_drv;
@@ -671,7 +670,7 @@  void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
     old_drv = bs->drv;
     flags = bs->open_flags;
 
-    if (!has_format) {
+    if (!format) {
         format = "qcow2";
     }
 
@@ -681,24 +680,26 @@  void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
         return;
     }
 
-    proto_drv = bdrv_find_protocol(snapshot_file);
+    proto_drv = bdrv_find_protocol(new_image_file);
     if (!proto_drv) {
         error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
         return;
     }
 
-    ret = bdrv_img_create(snapshot_file, format, bs->filename,
-                          bs->drv->format_name, NULL, -1, flags);
-    if (ret) {
-        error_set(errp, QERR_UNDEFINED_ERROR);
-        return;
+    if (create) {
+        ret = bdrv_img_create(new_image_file, format, bs->filename,
+                              bs->drv->format_name, NULL, -1, flags);
+        if (ret) {
+            error_set(errp, QERR_UNDEFINED_ERROR);
+            return;
+        }
     }
 
     bdrv_drain_all();
     bdrv_flush(bs);
 
     bdrv_close(bs);
-    ret = bdrv_open(bs, snapshot_file, flags, drv);
+    ret = bdrv_open(bs, new_image_file, flags, drv);
     /*
      * If reopening the image file we just created fails, fall back
      * and try to re-open the original image. If that fails too, we
@@ -709,11 +710,95 @@  void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
         if (ret != 0) {
             error_set(errp, QERR_OPEN_FILE_FAILED, old_filename);
         } else {
-            error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file);
+            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
         }
     }
 }
 
+void qmp_blockdev_reopen(const char *device, const char *new_image_file,
+                         bool has_format, const char *format, Error **errp)
+{
+    change_blockdev_image(device, new_image_file,
+                          has_format ? format : NULL, false, errp);
+}
+
+void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
+                                bool has_format, const char *format,
+                                Error **errp)
+{
+    change_blockdev_image(device, snapshot_file,
+                          has_format ? format : NULL, true, errp);
+}
+
+void qmp_blockdev_migrate(const char *device, BlockMigrateOp mode,
+                          const char *destination, bool has_new_image_file,
+                          const char *new_image_file, Error **errp)
+{
+    BlockDriverState *bs;
+    BlockDriver *drv;
+    int i, j, escape;
+    char filename[2048];
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+    if (bdrv_in_use(bs)) {
+        error_set(errp, QERR_DEVICE_IN_USE, device);
+        return;
+    }
+
+    if (mode == BLOCK_MIGRATE_OP_MIRROR) {
+        drv = bdrv_find_format("blkmirror");
+        if (!drv) {
+            error_set(errp, QERR_INVALID_BLOCK_FORMAT, "blkmirror");
+            return;
+        }
+
+        if (!has_new_image_file) {
+            new_image_file = bs->filename;
+        }
+
+        pstrcpy(filename, sizeof(filename), "blkmirror:");
+        i = strlen("blkmirror:");
+
+        escape = 0;
+        for (j = 0; j < strlen(new_image_file); j++) {
+ loop:
+            if (!(i < sizeof(filename) - 2)) {
+                error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                          "new-image-file", "shorter filename");
+                return;
+            }
+
+            if (new_image_file[j] == ':' || new_image_file[j] == '\\') {
+                if (!escape) {
+                    filename[i++] = '\\', escape = 1;
+                    goto loop;
+                } else {
+                    escape = 0;
+                }
+            }
+
+            filename[i++] = new_image_file[j];
+        }
+
+        if (i + strlen(destination) + 2 > sizeof(filename)) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                      "destination", "shorter filename");
+            return;
+        }
+
+        filename[i++] = ':';
+        pstrcpy(filename + i, sizeof(filename) - i - 2, destination);
+
+        change_blockdev_image(device, filename, "blkmirror", false, errp);
+    } else if (mode == BLOCK_MIGRATE_OP_STREAM) {
+        error_set(errp, QERR_NOT_SUPPORTED);
+    }
+}
+
 static void eject_device(BlockDriverState *bs, int force, Error **errp)
 {
     if (bdrv_in_use(bs)) {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 573b823..ccb1f62 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -886,6 +886,42 @@  Snapshot device, using snapshot file as target if provided
 ETEXI
 
     {
+        .name       = "blkdev_reopen",
+        .args_type  = "device:B,new-image-file:s?,format:s?",
+        .params     = "device [new-image-file] [format]",
+        .help       = "Assigns a new image file to a device.\n\t\t\t"
+                      "The image will be opened using the format\n\t\t\t"
+                      "specified or 'qcow2' by default.",
+        .mhandler.cmd = hmp_blkdev_reopen,
+    },
+
+STEXI
+@item blkdev_reopen
+@findex blkdev_reopen
+Assigns a new image file to a device.
+ETEXI
+
+    {
+        .name       = "blkdev_migrate",
+        .args_type  = "mirror:-m,device:B,destination:s,new-image-file:s?",
+        .params     = "device mode destination [new-image-file]",
+        .help       = "Migrates the device to a new destination.\n\t\t\t"
+                      "The default migration method is 'stream',\n\t\t\t"
+                      "the option -m is used to select 'mirror'\n\t\t\t"
+                      "(currently the only method supported).\n\t\t\t"
+                      "An optional existing image file that will\n\t\t\t"
+                      "replace the current one in the device\n\t\t\t"
+                      "can be specified.",
+        .mhandler.cmd = hmp_blkdev_migrate,
+    },
+
+STEXI
+@item blkdev_migrate
+@findex blkdev_migrate
+Migrates the device to a new destination.
+ETEXI
+
+    {
         .name       = "drive_add",
         .args_type  = "pci_addr:s,opts:s",
         .params     = "[[<domain>:]<bus>:]<slot>\n"
diff --git a/hmp.c b/hmp.c
index 8ff8c94..b687b8f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -701,6 +701,36 @@  void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &errp);
 }
 
+void hmp_blkdev_reopen(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *filename = qdict_get_str(qdict, "new-image-file");
+    const char *format = qdict_get_try_str(qdict, "format");
+    Error *errp = NULL;
+
+    qmp_blockdev_reopen(device, filename, !!format, format, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
+void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict)
+{
+    bool mirror = qdict_get_try_bool(qdict, "mirror", 0);
+    const char *device = qdict_get_str(qdict, "device");
+    const char *destination = qdict_get_str(qdict, "destination");
+    const char *new_image_file = qdict_get_try_str(qdict, "new-image-file");
+    BlockMigrateOp mode;
+    Error *errp = NULL;
+
+    if (mirror)
+        mode = BLOCK_MIGRATE_OP_MIRROR;
+    else
+        mode = BLOCK_MIGRATE_OP_STREAM;
+
+    qmp_blockdev_migrate(device, mode, destination,
+                         !!new_image_file, new_image_file, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 {
     qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index 18eecbd..f4e802b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -47,6 +47,8 @@  void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
+void hmp_blkdev_reopen(Monitor *mon, const QDict *qdict);
+void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index d02ee86..c86796a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1136,6 +1136,69 @@ 
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
 
 ##
+# @blockdev-reopen
+#
+# Assigns a new image file to a device.
+#
+# @device: the name of the device for which we are chainging the image file.
+#
+# @new-image-file: the target of the new image. If the file doesn't exists the
+#                  command will fail.
+#
+# @format: #optional the format of the new image, default is 'qcow2'.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @new-image-file can't be opened, OpenFileFailed
+#          If @format is invalid, InvalidBlockFormat
+#
+# Since 1.1
+##
+
+{ 'command': 'blockdev-reopen',
+  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }
+
+##
+# @BlockMigrateOp:
+#
+# An enumeration of block migration methods.
+#
+# @mirror: Only mirror future writes to the destination
+#
+# @stream: Stream the image to the destination
+#
+# Since 1.1
+##
+{ 'enum' : 'BlockMigrateOp', 'data': [ 'mirror', 'stream' ] }
+
+
+##
+# @blockdev-migrate
+#
+# Migrates the device to a new destination.
+#
+# @device: the name of the block device to migrate.
+#
+# @mode: the block migration method to be used.
+#
+# @destination: the destination of the block migration.
+#
+# @new-image-file: #optional an existing image file that will replace
+#                  the current one in the device.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @mode is not a valid migration mode, InvalidParameterValue
+#          If @destination can't be opened, OpenFileFailed
+#          If @new-image-file can't be opened, OpenFileFailed
+#
+# Since 1.1
+##
+{ 'command': 'blockdev-migrate',
+  'data': { 'device': 'str', 'mode' : 'BlockMigrateOp',
+            'destination': 'str', '*new-image-file': 'str' } }
+
+##
 # @human-monitor-command:
 #
 # Execute a command on the human monitor and return the output.