Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-07-01 Thread Max Reitz

On 01.07.2015 03:08, Wen Congyang wrote:

On 06/30/2015 09:17 PM, Max Reitz wrote:

On 29.06.2015 03:16, Wen Congyang wrote:

On 06/26/2015 11:16 PM, Max Reitz wrote:

I see two solutions to this issue: Either, we do it right and that means, if 
there is a change in the BDS graph (e.g. because of bdrv_swap()),
we have to call bdrv_refresh_filename() on all of the changed BDS's parents. In 
order to be able to enumerate a BDS's parents, we need Kevin's
series, as said before.

IIUC, the BDS can have more than one parent.

Indeed. Kevin's series adds a generalized parent-child relationship, which 
makes it possible to both enumerate all the children of a BDS and all its 
parents.

How to get all its parents? Is this patch not merged into upstream?


The patch is actually already merged upstream 
(6e93e7c41fdfdee3068770cae79380e1d986b76a), but it does not add a way to 
enumerate a BDS's parents, only a BDS's children. It should not be too 
difficult to implement the reverse however, I guess, based on that patch.

Konsole output

Or we revive my series block: Drop BDS.filename which drops the BDS.filename 
field completely and always rebuilds it if it is queried.
This would fix the issue as well, however, there is a reason it has been lying 
around for nine months now, and that reason is that we did
not yet fully examine the impacts of dropping that field, especially concerning 
libvirt.

If BDS.filename is dropped, this problem will go.

Right. But we have to make sure there won't be any negative impact if we do so, and 
because that is not really trivial, the series hasn't been applied yet and didn't receive 
further attention. Also, there was no real reason to do it other than Because we 
can until now. But I think the issue mentioned here is a good reason why we should 
indeed reconsider it.

I don't find this series.


Maybe because it's pretty old. :-) I sent it last September: 
http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg04878.html


Max



Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-30 Thread Wen Congyang
On 06/30/2015 09:17 PM, Max Reitz wrote:
 On 29.06.2015 03:16, Wen Congyang wrote:
 On 06/26/2015 11:16 PM, Max Reitz wrote:
 On 26.06.2015 16:27, Wen Congyang wrote:
 At 2015/6/26 21:47, Max Reitz Wrote:
 On 25.06.2015 08:41, Wen Congyang wrote:
 We can use block job mirror to repair broken quorum files. But the
 command
 'info block' doesn't output correct filename after block job mirror
 finishes.
 Which filename? The quorum filename is broken after this patch, too. In
 In my test, quorum has two children, s-common.bs-drv is quorum, and 
 s-to_replace
 is one of his child. Without this patch, info block will output obsolete 
 information.
 With this patch, the quorum's filename is right. I don't know why quorum 
 filename
 is broken.
 Oh, yes, you are right, I forgot to execute block-job-complete. However, 
 this patch relies on the Quorum BDS being the mirror source, which is the 
 intentional use case but certainly not the only one:

 $ x86_64-softmmu/qemu-system-x86_64 -drive 
 if=none,id=quorum,driver=quorum,children.0.file.filename=test1.qcow2,children.1.file.filename=test2.qcow2,children.1.node-name=ch1,vote-threshold=1
  -drive if=none,id=drv,file=test2.qcow2 -qmp stdio
 {QMP: {version: {qemu: {micro: 50, minor: 3, major: 2}, 
 package: }, capabilities: []}}
 {'execute':'qmp_capabilities'}
 {return: {}}
 {'execute':'drive-mirror','arguments':{'device':'drv','target':'tmp.qcow2','format':'qcow2','node-name':'nch1','replaces':'ch1','sync':'full'}}
 Formatting 'tmp.qcow2', fmt=qcow2 size=67108864 encryption=off 
 cluster_size=65536 lazy_refcounts=off refcount_bits=16
 {return: {}}
 {timestamp: {seconds: 1435331363, microseconds: 217707}, event: 
 BLOCK_JOB_READY, data: {device: drv, len: 0, offset: 0, 
 speed: 0, type: mirror}}
 {'execute':'block-job-complete','arguments':{'device':'drv'}}
 {return: {}}
 {timestamp: {seconds: 1435331373, microseconds: 152945}, event: 
 BLOCK_JOB_COMPLETED, data: {device: drv, len: 0, offset: 0, 
 speed: 0, type: mirror}}
 {'execute':'query-block'}
 {return: [{device: quorum, locked: false, removable: true, 
 inserted: {iops_rd: 0, detect_zeroes: off, image: 
 {virtual-size: 67108864, filename: json:{\children\: [{\driver\: 
 \qcow2\, \file\: {\driver\: \file\, \filename\: 
 \test1.qcow2\}}, {\driver\: \qcow2\, \file\: {\driver\: \file\, 
 \filename\: \test2.qcow2\}}], \driver\: \quorum\, \blkverify\: 
 false, \rewrite-corrupted\: false, \vote-threshold\: 1}, format: 
 quorum}, iops_wr: 0, ro: false, backing_file_depth: 0, drv: 
 quorum, iops: 0, bps_wr: 0, write_threshold: 0, encrypted: false, 
 bps: 0, bps_rd: 0, cache: {no-flush: false, direct: false, 
 writeback: true}, file: json:{\children\: [{\driver\: \qcow2\, 
 \file\: {\driver\: \file\, \filename\: \test1.qcow2\}}, 
 {\driver\: \qcow2\, \file\: {\driver\: \file\, \filename\: 
 \test2.qcow2\}}], \driver\: \quorum\, \blkverify\: false,
 \rewrite-corrupted\: false, \vote-threshold\: 1}, 
 encryption_key_missing: false}, tray_open: false, type: unknown}, 
 {device: drv, locked: false, removable: true, inserted: 
 {iops_rd: 0, detect_zeroes: off, image: {virtual-size: 67108864, 
 filename: test2.qcow2, cluster-size: 65536, format: qcow2, 
 actual-size: 200704, format-specific: {type: qcow2, data: 
 {compat: 1.1, lazy-refcounts: false, refcount-bits: 16, corrupt: 
 false}}, dirty-flag: false}, iops_wr: 0, ro: false, 
 backing_file_depth: 0, drv: qcow2, iops: 0, bps_wr: 0, 
 write_threshold: 0, encrypted: false, bps: 0, bps_rd: 0, cache: 
 {no-flush: false, direct: false, writeback: true}, file: 
 test2.qcow2, encryption_key_missing: false}, tray_open: false, 
 type: unknown}, {io-status: ok, device: ide1-cd0, locked: 
 false, removable: true, tray_open: false, type: unknown}, 
 {device: floppy0, locked: fals!
   e,
 removable: true, tray_open: false, type: unknown}, {device: 
 sd0, locked: false, removable: true, tray_open: false, type: 
 unknown}]}

 This patch makes mirror_exit() refresh the name of the block job's device 
 (in this case drv), which is not necessarily a parent of the node being 
 replaced.
 Even if it were, imagine a configuration where there are two nested 
 quorums, with the inner one being repaired using the replaces parameter 
 for drive-mirror.
 In this case, this patch would fix the inner Quorum's file name, but not 
 the outer one's.
 If both inner and outer quorum are BBs, the outer one's is not fixed.
 
 I think this is independent of whether which Quorum has a BB attached to it; 
 it's just that unless there is a BB at the outer Quorum BDS, you cannot query 
 it with query-block.
 

 I see two solutions to this issue: Either, we do it right and that means, 
 if there is a change in the BDS graph (e.g. because of bdrv_swap()),
 we have to call bdrv_refresh_filename() on all of the changed BDS's 
 parents. In order to be able to enumerate a BDS's parents, we need Kevin's
 series, as said before.
 IIUC, the BDS can have more than one parent.
 
 Indeed. Kevin's series adds a generalized 

Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-30 Thread Max Reitz

On 29.06.2015 03:16, Wen Congyang wrote:

On 06/26/2015 11:16 PM, Max Reitz wrote:

On 26.06.2015 16:27, Wen Congyang wrote:

At 2015/6/26 21:47, Max Reitz Wrote:

On 25.06.2015 08:41, Wen Congyang wrote:

We can use block job mirror to repair broken quorum files. But the
command
'info block' doesn't output correct filename after block job mirror
finishes.

Which filename? The quorum filename is broken after this patch, too. In

In my test, quorum has two children, s-common.bs-drv is quorum, and 
s-to_replace
is one of his child. Without this patch, info block will output obsolete 
information.
With this patch, the quorum's filename is right. I don't know why quorum 
filename
is broken.

Oh, yes, you are right, I forgot to execute block-job-complete. However, this 
patch relies on the Quorum BDS being the mirror source, which is the 
intentional use case but certainly not the only one:

$ x86_64-softmmu/qemu-system-x86_64 -drive 
if=none,id=quorum,driver=quorum,children.0.file.filename=test1.qcow2,children.1.file.filename=test2.qcow2,children.1.node-name=ch1,vote-threshold=1
 -drive if=none,id=drv,file=test2.qcow2 -qmp stdio
{QMP: {version: {qemu: {micro: 50, minor: 3, major: 2}, package: }, 
capabilities: []}}
{'execute':'qmp_capabilities'}
{return: {}}
{'execute':'drive-mirror','arguments':{'device':'drv','target':'tmp.qcow2','format':'qcow2','node-name':'nch1','replaces':'ch1','sync':'full'}}
Formatting 'tmp.qcow2', fmt=qcow2 size=67108864 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
{return: {}}
{timestamp: {seconds: 1435331363, microseconds: 217707}, event: BLOCK_JOB_READY, data: {device: drv, 
len: 0, offset: 0, speed: 0, type: mirror}}
{'execute':'block-job-complete','arguments':{'device':'drv'}}
{return: {}}
{timestamp: {seconds: 1435331373, microseconds: 152945}, event: BLOCK_JOB_COMPLETED, data: {device: drv, 
len: 0, offset: 0, speed: 0, type: mirror}}
{'execute':'query-block'}
{return: [{device: quorum, locked: false, removable: true, inserted: {iops_rd: 0, detect_zeroes: off, image: {virtual-size: 67108864, filename: json:{\children\: [{\driver\: \qcow2\, \file\: {\driver\: \file\, \filename\: \test1.qcow2\}}, {\driver\: \qcow2\, \file\: {\driver\: \file\, \filename\: \test2.qcow2\}}], \driver\: \quorum\, \blkverify\: false, \rewrite-corrupted\: false, 
\vote-threshold\: 1}, format: quorum}, iops_wr: 0, ro: false, backing_file_depth: 0, drv: quorum, iops: 0, bps_wr: 0, write_threshold: 0, encrypted: false, bps: 0, bps_rd: 0, cache: {no-flush: false, direct: false, writeback: true}, file: json:{\children\: [{\driver\: \qcow2\, \file\: {\driver\: \file\, \filename\: \test1.qcow2\}}, {\driver\: \qcow2\, \file\: {\driver\: 
\file\, \filename\: \test2.qcow2\}}], \driver\: \quorum\, \blkverify\: false,
\rewrite-corrupted\: false, \vote-threshold\: 1}, encryption_key_missing: false}, tray_open: false, type: unknown}, {device: drv, locked: false, removable: true, inserted: {iops_rd: 0, detect_zeroes: off, image: {virtual-size: 67108864, filename: test2.qcow2, cluster-size: 65536, format: qcow2, actual-size: 200704, format-specific: {type: qcow2, data: {compat: 1.1, 
lazy-refcounts: false, refcount-bits: 16, corrupt: false}}, dirty-flag: false}, iops_wr: 0, ro: false, backing_file_depth: 0, drv: qcow2, iops: 0, bps_wr: 0, write_threshold: 0, encrypted: false, bps: 0, bps_rd: 0, cache: {no-flush: false, direct: false, writeback: true}, file: test2.qcow2, encryption_key_missing: false}, tray_open: false, type: unknown}, {io-status: ok, device: 
ide1-cd0, locked: false, removable: true, tray_open: false, type: unknown}, {device: floppy0, locked: fals!

  e,

removable: true, tray_open: false, type: unknown}, {device: sd0, locked: false, removable: 
true, tray_open: false, type: unknown}]}

This patch makes mirror_exit() refresh the name of the block job's device (in this case 
drv), which is not necessarily a parent of the node being replaced.
Even if it were, imagine a configuration where there are two nested quorums, with the 
inner one being repaired using the replaces parameter for drive-mirror.
In this case, this patch would fix the inner Quorum's file name, but not the 
outer one's.

If both inner and outer quorum are BBs, the outer one's is not fixed.


I think this is independent of whether which Quorum has a BB attached to 
it; it's just that unless there is a BB at the outer Quorum BDS, you 
cannot query it with query-block.





I see two solutions to this issue: Either, we do it right and that means, if 
there is a change in the BDS graph (e.g. because of bdrv_swap()),
we have to call bdrv_refresh_filename() on all of the changed BDS's parents. In 
order to be able to enumerate a BDS's parents, we need Kevin's
series, as said before.

IIUC, the BDS can have more than one parent.


Indeed. Kevin's series adds a generalized parent-child relationship, 
which makes it possible to both enumerate all the children of a BDS and 
all its parents.



Or we revive my series 

Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-28 Thread Wen Congyang
On 06/26/2015 11:16 PM, Max Reitz wrote:
 On 26.06.2015 16:27, Wen Congyang wrote:
 At 2015/6/26 21:47, Max Reitz Wrote:
 On 25.06.2015 08:41, Wen Congyang wrote:
 We can use block job mirror to repair broken quorum files. But the
 command
 'info block' doesn't output correct filename after block job mirror
 finishes.

 Which filename? The quorum filename is broken after this patch, too. In

 In my test, quorum has two children, s-common.bs-drv is quorum, and 
 s-to_replace
 is one of his child. Without this patch, info block will output obsolete 
 information.
 With this patch, the quorum's filename is right. I don't know why quorum 
 filename
 is broken.
 
 Oh, yes, you are right, I forgot to execute block-job-complete. However, this 
 patch relies on the Quorum BDS being the mirror source, which is the 
 intentional use case but certainly not the only one:
 
 $ x86_64-softmmu/qemu-system-x86_64 -drive 
 if=none,id=quorum,driver=quorum,children.0.file.filename=test1.qcow2,children.1.file.filename=test2.qcow2,children.1.node-name=ch1,vote-threshold=1
  -drive if=none,id=drv,file=test2.qcow2 -qmp stdio
 {QMP: {version: {qemu: {micro: 50, minor: 3, major: 2}, 
 package: }, capabilities: []}}
 {'execute':'qmp_capabilities'}
 {return: {}}
 {'execute':'drive-mirror','arguments':{'device':'drv','target':'tmp.qcow2','format':'qcow2','node-name':'nch1','replaces':'ch1','sync':'full'}}
 Formatting 'tmp.qcow2', fmt=qcow2 size=67108864 encryption=off 
 cluster_size=65536 lazy_refcounts=off refcount_bits=16
 {return: {}}
 {timestamp: {seconds: 1435331363, microseconds: 217707}, event: 
 BLOCK_JOB_READY, data: {device: drv, len: 0, offset: 0, speed: 
 0, type: mirror}}
 {'execute':'block-job-complete','arguments':{'device':'drv'}}
 {return: {}}
 {timestamp: {seconds: 1435331373, microseconds: 152945}, event: 
 BLOCK_JOB_COMPLETED, data: {device: drv, len: 0, offset: 0, 
 speed: 0, type: mirror}}
 {'execute':'query-block'}
 {return: [{device: quorum, locked: false, removable: true, 
 inserted: {iops_rd: 0, detect_zeroes: off, image: {virtual-size: 
 67108864, filename: json:{\children\: [{\driver\: \qcow2\, \file\: 
 {\driver\: \file\, \filename\: \test1.qcow2\}}, {\driver\: 
 \qcow2\, \file\: {\driver\: \file\, \filename\: \test2.qcow2\}}], 
 \driver\: \quorum\, \blkverify\: false, \rewrite-corrupted\: false, 
 \vote-threshold\: 1}, format: quorum}, iops_wr: 0, ro: false, 
 backing_file_depth: 0, drv: quorum, iops: 0, bps_wr: 0, 
 write_threshold: 0, encrypted: false, bps: 0, bps_rd: 0, cache: 
 {no-flush: false, direct: false, writeback: true}, file: 
 json:{\children\: [{\driver\: \qcow2\, \file\: {\driver\: 
 \file\, \filename\: \test1.qcow2\}}, {\driver\: \qcow2\, \file\: 
 {\driver\: \file\, \filename\: \test2.qcow2\}}], \driver\: 
 \quorum\, \blkverify\: false,
 \rewrite-corrupted\: false, \vote-threshold\: 1}, 
 encryption_key_missing: false}, tray_open: false, type: unknown}, 
 {device: drv, locked: false, removable: true, inserted: {iops_rd: 
 0, detect_zeroes: off, image: {virtual-size: 67108864, filename: 
 test2.qcow2, cluster-size: 65536, format: qcow2, actual-size: 
 200704, format-specific: {type: qcow2, data: {compat: 1.1, 
 lazy-refcounts: false, refcount-bits: 16, corrupt: false}}, 
 dirty-flag: false}, iops_wr: 0, ro: false, backing_file_depth: 0, 
 drv: qcow2, iops: 0, bps_wr: 0, write_threshold: 0, encrypted: 
 false, bps: 0, bps_rd: 0, cache: {no-flush: false, direct: false, 
 writeback: true}, file: test2.qcow2, encryption_key_missing: false}, 
 tray_open: false, type: unknown}, {io-status: ok, device: 
 ide1-cd0, locked: false, removable: true, tray_open: false, type: 
 unknown}, {device: floppy0, locked: false,
 removable: true, tray_open: false, type: unknown}, {device: sd0, 
 locked: false, removable: true, tray_open: false, type: unknown}]}
 
 This patch makes mirror_exit() refresh the name of the block job's device (in 
 this case drv), which is not necessarily a parent of the node being 
 replaced.
 Even if it were, imagine a configuration where there are two nested quorums, 
 with the inner one being repaired using the replaces parameter for 
 drive-mirror.
 In this case, this patch would fix the inner Quorum's file name, but not the 
 outer one's.

If both inner and outer quorum are BBs, the outer one's is not fixed.

 
 I see two solutions to this issue: Either, we do it right and that means, if 
 there is a change in the BDS graph (e.g. because of bdrv_swap()),
 we have to call bdrv_refresh_filename() on all of the changed BDS's parents. 
 In order to be able to enumerate a BDS's parents, we need Kevin's
 series, as said before.

IIUC, the BDS can have more than one parent.

 
 Or we revive my series block: Drop BDS.filename which drops the 
 BDS.filename field completely and always rebuilds it if it is queried.
 This would fix the issue as well, however, there is a reason it has been 
 lying around for nine months now, and that reason is that we did
 not yet fully examine the 

Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-26 Thread Max Reitz

On 25.06.2015 08:41, Wen Congyang wrote:

We can use block job mirror to repair broken quorum files. But the command
'info block' doesn't output correct filename after block job mirror finishes.


Which filename? The quorum filename is broken after this patch, too. In 
order to fix this, we need to call bdrv_refresh_filename() after 
bdrv_swap() on all BDSs which reference one of the swapped BDSs. I think 
this will not be reasonably possible until Kevin's bdrv_reopen() 
overhaul series is merged which introduces a generic parent-child 
relationship for BDSs.


Max


Signed-off-by: Wen Congyang we...@cn.fujitsu.com
---
  block/mirror.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 8aa2b21..2ca2c21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -351,6 +351,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
  bdrv_set_backing_hd(s-base, NULL);
  bdrv_unref(p);
  }
+if (s-to_replace != s-common.bs) {
+bdrv_refresh_filename(s-common.bs);
+}
  }
  if (s-to_replace) {
  bdrv_op_unblock_all(s-to_replace, s-replace_blocker);





Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-26 Thread Wen Congyang

At 2015/6/26 21:47, Max Reitz Wrote:

On 25.06.2015 08:41, Wen Congyang wrote:

We can use block job mirror to repair broken quorum files. But the
command
'info block' doesn't output correct filename after block job mirror
finishes.


Which filename? The quorum filename is broken after this patch, too. In


In my test, quorum has two children, s-common.bs-drv is quorum, and 
s-to_replace
is one of his child. Without this patch, info block will output obsolete 
information.
With this patch, the quorum's filename is right. I don't know why quorum 
filename

is broken.

Thanks
Wen Congyang


order to fix this, we need to call bdrv_refresh_filename() after
bdrv_swap() on all BDSs which reference one of the swapped BDSs. I think
this will not be reasonably possible until Kevin's bdrv_reopen()
overhaul series is merged which introduces a generic parent-child
relationship for BDSs.

Max


Signed-off-by: Wen Congyang we...@cn.fujitsu.com
---
  block/mirror.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 8aa2b21..2ca2c21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -351,6 +351,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
  bdrv_set_backing_hd(s-base, NULL);
  bdrv_unref(p);
  }
+if (s-to_replace != s-common.bs) {
+bdrv_refresh_filename(s-common.bs);
+}
  }
  if (s-to_replace) {
  bdrv_op_unblock_all(s-to_replace, s-replace_blocker);








Re: [Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-26 Thread Max Reitz

On 26.06.2015 16:27, Wen Congyang wrote:

At 2015/6/26 21:47, Max Reitz Wrote:

On 25.06.2015 08:41, Wen Congyang wrote:

We can use block job mirror to repair broken quorum files. But the
command
'info block' doesn't output correct filename after block job mirror
finishes.


Which filename? The quorum filename is broken after this patch, too. In


In my test, quorum has two children, s-common.bs-drv is quorum, and 
s-to_replace
is one of his child. Without this patch, info block will output 
obsolete information.
With this patch, the quorum's filename is right. I don't know why 
quorum filename

is broken.


Oh, yes, you are right, I forgot to execute block-job-complete. However, 
this patch relies on the Quorum BDS being the mirror source, which is 
the intentional use case but certainly not the only one:


$ x86_64-softmmu/qemu-system-x86_64 -drive 
if=none,id=quorum,driver=quorum,children.0.file.filename=test1.qcow2,children.1.file.filename=test2.qcow2,children.1.node-name=ch1,vote-threshold=1 
-drive if=none,id=drv,file=test2.qcow2 -qmp stdio
{QMP: {version: {qemu: {micro: 50, minor: 3, major: 2}, 
package: }, capabilities: []}}

{'execute':'qmp_capabilities'}
{return: {}}
{'execute':'drive-mirror','arguments':{'device':'drv','target':'tmp.qcow2','format':'qcow2','node-name':'nch1','replaces':'ch1','sync':'full'}}
Formatting 'tmp.qcow2', fmt=qcow2 size=67108864 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16

{return: {}}
{timestamp: {seconds: 1435331363, microseconds: 217707}, event: 
BLOCK_JOB_READY, data: {device: drv, len: 0, offset: 0, 
speed: 0, type: mirror}}

{'execute':'block-job-complete','arguments':{'device':'drv'}}
{return: {}}
{timestamp: {seconds: 1435331373, microseconds: 152945}, event: 
BLOCK_JOB_COMPLETED, data: {device: drv, len: 0, offset: 0, 
speed: 0, type: mirror}}

{'execute':'query-block'}
{return: [{device: quorum, locked: false, removable: true, 
inserted: {iops_rd: 0, detect_zeroes: off, image: 
{virtual-size: 67108864, filename: json:{\children\: 
[{\driver\: \qcow2\, \file\: {\driver\: \file\, \filename\: 
\test1.qcow2\}}, {\driver\: \qcow2\, \file\: {\driver\: 
\file\, \filename\: \test2.qcow2\}}], \driver\: \quorum\, 
\blkverify\: false, \rewrite-corrupted\: false, \vote-threshold\: 
1}, format: quorum}, iops_wr: 0, ro: false, 
backing_file_depth: 0, drv: quorum, iops: 0, bps_wr: 0, 
write_threshold: 0, encrypted: false, bps: 0, bps_rd: 0, 
cache: {no-flush: false, direct: false, writeback: true}, 
file: json:{\children\: [{\driver\: \qcow2\, \file\: 
{\driver\: \file\, \filename\: \test1.qcow2\}}, {\driver\: 
\qcow2\, \file\: {\driver\: \file\, \filename\: 
\test2.qcow2\}}], \driver\: \quorum\, \blkverify\: false, 
\rewrite-corrupted\: false, \vote-threshold\: 1}, 
encryption_key_missing: false}, tray_open: false, type: 
unknown}, {device: drv, locked: false, removable: true, 
inserted: {iops_rd: 0, detect_zeroes: off, image: 
{virtual-size: 67108864, filename: test2.qcow2, cluster-size: 
65536, format: qcow2, actual-size: 200704, format-specific: 
{type: qcow2, data: {compat: 1.1, lazy-refcounts: false, 
refcount-bits: 16, corrupt: false}}, dirty-flag: false}, 
iops_wr: 0, ro: false, backing_file_depth: 0, drv: qcow2, 
iops: 0, bps_wr: 0, write_threshold: 0, encrypted: false, bps: 
0, bps_rd: 0, cache: {no-flush: false, direct: false, 
writeback: true}, file: test2.qcow2, encryption_key_missing: 
false}, tray_open: false, type: unknown}, {io-status: ok, 
device: ide1-cd0, locked: false, removable: true, tray_open: 
false, type: unknown}, {device: floppy0, locked: false, 
removable: true, tray_open: false, type: unknown}, {device: 
sd0, locked: false, removable: true, tray_open: false, type: 
unknown}]}


This patch makes mirror_exit() refresh the name of the block job's 
device (in this case drv), which is not necessarily a parent of the 
node being replaced. Even if it were, imagine a configuration where 
there are two nested quorums, with the inner one being repaired using 
the replaces parameter for drive-mirror. In this case, this patch 
would fix the inner Quorum's file name, but not the outer one's.


I see two solutions to this issue: Either, we do it right and that 
means, if there is a change in the BDS graph (e.g. because of 
bdrv_swap()), we have to call bdrv_refresh_filename() on all of the 
changed BDS's parents. In order to be able to enumerate a BDS's parents, 
we need Kevin's series, as said before.


Or we revive my series block: Drop BDS.filename which drops the 
BDS.filename field completely and always rebuilds it if it is queried. 
This would fix the issue as well, however, there is a reason it has been 
lying around for nine months now, and that reason is that we did not yet 
fully examine the impacts of dropping that field, especially concerning 
libvirt.


Max



Thanks
Wen Congyang


order to fix this, we need to call bdrv_refresh_filename() after
bdrv_swap() on all BDSs which reference one of the swapped BDSs. I think

[Qemu-devel] [PATCH] refresh filename after the node is replaced

2015-06-25 Thread Wen Congyang
We can use block job mirror to repair broken quorum files. But the command
'info block' doesn't output correct filename after block job mirror finishes.

Signed-off-by: Wen Congyang we...@cn.fujitsu.com
---
 block/mirror.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 8aa2b21..2ca2c21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -351,6 +351,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
 bdrv_set_backing_hd(s-base, NULL);
 bdrv_unref(p);
 }
+if (s-to_replace != s-common.bs) {
+bdrv_refresh_filename(s-common.bs);
+}
 }
 if (s-to_replace) {
 bdrv_op_unblock_all(s-to_replace, s-replace_blocker);
-- 
2.4.3