Re: [Qemu-devel] [PATCH] replication: Fix replication open fail

[Xie Changlong]

在 10/25/2017 2:51 PM, Wang Guang 写道:
> replication_child_perm request write
> permissions for all child which will lead bdrv_check_perm fail.
> replication_child_perm() should request write
> permissions only if it is writable itself.
>
> Signed-off-by: Wang Guang <wang.guan...@zte.com.cn>
> Signed-off-by: Wang Yong <wang.yong...@zte.com.cn>

Thanks, this patch fixs the problem in
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03358.html.

Reviewed-by: Xie Changlong <xiechangl...@cmss.chinamobile.com>

> ---
>   block/replication.c | 11 +++
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/block/replication.c b/block/replication.c
> index 3a4e682..1c95d67 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -161,10 +161,13 @@ static void replication_child_perm(BlockDriverState 
> *bs, BdrvChild *c,
>  uint64_t perm, uint64_t shared,
>  uint64_t *nperm, uint64_t *nshared)
>   {
> -*nperm = *nshared = BLK_PERM_CONSISTENT_READ \
> -| BLK_PERM_WRITE \
> -| BLK_PERM_WRITE_UNCHANGED;
> -
> +*nperm = BLK_PERM_CONSISTENT_READ;
> +if ((bs->open_flags & (BDRV_O_INACTIVE | BDRV_O_RDWR)) == BDRV_O_RDWR) {
> +*nperm |= BLK_PERM_WRITE;
> +}
> +*nshared = BLK_PERM_CONSISTENT_READ \
> +   | BLK_PERM_WRITE \
> +   | BLK_PERM_WRITE_UNCHANGED;
>   return;
>   }
>
>

-- 
Thanks
 -Xie

Re: [Qemu-devel] [PATCH 5/6] block: Fix write/resize permissions for inactive images

[Xie Changlong]

在 5/5/2017 12:52 AM, Kevin Wolf 写道:
  
+/* Returns whether the image file can be written to right now */

+bool bdrv_is_writable(BlockDriverState *bs)
+{
+return !bdrv_is_read_only(bs) && !(bs->open_flags & BDRV_O_INACTIVE);
+}
+


This commit use BDRV_O_INACTIVE to judge whether the image file can be 
written or not. But it blocks replication driver on the secondary node. 
For replication in secondary, we must ensure that the whole chain are 
writable:



  ||
  ||.--
  ||| Secondary
  ||'--
  ||
  ||

virtio-blk
 ^
-->  3 NBD   |
  || server  2 filter
  ||^^
  ||||
  ||  Secondary disk <- hidden-disk 5 <- active-disk 4
  |||  backing^   backing
  ||| |
  ||| |
  ||'-'
  ||   drive-backup sync=none 6

The root casue is when we run replication in secondary, vmstate changes 
to RUN_STATE_INMIGRATE, then blockdev_init() sets bdrv_flags |= 
BDRV_O_INACTIVE. So the whole chain become readonly. I've tried on my 
side, but it seems not easy to fix it. I wonder if there is any way to 
bypass this? Any suggestion would be appreciated.


It's very easy to reproduce this scenario:
(gdb) r
Starting program: /root/.xie/qemu-colo/x86_64-softmmu/qemu-system-x86_64 
-boot c -m 2048 -smp 2 -qmp stdio -vnc :0 -name secondary -enable-kvm 
-cpu qemu64,+kvmclock -device piix3-usb-uhci -device usb-tablet -drive 
if=none,id=colo-disk,file.filename=/root/.xie/suse.qcow2.orgin,file.node-name=secondary_disk,driver=qcow2,node-name=sec-qcow2-driver-for-nbd 
-drive 
if=ide,id=active-disk0,node-name=active-disk111,throttling.bps-total=7000,driver=replication,node-name=secondary-replication-driver,mode=secondary,top-id=active-disk0,file.driver=qcow2,file.node-name=active-qcow2-driver,file.file.filename=/mnt/ramfs/active_disk.img,file.file.node-name=active_disk,file.backing.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.node-name=hidden-qcow2-driver,file.backing.file.node-name=hidden_disk,file.backing.backing=colo-disk 
-incoming tcp:0:

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x74801700 (LWP 25252)]
[New Thread 0x74000700 (LWP 25255)]
qemu-system-x86_64: -drive 
if=ide,id=active-disk0,node-name=active-disk111,throttling.bps-total=7000,driver=replication,node-name=secondary-replication-driver,mode=secondary,top-id=active-disk0,file.driver=qcow2,file.node-name=active-qcow2-driver,file.file.filename=/mnt/ramfs/active_disk.img,file.file.node-name=active_disk,file.backing.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.node-name=hidden-qcow2-driver,file.backing.file.node-name=hidden_disk,file.backing.backing=colo-disk: 
Block node is read-only

[Thread 0x74000700 (LWP 25255) exited]
[Thread 0x74801700 (LWP 25252) exited]
[Inferior 1 (process 25248) exited with code 01]
Missing separate debuginfos, use: debuginfo-install 
glib2-2.46.2-4.el7.x86_64 glibc-2.17-157.el7_3.4.x86_64 
libacl-2.2.51-12.el7.x86_64 libattr-2.4.46-12.el7.x86_64 
libgcc-4.8.5-11.el7.x86_64 libgcrypt-1.5.3-13.el7_3.1.x86_64 
libgpg-error-1.12-3.el7.x86_64 libstdc++-4.8.5-11.el7.x86_64 
libuuid-2.23.2-33.el7_3.2.x86_64 openssl-libs-1.0.1e-60.el7_3.1.x86_64 
pixman-0.34.0-1.el7.x86_64 zlib-1.2.7-17.el7.x86_64

(gdb)

--
Thanks
-Xie

Re: [Qemu-devel] [PATCH] file-posix: Clear out first sector in hdev_create

[Xie Changlong]

在 8/10/2017 4:01 PM, Fam Zheng 写道:

People get surprised when, after "qemu-imc create -f raw /dev/sdX", they


s/qemu-imc/qemu-img/


still see qcow2 with "qemu-img info", if previously the bdev had a qcow2


--
Thanks
-Xie

Re: [Qemu-devel] block replication

[Xie Changlong]

在 8/10/2017 8:26 PM, Vladimir Sementsov-Ogievskiy 写道:

09.08.2017 17:11, Vladimir Sementsov-Ogievskiy wrote:

Hi Wen!

I'm trying to understand block/replication code and have a question.

Why should we block the region from intersecting cow requests when 
read? If I understand correctly


regardless of writes to the secondary-disk we have consistent view of 
it through hidden-disk:


Even if we are intersecting with some writes to secondary-disk (and 
corresponding cow-requests), the


data in secondary disk will not be updated until backed up to 
hidden-disk, therefore, for read we have two


options:

1. read old data from secondary-disk (unallocated region in 
hidden-disk means data in secondary-disk is not updated yet)


2. read backed-up data from hidden-disk (data in secondary-disk may be 
already updated but we don't care)


(the whole region to read may consists of parts, corresponding to 1 or 
2, but this should be ok too)


Where am I wrong?


Ok, now I think this is needed to prevent intersecting of writes and 
reads on hidden-disk. If it so, I think it is better to use serializing 


Hi, Vladimir. Pls refer 
https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg03025.html


BTW, wen's email has changed, also CC Zhang Hailiang, Zhang Chen, Li Zhijian


requests
mechanism (just serialize all requests on hidden-disk, and on write wait 
for all intersecting serializing requests, on read wait for intersecting 
serializing writes) - it may require additional
option for BlockDriverState, but it is more generic and more clear than 
export internal backup things to lock disk region. This also can be 
reused for image-fleecing scheme
(which is based on same pattern  [active-disk is backing for temp-disk, 
backup sync=none from active to temp, read from temp])





==

static coroutine_fn int replication_co_readv(BlockDriverState *bs,
 int64_t sector_num,
 int remaining_sectors,
 QEMUIOVector *qiov)
{
BDRVReplicationState *s = bs->opaque;
BdrvChild *child = s->secondary_disk;
BlockJob *job = NULL;
CowRequest req;
int ret;

if (s->mode == REPLICATION_MODE_PRIMARY) {
/* We only use it to forward primary write requests */
return -EIO;
}

ret = replication_get_io_status(s);
if (ret < 0) {
return ret;
}

if (child && child->bs) {
job = child->bs->job;
}

if (job) {
uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE;

backup_wait_for_overlapping_requests(child->bs->job,
 sector_num * 
BDRV_SECTOR_SIZE,

remaining_bytes);
backup_cow_request_begin(, child->bs->job,
 sector_num * BDRV_SECTOR_SIZE,
remaining_bytes);
ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors,
qiov);
backup_cow_request_end();
goto out;
}

ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, qiov);
out:
return replication_return_value(s, ret);
}





--
Thanks
-Xie

Re: [Qemu-devel] About the trace framework

[Xie Changlong]

在 7/11/2017 9:33 AM, Wang Dong 写道:



On 07/10/2017 01:24 PM, Xie Changlong wrote:

在 7/9/2017 5:57 PM, Wang Dong 写道:

Hi,

I am new to QEMU. But I got some problem so that  I want to figure it 
out.


So I try to debug qemu to see what happened.

And I found trace framework. I think this will help me understand the 
point.


So I compiled qemu with option:

## *--enable-trace-backends=simple*

And did as the docs/tracing.txt tell. But when I execute the example 
command in it, nothing just happens.


*./scripts/simpletrace.py trace-events-all trace-xx(my trace file 
produced by qemu)

I am not sure what happened. Just ask this.
Any clue will be appreciated. Thanks in advance.
*



Did you set *trace-event*? I've encountered this issue in the past :)
(qemu) help trace-event
trace-event name on|off -- changes status of a specific trace event
(qemu) info trace-events
yes, I set it. I just did as the docs told. If this does not work or 
misses something, it should be corrected I think.


Have you also tried out the examples in docs? how did you do that? I 


Yes, just follow the docs. No something special.


think I misses some. Could you tell more about
it? Thanks.


You should confirm the *trace-event*s you set are really triggered by 
qemu. Use gdb breakpoint to verify it is a good choice.






--
Thanks
-Xie

Re: [Qemu-devel] About the trace framework

[Xie Changlong]

在 7/9/2017 5:57 PM, Wang Dong 写道:

Hi,

I am new to QEMU. But I got some problem so that  I want to figure it out.

So I try to debug qemu to see what happened.

And I found trace framework. I think this will help me understand the 
point.


So I compiled qemu with option:

## *--enable-trace-backends=simple*

And did as the docs/tracing.txt tell. But when I execute the example 
command in it, nothing just happens.


*./scripts/simpletrace.py trace-events-all trace-xx(my trace file 
produced by qemu)

I am not sure what happened. Just ask this.
Any clue will be appreciated. Thanks in advance.
*



Did you set *trace-event*? I've encountered this issue in the past :)
(qemu) help trace-event
trace-event name on|off -- changes status of a specific trace event
(qemu) info trace-events

--
Thanks
-Xie

Re: [Qemu-devel] [PATCH v3 20/20] block: Make bdrv_is_allocated_above() byte-based

[Xie Changlong]

在 6/28/2017 3:24 AM, Eric Blake 写道:

We are gradually moving away from sector-based interfaces, towards
byte-based.  In the common case, allocation is unlikely to ever use
values that are not naturally sector-aligned, but it is possible
that byte-based values will let us be more precise about allocation
at the end of an unaligned file that can do byte-based access.

Changing the signature of the function to use int64_t *pnum ensures
that the compiler enforces that all callers are updated.  For now,
the io.c layer still assert()s that all callers are sector-aligned,
but that can be relaxed when a later patch implements byte-based
block status.  Therefore, for the most part this patch is just the
addition of scaling at the callers followed by inverse scaling at
bdrv_is_allocated().  But some code, particularly stream_run(),
gets a lot simpler because it no longer has to mess with sectors.

For ease of review, bdrv_is_allocated() was tackled separately.

Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: John Snow <js...@redhat.com>

---
v2: tweak function comments, favor bdrv_getlength() over ->total_sectors
---
  include/block/block.h |  2 +-
  block/commit.c| 20 
  block/io.c| 42 --
  block/mirror.c|  5 -
  block/replication.c   | 17 -


For replication part:

Reviewed-by: Xie Changlong <xiechangl...@cmss.chinamobile.com>


  block/stream.c| 21 +
  qemu-img.c| 10 +++---
  7 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 9b9d87b..13022d5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -428,7 +428,7 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
  int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum);
  int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
-int64_t sector_num, int nb_sectors, int *pnum);
+int64_t offset, int64_t bytes, int64_t *pnum);

  bool bdrv_is_read_only(BlockDriverState *bs);
  bool bdrv_is_writable(BlockDriverState *bs);
diff --git a/block/commit.c b/block/commit.c
index 241aa95..774a8a5 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -146,7 +146,7 @@ static void coroutine_fn commit_run(void *opaque)
  int64_t offset;
  uint64_t delay_ns = 0;
  int ret = 0;
-int n = 0; /* sectors */
+int64_t n = 0; /* bytes */
  void *buf = NULL;
  int bytes_written = 0;
  int64_t base_len;
@@ -171,7 +171,7 @@ static void coroutine_fn commit_run(void *opaque)

  buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);

-for (offset = 0; offset < s->common.len; offset += n * BDRV_SECTOR_SIZE) {
+for (offset = 0; offset < s->common.len; offset += n) {
  bool copy;

  /* Note that even when no rate limit is applied we need to yield
@@ -183,15 +183,12 @@ static void coroutine_fn commit_run(void *opaque)
  }
  /* Copy if allocated above the base */
  ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base),
-  offset / BDRV_SECTOR_SIZE,
-  COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
-  );
+  offset, COMMIT_BUFFER_SIZE, );
  copy = (ret == 1);
-trace_commit_one_iteration(s, offset, n * BDRV_SECTOR_SIZE, ret);
+trace_commit_one_iteration(s, offset, n, ret);
  if (copy) {
-ret = commit_populate(s->top, s->base, offset,
-  n * BDRV_SECTOR_SIZE, buf);
-bytes_written += n * BDRV_SECTOR_SIZE;
+ret = commit_populate(s->top, s->base, offset, n, buf);
+bytes_written += n;
  }
  if (ret < 0) {
  BlockErrorAction action =
@@ -204,11 +201,10 @@ static void coroutine_fn commit_run(void *opaque)
  }
  }
  /* Publish progress */
-s->common.offset += n * BDRV_SECTOR_SIZE;
+s->common.offset += n;

  if (copy && s->common.speed) {
-delay_ns = ratelimit_calculate_delay(>limit,
- n * BDRV_SECTOR_SIZE);
+delay_ns = ratelimit_calculate_delay(>limit, n);
  }
  }

diff --git a/block/io.c b/block/io.c
index 5bbf153..061a162 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1903,54 +1903,52 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState 
*bs, int64_t offset,
  /*
   * Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP]
   *
- * Return true if the given sector is allocated in any image between
- * BASE and TOP (inclusive).  BASE can be

Re: [Qemu-devel] [PATCH v3 15/20] backup: Switch block_backup.h to byte-based

[Xie Changlong]

在 6/28/2017 3:24 AM, Eric Blake 写道:

We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based.  Continue by converting
the public interface to backup jobs (no semantic change), including
a change to CowRequest to track by bytes instead of cluster indices.

Note that this does not change the difference between the public
interface (starting point, and size of the subsequent range) and
the internal interface (starting and end points).

Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: John Snow <js...@redhat.com>

---
v2: change a couple more parameter names
---
  include/block/block_backup.h | 11 +--
  block/backup.c   | 33 -
  block/replication.c  | 12 


Reviewed-by: Xie Changlong <xiechangl...@cmss.chinamobile.com>


  3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/include/block/block_backup.h b/include/block/block_backup.h
index 8a75947..994a3bd 100644
--- a/include/block/block_backup.h
+++ b/include/block/block_backup.h
@@ -21,17 +21,16 @@
  #include "block/block_int.h"

  typedef struct CowRequest {
-int64_t start;
-int64_t end;
+int64_t start_byte;
+int64_t end_byte;
  QLIST_ENTRY(CowRequest) list;
  CoQueue wait_queue; /* coroutines blocked on this request */
  } CowRequest;

-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
-  int nb_sectors);
+void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset,
+  uint64_t bytes);
  void backup_cow_request_begin(CowRequest *req, BlockJob *job,
-  int64_t sector_num,
-  int nb_sectors);
+  int64_t offset, uint64_t bytes);
  void backup_cow_request_end(CowRequest *req);

  void backup_do_checkpoint(BlockJob *job, Error **errp);
diff --git a/block/backup.c b/block/backup.c
index 4e64710..cfbd921 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -55,7 +55,7 @@ static inline int64_t cluster_size_sectors(BackupBlockJob 
*job)

  /* See if in-flight requests overlap and wait for them to complete */
  static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
-   int64_t start,
+   int64_t offset,
 int64_t end)
  {
  CowRequest *req;
@@ -64,7 +64,7 @@ static void coroutine_fn 
wait_for_overlapping_requests(BackupBlockJob *job,
  do {
  retry = false;
  QLIST_FOREACH(req, >inflight_reqs, list) {
-if (end > req->start && start < req->end) {
+if (end > req->start_byte && offset < req->end_byte) {
  qemu_co_queue_wait(>wait_queue, NULL);
  retry = true;
  break;
@@ -75,10 +75,10 @@ static void coroutine_fn 
wait_for_overlapping_requests(BackupBlockJob *job,

  /* Keep track of an in-flight request */
  static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
- int64_t start, int64_t end)
+  int64_t offset, int64_t end)
  {
-req->start = start;
-req->end = end;
+req->start_byte = offset;
+req->end_byte = end;
  qemu_co_queue_init(>wait_queue);
  QLIST_INSERT_HEAD(>inflight_reqs, req, list);
  }
@@ -114,8 +114,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
sector_num * BDRV_SECTOR_SIZE,
nb_sectors * BDRV_SECTOR_SIZE);

-wait_for_overlapping_requests(job, start, end);
-cow_request_begin(_request, job, start, end);
+wait_for_overlapping_requests(job, start * job->cluster_size,
+  end * job->cluster_size);
+cow_request_begin(_request, job, start * job->cluster_size,
+  end * job->cluster_size);

  for (; start < end; start++) {
  if (test_bit(start, job->done_bitmap)) {
@@ -277,32 +279,29 @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
  bitmap_zero(backup_job->done_bitmap, len);
  }

-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
-  int nb_sectors)
+void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset,
+  uint64_t bytes)
  {
  BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
-int64_t sectors_per_cluster = cluster_size_sectors(backup_job);
  int64_t start, end;

  assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP);

-start = sector_num / sectors_per_cluster;

Re: [Qemu-devel] [Question] How can we confirm hot-plug disk succesfully?

[Xie Changlong]

在 6/19/2017 6:49 PM, Kevin Wolf 写道:

'info block' shows nothing, but we can't add drive who's id
is'drive-virtio-disk1' too.

Yes, the old BlockBackend is only fully freed when the guest actually
unplugs the device. Specifically, we would have to free the QemuOpts
in DriveInfo that keeps the ID reserved. Currently, it is only freed
when the BlockBackend is destroyed:

 blockdev_auto_del()
 blk_unref()
 blk_delete()
 drive_info_del()

We can't free the DriveInfo earlier because it's still needed while the
guest device is still alive.

I'm not sure, but it may be possible to free just the QemuOpts in
monitor_remove_blk(), so that the ID can immediately be reused.

Markus, would you know?



Hi, all. Any ideas?


There is a more serious situation, we
could*never*  destory device memory with 'device_del', it's memory
leak to me if the guest os never support hot-plug and the user don't
know this information.

The user sees that they never get a DEVICE_REMOVED event, so in some way
the do know about it.


But the useless memory is always there and no way to free it, although 
we known that.

BTW, is it possible to force destroy the BlockBackend in this situation?

--
Thanks
-Xie

Re: [Qemu-devel] [Question] How can we confirm hot-plug disk succesfully?

[Xie Changlong]

在 6/19/2017 3:27 PM, Kevin Wolf 写道:

Am 18.06.2017 um 09:21 hat Xie Changlong geschrieben:

In device hot-remove scenario, if we don't probe acpiphp module on
the guest, 'device_del' will never emit DEVICE_DELETED event(because
guest will not write to __EJ0) . So we can confirm that hot-remove
failed. But IIUC, there is no event such as DEVICE_ADDED, so
1) How can we confirm hotplug('device_add') successfully?
2) It seems when hot-plug disk, we don't care acpiphp module status
on the guest, am I right?
3) Why there is no DEVICE_ADDED like event?


device_add doesn't need guest cooperation, so it is immediately
completed when the QMP command returns success.



That's what i though too. But I have a question, if we don't proble 
acpiphp on the guest, and execute below commands:


Hot plug:
(qemu) drive_add 0 
file=/resource/changlox/test.raw,id=drive-virtio-disk1,if=none

(qemu) device_add virtio-blk-pci,drive=drive-virtio-disk1,id=virtio-disk1

Hot remove:
(qemu) device_del virtio-disk1
(qemu) drive_del drive-virtio-disk1
(qemu) qom-list /machine/peripheral
type (string)
virtio-disk1 (child)
(qemu) info block
foo (#block122): suse1.qcow2 (qcow2)
Cache mode:   writeback, direct
Backing file: suse.qcow2.orgin (chain depth: 1)

ide1-cd0: [not inserted]
Removable device: not locked, tray closed

floppy0: [not inserted]
Removable device: not locked, tray closed

sd0: [not inserted]
Removable device: not locked, tray closed
(qemu) drive_add 0 
file=/resource/changlox/test.raw,id=drive-virtio-disk1,if=none

Duplicate ID 'drive-virtio-disk1' for drive

'info block' shows nothing, but we can't add drive who's id 
is'drive-virtio-disk1' too。 There is a more serious situation, we could 
*never* destory device memory with 'device_del', it's memory leak to me 
if the guest os never support hot-plug and the user don't know this 
information.





Kevin



--
Thanks
-Xie

Re: [Qemu-devel] [Question] How can we confirm hot-plug disk succesfully?

[Xie Changlong]

Resend

在 6/18/2017 3:21 PM, Xie Changlong 写道:

Hi all

In device hot-remove scenario, if we don't probe acpiphp module on the 
guest, 'device_del' will never emit DEVICE_DELETED event(because guest 
will not write to __EJ0) . So we can confirm that hot-remove failed. But 
IIUC, there is no event such as DEVICE_ADDED, so

1) How can we confirm hotplug('device_add') successfully?
2) It seems when hot-plug disk, we don't care acpiphp module status on 
the guest, am I right?

3) Why there is no DEVICE_ADDED like event?

Any answer would be appreciated, thanks.



--
Thanks
-Xie

[Qemu-devel] [Question] How can we confirm hot-plug disk succesfully?

[Xie Changlong]

Hi all

In device hot-remove scenario, if we don't probe acpiphp module on the 
guest, 'device_del' will never emit DEVICE_DELETED event(because guest 
will not write to __EJ0) . So we can confirm that hot-remove failed. But 
IIUC, there is no event such as DEVICE_ADDED, so

1) How can we confirm hotplug('device_add') successfully?
2) It seems when hot-plug disk, we don't care acpiphp module status on 
the guest, am I right?

3) Why there is no DEVICE_ADDED like event?

Any answer would be appreciated, thanks.

--
Thanks
-Xie

[Qemu-devel] [PATCH] MAINTAINERS: update my email address

[Xie Changlong]

From: Zhang Chen <zhangchen.f...@cn.fujitsu.com>

I'm leaving my job at Fujitsu, this email address will stop working
this week. Update it to one that I will have access to later.

Signed-off-by: Xie Changlong <xiecl.f...@cn.fujitsu.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c60235e..7421387 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1818,7 +1818,7 @@ F: tests/image-fuzzer/
 
 Replication
 M: Wen Congyang <we...@cn.fujitsu.com>
-M: Changlong Xie <xiecl.f...@cn.fujitsu.com>
+M: Xie Changlong <xiechanglon...@gmail.com>
 S: Supported
 F: replication*
 F: block/replication.c
-- 
2.7.4

Re: [Qemu-devel] [PATCH v4 2/6] replication: add shared-disk and shared-disk-id options

[Xie Changlong]

On 04/12/2017 10:05 PM, zhanghailiang wrote:

We use these two options to identify which disk is
shared

Signed-off-by: zhanghailiang 
Signed-off-by: Wen Congyang 
Signed-off-by: Zhang Chen 
---
v4:
- Add proper comment for primary_disk (Stefan)
v2:
- Move g_free(s->shared_disk_id) to the common fail process place (Stefan)
- Fix comments for these two options
---
  block/replication.c  | 43 +--
  qapi/block-core.json | 10 +-
  2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index bf3c395..418b81b 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -25,9 +25,12 @@
  typedef struct BDRVReplicationState {
  ReplicationMode mode;
  int replication_state;
+bool is_shared_disk;
+char *shared_disk_id;
  BdrvChild *active_disk;
  BdrvChild *hidden_disk;
  BdrvChild *secondary_disk;
+BdrvChild *primary_disk;
  char *top_id;
  ReplicationState *rs;
  Error *blocker;
@@ -53,6 +56,9 @@ static void replication_stop(ReplicationState *rs, bool 
failover,
  
  #define REPLICATION_MODE"mode"

  #define REPLICATION_TOP_ID  "top-id"
+#define REPLICATION_SHARED_DISK "shared-disk"
+#define REPLICATION_SHARED_DISK_ID "shared-disk-id"
+
  static QemuOptsList replication_runtime_opts = {
  .name = "replication",
  .head = QTAILQ_HEAD_INITIALIZER(replication_runtime_opts.head),
@@ -65,6 +71,14 @@ static QemuOptsList replication_runtime_opts = {
  .name = REPLICATION_TOP_ID,
  .type = QEMU_OPT_STRING,
  },
+{
+.name = REPLICATION_SHARED_DISK_ID,
+.type = QEMU_OPT_STRING,
+},
+{
+.name = REPLICATION_SHARED_DISK,
+.type = QEMU_OPT_BOOL,
+},
  { /* end of list */ }
  },
  };
@@ -85,6 +99,9 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
  QemuOpts *opts = NULL;
  const char *mode;
  const char *top_id;
+const char *shared_disk_id;
+BlockBackend *blk;
+BlockDriverState *tmp_bs;
  
  bs->file = bdrv_open_child(NULL, options, "file", bs, _file,

 false, errp);
@@ -125,12 +142,33 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
 "The option mode's value should be primary or secondary");
  goto fail;
  }
+s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
+


What If secondary side is supplied with 'REPLICATION_SHARED_DISK_ID'? 
Pls refer f4f2539bc to pefect the logical.

 false);
+if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
+shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
+if (!shared_disk_id) {
+error_setg(_err, "Missing shared disk blk option");
+goto fail;
+}
+s->shared_disk_id = g_strdup(shared_disk_id);
+blk = blk_by_name(s->shared_disk_id);
+if (!blk) {
+error_setg(_err, "There is no %s block", s->shared_disk_id);
+goto fail;
+}
+/* We have a BlockBackend for the primary disk but use BdrvChild for
+ * consistency - active_disk, secondary_disk, etc are also BdrvChild.
+ */
+tmp_bs = blk_bs(blk);
+s->primary_disk = QLIST_FIRST(_bs->parents);
+}
  
  s->rs = replication_new(bs, _ops);
  
-ret = 0;

-
+qemu_opts_del(opts);
+return 0;
  fail:
+g_free(s->shared_disk_id);
  qemu_opts_del(opts);
  error_propagate(errp, local_err);
  
@@ -141,6 +179,7 @@ static void replication_close(BlockDriverState *bs)

  {
  BDRVReplicationState *s = bs->opaque;
  
+g_free(s->shared_disk_id);

  if (s->replication_state == BLOCK_REPLICATION_RUNNING) {
  replication_stop(s->rs, false, NULL);
  }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 033457c..361c932 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2661,12 +2661,20 @@
  #  node who owns the replication node chain. Must not be given in
  #  primary mode.
  #
+# @shared-disk-id: Id of shared disk while is replication mode, if @shared-disk
+#  is true, this option is required (Since: 2.10)
+#

Further explanations:

For @shared-disk-id, it must/only be given when @shared-disk enable on
Primary side.


+# @shared-disk: To indicate whether or not a disk is shared by primary VM
+#   and secondary VM. (The default is false) (Since: 2.10)
+#

Further explanations:

For @shared-disk, it must be given or not-given on both side at the same 
time.



  # Since: 2.9
  ##
  { 'struct': 'BlockdevOptionsReplication',
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
-

Re: [Qemu-devel] [PULL 2/8] replication: clarify permissions

[Xie Changlong]

On 04/18/2017 09:36 AM, Hailiang Zhang wrote:

On 2017/4/18 9:23, Eric Blake wrote:

On 03/17/2017 08:15 AM, Kevin Wolf wrote:

From: Changlong Xie 

Even if hidden_disk, secondary_disk are backing files, they all need
write permissions in replication scenario. Otherwise we will encouter
below exceptions on secondary side during adding nbd server:

{'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk', 
'writable': true } }
{"error": {"class": "GenericError", "desc": "Conflicts with use by 
hidden-qcow2-driver as 'backing', which does not allow 'write' on 
sec-qcow2-driver-for-nbd"}}


CC: Zhang Hailiang 
CC: Zhang Chen 
CC: Wen Congyang 

This address for Wen Congyang is different than the one listed in
MAINTAINERS for replication (M: Wen Congyang ),
and different still from addresses my mailer has harvested from other
posts (wencongy...@gmail.com).  The MAINTAINERS entry is now resulting
in 'undelivered mail' bounce messages, can you please submit an update
to MAINTAINERS with your new preferred address? [or gently correct me if
I'm confusing two people with the same name?]



No, the same people, he just left his job from fujitsu, the entry in 
MAINTAINERS

file needs to be updated.

Cc: Changlong Xie 
Hi Changlong, would you please send a patch to update it ?

Hi all

After a short talk with Wen, i would like to update his new email 
address now.


Thanks
-Xie


Hailiang




.