Re: [Qemu-devel] [PATCH 1/3] block: Support streaming to an intermediate layer
(Ccing Markus and Jeff as suggested) On Thu, Mar 05, 2015 at 03:04:25PM +0100, Kevin Wolf wrote: My question is whether we can't simply call stream_start() with an intermediate node as bs instead of introducing a new parameter. I'm not completely sure about the consequences of 3., i.e. moving ownership of a block job to some BDS somewhere down the chain, but otherwise it should be possible and seems cleaner. I would like to get some feedback about how to properly block jobs during a block streaming operation to an intermediate node. So let's suppose we have a graph like this: [A] - [B] - [C] - [D] - [E] - [F] [F] is the active layer, and to its left is the chain of backing files. So if we stream from [B] (base) to [D] (top) this is the result: [A] - [B] - [D] - [E] - [F] The idea is that the block job would be owned by the node that is receiving the data ([D] in this example) so other operations would still be allowed in other parts of the chain. I would also update query-block-jobs so it would return jobs owned by inner nodes, not just the ones at the root (there's still the issue of how to refer to those nodes, yesterday I wrote a separate e-mail about that). During this process we would block all operations involving any node between base and top ([C] in this example): - Streaming from [D] to [F] would be allowed. - Streaming from [A], [B] or [C] would not be allowed. - Streaming with no base would not be allowed either. Are those assumptions correct? What would we do if part of the chain is shared, like in this case? [A] - [B] - [C] - [D] - [E] - [F] ^ \ [G] - [H] - [I] - [J] Berto
Re: [Qemu-devel] [PATCH 1/3] block: Support streaming to an intermediate layer
Am 20.02.2015 um 14:53 hat Alberto Garcia geschrieben: This adds a new 'top' parameter to stream_start(), that specifies the block device where the data will be written. The image is changed to read-write mode during the streaming and back to read-only afterwards. This also unblocks the stream operation in backing files. Signed-off-by: Alberto Garcia be...@igalia.com The bs parameter is now only used for the following things: 1. As the default for top 2. For error handling: Any errors are reported for bs, even though they are actually for top. Is this correct behaviour? It looks questionable to me. 3. As the BDS that owns the job My question is whether we can't simply call stream_start() with an intermediate node as bs instead of introducing a new parameter. I'm not completely sure about the consequences of 3., i.e. moving ownership of a block job to some BDS somewhere down the chain, but otherwise it should be possible and seems cleaner. Kevin
Re: [Qemu-devel] [PATCH 1/3] block: Support streaming to an intermediate layer
On Thu, Mar 05, 2015 at 03:04:25PM +0100, Kevin Wolf wrote: The bs parameter is now only used for the following things: 1. As the default for top Right. 2. For error handling: Any errors are reported for bs, even though they are actually for top. Is this correct behaviour? It looks questionable to me. Hmm... I guess you mean when calling block_job_error_action(), I probably overlooked that. 3. As the BDS that owns the job My question is whether we can't simply call stream_start() with an intermediate node as bs instead of introducing a new parameter. I'm not completely sure about the consequences of 3., i.e. moving ownership of a block job to some BDS somewhere down the chain, but otherwise it should be possible and seems cleaner. We can, that was actually the first thing I tried and it does work, but then I noticed that other parts of the code would need changes, e.g. qmp_query_block_jobs() must be modified to iterate over all nodes of each device. Since I was also not sure about the consequences of such a change I opted for the conservative approach. Berto
Re: [Qemu-devel] [PATCH 1/3] block: Support streaming to an intermediate layer
On Thu, Mar 05, 2015 at 04:15:52PM +0100, Kevin Wolf wrote: 3. As the BDS that owns the job My question is whether we can't simply call stream_start() with an intermediate node as bs instead of introducing a new parameter. I'm not completely sure about the consequences of 3., i.e. moving ownership of a block job to some BDS somewhere down the chain, but otherwise it should be possible and seems cleaner. Since I was also not sure about the consequences of such a change I opted for the conservative approach. I see. I'm worried about the external API. If we let the root node own the job, we may paint ourselves into a corner with respect to nodes with multiple users and therefore multiple possible root nodes. I'll try to move the job to the intermediate node and see where I can get then. Berto
Re: [Qemu-devel] [PATCH 1/3] block: Support streaming to an intermediate layer
Am 05.03.2015 um 15:58 hat Alberto Garcia geschrieben: On Thu, Mar 05, 2015 at 03:04:25PM +0100, Kevin Wolf wrote: The bs parameter is now only used for the following things: 1. As the default for top Right. 2. For error handling: Any errors are reported for bs, even though they are actually for top. Is this correct behaviour? It looks questionable to me. Hmm... I guess you mean when calling block_job_error_action(), I probably overlooked that. Yes, that and the check whether iostatus is enabled. 3. As the BDS that owns the job My question is whether we can't simply call stream_start() with an intermediate node as bs instead of introducing a new parameter. I'm not completely sure about the consequences of 3., i.e. moving ownership of a block job to some BDS somewhere down the chain, but otherwise it should be possible and seems cleaner. We can, that was actually the first thing I tried and it does work, but then I noticed that other parts of the code would need changes, e.g. qmp_query_block_jobs() must be modified to iterate over all nodes of each device. Since I was also not sure about the consequences of such a change I opted for the conservative approach. I see. I'm worried about the external API. If we let the root node own the job, we may paint ourselves into a corner with respect to nodes with multiple users and therefore multiple possible root nodes. Kevin
