All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wen Congyang <wency@cn.fujitsu.com>
To: Eric Blake <eblake@redhat.com>,
	qemu devel <qemu-devel@nongnu.org>, Fam Zheng <famz@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	zhanghailiang <zhang.zhanghailiang@huawei.com>,
	qemu block <qemu-block@nongnu.org>,
	Jiang Yunhong <yunhong.jiang@intel.com>,
	Dong Eddie <eddie.dong@intel.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Michael R. Hines" <mrhines@linux.vnet.ibm.com>,
	Gonglei <arei.gonglei@huawei.com>,
	Yang Hongyang <yanghy@cn.fujitsu.com>
Subject: Re: [Qemu-devel] [PATCH v10 01/10] allow writing to the backing file
Date: Fri, 30 Oct 2015 13:49:01 +0800	[thread overview]
Message-ID: <563304CD.8040201@cn.fujitsu.com> (raw)
In-Reply-To: <56181072.1020805@redhat.com>

On 10/10/2015 03:07 AM, Eric Blake wrote:
> On 09/25/2015 12:17 AM, Wen Congyang wrote:
>> For block replication, we have such backing chain:
>>     secondary disk <-- hidden disk <-- active disk
>> secondary disk is top BDS(use bacing reference), so it can be opened in
> 
> s/BDS(use bacing/BDS (use backing/
> 
>> read-write mode. But hidden disk is read only, and we need to write to
>> hidden disk(backup job will write data to it).
> 
> s/disk(/disk (/
> 
>>
>> TODO: support opening backing file in read-write mode if the BDS is
>> created by QMP command blockdev-add.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  block.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 40 insertions(+), 1 deletion(-)
>>
> 
> I really don't like this patch.  We are able to automatically (re-)open
> backing files for write during block-commit, without having to expose a
> knob to the user then, so exposing a knob to the user here feels wrong.

I try to reopen the backing files for write when starting block replication.
I tested it, and it doesn't work.
Here is my usage:
command line:
-drive if=none,id=colo-disk1,file.filename=/data/images/kvm/suse/temp.img,driver=raw
-drive if=virtio,id=active-disk1,driver=replication,mode=secondary,file.driver=qcow2,file.file.filename=/mnt/ramfs/active_disk.img,file.backing.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.backing.file.filename=/data/images/kvm/suse/suse11_3.img,file.backing.backing.driver=raw,file.backing.backing.node-name=sdisk

{'execute': 'blockdev-remove-medium', 'arguments': {'device': 'colo-disk1'} }
{'execute': 'blockdev-insert-medium', 'arguments': {'device': 'colo-disk1', 'node-name': 'sdisk'} }
{'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet', 'data': {'host': '192.168.3.1', 'port': '8889'} } } }
{'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk1', 'writable': true } }

All qmp command success, but the disk exported to nbd server is readonly even if I specify 
'writable': true in the QMP commad. The reason is that the BDS is readonly.

> 
>> +#define ALLOW_WRITE_BACKING_FILE    "allow-write-backing-file"
>> +static QemuOptsList backing_file_opts = {
>> +    .name = "backing_file",
>> +    .head = QTAILQ_HEAD_INITIALIZER(backing_file_opts.head),
>> +    .desc = {
>> +        {
>> +            .name = ALLOW_WRITE_BACKING_FILE,
>> +            .type = QEMU_OPT_BOOL,
>> +            .help = "allow writes to backing file",
>> +        },
> 
> And even if we DO need this knob (which I doubt), you need corresponding
> documentation of the knob in qapi/block-core.json, since we are trying
> to keep the command line and QMP in sync when it comes to adding new
> options.
> 

Yes, I know it, but I don't know how to do it. Update BlockdevOptions?

Thanks
Wen Congyang

  reply	other threads:[~2015-10-30  5:50 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-25  6:17 [Qemu-devel] [PATCH v10 00/10] Block replication for continuous checkpoints Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 01/10] allow writing to the backing file Wen Congyang
2015-10-09 19:07   ` Eric Blake
2015-10-30  5:49     ` Wen Congyang [this message]
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 02/10] Backup: clear all bitmap when doing block checkpoint Wen Congyang
2015-10-09 19:09   ` Eric Blake
2015-10-12 13:45   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-10-13  9:13     ` Wen Congyang
2015-10-14 14:12       ` Stefan Hajnoczi
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 03/10] Allow creating backup jobs when opening BDS Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 04/10] block: make bdrv_put_ref_bh_schedule() as a public API Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 05/10] docs: block replication's description Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 06/10] Add new block driver interfaces to control block replication Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 07/10] quorum: implement block driver interfaces for " Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 08/10] Implement new driver " Wen Congyang
2015-10-12 16:25   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-10-13  8:59     ` Wen Congyang
2015-10-13  9:41       ` Fam Zheng
2015-10-13  9:46         ` Wen Congyang
2015-10-13 10:12           ` Fam Zheng
2015-10-14  0:55             ` Wen Congyang
2015-10-12 16:27   ` Stefan Hajnoczi
2015-10-13  9:08     ` Wen Congyang
2015-10-14 14:27       ` Stefan Hajnoczi
2015-10-15  2:19         ` Wen Congyang
2015-10-15 14:55           ` Stefan Hajnoczi
2015-10-16  0:52             ` Wen Congyang
2015-10-16  2:22             ` Wen Congyang
2015-10-16 11:37               ` Stefan Hajnoczi
2015-10-27  2:57                 ` Wen Congyang
2015-10-27 14:41                   ` Stefan Hajnoczi
2015-10-12 16:31   ` Stefan Hajnoczi
2015-10-13  9:09     ` Wen Congyang
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 09/10] support replication driver in blockdev-add Wen Congyang
2015-10-09 14:40   ` Eric Blake
2015-09-25  6:17 ` [Qemu-devel] [PATCH v10 10/10] Add a new API to start/stop replication, do checkpoint to all BDSes Wen Congyang
2015-10-07  6:39 ` [Qemu-devel] [PATCH v10 00/10] Block replication for continuous checkpoints Wen Congyang
2015-10-09 12:52   ` Stefan Hajnoczi

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=563304CD.8040201@cn.fujitsu.com \
    --to=wency@cn.fujitsu.com \
    --cc=arei.gonglei@huawei.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=eddie.dong@intel.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=mrhines@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=yanghy@cn.fujitsu.com \
    --cc=yunhong.jiang@intel.com \
    --cc=zhang.zhanghailiang@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.