qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Alberto Garcia <berto@igalia.com>,
	Markus Armbruster <armbru@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: [Qemu-devel] [PATCH v2 0/5] Support streaming to an intermediate layer
Date: Mon, 23 Mar 2015 17:12:47 +0200	[thread overview]
Message-ID: <cover.1427119793.git.berto@igalia.com> (raw)

This is a new version of the patches that add support for streaming
to any intermediate layer. You can check the previous version here:

   https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04116.html

I followed Kevin's idea to put the ownership of the block job directly
on the node that receives the data instead of the root node.

There's a few things that I'm not completely sure of and will
certainly generate some debate, but I decided to send the code anyway
so the actual changes can be seen. Note that this depends on the "Add
bdrv_get_device_or_node_name()" patchset I sent last week.

Here are the changes:

1) The 'top' parameter of block-stream disappears. 'device' also
   accepts a node name now, which is used to specify the node where
   the data will be written.

2) Block jobs can now be owned by any node. This implies:

   - The block-job-* commands now also accept a node name in the
     'device' parameter. I decided not to add a separate 'node-name'
     parameter, following what we agreed with the 'block-stream'
     command.

   - The BlockJobInfo type and BLOCK_JOB_* events will report the node
     name in the 'device' field if the node does not have a device
     name. It seems that we had an agreement for the BlockJobInfo case
     so I decided to follow the same approach for these events.

     However, in the BLOCK_IMAGE_CORRUPTED case the preferred solution
     was to add a new 'node-name' field, so I guess we might want to
     do the same here?

   - query-block-jobs now searches the whole tree, not just the root
     nodes.

3) Operations are blocked in all intermediate nodes during the job. If
   we have a chain [A] -> [B] -> [C] -> [D] -> [E] and we stream from
   [B] to [E], then [C] and [D] will also be blocked during the job.

   Since [C] and [D] will be removed from the chain after the job is
   finished I understand that we don't want to perform any operation
   with them.

4) As a consequence of 3), op blockers are also checked in all
   intermediate nodes (not just in the topmost one) before starting a
   streaming operation. I'm currently checking BLOCK_OP_TYPE_STREAM,
   but maybe I should use a different op for the intermediate nodes
   since what I'm going to do there is removing them from the chain?

I think those are the most important changes. Any feedback is welcome!

Berto

v2:
- The 'block-stream' command does not have a 'node-name' parameter
  anymore and reuses 'device' for that purpose.
- Block jobs can now be owned by any intermediate node, and not just
  by the ones at the root. query-block-jobs is updated to reflect that
  change.
- The 'device' parameter of all 'block-job-*' commands can now take a
  node name.
- The BlockJobInfo type and all BLOCK_JOB_* events report the node
  name in the 'device' field if the node does not have a device name.
- All intermediate nodes are blocked (and checked for blockers) during
  the streaming operation.

Alberto Garcia (5):
  block: allow block jobs in any arbitrary node
  block: never cancel a streaming job without running stream_complete()
  block: Support streaming to an intermediate layer
  block: Add QMP support for streaming to an intermediate layer
  docs: Document how to stream to an intermediate layer

 block.c                   |  4 +++-
 block/mirror.c            |  5 +++--
 block/stream.c            | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 blockdev.c                | 47 +++++++++++++++++++++++++----------------------
 blockjob.c                | 17 +++++++++--------
 docs/live-block-ops.txt   | 32 ++++++++++++++++++++------------
 docs/qmp/qmp-events.txt   |  8 ++++----
 include/qapi/qmp/qerror.h |  3 ---
 qapi/block-core.json      | 28 ++++++++++++++++------------
 9 files changed, 122 insertions(+), 69 deletions(-)

-- 
2.1.4

             reply	other threads:[~2015-03-23 15:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-23 15:12 Alberto Garcia [this message]
2015-03-23 15:12 ` [Qemu-devel] [PATCH 1/5] block: allow block jobs in any arbitrary node Alberto Garcia
2015-03-24 22:46   ` Eric Blake
2015-03-23 15:12 ` [Qemu-devel] [PATCH 2/5] block: never cancel a streaming job without running stream_complete() Alberto Garcia
2015-03-23 15:12 ` [Qemu-devel] [PATCH 3/5] block: Support streaming to an intermediate layer Alberto Garcia
2015-03-23 15:12 ` [Qemu-devel] [PATCH 4/5] block: Add QMP support for " Alberto Garcia
2015-03-23 15:12 ` [Qemu-devel] [PATCH 5/5] docs: Document how to stream " Alberto Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1427119793.git.berto@igalia.com \
    --to=berto@igalia.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).