Re: [Qemu-devel] scp during migration with vhost fails

2013-02-26 Thread Michael S. Tsirkin
On Tue, Feb 26, 2013 at 02:41:03PM +0800, Jason Wang wrote:
 On 02/25/2013 06:01 PM, Michael S. Tsirkin wrote:
  On Mon, Feb 25, 2013 at 02:11:44PM +0800, Jason Wang wrote:
  On 02/25/2013 01:57 PM, Jason Wang wrote:
  On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
  On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
  On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
  On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
  On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
  On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
  Hello all:
 
  During testing, I find doing scp during migration with vhost 
  fails with 
  warnings in guest like:
 
  Corrupted MAC on input.
  Disconnecting: Packet corrupt.
  lost connection
 
  Here's the bisect result:
 
  Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
  MemoryListener 
  API is the last commit that works well.
 
  With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: 
  convert to 
  MemoryListener API, guest network is unusable with warning of 
  bad gso type
 
  With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
  incorrect 
  userspace address, guest network is available, but scp during 
  migration may 
  fail.
 
  Looks like the issue is related to memory api, any thoughts?
 
  Thanks
  Tried to reproduce this for a while without success.
  Which command line was used?
 
 
  -- 
  MST
  Could be we are not syncing all that we should?
  Does the following hack make the problem go away?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..a7a0412 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct 
  vhost_dev *dev,
  hwaddr end_addr)
   {
   int i;
  +start_addr = 0x0;
  +end_addr = ~0x0ull;
   
   if (!dev-log_enabled || !dev-started) {
   return 0;
 
  Still can reproduce with this. From the bisect result, the vhost 
  dirty
  bitmap sync itself looks ok but something wrong when converting to
  memory listener.
  Reading the code carefully, I found two bugs introduced during
  this conversion. Patch below, could you please try?
 
  vhost: memory sync fixes
  
  This fixes two bugs related to memory sync during
  migration:
  - ram address calculation was missing the chunk
address, so the wrong page was dirtied
  - one after last was used instead of the
end address of a region, which might overflow to 0
and cause us to skip the region when the region ends at
~0x0ull.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
 
  ---
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..dbf6b46 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
  *dev,
   ffsll(log) : ffs(log))) {
   ram_addr_t ram_addr;
   bit -= 1;
  -ram_addr = section-offset_within_region + bit * 
  VHOST_LOG_PAGE;
  +ram_addr = section-offset_within_region + addr + bit * 
  VHOST_LOG_PAGE;
   memory_region_set_dirty(section-mr, ram_addr, 
  VHOST_LOG_PAGE);
   log = ~(0x1ull  bit);
   }
  @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
   struct vhost_dev *dev = container_of(listener, struct vhost_dev,
memory_listener);
   hwaddr start_addr = section-offset_within_address_space;
  -hwaddr end_addr = start_addr + section-size;
  +hwaddr end_addr = start_addr + section-size - 1;
   
   vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
   }
 
  I can still reproduce the issue with this patch.
  Yes it's still wrong. We need the following on top.
  Could you try please?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index dbf6b46..c324903 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
  *dev,
   uint64_t end = MIN(mlast, rlast);
   vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
   vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
  -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
  +uint64_t addr = 0;
   
   if (end  start) {
   return;
  Sorry, scratch that last one, sorry.
  This should be the right thing, I think: on top of
  'vhost: memory sync fixes'.
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index dbf6b46..72c0095 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev 
  *dev,
   log = __sync_fetch_and_and(from, 0);
   while ((bit = sizeof(log)  sizeof(int) ?
   ffsll(log) : ffs(log))) {
  -   

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-26 Thread Jason Wang
On 02/26/2013 04:44 PM, Michael S. Tsirkin wrote:
 On Tue, Feb 26, 2013 at 02:41:03PM +0800, Jason Wang wrote:
 On 02/25/2013 06:01 PM, Michael S. Tsirkin wrote:
 On Mon, Feb 25, 2013 at 02:11:44PM +0800, Jason Wang wrote:
 On 02/25/2013 01:57 PM, Jason Wang wrote:
 On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
 On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
 On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
 On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost 
 fails with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: 
 convert to 
 MemoryListener API, guest network is unusable with warning of 
 bad gso type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
 incorrect 
 userspace address, guest network is available, but scp during 
 migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct 
 vhost_dev *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;

 Still can reproduce with this. From the bisect result, the vhost 
 dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.
 Reading the code carefully, I found two bugs introduced during
 this conversion. Patch below, could you please try?

 vhost: memory sync fixes
 
 This fixes two bugs related to memory sync during
 migration:
 - ram address calculation was missing the chunk
   address, so the wrong page was dirtied
 - one after last was used instead of the
   end address of a region, which might overflow to 0
   and cause us to skip the region when the region ends at
   ~0x0ull.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..dbf6b46 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
 *dev,
  ffsll(log) : ffs(log))) {
  ram_addr_t ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
  memory_region_set_dirty(section-mr, ram_addr, 
 VHOST_LOG_PAGE);
  log = ~(0x1ull  bit);
  }
 @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
   memory_listener);
  hwaddr start_addr = section-offset_within_address_space;
 -hwaddr end_addr = start_addr + section-size;
 +hwaddr end_addr = start_addr + section-size - 1;
  
  vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
  }

 I can still reproduce the issue with this patch.
 Yes it's still wrong. We need the following on top.
 Could you try please?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..c324903 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
 *dev,
  uint64_t end = MIN(mlast, rlast);
  vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
  vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
 -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 +uint64_t addr = 0;
  
  if (end  start) {
  return;
 Sorry, scratch that last one, sorry.
 This should be the right thing, I think: on top of
 'vhost: memory sync fixes'.

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..72c0095 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev 
 *dev,
  log = __sync_fetch_and_and(from, 0);
  while ((bit = sizeof(log)  sizeof(int) ?
  ffsll(log) : ffs(log))) {
 -ram_addr_t ram_addr;
 +hwaddr ram_addr;
  bit -= 1;
 -   

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-25 Thread Michael S. Tsirkin
On Sat, Feb 23, 2013 at 11:54:55PM +0200, Michael S. Tsirkin wrote:
 On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
   On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
Hello all:
   
During testing, I find doing scp during migration with vhost fails 
with 
warnings in guest like:
   
Corrupted MAC on input.
Disconnecting: Packet corrupt.
lost connection
   
Here's the bisect result:
   
Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
MemoryListener 
API is the last commit that works well.
   
With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert 
to 
MemoryListener API, guest network is unusable with warning of bad 
gso type
   
With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
incorrect 
userspace address, guest network is available, but scp during 
migration may 
fail.
   
Looks like the issue is related to memory api, any thoughts?
   
Thanks
Tried to reproduce this for a while without success.
Which command line was used?
   
   
-- 
MST
Could be we are not syncing all that we should?
Does the following hack make the problem go away?
   
diff --git a/hw/vhost.c b/hw/vhost.c
index 8d41fdb..a7a0412 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
*dev,
hwaddr end_addr)
 {
 int i;
+start_addr = 0x0;
+end_addr = ~0x0ull;
 
 if (!dev-log_enabled || !dev-started) {
 return 0;
   
Still can reproduce with this. From the bisect result, the vhost dirty
bitmap sync itself looks ok but something wrong when converting to
memory listener.
Reading the code carefully, I found two bugs introduced during
this conversion. Patch below, could you please try?
   
vhost: memory sync fixes

This fixes two bugs related to memory sync during
migration:
- ram address calculation was missing the chunk
  address, so the wrong page was dirtied
- one after last was used instead of the
  end address of a region, which might overflow to 0
  and cause us to skip the region when the region ends at
  ~0x0ull.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
   
---
   
diff --git a/hw/vhost.c b/hw/vhost.c
index 8d41fdb..dbf6b46 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
*dev,
 ffsll(log) : ffs(log))) {
 ram_addr_t ram_addr;
 bit -= 1;
-ram_addr = section-offset_within_region + bit * 
VHOST_LOG_PAGE;
+ram_addr = section-offset_within_region + addr + bit * 
VHOST_LOG_PAGE;
 memory_region_set_dirty(section-mr, ram_addr, 
VHOST_LOG_PAGE);
 log = ~(0x1ull  bit);
 }
@@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
 struct vhost_dev *dev = container_of(listener, struct vhost_dev,
  memory_listener);
 hwaddr start_addr = section-offset_within_address_space;
-hwaddr end_addr = start_addr + section-size;
+hwaddr end_addr = start_addr + section-size - 1;
 
 vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
 }
   
   
   I can still reproduce the issue with this patch.
  
  
  Yes it's still wrong. We need the following on top.
  Could you try please?
  
  diff --git a/hw/vhost.c b/hw/vhost.c
  index dbf6b46..c324903 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
   uint64_t end = MIN(mlast, rlast);
   vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
   vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
  -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
  +uint64_t addr = 0;
   
   if (end  start) {
   return;
 
 
 Sorry, scratch that last one, sorry.
 This should be the right thing, I think: on top of
 'vhost: memory sync fixes'.
 
 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..72c0095 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  log = __sync_fetch_and_and(from, 0);
  while ((bit = sizeof(log)  sizeof(int) ?
  ffsll(log) : ffs(log))) 

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-25 Thread Michael S. Tsirkin
On Mon, Feb 25, 2013 at 02:11:44PM +0800, Jason Wang wrote:
 On 02/25/2013 01:57 PM, Jason Wang wrote:
  On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
  On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
  On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
  On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
  On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
  On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
  Hello all:
 
  During testing, I find doing scp during migration with vhost fails 
  with 
  warnings in guest like:
 
  Corrupted MAC on input.
  Disconnecting: Packet corrupt.
  lost connection
 
  Here's the bisect result:
 
  Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
  MemoryListener 
  API is the last commit that works well.
 
  With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert 
  to 
  MemoryListener API, guest network is unusable with warning of bad 
  gso type
 
  With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
  incorrect 
  userspace address, guest network is available, but scp during 
  migration may 
  fail.
 
  Looks like the issue is related to memory api, any thoughts?
 
  Thanks
  Tried to reproduce this for a while without success.
  Which command line was used?
 
 
  -- 
  MST
  Could be we are not syncing all that we should?
  Does the following hack make the problem go away?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..a7a0412 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
  *dev,
  hwaddr end_addr)
   {
   int i;
  +start_addr = 0x0;
  +end_addr = ~0x0ull;
   
   if (!dev-log_enabled || !dev-started) {
   return 0;
 
  Still can reproduce with this. From the bisect result, the vhost dirty
  bitmap sync itself looks ok but something wrong when converting to
  memory listener.
  Reading the code carefully, I found two bugs introduced during
  this conversion. Patch below, could you please try?
 
  vhost: memory sync fixes
  
  This fixes two bugs related to memory sync during
  migration:
  - ram address calculation was missing the chunk
address, so the wrong page was dirtied
  - one after last was used instead of the
end address of a region, which might overflow to 0
and cause us to skip the region when the region ends at
~0x0ull.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
 
  ---
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..dbf6b46 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
  *dev,
   ffsll(log) : ffs(log))) {
   ram_addr_t ram_addr;
   bit -= 1;
  -ram_addr = section-offset_within_region + bit * 
  VHOST_LOG_PAGE;
  +ram_addr = section-offset_within_region + addr + bit * 
  VHOST_LOG_PAGE;
   memory_region_set_dirty(section-mr, ram_addr, 
  VHOST_LOG_PAGE);
   log = ~(0x1ull  bit);
   }
  @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
   struct vhost_dev *dev = container_of(listener, struct vhost_dev,
memory_listener);
   hwaddr start_addr = section-offset_within_address_space;
  -hwaddr end_addr = start_addr + section-size;
  +hwaddr end_addr = start_addr + section-size - 1;
   
   vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
   }
 
  I can still reproduce the issue with this patch.
  Yes it's still wrong. We need the following on top.
  Could you try please?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index dbf6b46..c324903 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
   uint64_t end = MIN(mlast, rlast);
   vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
   vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
  -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
  +uint64_t addr = 0;
   
   if (end  start) {
   return;
  Sorry, scratch that last one, sorry.
  This should be the right thing, I think: on top of
  'vhost: memory sync fixes'.
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index dbf6b46..72c0095 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
   log = __sync_fetch_and_and(from, 0);
   while ((bit = sizeof(log)  sizeof(int) ?
   ffsll(log) : ffs(log))) {
  -ram_addr_t ram_addr;
  +hwaddr ram_addr;
   bit -= 1;
  -ram_addr = 

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-25 Thread Jason Wang
On 02/25/2013 06:01 PM, Michael S. Tsirkin wrote:
 On Mon, Feb 25, 2013 at 02:11:44PM +0800, Jason Wang wrote:
 On 02/25/2013 01:57 PM, Jason Wang wrote:
 On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
 On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
 On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
 On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails 
 with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert 
 to 
 MemoryListener API, guest network is unusable with warning of bad 
 gso type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
 incorrect 
 userspace address, guest network is available, but scp during 
 migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
 *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;

 Still can reproduce with this. From the bisect result, the vhost dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.
 Reading the code carefully, I found two bugs introduced during
 this conversion. Patch below, could you please try?

 vhost: memory sync fixes
 
 This fixes two bugs related to memory sync during
 migration:
 - ram address calculation was missing the chunk
   address, so the wrong page was dirtied
 - one after last was used instead of the
   end address of a region, which might overflow to 0
   and cause us to skip the region when the region ends at
   ~0x0ull.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..dbf6b46 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev 
 *dev,
  ffsll(log) : ffs(log))) {
  ram_addr_t ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
  memory_region_set_dirty(section-mr, ram_addr, 
 VHOST_LOG_PAGE);
  log = ~(0x1ull  bit);
  }
 @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
   memory_listener);
  hwaddr start_addr = section-offset_within_address_space;
 -hwaddr end_addr = start_addr + section-size;
 +hwaddr end_addr = start_addr + section-size - 1;
  
  vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
  }

 I can still reproduce the issue with this patch.
 Yes it's still wrong. We need the following on top.
 Could you try please?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..c324903 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  uint64_t end = MIN(mlast, rlast);
  vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
  vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
 -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 +uint64_t addr = 0;
  
  if (end  start) {
  return;
 Sorry, scratch that last one, sorry.
 This should be the right thing, I think: on top of
 'vhost: memory sync fixes'.

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..72c0095 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  log = __sync_fetch_and_and(from, 0);
  while ((bit = sizeof(log)  sizeof(int) ?
  ffsll(log) : ffs(log))) {
 -ram_addr_t ram_addr;
 +hwaddr ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = addr + bit * 

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-24 Thread Jason Wang
On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
 On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
 On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
 On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails 
 with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso 
 type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
 incorrect 
 userspace address, guest network is available, but scp during 
 migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
 *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;

 Still can reproduce with this. From the bisect result, the vhost dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.
 Reading the code carefully, I found two bugs introduced during
 this conversion. Patch below, could you please try?

 vhost: memory sync fixes
 
 This fixes two bugs related to memory sync during
 migration:
 - ram address calculation was missing the chunk
   address, so the wrong page was dirtied
 - one after last was used instead of the
   end address of a region, which might overflow to 0
   and cause us to skip the region when the region ends at
   ~0x0ull.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..dbf6b46 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  ffsll(log) : ffs(log))) {
  ram_addr_t ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
  memory_region_set_dirty(section-mr, ram_addr, 
 VHOST_LOG_PAGE);
  log = ~(0x1ull  bit);
  }
 @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
   memory_listener);
  hwaddr start_addr = section-offset_within_address_space;
 -hwaddr end_addr = start_addr + section-size;
 +hwaddr end_addr = start_addr + section-size - 1;
  
  vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
  }

 I can still reproduce the issue with this patch.

 Yes it's still wrong. We need the following on top.
 Could you try please?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..c324903 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  uint64_t end = MIN(mlast, rlast);
  vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
  vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
 -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 +uint64_t addr = 0;
  
  if (end  start) {
  return;

 Sorry, scratch that last one, sorry.
 This should be the right thing, I think: on top of
 'vhost: memory sync fixes'.

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..72c0095 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  log = __sync_fetch_and_and(from, 0);
  while ((bit = sizeof(log)  sizeof(int) ?
  ffsll(log) : ffs(log))) {
 -ram_addr_t ram_addr;
 +hwaddr ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = addr + bit * VHOST_LOG_PAGE -
 +section-mr-offset_within_address_space;

should be section-offset_within_address_space
  

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-24 Thread Jason Wang
On 02/25/2013 01:57 PM, Jason Wang wrote:
 On 02/24/2013 05:54 AM, Michael S. Tsirkin wrote:
 On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
 On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
 On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails 
 with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert 
 to 
 MemoryListener API, guest network is unusable with warning of bad 
 gso type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
 incorrect 
 userspace address, guest network is available, but scp during 
 migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
 *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;

 Still can reproduce with this. From the bisect result, the vhost dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.
 Reading the code carefully, I found two bugs introduced during
 this conversion. Patch below, could you please try?

 vhost: memory sync fixes
 
 This fixes two bugs related to memory sync during
 migration:
 - ram address calculation was missing the chunk
   address, so the wrong page was dirtied
 - one after last was used instead of the
   end address of a region, which might overflow to 0
   and cause us to skip the region when the region ends at
   ~0x0ull.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..dbf6b46 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  ffsll(log) : ffs(log))) {
  ram_addr_t ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
  memory_region_set_dirty(section-mr, ram_addr, 
 VHOST_LOG_PAGE);
  log = ~(0x1ull  bit);
  }
 @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
   memory_listener);
  hwaddr start_addr = section-offset_within_address_space;
 -hwaddr end_addr = start_addr + section-size;
 +hwaddr end_addr = start_addr + section-size - 1;
  
  vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
  }

 I can still reproduce the issue with this patch.
 Yes it's still wrong. We need the following on top.
 Could you try please?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..c324903 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  uint64_t end = MIN(mlast, rlast);
  vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
  vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
 -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 +uint64_t addr = 0;
  
  if (end  start) {
  return;
 Sorry, scratch that last one, sorry.
 This should be the right thing, I think: on top of
 'vhost: memory sync fixes'.

 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..72c0095 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  log = __sync_fetch_and_and(from, 0);
  while ((bit = sizeof(log)  sizeof(int) ?
  ffsll(log) : ffs(log))) {
 -ram_addr_t ram_addr;
 +hwaddr ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
 +ram_addr = addr + bit * VHOST_LOG_PAGE -
 +section-mr-offset_within_address_space;
 should be 

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-23 Thread Michael S. Tsirkin
On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
 On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
  On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
  On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
  On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
  Hello all:
 
  During testing, I find doing scp during migration with vhost fails with 
  warnings in guest like:
 
  Corrupted MAC on input.
  Disconnecting: Packet corrupt.
  lost connection
 
  Here's the bisect result:
 
  Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
  MemoryListener 
  API is the last commit that works well.
 
  With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
  MemoryListener API, guest network is unusable with warning of bad gso 
  type
 
  With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
  incorrect 
  userspace address, guest network is available, but scp during migration 
  may 
  fail.
 
  Looks like the issue is related to memory api, any thoughts?
 
  Thanks
  Tried to reproduce this for a while without success.
  Which command line was used?
 
 
  -- 
  MST
  Could be we are not syncing all that we should?
  Does the following hack make the problem go away?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..a7a0412 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
  *dev,
  hwaddr end_addr)
   {
   int i;
  +start_addr = 0x0;
  +end_addr = ~0x0ull;
   
   if (!dev-log_enabled || !dev-started) {
   return 0;
 
  Still can reproduce with this. From the bisect result, the vhost dirty
  bitmap sync itself looks ok but something wrong when converting to
  memory listener.
  Reading the code carefully, I found two bugs introduced during
  this conversion. Patch below, could you please try?
 
  vhost: memory sync fixes
  
  This fixes two bugs related to memory sync during
  migration:
  - ram address calculation was missing the chunk
address, so the wrong page was dirtied
  - one after last was used instead of the
end address of a region, which might overflow to 0
and cause us to skip the region when the region ends at
~0x0ull.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
 
  ---
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..dbf6b46 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
   ffsll(log) : ffs(log))) {
   ram_addr_t ram_addr;
   bit -= 1;
  -ram_addr = section-offset_within_region + bit * 
  VHOST_LOG_PAGE;
  +ram_addr = section-offset_within_region + addr + bit * 
  VHOST_LOG_PAGE;
   memory_region_set_dirty(section-mr, ram_addr, VHOST_LOG_PAGE);
   log = ~(0x1ull  bit);
   }
  @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
   struct vhost_dev *dev = container_of(listener, struct vhost_dev,
memory_listener);
   hwaddr start_addr = section-offset_within_address_space;
  -hwaddr end_addr = start_addr + section-size;
  +hwaddr end_addr = start_addr + section-size - 1;
   
   vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
   }
 
 
 I can still reproduce the issue with this patch.


Yes it's still wrong. We need the following on top.
Could you try please?

diff --git a/hw/vhost.c b/hw/vhost.c
index dbf6b46..c324903 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
 uint64_t end = MIN(mlast, rlast);
 vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
 vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
-uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
+uint64_t addr = 0;
 
 if (end  start) {
 return;



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-23 Thread Michael S. Tsirkin
On Sat, Feb 23, 2013 at 10:49:29PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 22, 2013 at 11:33:53PM +0800, Jason Wang wrote:
  On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
   On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
   On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
   On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
   On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
   Hello all:
  
   During testing, I find doing scp during migration with vhost fails 
   with 
   warnings in guest like:
  
   Corrupted MAC on input.
   Disconnecting: Packet corrupt.
   lost connection
  
   Here's the bisect result:
  
   Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
   MemoryListener 
   API is the last commit that works well.
  
   With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert 
   to 
   MemoryListener API, guest network is unusable with warning of bad 
   gso type
  
   With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix 
   incorrect 
   userspace address, guest network is available, but scp during 
   migration may 
   fail.
  
   Looks like the issue is related to memory api, any thoughts?
  
   Thanks
   Tried to reproduce this for a while without success.
   Which command line was used?
  
  
   -- 
   MST
   Could be we are not syncing all that we should?
   Does the following hack make the problem go away?
  
   diff --git a/hw/vhost.c b/hw/vhost.c
   index 8d41fdb..a7a0412 100644
   --- a/hw/vhost.c
   +++ b/hw/vhost.c
   @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev 
   *dev,
   hwaddr end_addr)
{
int i;
   +start_addr = 0x0;
   +end_addr = ~0x0ull;

if (!dev-log_enabled || !dev-started) {
return 0;
  
   Still can reproduce with this. From the bisect result, the vhost dirty
   bitmap sync itself looks ok but something wrong when converting to
   memory listener.
   Reading the code carefully, I found two bugs introduced during
   this conversion. Patch below, could you please try?
  
   vhost: memory sync fixes
   
   This fixes two bugs related to memory sync during
   migration:
   - ram address calculation was missing the chunk
 address, so the wrong page was dirtied
   - one after last was used instead of the
 end address of a region, which might overflow to 0
 and cause us to skip the region when the region ends at
 ~0x0ull.
   
   Signed-off-by: Michael S. Tsirkin m...@redhat.com
  
   ---
  
   diff --git a/hw/vhost.c b/hw/vhost.c
   index 8d41fdb..dbf6b46 100644
   --- a/hw/vhost.c
   +++ b/hw/vhost.c
   @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
ffsll(log) : ffs(log))) {
ram_addr_t ram_addr;
bit -= 1;
   -ram_addr = section-offset_within_region + bit * 
   VHOST_LOG_PAGE;
   +ram_addr = section-offset_within_region + addr + bit * 
   VHOST_LOG_PAGE;
memory_region_set_dirty(section-mr, ram_addr, 
   VHOST_LOG_PAGE);
log = ~(0x1ull  bit);
}
   @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
struct vhost_dev *dev = container_of(listener, struct vhost_dev,
 memory_listener);
hwaddr start_addr = section-offset_within_address_space;
   -hwaddr end_addr = start_addr + section-size;
   +hwaddr end_addr = start_addr + section-size - 1;

vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
}
  
  
  I can still reproduce the issue with this patch.
 
 
 Yes it's still wrong. We need the following on top.
 Could you try please?
 
 diff --git a/hw/vhost.c b/hw/vhost.c
 index dbf6b46..c324903 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -29,7 +29,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  uint64_t end = MIN(mlast, rlast);
  vhost_log_chunk_t *from = dev-log + start / VHOST_LOG_CHUNK;
  vhost_log_chunk_t *to = dev-log + end / VHOST_LOG_CHUNK + 1;
 -uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 +uint64_t addr = 0;
  
  if (end  start) {
  return;


Sorry, scratch that last one, sorry.
This should be the right thing, I think: on top of
'vhost: memory sync fixes'.

diff --git a/hw/vhost.c b/hw/vhost.c
index dbf6b46..72c0095 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -53,9 +53,10 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
 log = __sync_fetch_and_and(from, 0);
 while ((bit = sizeof(log)  sizeof(int) ?
 ffsll(log) : ffs(log))) {
-ram_addr_t ram_addr;
+hwaddr ram_addr;
 bit -= 1;
-ram_addr = section-offset_within_region + addr + bit * 
VHOST_LOG_PAGE;
+ram_addr = addr + bit * 

Re: [Qemu-devel] scp during migration with vhost fails

2013-02-22 Thread Jason Wang
On 02/21/2013 07:23 PM, Michael S. Tsirkin wrote:
 On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso 
 type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
 userspace address, guest network is available, but scp during migration 
 may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;

 Still can reproduce with this. From the bisect result, the vhost dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.
 Reading the code carefully, I found two bugs introduced during
 this conversion. Patch below, could you please try?

 vhost: memory sync fixes
 
 This fixes two bugs related to memory sync during
 migration:
 - ram address calculation was missing the chunk
   address, so the wrong page was dirtied
 - one after last was used instead of the
   end address of a region, which might overflow to 0
   and cause us to skip the region when the region ends at
   ~0x0ull.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

 ---

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..dbf6b46 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
  ffsll(log) : ffs(log))) {
  ram_addr_t ram_addr;
  bit -= 1;
 -ram_addr = section-offset_within_region + bit * VHOST_LOG_PAGE;
 +ram_addr = section-offset_within_region + addr + bit * 
 VHOST_LOG_PAGE;
  memory_region_set_dirty(section-mr, ram_addr, VHOST_LOG_PAGE);
  log = ~(0x1ull  bit);
  }
 @@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
   memory_listener);
  hwaddr start_addr = section-offset_within_address_space;
 -hwaddr end_addr = start_addr + section-size;
 +hwaddr end_addr = start_addr + section-size - 1;
  
  vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
  }


I can still reproduce the issue with this patch.



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-21 Thread Jason Wang
On 02/20/2013 10:23 PM, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
 userspace address, guest network is available, but scp during migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?



I use this simple cli, sometimes, it's not easy to be reproduced.

qemu-system-x86_64 /home/kvm_autotest_root/images/mq.qcow2 -netdev
tap,id=hn0,vhost=on -device
virtio-net-pci,netdev=hn0,mac=00:00:05:00:00:06 -m 2G -enable-kvm -cpu
host -smp 2 -monitor tcp:0:5000,server,nowait -daemonize




Re: [Qemu-devel] scp during migration with vhost fails

2013-02-21 Thread Jason Wang
On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
 On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:

 During testing, I find doing scp during migration with vhost fails with 
 warnings in guest like:

 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection

 Here's the bisect result:

 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.

 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso type

 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
 userspace address, guest network is available, but scp during migration may 
 fail.

 Looks like the issue is related to memory api, any thoughts?

 Thanks
 Tried to reproduce this for a while without success.
 Which command line was used?


 -- 
 MST
 Could be we are not syncing all that we should?
 Does the following hack make the problem go away?

 diff --git a/hw/vhost.c b/hw/vhost.c
 index 8d41fdb..a7a0412 100644
 --- a/hw/vhost.c
 +++ b/hw/vhost.c
 @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
 hwaddr end_addr)
  {
  int i;
 +start_addr = 0x0;
 +end_addr = ~0x0ull;
  
  if (!dev-log_enabled || !dev-started) {
  return 0;


Still can reproduce with this. From the bisect result, the vhost dirty
bitmap sync itself looks ok but something wrong when converting to
memory listener.



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-21 Thread Michael S. Tsirkin
On Thu, Feb 21, 2013 at 05:57:04PM +0800, Jason Wang wrote:
 On 02/21/2013 12:48 AM, Michael S. Tsirkin wrote:
  On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
  On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
  Hello all:
 
  During testing, I find doing scp during migration with vhost fails with 
  warnings in guest like:
 
  Corrupted MAC on input.
  Disconnecting: Packet corrupt.
  lost connection
 
  Here's the bisect result:
 
  Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
  MemoryListener 
  API is the last commit that works well.
 
  With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
  MemoryListener API, guest network is unusable with warning of bad gso 
  type
 
  With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
  userspace address, guest network is available, but scp during migration 
  may 
  fail.
 
  Looks like the issue is related to memory api, any thoughts?
 
  Thanks
  Tried to reproduce this for a while without success.
  Which command line was used?
 
 
  -- 
  MST
  Could be we are not syncing all that we should?
  Does the following hack make the problem go away?
 
  diff --git a/hw/vhost.c b/hw/vhost.c
  index 8d41fdb..a7a0412 100644
  --- a/hw/vhost.c
  +++ b/hw/vhost.c
  @@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
  hwaddr end_addr)
   {
   int i;
  +start_addr = 0x0;
  +end_addr = ~0x0ull;
   
   if (!dev-log_enabled || !dev-started) {
   return 0;
 
 
 Still can reproduce with this. From the bisect result, the vhost dirty
 bitmap sync itself looks ok but something wrong when converting to
 memory listener.

Reading the code carefully, I found two bugs introduced during
this conversion. Patch below, could you please try?

vhost: memory sync fixes

This fixes two bugs related to memory sync during
migration:
- ram address calculation was missing the chunk
  address, so the wrong page was dirtied
- one after last was used instead of the
  end address of a region, which might overflow to 0
  and cause us to skip the region when the region ends at
  ~0x0ull.

Signed-off-by: Michael S. Tsirkin m...@redhat.com

---

diff --git a/hw/vhost.c b/hw/vhost.c
index 8d41fdb..dbf6b46 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -55,7 +55,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
 ffsll(log) : ffs(log))) {
 ram_addr_t ram_addr;
 bit -= 1;
-ram_addr = section-offset_within_region + bit * VHOST_LOG_PAGE;
+ram_addr = section-offset_within_region + addr + bit * 
VHOST_LOG_PAGE;
 memory_region_set_dirty(section-mr, ram_addr, VHOST_LOG_PAGE);
 log = ~(0x1ull  bit);
 }
@@ -94,7 +94,7 @@ static void vhost_log_sync(MemoryListener *listener,
 struct vhost_dev *dev = container_of(listener, struct vhost_dev,
  memory_listener);
 hwaddr start_addr = section-offset_within_address_space;
-hwaddr end_addr = start_addr + section-size;
+hwaddr end_addr = start_addr + section-size - 1;
 
 vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr);
 }



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-20 Thread Michael S. Tsirkin
On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:
 
 During testing, I find doing scp during migration with vhost fails with 
 warnings in guest like:
 
 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection
 
 Here's the bisect result:
 
 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.
 
 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso type
 
 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
 userspace address, guest network is available, but scp during migration may 
 fail.
 
 Looks like the issue is related to memory api, any thoughts?
 
 Thanks

Tried to reproduce this for a while without success.
Which command line was used?


-- 
MST



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-20 Thread Michael S. Tsirkin
On Wed, Feb 20, 2013 at 04:23:52PM +0200, Michael S. Tsirkin wrote:
 On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
  Hello all:
  
  During testing, I find doing scp during migration with vhost fails with 
  warnings in guest like:
  
  Corrupted MAC on input.
  Disconnecting: Packet corrupt.
  lost connection
  
  Here's the bisect result:
  
  Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
  MemoryListener 
  API is the last commit that works well.
  
  With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
  MemoryListener API, guest network is unusable with warning of bad gso type
  
  With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
  userspace address, guest network is available, but scp during migration may 
  fail.
  
  Looks like the issue is related to memory api, any thoughts?
  
  Thanks
 
 Tried to reproduce this for a while without success.
 Which command line was used?
 
 
 -- 
 MST

Could be we are not syncing all that we should?
Does the following hack make the problem go away?

diff --git a/hw/vhost.c b/hw/vhost.c
index 8d41fdb..a7a0412 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -69,6 +69,8 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
hwaddr end_addr)
 {
 int i;
+start_addr = 0x0;
+end_addr = ~0x0ull;
 
 if (!dev-log_enabled || !dev-started) {
 return 0;



Re: [Qemu-devel] scp during migration with vhost fails

2013-02-03 Thread Michael S. Tsirkin
On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
 Hello all:
 
 During testing, I find doing scp during migration with vhost fails with 
 warnings in guest like:
 
 Corrupted MAC on input.
 Disconnecting: Packet corrupt.
 lost connection
 
 Here's the bisect result:
 
 Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
 MemoryListener 
 API is the last commit that works well.
 
 With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
 MemoryListener API, guest network is unusable with warning of bad gso type
 
 With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
 userspace address, guest network is available, but scp during migration may 
 fail.
 
 Looks like the issue is related to memory api, any thoughts?
 
 Thanks

Hmm this is worrying.
The problem doesn't seem to reproduce for me.  One explanation would be
that the memory table is setup incorrectly, if this happens it's very
worrying.
I think a good way to debug would be by dumping the vhost tables before
and after the change, verify that they are identical.

-- 
MST



[Qemu-devel] scp during migration with vhost fails

2013-02-01 Thread Jason Wang
Hello all:

During testing, I find doing scp during migration with vhost fails with 
warnings in guest like:

Corrupted MAC on input.
Disconnecting: Packet corrupt.
lost connection

Here's the bisect result:

Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to MemoryListener 
API is the last commit that works well.

With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
MemoryListener API, guest network is unusable with warning of bad gso type

With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
userspace address, guest network is available, but scp during migration may 
fail.

Looks like the issue is related to memory api, any thoughts?

Thanks