| Message ID | fc8559ce754d2679924881b76d1a6993041bcdf4.1459776815.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04.04.2016 15:43, Alberto Garcia wrote: > Currently, block jobs can only be owned by root nodes. This patch > allows block jobs to be in any arbitrary node, by making the following > changes: > > - Block jobs can now be identified by the node name of their > BlockDriverState in addition to the device name. Since both device > and node names live in the same namespace there's no ambiguity. > > - The "device" parameter used by all commands that operate on block > jobs can also be a node name now. > > - The node name is used as a fallback to fill in the BlockJobInfo > structure and all BLOCK_JOB_* events if there is no device name for > that job. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > blockdev.c | 18 ++++++++++-------- > blockjob.c | 5 +++-- > docs/qmp-events.txt | 8 ++++---- > qapi/block-core.json | 20 ++++++++++---------- > 4 files changed, 27 insertions(+), 24 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index edbcc19..d1f5dfb 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3685,8 +3685,10 @@ void qmp_blockdev_mirror(const char *device, const char *target, > aio_context_release(aio_context); > } > > -/* Get the block job for a given device name and acquire its AioContext */ > -static BlockJob *find_block_job(const char *device, AioContext **aio_context, > +/* Get the block job for a given device or node name > + * and acquire its AioContext */ > +static BlockJob *find_block_job(const char *device_or_node, > + AioContext **aio_context, > Error **errp) > { > BlockBackend *blk; > @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, > > *aio_context = NULL; > > - blk = blk_by_name(device); > - if (!blk) { > + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); > + if (!bs) { If we get here, *errp is already set... [1] > goto notfound; > } > > - *aio_context = blk_get_aio_context(blk); > + *aio_context = bdrv_get_aio_context(bs); > aio_context_acquire(*aio_context); > > - if (!blk_is_available(blk)) { > + blk = blk_by_name(device_or_node); > + if (blk && !blk_is_available(blk)) { I'd just drop this. The reason blk_is_available() was added here because it became possible for BBs not to have a BDS. Now that you get the BDS directly through bdrv_lookup_bs(), it's no longer necessary. > goto notfound; > } > - bs = blk_bs(blk); > > if (!bs->job) { > goto notfound; > @@ -3715,7 +3717,7 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, > > notfound: > error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, > - "No active block job on device '%s'", device); > + "No active block job on node '%s'", device_or_node); [1] ...and then we'll get a failed assertion here (through error_setv()). > if (*aio_context) { > aio_context_release(*aio_context); > *aio_context = NULL; > diff --git a/blockjob.c b/blockjob.c > index 3557048..2ab4794 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -67,7 +67,8 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, > BlockJob *job; > > if (bs->job) { > - error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); > + error_setg(errp, "Node '%s' is in use", > + bdrv_get_device_or_node_name(bs)); > return NULL; > } > bdrv_ref(bs); > @@ -78,7 +79,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, > bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); > > job->driver = driver; > - job->id = g_strdup(bdrv_get_device_name(bs)); > + job->id = g_strdup(bdrv_get_device_or_node_name(bs)); Hm, since this is only used for events, it's not too bad that a block job will have a different name if it was created on a root BDS by specifying its node name. But it still is kind of strange. But as I said, it should be fine as long as one can still use the node name for controlling it (which is the case). Max
On Wed 27 Apr 2016 02:30:55 PM CEST, Max Reitz wrote: >> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, >> >> *aio_context = NULL; >> >> - blk = blk_by_name(device); >> - if (!blk) { >> + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); >> + if (!bs) { > > If we get here, *errp is already set... [1] Good catch, thanks. >> - if (!blk_is_available(blk)) { >> + blk = blk_by_name(device_or_node); >> + if (blk && !blk_is_available(blk)) { > > I'd just drop this. The reason blk_is_available() was added here > because it became possible for BBs not to have a BDS. > > Now that you get the BDS directly through bdrv_lookup_bs(), it's no > longer necessary. Ok. >> @@ -78,7 +79,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, >> bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); >> >> job->driver = driver; >> - job->id = g_strdup(bdrv_get_device_name(bs)); >> + job->id = g_strdup(bdrv_get_device_or_node_name(bs)); > > Hm, since this is only used for events, it's not too bad that a block > job will have a different name if it was created on a root BDS by > specifying its node name. But it still is kind of strange. The idea is that if you create a block job on a root BDS (which is still the most common case) you get the same id that you used to get before this series. Berto
Am 04.04.2016 um 15:43 hat Alberto Garcia geschrieben: > Currently, block jobs can only be owned by root nodes. This patch > allows block jobs to be in any arbitrary node, by making the following > changes: > > - Block jobs can now be identified by the node name of their > BlockDriverState in addition to the device name. Since both device > and node names live in the same namespace there's no ambiguity. Careful, we know this is a part of our API that we have already messed up and we don't want to make things worse while adding new things before we've cleaned it up. Let's keep in mind where we are planning to go with block jobs: They should become background jobs, not tied to any block device. The close connection to a single BDS already doesn't make a lot of sense today because most block jobs involve at least two BDSes. In the final state, we will have a job ID that uniquely identifies the job, and each command that starts a job will take an ID from the client. For compatibility, we'll use the block device name as the job ID when using old commands that don't get an explicit ID yet. In the existing qemu version, you can't start two block jobs on the same device, and in future versions, you're supposed to specify an ID each time. This is why the default can always be supposed to work without conflicts. If in newer versions, the client mixes both ways (which it shouldn't do), starting a new block job may fail because the device name is already in use as an ID for another job. Now we can probably make the same argument for node names, so we can extend the interface and still keep it compatible. Where we need to be careful is that with device names and node names, we have potentially two names describing the same BDS and therefore the same job. This must not happen, because we won't be able to represent that in the generic background job API. Any job has just a single ID there. > - The "device" parameter used by all commands that operate on block > jobs can also be a node name now. So I think the least we need to do is change this one to resolve the block job by its ID (job->id) rather than device or node names. > - The node name is used as a fallback to fill in the BlockJobInfo > structure and all BLOCK_JOB_* events if there is no device name for > that job. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > blockdev.c | 18 ++++++++++-------- > blockjob.c | 5 +++-- > docs/qmp-events.txt | 8 ++++---- > qapi/block-core.json | 20 ++++++++++---------- > 4 files changed, 27 insertions(+), 24 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index edbcc19..d1f5dfb 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3685,8 +3685,10 @@ void qmp_blockdev_mirror(const char *device, const char *target, > aio_context_release(aio_context); > } > > -/* Get the block job for a given device name and acquire its AioContext */ > -static BlockJob *find_block_job(const char *device, AioContext **aio_context, > +/* Get the block job for a given device or node name > + * and acquire its AioContext */ > +static BlockJob *find_block_job(const char *device_or_node, > + AioContext **aio_context, > Error **errp) > { > BlockBackend *blk; > @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, > > *aio_context = NULL; > > - blk = blk_by_name(device); > - if (!blk) { > + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); Specifically, this one is bad. It allows two different ways to specify the same job. The other thing about this patch is that I'm not sure how badly it conflicts with my series to convert block jobs to BlockBackend. It seems that you switch everything from blk to bs here, and I'll have to switch back to blk (once job->blk exists). Kevin
On 04/04/2016 07:43 AM, Alberto Garcia wrote: > Currently, block jobs can only be owned by root nodes. This patch > allows block jobs to be in any arbitrary node, by making the following > changes: > > - Block jobs can now be identified by the node name of their > BlockDriverState in addition to the device name. Since both device > and node names live in the same namespace there's no ambiguity. > > - The "device" parameter used by all commands that operate on block > jobs can also be a node name now. > > - The node name is used as a fallback to fill in the BlockJobInfo > structure and all BLOCK_JOB_* events if there is no device name for > that job. Having more than one way to name a job might be okay for convenience, but only if the canonical name is unambiguous. I agree with Kevin's concern that using the device and/or node name as the canonical name of the job is worrisome, because it locks us into having only one job with that name at a time. > +++ b/docs/qmp-events.txt > @@ -92,7 +92,7 @@ Data: > > - "type": Job type (json-string; "stream" for image streaming > "commit" for block commit) > -- "device": Device name (json-string) > +- "device": Device name, or node name if not present (json-string) On the surface this sounds okay (you are promising to return a canonical name, insofar as job canonical names are currently the node/device name until the time that we allow job ids with multiple jobs per device) - even if it means that you return a device name when the user started a job based on a node name. But I'm also worried about jobs where the node name tied to a device changes over time (creating a snapshot, pivoting to a mirror, doing an active commit with a pivot to the backing file - all of these are cases where the device name stays the same, but the top node name associated with the device differs over time). If the device name is the canonical one, then a job started on node "A" but reported as device "D", needs to STILL be known as job "D" even if by the end of the job node "A" is no longer associated with device "D" (because "D" is now serving the top-level node "B"). > @@ -1513,7 +1513,7 @@ > # the operation is actually paused. Cancelling a paused job automatically > # resumes it. > # > -# @device: the device name > +# @device: the device or node name of the owner of the block job. We aren't consistent on whether to use a trailing '.'. I don't care enough to standardize on a style, so no need to respin for just that.
On Fri 29 Apr 2016 05:00:57 PM CEST, Kevin Wolf wrote: >> - Block jobs can now be identified by the node name of their >> BlockDriverState in addition to the device name. Since both device >> and node names live in the same namespace there's no ambiguity. > > Careful, we know this is a part of our API that we have already messed > up and we don't want to make things worse while adding new things > before we've cleaned it up. > > Let's keep in mind where we are planning to go with block jobs: They > should become background jobs, not tied to any block device. The close > connection to a single BDS already doesn't make a lot of sense today > because most block jobs involve at least two BDSes. > > In the final state, we will have a job ID that uniquely identifies the > job, and each command that starts a job will take an ID from the > client. For compatibility, we'll use the block device name as the job > ID when using old commands that don't get an explicit ID yet. > > In the existing qemu version, you can't start two block jobs on the > same device, and in future versions, you're supposed to specify an ID > each time. This is why the default can always be supposed to work > without conflicts. If in newer versions, the client mixes both ways > (which it shouldn't do), starting a new block job may fail because the > device name is already in use as an ID for another job. > > Now we can probably make the same argument for node names, so we can > extend the interface and still keep it compatible. > > Where we need to be careful is that with device names and node names, > we have potentially two names describing the same BDS and therefore > the same job. This must not happen, because we won't be able to > represent that in the generic background job API. Any job has just a > single ID there. Ok, what can be done in this case is to keep the name that the client used when the job was created. block-stream virti0 <-- job id is 'virtio0' block-stream node5 <-- job id is 'node5' In this case it wouldn't matter if 'node5' is the one attached to 'virtio0'. >> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, >> >> *aio_context = NULL; >> >> - blk = blk_by_name(device); >> - if (!blk) { >> + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); > > Specifically, this one is bad. It allows two different ways to specify > the same job. I think we can simply iterate over all block jobs (now that we have a function to do that) and return the one with the matching ID. Berto
On 05/06/2016 06:00 AM, Alberto Garcia wrote: > On Fri 29 Apr 2016 05:00:57 PM CEST, Kevin Wolf wrote: > >>> - Block jobs can now be identified by the node name of their >>> BlockDriverState in addition to the device name. Since both device >>> and node names live in the same namespace there's no ambiguity. >> >> Careful, we know this is a part of our API that we have already messed >> up and we don't want to make things worse while adding new things >> before we've cleaned it up. >> >> Let's keep in mind where we are planning to go with block jobs: They >> should become background jobs, not tied to any block device. The close >> connection to a single BDS already doesn't make a lot of sense today >> because most block jobs involve at least two BDSes. >> >> In the final state, we will have a job ID that uniquely identifies the >> job, and each command that starts a job will take an ID from the >> client. For compatibility, we'll use the block device name as the job >> ID when using old commands that don't get an explicit ID yet. >> >> In the existing qemu version, you can't start two block jobs on the >> same device, and in future versions, you're supposed to specify an ID >> each time. This is why the default can always be supposed to work >> without conflicts. If in newer versions, the client mixes both ways >> (which it shouldn't do), starting a new block job may fail because the >> device name is already in use as an ID for another job. >> >> Now we can probably make the same argument for node names, so we can >> extend the interface and still keep it compatible. >> >> Where we need to be careful is that with device names and node names, >> we have potentially two names describing the same BDS and therefore >> the same job. This must not happen, because we won't be able to >> represent that in the generic background job API. Any job has just a >> single ID there. > > Ok, what can be done in this case is to keep the name that the client > used when the job was created. > > block-stream virti0 <-- job id is 'virtio0' > block-stream node5 <-- job id is 'node5' > > In this case it wouldn't matter if 'node5' is the one attached to > 'virtio0'. > >>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, >>> >>> *aio_context = NULL; >>> >>> - blk = blk_by_name(device); >>> - if (!blk) { >>> + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); >> >> Specifically, this one is bad. It allows two different ways to specify >> the same job. > > I think we can simply iterate over all block jobs (now that we have a > function to do that) and return the one with the matching ID. > > Berto > I think it's time to add a proper ID field to Block Jobs. Currently, we just use the node-name as the ID, but as you are aware this is a poor mechanism for fetching the job. I'd really like to avoid continue using any kind of node-name or block/device-name for job IDs, and instead start using either a user-provided value or a QEMU auto-generated one. Then, for legacy support, we'd have an "id" field (that's the new proper globally unique ID) and an old legacy "node" field or similar. Then, for events/etc we can report two things back: (1) Legacy: the name of the node we were created under. This is like it works currently, and it should keep libvirt happy. (2) New: the proper, real ID that all management utilities should begin using in the future. I've got some patches that work towards this angle, but they're currently intermingled with a bunch of experimental job re-factoring stuff. If you give me a few days I can submit a proposal series to re-do the job ID situation. --js
Am 06.05.2016 um 19:54 hat John Snow geschrieben: > > > On 05/06/2016 06:00 AM, Alberto Garcia wrote: > > On Fri 29 Apr 2016 05:00:57 PM CEST, Kevin Wolf wrote: > > > >>> - Block jobs can now be identified by the node name of their > >>> BlockDriverState in addition to the device name. Since both device > >>> and node names live in the same namespace there's no ambiguity. > >> > >> Careful, we know this is a part of our API that we have already messed > >> up and we don't want to make things worse while adding new things > >> before we've cleaned it up. > >> > >> Let's keep in mind where we are planning to go with block jobs: They > >> should become background jobs, not tied to any block device. The close > >> connection to a single BDS already doesn't make a lot of sense today > >> because most block jobs involve at least two BDSes. > >> > >> In the final state, we will have a job ID that uniquely identifies the > >> job, and each command that starts a job will take an ID from the > >> client. For compatibility, we'll use the block device name as the job > >> ID when using old commands that don't get an explicit ID yet. > >> > >> In the existing qemu version, you can't start two block jobs on the > >> same device, and in future versions, you're supposed to specify an ID > >> each time. This is why the default can always be supposed to work > >> without conflicts. If in newer versions, the client mixes both ways > >> (which it shouldn't do), starting a new block job may fail because the > >> device name is already in use as an ID for another job. > >> > >> Now we can probably make the same argument for node names, so we can > >> extend the interface and still keep it compatible. > >> > >> Where we need to be careful is that with device names and node names, > >> we have potentially two names describing the same BDS and therefore > >> the same job. This must not happen, because we won't be able to > >> represent that in the generic background job API. Any job has just a > >> single ID there. > > > > Ok, what can be done in this case is to keep the name that the client > > used when the job was created. > > > > block-stream virti0 <-- job id is 'virtio0' > > block-stream node5 <-- job id is 'node5' > > > > In this case it wouldn't matter if 'node5' is the one attached to > > 'virtio0'. > > > >>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, > >>> > >>> *aio_context = NULL; > >>> > >>> - blk = blk_by_name(device); > >>> - if (!blk) { > >>> + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); > >> > >> Specifically, this one is bad. It allows two different ways to specify > >> the same job. > > > > I think we can simply iterate over all block jobs (now that we have a > > function to do that) and return the one with the matching ID. > > > > Berto > > > > I think it's time to add a proper ID field to Block Jobs. Currently, we > just use the node-name as the ID, but as you are aware this is a poor > mechanism for fetching the job. > > I'd really like to avoid continue using any kind of node-name or > block/device-name for job IDs, and instead start using either a > user-provided value or a QEMU auto-generated one. We already have a job->id, we just need to start using it not only for events, but also for finding block jobs. The existing auto-generation mechanism is a very simple one (just copy the device name), but it's effective and produces unique IDs in our current environment where only a single block job per device is allowed. > Then, for legacy support, we'd have an "id" field (that's the new proper > globally unique ID) and an old legacy "node" field or similar. > > Then, for events/etc we can report two things back: > > (1) Legacy: the name of the node we were created under. This is like it > works currently, and it should keep libvirt happy. > (2) New: the proper, real ID that all management utilities should begin > using in the future. I don't think that's actually necessary, legacy and new can be the same thing. What would remain from the legacy model is the field name "device" both in commands and returned objects, which would then be a misnomer. But I'm not sure if a bad name is a good reason for duplicating a field. > I've got some patches that work towards this angle, but they're > currently intermingled with a bunch of experimental job re-factoring > stuff. If you give me a few days I can submit a proposal series to re-do > the job ID situation. Sounds good. Kevin
On Fri 06 May 2016 07:54:36 PM CEST, John Snow wrote: >>>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, >>>> >>>> *aio_context = NULL; >>>> >>>> - blk = blk_by_name(device); >>>> - if (!blk) { >>>> + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); >>> >>> Specifically, this one is bad. It allows two different ways to specify >>> the same job. >> >> I think we can simply iterate over all block jobs (now that we have a >> function to do that) and return the one with the matching ID. > > I think it's time to add a proper ID field to Block Jobs. Currently, > we just use the node-name as the ID, but as you are aware this is a > poor mechanism for fetching the job. > > I'd really like to avoid continue using any kind of node-name or > block/device-name for job IDs, and instead start using either a > user-provided value or a QEMU auto-generated one. > > Then, for legacy support, we'd have an "id" field (that's the new > proper globally unique ID) and an old legacy "node" field or similar. > > Then, for events/etc we can report two things back: > > (1) Legacy: the name of the node we were created under. This is like > it works currently, and it should keep libvirt happy. > (2) New: the proper, real ID that all management utilities should > begin using in the future. Do we really need to have those two? Isn't it possible to do it with a single ID?: - New commands that create block jobs have a parameter to specify the job ID (if we don't choose to generate them automatically). - Existing commands (block-stream, block-commit, etc) can also have a new (optional) parameter to set the job ID. If omitted, they use the device name (exactly as they do now). Berto
diff --git a/blockdev.c b/blockdev.c index edbcc19..d1f5dfb 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3685,8 +3685,10 @@ void qmp_blockdev_mirror(const char *device, const char *target, aio_context_release(aio_context); } -/* Get the block job for a given device name and acquire its AioContext */ -static BlockJob *find_block_job(const char *device, AioContext **aio_context, +/* Get the block job for a given device or node name + * and acquire its AioContext */ +static BlockJob *find_block_job(const char *device_or_node, + AioContext **aio_context, Error **errp) { BlockBackend *blk; @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, *aio_context = NULL; - blk = blk_by_name(device); - if (!blk) { + bs = bdrv_lookup_bs(device_or_node, device_or_node, errp); + if (!bs) { goto notfound; } - *aio_context = blk_get_aio_context(blk); + *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(*aio_context); - if (!blk_is_available(blk)) { + blk = blk_by_name(device_or_node); + if (blk && !blk_is_available(blk)) { goto notfound; } - bs = blk_bs(blk); if (!bs->job) { goto notfound; @@ -3715,7 +3717,7 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context, notfound: error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, - "No active block job on device '%s'", device); + "No active block job on node '%s'", device_or_node); if (*aio_context) { aio_context_release(*aio_context); *aio_context = NULL; diff --git a/blockjob.c b/blockjob.c index 3557048..2ab4794 100644 --- a/blockjob.c +++ b/blockjob.c @@ -67,7 +67,8 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, BlockJob *job; if (bs->job) { - error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); + error_setg(errp, "Node '%s' is in use", + bdrv_get_device_or_node_name(bs)); return NULL; } bdrv_ref(bs); @@ -78,7 +79,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); job->driver = driver; - job->id = g_strdup(bdrv_get_device_name(bs)); + job->id = g_strdup(bdrv_get_device_or_node_name(bs)); job->bs = bs; job->cb = cb; job->opaque = opaque; diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt index fa7574d..2d238c5 100644 --- a/docs/qmp-events.txt +++ b/docs/qmp-events.txt @@ -92,7 +92,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) -- "device": Device name (json-string) +- "device": Device name, or node name if not present (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int) On success this is equal to len. @@ -116,7 +116,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) -- "device": Device name (json-string) +- "device": Device name, or node name if not present (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int) On success this is equal to len. @@ -143,7 +143,7 @@ Emitted when a block job encounters an error. Data: -- "device": device name (json-string) +- "device": device name, or node name if not present (json-string) - "operation": I/O operation (json-string, "read" or "write") - "action": action that has been taken, it's one of the following (json-string): "ignore": error has been ignored, the job may fail later @@ -167,7 +167,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) -- "device": Device name (json-string) +- "device": Device name, or node name if not present (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int) On success this is equal to len. diff --git a/qapi/block-core.json b/qapi/block-core.json index 1d09079..59107ed 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -713,7 +713,7 @@ # # @type: the job type ('stream' for image streaming) # -# @device: the block device name +# @device: the block device name, or node name if not present # # @len: the maximum progress value # @@ -1456,7 +1456,7 @@ # # Throttling can be disabled by setting the speed to 0. # -# @device: the device name +# @device: the device or node name of the owner of the block job. # # @speed: the maximum speed, in bytes per second, or 0 for unlimited. # Defaults to 0. @@ -1487,7 +1487,7 @@ # operation can be started at a later time to finish copying all data from the # backing file. # -# @device: the device name +# @device: the device or node name of the owner of the block job. # # @force: #optional whether to allow cancellation of a paused job (default # false). Since 1.3. @@ -1513,7 +1513,7 @@ # the operation is actually paused. Cancelling a paused job automatically # resumes it. # -# @device: the device name +# @device: the device or node name of the owner of the block job. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive @@ -1533,7 +1533,7 @@ # # This command also clears the error status of the job. # -# @device: the device name +# @device: the device or node name of the owner of the block job. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive @@ -1559,7 +1559,7 @@ # # A cancelled or paused job cannot be completed. # -# @device: the device name +# @device: the device or node name of the owner of the block job. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive @@ -2404,7 +2404,7 @@ # # @type: job type # -# @device: device name +# @device: device name, or node name if not present # # @len: maximum progress value # @@ -2435,7 +2435,7 @@ # # @type: job type # -# @device: device name +# @device: device name, or node name if not present # # @len: maximum progress value # @@ -2458,7 +2458,7 @@ # # Emitted when a block job encounters an error # -# @device: device name +# @device: device name, or node name if not present # # @operation: I/O operation # @@ -2478,7 +2478,7 @@ # # @type: job type # -# @device: device name +# @device: device name, or node name if not present # # @len: maximum progress value #
Currently, block jobs can only be owned by root nodes. This patch allows block jobs to be in any arbitrary node, by making the following changes: - Block jobs can now be identified by the node name of their BlockDriverState in addition to the device name. Since both device and node names live in the same namespace there's no ambiguity. - The "device" parameter used by all commands that operate on block jobs can also be a node name now. - The node name is used as a fallback to fill in the BlockJobInfo structure and all BLOCK_JOB_* events if there is no device name for that job. Signed-off-by: Alberto Garcia <berto@igalia.com> --- blockdev.c | 18 ++++++++++-------- blockjob.c | 5 +++-- docs/qmp-events.txt | 8 ++++---- qapi/block-core.json | 20 ++++++++++---------- 4 files changed, 27 insertions(+), 24 deletions(-)