| Message ID | 1330083459-13583-2-git-send-email-fsimonce@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
----- 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.
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
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.
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" :)
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.
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(-)