| Message ID | 1367221335-22777-3-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 29.04.2013 um 09:42 hat Stefan Hajnoczi geschrieben: > @block-backup > > Start a point-in-time copy of a block device to a new destination. The > status of ongoing block backup operations can be checked with > query-block-jobs. The operation can be stopped before it has completed using > the block-job-cancel command. > > @device: the name of the device whose writes should be mirrored. > > @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. > > @format: #optional the format of the new destination, default is to > probe if @mode is 'existing', else the format of the source > > @mode: #optional whether and how QEMU should create a new image, default is > 'absolute-paths'. > > @speed: #optional the maximum speed, in bytes per second > > Returns: nothing on success > If @device is not a valid block device, DeviceNotFound > > Since 1.6 > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> drive-backup would probably be a more consistent naming. We would then still have block-backup for a future low-level command that doesn't create everything by itself but takes an existing BlockDriverState (e.g. created by blockdev-add). We should also make it transactionable from the beginning, as we don't have schema introspection yet. This way we allow to assume that if the standalone command exists, the transaction subcommand exists as well. Kevin
On 05/08/2013 06:49 AM, Kevin Wolf wrote: > Am 29.04.2013 um 09:42 hat Stefan Hajnoczi geschrieben: >> @block-backup >> > drive-backup would probably be a more consistent naming. We would then > still have block-backup for a future low-level command that doesn't > create everything by itself but takes an existing BlockDriverState (e.g. > created by blockdev-add). At least it would match why we named a command 'drive-mirror' instead of 'block-mirror'. Hmm, looking at qapi-schema.json, I wonder if we can rename 'BlockdevAction' to 'TransactionAction' as used in the @transaction command. It wouldn't change what is sent over the wire in JSON, and until we have full introspection, there is no visibility into the type name used. Changing the name now would let it be more generic to adding future transaction items that are not blockdev related. > > We should also make it transactionable from the beginning, as we don't > have schema introspection yet. This way we allow to assume that if the > standalone command exists, the transaction subcommand exists as well. Agreed - existence of a command at the same time the command is made transactionable serves as a nice substitute for not having full introspection into the 'BlockdevAction' union type, whereas if we introduce the command now but not transaction support until 1.7, life becomes tougher to know when it can be used where (although I HOPE we have introspection in 1.6).
On 04/29/2013 01:42 AM, Stefan Hajnoczi wrote: > @block-backup > > +++ b/qapi-schema.json > @@ -1715,6 +1715,37 @@ > '*speed': 'int' } } > > ## > +# @block-backup > +# > +# Start a point-in-time copy of a block device to a new destination. The > +# status of ongoing block backup operations can be checked with > +# query-block-jobs. The operation can be stopped before it has completed using > +# the block-job-cancel command. Still might be worth mentioning that 'query-block-jobs' will list it as a job of type 'backup'. > +# > +# @device: the name of the device whose writes should be mirrored. > +# > +# @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. > +# > +# @format: #optional the format of the new destination, default is to > +# probe if @mode is 'existing', else the format of the source > +# > +# @mode: #optional whether and how QEMU should create a new image, default is > +# 'absolute-paths'. > +# > +# @speed: #optional the maximum speed, in bytes per second > +# > +# Returns: nothing on success > +# If @device is not a valid block device, DeviceNotFound > +# > +# Since 1.6 > +## > +{ 'command': 'block-backup', > + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', Hmm - wondering if we should add an enum type for supported disk formats instead of using free-form strings. The wire representation would be the same, and now's the time to do it before we add introspection (it's more than just this command impacted).
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Il 11/05/2013 05:34, Eric Blake ha scritto: > On 05/08/2013 06:49 AM, Kevin Wolf wrote: >> Am 29.04.2013 um 09:42 hat Stefan Hajnoczi geschrieben: >>> @block-backup >>> > >> drive-backup would probably be a more consistent naming. We would >> then still have block-backup for a future low-level command that >> doesn't create everything by itself but takes an existing >> BlockDriverState (e.g. created by blockdev-add). > > At least it would match why we named a command 'drive-mirror' > instead of 'block-mirror'. > > Hmm, looking at qapi-schema.json, I wonder if we can rename > 'BlockdevAction' to 'TransactionAction' as used in the > @transaction command. It wouldn't change what is sent over the > wire in JSON, and until we have full introspection, there is no > visibility into the type name used. Changing the name now would > let it be more generic to adding future transaction items that are > not blockdev related. Right. For example, "cont" could be made transactionable too (and executed only if the transaction succeeds). Paolo >> >> We should also make it transactionable from the beginning, as we >> don't have schema introspection yet. This way we allow to assume >> that if the standalone command exists, the transaction subcommand >> exists as well. > > Agreed - existence of a command at the same time the command is > made transactionable serves as a nice substitute for not having > full introspection into the 'BlockdevAction' union type, whereas if > we introduce the command now but not transaction support until 1.7, > life becomes tougher to know when it can be used where (although I > HOPE we have introspection in 1.6). > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRjfslAAoJEBvWZb6bTYbyiVkQAJpqoUcmTmzY9P8Me7pSlR5p MVogKDdvtFpr+GVWWaiykB4rN79gGjduOGHtMpScuE3Grr42nFSeGnJoKqKP788T 1ZCaEaOt/Il3PeGWJOM7y8RkxnieTOqehPIUODq/qHVKE+mN0+sFlvGI67lhQveI NWJCV2gzN6z7aCJE291BZxU4dtZzJv1SnkGqPZ2z/sEfQZulpVhmleE44SQRgFE1 oaSdfbniz/TRqmB5x8E3444X7YIZ9I+NTZuGlWr6V8tT9C5tnrB3jhMId9TVbQQg 2tDuYdm735kEC7K7byOtYWxJEKsEca/6dV5LbLh1gyoJWWb6/DM/bZ0je4XbtBFY sxbKAV2llDbBRa8yWkp7p+N9THARj00skb3u9rE38+UP9p7aQUW6ZXwsn0cawBec njDjdmnq4JiZQ+ez+wAFfxZmC1kx2Zfxsg/2aws67cf3ySzr14PSe0czgQaW0rgM BP+7W4pd6pGmrnK+ASEK2r2gWniiF1OyngV4Q3v6d7SIAkKU6fPcu5iY/9INveNv JRlMw/GE5/POENCuFhA6CEv/Dg48H6j9u9N44fC5i3KpS8mJaTn83av3MoBvAfqv JAH2ZNHxaK+HoV4oxSm4gbIehlI1gh19Mb08u226EdGYcupTkPvDnOU+GhJFEqoN 0nQLUIysLgHjGJzQtX1/ =ULCr -----END PGP SIGNATURE-----
Am 11.05.2013 um 05:34 hat Eric Blake geschrieben: > On 05/08/2013 06:49 AM, Kevin Wolf wrote: > > Am 29.04.2013 um 09:42 hat Stefan Hajnoczi geschrieben: > >> @block-backup > >> > > > drive-backup would probably be a more consistent naming. We would then > > still have block-backup for a future low-level command that doesn't > > create everything by itself but takes an existing BlockDriverState (e.g. > > created by blockdev-add). > > At least it would match why we named a command 'drive-mirror' instead of > 'block-mirror'. > > Hmm, looking at qapi-schema.json, I wonder if we can rename > 'BlockdevAction' to 'TransactionAction' as used in the @transaction > command. It wouldn't change what is sent over the wire in JSON, and > until we have full introspection, there is no visibility into the type > name used. Changing the name now would let it be more generic to adding > future transaction items that are not blockdev related. Good point, I never realised that once we have schema introspection, doing such changes is harder. I'm all for it - it's bad enough that we have specifically block jobs instead of just background jobs, we shouldn't repeat the same mistake with transactions. Kevin
Am 11.05.2013 um 06:02 hat Eric Blake geschrieben: > On 04/29/2013 01:42 AM, Stefan Hajnoczi wrote: > > @block-backup > > > > +++ b/qapi-schema.json > > @@ -1715,6 +1715,37 @@ > > '*speed': 'int' } } > > > > ## > > +# @block-backup > > +# > > +# Start a point-in-time copy of a block device to a new destination. The > > +# status of ongoing block backup operations can be checked with > > +# query-block-jobs. The operation can be stopped before it has completed using > > +# the block-job-cancel command. > > Still might be worth mentioning that 'query-block-jobs' will list it as > a job of type 'backup'. > > > +# > > +# @device: the name of the device whose writes should be mirrored. > > +# > > +# @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. > > +# > > +# @format: #optional the format of the new destination, default is to > > +# probe if @mode is 'existing', else the format of the source > > +# > > +# @mode: #optional whether and how QEMU should create a new image, default is > > +# 'absolute-paths'. > > +# > > +# @speed: #optional the maximum speed, in bytes per second > > +# > > +# Returns: nothing on success > > +# If @device is not a valid block device, DeviceNotFound > > +# > > +# Since 1.6 > > +## > > +{ 'command': 'block-backup', > > + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > > Hmm - wondering if we should add an enum type for supported disk formats > instead of using free-form strings. The wire representation would be > the same, and now's the time to do it before we add introspection (it's > more than just this command impacted). And ideally we shouldn't make it a static list that contains every format for which qemu has some code, but only those that are actually compiled in. (Hm, and probably not protocols?) Luiz, any idea how to do something like this, a QAPI enum with values that are determined at runtime? Especially with respect to the coming schema introspection? Kevin
On 05/13/2013 02:28 AM, Kevin Wolf wrote: >>> +{ 'command': 'block-backup', >>> + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', >> >> Hmm - wondering if we should add an enum type for supported disk formats >> instead of using free-form strings. The wire representation would be >> the same, and now's the time to do it before we add introspection (it's >> more than just this command impacted). > > And ideally we shouldn't make it a static list that contains every > format for which qemu has some code, but only those that are actually > compiled in. (Hm, and probably not protocols?) > > Luiz, any idea how to do something like this, a QAPI enum with values > that are determined at runtime? Especially with respect to the coming > schema introspection? Or maybe we make the 'enum' list ALL possible types, but then add a query-* command that returns an array of only those enum values that are supported. Introspection would see all types, but the query command would be the useful variant that is runtime-dependent.
Am 13.05.2013 um 14:56 hat Eric Blake geschrieben: > On 05/13/2013 02:28 AM, Kevin Wolf wrote: > > >>> +{ 'command': 'block-backup', > >>> + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > >> > >> Hmm - wondering if we should add an enum type for supported disk formats > >> instead of using free-form strings. The wire representation would be > >> the same, and now's the time to do it before we add introspection (it's > >> more than just this command impacted). > > > > And ideally we shouldn't make it a static list that contains every > > format for which qemu has some code, but only those that are actually > > compiled in. (Hm, and probably not protocols?) > > > > Luiz, any idea how to do something like this, a QAPI enum with values > > that are determined at runtime? Especially with respect to the coming > > schema introspection? > > Or maybe we make the 'enum' list ALL possible types, but then add a > query-* command that returns an array of only those enum values that are > supported. Introspection would see all types, but the query command > would be the useful variant that is runtime-dependent. Then is there any advantage in making it an enum in the first place? Can libvirt really make use of the information "this qemu version could have been compiled with VHDX support, but it hasn't"? Kevin
On Mon, 13 May 2013 15:09:31 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 13.05.2013 um 14:56 hat Eric Blake geschrieben: > > On 05/13/2013 02:28 AM, Kevin Wolf wrote: > > > > >>> +{ 'command': 'block-backup', > > >>> + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > > >> > > >> Hmm - wondering if we should add an enum type for supported disk formats > > >> instead of using free-form strings. The wire representation would be > > >> the same, and now's the time to do it before we add introspection (it's > > >> more than just this command impacted). > > > > > > And ideally we shouldn't make it a static list that contains every > > > format for which qemu has some code, but only those that are actually > > > compiled in. (Hm, and probably not protocols?) > > > > > > Luiz, any idea how to do something like this, a QAPI enum with values > > > that are determined at runtime? Especially with respect to the coming > > > schema introspection? > > > > Or maybe we make the 'enum' list ALL possible types, but then add a > > query-* command that returns an array of only those enum values that are > > supported. Introspection would see all types, but the query command > > would be the useful variant that is runtime-dependent. Agreed. This is a capability, and we're adding query- commands to query capabilities. > Then is there any advantage in making it an enum in the first place? Eric is in a better position to answer this, but the fact that this can be queried isn't a strong pro for having it?
On 05/13/2013 07:18 AM, Luiz Capitulino wrote: >>>> Luiz, any idea how to do something like this, a QAPI enum with values >>>> that are determined at runtime? Especially with respect to the coming >>>> schema introspection? >>> >>> Or maybe we make the 'enum' list ALL possible types, but then add a >>> query-* command that returns an array of only those enum values that are >>> supported. Introspection would see all types, but the query command >>> would be the useful variant that is runtime-dependent. > > Agreed. This is a capability, and we're adding query- commands to query > capabilities. > >> Then is there any advantage in making it an enum in the first place? > > Eric is in a better position to answer this, but the fact that this can > be queried isn't a strong pro for having it? Hmm, you raise an interesting point - if we have a query-block-formats command that returns an array of strings, then keep 'str' everywhere else a format is required, that is no different for what is sent over the wire compared to a query-block-formats that returns an array of 'BlockFormat' enum values, with the enum showing all possible formats (even if support wasn't compiled in), and with 'BlockFormat' everywhere else. Introspection-wise, you'd have to know that you call query-block-formats instead of introspecting on the type of the format argument, but information-wise, there's no loss of details if the query- command provides the runtime list, and the remaining commands stick with 'str'. I still think an enum is a bit nicer from the type-safety aspect, but I'm finding it hard to envision any scenario where libvirt would have to resort to introspection if we have a query-block-formats in place, and thus am not opposed to your idea of avoiding the enum.
Am 13.05.2013 um 16:14 hat Eric Blake geschrieben: > On 05/13/2013 07:18 AM, Luiz Capitulino wrote: > >>>> Luiz, any idea how to do something like this, a QAPI enum with values > >>>> that are determined at runtime? Especially with respect to the coming > >>>> schema introspection? > >>> > >>> Or maybe we make the 'enum' list ALL possible types, but then add a > >>> query-* command that returns an array of only those enum values that are > >>> supported. Introspection would see all types, but the query command > >>> would be the useful variant that is runtime-dependent. > > > > Agreed. This is a capability, and we're adding query- commands to query > > capabilities. > > > >> Then is there any advantage in making it an enum in the first place? > > > > Eric is in a better position to answer this, but the fact that this can > > be queried isn't a strong pro for having it? > > Hmm, you raise an interesting point - if we have a query-block-formats > command that returns an array of strings, then keep 'str' everywhere > else a format is required, that is no different for what is sent over > the wire compared to a query-block-formats that returns an array of > 'BlockFormat' enum values, with the enum showing all possible formats > (even if support wasn't compiled in), and with 'BlockFormat' everywhere > else. Introspection-wise, you'd have to know that you call > query-block-formats instead of introspecting on the type of the format > argument, but information-wise, there's no loss of details if the query- > command provides the runtime list, and the remaining commands stick with > 'str'. I still think an enum is a bit nicer from the type-safety > aspect, but I'm finding it hard to envision any scenario where libvirt > would have to resort to introspection if we have a query-block-formats > in place, and thus am not opposed to your idea of avoiding the enum. I think long term we'll need a dynamic schema anyway. As we go forward with modularisation and putting things into shared libraries, we'll have modules that add support for commands, enum values, etc. Providing the full list of theoretically available elements (i.e. what would be there, if everything was compiled and all modules were loaded) would probably implement the spec for the introspection interfaces by the letter, but just give useless information. Callers want to know what's really there. If we're going to have a query-* command for everything, then we don't need introspection at all. I would however prefer having the uniform schema introspection and building the schema at runtime instead of many separate query-* commands. Kevin
On 05/13/2013 08:27 AM, Kevin Wolf wrote: > I think long term we'll need a dynamic schema anyway. As we go forward > with modularisation and putting things into shared libraries, we'll have > modules that add support for commands, enum values, etc. In other words, qapi-schema.json should have a way to declare a dynamic-enum, where introspection on that enum will see what is made available at runtime, rather than manually listing the enum contents directly in the .json file. > > Providing the full list of theoretically available elements (i.e. what > would be there, if everything was compiled and all modules were loaded) > would probably implement the spec for the introspection interfaces by the > letter, but just give useless information. Callers want to know what's > really there. > > If we're going to have a query-* command for everything, then we don't > need introspection at all. I would however prefer having the uniform > schema introspection and building the schema at runtime instead of many > separate query-* commands. Indeed, having introspection of a dynamic enum results in fewer commands overall, by making a reusable command have more power. And maybe it's possible to have introspection do both - with an optional boolean parameter that distinguishes between full vs. runtime querying of any enum type.
δΊ 2013-5-13 22:50, Eric Blake ει: > On 05/13/2013 08:27 AM, Kevin Wolf wrote: >> I think long term we'll need a dynamic schema anyway. As we go forward >> with modularisation and putting things into shared libraries, we'll have >> modules that add support for commands, enum values, etc. > > In other words, qapi-schema.json should have a way to declare a > dynamic-enum, where introspection on that enum will see what is made > available at runtime, rather than manually listing the enum contents > directly in the .json file. > >> >> Providing the full list of theoretically available elements (i.e. what >> would be there, if everything was compiled and all modules were loaded) >> would probably implement the spec for the introspection interfaces by the >> letter, but just give useless information. Callers want to know what's >> really there. >> >> If we're going to have a query-* command for everything, then we don't >> need introspection at all. I would however prefer having the uniform >> schema introspection and building the schema at runtime instead of many >> separate query-* commands. > > Indeed, having introspection of a dynamic enum results in fewer commands > overall, by making a reusable command have more power. And maybe it's > possible to have introspection do both - with an optional boolean > parameter that distinguishes between full vs. runtime querying of any > enum type. > "string <-> enum mapping" issue happens when I try coding libqblock, what I found is that, since lower level implement code still depends string, so there will be another translating back to string inside later. For format string, it is probably OK since two functions will be enough, but when more enum is wanted, things become complex. So I feel it is right to try use enum in the API now, but maybe a plan to reorganize block layer will be also needed, which may be a bit complex needing serous plan.
On Wed, May 08, 2013 at 02:49:00PM +0200, Kevin Wolf wrote: > Am 29.04.2013 um 09:42 hat Stefan Hajnoczi geschrieben: > > @block-backup > > > > Start a point-in-time copy of a block device to a new destination. The > > status of ongoing block backup operations can be checked with > > query-block-jobs. The operation can be stopped before it has completed using > > the block-job-cancel command. > > > > @device: the name of the device whose writes should be mirrored. > > > > @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. > > > > @format: #optional the format of the new destination, default is to > > probe if @mode is 'existing', else the format of the source > > > > @mode: #optional whether and how QEMU should create a new image, default is > > 'absolute-paths'. > > > > @speed: #optional the maximum speed, in bytes per second > > > > Returns: nothing on success > > If @device is not a valid block device, DeviceNotFound > > > > Since 1.6 > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > drive-backup would probably be a more consistent naming. We would then > still have block-backup for a future low-level command that doesn't > create everything by itself but takes an existing BlockDriverState (e.g. > created by blockdev-add). > > We should also make it transactionable from the beginning, as we don't > have schema introspection yet. This way we allow to assume that if the > standalone command exists, the transaction subcommand exists as well. Sounds good. I'll make both changes. Stefan
On Fri, May 10, 2013 at 10:02:14PM -0600, Eric Blake wrote: > On 04/29/2013 01:42 AM, Stefan Hajnoczi wrote: > > @block-backup > > > > +++ b/qapi-schema.json > > @@ -1715,6 +1715,37 @@ > > '*speed': 'int' } } > > > > ## > > +# @block-backup > > +# > > +# Start a point-in-time copy of a block device to a new destination. The > > +# status of ongoing block backup operations can be checked with > > +# query-block-jobs. The operation can be stopped before it has completed using > > +# the block-job-cancel command. > > Still might be worth mentioning that 'query-block-jobs' will list it as > a job of type 'backup'. Will fix in v3. > > +# > > +# @device: the name of the device whose writes should be mirrored. > > +# > > +# @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. > > +# > > +# @format: #optional the format of the new destination, default is to > > +# probe if @mode is 'existing', else the format of the source > > +# > > +# @mode: #optional whether and how QEMU should create a new image, default is > > +# 'absolute-paths'. > > +# > > +# @speed: #optional the maximum speed, in bytes per second > > +# > > +# Returns: nothing on success > > +# If @device is not a valid block device, DeviceNotFound > > +# > > +# Since 1.6 > > +## > > +{ 'command': 'block-backup', > > + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > > Hmm - wondering if we should add an enum type for supported disk formats > instead of using free-form strings. The wire representation would be > the same, and now's the time to do it before we add introspection (it's > more than just this command impacted). Interesting discussion about dynamic schema. Since the wire format is the same, I don't want to attempt solving the problem in the block-backup series.
diff --git a/blockdev.c b/blockdev.c index 6e293e9..afb9b4a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1279,6 +1279,98 @@ void qmp_block_commit(const char *device, drive_get_ref(drive_get_by_blockdev(bs)); } +void qmp_block_backup(const char *device, const char *target, + bool has_format, const char *format, + bool has_mode, enum NewImageMode mode, + bool has_speed, int64_t speed, + Error **errp) +{ + BlockDriverState *bs; + BlockDriverState *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; + + proto_drv = bdrv_find_protocol(target); + if (!proto_drv) { + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); + return; + } + + bdrv_get_geometry(bs, &size); + size *= 512; + if (mode != NEW_IMAGE_MODE_EXISTING) { + assert(format && drv); + bdrv_img_create(target, format, + NULL, NULL, NULL, size, flags, &local_err, false); + } + + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + return; + } + + target_bs = bdrv_new(""); + ret = bdrv_open(target_bs, target, NULL, flags, drv); + + if (ret < 0) { + bdrv_delete(target_bs); + error_set(errp, QERR_OPEN_FILE_FAILED, target); + return; + } + + backup_start(bs, target_bs, speed, block_job_cb, bs, &local_err); + if (local_err != NULL) { + bdrv_delete(target_bs); + error_propagate(errp, local_err); + return; + } + + /* Grab a reference so hotplug does not delete the BlockDriverState from + * underneath us. + */ + drive_get_ref(drive_get_by_blockdev(bs)); +} + #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) void qmp_drive_mirror(const char *device, const char *target, diff --git a/qapi-schema.json b/qapi-schema.json index 5b0fb3b..550ccac 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1715,6 +1715,37 @@ '*speed': 'int' } } ## +# @block-backup +# +# Start a point-in-time copy of a block device to a new destination. The +# status of ongoing block backup operations can be checked with +# query-block-jobs. The operation can be stopped before it has completed using +# the block-job-cancel command. +# +# @device: the name of the device whose writes should be mirrored. +# +# @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. +# +# @format: #optional the format of the new destination, default is to +# probe if @mode is 'existing', else the format of the source +# +# @mode: #optional whether and how QEMU should create a new image, default is +# 'absolute-paths'. +# +# @speed: #optional the maximum speed, in bytes per second +# +# Returns: nothing on success +# If @device is not a valid block device, DeviceNotFound +# +# Since 1.6 +## +{ 'command': 'block-backup', + 'data': { 'device': 'str', 'target': 'str', '*format': 'str', + '*mode': 'NewImageMode', '*speed': 'int' } } + +## # @drive-mirror # # Start mirroring a block device's writes to a new destination. diff --git a/qmp-commands.hx b/qmp-commands.hx index 0e89132..f9fb926 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -889,6 +889,12 @@ EQMP }, { + .name = "block-backup", + .args_type = "device:B,target:s,speed:i?,mode:s?,format:s?", + .mhandler.cmd_new = qmp_marshal_input_block_backup, + }, + + { .name = "block-job-set-speed", .args_type = "device:B,speed:o", .mhandler.cmd_new = qmp_marshal_input_block_job_set_speed,
@block-backup Start a point-in-time copy of a block device to a new destination. The status of ongoing block backup operations can be checked with query-block-jobs. The operation can be stopped before it has completed using the block-job-cancel command. @device: the name of the device whose writes should be mirrored. @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. @format: #optional the format of the new destination, default is to probe if @mode is 'existing', else the format of the source @mode: #optional whether and how QEMU should create a new image, default is 'absolute-paths'. @speed: #optional the maximum speed, in bytes per second Returns: nothing on success If @device is not a valid block device, DeviceNotFound Since 1.6 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- blockdev.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 31 +++++++++++++++++++ qmp-commands.hx | 6 ++++ 3 files changed, 129 insertions(+)