Re: [Qemu-block] [PATCH 0/4] ahci: clean up signature generation

2015-09-17 Thread Stefan Hajnoczi 
On Tue, Sep 01, 2015 at 04:50:37PM -0400, John Snow wrote:
> Ultimately, clean up the signature generation and as a result, tidy up
> the port_reset and init_d2h functions.
> 
> 
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch ahci-sigfix
> https://github.com/jnsnow/qemu/tree/ahci-sigfix
> 
> This version is tagged ahci-sigfix-v1:
> https://github.com/jnsnow/qemu/releases/tag/ahci-sigfix-v1
> 
> John Snow (4):
>   ahci: remove dead reset code
>   ahci: fix signature generation
>   ahci: remove cmd_fis argument from write_fis_d2h
>   ahci: clean up initial d2h semantics
> 
>  hw/ide/ahci.c | 53 ++---
>  1 file changed, 30 insertions(+), 23 deletions(-)
> 
> -- 
> 2.4.3
> 

Reviewed-by: Stefan Hajnoczi

Re: [Qemu-block] [PATCH 2/4] ahci: fix signature generation

2015-09-17 Thread Stefan Hajnoczi 
On Tue, Sep 01, 2015 at 04:50:39PM -0400, John Snow wrote:
> The initial register device-to-host FIS no longer needs to specially
> set certain fields, as these can be handled generically by setting those
> fields explicitly with the signatures we want at port reset time.
> 
> (1) Signatures are decomposed into their four component registers and
> set upon (AHCI) port reset.
> (2) the signature cache register is no longer set manually per-each
> device type, but instead just once during ahci_init_d2h.
> 
> Signed-off-by: John Snow 
> ---
>  hw/ide/ahci.c | 32 
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index d04a161..3c50ccb 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -542,20 +542,31 @@ static void ahci_init_d2h(AHCIDevice *ad)
>  {
>  uint8_t init_fis[20];
>  IDEState *ide_state = >port.ifs[0];
> +AHCIPortRegs *pr = >port_regs;
>  
>  memset(init_fis, 0, sizeof(init_fis));
>  
> -init_fis[4] = 1;
> -init_fis[12] = 1;
> -
> -if (ide_state->drive_kind == IDE_CD) {
> -init_fis[5] = ide_state->lcyl;
> -init_fis[6] = ide_state->hcyl;
> -}
> +/* We're emulating receiving the first Reg H2D Fis from the device;
> + * Update the SIG register, but otherwise procede as normal. */

s/procede/proceed/

Re: [Qemu-block] [PATCH] vmdk: Create streamOptimized as version 3

2015-09-17 Thread Kevin Wolf 
Am 17.09.2015 um 07:04 hat Fam Zheng geschrieben:
> VMware products accept only version 3 for streamOptimized, let's bump
> the version.
> 
> Reported-by: Radoslav Gerganov 
> Signed-off-by: Fam Zheng 

Radoslav, can I have your Reviewed-by and/or Tested-by for this patch?

Kevin

>  block/vmdk.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index be0d640..37326c3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1651,7 +1651,13 @@ static int vmdk_create_extent(const char *filename, 
> int64_t filesize,
>  }
>  magic = cpu_to_be32(VMDK4_MAGIC);
>  memset(, 0, sizeof(header));
> -header.version = zeroed_grain ? 2 : 1;
> +if (compress) {
> +header.version = 3;
> +} else if (zeroed_grain) {
> +header.version = 2;
> +} else {
> +header.version = 1;
> +}
>  header.flags = VMDK4_FLAG_RGD | VMDK4_FLAG_NL_DETECT
> | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
> | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
> -- 
> 2.4.3
>

[Qemu-block] [PATCH 00/16] block: Get rid of bdrv_swap()

2015-09-17 Thread Kevin Wolf 
bdrv_swap() has always been an ugly hack that we would rather have
avoided.  When it was introduced, we simply didn't have the
infrastructure to update pointers instead of transplanting the contents
of BDS object, so we grudgingly added bdrv_swap() as a quick solution.
Meanwhile, most of the infrastructure exists and this series implements
the final step necessary to implement the required functionality in a
less adventurous way.

Kevin Wolf (16):
  block: Introduce BDS.file_child
  vmdk: Use BdrvChild instead of BDS for references to extents
  blkverify: Convert s->test_file to BdrvChild
  quorum: Convert to BdrvChild
  block: Convert bs->file to BdrvChild
  block: Remove bdrv_open_image()
  block: Convert bs->backing_hd to BdrvChild
  block: Manage backing file references in bdrv_set_backing_hd()
  block: Split bdrv_move_feature_fields()
  block/io: Make bdrv_requests_pending() public
  block-backend: Add blk_set_bs()
  block: Introduce parents list
  block: Implement bdrv_append() without bdrv_swap()
  blockjob: Store device name at job creation
  block: Add and use bdrv_replace_in_backing_chain()
  block: Remove bdrv_swap()

 block.c   | 465 +++---
 block/blkdebug.c  |  32 ++--
 block/blkverify.c |  68 +++
 block/block-backend.c |  16 ++
 block/bochs.c |   8 +-
 block/cloop.c |  10 +-
 block/dmg.c   |  20 +-
 block/io.c|  76 
 block/mirror.c|  19 +-
 block/parallels.c |  38 ++--
 block/qapi.c  |  10 +-
 block/qcow.c  |  46 ++---
 block/qcow2-cache.c   |  11 +-
 block/qcow2-cluster.c |  42 +++--
 block/qcow2-refcount.c|  45 ++---
 block/qcow2-snapshot.c|  30 +--
 block/qcow2.c |  68 +++
 block/qed-table.c |   4 +-
 block/qed.c   |  51 +++--
 block/quorum.c|  63 ---
 block/raw_bsd.c   |  40 ++--
 block/snapshot.c  |  12 +-
 block/stream.c|  27 +--
 block/vdi.c   |  17 +-
 block/vhdx-log.c  |  25 +--
 block/vhdx.c  |  36 ++--
 block/vmdk.c  | 133 ++---
 block/vpc.c   |  34 ++--
 block/vvfat.c |  19 +-
 blockdev.c|   6 +-
 blockjob.c|   8 +-
 include/block/block.h |  15 +-
 include/block/block_int.h |  20 +-
 include/block/blockjob.h  |   8 +
 include/qemu/queue.h  |   6 -
 qemu-img.c|  24 +--
 36 files changed, 755 insertions(+), 797 deletions(-)

-- 
1.8.3.1

[Qemu-block] [PATCH 02/16] vmdk: Use BdrvChild instead of BDS for references to extents

2015-09-17 Thread Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/vmdk.c | 99 +++-
 1 file changed, 51 insertions(+), 48 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index be0d640..9702132 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -87,7 +87,7 @@ typedef struct {
 #define L2_CACHE_SIZE 16
 
 typedef struct VmdkExtent {
-BlockDriverState *file;
+BdrvChild *file;
 bool flat;
 bool compressed;
 bool has_marker;
@@ -221,8 +221,8 @@ static void vmdk_free_extents(BlockDriverState *bs)
 g_free(e->l2_cache);
 g_free(e->l1_backup_table);
 g_free(e->type);
-if (e->file != bs->file) {
-bdrv_unref(e->file);
+if (e->file != bs->file_child) {
+bdrv_unref_child(bs, e->file);
 }
 }
 g_free(s->extents);
@@ -367,7 +367,7 @@ static int vmdk_parent_open(BlockDriverState *bs)
 /* Create and append extent to the extent array. Return the added VmdkExtent
  * address. return NULL if allocation failed. */
 static int vmdk_add_extent(BlockDriverState *bs,
-   BlockDriverState *file, bool flat, int64_t sectors,
+   BdrvChild *file, bool flat, int64_t sectors,
int64_t l1_offset, int64_t l1_backup_offset,
uint32_t l1_size,
int l2_size, uint64_t cluster_sectors,
@@ -392,7 +392,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
 return -EFBIG;
 }
 
-nb_sectors = bdrv_nb_sectors(file);
+nb_sectors = bdrv_nb_sectors(file->bs);
 if (nb_sectors < 0) {
 return nb_sectors;
 }
@@ -439,14 +439,14 @@ static int vmdk_init_tables(BlockDriverState *bs, 
VmdkExtent *extent,
 return -ENOMEM;
 }
 
-ret = bdrv_pread(extent->file,
+ret = bdrv_pread(extent->file->bs,
  extent->l1_table_offset,
  extent->l1_table,
  l1_size);
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Could not read l1 table from extent '%s'",
- extent->file->filename);
+ extent->file->bs->filename);
 goto fail_l1;
 }
 for (i = 0; i < extent->l1_size; i++) {
@@ -459,14 +459,14 @@ static int vmdk_init_tables(BlockDriverState *bs, 
VmdkExtent *extent,
 ret = -ENOMEM;
 goto fail_l1;
 }
-ret = bdrv_pread(extent->file,
+ret = bdrv_pread(extent->file->bs,
  extent->l1_backup_table_offset,
  extent->l1_backup_table,
  l1_size);
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Could not read l1 backup table from extent '%s'",
- extent->file->filename);
+ extent->file->bs->filename);
 goto fail_l1b;
 }
 for (i = 0; i < extent->l1_size; i++) {
@@ -485,7 +485,7 @@ static int vmdk_init_tables(BlockDriverState *bs, 
VmdkExtent *extent,
 }
 
 static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
- BlockDriverState *file,
+ BdrvChild *file,
  int flags, Error **errp)
 {
 int ret;
@@ -493,11 +493,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
 VMDK3Header header;
 VmdkExtent *extent;
 
-ret = bdrv_pread(file, sizeof(magic), , sizeof(header));
+ret = bdrv_pread(file->bs, sizeof(magic), , sizeof(header));
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Could not read header from file '%s'",
- file->filename);
+ file->bs->filename);
 return ret;
 }
 ret = vmdk_add_extent(bs, file, false,
@@ -559,7 +559,7 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 }
 
 static int vmdk_open_vmdk4(BlockDriverState *bs,
-   BlockDriverState *file,
+   BdrvChild *file,
int flags, QDict *options, Error **errp)
 {
 int ret;
@@ -570,17 +570,17 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 BDRVVmdkState *s = bs->opaque;
 int64_t l1_backup_offset = 0;
 
-ret = bdrv_pread(file, sizeof(magic), , sizeof(header));
+ret = bdrv_pread(file->bs, sizeof(magic), , sizeof(header));
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Could not read header from file '%s'",
- file->filename);
+ file->bs->filename);
 return -EINVAL;
 }
 if (header.capacity == 0) {
 uint64_t desc_offset = le64_to_cpu(header.desc_offset);
 if (desc_offset) {
-char *buf = vmdk_read_desc(file, desc_offset << 9, errp);
+

[Qemu-block] [PATCH 05/16] block: Convert bs->file to BdrvChild

2015-09-17 Thread Kevin Wolf 
This patch removes the temporary duplication between bs->file and
bs->file_child by converting everything to BdrvChild.

Signed-off-by: Kevin Wolf 
---
 block.c   | 61 ++
 block/blkdebug.c  | 32 
 block/blkverify.c | 33 ++---
 block/bochs.c |  8 +++---
 block/cloop.c | 10 
 block/dmg.c   | 20 +++
 block/io.c| 50 +++---
 block/parallels.c | 38 +++--
 block/qapi.c  |  2 +-
 block/qcow.c  | 42 +---
 block/qcow2-cache.c   | 11 +
 block/qcow2-cluster.c | 38 +
 block/qcow2-refcount.c| 45 ++
 block/qcow2-snapshot.c| 30 ---
 block/qcow2.c | 62 ---
 block/qed-table.c |  4 +--
 block/qed.c   | 32 
 block/raw_bsd.c   | 40 +++---
 block/snapshot.c  | 12 -
 block/vdi.c   | 17 +++--
 block/vhdx-log.c  | 25 ++-
 block/vhdx.c  | 36 ++-
 block/vmdk.c  | 27 +++--
 block/vpc.c   | 34 ++
 include/block/block.h |  8 +-
 include/block/block_int.h |  3 +--
 26 files changed, 377 insertions(+), 343 deletions(-)

diff --git a/block.c b/block.c
index 2e9e5e2..93d713b 100644
--- a/block.c
+++ b/block.c
@@ -809,7 +809,7 @@ static QemuOptsList bdrv_runtime_opts = {
  *
  * Removes all processed options from *options.
  */
-static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
+static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
 QDict *options, int flags, BlockDriver *drv, Error **errp)
 {
 int ret, open_flags;
@@ -823,7 +823,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 assert(options != NULL && bs->options != options);
 
 if (file != NULL) {
-filename = file->filename;
+filename = file->bs->filename;
 } else {
 filename = qdict_get_try_str(options, "filename");
 }
@@ -1401,7 +1401,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
  const BdrvChildRole *child_role, Error **errp)
 {
 int ret;
-BlockDriverState *file = NULL, *bs;
+BdrvChild *file = NULL;
+BlockDriverState *bs;
 BlockDriver *drv = NULL;
 const char *drvname;
 Error *local_err = NULL;
@@ -1485,25 +1486,20 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 flags = bdrv_backing_flags(flags);
 }
 
-assert(file == NULL);
 bs->open_flags = flags;
 
-bs->file_child = bdrv_open_child(filename, options, "file", bs,
- _file, true, _err);
+file = bdrv_open_child(filename, options, "file", bs,
+   _file, true, _err);
 if (local_err) {
 ret = -EINVAL;
 goto fail;
 }
-
-if (bs->file_child) {
-file = bs->file_child->bs;
-}
 }
 
 /* Image format probing */
 bs->probed = !drv;
 if (!drv && file) {
-ret = find_image_format(file, filename, , _err);
+ret = find_image_format(file->bs, filename, , _err);
 if (ret < 0) {
 goto fail;
 }
@@ -1526,7 +1522,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 }
 
 if (file && (bs->file != file)) {
-bdrv_unref(file);
+bdrv_unref_child(bs, file);
 file = NULL;
 }
 
@@ -1587,7 +1583,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 fail:
 if (file != NULL) {
-bdrv_unref(file);
+bdrv_unref_child(bs, file);
 }
 QDECREF(bs->options);
 QDECREF(options);
@@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)
 bdrv_unref(backing_hd);
 }
 
+if (bs->file != NULL) {
+bdrv_unref(bs->file->bs);
+bs->file = NULL;
+}
+
 QLIST_FOREACH_SAFE(child, >children, next, next) {
 /* TODO Remove bdrv_unref() from drivers' close function and use
  * bdrv_unref_child() here */
@@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs)
 bs->options = NULL;
 QDECREF(bs->full_open_options);
 bs->full_open_options = NULL;
-
-if (bs->file != NULL) {
-bdrv_unref(bs->file);
-bs->file = NULL;
-}
 }
 
 if (bs->blk) {
@@ -2565,7 +2561,7 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)

[Qemu-block] [PATCH 12/16] block: Introduce parents list

2015-09-17 Thread Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block.c   | 3 +++
 include/block/block_int.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/block.c b/block.c
index 7930f3c..c196f83 100644
--- a/block.c
+++ b/block.c
@@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState 
*parent_bs,
 };
 
 QLIST_INSERT_HEAD(_bs->children, child, next);
+QLIST_INSERT_HEAD(_bs->parents, child, next_parent);
 
 return child;
 }
@@ -1097,6 +1098,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState 
*parent_bs,
 void bdrv_detach_child(BdrvChild *child)
 {
 QLIST_REMOVE(child, next);
+QLIST_REMOVE(child, next_parent);
 g_free(child);
 }
 
@@ -2037,6 +2039,7 @@ static void bdrv_move_reference_fields(BlockDriverState 
*bs_dest,
 /* keep the same entry in bdrv_states */
 bs_dest->device_list = bs_src->device_list;
 bs_dest->blk = bs_src->blk;
+bs_dest->parents = bs_src->parents;
 
 memcpy(bs_dest->op_blockers, bs_src->op_blockers,
sizeof(bs_dest->op_blockers));
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cfcae52..52ea7c0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -339,6 +339,7 @@ struct BdrvChild {
 BlockDriverState *bs;
 const BdrvChildRole *role;
 QLIST_ENTRY(BdrvChild) next;
+QLIST_ENTRY(BdrvChild) next_parent;
 };
 
 /*
@@ -445,6 +446,7 @@ struct BlockDriverState {
  * parent node of this node. */
 BlockDriverState *inherits_from;
 QLIST_HEAD(, BdrvChild) children;
+QLIST_HEAD(, BdrvChild) parents;
 
 QDict *options;
 BlockdevDetectZeroesOptions detect_zeroes;
-- 
1.8.3.1

[Qemu-block] [PATCH 11/16] block-backend: Add blk_set_bs()

2015-09-17 Thread Kevin Wolf 
It allows changing the BlockDriverState that a BlockBackend points to.

Signed-off-by: Kevin Wolf 
---
 block/block-backend.c | 16 
 include/block/block_int.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index c2e8732..a2afcff 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -239,6 +239,22 @@ BlockDriverState *blk_bs(BlockBackend *blk)
 }
 
 /*
+ * Changes the BlockDriverState attached to @blk
+ */
+void blk_set_bs(BlockBackend *blk, BlockDriverState *bs)
+{
+bdrv_ref(bs);
+
+if (blk->bs) {
+blk->bs->blk = NULL;
+bdrv_unref(blk->bs);
+}
+
+blk->bs = bs;
+bs->blk = blk;
+}
+
+/*
  * Return @blk's DriveInfo if any, else null.
  */
 DriveInfo *blk_legacy_dinfo(BlockBackend *blk)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4598101..cfcae52 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -659,6 +659,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
   BlockCompletionFunc *cb, void *opaque,
   Error **errp);
 
+void blk_set_bs(BlockBackend *blk, BlockDriverState *bs);
+
 void blk_dev_change_media_cb(BlockBackend *blk, bool load);
 bool blk_dev_has_removable_media(BlockBackend *blk);
 void blk_dev_eject_request(BlockBackend *blk, bool force);
-- 
1.8.3.1

[Qemu-block] [PATCH 04/16] quorum: Convert to BdrvChild

2015-09-17 Thread Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/quorum.c | 63 ++
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 8fe53b4..8e6e2dc 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -64,7 +64,7 @@ typedef struct QuorumVotes {
 
 /* the following structure holds the state of one quorum instance */
 typedef struct BDRVQuorumState {
-BlockDriverState **bs; /* children BlockDriverStates */
+BdrvChild **children;  /* children BlockDriverStates */
 int num_children;  /* children count */
 int threshold; /* if less than threshold children reads gave the
 * same result a quorum error occurs.
@@ -336,7 +336,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
 continue;
 }
 QLIST_FOREACH(item, >items, next) {
-quorum_report_bad(acb, s->bs[item->index]->node_name, 0);
+quorum_report_bad(acb, s->children[item->index]->bs->node_name, 0);
 }
 }
 }
@@ -369,8 +369,9 @@ static bool quorum_rewrite_bad_versions(BDRVQuorumState *s, 
QuorumAIOCB *acb,
 continue;
 }
 QLIST_FOREACH(item, >items, next) {
-bdrv_aio_writev(s->bs[item->index], acb->sector_num, acb->qiov,
-acb->nb_sectors, quorum_rewrite_aio_cb, acb);
+bdrv_aio_writev(s->children[item->index]->bs, acb->sector_num,
+acb->qiov, acb->nb_sectors, quorum_rewrite_aio_cb,
+acb);
 }
 }
 
@@ -639,13 +640,13 @@ static BlockAIOCB *read_quorum_children(QuorumAIOCB *acb)
 int i;
 
 for (i = 0; i < s->num_children; i++) {
-acb->qcrs[i].buf = qemu_blockalign(s->bs[i], acb->qiov->size);
+acb->qcrs[i].buf = qemu_blockalign(s->children[i]->bs, 
acb->qiov->size);
 qemu_iovec_init(>qcrs[i].qiov, acb->qiov->niov);
 qemu_iovec_clone(>qcrs[i].qiov, acb->qiov, acb->qcrs[i].buf);
 }
 
 for (i = 0; i < s->num_children; i++) {
-bdrv_aio_readv(s->bs[i], acb->sector_num, >qcrs[i].qiov,
+bdrv_aio_readv(s->children[i]->bs, acb->sector_num, >qcrs[i].qiov,
acb->nb_sectors, quorum_aio_cb, >qcrs[i]);
 }
 
@@ -656,12 +657,12 @@ static BlockAIOCB *read_fifo_child(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->common.bs->opaque;
 
-acb->qcrs[acb->child_iter].buf = qemu_blockalign(s->bs[acb->child_iter],
- acb->qiov->size);
+acb->qcrs[acb->child_iter].buf =
+qemu_blockalign(s->children[acb->child_iter]->bs, acb->qiov->size);
 qemu_iovec_init(>qcrs[acb->child_iter].qiov, acb->qiov->niov);
 qemu_iovec_clone(>qcrs[acb->child_iter].qiov, acb->qiov,
  acb->qcrs[acb->child_iter].buf);
-bdrv_aio_readv(s->bs[acb->child_iter], acb->sector_num,
+bdrv_aio_readv(s->children[acb->child_iter]->bs, acb->sector_num,
>qcrs[acb->child_iter].qiov, acb->nb_sectors,
quorum_aio_cb, >qcrs[acb->child_iter]);
 
@@ -702,8 +703,8 @@ static BlockAIOCB *quorum_aio_writev(BlockDriverState *bs,
 int i;
 
 for (i = 0; i < s->num_children; i++) {
-acb->qcrs[i].aiocb = bdrv_aio_writev(s->bs[i], sector_num, qiov,
- nb_sectors, _aio_cb,
+acb->qcrs[i].aiocb = bdrv_aio_writev(s->children[i]->bs, sector_num,
+ qiov, nb_sectors, _aio_cb,
  >qcrs[i]);
 }
 
@@ -717,12 +718,12 @@ static int64_t quorum_getlength(BlockDriverState *bs)
 int i;
 
 /* check that all file have the same length */
-result = bdrv_getlength(s->bs[0]);
+result = bdrv_getlength(s->children[0]->bs);
 if (result < 0) {
 return result;
 }
 for (i = 1; i < s->num_children; i++) {
-int64_t value = bdrv_getlength(s->bs[i]);
+int64_t value = bdrv_getlength(s->children[i]->bs);
 if (value < 0) {
 return value;
 }
@@ -741,7 +742,7 @@ static void quorum_invalidate_cache(BlockDriverState *bs, 
Error **errp)
 int i;
 
 for (i = 0; i < s->num_children; i++) {
-bdrv_invalidate_cache(s->bs[i], _err);
+bdrv_invalidate_cache(s->children[i]->bs, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
@@ -762,7 +763,7 @@ static coroutine_fn int quorum_co_flush(BlockDriverState 
*bs)
 error_votes.compare = quorum_64bits_compare;
 
 for (i = 0; i < s->num_children; i++) {
-result = bdrv_co_flush(s->bs[i]);
+result = bdrv_co_flush(s->children[i]->bs);
 result_value.l = result;
 quorum_count_vote(_votes, _value, i);
 }
@@ -782,7 +783,7 @@ static bool 
quorum_recurse_is_first_non_filter(BlockDriverState *bs,
 int i;

[Qemu-block] [PATCH 01/16] block: Introduce BDS.file_child

2015-09-17 Thread Kevin Wolf 
Store the BdrvChild for bs->file. At this point, bs->file_child->bs just
duplicates the bs->file pointer. Later, it will completely replace it.

Signed-off-by: Kevin Wolf 
---
 block.c   | 12 +---
 include/block/block_int.h |  1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 6268e37..2e9e5e2 100644
--- a/block.c
+++ b/block.c
@@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 assert(file == NULL);
 bs->open_flags = flags;
-ret = bdrv_open_image(, filename, options, "file",
-  bs, _file, true, _err);
-if (ret < 0) {
+
+bs->file_child = bdrv_open_child(filename, options, "file", bs,
+ _file, true, _err);
+if (local_err) {
+ret = -EINVAL;
 goto fail;
 }
+
+if (bs->file_child) {
+file = bs->file_child->bs;
+}
 }
 
 /* Image format probing */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 14ad4c3..d0dd93e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -381,6 +381,7 @@ struct BlockDriverState {
 BlockDriverState *backing_hd;
 BdrvChild *backing_child;
 BlockDriverState *file;
+BdrvChild *file_child;
 
 NotifierList close_notifiers;
 
-- 
1.8.3.1

[Qemu-block] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap()

2015-09-17 Thread Kevin Wolf 
Remember all parent nodes and just change the pointers there instead of
swapping the contents of the BlockDriverState.

Handling of snapshot=on must be moved further down in bdrv_open()
because *pbs (which is the bs pointer in the BlockBackend) must already
be set before bdrv_append() is called. Otherwise bdrv_append() changes
the BB's pointer to the temporary snapshot, but bdrv_open() overwrites
it with the read-only original image.

Signed-off-by: Kevin Wolf 
---
 block.c | 109 +++-
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/block.c b/block.c
index c196f83..98fc17c 100644
--- a/block.c
+++ b/block.c
@@ -1516,15 +1516,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 bdrv_refresh_filename(bs);
 
-/* For snapshot=on, create a temporary qcow2 overlay. bs points to the
- * temporary snapshot afterwards. */
-if (snapshot_flags) {
-ret = bdrv_append_temp_snapshot(bs, snapshot_flags, _err);
-if (local_err) {
-goto close_and_fail;
-}
-}
-
 /* Check if any unknown options were used */
 if (options && (qdict_size(options) != 0)) {
 const QDictEntry *entry = qdict_first(options);
@@ -1556,6 +1547,16 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 QDECREF(options);
 *pbs = bs;
+
+/* For snapshot=on, create a temporary qcow2 overlay. bs points to the
+ * temporary snapshot afterwards. */
+if (snapshot_flags) {
+ret = bdrv_append_temp_snapshot(bs, snapshot_flags, _err);
+if (local_err) {
+goto close_and_fail;
+}
+}
+
 return 0;
 
 fail:
@@ -1999,20 +2000,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 
 bs_dest->enable_write_cache = bs_src->enable_write_cache;
 
-/* i/o throttled req */
-bs_dest->throttle_state = bs_src->throttle_state,
-bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
-bs_dest->pending_reqs[0]= bs_src->pending_reqs[0];
-bs_dest->pending_reqs[1]= bs_src->pending_reqs[1];
-bs_dest->throttled_reqs[0]  = bs_src->throttled_reqs[0];
-bs_dest->throttled_reqs[1]  = bs_src->throttled_reqs[1];
-memcpy(_dest->round_robin,
-   _src->round_robin,
-   sizeof(bs_dest->round_robin));
-memcpy(_dest->throttle_timers,
-   _src->throttle_timers,
-   sizeof(ThrottleTimers));
-
 /* r/w error */
 bs_dest->on_read_error  = bs_src->on_read_error;
 bs_dest->on_write_error = bs_src->on_write_error;
@@ -2026,10 +2013,25 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 }
 
 /* Fields that only need to be swapped if the contents of BDSes is swapped
- * rather than pointers being changed in the parents. */
+ * rather than pointers being changed in the parents, and throttling fields
+ * because only bdrv_swap() messes with internals of throttling. */
 static void bdrv_move_reference_fields(BlockDriverState *bs_dest,
BlockDriverState *bs_src)
 {
+/* i/o throttled req */
+bs_dest->throttle_state = bs_src->throttle_state,
+bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
+bs_dest->pending_reqs[0]= bs_src->pending_reqs[0];
+bs_dest->pending_reqs[1]= bs_src->pending_reqs[1];
+bs_dest->throttled_reqs[0]  = bs_src->throttled_reqs[0];
+bs_dest->throttled_reqs[1]  = bs_src->throttled_reqs[1];
+memcpy(_dest->round_robin,
+   _src->round_robin,
+   sizeof(bs_dest->round_robin));
+memcpy(_dest->throttle_timers,
+   _src->throttle_timers,
+   sizeof(ThrottleTimers));
+
 /* reference count */
 bs_dest->refcnt = bs_src->refcnt;
 
@@ -2155,6 +2157,42 @@ void bdrv_swap(BlockDriverState *bs_new, 
BlockDriverState *bs_old)
 bdrv_rebind(bs_old);
 }
 
+static void change_parent_backing_link(BlockDriverState *from,
+   BlockDriverState *to)
+{
+BdrvChild *c, *next;
+
+QLIST_FOREACH_SAFE(c, >parents, next_parent, next) {
+assert(c->role != _backing);
+c->bs = to;
+QLIST_REMOVE(c, next_parent);
+QLIST_INSERT_HEAD(>parents, c, next_parent);
+}
+if (from->blk) {
+blk_set_bs(from->blk, to);
+if (!to->device_list.tqe_prev) {
+QTAILQ_INSERT_BEFORE(from, to, device_list);
+}
+QTAILQ_REMOVE(_states, from, device_list);
+}
+}
+
+static void swap_feature_fields(BlockDriverState *bs_top,
+BlockDriverState *bs_new)
+{
+BlockDriverState tmp;
+
+bdrv_move_feature_fields(, bs_top);
+bdrv_move_feature_fields(bs_top, bs_new);
+bdrv_move_feature_fields(bs_new, );
+
+assert(!bs_new->io_limits_enabled);
+if (bs_top->io_limits_enabled) {
+bdrv_io_limits_enable(bs_new,

[Qemu-block] [PATCH 16/16] block: Remove bdrv_swap()

2015-09-17 Thread Kevin Wolf 
bdrv_swap() is unused now. Remove it and all functions that have
no other users than bdrv_swap(). In particular, this removes the
.bdrv_rebind callbacks from block drivers.

Signed-off-by: Kevin Wolf 
---
 block.c   | 155 +-
 block/qed.c   |   7 ---
 block/vvfat.c |   7 ---
 include/block/block_int.h |   1 -
 include/qemu/queue.h  |   6 --
 5 files changed, 1 insertion(+), 175 deletions(-)

diff --git a/block.c b/block.c
index 7c21659..5973409 100644
--- a/block.c
+++ b/block.c
@@ -1980,15 +1980,7 @@ void bdrv_make_anon(BlockDriverState *bs)
 bs->node_name[0] = '\0';
 }
 
-static void bdrv_rebind(BlockDriverState *bs)
-{
-if (bs->drv && bs->drv->bdrv_rebind) {
-bs->drv->bdrv_rebind(bs);
-}
-}
-
-/* Fields that need to stay with the top-level BDS, no matter whether the
- * address of the top-level BDS stays the same or not. */
+/* Fields that need to stay with the top-level BDS */
 static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
  BlockDriverState *bs_src)
 {
@@ -2012,151 +2004,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 bs_dest->dirty_bitmaps  = bs_src->dirty_bitmaps;
 }
 
-/* Fields that only need to be swapped if the contents of BDSes is swapped
- * rather than pointers being changed in the parents, and throttling fields
- * because only bdrv_swap() messes with internals of throttling. */
-static void bdrv_move_reference_fields(BlockDriverState *bs_dest,
-   BlockDriverState *bs_src)
-{
-/* i/o throttled req */
-bs_dest->throttle_state = bs_src->throttle_state,
-bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
-bs_dest->pending_reqs[0]= bs_src->pending_reqs[0];
-bs_dest->pending_reqs[1]= bs_src->pending_reqs[1];
-bs_dest->throttled_reqs[0]  = bs_src->throttled_reqs[0];
-bs_dest->throttled_reqs[1]  = bs_src->throttled_reqs[1];
-memcpy(_dest->round_robin,
-   _src->round_robin,
-   sizeof(bs_dest->round_robin));
-memcpy(_dest->throttle_timers,
-   _src->throttle_timers,
-   sizeof(ThrottleTimers));
-
-/* reference count */
-bs_dest->refcnt = bs_src->refcnt;
-
-/* job */
-bs_dest->job= bs_src->job;
-
-/* keep the same entry in bdrv_states */
-bs_dest->device_list = bs_src->device_list;
-bs_dest->blk = bs_src->blk;
-bs_dest->parents = bs_src->parents;
-
-memcpy(bs_dest->op_blockers, bs_src->op_blockers,
-   sizeof(bs_dest->op_blockers));
-}
-
-/*
- * Swap bs contents for two image chains while they are live,
- * while keeping required fields on the BlockDriverState that is
- * actually attached to a device.
- *
- * This will modify the BlockDriverState fields, and swap contents
- * between bs_new and bs_old. Both bs_new and bs_old are modified.
- *
- * bs_new must not be attached to a BlockBackend.
- *
- * This function does not create any image files.
- */
-void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
-{
-BlockDriverState tmp;
-BdrvChild *child;
-
-bdrv_drain(bs_new);
-bdrv_drain(bs_old);
-
-/* The code needs to swap the node_name but simply swapping node_list won't
- * work so first remove the nodes from the graph list, do the swap then
- * insert them back if needed.
- */
-if (bs_new->node_name[0] != '\0') {
-QTAILQ_REMOVE(_bdrv_states, bs_new, node_list);
-}
-if (bs_old->node_name[0] != '\0') {
-QTAILQ_REMOVE(_bdrv_states, bs_old, node_list);
-}
-
-/* If the BlockDriverState is part of a throttling group acquire
- * its lock since we're going to mess with the protected fields.
- * Otherwise there's no need to worry since no one else can touch
- * them. */
-if (bs_old->throttle_state) {
-throttle_group_lock(bs_old);
-}
-
-/* bs_new must be unattached and shouldn't have anything fancy enabled */
-assert(!bs_new->blk);
-assert(QLIST_EMPTY(_new->dirty_bitmaps));
-assert(bs_new->job == NULL);
-assert(bs_new->io_limits_enabled == false);
-assert(bs_new->throttle_state == NULL);
-assert(!throttle_timers_are_initialized(_new->throttle_timers));
-
-tmp = *bs_new;
-*bs_new = *bs_old;
-*bs_old = tmp;
-
-/* there are some fields that should not be swapped, move them back */
-bdrv_move_feature_fields(, bs_old);
-bdrv_move_feature_fields(bs_old, bs_new);
-bdrv_move_feature_fields(bs_new, );
-
-bdrv_move_reference_fields(, bs_old);
-bdrv_move_reference_fields(bs_old, bs_new);
-bdrv_move_reference_fields(bs_new, );
-
-/* bs_new must remain unattached */
-assert(!bs_new->blk);
-
-/* Check a few fields that should remain attached to the device */
-assert(bs_new->job == NULL);
-assert(bs_new->io_limits_enabled ==

[Qemu-block] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain()

2015-09-17 Thread Kevin Wolf 
This cleans up the mess we left behind in the mirror code after the
previous patch. Instead of using bdrv_swap(), just change pointers.

The interface change of the mirror job that callers must consider is
that after job completion, their local BDS pointers still point to the
same node now. qemu-img must change its code accordingly (which makes it
easier to understand); the other callers stays unchanged because after
completion they don't do anything with the BDS, but just with the job,
and the job is still owned by the source BDS.

Signed-off-by: Kevin Wolf 
---
 block.c   | 32 +++-
 block/mirror.c| 23 +++
 include/block/block.h |  4 +++-
 qemu-img.c| 16 
 4 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index 98fc17c..7c21659 100644
--- a/block.c
+++ b/block.c
@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState 
*parent_bs,
 return child;
 }
 
-void bdrv_detach_child(BdrvChild *child)
+static void bdrv_detach_child(BdrvChild *child)
 {
 QLIST_REMOVE(child, next);
 QLIST_REMOVE(child, next_parent);
@@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, 
BlockDriverState *bs_top)
 bdrv_unref(bs_new);
 }
 
+void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState 
*new)
+{
+assert(!bdrv_requests_pending(old));
+assert(!bdrv_requests_pending(new));
+
+bdrv_ref(old);
+
+if (old->blk) {
+/* As long as these fields aren't in BlockBackend, but in the top-level
+ * BlockDriverState, it's not possible for a BDS to have two BBs.
+ *
+ * We really want to copy the fields from old to new, but we go for a
+ * swap instead so that pointers aren't duplicated and cause trouble.
+ * (Also, bdrv_swap() used to do the same.) */
+assert(!new->blk);
+swap_feature_fields(old, new);
+}
+change_parent_backing_link(old, new);
+
+/* Change backing files if a previously independent node is added to the
+ * chain. For active commit, we replace top by its own (indirect) backing
+ * file and don't do anything here so we don't build a loop. */
+if (new->backing == NULL && !bdrv_chain_contains(backing_bs(old), new)) {
+bdrv_set_backing_hd(new, backing_bs(old));
+bdrv_set_backing_hd(old, NULL);
+}
+
+bdrv_unref(old);
+}
+
 static void bdrv_delete(BlockDriverState *bs)
 {
 assert(!bs->job);
diff --git a/block/mirror.c b/block/mirror.c
index 571da27..443d97c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -353,6 +353,11 @@ static void mirror_exit(BlockJob *job, void *opaque)
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
 MirrorExitData *data = opaque;
 AioContext *replace_aio_context = NULL;
+BlockDriverState *src = s->common.bs;
+
+/* Make sure that the source BDS doesn't go away before we called
+ * block_job_completed(). */
+bdrv_ref(src);
 
 if (s->to_replace) {
 replace_aio_context = bdrv_get_aio_context(s->to_replace);
@@ -367,22 +372,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
 if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
 bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
 }
-bdrv_swap(s->target, to_replace);
-if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
-/* drop the bs loop chain formed by the swap: break the loop then
- * trigger the unref */
-/* FIXME This duplicates bdrv_set_backing_hd(), except for the
- * actual detach/unref so that the loop can be broken. When
- * bdrv_swap() gets replaced, this will become sane again. */
-BlockDriverState *backing = s->base->backing->bs;
-assert(s->base->backing_blocker);
-bdrv_op_unblock_all(s->base->backing->bs, 
s->base->backing_blocker);
-error_free(s->base->backing_blocker);
-s->base->backing_blocker = NULL;
-bdrv_detach_child(s->base->backing);
-s->base->backing = NULL;
-bdrv_unref(backing);
-}
+bdrv_replace_in_backing_chain(to_replace, s->target);
 }
 if (s->to_replace) {
 bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
@@ -396,6 +386,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
 bdrv_unref(s->target);
 block_job_completed(>common, data->ret);
 g_free(data);
+bdrv_unref(src);
 }
 
 static void coroutine_fn mirror_run(void *opaque)
diff --git a/include/block/block.h b/include/block/block.h
index 6dc6766..a046bd0 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -202,6 +202,9 @@ BlockDriverState *bdrv_new(void);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void

[Qemu-block] [PATCH] throttle: test that snapshots move the throttling configuration

2015-09-17 Thread Alberto Garcia 
If a snapshot is performed on a device that has I/O limits they should
be moved to the target image (the new active layer).

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/096 | 69 ++
 tests/qemu-iotests/096.out |  5 
 tests/qemu-iotests/group   |  1 +
 3 files changed, 75 insertions(+)
 create mode 100644 tests/qemu-iotests/096
 create mode 100644 tests/qemu-iotests/096.out

diff --git a/tests/qemu-iotests/096 b/tests/qemu-iotests/096
new file mode 100644
index 000..e34204b
--- /dev/null
+++ b/tests/qemu-iotests/096
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+#
+# Test that snapshots move the throttling configuration to the active
+# layer
+#
+# Copyright (C) 2015 Igalia, S.L.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import iotests
+import os
+
+class TestLiveSnapshot(iotests.QMPTestCase):
+base_img = os.path.join(iotests.test_dir, 'base.img')
+target_img = os.path.join(iotests.test_dir, 'target.img')
+group = 'mygroup'
+iops = 6000
+iops_size = 1024
+
+def setUp(self):
+opts = []
+opts.append('node-name=base')
+opts.append('throttling.group=%s' % self.group)
+opts.append('throttling.iops-total=%d' % self.iops)
+opts.append('throttling.iops-size=%d' % self.iops_size)
+iotests.qemu_img('create', '-f', iotests.imgfmt, self.base_img, '100M')
+self.vm = iotests.VM().add_drive(self.base_img, ','.join(opts))
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.base_img)
+os.remove(self.target_img)
+
+def checkConfig(self, active_layer):
+result = self.vm.qmp('query-named-block-nodes')
+for r in result['return']:
+if r['node-name'] == active_layer:
+self.assertEqual(r['group'], self.group)
+self.assertEqual(r['iops'], self.iops)
+self.assertEqual(r['iops_size'], self.iops_size)
+else:
+self.assertFalse(r.has_key('group'))
+self.assertEqual(r['iops'], 0)
+self.assertFalse(r.has_key('iops_size'))
+
+def testSnapshot(self):
+self.checkConfig('base')
+self.vm.qmp('blockdev-snapshot-sync',
+node_name = 'base',
+snapshot_node_name = 'target',
+snapshot_file = self.target_img,
+format = iotests.imgfmt)
+self.checkConfig('target')
+
+if __name__ == '__main__':
+iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/096.out b/tests/qemu-iotests/096.out
new file mode 100644
index 000..ae1213e
--- /dev/null
+++ b/tests/qemu-iotests/096.out
@@ -0,0 +1,5 @@
+.
+--
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 439b1d2..30c784e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -102,6 +102,7 @@
 093 auto
 094 rw auto quick
 095 rw auto quick
+096 rw auto quick
 097 rw auto backing
 098 rw auto backing quick
 099 rw auto quick
-- 
2.5.1

[Qemu-block] [PATCH 14/16] blockjob: Store device name at job creation

2015-09-17 Thread Kevin Wolf 
Some block jobs change the block device graph on completion. This means
that the device that owns the job and originally was addressed with its
device name may no longer be what the corresponding BlockBackend points
to.

Previously, the effects of bdrv_swap() ensured that the job was (at
least partially) transferred to the target image. Events that contain
the device name could still use bdrv_get_device_name(job->bs) and get
the same result.

After removing bdrv_swap(), this won't work any more. Instead, save the
device name at job creation and use that copy for the events.

Signed-off-by: Kevin Wolf 
---
 blockjob.c   | 8 +---
 include/block/blockjob.h | 8 
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 62bb906..1b5e535 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -54,6 +54,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->bs= bs;
 job->cb= cb;
 job->opaque= opaque;
@@ -81,6 +82,7 @@ void block_job_release(BlockDriverState *bs)
 bs->job = NULL;
 bdrv_op_unblock_all(bs, job->blocker);
 error_free(job->blocker);
+g_free(job->id);
 g_free(job);
 }
 
@@ -291,7 +293,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int 
error)
 void block_job_event_cancelled(BlockJob *job)
 {
 qapi_event_send_block_job_cancelled(job->driver->job_type,
-bdrv_get_device_name(job->bs),
+job->id,
 job->len,
 job->offset,
 job->speed,
@@ -301,7 +303,7 @@ void block_job_event_cancelled(BlockJob *job)
 void block_job_event_completed(BlockJob *job, const char *msg)
 {
 qapi_event_send_block_job_completed(job->driver->job_type,
-bdrv_get_device_name(job->bs),
+job->id,
 job->len,
 job->offset,
 job->speed,
@@ -315,7 +317,7 @@ void block_job_event_ready(BlockJob *job)
 job->ready = true;
 
 qapi_event_send_block_job_ready(job->driver->job_type,
-bdrv_get_device_name(job->bs),
+job->id,
 job->len,
 job->offset,
 job->speed, _abort);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index dd9d5e6..289b13f 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -65,6 +65,14 @@ struct BlockJob {
 BlockDriverState *bs;
 
 /**
+ * The ID of the block job. Currently the BlockBackend name of the BDS
+ * owning the job at the time when the job is started.
+ *
+ * TODO Decouple block job IDs from BlockBackend names
+ */
+char *id;
+
+/**
  * The coroutine that executes the job.  If not NULL, it is
  * reentered when busy is false and the job is cancelled.
  */
-- 
1.8.3.1

[Qemu-block] [PATCH 06/16] block: Remove bdrv_open_image()

2015-09-17 Thread Kevin Wolf 
It is unused now.

Signed-off-by: Kevin Wolf 
---
 block.c   | 34 --
 include/block/block.h |  4 
 2 files changed, 38 deletions(-)

diff --git a/block.c b/block.c
index 93d713b..3992241 100644
--- a/block.c
+++ b/block.c
@@ -1279,40 +1279,6 @@ done:
 return c;
 }
 
-/*
- * This is a version of bdrv_open_child() that returns 0/-EINVAL instead of
- * a BdrvChild object.
- *
- * If allow_none is true, no image will be opened if filename is false and no
- * BlockdevRef is given. *pbs will remain unchanged and 0 will be returned.
- *
- * To conform with the behavior of bdrv_open(), *pbs has to be NULL.
- */
-int bdrv_open_image(BlockDriverState **pbs, const char *filename,
-QDict *options, const char *bdref_key,
-BlockDriverState* parent, const BdrvChildRole *child_role,
-bool allow_none, Error **errp)
-{
-Error *local_err = NULL;
-BdrvChild *c;
-
-assert(pbs);
-assert(*pbs == NULL);
-
-c = bdrv_open_child(filename, options, bdref_key, parent, child_role,
-allow_none, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return -EINVAL;
-}
-
-if (c != NULL) {
-*pbs = c->bs;
-}
-
-return 0;
-}
-
 int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
 {
 /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
diff --git a/include/block/block.h b/include/block/block.h
index 79b2e91..130477c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -204,10 +204,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
-int bdrv_open_image(BlockDriverState **pbs, const char *filename,
-QDict *options, const char *bdref_key,
-BlockDriverState* parent, const BdrvChildRole *child_role,
-bool allow_none, Error **errp);
 BdrvChild *bdrv_open_child(const char *filename,
QDict *options, const char *bdref_key,
BlockDriverState* parent,
-- 
1.8.3.1

[Qemu-block] [PATCH 09/16] block: Split bdrv_move_feature_fields()

2015-09-17 Thread Kevin Wolf 
After bdrv_swap(), some fields must be moved back to their original BDS
to compensate for the effects that a swap of the contents of the objects
has while keeping the old addresses. Other fields must be moved back
because they should logically be moved and must stay on top

When replacing bdrv_swap() with operations changing the pointers in the
parents, we only need the latter and must avoid swapping the former.
Split the function accordingly.

Signed-off-by: Kevin Wolf 
---
 block.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/block.c b/block.c
index 743f82e..7930f3c 100644
--- a/block.c
+++ b/block.c
@@ -1984,6 +1984,8 @@ static void bdrv_rebind(BlockDriverState *bs)
 }
 }
 
+/* Fields that need to stay with the top-level BDS, no matter whether the
+ * address of the top-level BDS stays the same or not. */
 static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
  BlockDriverState *bs_src)
 {
@@ -2019,7 +2021,13 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 
 /* dirty bitmap */
 bs_dest->dirty_bitmaps  = bs_src->dirty_bitmaps;
+}
 
+/* Fields that only need to be swapped if the contents of BDSes is swapped
+ * rather than pointers being changed in the parents. */
+static void bdrv_move_reference_fields(BlockDriverState *bs_dest,
+   BlockDriverState *bs_src)
+{
 /* reference count */
 bs_dest->refcnt = bs_src->refcnt;
 
@@ -2090,6 +2098,10 @@ void bdrv_swap(BlockDriverState *bs_new, 
BlockDriverState *bs_old)
 bdrv_move_feature_fields(bs_old, bs_new);
 bdrv_move_feature_fields(bs_new, );
 
+bdrv_move_reference_fields(, bs_old);
+bdrv_move_reference_fields(bs_old, bs_new);
+bdrv_move_reference_fields(bs_new, );
+
 /* bs_new must remain unattached */
 assert(!bs_new->blk);
 
-- 
1.8.3.1

[Qemu-block] [PATCH 07/16] block: Convert bs->backing_hd to BdrvChild

2015-09-17 Thread Kevin Wolf 
This is the final step in converting all of the BlockDriverState
pointers that block drivers use to BdrvChild.

After this patch, bs->children contains the full list of child nodes
that are referenced by a given BDS, and these children are only
referenced through BdrvChild, so that updating the pointer in there is
enough for changing edges in the graph.

Signed-off-by: Kevin Wolf 
---
 block.c   | 116 +-
 block/io.c|  24 +-
 block/mirror.c|   7 +--
 block/qapi.c  |   8 ++--
 block/qcow.c  |   4 +-
 block/qcow2-cluster.c |   4 +-
 block/qcow2.c |   6 +--
 block/qed.c   |  12 ++---
 block/stream.c|  10 ++--
 block/vmdk.c  |  21 +
 block/vvfat.c |   6 +--
 blockdev.c|   6 +--
 include/block/block_int.h |  12 +++--
 qemu-img.c|   8 ++--
 14 files changed, 130 insertions(+), 114 deletions(-)

diff --git a/block.c b/block.c
index 3992241..fb94567 100644
--- a/block.c
+++ b/block.c
@@ -721,7 +721,7 @@ const BdrvChildRole child_format = {
 };
 
 /*
- * Returns the flags that bs->backing_hd should get, based on the given flags
+ * Returns the flags that bs->backing should get, based on the given flags
  * for the parent BDS
  */
 static int bdrv_backing_flags(int flags)
@@ -1115,32 +1115,31 @@ void bdrv_unref_child(BlockDriverState *parent, 
BdrvChild *child)
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
 {
 
-if (bs->backing_hd) {
+if (bs->backing) {
 assert(bs->backing_blocker);
-bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
-bdrv_detach_child(bs->backing_child);
+bdrv_op_unblock_all(bs->backing->bs, bs->backing_blocker);
+bdrv_detach_child(bs->backing);
 } else if (backing_hd) {
 error_setg(>backing_blocker,
"node is used as backing hd of '%s'",
bdrv_get_device_or_node_name(bs));
 }
 
-bs->backing_hd = backing_hd;
 if (!backing_hd) {
 error_free(bs->backing_blocker);
 bs->backing_blocker = NULL;
-bs->backing_child = NULL;
+bs->backing = NULL;
 goto out;
 }
-bs->backing_child = bdrv_attach_child(bs, backing_hd, _backing);
+bs->backing = bdrv_attach_child(bs, backing_hd, _backing);
 bs->open_flags &= ~BDRV_O_NO_BACKING;
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
 pstrcpy(bs->backing_format, sizeof(bs->backing_format),
 backing_hd->drv ? backing_hd->drv->format_name : "");
 
-bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
+bdrv_op_block_all(backing_hd, bs->backing_blocker);
 /* Otherwise we won't be able to commit due to check in bdrv_commit */
-bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
+bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
 bs->backing_blocker);
 out:
 bdrv_refresh_limits(bs, NULL);
@@ -1161,7 +1160,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 BlockDriverState *backing_hd;
 Error *local_err = NULL;
 
-if (bs->backing_hd != NULL) {
+if (bs->backing != NULL) {
 QDECREF(options);
 goto free_exit;
 }
@@ -1201,7 +1200,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 qdict_put(options, "driver", qstring_from_str(bs->backing_format));
 }
 
-assert(bs->backing_hd == NULL);
+assert(bs->backing == NULL);
 ret = bdrv_open_inherit(_hd,
 *backing_filename ? backing_filename : NULL,
 NULL, options, 0, bs, _backing, _err);
@@ -1885,8 +1884,8 @@ void bdrv_close(BlockDriverState *bs)
 
 bs->drv->bdrv_close(bs);
 
-if (bs->backing_hd) {
-BlockDriverState *backing_hd = bs->backing_hd;
+if (bs->backing) {
+BlockDriverState *backing_hd = bs->backing->bs;
 bdrv_set_backing_hd(bs, NULL);
 bdrv_unref(backing_hd);
 }
@@ -2204,20 +2203,20 @@ int bdrv_commit(BlockDriverState *bs)
 if (!drv)
 return -ENOMEDIUM;
 
-if (!bs->backing_hd) {
+if (!bs->backing) {
 return -ENOTSUP;
 }
 
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, NULL) ||
-bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET, NULL)) 
{
+bdrv_op_is_blocked(bs->backing->bs, BLOCK_OP_TYPE_COMMIT_TARGET, 
NULL)) {
 return -EBUSY;
 }
 
-ro = bs->backing_hd->read_only;
-open_flags =  bs->backing_hd->open_flags;
+ro = bs->backing->bs->read_only;
+open_flags =  bs->backing->bs->open_flags;
 
 if (ro) {
-if (bdrv_reopen(bs->backing_hd, open_flags | BDRV_O_RDWR, NULL)) {
+if (bdrv_reopen(bs->backing->bs,

Re: [Qemu-block] [Qemu-devel] [PATCH 0/4] ahci: clean up signature generation

2015-09-17 Thread John Snow 


On 09/17/2015 04:40 AM, Stefan Hajnoczi wrote:
> On Tue, Sep 01, 2015 at 04:50:37PM -0400, John Snow wrote:
>> Ultimately, clean up the signature generation and as a result, tidy up
>> the port_reset and init_d2h functions.
>>
>> 
>>
>> For convenience, this branch is available at:
>> https://github.com/jnsnow/qemu.git branch ahci-sigfix
>> https://github.com/jnsnow/qemu/tree/ahci-sigfix
>>
>> This version is tagged ahci-sigfix-v1:
>> https://github.com/jnsnow/qemu/releases/tag/ahci-sigfix-v1
>>
>> John Snow (4):
>>   ahci: remove dead reset code
>>   ahci: fix signature generation
>>   ahci: remove cmd_fis argument from write_fis_d2h
>>   ahci: clean up initial d2h semantics
>>
>>  hw/ide/ahci.c | 53 ++---
>>  1 file changed, 30 insertions(+), 23 deletions(-)
>>
>> -- 
>> 2.4.3
>>
> 
> Reviewed-by: Stefan Hajnoczi 
> 

With the comment in patch 2 fixed:

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js