[Qemu-devel] MultiThread TCG mail list

2014-12-03 Thread Mark Burton

All - to make things easier to track, there is now a mail list specifically for 
MultiThread development issues
mt...@listserver.greensocs.com 
You can subscribe etc here:
http://listserver.greensocs.com/wws/info/mttcg 
http://listserver.greensocs.com/wws/info/mttcg

If you send to this mail list, please make sure to copy qemu-devel as well.


Cheers

Mark.





Re: [Qemu-devel] advent calendar

2014-12-03 Thread Paolo Bonzini


On 03/12/2014 04:22, Paul Ford wrote:
 Love the advent calendar! Wonderful idea.
 
 One tiny note: On a Mac with qemu installed via the brew installer I
 don't have access to kvm, so to run the Slackware image I had to remove
 the kvm command-line option from the Run shell script.
 
 It was all a little new to me (not a regular qemu user) so it might be
 worth flagging or making two Run scripts, one for mac and one for
 everyone else, or adding a README?

We can use -machine accel=kvm:tcg instead of --enable-kvm.

Thanks for the report!

Paolo



Re: [Qemu-devel] [PATCH for-2.3 v2 3/6] vmdk: Clean up descriptor file reading

2014-12-03 Thread Markus Armbruster
Fam Zheng f...@redhat.com writes:

 Zeroing a buffer that will be filled right after is not necessary, and
 allocating a power of two + 1 is naughty.

 Suggested-by: Markus Armbruster arm...@redhat.com
 Signed-off-by: Fam Zheng f...@redhat.com
 ---
  block/vmdk.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/block/vmdk.c b/block/vmdk.c
 index 28d22db..e863a09 100644
 --- a/block/vmdk.c
 +++ b/block/vmdk.c
 @@ -558,14 +558,15 @@ static char *vmdk_read_desc(BlockDriverState *file, 
 uint64_t desc_offset,
  }
  
   size = bdrv_getlength(file);
   if (size  0) {
   error_setg_errno(errp, -size, Could not access file);
   return NULL;
   }

  size = MIN(size, 1  20);  /* avoid unbounded allocation */

Consider the case where size = 1  20, i.e. this line is a no-op.

 -buf = g_malloc0(size + 1);
 +buf = g_malloc(size);
  
 -ret = bdrv_pread(file, desc_offset, buf, size);
 +ret = bdrv_pread(file, desc_offset, buf, size - 1);

Then this reads everything except the last byte (thanks to Don for
spotting it).

  if (ret  0) {
  error_setg_errno(errp, -ret, Could not read from file);
  g_free(buf);
  return NULL;
  }
 +buf[ret] = 0;
  
  return buf;
  }

I figure Don suggested this instead:

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..b7feb15 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -556,8 +556,8 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 return NULL;
 }
 
-size = MIN(size, 1  20);  /* avoid unbounded allocation */
-buf = g_malloc0(size + 1);
+size = MIN(size, (1  20) - 1);  /* avoid unbounded allocation */
+buf = g_malloc(size + 1);
 
 ret = bdrv_pread(file, desc_offset, buf, size);
 if (ret  0) {
@@ -565,6 +565,7 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 g_free(buf);
 return NULL;
 }
+buf[ret] = 0;
 
 return buf;
 }



Re: [Qemu-devel] [PATCH] qemu-iotests: Skip 099 for VMDK subformats with desc file

2014-12-03 Thread Max Reitz

On 2014-12-03 at 02:49, Fam Zheng wrote:

VMDK extent parsing code doesn't handle the JSON file name, so the case
fails for these subformats. Disabled them.

Signed-off-by: Fam Zheng f...@redhat.com
---
  tests/qemu-iotests/099 | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index ffc7ea7..fb920fe 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -44,7 +44,8 @@ trap _cleanup; exit \$status 0 1 2 3 15
  _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc
  _supported_proto file
  _supported_os Linux
-
+_unsupported_imgopts subformat=monolithicFlat subformat=twoGbMaxExtentFlat 
\
+subformat=twoGbMaxExtentSparse
  
  function do_run_qemu()

  {


This is most certainly necessary, therefore: Thanks, applied to my 
block-next tree:


https://github.com/XanClic/qemu/commits/block-next

However, as I said in our private email exchange, we need to fix the 
error message, too.


$ qemu-img create -f vmdk -o subformat=monolithicFlat test.vmdk 64M
Formatting 'test.vmdk', fmt=vmdk size=67108864 compat6=off 
subformat='monolithicFlat'
$ qemu-io -c 'open -o 
file.driver=blkdebug,file.image.filename=test.vmdk,file.inject-error.0.event=read_aio' 


qemu-io: can't open: Could not parse the JSON options

That's not very helpful. I'll send a patch for that.

Max



Re: [Qemu-devel] KVM call for agenda for 2014-12-08

2014-12-03 Thread Mark Burton
Hi Juan, is this for the 9th, or did I get the day wrong

Anyway - I would like to talk about Multi-core - a huge thank you to everybody 
for your feedback, we’ll be starting work on this, and I’d like to bring a 
proposal in terms of the path we’ll take and get consensus on the first steps.

Cheers
Mark.

 On 2 Dec 2014, at 20:56, Juan Quintela quint...@redhat.com wrote:
 
 
 Hi
 
 Please, send any topic that you are interested in covering.
 
 hanks, Juan.
 
 By popular demand, a google calendar public entry with it
 
 https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ
 
 (Let me know if you have any problems with the calendar entry)
 
 If you need phone number details,  contact me privately
 
 Thanks, Juan.
 
 PD.  Use the google calendar entry to now the time, I gave up at getting
 three timezones right.
 


 +44 (0)20 7100 3485 x 210
 +33 (0)5 33 52 01 77x 210

+33 (0)603762104
mark.burton




Re: [Qemu-devel] [PATCH RESCEND v2] target-i386: Intel xsaves

2014-12-03 Thread Paolo Bonzini


On 03/12/2014 03:36, Wanpeng Li wrote:
 Add xsaves related definition, it also adds corresponding part 
 to kvm_get/put, and vmstate.
 
 Signed-off-by: Wanpeng Li wanpeng...@linux.intel.com
 ---
 v1 - v2:
  * use a subsection instead of bumping the version number.
 
  target-i386/cpu.h |  2 ++
  target-i386/kvm.c | 15 +++
  target-i386/machine.c | 21 +
  3 files changed, 38 insertions(+)
 
 diff --git a/target-i386/cpu.h b/target-i386/cpu.h
 index 015f5b5..cff7433 100644
 --- a/target-i386/cpu.h
 +++ b/target-i386/cpu.h
 @@ -389,6 +389,7 @@
  #define MSR_VM_HSAVE_PA 0xc0010117
  
  #define MSR_IA32_BNDCFGS0x0d90
 +#define MSR_IA32_XSS0x0da0
  
  #define XSTATE_FP   (1ULL  0)
  #define XSTATE_SSE  (1ULL  1)
 @@ -1019,6 +1020,7 @@ typedef struct CPUX86State {
  uint64_t xstate_bv;
  
  uint64_t xcr0;
 +uint64_t xss;
  
  TPRAccess tpr_access_type;
  } CPUX86State;
 diff --git a/target-i386/kvm.c b/target-i386/kvm.c
 index ccf36e8..c6fc417 100644
 --- a/target-i386/kvm.c
 +++ b/target-i386/kvm.c
 @@ -80,6 +80,7 @@ static bool has_msr_hv_hypercall;
  static bool has_msr_hv_vapic;
  static bool has_msr_hv_tsc;
  static bool has_msr_mtrr;
 +static bool has_msr_xss;
  
  static bool has_msr_architectural_pmu;
  static uint32_t num_architectural_pmu_counters;
 @@ -826,6 +827,10 @@ static int kvm_get_supported_msrs(KVMState *s)
  has_msr_bndcfgs = true;
  continue;
  }
 +if (kvm_msr_list-indices[i] == MSR_IA32_XSS) {
 +has_msr_xss = true;
 +continue;
 +}
  }
  }
  
 @@ -1224,6 +1229,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
  if (has_msr_bndcfgs) {
  kvm_msr_entry_set(msrs[n++], MSR_IA32_BNDCFGS, env-msr_bndcfgs);
  }
 +if (has_msr_xss) {
 +kvm_msr_entry_set(msrs[n++], MSR_IA32_XSS, env-xss);
 +}
  #ifdef TARGET_X86_64
  if (lm_capable_kernel) {
  kvm_msr_entry_set(msrs[n++], MSR_CSTAR, env-cstar);
 @@ -1570,6 +1578,10 @@ static int kvm_get_msrs(X86CPU *cpu)
  if (has_msr_bndcfgs) {
  msrs[n++].index = MSR_IA32_BNDCFGS;
  }
 +if (has_msr_xss) {
 +msrs[n++].index = MSR_IA32_XSS;
 +}
 +
  
  if (!env-tsc_valid) {
  msrs[n++].index = MSR_IA32_TSC;
 @@ -1717,6 +1729,9 @@ static int kvm_get_msrs(X86CPU *cpu)
  case MSR_IA32_BNDCFGS:
  env-msr_bndcfgs = msrs[i].data;
  break;
 +case MSR_IA32_XSS:
 +env-xss = msrs[i].data;
 +break;
  default:
  if (msrs[i].index = MSR_MC0_CTL 
  msrs[i].index  MSR_MC0_CTL + (env-mcg_cap  0xff) * 4) {
 diff --git a/target-i386/machine.c b/target-i386/machine.c
 index 1c13b14..722d62e 100644
 --- a/target-i386/machine.c
 +++ b/target-i386/machine.c
 @@ -687,6 +687,24 @@ static const VMStateDescription vmstate_avx512 = {
  }
  };
  
 +static bool xss_needed(void *opaque)
 +{
 +X86CPU *cpu = opaque;
 +CPUX86State *env = cpu-env;
 +
 +return env-xss != 0;
 +}
 +
 +static const VMStateDescription vmstate_xss = {
 +.name = cpu/xss,
 +.version_id = 1,
 +.minimum_version_id = 1,
 +.fields = (VMStateField[]) {
 +VMSTATE_UINT64(env.xss, X86CPU),
 +VMSTATE_END_OF_LIST()
 +}
 +};
 +
  VMStateDescription vmstate_x86_cpu = {
  .name = cpu,
  .version_id = 12,
 @@ -832,6 +850,9 @@ VMStateDescription vmstate_x86_cpu = {
  }, {
  .vmsd = vmstate_avx512,
  .needed = avx512_needed,
 + }, {
 +.vmsd = vmstate_xss,
 +.needed = xss_needed,
  } , {
  /* empty */
  }
 

Thanks, applied to uq/master.

Paolo



[Qemu-devel] [PATCH] iotests: Specify qcow2 format for qemu-io in 059

2014-12-03 Thread Max Reitz
There are two instances of iotest 059 using qemu-io on a qcow2 image. As
of qemu-iotests: Use qemu-io -f $IMGFMT the iotests can no longer rely
on $QEMU_IO doing probing, therefore the qcow2 format has to be
specified explicitly here.

Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/059 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 3c053c2..2ed1a7f 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -106,8 +106,8 @@ _img_info
 echo
 echo === Converting to streamOptimized from image with small cluster size===
 TEST_IMG=$TEST_IMG.qcow2 IMGFMT=qcow2 IMGOPTS=cluster_size=4096 
_make_test_img 1G
-$QEMU_IO -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
-$QEMU_IO -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | _filter_qemu_io
+$QEMU_IO -f qcow2 -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
+$QEMU_IO -f qcow2 -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | 
_filter_qemu_io
 $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
$TEST_IMG.qcow2 $TEST_IMG 21
 
 echo
-- 
1.9.3




Re: [Qemu-devel] [PATCH for-2.3 v2 3/6] vmdk: Clean up descriptor file reading

2014-12-03 Thread Fam Zheng
On Wed, 12/03 09:21, Markus Armbruster wrote:
 Fam Zheng f...@redhat.com writes:
 
  Zeroing a buffer that will be filled right after is not necessary, and
  allocating a power of two + 1 is naughty.
 
  Suggested-by: Markus Armbruster arm...@redhat.com
  Signed-off-by: Fam Zheng f...@redhat.com
  ---
   block/vmdk.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)
 
  diff --git a/block/vmdk.c b/block/vmdk.c
  index 28d22db..e863a09 100644
  --- a/block/vmdk.c
  +++ b/block/vmdk.c
  @@ -558,14 +558,15 @@ static char *vmdk_read_desc(BlockDriverState *file, 
  uint64_t desc_offset,
   }
   
size = bdrv_getlength(file);
if (size  0) {
error_setg_errno(errp, -size, Could not access file);
return NULL;
}
 
   size = MIN(size, 1  20);  /* avoid unbounded allocation */
 
 Consider the case where size = 1  20, i.e. this line is a no-op.
 
  -buf = g_malloc0(size + 1);
  +buf = g_malloc(size);
   
  -ret = bdrv_pread(file, desc_offset, buf, size);
  +ret = bdrv_pread(file, desc_offset, buf, size - 1);
 
 Then this reads everything except the last byte (thanks to Don for
 spotting it).

Yes, I was wrong.

 
   if (ret  0) {
   error_setg_errno(errp, -ret, Could not read from file);
   g_free(buf);
   return NULL;
   }
  +buf[ret] = 0;
   
   return buf;
   }
 
 I figure Don suggested this instead:

Yes. Thanks.

Fam

 
 diff --git a/block/vmdk.c b/block/vmdk.c
 index 2cbfd3e..b7feb15 100644
 --- a/block/vmdk.c
 +++ b/block/vmdk.c
 @@ -556,8 +556,8 @@ static char *vmdk_read_desc(BlockDriverState *file, 
 uint64_t desc_offset,
  return NULL;
  }
  
 -size = MIN(size, 1  20);  /* avoid unbounded allocation */
 -buf = g_malloc0(size + 1);
 +size = MIN(size, (1  20) - 1);  /* avoid unbounded allocation */
 +buf = g_malloc(size + 1);
  
  ret = bdrv_pread(file, desc_offset, buf, size);
  if (ret  0) {
 @@ -565,6 +565,7 @@ static char *vmdk_read_desc(BlockDriverState *file, 
 uint64_t desc_offset,
  g_free(buf);
  return NULL;
  }
 +buf[ret] = 0;
  
  return buf;
  }



[Qemu-devel] [PATCH 2/2] iotests: Add test for vmdk JSON file names

2014-12-03 Thread Max Reitz
Add a test for vmdk files which use a file with a JSON file name, and
which then try to open extents. That should fail and the error message
should at least try to look helpful.

Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/059 | 6 ++
 tests/qemu-iotests/059.out | 4 
 2 files changed, 10 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 2ed1a7f..50ca5ce 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -111,6 +111,12 @@ $QEMU_IO -f qcow2 -c write -P 0xb 10240 512 
$TEST_IMG.qcow2 | _filter_qemu_i
 $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
$TEST_IMG.qcow2 $TEST_IMG 21
 
 echo
+echo === Testing monolithicFlat with internally generated JSON file name ===
+IMGOPTS=subformat=monolithicFlat _make_test_img 64M
+$QEMU_IO -c open -o 
driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio
 21 \
+| _filter_testdir | _filter_imgfmt
+
+echo
 echo === Testing version 3 ===
 _use_sample_img iotest-version3.vmdk.bz2
 _img_info
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 0dadba6..97509ab 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2053,6 +2053,10 @@ wrote 512/512 bytes at offset 0
 wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+=== Testing monolithicFlat with internally generated JSON file name ===
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-io: can't open: Cannot use extents with VMDK descriptor file 
'json:{image: {driver: file, filename: TEST_DIR/t.IMGFMT}, driver: 
blkdebug, inject-error.0.event: read_aio}'
+
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
 file format: IMGFMT
-- 
1.9.3




[Qemu-devel] [PATCH 1/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Max Reitz
If vmdk blindly tries to use path_combine() using bs-file-filename as
the base file name, this will result in a bad error message for JSON
file names when calling bdrv_open(). It is better to only try
bs-file-exact_filename; if that is empty, bs-file-filename will be
useless for path_combine() and an error should be emitted (containing
bs-file-filename because bs-file-exact_filename is empty).

Note that s-create_type does not need to be freed on error because it
will be freed by the caller (which ultimately is vmdk_open()).

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/vmdk.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..fe549c2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -894,7 +894,14 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags, char *buf,
 }
 s-create_type = g_strdup(ct);
 s-desc_offset = 0;
-ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
+
+if (!bs-file-exact_filename[0]) {
+error_setg(errp, Cannot use extents with VMDK descriptor file '%s',
+   bs-file-filename);
+ret = -EINVAL;
+goto exit;
+}
+ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp);
 exit:
 return ret;
 }
-- 
1.9.3




Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Cornelia Huck
On Tue, 2 Dec 2014 21:03:45 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
   void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
   {
  +/*
  + * For virtio-1 devices, the number of buffers may only be
  + * updated if the ring addresses have not yet been set up.
 
 Where does it say that?

Hmpf, may have imagined that.

This means we either need to track whether used/avail have been
specified or calculated or move responsibility for re-calculation of
used/avail for the old layout into the callers.
 
  + */
  +if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1) 
  +vdev-vq[n].vring.desc) {
  +error_report(tried to modify buffer num for virtio-1 device);
  +return;
  +}
   /* Don't allow guest to flip queue between existent and
* nonexistent states, or to set it to an invalid size.
*/
 




[Qemu-devel] [PATCH v3 1/5] libqos: Change use of pointers to uint64_t in virtio

2014-12-03 Thread Marc Marí
Convert use of pointers in functions of virtio to uint64_t in order to make it
platform-independent.

Add casting from pointers (in PCI functions) to uint64_t and vice versa through
uintptr_t.

Signed-off-by: Marc Marí marc.mari.barc...@gmail.com
---
 tests/libqos/virtio-pci.c |   20 +++-
 tests/libqos/virtio.c |8 
 tests/libqos/virtio.h |   16 
 tests/virtio-blk-test.c   |   21 ++---
 4 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 788ebaf..92bcac1 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -60,25 +60,25 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, 
void *data)
 *vpcidev = (QVirtioPCIDevice *)d;
 }
 
-static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr)
+static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readb(dev-pdev, addr);
+return qpci_io_readb(dev-pdev, (void *)(uintptr_t)addr);
 }
 
-static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, void *addr)
+static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readw(dev-pdev, addr);
+return qpci_io_readw(dev-pdev, (void *)(uintptr_t)addr);
 }
 
-static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, void *addr)
+static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readl(dev-pdev, addr);
+return qpci_io_readl(dev-pdev, (void *)(uintptr_t)addr);
 }
 
-static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
+static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
 int i;
@@ -86,11 +86,13 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, 
void *addr)
 
 if (qtest_big_endian()) {
 for (i = 0; i  8; ++i) {
-u64 |= (uint64_t)qpci_io_readb(dev-pdev, addr + i)  (7 - i) * 8;
+u64 |= (uint64_t)qpci_io_readb(dev-pdev,
+(void *)(uintptr_t)addr + i)  (7 - i) * 8;
 }
 } else {
 for (i = 0; i  8; ++i) {
-u64 |= (uint64_t)qpci_io_readb(dev-pdev, addr + i)  i * 8;
+u64 |= (uint64_t)qpci_io_readb(dev-pdev,
+(void *)(uintptr_t)addr + i)  i * 8;
 }
 }
 
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index a061289..3205b88 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -12,25 +12,25 @@
 #include libqos/virtio.h
 
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
-void *addr)
+uint64_t addr)
 {
 return bus-config_readb(d, addr);
 }
 
 uint16_t qvirtio_config_readw(const QVirtioBus *bus, QVirtioDevice *d,
-void *addr)
+uint64_t addr)
 {
 return bus-config_readw(d, addr);
 }
 
 uint32_t qvirtio_config_readl(const QVirtioBus *bus, QVirtioDevice *d,
-void *addr)
+uint64_t addr)
 {
 return bus-config_readl(d, addr);
 }
 
 uint64_t qvirtio_config_readq(const QVirtioBus *bus, QVirtioDevice *d,
-void *addr)
+uint64_t addr)
 {
 return bus-config_readq(d, addr);
 }
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 29fbacb..2449fee 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -93,10 +93,10 @@ typedef struct QVRingIndirectDesc {
 } QVRingIndirectDesc;
 
 typedef struct QVirtioBus {
-uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
-uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
-uint32_t (*config_readl)(QVirtioDevice *d, void *addr);
-uint64_t (*config_readq)(QVirtioDevice *d, void *addr);
+uint8_t (*config_readb)(QVirtioDevice *d, uint64_t addr);
+uint16_t (*config_readw)(QVirtioDevice *d, uint64_t addr);
+uint32_t (*config_readl)(QVirtioDevice *d, uint64_t addr);
+uint64_t (*config_readq)(QVirtioDevice *d, uint64_t addr);
 
 /* Get features of the device */
 uint32_t (*get_features)(QVirtioDevice *d);
@@ -144,13 +144,13 @@ static inline uint32_t qvring_size(uint32_t num, uint32_t 
align)
 }
 
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
-void *addr);
+   

[Qemu-devel] [PATCH v3 0/5] libqos: Virtio MMIO driver

2014-12-03 Thread Marc Marí
Add virtio-mmio support to libqos and test case for virtio-blk.

This series depends on patch libqos: Convert malloc-pc allocator to a generic 
allocator

Changes from version 2:
 - Fix leaks and minor bugs
 - Extract basic test case to a function

Marc Marí (5):
  libqos: Change use of pointers to uint64_t in virtio
  tests: Prepare virtio-blk-test for multi-arch implementation
  libqos: Remove PCI assumptions in constants of virtio driver
  libqos: Add malloc generic
  libqos: Add virtio MMIO support

 tests/Makefile|4 +-
 tests/libqos/malloc-generic.c |   50 +
 tests/libqos/malloc-generic.h |   21 
 tests/libqos/virtio-mmio.c|  190 +++
 tests/libqos/virtio-mmio.h|   46 
 tests/libqos/virtio-pci.c |   50 +
 tests/libqos/virtio-pci.h |   24 ++--
 tests/libqos/virtio.c |8 +-
 tests/libqos/virtio.h |   16 +--
 tests/virtio-blk-test.c   |  249 -
 10 files changed, 534 insertions(+), 124 deletions(-)
 create mode 100644 tests/libqos/malloc-generic.c
 create mode 100644 tests/libqos/malloc-generic.h
 create mode 100644 tests/libqos/virtio-mmio.c
 create mode 100644 tests/libqos/virtio-mmio.h

-- 
1.7.10.4




[Qemu-devel] [PATCH v3 3/5] libqos: Remove PCI assumptions in constants of virtio driver

2014-12-03 Thread Marc Marí
Convert PCI-specific constants names of libqos virtio driver.

Signed-off-by: Marc Marí marc.mari.barc...@gmail.com
---
 tests/libqos/virtio-pci.c |   30 +++---
 tests/libqos/virtio-pci.h |   24 
 tests/virtio-blk-test.c   |   11 ++-
 3 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 92bcac1..046a316 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -102,31 +102,31 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice 
*d, uint64_t addr)
 static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readl(dev-pdev, dev-addr + QVIRTIO_DEVICE_FEATURES);
+return qpci_io_readl(dev-pdev, dev-addr + QVIRTIO_PCI_DEVICE_FEATURES);
 }
 
 static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writel(dev-pdev, dev-addr + QVIRTIO_GUEST_FEATURES, features);
+qpci_io_writel(dev-pdev, dev-addr + QVIRTIO_PCI_GUEST_FEATURES, 
features);
 }
 
 static uint32_t qvirtio_pci_get_guest_features(QVirtioDevice *d)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readl(dev-pdev, dev-addr + QVIRTIO_GUEST_FEATURES);
+return qpci_io_readl(dev-pdev, dev-addr + QVIRTIO_PCI_GUEST_FEATURES);
 }
 
 static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_DEVICE_STATUS);
+return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_PCI_DEVICE_STATUS);
 }
 
 static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writeb(dev-pdev, dev-addr + QVIRTIO_DEVICE_STATUS, status);
+qpci_io_writeb(dev-pdev, dev-addr + QVIRTIO_PCI_DEVICE_STATUS, status);
 }
 
 static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
@@ -146,7 +146,7 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice 
*d, QVirtQueue *vq)
 return data == vqpci-msix_data;
 }
 } else {
-return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_ISR_STATUS)  1;
+return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_PCI_ISR_STATUS)  
1;
 }
 }
 
@@ -166,26 +166,26 @@ static bool 
qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
 return data == dev-config_msix_data;
 }
 } else {
-return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_ISR_STATUS)  2;
+return qpci_io_readb(dev-pdev, dev-addr + QVIRTIO_PCI_ISR_STATUS)  
2;
 }
 }
 
 static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writeb(dev-pdev, dev-addr + QVIRTIO_QUEUE_SELECT, index);
+qpci_io_writeb(dev-pdev, dev-addr + QVIRTIO_PCI_QUEUE_SELECT, index);
 }
 
 static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readw(dev-pdev, dev-addr + QVIRTIO_QUEUE_SIZE);
+return qpci_io_readw(dev-pdev, dev-addr + QVIRTIO_PCI_QUEUE_SIZE);
 }
 
 static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writel(dev-pdev, dev-addr + QVIRTIO_QUEUE_ADDRESS, pfn);
+qpci_io_writel(dev-pdev, dev-addr + QVIRTIO_PCI_QUEUE_ADDRESS, pfn);
 }
 
 static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
@@ -227,7 +227,7 @@ static QVirtQueue 
*qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
 static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writew(dev-pdev, dev-addr + QVIRTIO_QUEUE_NOTIFY, vq-index);
+qpci_io_writew(dev-pdev, dev-addr + QVIRTIO_PCI_QUEUE_NOTIFY, vq-index);
 }
 
 const QVirtioBus qvirtio_pci = {
@@ -307,8 +307,8 @@ void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, 
QVirtQueuePCI *vqpci,
 control  
~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
 qvirtio_pci_queue_select(d-vdev, vqpci-vq.index);
-qpci_io_writew(d-pdev, d-addr + QVIRTIO_MSIX_QUEUE_VECTOR, entry);
-vector = qpci_io_readw(d-pdev, d-addr + QVIRTIO_MSIX_QUEUE_VECTOR);
+qpci_io_writew(d-pdev, d-addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR, entry);
+vector = qpci_io_readw(d-pdev, d-addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR);
 g_assert_cmphex(vector, !=, QVIRTIO_MSI_NO_VECTOR);
 }
 
@@ -339,7 +339,7 @@ void 
qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
 qpci_io_writel(d-pdev, addr + PCI_MSIX_ENTRY_VECTOR_CTRL,
 control  
~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
-qpci_io_writew(d-pdev, d-addr + QVIRTIO_MSIX_CONF_VECTOR, entry);
-vector = qpci_io_readw(d-pdev, d-addr + QVIRTIO_MSIX_CONF_VECTOR);
+

[Qemu-devel] [PATCH v3 4/5] libqos: Add malloc generic

2014-12-03 Thread Marc Marí
This malloc is a basic interface implementation that works for any platform.
It should be replaced in the future for a real malloc implementation for each
of the platforms.

Signed-off-by: Marc Marí marc.mari.barc...@gmail.com
---
 tests/libqos/malloc-generic.c |   50 +
 tests/libqos/malloc-generic.h |   21 +
 2 files changed, 71 insertions(+)
 create mode 100644 tests/libqos/malloc-generic.c
 create mode 100644 tests/libqos/malloc-generic.h

diff --git a/tests/libqos/malloc-generic.c b/tests/libqos/malloc-generic.c
new file mode 100644
index 000..a0878c5
--- /dev/null
+++ b/tests/libqos/malloc-generic.c
@@ -0,0 +1,50 @@
+/*
+ * Basic libqos generic malloc support
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include glib.h
+#include libqos/malloc-generic.h
+#include libqos/malloc.h
+
+/*
+ * Mostly for valgrind happiness, but it does offer
+ * a chokepoint for debugging guest memory leaks, too.
+ */
+void generic_alloc_uninit(QGuestAllocator *allocator)
+{
+alloc_uninit(allocator);
+}
+
+QGuestAllocator *generic_alloc_init_flags(uint64_t base_addr, uint64_t size,
+uint32_t page_size, QAllocOpts flags)
+{
+QGuestAllocator *s = g_malloc0(sizeof(*s));
+MemBlock *node;
+
+s-opts = flags;
+s-page_size = page_size;
+
+/* Start at 1MB */
+s-start = base_addr + (1  20);
+
+s-end = s-start + size;
+
+QTAILQ_INIT(s-used);
+QTAILQ_INIT(s-free);
+
+node = mlist_new(s-start, s-end - s-start);
+QTAILQ_INSERT_HEAD(s-free, node, MLIST_ENTNAME);
+
+return s;
+}
+
+inline QGuestAllocator *generic_alloc_init(uint64_t base_addr, uint64_t size,
+uint32_t page_size)
+{
+return generic_alloc_init_flags(base_addr, size, page_size, 
ALLOC_NO_FLAGS);
+}
diff --git a/tests/libqos/malloc-generic.h b/tests/libqos/malloc-generic.h
new file mode 100644
index 000..90104ec
--- /dev/null
+++ b/tests/libqos/malloc-generic.h
@@ -0,0 +1,21 @@
+/*
+ * Basic libqos generic malloc support
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_MALLOC_GENERIC_H
+#define LIBQOS_MALLOC_GENERIC_H
+
+#include libqos/malloc.h
+
+QGuestAllocator *generic_alloc_init(uint64_t base_addr, uint64_t size,
+uint32_t 
page_size);
+QGuestAllocator *generic_alloc_init_flags(uint64_t base_addr, uint64_t size,
+uint32_t page_size, QAllocOpts flags);
+void generic_alloc_uninit(QGuestAllocator *allocator);
+
+#endif
-- 
1.7.10.4




[Qemu-devel] [PATCH v3 2/5] tests: Prepare virtio-blk-test for multi-arch implementation

2014-12-03 Thread Marc Marí
Modularize functions in virtio-blk-test and add PCI suffix for PCI specific
components.

Signed-off-by: Marc Marí marc.mari.barc...@gmail.com
---
 tests/virtio-blk-test.c |  146 +++
 1 file changed, 85 insertions(+), 61 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 33e8094..0e4aee4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -55,11 +55,10 @@ typedef struct QVirtioBlkReq {
 uint8_t status;
 } QVirtioBlkReq;
 
-static QPCIBus *test_start(void)
+static char *drive_create(void)
 {
-char *cmdline;
-char tmp_path[] = /tmp/qtest.XX;
 int fd, ret;
+char *tmp_path = g_strdup(/tmp/qtest.XX);
 
 /* Create a temporary raw image */
 fd = mkstemp(tmp_path);
@@ -68,6 +67,16 @@ static QPCIBus *test_start(void)
 g_assert_cmpint(ret, ==, 0);
 close(fd);
 
+return tmp_path;
+}
+
+static QPCIBus *pci_test_start(void)
+{
+char *cmdline;
+char *tmp_path;
+
+tmp_path = drive_create();
+
 cmdline = g_strdup_printf(-drive if=none,id=drive0,file=%s 
   -drive if=none,id=drive1,file=/dev/null 
   -device virtio-blk-pci,id=drv0,drive=drive0,
@@ -75,6 +84,7 @@ static QPCIBus *test_start(void)
   tmp_path, PCI_SLOT, PCI_FN);
 qtest_start(cmdline);
 unlink(tmp_path);
+g_free(tmp_path);
 g_free(cmdline);
 
 return qpci_init_pc();
@@ -85,7 +95,7 @@ static void test_end(void)
 qtest_end();
 }
 
-static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus, int slot)
+static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot)
 {
 QVirtioPCIDevice *dev;
 
@@ -135,14 +145,10 @@ static uint64_t virtio_blk_request(QGuestAllocator 
*alloc, QVirtioBlkReq *req,
 return addr;
 }
 
-static void pci_basic(void)
+static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
+QGuestAllocator *alloc, QVirtQueue *vq, uint64_t device_specific)
 {
-QVirtioPCIDevice *dev;
-QPCIBus *bus;
-QVirtQueuePCI *vqpci;
-QGuestAllocator *alloc;
 QVirtioBlkReq req;
-void *addr;
 uint64_t req_addr;
 uint64_t capacity;
 uint32_t features;
@@ -150,28 +156,16 @@ static void pci_basic(void)
 uint8_t status;
 char *data;
 
-bus = test_start();
-
-dev = virtio_blk_init(bus, PCI_SLOT);
-
-/* MSI-X is not enabled */
-addr = dev-addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX;
-
-capacity = qvirtio_config_readq(qvirtio_pci, dev-vdev,
-(uint64_t)(uintptr_t)addr);
+capacity = qvirtio_config_readq(bus, dev, device_specific);
 g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
-features = qvirtio_get_features(qvirtio_pci, dev-vdev);
+features = qvirtio_get_features(bus, dev);
 features = features  ~(QVIRTIO_F_BAD_FEATURE |
 QVIRTIO_F_RING_INDIRECT_DESC | QVIRTIO_F_RING_EVENT_IDX |
 QVIRTIO_BLK_F_SCSI);
-qvirtio_set_features(qvirtio_pci, dev-vdev, features);
+qvirtio_set_features(bus, dev, features);
 
-alloc = pc_alloc_init();
-vqpci = (QVirtQueuePCI *)qvirtqueue_setup(qvirtio_pci, dev-vdev,
-alloc, 0);
-
-qvirtio_set_driver_ok(qvirtio_pci, dev-vdev);
+qvirtio_set_driver_ok(bus, dev);
 
 /* Write and read with 2 descriptor layout */
 /* Write request */
@@ -185,12 +179,11 @@ static void pci_basic(void)
 
 g_free(req.data);
 
-free_head = qvirtqueue_add(vqpci-vq, req_addr, 528, false, true);
-qvirtqueue_add(vqpci-vq, req_addr + 528, 1, true, false);
-qvirtqueue_kick(qvirtio_pci, dev-vdev, vqpci-vq, free_head);
+free_head = qvirtqueue_add(vq, req_addr, 528, false, true);
+qvirtqueue_add(vq, req_addr + 528, 1, true, false);
+qvirtqueue_kick(bus, dev, vq, free_head);
 
-qvirtio_wait_queue_isr(qvirtio_pci, dev-vdev, vqpci-vq,
-   QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -206,13 +199,12 @@ static void pci_basic(void)
 
 g_free(req.data);
 
-free_head = qvirtqueue_add(vqpci-vq, req_addr, 16, false, true);
-qvirtqueue_add(vqpci-vq, req_addr + 16, 513, true, false);
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, 513, true, false);
 
-qvirtqueue_kick(qvirtio_pci, dev-vdev, vqpci-vq, free_head);
+qvirtqueue_kick(bus, dev, vq, free_head);
 
-qvirtio_wait_queue_isr(qvirtio_pci, dev-vdev, vqpci-vq,
-   QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_queue_isr(bus, dev, vq, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -233,14 +225,15 @@ static void pci_basic(void)
 
 req_addr = 

[Qemu-devel] [PATCH v3 5/5] libqos: Add virtio MMIO support

2014-12-03 Thread Marc Marí
Add virtio MMIO support.
Add virtio-blk-test MMIO test case.

Signed-off-by: Marc Marí marc.mari.barc...@gmail.com
---
 tests/Makefile |4 +-
 tests/libqos/virtio-mmio.c |  190 
 tests/libqos/virtio-mmio.h |   46 +++
 tests/virtio-blk-test.c|   81 +--
 4 files changed, 313 insertions(+), 8 deletions(-)
 create mode 100644 tests/libqos/virtio-mmio.c
 create mode 100644 tests/libqos/virtio-mmio.h

diff --git a/tests/Makefile b/tests/Makefile
index 4ae0ca4..f193b03 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -183,6 +183,8 @@ gcov-files-sparc-y += hw/timer/m48t59.c
 gcov-files-sparc64-y += hw/timer/m48t59.c
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
+check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
+gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF)
@@ -300,8 +302,8 @@ libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
-libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o 
tests/libqos/virtio-pci.o
 libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
+libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o 
tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o 
tests/libqos/malloc-generic.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
new file mode 100644
index 000..f5ca6c4
--- /dev/null
+++ b/tests/libqos/virtio-mmio.c
@@ -0,0 +1,190 @@
+/*
+ * libqos virtio MMIO driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include glib.h
+#include stdio.h
+#include libqtest.h
+#include libqos/virtio.h
+#include libqos/virtio-mmio.h
+#include libqos/malloc.h
+#include libqos/malloc-generic.h
+
+static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t addr)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return readb(dev-addr + addr);
+}
+
+static uint16_t qvirtio_mmio_config_readw(QVirtioDevice *d, uint64_t addr)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return readw(dev-addr + addr);
+}
+
+static uint32_t qvirtio_mmio_config_readl(QVirtioDevice *d, uint64_t addr)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return readl(dev-addr + addr);
+}
+
+static uint64_t qvirtio_mmio_config_readq(QVirtioDevice *d, uint64_t addr)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return readq(dev-addr + addr);
+}
+
+static uint32_t qvirtio_mmio_get_features(QVirtioDevice *d)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+writel(dev-addr + QVIRTIO_MMIO_HOST_FEATURES_SEL, 0);
+return readl(dev-addr + QVIRTIO_MMIO_HOST_FEATURES);
+}
+
+static void qvirtio_mmio_set_features(QVirtioDevice *d, uint32_t features)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+dev-features = features;
+writel(dev-addr + QVIRTIO_MMIO_GUEST_FEATURES_SEL, 0);
+writel(dev-addr + QVIRTIO_MMIO_GUEST_FEATURES, features);
+}
+
+static uint32_t qvirtio_mmio_get_guest_features(QVirtioDevice *d)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return dev-features;
+}
+
+static uint8_t qvirtio_mmio_get_status(QVirtioDevice *d)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+return (uint8_t)readl(dev-addr + QVIRTIO_MMIO_DEVICE_STATUS);
+}
+
+static void qvirtio_mmio_set_status(QVirtioDevice *d, uint8_t status)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+writel(dev-addr + QVIRTIO_MMIO_DEVICE_STATUS, (uint32_t)status);
+}
+
+static bool qvirtio_mmio_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+uint32_t isr;
+
+isr = readl(dev-addr + QVIRTIO_MMIO_INTERRUPT_STATUS)  1;
+writel(dev-addr + QVIRTIO_MMIO_INTERRUPT_ACK, 1);
+return isr != 0;
+}
+
+static bool qvirtio_mmio_get_config_isr_status(QVirtioDevice *d)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+uint32_t isr;
+
+isr = readl(dev-addr + QVIRTIO_MMIO_INTERRUPT_STATUS)  2;
+writel(dev-addr + QVIRTIO_MMIO_INTERRUPT_ACK, 2);
+return isr != 0;
+}
+
+static void qvirtio_mmio_queue_select(QVirtioDevice *d, uint16_t index)
+{
+QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
+writel(dev-addr + QVIRTIO_MMIO_QUEUE_SEL, (uint32_t)index);
+
+g_assert_cmphex(readl(dev-addr + QVIRTIO_MMIO_QUEUE_PFN), ==, 0);
+}
+
+static uint16_t qvirtio_mmio_get_queue_size(QVirtioDevice *d)
+{
+QVirtioMMIODevice *dev = 

Re: [Qemu-devel] [PATCH] iotests: Specify qcow2 format for qemu-io in 059

2014-12-03 Thread Fam Zheng
On Wed, 12/03 10:15, Max Reitz wrote:
 There are two instances of iotest 059 using qemu-io on a qcow2 image. As
 of qemu-iotests: Use qemu-io -f $IMGFMT the iotests can no longer rely
 on $QEMU_IO doing probing, therefore the qcow2 format has to be
 specified explicitly here.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  tests/qemu-iotests/059 | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
 index 3c053c2..2ed1a7f 100755
 --- a/tests/qemu-iotests/059
 +++ b/tests/qemu-iotests/059
 @@ -106,8 +106,8 @@ _img_info
  echo
  echo === Converting to streamOptimized from image with small cluster 
 size===
  TEST_IMG=$TEST_IMG.qcow2 IMGFMT=qcow2 IMGOPTS=cluster_size=4096 
 _make_test_img 1G
 -$QEMU_IO -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
 -$QEMU_IO -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | _filter_qemu_io
 +$QEMU_IO -f qcow2 -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
 +$QEMU_IO -f qcow2 -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | 
 _filter_qemu_io
  $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
 $TEST_IMG.qcow2 $TEST_IMG 21
  
  echo
 -- 
 1.9.3
 

Reviewed-by: Fam Zheng f...@redhat.com



Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically

2014-12-03 Thread Daniel P. Berrange
On Wed, Dec 03, 2014 at 02:55:40PM +0800, arei.gong...@huawei.com wrote:
 From: Gonglei arei.gong...@huawei.com
 
 A bonus of this feature is that supporting different
 people (in different countries) using defferent keyboard
 to connect the same guest but not need to configure
 command line or libivrt xml file then restart guest.
 
 Using a new QMP command:
 - { execute: change-vnc-kbd-layout,
 arguments: { keymap: de } }
 - { return: {}
  
 I knew sdl and curses are using keyboard layout, but I don't know
 whether they both need to support this feature and add some new
 qmp command for them?
 
 If you have some ideas, please let me know. Thanks!

FWIW users of VNC are much better off not setting any keymap at all
in QEMU, and then using a client (such as GTK-VNC) that supports the
raw scancode extension. This takes QEMU out of the key remapping
business entirely, so that everything just works with no extra
configuration required in QEMU. This is what SPICE does by default
too.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH 1/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Fam Zheng
On Wed, 12/03 10:23, Max Reitz wrote:
 If vmdk blindly tries to use path_combine() using bs-file-filename as
 the base file name, this will result in a bad error message for JSON
 file names when calling bdrv_open(). It is better to only try
 bs-file-exact_filename; if that is empty, bs-file-filename will be
 useless for path_combine() and an error should be emitted (containing
 bs-file-filename because bs-file-exact_filename is empty).
 
 Note that s-create_type does not need to be freed on error because it
 will be freed by the caller (which ultimately is vmdk_open()).
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block/vmdk.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/block/vmdk.c b/block/vmdk.c
 index 2cbfd3e..fe549c2 100644
 --- a/block/vmdk.c
 +++ b/block/vmdk.c
 @@ -894,7 +894,14 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
 flags, char *buf,
  }
  s-create_type = g_strdup(ct);
  s-desc_offset = 0;
 -ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
 +
 +if (!bs-file-exact_filename[0]) {
 +error_setg(errp, Cannot use extents with VMDK descriptor file '%s',
 +   bs-file-filename);
 +ret = -EINVAL;
 +goto exit;
 +}
 +ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp);
  exit:
  return ret;
  }
 -- 
 1.9.3
 

Reviewed-by: Fam Zheng f...@redhat.com



Re: [Qemu-devel] [PATCH] iotests: Specify qcow2 format for qemu-io in 059

2014-12-03 Thread Fam Zheng
On Wed, 12/03 10:15, Max Reitz wrote:
 There are two instances of iotest 059 using qemu-io on a qcow2 image. As
 of qemu-iotests: Use qemu-io -f $IMGFMT the iotests can no longer rely
 on $QEMU_IO doing probing, therefore the qcow2 format has to be
 specified explicitly here.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  tests/qemu-iotests/059 | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
 index 3c053c2..2ed1a7f 100755
 --- a/tests/qemu-iotests/059
 +++ b/tests/qemu-iotests/059
 @@ -106,8 +106,8 @@ _img_info
  echo
  echo === Converting to streamOptimized from image with small cluster 
 size===
  TEST_IMG=$TEST_IMG.qcow2 IMGFMT=qcow2 IMGOPTS=cluster_size=4096 
 _make_test_img 1G
 -$QEMU_IO -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
 -$QEMU_IO -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | _filter_qemu_io
 +$QEMU_IO -f qcow2 -c write -P 0xa 0 512 $TEST_IMG.qcow2 | _filter_qemu_io
 +$QEMU_IO -f qcow2 -c write -P 0xb 10240 512 $TEST_IMG.qcow2 | 
 _filter_qemu_io
  $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
 $TEST_IMG.qcow2 $TEST_IMG 21
  
  echo
 -- 
 1.9.3
 

Reviewed-by: Fam Zheng f...@redhat.com



Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for vmdk JSON file names

2014-12-03 Thread Fam Zheng
On Wed, 12/03 10:23, Max Reitz wrote:
 Add a test for vmdk files which use a file with a JSON file name, and
 which then try to open extents. That should fail and the error message
 should at least try to look helpful.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  tests/qemu-iotests/059 | 6 ++
  tests/qemu-iotests/059.out | 4 
  2 files changed, 10 insertions(+)
 
 diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
 index 2ed1a7f..50ca5ce 100755
 --- a/tests/qemu-iotests/059
 +++ b/tests/qemu-iotests/059
 @@ -111,6 +111,12 @@ $QEMU_IO -f qcow2 -c write -P 0xb 10240 512 
 $TEST_IMG.qcow2 | _filter_qemu_i
  $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
 $TEST_IMG.qcow2 $TEST_IMG 21
  
  echo
 +echo === Testing monolithicFlat with internally generated JSON file name 
 ===
 +IMGOPTS=subformat=monolithicFlat _make_test_img 64M
 +$QEMU_IO -c open -o 
 driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio
  21 \
 +| _filter_testdir | _filter_imgfmt
 +
 +echo
  echo === Testing version 3 ===
  _use_sample_img iotest-version3.vmdk.bz2
  _img_info
 diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
 index 0dadba6..97509ab 100644
 --- a/tests/qemu-iotests/059.out
 +++ b/tests/qemu-iotests/059.out
 @@ -2053,6 +2053,10 @@ wrote 512/512 bytes at offset 0
  wrote 512/512 bytes at offset 10240
  512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  
 +=== Testing monolithicFlat with internally generated JSON file name ===
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 +qemu-io: can't open: Cannot use extents with VMDK descriptor file 
 'json:{image: {driver: file, filename: TEST_DIR/t.IMGFMT}, 
 driver: blkdebug, inject-error.0.event: read_aio}'
 +
  === Testing version 3 ===
  image: TEST_DIR/iotest-version3.IMGFMT
  file format: IMGFMT
 -- 
 1.9.3
 

Reviewed-by: Fam Zheng f...@redhat.com



[Qemu-devel] [Bug 918791] Re: qemu-kvm dies when using vmvga driver and unity in the guest

2014-12-03 Thread Rolf Leggewie
oneiric has seen the end of its life and is no longer receiving any
updates. Marking the oneiric task for this ticket as Won't Fix.

** Changed in: qemu-kvm (Ubuntu Oneiric)
   Status: Fix Committed = Won't Fix

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/918791

Title:
  qemu-kvm dies when using vmvga driver and unity in the guest

Status in QEMU:
  New
Status in qemu-kvm package in Ubuntu:
  Fix Released
Status in xserver-xorg-video-vmware package in Ubuntu:
  Invalid
Status in qemu-kvm source package in Oneiric:
  Won't Fix
Status in xserver-xorg-video-vmware source package in Oneiric:
  Invalid
Status in qemu-kvm source package in Precise:
  Fix Released
Status in xserver-xorg-video-vmware source package in Precise:
  Invalid

Bug description:
  =
  SRU Justification:
  1. impact: kvm crashes
  2. Development fix: don't allow attempts to set_bit to negative offsets
  3. Stable fix: same as development fix
  4. Test case (see below)
  5. Regression potential: if the patch is wrong, graphics for vmware vga over 
vnc could get messed up
  =

  12.04's qemu-kvm has been unstable for me and Marc Deslauriers and I
  figured out it has something to do with the interaction of qemu-kvm,
  unity and the vmvga driver. This is a regression over qemu-kvm in
  11.10.

  TEST CASE:
  1. start a VM that uses unity (eg, 11.04, 11.10 or 12.04). My tests use 
unity-2d on an amd64 host and amd64 guests
  2. on 11.04 and 11.10, open empathy via the messaging indicator and click 
'Chat'. On 12.04, open empathy via the messaging indicator and click 'Chat', 
close the empathy wizard, move the empathy window over the unity luancher (so 
it autohides), then do 'ctrl+alt+t' to open a terminal

  When the launcher tries to auto(un)hide, qemu-kvm dies with this:
  [10574.958149] do_general_protection: 132 callbacks suppressed
  [10574.958154] kvm[13192] general protection ip:7fab9680ea0f sp:74440148 
error:0 in qemu-system-x86_64[7fab966c4000+2c9000]

  Relevant libvirt xml:
  video
    model type='vmvga' vram='9216' heads='1'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  If I change to using 'cirrus', then qemu-kvm no longer crashes. Eg:
  video
    model type='cirrus' vram='9216' heads='1'/
    alias name='video0'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  The workaround is therefore to use the cirrus driver instead of vmvga,
  however being able to kill qemu-kvm in this manner is not ideal. Also,
  unfortunately unity-2d does not run with with cirrus driver under
  11.04, so the security and SRU teams are unable to properly test
  updates in GUI applications under unity when using the current 12.04
  qemu-kvm.

  I tried to report this via apport, but apport complained about a CRC
  error, so I could not.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/918791/+subscriptions



Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Cornelia Huck
On Wed, 3 Dec 2014 10:27:36 +0100
Cornelia Huck cornelia.h...@de.ibm.com wrote:

 On Tue, 2 Dec 2014 21:03:45 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
{
   +/*
   + * For virtio-1 devices, the number of buffers may only be
   + * updated if the ring addresses have not yet been set up.
  
  Where does it say that?
 
 Hmpf, may have imagined that.
 
 This means we either need to track whether used/avail have been
 specified or calculated or move responsibility for re-calculation of
 used/avail for the old layout into the callers.

What about this one instead?

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 43b7e02..1e2a720 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -244,9 +244,13 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, 
uint64_t value,
 case VIRTIO_MMIO_QUEUENUM:
 DPRINTF(mmio_queue write %d max %d\n, (int)value, 
VIRTQUEUE_MAX_SIZE);
 virtio_queue_set_num(vdev, vdev-queue_sel, value);
+/* Note: only call this function for legacy devices */
+virtio_queue_update_rings(vdev, vdev-queue_sel);
 break;
 case VIRTIO_MMIO_QUEUEALIGN:
+/* Note: this is only valid for legacy devices */
 virtio_queue_set_align(vdev, vdev-queue_sel, value);
+virtio_queue_update_rings(vdev, vdev-queue_sel);
 break;
 case VIRTIO_MMIO_QUEUEPFN:
 if (value == 0) {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8f69ffa..b2d553e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -69,7 +69,6 @@ typedef struct VRing
 struct VirtQueue
 {
 VRing vring;
-hwaddr pa;
 uint16_t last_avail_idx;
 /* Last used index value we have signalled on */
 uint16_t signalled_used;
@@ -92,15 +91,18 @@ struct VirtQueue
 };
 
 /* virt queue functions */
-static void virtqueue_init(VirtQueue *vq)
+void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 {
-hwaddr pa = vq-pa;
+VRing *vring = vdev-vq[n].vring;
 
-vq-vring.desc = pa;
-vq-vring.avail = pa + vq-vring.num * sizeof(VRingDesc);
-vq-vring.used = vring_align(vq-vring.avail +
- offsetof(VRingAvail, ring[vq-vring.num]),
- vq-vring.align);
+if (!vring-desc) {
+/* not yet setup - nothing to do */
+return;
+}
+vring-avail = vring-desc + vring-num * sizeof(VRingDesc);
+vring-used = vring_align(vring-avail +
+  offsetof(VRingAvail, ring[vring-num]),
+  vring-align);
 }
 
 static inline uint64_t vring_desc_addr(VirtIODevice *vdev, hwaddr desc_pa,
@@ -605,7 +607,6 @@ void virtio_reset(void *opaque)
 vdev-vq[i].vring.avail = 0;
 vdev-vq[i].vring.used = 0;
 vdev-vq[i].last_avail_idx = 0;
-vdev-vq[i].pa = 0;
 vdev-vq[i].vector = VIRTIO_NO_VECTOR;
 vdev-vq[i].signalled_used = 0;
 vdev-vq[i].signalled_used_valid = false;
@@ -708,13 +709,21 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t 
addr, uint32_t data)
 
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr)
 {
-vdev-vq[n].pa = addr;
-virtqueue_init(vdev-vq[n]);
+vdev-vq[n].vring.desc = addr;
+virtio_queue_update_rings(vdev, n);
 }
 
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
 {
-return vdev-vq[n].pa;
+return vdev-vq[n].vring.desc;
+}
+
+void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
+hwaddr avail, hwaddr used)
+{
+vdev-vq[n].vring.desc = desc;
+vdev-vq[n].vring.avail = avail;
+vdev-vq[n].vring.used = used;
 }
 
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
@@ -728,7 +737,6 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int 
num)
 return;
 }
 vdev-vq[n].vring.num = num;
-virtqueue_init(vdev-vq[n]);
 }
 
 int virtio_queue_get_num(VirtIODevice *vdev, int n)
@@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int 
align)
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
+/* virtio-1 compliant devices cannot change the aligment */
+if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+error_report(tried to modify queue alignment for virtio-1 device);
+return;
+}
 /* Check that the transport told us it was going to do this
  * (so a buggy transport will immediately assert rather than
  * silently failing to migrate this state)
@@ -755,7 +768,6 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int 
align)
 assert(k-has_variable_vring_alignment);
 
 vdev-vq[n].vring.align = align;
-virtqueue_init(vdev-vq[n]);
 }
 
 void virtio_queue_notify_vq(VirtQueue *vq)
@@ -949,7 +961,8 @@ void 

Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically

2014-12-03 Thread Gonglei
On 2014/12/3 17:38, Daniel P. Berrange wrote:

 On Wed, Dec 03, 2014 at 02:55:40PM +0800, arei.gong...@huawei.com wrote:
 From: Gonglei arei.gong...@huawei.com

 A bonus of this feature is that supporting different
 people (in different countries) using defferent keyboard
 to connect the same guest but not need to configure
 command line or libivrt xml file then restart guest.

 Using a new QMP command:
 - { execute: change-vnc-kbd-layout,
 arguments: { keymap: de } }
 - { return: {}
  
 I knew sdl and curses are using keyboard layout, but I don't know
 whether they both need to support this feature and add some new
 qmp command for them?

 If you have some ideas, please let me know. Thanks!
 
 FWIW users of VNC are much better off not setting any keymap at all
 in QEMU, and then using a client (such as GTK-VNC) that supports the
 raw scancode extension. This takes QEMU out of the key remapping
 business entirely, so that everything just works with no extra
 configuration required in QEMU. This is what SPICE does by default
 too.
 

Hi, Daniel
Actually, my team had received the requirement of changing VNC keyboard
layout dynamically on the scenario of  Desktop Cloud. The clientele just use
the simplest tight vnc client, but not GTK-VNC etc. I think we should support
this scenario, isn't it ?

Regards,
-Gonglei




Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically

2014-12-03 Thread Daniel P. Berrange
On Wed, Dec 03, 2014 at 05:50:57PM +0800, Gonglei wrote:
 On 2014/12/3 17:38, Daniel P. Berrange wrote:
 
  On Wed, Dec 03, 2014 at 02:55:40PM +0800, arei.gong...@huawei.com wrote:
  From: Gonglei arei.gong...@huawei.com
 
  A bonus of this feature is that supporting different
  people (in different countries) using defferent keyboard
  to connect the same guest but not need to configure
  command line or libivrt xml file then restart guest.
 
  Using a new QMP command:
  - { execute: change-vnc-kbd-layout,
  arguments: { keymap: de } }
  - { return: {}
   
  I knew sdl and curses are using keyboard layout, but I don't know
  whether they both need to support this feature and add some new
  qmp command for them?
 
  If you have some ideas, please let me know. Thanks!
  
  FWIW users of VNC are much better off not setting any keymap at all
  in QEMU, and then using a client (such as GTK-VNC) that supports the
  raw scancode extension. This takes QEMU out of the key remapping
  business entirely, so that everything just works with no extra
  configuration required in QEMU. This is what SPICE does by default
  too.
 
 Actually, my team had received the requirement of changing VNC keyboard
 layout dynamically on the scenario of  Desktop Cloud. The clientele just use
 the simplest tight vnc client, but not GTK-VNC etc. I think we should support
 this scenario, isn't it ?

Personally I think effort is better spent adding support for the keyboard
extension to more of the various VNC clients that exist. Having to issue
monitor commands to change keymap each time a different client wants to
connect is still a pretty sucky solution IMHO.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically

2014-12-03 Thread Gonglei
On 2014/12/3 17:54, Daniel P. Berrange wrote:

 On Wed, Dec 03, 2014 at 05:50:57PM +0800, Gonglei wrote:
 On 2014/12/3 17:38, Daniel P. Berrange wrote:

 On Wed, Dec 03, 2014 at 02:55:40PM +0800, arei.gong...@huawei.com wrote:
 From: Gonglei arei.gong...@huawei.com

 A bonus of this feature is that supporting different
 people (in different countries) using defferent keyboard
 to connect the same guest but not need to configure
 command line or libivrt xml file then restart guest.

 Using a new QMP command:
 - { execute: change-vnc-kbd-layout,
 arguments: { keymap: de } }
 - { return: {}
  
 I knew sdl and curses are using keyboard layout, but I don't know
 whether they both need to support this feature and add some new
 qmp command for them?

 If you have some ideas, please let me know. Thanks!

 FWIW users of VNC are much better off not setting any keymap at all
 in QEMU, and then using a client (such as GTK-VNC) that supports the
 raw scancode extension. This takes QEMU out of the key remapping
 business entirely, so that everything just works with no extra
 configuration required in QEMU. This is what SPICE does by default
 too.

 Actually, my team had received the requirement of changing VNC keyboard
 layout dynamically on the scenario of  Desktop Cloud. The clientele just use
 the simplest tight vnc client, but not GTK-VNC etc. I think we should support
 this scenario, isn't it ?
 
 Personally I think effort is better spent adding support for the keyboard
 extension to more of the various VNC clients that exist.

Your meaning pass different keymaps to Qemu at command line one time?

Regards,
-Gonglei

 Having to issue
 monitor commands to change keymap each time a different client wants to
 connect is still a pretty sucky solution IMHO.
 

 Regards,
 Daniel






Re: [Qemu-devel] [RFC PATCH v5 07/31] icount: implement icount requesting

2014-12-03 Thread Paolo Bonzini


On 26/11/2014 11:39, Pavel Dovgalyuk wrote:
 +int64_t cpu_get_instructions_counter(void)
 +{
 +/* This function calls are synchnonized to timer changes,
 +   calling cpu_get_instructions_counter_locked without lock is safe */
 +int64_t icount = timers_state.qemu_icount;
 +CPUState *cpu = current_cpu;
 +
 +if (cpu) {
 +icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
 +}
 +return icount;

Why do you need to do this if !cpu_can_do_io(cpu)?

Perhaps a better name for the functions is

- cpu_get_instructions_counter_locked - cpu_get_icount_raw

- cpu_get_instructions_counter - cpu_get_icount_raw_nocheck

This makes it clear that cpu_get_instructions_counter should raise
questions to a reviewer.

Paolo



Re: [Qemu-devel] Vhost-user - multi queue support

2014-12-03 Thread Nikolay Nikolaev
On Tue, Dec 2, 2014 at 1:42 PM, Long, Thomas thomas.l...@intel.com wrote:

  Hi All,

 I’m just wondering what the status is with regards to supporting
 multi-queue in Vhost-user?



 I see that Nikolaev has developed a patch to support this feature:


 https://github.com/SnabbCo/qemu/commit/f41eeccf4ab6ea5970e2941ce2de0aae893b10f9




This patch was developed by VirtualOpenSystems  for snabbswitch. We're
plannig to send the patch to
the qemu devel list by the end of this week.

regards,
Nikolay NIkolaev


  Are there any current plans to pull either this patch or a different
 patch into QEMU ?



 Regards,

 Tommy





[Qemu-devel] [PATCH] Drop superfluous conditionals around qemu_opts_del()

2014-12-03 Thread Markus Armbruster
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/core/qdev.c | 4 +---
 qemu-char.c| 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 35fd00d..901f289 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1141,9 +1141,7 @@ static void device_finalize(Object *obj)
 NamedGPIOList *ngl, *next;
 
 DeviceState *dev = DEVICE(obj);
-if (dev-opts) {
-qemu_opts_del(dev-opts);
-}
+qemu_opts_del(dev-opts);
 
 QLIST_FOREACH_SAFE(ngl, dev-gpios, node, next) {
 QLIST_REMOVE(ngl, node);
diff --git a/qemu-char.c b/qemu-char.c
index a8b01da..ef84b53 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3825,9 +3825,7 @@ void qemu_chr_delete(CharDriverState *chr)
 }
 g_free(chr-filename);
 g_free(chr-label);
-if (chr-opts) {
-qemu_opts_del(chr-opts);
-}
+qemu_opts_del(chr-opts);
 g_free(chr);
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH v3 0/6] vmdk: A few small fixes

2014-12-03 Thread Fam Zheng
v3: Fix 3/6 again. (Markus)
v2: 3/6: Don't overwrite the last byte of buffer. (Don)
Other patches are unchanged (added Markus' rev-by line).

Here are some improvements on miscellaneous things such as CID generation,
comments, input validation.


Fam Zheng (6):
  vmdk: Use g_random_int to generate CID
  vmdk: Fix comment to match code of extent lines
  vmdk: Clean up descriptor file reading
  vmdk: Check descriptor file length when reading it
  vmdk: Remove unnecessary initialization
  vmdk: Set errp on failures in vmdk_open_vmdk4

 block/vmdk.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH v3 5/6] vmdk: Remove unnecessary initialization

2014-12-03 Thread Fam Zheng
It will be assigned to the return value of vmdk_read_desc.

Suggested-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 045fd7a..ed492bd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -910,7 +910,7 @@ exit:
 static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
-char *buf = NULL;
+char *buf;
 int ret;
 BDRVVmdkState *s = bs-opaque;
 uint32_t magic;
-- 
1.9.3




[Qemu-devel] [PATCH v3 3/6] vmdk: Clean up descriptor file reading

2014-12-03 Thread Fam Zheng
Zeroing a buffer that will be filled right after is not necessary, and
allocating a power of two + 1 is naughty.

Suggested-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
---
 block/vmdk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 28d22db..82257cd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -557,8 +557,8 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 return NULL;
 }
 
-size = MIN(size, 1  20);  /* avoid unbounded allocation */
-buf = g_malloc0(size + 1);
+size = MIN(size, (1  20) - 1);  /* avoid unbounded allocation */
+buf = g_malloc(size + 1);
 
 ret = bdrv_pread(file, desc_offset, buf, size);
 if (ret  0) {
@@ -566,6 +566,7 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 g_free(buf);
 return NULL;
 }
+buf[ret] = 0;
 
 return buf;
 }
-- 
1.9.3




[Qemu-devel] [PATCH v3 2/6] vmdk: Fix comment to match code of extent lines

2014-12-03 Thread Fam Zheng
commit 04d542c8b (vmdk: support vmfs files) added support of VMFS extent
type but the comment above the changed code is left out. Update the
comment so they are consistent.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
 block/vmdk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index ebb4b70..28d22db 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -785,10 +785,11 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 VmdkExtent *extent;
 
 while (*p) {
-/* parse extent line:
+/* parse extent line in one of below formats:
+ *
  * RW [size in sectors] FLAT file-name.vmdk OFFSET
- * or
  * RW [size in sectors] SPARSE file-name.vmdk
+ * RW [size in sectors] VMFS file-name.vmdk
  */
 flat_offset = -1;
 ret = sscanf(p, %10s % SCNd64  %10s \%511[^\n\r\]\ % SCNd64,
-- 
1.9.3




Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around qemu_opts_del()

2014-12-03 Thread Paolo Bonzini


On 03/12/2014 11:28, Markus Armbruster wrote:
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  hw/core/qdev.c | 4 +---
  qemu-char.c| 4 +---
  2 files changed, 2 insertions(+), 6 deletions(-)
 
 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index 35fd00d..901f289 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -1141,9 +1141,7 @@ static void device_finalize(Object *obj)
  NamedGPIOList *ngl, *next;
  
  DeviceState *dev = DEVICE(obj);
 -if (dev-opts) {
 -qemu_opts_del(dev-opts);
 -}
 +qemu_opts_del(dev-opts);
  
  QLIST_FOREACH_SAFE(ngl, dev-gpios, node, next) {
  QLIST_REMOVE(ngl, node);
 diff --git a/qemu-char.c b/qemu-char.c
 index a8b01da..ef84b53 100644
 --- a/qemu-char.c
 +++ b/qemu-char.c
 @@ -3825,9 +3825,7 @@ void qemu_chr_delete(CharDriverState *chr)
  }
  g_free(chr-filename);
  g_free(chr-label);
 -if (chr-opts) {
 -qemu_opts_del(chr-opts);
 -}
 +qemu_opts_del(chr-opts);
  g_free(chr);
  }
  
 

Reviewed-by: Paolo Bonzini pbonz...@redhat.com



[Qemu-devel] [PATCH v3 6/6] vmdk: Set errp on failures in vmdk_open_vmdk4

2014-12-03 Thread Fam Zheng
Reported-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
 block/vmdk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index ed492bd..127479d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -642,6 +642,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 bs-file-total_sectors * 512 - 1536,
 footer, sizeof(footer));
 if (ret  0) {
+error_setg_errno(errp, -ret, Failed to read footer);
 return ret;
 }
 
@@ -653,6 +654,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 le32_to_cpu(footer.eos_marker.size) != 0  ||
 le32_to_cpu(footer.eos_marker.type) != MARKER_END_OF_STREAM)
 {
+error_setg(errp, Invalid footer);
 return -EINVAL;
 }
 
@@ -683,6 +685,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gt)
 * le64_to_cpu(header.granularity);
 if (l1_entry_sectors == 0) {
+error_setg(errp, L1 entry size is invalid);
 return -EINVAL;
 }
 l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
-- 
1.9.3




[Qemu-devel] [PATCH v3 1/6] vmdk: Use g_random_int to generate CID

2014-12-03 Thread Fam Zheng
This replaces two time(NULL) invocations with g_random_int().
According to VMDK spec, CID is a random 32‐bit value updated the first
time the content of the virtual disk is modified after the virtual disk
is opened. Using seconds since epoch is just a lame way to generate
it, and not completely safe because of the low precision.

Suggested-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
 block/vmdk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..ebb4b70 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -28,6 +28,7 @@
 #include qemu/module.h
 #include migration/migration.h
 #include zlib.h
+#include glib.h
 
 #define VMDK3_MAGIC (('C'  24) | ('O'  16) | ('W'  8) | 'D')
 #define VMDK4_MAGIC (('K'  24) | ('D'  16) | ('M'  8) | 'V')
@@ -1538,7 +1539,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 /* update CID on the first write every time the virtual disk is
  * opened */
 if (!s-cid_updated) {
-ret = vmdk_write_cid(bs, time(NULL));
+ret = vmdk_write_cid(bs, g_random_int());
 if (ret  0) {
 return ret;
 }
@@ -1922,7 +1923,7 @@ static int vmdk_create(const char *filename, QemuOpts 
*opts, Error **errp)
 }
 /* generate descriptor file */
 desc = g_strdup_printf(desc_template,
-   (uint32_t)time(NULL),
+   g_random_int(),
parent_cid,
fmt,
parent_desc_line,
-- 
1.9.3




[Qemu-devel] [PATCH v3 4/6] vmdk: Check descriptor file length when reading it

2014-12-03 Thread Fam Zheng
Since a too small file cannot be a valid VMDK image, and also since the
buffer's first 4 bytes will be unconditionally examined by
vmdk_open_sparse, let's error out the small file case to be clear.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
 block/vmdk.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 82257cd..045fd7a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -557,6 +557,11 @@ static char *vmdk_read_desc(BlockDriverState *file, 
uint64_t desc_offset,
 return NULL;
 }
 
+if (size  4) {
+error_setg_errno(errp, -size, File is too small, not a valid image);
+return NULL;
+}
+
 size = MIN(size, (1  20) - 1);  /* avoid unbounded allocation */
 buf = g_malloc(size + 1);
 
-- 
1.9.3




Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name

2014-12-03 Thread Kevin Wolf
[ CCed Benoît and Max, this is blockdev work ]
[ CCed Jeff, we're also talking about op blockers ]

Not stripping quoted text for their convenience.

Am 02.12.2014 um 20:06 hat Markus Armbruster geschrieben:
 = Introduction =
 
 The block layer and its monitor commands have evolved, resulting in a
 bit of a mess when it comes to referring to block layer objects in
 commands.
 
 Semantically, we refer to two different kinds of objects: backends and
 nodes within backends.
 
 Example: eject parameter @device names a backend.
 
 Example: change-backing-file parameter @image-node-name names a node.
 
 Backend and node names share a name space.  Names are unique.
 Corollary: a name unambiguously identifies either a backend, or a node,
 or nothing.
 
 Example: blockdev-add parameter @options is a union with discriminator
 driver.  With its value is raw, member file is an anonymous union.
 Its string value can name either a backend or a node.
 
 Node names are a fairly recent feature.  Before, we referenced nodes by
 their file name.  Pretty schlocky.  Should be replaced by node name.
 
 Example: block-commit parameter @base is the file name of the backing
 image to write data into.  In other words, it identifies a node.  We
 should add a node name parameter, and deprecate @base.
 
 Some commands predating the node name feature can reference only
 backends even though nodes could make sense, too.  Such restrictions
 should be lifted.  Others reference backends where only nodes make
 sense.  Should be cleaned up.
 
 Example: drive-mirror parameter @device names a backend.  This restricts
 mirroring to backends.  If we want to support mirroring nodes, we need
 to extend the command to permit referencing nodes.
 
 Example: block_passwd parameter @device names a backend.  However,
 backends aren't encryped, nodes are.  In 2.0, we cleaned this up: we
 added parameter @node-name.  We kept @device for backward compatibility.
 Either @device or @node-name must be given.
 
 Note: in block_passwd, we have separate parameters for backend and node
 name.  In the blockdev-add example above, we have only one, and use its
 value to figure out what it is.
 
 I find this inconsistency rather ugly.  We discussed cleaning it up in
 the BB name vs. BDS name in the QAPI schema thread.
 
 A backend has a tree of nodes.  We call the tree's root the backend's
 root node.
 
 For convenience, we sometimes accept a backend name when a node name is
 wanted, and resolve it to the backend's root node.
 
 Example: block_passwd again; it's how its @device parameter works.
 
 Recent development (blockdev-add) permits nodes to be shared by multiple
 backends.  Op blockers should ensure the sharing is safe.  I wouldn't
 bet a nickel on this to work today.
 
 
 = Block layer data structures =
 
 A backend is represented by a BlockBackend object (short: BB).
 
 A node is represented by a BlockDriverState object (short: BDS).
 
 A backend (BB) has a tree of nodes (BDSes).
 
 Two of a nodes children carry special meaning: the one stored in BDS
 member file (sometimes called parent), and the one stored in BDS
 member backing_hd (sometimes called backing).  These used to be the
 only children a node could have.
 
 A BB always has a name.  We sometimes call it device name.  Stored in BB
 member name.
 
 A BDS may have a name.  We call it node name.  Stored in BDS member
 node_name[], empty when the node has no name.
 
 A BDS has a file name.  The actual matching of a command's file name
 argument against a BDS's file name is complex, but we don't have to
 worry about that here.
 
 BBs are a fairly recent invention.  Before, a backend was represented by
 a BDS with a non-empty member device_name[].
 
 
 = Commands =
 
 I'm going to discuss all QMP commands whose handlers are defined in
 block*.  To make sense of it, you'll probably have to peruse the command
 documentation in the schema and/or qmp-commands.hx.
 
 When I think it's fairly obvious what needs to be done for a command, I
 write it down as TODO.  Please challenge it if you think I'm wrong.
 
 When it's not so obvious, I write it down as questions.  Answers
 appreciated.
 
 == block-core.json ==
 
 * block-commit
 
   @device names a backend, @top and @base each name one of its nodes via
   file name matching.
 
   TODO: support specifying the two nodes via node name.
 
   Why do we need @device when a top node is specified?
 
   * We currently need the backend to set op blockers, and finding a
 node's backend isn't easy.  Implementation detail shows through the
 interface, blech.
 
   * We need to know whether the top node is the active layer.
 
 Complication: if it's shared by multiple backends, it may be the
 active layer in one but not the other.  Specifying the backend
 resolves the ambiguity.  Whether that makes sense I don't know.

It doesn't.

The real requirement for the commit job (for inactive layers) is that
base...top (I'm using git-rev-parse syntax for 

Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around qemu_opts_del()

2014-12-03 Thread Gonglei
On 2014/12/3 18:28, Markus Armbruster wrote:

 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  hw/core/qdev.c | 4 +---
  qemu-char.c| 4 +---
  2 files changed, 2 insertions(+), 6 deletions(-)
 

Reviewed-by: Gonglei arei.gong...@huawei.com

 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index 35fd00d..901f289 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -1141,9 +1141,7 @@ static void device_finalize(Object *obj)
  NamedGPIOList *ngl, *next;
  
  DeviceState *dev = DEVICE(obj);
 -if (dev-opts) {
 -qemu_opts_del(dev-opts);
 -}
 +qemu_opts_del(dev-opts);
  
  QLIST_FOREACH_SAFE(ngl, dev-gpios, node, next) {
  QLIST_REMOVE(ngl, node);
 diff --git a/qemu-char.c b/qemu-char.c
 index a8b01da..ef84b53 100644
 --- a/qemu-char.c
 +++ b/qemu-char.c
 @@ -3825,9 +3825,7 @@ void qemu_chr_delete(CharDriverState *chr)
  }
  g_free(chr-filename);
  g_free(chr-label);
 -if (chr-opts) {
 -qemu_opts_del(chr-opts);
 -}
 +qemu_opts_del(chr-opts);
  g_free(chr);
  }
  






Re: [Qemu-devel] [PATCH 1/2] linux-headers: Update KVM headers from linux-next tag ToBeFilled

2014-12-03 Thread Christoffer Dall
On Tue, Dec 02, 2014 at 06:28:13PM +, Eric Auger wrote:
 Syncup KVM related linux headers from linux-next tree using
 scripts/update-linux-headers.sh.
 
 Add a new group/attribute in VGIC KVM device enabling to force
 vgic init: KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_VGIC_CTRL_INIT
 
 Signed-off-by: Eric Auger eric.au...@linaro.org

How can this be from linux-next?  I'm pretty sure we didn't merge this
patch yet.

Also, if you synced your headers from your local tree, I think that
generally makes such a QEMU patches series an RFC one.

-Christoffer



Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Michael S. Tsirkin
On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
 On Wed, 3 Dec 2014 10:27:36 +0100
 Cornelia Huck cornelia.h...@de.ibm.com wrote:
 
  On Tue, 2 Dec 2014 21:03:45 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
 {
+/*
+ * For virtio-1 devices, the number of buffers may only be
+ * updated if the ring addresses have not yet been set up.
   
   Where does it say that?
  
  Hmpf, may have imagined that.
  
  This means we either need to track whether used/avail have been
  specified or calculated or move responsibility for re-calculation of
  used/avail for the old layout into the callers.
 
 What about this one instead?

Looks ok overall - some questions below.

 diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
 index 43b7e02..1e2a720 100644
 --- a/hw/virtio/virtio-mmio.c
 +++ b/hw/virtio/virtio-mmio.c
 @@ -244,9 +244,13 @@ static void virtio_mmio_write(void *opaque, hwaddr 
 offset, uint64_t value,
  case VIRTIO_MMIO_QUEUENUM:
  DPRINTF(mmio_queue write %d max %d\n, (int)value, 
 VIRTQUEUE_MAX_SIZE);
  virtio_queue_set_num(vdev, vdev-queue_sel, value);
 +/* Note: only call this function for legacy devices */
 +virtio_queue_update_rings(vdev, vdev-queue_sel);
  break;
  case VIRTIO_MMIO_QUEUEALIGN:
 +/* Note: this is only valid for legacy devices */
  virtio_queue_set_align(vdev, vdev-queue_sel, value);
 +virtio_queue_update_rings(vdev, vdev-queue_sel);
  break;
  case VIRTIO_MMIO_QUEUEPFN:
  if (value == 0) {

Let's just call virtio_queue_update_rings from virtio_queue_set_align?



 diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
 index 8f69ffa..b2d553e 100644
 --- a/hw/virtio/virtio.c
 +++ b/hw/virtio/virtio.c
 @@ -69,7 +69,6 @@ typedef struct VRing
  struct VirtQueue
  {
  VRing vring;
 -hwaddr pa;
  uint16_t last_avail_idx;
  /* Last used index value we have signalled on */
  uint16_t signalled_used;
 @@ -92,15 +91,18 @@ struct VirtQueue
  };
  
  /* virt queue functions */
 -static void virtqueue_init(VirtQueue *vq)
 +void virtio_queue_update_rings(VirtIODevice *vdev, int n)
  {
 -hwaddr pa = vq-pa;
 +VRing *vring = vdev-vq[n].vring;
  
 -vq-vring.desc = pa;
 -vq-vring.avail = pa + vq-vring.num * sizeof(VRingDesc);
 -vq-vring.used = vring_align(vq-vring.avail +
 - offsetof(VRingAvail, ring[vq-vring.num]),
 - vq-vring.align);
 +if (!vring-desc) {
 +/* not yet setup - nothing to do */
 +return;
 +}
 +vring-avail = vring-desc + vring-num * sizeof(VRingDesc);
 +vring-used = vring_align(vring-avail +
 +  offsetof(VRingAvail, ring[vring-num]),
 +  vring-align);
  }
  
  static inline uint64_t vring_desc_addr(VirtIODevice *vdev, hwaddr desc_pa,
 @@ -605,7 +607,6 @@ void virtio_reset(void *opaque)
  vdev-vq[i].vring.avail = 0;
  vdev-vq[i].vring.used = 0;
  vdev-vq[i].last_avail_idx = 0;
 -vdev-vq[i].pa = 0;
  vdev-vq[i].vector = VIRTIO_NO_VECTOR;
  vdev-vq[i].signalled_used = 0;
  vdev-vq[i].signalled_used_valid = false;
 @@ -708,13 +709,21 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t 
 addr, uint32_t data)
  
  void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr)
  {
 -vdev-vq[n].pa = addr;
 -virtqueue_init(vdev-vq[n]);
 +vdev-vq[n].vring.desc = addr;
 +virtio_queue_update_rings(vdev, n);
  }
  
  hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
  {
 -return vdev-vq[n].pa;
 +return vdev-vq[n].vring.desc;
 +}
 +
 +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
 +hwaddr avail, hwaddr used)
 +{
 +vdev-vq[n].vring.desc = desc;
 +vdev-vq[n].vring.avail = avail;
 +vdev-vq[n].vring.used = used;
  }
  
  void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
 @@ -728,7 +737,6 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int 
 num)
  return;
  }
  vdev-vq[n].vring.num = num;
 -virtqueue_init(vdev-vq[n]);
  }
  
  int virtio_queue_get_num(VirtIODevice *vdev, int n)
 @@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, 
 int align)
  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
  
 +/* virtio-1 compliant devices cannot change the aligment */
 +if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
 +error_report(tried to modify queue alignment for virtio-1 device);
 +return;
 +}
  /* Check that the transport told us it was going to do this
   * (so a buggy transport will immediately assert rather than
   * 

Re: [Qemu-devel] [Xen-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap

2014-12-03 Thread Wei Liu
On Tue, Dec 02, 2014 at 03:23:29PM -0500, Don Slutz wrote:
[...]
hw_error(xc_domain_getinfo failed);
}
 -if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
 -(nr_pfn * XC_PAGE_SIZE / 1024))  0) {
 +max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE;
 +free_pages = max_pages - info.nr_pages;
 +real_free = free_pages;
 +if (free_pages  VGA_HOLE_SIZE) {
 +free_pages -= VGA_HOLE_SIZE;
 +} else {
 +free_pages = 0;
 +}
 I don't think we need to subtract VGA_HOLE_SIZE.
 
 If you do not use some slack (like 32 here), xen does report:
 
 
 (d5) [2014-11-29 17:07:21] Loaded SeaBIOS
 (d5) [2014-11-29 17:07:21] Creating MP tables ...
 (d5) [2014-11-29 17:07:21] Loading ACPI ...
 (XEN) [2014-11-29 17:07:21] page_alloc.c:1568:d5 Over-allocation for domain
 5: 1057417  1057416
 (XEN) [2014-11-29 17:07:21] memory.c:158:d5 Could not allocate order=0

This message is a bit red herring.

It's hvmloader trying to populate ram for firmware data. The actual
amount of extra pages needed depends on the firmware.

In any case it's safe to disallow hvmloader from doing so, it will just
relocate some pages from ram (hence shrinking *mem_end).

 extent: id=5 memflags=0 (0 of 1)
 (d5) [2014-11-29 17:07:21] vm86 TSS at 00098c00
 (d5) [2014-11-29 17:07:21] BIOS map:
 
 
 However while QEMU startup ends with 32 free pages (maxmem - curmem)
 this quickly changes to 23.  I.E. there are 7 more places that act like a
 call
 to xc_domain_populate_physmap_exact() but do not log errors if there is
 no free pages.  And just to make sure I do not fully understand what is
 happening here, after the message above, the domain appears to work
 fine and ends up with 8 free pages (code I do not know about ends up
 releasing populated pages).
 
 So my current thinking is to have QEMU leave a few (9 to 32 (64?)) pages
 free.
 

Unless we know before hand how many pages hvmloader needs this number is
estimation at best.

Wei.



Re: [Qemu-devel] [PATCH 1/2] linux-headers: Update KVM headers from linux-next tag ToBeFilled

2014-12-03 Thread Eric Auger
On 12/03/2014 11:49 AM, Christoffer Dall wrote:
 On Tue, Dec 02, 2014 at 06:28:13PM +, Eric Auger wrote:
 Syncup KVM related linux headers from linux-next tree using
 scripts/update-linux-headers.sh.

 Add a new group/attribute in VGIC KVM device enabling to force
 vgic init: KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_VGIC_CTRL_INIT

 Signed-off-by: Eric Auger eric.au...@linaro.org
 
 How can this be from linux-next?  I'm pretty sure we didn't merge this
 patch yet.

Hi Christoffer,

yes that's correct . I don't know how to format that kind of patch that
refers to linux headers (I mentioned in the title tag ToBeFilled). I
looked at other similar ones and I thought this was the wording that is
generally used. I can move it to RFC but it will stay as is until kernel
patches are upstreamed. In that case I should also move the QEMU VFIO
patch back to RFC due to forwarding stuff;-)

Best Regards

Eric
 
 Also, if you synced your headers from your local tree, I think that
 generally makes such a QEMU patches series an RFC one.
 
 -Christoffer
 




Re: [Qemu-devel] [PATCH 1/2] linux-headers: Update KVM headers from linux-next tag ToBeFilled

2014-12-03 Thread Christoffer Dall
On Wed, Dec 03, 2014 at 11:53:58AM +0100, Eric Auger wrote:
 On 12/03/2014 11:49 AM, Christoffer Dall wrote:
  On Tue, Dec 02, 2014 at 06:28:13PM +, Eric Auger wrote:
  Syncup KVM related linux headers from linux-next tree using
  scripts/update-linux-headers.sh.
 
  Add a new group/attribute in VGIC KVM device enabling to force
  vgic init: KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_VGIC_CTRL_INIT
 
  Signed-off-by: Eric Auger eric.au...@linaro.org
  
  How can this be from linux-next?  I'm pretty sure we didn't merge this
  patch yet.
 
 Hi Christoffer,
 
 yes that's correct . I don't know how to format that kind of patch that
 refers to linux headers (I mentioned in the title tag ToBeFilled). I
 looked at other similar ones and I thought this was the wording that is
 generally used. I can move it to RFC but it will stay as is until kernel
 patches are upstreamed. In that case I should also move the QEMU VFIO
 patch back to RFC due to forwarding stuff;-)
 
This patch subject should just be something like this:

Subject: [RFC PATCH 1/2] linux-headers: Update KVM headers for 
KVM_DEV_ARM_VGIC_GRP_CTRL

And in the commit message you point to a personal git tree that contains
the patches you're syncing against.

-Christoffer



[Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface

2014-12-03 Thread Paul Durrant
The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
models explicitly register with Xen for config space accesses. This patch
adds a listener interface into qdev-core which can be used by the Xen
interface code to monitor for arrival and departure of PCI devices.

Signed-off-by: Paul Durrant paul.durr...@citrix.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Andreas Faerber afaer...@suse.de
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Peter Crosthwaite peter.crosthwa...@xilinx.com
Cc: Igor Mammedov imamm...@redhat.com
Cc: Markus Armbruster arm...@redhat.com
Cc: Thomas Huth th...@linux.vnet.ibm.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
---
 hw/core/qdev.c  |   54 +++
 include/hw/qdev-core.h  |   10 +
 include/qemu/typedefs.h |1 +
 3 files changed, 65 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fcb1638..4a9c1f6 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
 return 0;
 }
 
+static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
+= QTAILQ_HEAD_INITIALIZER(qdev_listeners);
+
+enum ListenerDirection { Forward, Reverse };
+
+#define QDEV_LISTENER_CALL(_callback, _direction, _args...) \
+do {\
+DeviceListener *_listener;  \
+\
+switch (_direction) {   \
+case Forward:   \
+QTAILQ_FOREACH(_listener, qdev_listeners, link) {  \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args);   \
+}   \
+}   \
+break;  \
+case Reverse:   \
+QTAILQ_FOREACH_REVERSE(_listener, qdev_listeners,  \
+   qdev_listeners, link) {  \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args);   \
+}   \
+}   \
+break;  \
+default:\
+abort();\
+}   \
+} while (0)
+
+static int qdev_listener_add(DeviceState *dev, void *opaque)
+{
+QDEV_LISTENER_CALL(realize, Forward, dev);
+
+return 0;
+}
+
+void qdev_listener_register(DeviceListener *listener)
+{
+QTAILQ_INSERT_TAIL(qdev_listeners, listener, link);
+
+qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
+   NULL, NULL);
+}
+
+void qdev_listener_unregister(DeviceListener *listener)
+{
+QTAILQ_REMOVE(qdev_listeners, listener, link);
+}
+
 static void device_realize(DeviceState *dev, Error **errp)
 {
 DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error **errp)
 return;
 }
 }
+
+QDEV_LISTENER_CALL(realize, Forward, dev);
 }
 
 static void device_unrealize(DeviceState *dev, Error **errp)
 {
 DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
+QDEV_LISTENER_CALL(unrealize, Reverse, dev);
+
 if (dc-exit) {
 int rc = dc-exit(dev);
 if (rc  0) {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 178fee2..f2dc267 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -167,6 +167,12 @@ struct DeviceState {
 int alias_required_for_version;
 };
 
+struct DeviceListener {
+void (*realize)(DeviceListener *listener, DeviceState *dev);
+void (*unrealize)(DeviceListener *listener, DeviceState *dev);
+QTAILQ_ENTRY(DeviceListener) link;
+};
+
 #define TYPE_BUS bus
 #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
 #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
@@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState *bus, 
DeviceState *handler,
  QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
 bus-allow_hotplug = 1;
 }
+
+void qdev_listener_register(DeviceListener *listener);
+void qdev_listener_unregister(DeviceListener *listener);
+
 #endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 04df51b..e32bca2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -20,6 +20,7 @@ typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
 typedef struct CompatProperty 

[Qemu-devel] [PATCH v4 REPOST 2/2] Use ioreq-server API

2014-12-03 Thread Paul Durrant
This patch series is a re-post v4 of what was originally the single
patch Xen: Use the ioreq-server API when available. It was originally
posted on October 16th and pinged on October 29th. I am still awaiting
an ack/nack for patch #1, hence the re-post.

v2 of the series moved the code that added the PCI bus listener
to patch #1 and the remainder of the changes to patch #2. Patch #2
was then re-worked to constrain the #ifdefing to xen_common.h, as
requested by Stefano.

v3 of the series modifies patch #1 to add the listener interface
into the core qdev, rather than the PCI bus code. This change only
requires trivial modification to patch #2, to only act on realize/
unrealize of PCI devices. Patch #2 was also modified at Stefano's
request to remove an extra identity check of memory sections
against the ram region.

v4 of the series replaces the use of a vmstate pre_save callback
with extra code in the existing runstate change notification
callback. It also tidies up some things in xen-hvm.c pointed out
by Stefano and adds his ack to patch #2.




Re: [Qemu-devel] [PATCH 1/2] linux-headers: Update KVM headers from linux-next tag ToBeFilled

2014-12-03 Thread Eric Auger
On 12/03/2014 12:03 PM, Christoffer Dall wrote:
 On Wed, Dec 03, 2014 at 11:53:58AM +0100, Eric Auger wrote:
 On 12/03/2014 11:49 AM, Christoffer Dall wrote:
 On Tue, Dec 02, 2014 at 06:28:13PM +, Eric Auger wrote:
 Syncup KVM related linux headers from linux-next tree using
 scripts/update-linux-headers.sh.

 Add a new group/attribute in VGIC KVM device enabling to force
 vgic init: KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_VGIC_CTRL_INIT

 Signed-off-by: Eric Auger eric.au...@linaro.org

 How can this be from linux-next?  I'm pretty sure we didn't merge this
 patch yet.

 Hi Christoffer,

 yes that's correct . I don't know how to format that kind of patch that
 refers to linux headers (I mentioned in the title tag ToBeFilled). I
 looked at other similar ones and I thought this was the wording that is
 generally used. I can move it to RFC but it will stay as is until kernel
 patches are upstreamed. In that case I should also move the QEMU VFIO
 patch back to RFC due to forwarding stuff;-)

 This patch subject should just be something like this:
 
 Subject: [RFC PATCH 1/2] linux-headers: Update KVM headers for 
 KVM_DEV_ARM_VGIC_GRP_CTRL
 
 And in the commit message you point to a personal git tree that contains
 the patches you're syncing against.
OK thanks

I will update accordingly

Eric
 
 -Christoffer
 




[Qemu-devel] [PATCH v4 REPOST 2/2] Xen: Use the ioreq-server API when available

2014-12-03 Thread Paul Durrant
The ioreq-server API added to Xen 4.5 offers better security than
the existing Xen/QEMU interface because the shared pages that are
used to pass emulation request/results back and forth are removed
from the guest's memory space before any requests are serviced.
This prevents the guest from mapping these pages (they are in a
well known location) and attempting to attack QEMU by synthesizing
its own request structures. Hence, this patch modifies configure
to detect whether the API is available, and adds the necessary
code to use the API if it is.

Signed-off-by: Paul Durrant paul.durr...@citrix.com
Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Cc: Peter Maydell peter.mayd...@linaro.org
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Michael Tokarev m...@tls.msk.ru
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Stefan Weil s...@weilnetz.de
Cc: Olaf Hering o...@aepfle.de
Cc: Gerd Hoffmann kra...@redhat.com
Cc: Alexey Kardashevskiy a...@ozlabs.ru
Cc: Alexander Graf ag...@suse.de
---
 configure   |   29 ++
 include/hw/xen/xen_common.h |  223 +++
 trace-events|9 ++
 xen-hvm.c   |  156 ++
 4 files changed, 396 insertions(+), 21 deletions(-)

diff --git a/configure b/configure
index 9ac2600..c2db574 100755
--- a/configure
+++ b/configure
@@ -1876,6 +1876,32 @@ int main(void) {
   xc_gnttab_open(NULL, 0);
   xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
   xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+  xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
+  return 0;
+}
+EOF
+  compile_prog  $xen_libs
+then
+xen_ctrl_version=450
+xen=yes
+
+  elif
+  cat  $TMPC EOF 
+#include xenctrl.h
+#include xenstore.h
+#include stdint.h
+#include xen/hvm/hvm_info_table.h
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xc_interface *xc;
+  xs_daemon_open();
+  xc = xc_interface_open(0, 0, 0);
+  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+  xc_gnttab_open(NULL, 0);
+  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
   return 0;
 }
 EOF
@@ -4282,6 +4308,9 @@ if test -n $sparc_cpu; then
 echo Target Sparc Arch $sparc_cpu
 fi
 echo xen support   $xen
+if test $xen = yes ; then
+  echo xen ctrl version  $xen_ctrl_version
+fi
 echo brlapi support$brlapi
 echo bluez  support$bluez
 echo Documentation $docs
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 07731b9..aec1372 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -16,7 +16,9 @@
 
 #include hw/hw.h
 #include hw/xen/xen.h
+#include hw/pci/pci.h
 #include qemu/queue.h
+#include trace.h
 
 /*
  * We don't support Xen prior to 3.3.0.
@@ -164,4 +166,225 @@ void destroy_hvm_domain(bool reboot);
 /* shutdown/destroy current domain because of an error */
 void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
+/* Xen before 4.5 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION  450
+
+#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
+#define HVM_PARAM_BUFIOREQ_EVTCHN 26
+#endif
+
+#define IOREQ_TYPE_PCI_CONFIG 2
+
+typedef uint32_t ioservid_t;
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  PCIDevice *pci_dev)
+{
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+PCIDevice *pci_dev)
+{
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+  ioservid_t *ioservid)
+{
+return 0;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+ioservid_t ioservid)
+{
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+xen_pfn_t *ioreq_pfn,
+

Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name

2014-12-03 Thread Paolo Bonzini


On 03/12/2014 06:52, Fam Zheng wrote:
  * drive-backup
  
@device names a backend.
  
Do we want to be able to back up any node, or only a backend?

If non-backend nodes are read-only, we can just copy them outside QEMU.

Note: documentation of @target sounds like it could somehow name a
backend, but as far as I can tell it's always interpreted as file
name.

I think block-backup should be added that takes a backend for target.
It could also take a node name for the source, letting you copy any node
if you really want to.

  * drive-mirror
  
@device names a backend, @replaces names a node, and @node-name
defines the name of the new node.
  
Do we want to be able to mirror any node, or only a backend?

Same as above.

Note: documentation of @target sounds like it could somehow name a
backend, but as far as I can tell it's always interpreted as file
name.

Again, block-mirror could be added that takes a backend for @target (and
if you want to, a node name for the source).

Paolo



Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Cornelia Huck
On Wed, 3 Dec 2014 12:52:51 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:

  diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
  index 43b7e02..1e2a720 100644
  --- a/hw/virtio/virtio-mmio.c
  +++ b/hw/virtio/virtio-mmio.c
  @@ -244,9 +244,13 @@ static void virtio_mmio_write(void *opaque, hwaddr 
  offset, uint64_t value,
   case VIRTIO_MMIO_QUEUENUM:
   DPRINTF(mmio_queue write %d max %d\n, (int)value, 
  VIRTQUEUE_MAX_SIZE);
   virtio_queue_set_num(vdev, vdev-queue_sel, value);
  +/* Note: only call this function for legacy devices */
  +virtio_queue_update_rings(vdev, vdev-queue_sel);
   break;
   case VIRTIO_MMIO_QUEUEALIGN:
  +/* Note: this is only valid for legacy devices */
   virtio_queue_set_align(vdev, vdev-queue_sel, value);
  +virtio_queue_update_rings(vdev, vdev-queue_sel);
   break;
   case VIRTIO_MMIO_QUEUEPFN:
   if (value == 0) {
 
 Let's just call virtio_queue_update_rings from virtio_queue_set_align?

You're right, set_align is legacy only so we can always call
update_rings.


  @@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, 
  int align)
   BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
   VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
   
  +/* virtio-1 compliant devices cannot change the aligment */
  +if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
  +error_report(tried to modify queue alignment for virtio-1 
  device);
  +return;
  +}
   /* Check that the transport told us it was going to do this
* (so a buggy transport will immediately assert rather than
* silently failing to migrate this state)
 
 Do we have to touch this now?
 It's only used by MMIO, right?

I don't think it hurts to put a guard in here.
 
 
  @@ -755,7 +768,6 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, 
  int align)
   assert(k-has_variable_vring_alignment);
   
   vdev-vq[n].vring.align = align;
  -virtqueue_init(vdev-vq[n]);
 
 Don't we need to update rings?

See above, I'll call update_rings in there.

 
   }
   
   void virtio_queue_notify_vq(VirtQueue *vq)
  @@ -949,7 +961,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
   if (k-has_variable_vring_alignment) {
   qemu_put_be32(f, vdev-vq[i].vring.align);
   }
  -qemu_put_be64(f, vdev-vq[i].pa);
  +/* XXX virtio-1 devices */
  +qemu_put_be64(f, vdev-vq[i].vring.desc);
   qemu_put_be16s(f, vdev-vq[i].last_avail_idx);
   if (k-save_queue) {
   k-save_queue(qbus-parent, i, f);
  @@ -1044,13 +1057,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, 
  int version_id)
   if (k-has_variable_vring_alignment) {
   vdev-vq[i].vring.align = qemu_get_be32(f);
   }
  -vdev-vq[i].pa = qemu_get_be64(f);
  +vdev-vq[i].vring.desc = qemu_get_be64(f);
   qemu_get_be16s(f, vdev-vq[i].last_avail_idx);
   vdev-vq[i].signalled_used_valid = false;
   vdev-vq[i].notification = true;
   
  -if (vdev-vq[i].pa) {
  -virtqueue_init(vdev-vq[i]);
  +if (vdev-vq[i].vring.desc) {
  +/* XXX virtio-1 devices */
 
 What does XXX mean here?

That I have not cared about migration of virtio-1 devices yet :)

 
  +virtio_queue_update_rings(vdev, i);
   } else if (vdev-vq[i].last_avail_idx) {
   error_report(VQ %d address 0x0 
inconsistent with Host index 0x%x,




Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Michael S. Tsirkin
On Wed, Dec 03, 2014 at 12:14:10PM +0100, Cornelia Huck wrote:
 On Wed, 3 Dec 2014 12:52:51 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
 
   diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
   index 43b7e02..1e2a720 100644
   --- a/hw/virtio/virtio-mmio.c
   +++ b/hw/virtio/virtio-mmio.c
   @@ -244,9 +244,13 @@ static void virtio_mmio_write(void *opaque, hwaddr 
   offset, uint64_t value,
case VIRTIO_MMIO_QUEUENUM:
DPRINTF(mmio_queue write %d max %d\n, (int)value, 
   VIRTQUEUE_MAX_SIZE);
virtio_queue_set_num(vdev, vdev-queue_sel, value);
   +/* Note: only call this function for legacy devices */
   +virtio_queue_update_rings(vdev, vdev-queue_sel);
break;
case VIRTIO_MMIO_QUEUEALIGN:
   +/* Note: this is only valid for legacy devices */
virtio_queue_set_align(vdev, vdev-queue_sel, value);
   +virtio_queue_update_rings(vdev, vdev-queue_sel);
break;
case VIRTIO_MMIO_QUEUEPFN:
if (value == 0) {
  
  Let's just call virtio_queue_update_rings from virtio_queue_set_align?
 
 You're right, set_align is legacy only so we can always call
 update_rings.
 
 
   @@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int 
   n, int align)
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);

   +/* virtio-1 compliant devices cannot change the aligment */
   +if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
   +error_report(tried to modify queue alignment for virtio-1 
   device);
   +return;
   +}
/* Check that the transport told us it was going to do this
 * (so a buggy transport will immediately assert rather than
 * silently failing to migrate this state)
  
  Do we have to touch this now?
  It's only used by MMIO, right?
 
 I don't think it hurts to put a guard in here.

I'd say let's not touch mmio ATM.

  
  
   @@ -755,7 +768,6 @@ void virtio_queue_set_align(VirtIODevice *vdev, int 
   n, int align)
assert(k-has_variable_vring_alignment);

vdev-vq[n].vring.align = align;
   -virtqueue_init(vdev-vq[n]);
  
  Don't we need to update rings?
 
 See above, I'll call update_rings in there.
 
  
}

void virtio_queue_notify_vq(VirtQueue *vq)
   @@ -949,7 +961,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
if (k-has_variable_vring_alignment) {
qemu_put_be32(f, vdev-vq[i].vring.align);
}
   -qemu_put_be64(f, vdev-vq[i].pa);
   +/* XXX virtio-1 devices */
   +qemu_put_be64(f, vdev-vq[i].vring.desc);
qemu_put_be16s(f, vdev-vq[i].last_avail_idx);
if (k-save_queue) {
k-save_queue(qbus-parent, i, f);
   @@ -1044,13 +1057,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, 
   int version_id)
if (k-has_variable_vring_alignment) {
vdev-vq[i].vring.align = qemu_get_be32(f);
}
   -vdev-vq[i].pa = qemu_get_be64(f);
   +vdev-vq[i].vring.desc = qemu_get_be64(f);
qemu_get_be16s(f, vdev-vq[i].last_avail_idx);
vdev-vq[i].signalled_used_valid = false;
vdev-vq[i].notification = true;

   -if (vdev-vq[i].pa) {
   -virtqueue_init(vdev-vq[i]);
   +if (vdev-vq[i].vring.desc) {
   +/* XXX virtio-1 devices */
  
  What does XXX mean here?
 
 That I have not cared about migration of virtio-1 devices yet :)

OK sure, but why put comment here not at start of
function?

  
   +virtio_queue_update_rings(vdev, i);
} else if (vdev-vq[i].last_avail_idx) {
error_report(VQ %d address 0x0 
 inconsistent with Host index 0x%x,



Re: [Qemu-devel] [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout

2014-12-03 Thread Cornelia Huck
On Wed, 3 Dec 2014 13:19:17 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Wed, Dec 03, 2014 at 12:14:10PM +0100, Cornelia Huck wrote:
  On Wed, 3 Dec 2014 12:52:51 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:

@@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, 
int n, int align)
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
+/* virtio-1 compliant devices cannot change the aligment */
+if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+error_report(tried to modify queue alignment for virtio-1 
device);
+return;
+}
 /* Check that the transport told us it was going to do this
  * (so a buggy transport will immediately assert rather than
  * silently failing to migrate this state)
   
   Do we have to touch this now?
   It's only used by MMIO, right?
  
  I don't think it hurts to put a guard in here.
 
 I'd say let's not touch mmio ATM.

This is not mmio but common code :) I don't really see how this can
possibly hurt us; when mmio is converted to virtio-1, their queue setup
code needs to be changed anyway.


@@ -949,7 +961,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 if (k-has_variable_vring_alignment) {
 qemu_put_be32(f, vdev-vq[i].vring.align);
 }
-qemu_put_be64(f, vdev-vq[i].pa);
+/* XXX virtio-1 devices */
+qemu_put_be64(f, vdev-vq[i].vring.desc);
 qemu_put_be16s(f, vdev-vq[i].last_avail_idx);
 if (k-save_queue) {
 k-save_queue(qbus-parent, i, f);
@@ -1044,13 +1057,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile 
*f, int version_id)
 if (k-has_variable_vring_alignment) {
 vdev-vq[i].vring.align = qemu_get_be32(f);
 }
-vdev-vq[i].pa = qemu_get_be64(f);
+vdev-vq[i].vring.desc = qemu_get_be64(f);
 qemu_get_be16s(f, vdev-vq[i].last_avail_idx);
 vdev-vq[i].signalled_used_valid = false;
 vdev-vq[i].notification = true;
 
-if (vdev-vq[i].pa) {
-virtqueue_init(vdev-vq[i]);
+if (vdev-vq[i].vring.desc) {
+/* XXX virtio-1 devices */
   
   What does XXX mean here?
  
  That I have not cared about migration of virtio-1 devices yet :)
 
 OK sure, but why put comment here not at start of
 function?

I find it easier to annotate the places I notice. YMMV.

 
   
+virtio_queue_update_rings(vdev, i);
 } else if (vdev-vq[i].last_avail_idx) {
 error_report(VQ %d address 0x0 
  inconsistent with Host index 0x%x,
 




Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically

2014-12-03 Thread Gerd Hoffmann
  Hi,

 Hi, Daniel
 Actually, my team had received the requirement of changing VNC keyboard
 layout dynamically on the scenario of  Desktop Cloud. The clientele just use
 the simplest tight vnc client, but not GTK-VNC etc. I think we should support
 this scenario, isn't it ?

It boils down to doing the keysym - scancode translation on the server
side (tightvnc client, qemu vnc server needs -k) or on the client side
(anything gtk-vnc based, such as virt-viewer / remote-viewer or vinagre
(gnome vnc viewer)).

The big advantage of doing it on the client side (then send the
scancodes using the scancode extension as mentioned by Daniel) is that
it works without any manual configuration, and you can even have two vnc
clients with different local keymaps connected at the same time and
things are still working properly.

In case the client can't do the translation qemu will fallback to do it
on the server side.  It's not the recommended way to operate though.

cheers,
  Gerd





Re: [Qemu-devel] [Xen-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap

2014-12-03 Thread Stefano Stabellini
On Wed, 3 Dec 2014, Wei Liu wrote:
 On Tue, Dec 02, 2014 at 03:23:29PM -0500, Don Slutz wrote:
 [...]
 hw_error(xc_domain_getinfo failed);
 }
  -if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
  -(nr_pfn * XC_PAGE_SIZE / 1024))  0) {
  +max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE;
  +free_pages = max_pages - info.nr_pages;
  +real_free = free_pages;
  +if (free_pages  VGA_HOLE_SIZE) {
  +free_pages -= VGA_HOLE_SIZE;
  +} else {
  +free_pages = 0;
  +}
  I don't think we need to subtract VGA_HOLE_SIZE.
  
  If you do not use some slack (like 32 here), xen does report:
  
  
  (d5) [2014-11-29 17:07:21] Loaded SeaBIOS
  (d5) [2014-11-29 17:07:21] Creating MP tables ...
  (d5) [2014-11-29 17:07:21] Loading ACPI ...
  (XEN) [2014-11-29 17:07:21] page_alloc.c:1568:d5 Over-allocation for domain
  5: 1057417  1057416
  (XEN) [2014-11-29 17:07:21] memory.c:158:d5 Could not allocate order=0
 
 This message is a bit red herring.
 
 It's hvmloader trying to populate ram for firmware data. The actual
 amount of extra pages needed depends on the firmware.
 
 In any case it's safe to disallow hvmloader from doing so, it will just
 relocate some pages from ram (hence shrinking *mem_end).

That looks like a better solution


  extent: id=5 memflags=0 (0 of 1)
  (d5) [2014-11-29 17:07:21] vm86 TSS at 00098c00
  (d5) [2014-11-29 17:07:21] BIOS map:
  
  
  However while QEMU startup ends with 32 free pages (maxmem - curmem)
  this quickly changes to 23.  I.E. there are 7 more places that act like a
  call
  to xc_domain_populate_physmap_exact() but do not log errors if there is
  no free pages.  And just to make sure I do not fully understand what is
  happening here, after the message above, the domain appears to work
  fine and ends up with 8 free pages (code I do not know about ends up
  releasing populated pages).
  
  So my current thinking is to have QEMU leave a few (9 to 32 (64?)) pages
  free.
  
 
 Unless we know before hand how many pages hvmloader needs this number is
 estimation at best.
 
Right. It would be nice to get rid of any estimations by:
- making hvmloader use normal ram
- making qemu increase maxmem
- removing all the estimation from libxl



Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface

2014-12-03 Thread Stefano Stabellini
The second patch is already Acked.
You just need a review on this one to move forward, right?

Andreas, Michael?

On Wed, 3 Dec 2014, Paul Durrant wrote:
 The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
 models explicitly register with Xen for config space accesses. This patch
 adds a listener interface into qdev-core which can be used by the Xen
 interface code to monitor for arrival and departure of PCI devices.
 
 Signed-off-by: Paul Durrant paul.durr...@citrix.com
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Andreas Faerber afaer...@suse.de
 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Peter Crosthwaite peter.crosthwa...@xilinx.com
 Cc: Igor Mammedov imamm...@redhat.com
 Cc: Markus Armbruster arm...@redhat.com
 Cc: Thomas Huth th...@linux.vnet.ibm.com
 Cc: Christian Borntraeger borntrae...@de.ibm.com
 ---
  hw/core/qdev.c  |   54 
 +++
  include/hw/qdev-core.h  |   10 +
  include/qemu/typedefs.h |1 +
  3 files changed, 65 insertions(+)
 
 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index fcb1638..4a9c1f6 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
  return 0;
  }
  
 +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
 += QTAILQ_HEAD_INITIALIZER(qdev_listeners);
 +
 +enum ListenerDirection { Forward, Reverse };
 +
 +#define QDEV_LISTENER_CALL(_callback, _direction, _args...) \
 +do {\
 +DeviceListener *_listener;  \
 +\
 +switch (_direction) {   \
 +case Forward:   \
 +QTAILQ_FOREACH(_listener, qdev_listeners, link) {  \
 +if (_listener-_callback) { \
 +_listener-_callback(_listener, ##_args);   \
 +}   \
 +}   \
 +break;  \
 +case Reverse:   \
 +QTAILQ_FOREACH_REVERSE(_listener, qdev_listeners,  \
 +   qdev_listeners, link) {  \
 +if (_listener-_callback) { \
 +_listener-_callback(_listener, ##_args);   \
 +}   \
 +}   \
 +break;  \
 +default:\
 +abort();\
 +}   \
 +} while (0)
 +
 +static int qdev_listener_add(DeviceState *dev, void *opaque)
 +{
 +QDEV_LISTENER_CALL(realize, Forward, dev);
 +
 +return 0;
 +}
 +
 +void qdev_listener_register(DeviceListener *listener)
 +{
 +QTAILQ_INSERT_TAIL(qdev_listeners, listener, link);
 +
 +qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
 +   NULL, NULL);
 +}
 +
 +void qdev_listener_unregister(DeviceListener *listener)
 +{
 +QTAILQ_REMOVE(qdev_listeners, listener, link);
 +}
 +
  static void device_realize(DeviceState *dev, Error **errp)
  {
  DeviceClass *dc = DEVICE_GET_CLASS(dev);
 @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error 
 **errp)
  return;
  }
  }
 +
 +QDEV_LISTENER_CALL(realize, Forward, dev);
  }
  
  static void device_unrealize(DeviceState *dev, Error **errp)
  {
  DeviceClass *dc = DEVICE_GET_CLASS(dev);
  
 +QDEV_LISTENER_CALL(unrealize, Reverse, dev);
 +
  if (dc-exit) {
  int rc = dc-exit(dev);
  if (rc  0) {
 diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
 index 178fee2..f2dc267 100644
 --- a/include/hw/qdev-core.h
 +++ b/include/hw/qdev-core.h
 @@ -167,6 +167,12 @@ struct DeviceState {
  int alias_required_for_version;
  };
  
 +struct DeviceListener {
 +void (*realize)(DeviceListener *listener, DeviceState *dev);
 +void (*unrealize)(DeviceListener *listener, DeviceState *dev);
 +QTAILQ_ENTRY(DeviceListener) link;
 +};
 +
  #define TYPE_BUS bus
  #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
  #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
 @@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState 
 *bus, DeviceState *handler,
   QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
  bus-allow_hotplug = 1;
  }
 +
 +void qdev_listener_register(DeviceListener *listener);
 +void qdev_listener_unregister(DeviceListener *listener);
 +
  #endif
 diff 

[Qemu-devel] [PATCH] ide: Check validity of logical block size

2014-12-03 Thread Kevin Wolf
Our IDE emulation can't handle logical block sizes other than 512. Check
for it.

The original assumption was that other values would silently be ignored
(which is bad enough), but it's not quite true: The physical block size
is exposed in IDENTIFY DEVICE as a multiple of the logical block size.
Setting a logical block size therefore also corrupts the physical block
size (4096/4096 doesn't silently downgrade to 4096/512, but 512/512).

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 hw/ide/qdev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b4f096e..1ebb58d 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -163,6 +163,11 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind)
 return -1;
 }
 
+if (dev-conf.logical_block_size != 512) {
+error_report(logical_block_size must be 512 for IDE);
+return -1;
+}
+
 blkconf_serial(dev-conf, dev-serial);
 if (kind != IDE_CD) {
 blkconf_geometry(dev-conf, dev-chs_trans, 65536, 16, 255, err);
-- 
1.8.3.1




[Qemu-devel] qemu rdma live migration

2014-12-03 Thread Vasiliy Tolstov
Hello. I don't find info about how qemu doing live migration via RDAM
- my specific qeustion - does it need ethernet connection for some
info, or can use native RDMA and resolve nodes via GUIDs ?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru



Re: [Qemu-devel] [PATCH] iotests: Specify qcow2 format for qemu-io in 059

2014-12-03 Thread Kevin Wolf
Am 03.12.2014 um 10:15 hat Max Reitz geschrieben:
 There are two instances of iotest 059 using qemu-io on a qcow2 image. As
 of qemu-iotests: Use qemu-io -f $IMGFMT the iotests can no longer rely
 on $QEMU_IO doing probing, therefore the qcow2 format has to be
 specified explicitly here.
 
 Signed-off-by: Max Reitz mre...@redhat.com

Thanks, applied to block-next.

Kevin



Re: [Qemu-devel] [PATCH 1/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Kevin Wolf
Am 03.12.2014 um 10:23 hat Max Reitz geschrieben:
 If vmdk blindly tries to use path_combine() using bs-file-filename as
 the base file name, this will result in a bad error message for JSON
 file names when calling bdrv_open(). It is better to only try
 bs-file-exact_filename; if that is empty, bs-file-filename will be
 useless for path_combine() and an error should be emitted (containing
 bs-file-filename because bs-file-exact_filename is empty).
 
 Note that s-create_type does not need to be freed on error because it
 will be freed by the caller (which ultimately is vmdk_open()).
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block/vmdk.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/block/vmdk.c b/block/vmdk.c
 index 2cbfd3e..fe549c2 100644
 --- a/block/vmdk.c
 +++ b/block/vmdk.c
 @@ -894,7 +894,14 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
 flags, char *buf,
  }
  s-create_type = g_strdup(ct);
  s-desc_offset = 0;
 -ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
 +
 +if (!bs-file-exact_filename[0]) {
 +error_setg(errp, Cannot use extents with VMDK descriptor file '%s',
 +   bs-file-filename);
 +ret = -EINVAL;
 +goto exit;
 +}

Isn't this overly restrictive? If the extent paths are all absolute,
there is no reason not to open them. Or does the VMDK spec say that they
are always relative?

 +ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp);
  exit:
  return ret;
  }

Kevin



Re: [Qemu-devel] [PATCH 0/3 V1] add PCI support for the s390 platform

2014-12-03 Thread Cornelia Huck
On Wed, 26 Nov 2014 10:05:10 +0100
Frank Blaschka blasc...@linux.vnet.ibm.com wrote:

 This set of patches implemets PCI support for the s390 platform.
 Now it is possible to run virtio-net-pci and potentially all
 virtual pci devices conforming to s390 platform constrains.
 
 V1 added lot of feedback from Alex Graf
fixed tons of endian issues
 
 Please review and consider for integration into 2.3

Looks good; added to my s390-next queue.

Alex: Any chance of some r-bs from you?




Re: [Qemu-devel] [PATCH 1/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Max Reitz

On 2014-12-03 at 13:44, Kevin Wolf wrote:

Am 03.12.2014 um 10:23 hat Max Reitz geschrieben:

If vmdk blindly tries to use path_combine() using bs-file-filename as
the base file name, this will result in a bad error message for JSON
file names when calling bdrv_open(). It is better to only try
bs-file-exact_filename; if that is empty, bs-file-filename will be
useless for path_combine() and an error should be emitted (containing
bs-file-filename because bs-file-exact_filename is empty).

Note that s-create_type does not need to be freed on error because it
will be freed by the caller (which ultimately is vmdk_open()).

Signed-off-by: Max Reitz mre...@redhat.com
---
  block/vmdk.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..fe549c2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -894,7 +894,14 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags, char *buf,
  }
  s-create_type = g_strdup(ct);
  s-desc_offset = 0;
-ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
+
+if (!bs-file-exact_filename[0]) {
+error_setg(errp, Cannot use extents with VMDK descriptor file '%s',
+   bs-file-filename);
+ret = -EINVAL;
+goto exit;
+}

Isn't this overly restrictive? If the extent paths are all absolute,
there is no reason not to open them. Or does the VMDK spec say that they
are always relative?


Yes, you're right. Will respin.

Max


+ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp);
  exit:
  return ret;
  }

Kevin





[Qemu-devel] [PATCH for 2.3 v2 1/1] xen-hvm: increase maxmem before calling xc_domain_populate_physmap

2014-12-03 Thread Don Slutz
From: Stefano Stabellini stefano.stabell...@eu.citrix.com

Increase maxmem before calling xc_domain_populate_physmap_exact to
avoid the risk of running out of guest memory. This way we can also
avoid complex memory calculations in libxl at domain construction
time.

This patch fixes an abort() when assigning more than 4 NICs to a VM.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Signed-off-by: Don Slutz dsl...@verizon.com
---
v2: Changes by Don Slutz
  Switch from xc_domain_getinfo to xc_domain_getinfolist
  Fix error check for xc_domain_getinfolist
  Limit increase of maxmem to only do when needed:
Add QEMU_SPARE_PAGES (How many pages to leave free)
Add free_pages calculation

 xen-hvm.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/xen-hvm.c b/xen-hvm.c
index 7548794..d30e77e 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -90,6 +90,7 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t 
*shared_page, int vcpu)
 #endif
 
 #define BUFFER_IO_MAX_DELAY  100
+#define QEMU_SPARE_PAGES 16
 
 typedef struct XenPhysmap {
 hwaddr start_addr;
@@ -244,6 +245,8 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr)
 unsigned long nr_pfn;
 xen_pfn_t *pfn_list;
 int i;
+xc_domaininfo_t info;
+unsigned long free_pages;
 
 if (runstate_check(RUN_STATE_INMIGRATE)) {
 /* RAM already populated in Xen */
@@ -266,6 +269,22 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr)
 pfn_list[i] = (ram_addr  TARGET_PAGE_BITS) + i;
 }
 
+if ((xc_domain_getinfolist(xen_xc, xen_domid, 1, info) != 1) ||
+(info.domain != xen_domid)) {
+hw_error(xc_domain_getinfolist failed);
+}
+free_pages = info.max_pages - info.tot_pages;
+if (free_pages  QEMU_SPARE_PAGES) {
+free_pages -= QEMU_SPARE_PAGES;
+} else {
+free_pages = 0;
+}
+if ((free_pages  nr_pfn) 
+(xc_domain_setmaxmem(xen_xc, xen_domid,
+ ((info.max_pages + nr_pfn - free_pages)
+   (XC_PAGE_SHIFT - 10)))  0)) {
+hw_error(xc_domain_setmaxmem failed);
+}
 if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, 
pfn_list)) {
 hw_error(xen: failed to populate ram at  RAM_ADDR_FMT, ram_addr);
 }
-- 
1.8.4




[Qemu-devel] [PATCH v4 01/26] qcow2: Add two new fields to BDRVQcowState

2014-12-03 Thread Max Reitz
Add two new fields regarding refcount information (the bit width of
every entry and the maximum refcount value) to the BDRVQcowState.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 2 +-
 block/qcow2.c  | 3 +++
 block/qcow2.h  | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9afdb40..6016211 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -584,7 +584,7 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 
 refcount = be16_to_cpu(refcount_block[block_index]);
 refcount += addend;
-if (refcount  0 || refcount  0x) {
+if (refcount  0 || refcount  s-refcount_max) {
 ret = -EINVAL;
 goto fail;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 8b9ffc4..5ed982b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -684,6 +684,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto fail;
 }
 s-refcount_order = header.refcount_order;
+s-refcount_bits = 1  s-refcount_order;
+s-refcount_max = UINT64_C(1)  (s-refcount_bits - 1);
+s-refcount_max += s-refcount_max - 1;
 
 if (header.crypt_method  QCOW_CRYPT_AES) {
 error_setg(errp, Unsupported encryption method: % PRIu32,
diff --git a/block/qcow2.h b/block/qcow2.h
index 6e39a1b..4d8c902 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -258,6 +258,8 @@ typedef struct BDRVQcowState {
 int qcow_version;
 bool use_lazy_refcounts;
 int refcount_order;
+int refcount_bits;
+uint64_t refcount_max;
 
 bool discard_passthrough[QCOW2_DISCARD_MAX];
 
-- 
1.9.3




[Qemu-devel] [PATCH v4 02/26] qcow2: Add refcount_bits to format-specific info

2014-12-03 Thread Max Reitz
Add the bit width of every refcount entry to the format-specific
information.

In contrast to lazy_refcounts and the corrupt flag, this should be
always emitted, even for compat=0.10 although it does not support any
refcount width other than 16 bits. This is because if a boolean is
optional, one normally assumes it to be false when omitted; but if an
integer is not specified, it is rather difficult to guess its value.

This new field breaks some test outputs, fix them.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2.c  |  4 +++-
 qapi/block-core.json   |  5 -
 tests/qemu-iotests/060.out |  1 +
 tests/qemu-iotests/065 | 23 +++
 tests/qemu-iotests/067.out |  5 +
 tests/qemu-iotests/082.out |  7 +++
 tests/qemu-iotests/089.out |  2 ++
 7 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 5ed982b..25d6a66 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2469,7 +2469,8 @@ static ImageInfoSpecific 
*qcow2_get_specific_info(BlockDriverState *bs)
 };
 if (s-qcow_version == 2) {
 *spec_info-qcow2 = (ImageInfoSpecificQCow2){
-.compat = g_strdup(0.10),
+.compat = g_strdup(0.10),
+.refcount_bits  = s-refcount_bits,
 };
 } else if (s-qcow_version == 3) {
 *spec_info-qcow2 = (ImageInfoSpecificQCow2){
@@ -2480,6 +2481,7 @@ static ImageInfoSpecific 
*qcow2_get_specific_info(BlockDriverState *bs)
 .corrupt= s-incompatible_features 
   QCOW2_INCOMPAT_CORRUPT,
 .has_corrupt= true,
+.refcount_bits  = s-refcount_bits,
 };
 }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6e8db15..008215e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -41,13 +41,16 @@
 # @corrupt: #optional true if the image has been marked corrupt; only valid for
 #   compat = 1.1 (since 2.2)
 #
+# @refcount-bits: width of a refcount entry in bits (since 2.3)
+#
 # Since: 1.7
 ##
 { 'type': 'ImageInfoSpecificQCow2',
   'data': {
   'compat': 'str',
   '*lazy-refcounts': 'bool',
-  '*corrupt': 'bool'
+  '*corrupt': 'bool',
+  'refcount-bits': 'int'
   } }
 
 ##
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 4cdf62b..a289eea 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -18,6 +18,7 @@ cluster_size: 65536
 Format specific information:
 compat: 1.1
 lazy refcounts: false
+refcount bits: 16
 corrupt: true
 qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot 
be opened read/write
 read 512/512 bytes at offset 0
diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 8d3a9c9..72aa970 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -88,34 +88,41 @@ class TestQMP(TestImageInfoSpecific):
 class TestQCow2(TestQemuImgInfo):
 '''Testing a qcow2 version 2 image'''
 img_options = 'compat=0.10'
-json_compare = { 'compat': '0.10' }
-human_compare = [ 'compat: 0.10' ]
+json_compare = { 'compat': '0.10', 'refcount-bits': 16 }
+human_compare = [ 'compat: 0.10', 'refcount bits: 16' ]
 
 class TestQCow3NotLazy(TestQemuImgInfo):
 '''Testing a qcow2 version 3 image with lazy refcounts disabled'''
 img_options = 'compat=1.1,lazy_refcounts=off'
-json_compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': 
False }
-human_compare = [ 'compat: 1.1', 'lazy refcounts: false', 'corrupt: false' 
]
+json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
+ 'refcount-bits': 16, 'corrupt': False }
+human_compare = [ 'compat: 1.1', 'lazy refcounts: false',
+  'refcount bits: 16', 'corrupt: false' ]
 
 class TestQCow3Lazy(TestQemuImgInfo):
 '''Testing a qcow2 version 3 image with lazy refcounts enabled'''
 img_options = 'compat=1.1,lazy_refcounts=on'
-json_compare = { 'compat': '1.1', 'lazy-refcounts': True, 'corrupt': False 
}
-human_compare = [ 'compat: 1.1', 'lazy refcounts: true', 'corrupt: false' ]
+json_compare = { 'compat': '1.1', 'lazy-refcounts': True,
+ 'refcount-bits': 16, 'corrupt': False }
+human_compare = [ 'compat: 1.1', 'lazy refcounts: true',
+  'refcount bits: 16', 'corrupt: false' ]
 
 class TestQCow3NotLazyQMP(TestQMP):
 '''Testing a qcow2 version 3 image with lazy refcounts disabled, opening
with lazy refcounts enabled'''
 img_options = 'compat=1.1,lazy_refcounts=off'
 qemu_options = 'lazy-refcounts=on'
-compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': False }
+compare = { 'compat': '1.1', 'lazy-refcounts': False,
+'refcount-bits': 16, 'corrupt': False }
+
 
 class TestQCow3LazyQMP(TestQMP):
 '''Testing a qcow2 version 3 image with lazy refcounts enabled, 

[Qemu-devel] [PATCH v4 05/26] qcow2: Use unsigned addend for update_refcount()

2014-12-03 Thread Max Reitz
update_refcount() and qcow2_update_cluster_refcount() currently take a
signed addend. At least one caller passes a value directly derived from
an absolute refcount that should be reached (l2_refcount - 1 in
expand_zero_clusters_in_l1()). Therefore, the addend should be unsigned
because unsigned overflow is well-defined in contrast to signed
overflow. This will be especially important for 64 bit refcounts.

Because update_refcount() then no longer knows whether the refcount
should be increased or decreased (which is important for setting the
refblock-L2-table cache dependency and for overflow/underflow checks),
it now requires an additional flag which specified exactly that. The
same applies to qcow2_update_cluster_refcount().

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c | 63 --
 block/qcow2.h  |  3 ++-
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a8baecb..da37535 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1699,7 +1699,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 /* For shared L2 tables, set the refcount accordingly (it 
is
  * already 1 and needs to be l2_refcount) */
 ret = qcow2_update_cluster_refcount(bs,
-offset  s-cluster_bits, l2_refcount - 1,
+offset  s-cluster_bits, l2_refcount - 1, false,
 QCOW2_DISCARD_OTHER);
 if (ret  0) {
 qcow2_free_clusters(bs, offset, s-cluster_size,
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index bd37b8d..b3aed9c 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,8 +29,8 @@
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-int64_t offset, int64_t length,
-int addend, enum qcow2_discard_type type);
+int64_t offset, int64_t length, uint16_t addend,
+bool decrease, enum qcow2_discard_type type);
 
 
 /*/
@@ -263,7 +263,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 } else {
 /* Described somewhere else. This can recurse at most twice before we
  * arrive at a block that describes itself. */
-ret = update_refcount(bs, new_block, s-cluster_size, 1,
+ret = update_refcount(bs, new_block, s-cluster_size, 1, false,
   QCOW2_DISCARD_NEVER);
 if (ret  0) {
 goto fail_block;
@@ -530,8 +530,16 @@ found:
 }
 
 /* XXX: cache several refcount block clusters ? */
+/* In order to decrease refcounts, set @addend to the two's complement (giving 
a
+ * negative value and letting the implicit cast handle it is enough) and set
+ * @decrease to true. @decrease must be false if the refcount should be
+ * increased. */
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-int64_t offset, int64_t length, int addend, enum qcow2_discard_type type)
+   int64_t offset,
+   int64_t length,
+   uint16_t addend,
+   bool decrease,
+   enum qcow2_discard_type 
type)
 {
 BDRVQcowState *s = bs-opaque;
 int64_t start, last, cluster_offset;
@@ -540,8 +548,9 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 int ret;
 
 #ifdef DEBUG_ALLOC2
-fprintf(stderr, update_refcount: offset=% PRId64  size=% PRId64  
addend=%d\n,
-   offset, length, addend);
+fprintf(stderr, update_refcount: offset=% PRId64  size=% PRId64
+ addend=% PRId16  decrease=%d\n, offset, length,
+(int16_t)addend, decrease);
 #endif
 if (length  0) {
 return -EINVAL;
@@ -549,7 +558,7 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 return 0;
 }
 
-if (addend  0) {
+if (decrease) {
 qcow2_cache_set_dependency(bs, s-refcount_block_cache,
 s-l2_table_cache);
 }
@@ -559,7 +568,8 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 for(cluster_offset = start; cluster_offset = last;
 cluster_offset += s-cluster_size)
 {
-int block_index, refcount;
+int block_index;
+uint16_t refcount;
 int64_t cluster_index = cluster_offset  s-cluster_bits;
 int64_t table_index = cluster_index  s-refcount_block_bits;
 
@@ -586,11 +596,14 

[Qemu-devel] [PATCH v4 00/26] qcow2: Support refcount orders != 4

2014-12-03 Thread Max Reitz
As of version 3, the qcow2 file format supports different widths for
refcount entries, ranging from 1 to 64 bit (only powers of two).
Currently, qemu only supports 16 bit, which is the only width supported
by version 2 (compat=0.10) images.

This series adds support to qemu for all other valid refcount orders.
This is mainly done by adding two function pointers into the
BDRVQcowState structure for reading and writing refcount values
independently of the current refcount entry width; all in-memory
refcount arrays (mostly cached refcount blocks) now are void pointers
and are accessed through these functions alone.

Thanks to previous work of making the qemu code agnostic of e.g. the
number of refcount entries per refcount block, the rest is fairly
trivial. The most complex patch in this series is patch 18 which
implements changing the refcount order through qemu-img amend.

To test different refcount widths, simply invoke the qemu-iotests check
program with -o refcount_width=${your_desired_width}. The final test in
this series adds some tests for operations which do not work with
certain refcount orders and for refcount order amendment.


v4:
- Patch 1: No longer limit to INT64_MAX but allow full range up to
  UINT64_MAX; also, use a non-overflowing shift to avoid the special
  case for s-refcount_order == 6 [Stefan]
- Patch 2: %s/refcount_width/refcount_bits/g (etc.) [Stefan]
- Patch 3: Added
- Patch 4: Added [Stefan]
- Patch 5: Added
- Patch 6 (prev. 3):
  - Rebase conflicts due to patches 3, 4 and 5
  - Always use uint64_t for refcounts instead of int64_t [Stefan]
- Patch 7 (prev. 4):
  - Rebase conflicts due to patch 5
  - qcow2_update_cluster_refcount() only returns an int (not an
int64_t), thus int ret will suffice (due to patch 3)
- Patch 8 (prev. 5):
  - Refcounts are always uint64_t [Stefan]
  - Rebase conflict due to patch 4
- Patch 9 (prev. 6): Allow shrinking of the array in
  realloc_refcount_array() [Stefan]
- Patch 10 (prev. 7):
  - Rebase conflicts due to patch 4
  - Range checks are no longer necessary [Stefan]
  - Refcounts are always uint64_t [Stefan]
- Patch 11 (prev 8): It is now allowed to set the MSb in 64 bit
  refcounts, therefore drop the assert() in set_refcount_ro6() [Stefan]
- Patch 13 (prev. 10): %s/refcount_width/refcount_bits/g [Stefan]
- Patch 14: Added
- Patch 15 (prev. 11): %s/refcount_width/refcount_bits/g [Stefan]
- Patch 16 (prev. 12):
  - s/BLOCK_OPT_REFCOUNT_WIDTH/BLOCK_OPT_REFCOUNT_BITS/g [Stefan]
  - s/refcount_width/BLOCK_OPT_REFCOUNT_BITS/
  - %s/refcount_width/refcount_bits/g [Stefan]
- Patch 17 (prev. 13): As a very good example on why I am opposed to
  leaving the opening brace of an if block on the last line of
  multi-line conditions, I followed Eric's proposal and removed the
  superfluous parentheses in the condition touched by this patch [Eric]
- Patch 23 (prev. 19): Same change as in patch 1; and fixed an overly
  long line
- Patch 24 (prev. 20):
  - s/refcount_width/BLOCK_OPT_REFCOUNT_BITS/
  - %s/refcount_width/refcount_bits/g [Stefan]
- Patch 26 (prev. 22):
  - %s/refcount_width/refcount_bits/g [Stefan]
  - Setting the MSb in a 64 bit refcount value should work now [Stefan]
  - Added a test for snapshotting a cluster with a refcount of 2^64 - 1


git-backport-diff against v3:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/26:[0010] [FC] 'qcow2: Add two new fields to BDRVQcowState'
002/26:[down] 'qcow2: Add refcount_bits to format-specific info'
 wrong, should be:
   [0054] [FC]
003/26:[down] 'qcow2: Do not return new value after refcount update'
004/26:[down] 'qcow2: Only return status from qcow2_get_refcount'
005/26:[down] 'qcow2: Use unsigned addend for update_refcount()'
006/26:[0093] [FC] 'qcow2: Use 64 bits for refcount values'
007/26:[0011] [FC] 'qcow2: Respect error in qcow2_alloc_bytes()'
008/26:[0015] [FC] 'qcow2: Refcount overflow and qcow2_alloc_bytes()'
009/26:[0008] [FC] 'qcow2: Helper for refcount array reallocation'
010/26:[0017] [FC] 'qcow2: Helper function for refcount modification'
011/26:[0003] [FC] 'qcow2: More helpers for refcount modification'
012/26:[] [--] 'qcow2: Open images with refcount order != 4'
013/26:[0006] [FC] 'qcow2: refcount_order parameter for qcow2_create2'
014/26:[down] 'qcow2: Use symbolic macros in qcow2_amend_options'
015/26:[down] 'iotests: Prepare for refcount_bits option'
 wrong, should be:
   [0038] [FC]
016/26:[0228] [FC] 'qcow2: Allow creation with refcount order != 4'
017/26:[0002] [FC] 'progress: Allow regressing progress'
018/26:[] [--] 'block: Add opaque value to the amend CB'
019/26:[] [-C] 'qcow2: Use error_report() in qcow2_amend_options()'
020/26:[] [--] 'qcow2: Use abort() instead of assert(false)'
021/26:[] [--] 'qcow2: Split upgrade/downgrade paths for 

[Qemu-devel] [PATCH v4 04/26] qcow2: Only return status from qcow2_get_refcount

2014-12-03 Thread Max Reitz
Refcounts can theoretically be of type uint64_t; in order to be able to
represent the full range, qcow2_get_refcount() cannot use a single
variable to represent both all refcount values and also keep some values
reserved for errors.

One solution would be to add an Error pointer parameter to
qcow2_get_refcount(); however, no caller could (currently) pass that
error message, so it would have to be emitted immediately and be
passed to the next caller by returning -EIO or something similar.
Therefore, an Error parameter does not offer any advantages here.

The solution applied by this patch is simpler to use. Because no caller
would be able to pass the error message, they would have to print it and
free it, whereas with this patch the caller only needs to pass the
returned integer (which is often a no-op from the code perspective,
because that integer will be stored in a variable ret which will be
returned by the fail path of many callers).

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-cluster.c  |  8 ++---
 block/qcow2-refcount.c | 79 +++---
 block/qcow2.h  |  3 +-
 3 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index df0b2c9..a8baecb 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1640,7 +1640,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 for (i = 0; i  l1_size; i++) {
 uint64_t l2_offset = l1_table[i]  L1E_OFFSET_MASK;
 bool l2_dirty = false;
-int l2_refcount;
+uint16_t l2_refcount;
 
 if (!l2_offset) {
 /* unallocated */
@@ -1664,9 +1664,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 goto fail;
 }
 
-l2_refcount = qcow2_get_refcount(bs, l2_offset  s-cluster_bits);
-if (l2_refcount  0) {
-ret = l2_refcount;
+ret = qcow2_get_refcount(bs, l2_offset  s-cluster_bits,
+ l2_refcount);
+if (ret  0) {
 goto fail;
 }
 
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7556384..bd37b8d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -87,26 +87,29 @@ static int load_refcount_block(BlockDriverState *bs,
 }
 
 /*
- * Returns the refcount of the cluster given by its index. Any non-negative
- * return value is the refcount of the cluster, negative values are -errno
- * and indicate an error.
+ * Retrieves the refcount of the cluster given by its index and stores it in
+ * *refcount. Returns 0 on success and -errno on failure.
  */
-int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
+int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
+   uint16_t *refcount)
 {
 BDRVQcowState *s = bs-opaque;
 uint64_t refcount_table_index, block_index;
 int64_t refcount_block_offset;
 int ret;
 uint16_t *refcount_block;
-uint16_t refcount;
 
 refcount_table_index = cluster_index  s-refcount_block_bits;
-if (refcount_table_index = s-refcount_table_size)
+if (refcount_table_index = s-refcount_table_size) {
+*refcount = 0;
 return 0;
+}
 refcount_block_offset =
 s-refcount_table[refcount_table_index]  REFT_OFFSET_MASK;
-if (!refcount_block_offset)
+if (!refcount_block_offset) {
+*refcount = 0;
 return 0;
+}
 
 if (offset_into_cluster(s, refcount_block_offset)) {
 qcow2_signal_corruption(bs, true, -1, -1, Refblock offset %# PRIx64
@@ -122,7 +125,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t 
cluster_index)
 }
 
 block_index = cluster_index  (s-refcount_block_size - 1);
-refcount = be16_to_cpu(refcount_block[block_index]);
+*refcount = be16_to_cpu(refcount_block[block_index]);
 
 ret = qcow2_cache_put(bs, s-refcount_block_cache,
 (void**) refcount_block);
@@ -130,7 +133,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t 
cluster_index)
 return ret;
 }
 
-return refcount;
+return 0;
 }
 
 /*
@@ -662,16 +665,17 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs, 
uint64_t size)
 {
 BDRVQcowState *s = bs-opaque;
 uint64_t i, nb_clusters;
-int refcount;
+uint16_t refcount;
+int ret;
 
 nb_clusters = size_to_clusters(s, size);
 retry:
 for(i = 0; i  nb_clusters; i++) {
 uint64_t next_cluster_index = s-free_cluster_index++;
-refcount = qcow2_get_refcount(bs, next_cluster_index);
+ret = qcow2_get_refcount(bs, next_cluster_index, refcount);
 
-if (refcount  0) {
-return refcount;
+if (ret  0) {
+return ret;
 } else if (refcount != 0) {
 goto retry;
 }
@@ -721,7 +725,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t 
offset,
 BDRVQcowState *s = 

[Qemu-devel] [PATCH v4 03/26] qcow2: Do not return new value after refcount update

2014-12-03 Thread Max Reitz
qcow2_update_cluster_refcount() does not have any quick access to the
new refcount value, it has to call qcow2_get_refcount(). Some callers do
not need that new value at all, others call qcow2_get_refcount()
themselves anyway (albeit in a different code path, which can however be
easily changed), therefore there is no advantage in making
qcow2_update_cluster_refcount() return the new value. Drop it.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 6016211..7556384 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -631,8 +631,7 @@ fail:
 /*
  * Increases or decreases the refcount of a given cluster.
  *
- * If the return value is non-negative, it is the new refcount of the cluster.
- * If it is negative, it is -errno and indicates an error.
+ * On success 0 is returned; on failure -errno is returned.
  */
 int qcow2_update_cluster_refcount(BlockDriverState *bs,
   int64_t cluster_index,
@@ -648,7 +647,7 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,
 return ret;
 }
 
-return qcow2_get_refcount(bs, cluster_index);
+return 0;
 }
 
 
@@ -976,13 +975,15 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 break;
 }
 if (addend != 0) {
-refcount = qcow2_update_cluster_refcount(bs,
+ret = qcow2_update_cluster_refcount(bs,
 cluster_index, addend,
 QCOW2_DISCARD_SNAPSHOT);
-} else {
-refcount = qcow2_get_refcount(bs, cluster_index);
+if (ret  0) {
+goto fail;
+}
 }
 
+refcount = qcow2_get_refcount(bs, cluster_index);
 if (refcount  0) {
 ret = refcount;
 goto fail;
@@ -1017,11 +1018,15 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 
 
 if (addend != 0) {
-refcount = qcow2_update_cluster_refcount(bs, l2_offset 
-s-cluster_bits, addend, QCOW2_DISCARD_SNAPSHOT);
-} else {
-refcount = qcow2_get_refcount(bs, l2_offset  
s-cluster_bits);
+ret = qcow2_update_cluster_refcount(bs, l2_offset 
+s-cluster_bits,
+addend,
+QCOW2_DISCARD_SNAPSHOT);
+if (ret  0) {
+goto fail;
+}
 }
+refcount = qcow2_get_refcount(bs, l2_offset  s-cluster_bits);
 if (refcount  0) {
 ret = refcount;
 goto fail;
-- 
1.9.3




[Qemu-devel] [PATCH v4 06/26] qcow2: Use 64 bits for refcount values

2014-12-03 Thread Max Reitz
Refcounts may have a width of up to 64 bits, so qemu should use the same
width to represent refcount values internally.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c | 42 --
 block/qcow2.h  |  4 ++--
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index da37535..5a678f3 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1640,7 +1640,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 for (i = 0; i  l1_size; i++) {
 uint64_t l2_offset = l1_table[i]  L1E_OFFSET_MASK;
 bool l2_dirty = false;
-uint16_t l2_refcount;
+uint64_t l2_refcount;
 
 if (!l2_offset) {
 /* unallocated */
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index b3aed9c..095ff9b 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,7 +29,7 @@
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-int64_t offset, int64_t length, uint16_t addend,
+int64_t offset, int64_t length, uint64_t addend,
 bool decrease, enum qcow2_discard_type type);
 
 
@@ -91,7 +91,7 @@ static int load_refcount_block(BlockDriverState *bs,
  * *refcount. Returns 0 on success and -errno on failure.
  */
 int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
-   uint16_t *refcount)
+   uint64_t *refcount)
 {
 BDRVQcowState *s = bs-opaque;
 uint64_t refcount_table_index, block_index;
@@ -537,7 +537,7 @@ found:
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset,
int64_t length,
-   uint16_t addend,
+   uint64_t addend,
bool decrease,
enum qcow2_discard_type 
type)
 {
@@ -549,8 +549,8 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 
 #ifdef DEBUG_ALLOC2
 fprintf(stderr, update_refcount: offset=% PRId64  size=% PRId64
- addend=% PRId16  decrease=%d\n, offset, length,
-(int16_t)addend, decrease);
+ addend=% PRId64  decrease=%d\n, offset, length,
+(int64_t)addend, decrease);
 #endif
 if (length  0) {
 return -EINVAL;
@@ -569,7 +569,7 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 cluster_offset += s-cluster_size)
 {
 int block_index;
-uint16_t refcount;
+uint64_t refcount;
 int64_t cluster_index = cluster_offset  s-cluster_bits;
 int64_t table_index = cluster_index  s-refcount_block_bits;
 
@@ -596,9 +596,9 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 block_index = cluster_index  (s-refcount_block_size - 1);
 
 refcount = be16_to_cpu(refcount_block[block_index]);
-if ((uint16_t)(refcount + addend)  s-refcount_max ||
-(!decrease  (uint16_t)(refcount + addend)  refcount) ||
-( decrease  (uint16_t)(refcount + addend)  refcount))
+if (refcount + addend  s-refcount_max ||
+(!decrease  refcount + addend  refcount) ||
+( decrease  refcount + addend  refcount))
 {
 ret = -EINVAL;
 goto fail;
@@ -656,7 +656,7 @@ fail:
  */
 int qcow2_update_cluster_refcount(BlockDriverState *bs,
   int64_t cluster_index,
-  uint16_t addend, bool decrease,
+  uint64_t addend, bool decrease,
   enum qcow2_discard_type type)
 {
 BDRVQcowState *s = bs-opaque;
@@ -682,8 +682,7 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,
 static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
 {
 BDRVQcowState *s = bs-opaque;
-uint64_t i, nb_clusters;
-uint16_t refcount;
+uint64_t i, nb_clusters, refcount;
 int ret;
 
 nb_clusters = size_to_clusters(s, size);
@@ -741,9 +740,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t 
offset,
 int nb_clusters)
 {
 BDRVQcowState *s = bs-opaque;
-uint64_t cluster_index;
+uint64_t cluster_index, refcount;
 uint64_t i;
-uint16_t refcount;
 int ret;
 
 assert(nb_clusters = 0);
@@ -897,11 +895,10 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int64_t l1_table_offset, int l1_size, int addend)
 {
 BDRVQcowState *s = bs-opaque;
-uint64_t 

Re: [Qemu-devel] qemu rdma live migration

2014-12-03 Thread Dr. David Alan Gilbert
* Vasiliy Tolstov (v.tols...@selfip.ru) wrote:
 Hello. I don't find info about how qemu doing live migration via RDAM
 - my specific qeustion - does it need ethernet connection for some
 info, or can use native RDMA and resolve nodes via GUIDs ?

The entire migration stream is passed over the RDMA; however
I'm not clear how the address lookup works, what you pass to the
migration command is the IP associated with the interface.

Dave

 
 -- 
 Vasiliy Tolstov,
 e-mail: v.tols...@selfip.ru
 jabber: v...@selfip.ru
 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PATCH v4 08/26] qcow2: Refcount overflow and qcow2_alloc_bytes()

2014-12-03 Thread Max Reitz
qcow2_alloc_bytes() may reuse a cluster multiple times, in which case
the refcount is increased accordingly. However, if this would lead to an
overflow the function should instead just not reuse this cluster and
allocate a new one.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 6166f7d..152ca22 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -780,9 +780,11 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 BDRVQcowState *s = bs-opaque;
 int64_t offset, cluster_offset, new_cluster;
 int free_in_cluster, ret;
+uint64_t refcount;
 
 BLKDBG_EVENT(bs-file, BLKDBG_CLUSTER_ALLOC_BYTES);
 assert(size  0  size = s-cluster_size);
+ redo:
 if (s-free_byte_offset == 0) {
 offset = qcow2_alloc_clusters(bs, s-cluster_size);
 if (offset  0) {
@@ -790,12 +792,25 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 }
 s-free_byte_offset = offset;
 }
- redo:
+
 free_in_cluster = s-cluster_size -
 offset_into_cluster(s, s-free_byte_offset);
 if (size = free_in_cluster) {
 /* enough space in current cluster */
 offset = s-free_byte_offset;
+
+if (offset_into_cluster(s, offset) != 0) {
+/* We will have to increase the refcount of this cluster; if the
+ * maximum has been reached already, this cluster cannot be used */
+ret = qcow2_get_refcount(bs, offset  s-cluster_bits, refcount);
+if (ret  0) {
+return ret;
+} else if (refcount == s-refcount_max) {
+s-free_byte_offset = 0;
+goto redo;
+}
+}
+
 s-free_byte_offset += size;
 free_in_cluster -= size;
 if (free_in_cluster == 0)
@@ -816,6 +831,20 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 if ((cluster_offset + s-cluster_size) == new_cluster) {
 /* we are lucky: contiguous data */
 offset = s-free_byte_offset;
+
+/* Same as above: In order to reuse the cluster, the refcount has 
to
+ * be increased; if that will not work, we are not so lucky after
+ * all */
+ret = qcow2_get_refcount(bs, offset  s-cluster_bits, refcount);
+if (ret  0) {
+qcow2_free_clusters(bs, new_cluster, s-cluster_size,
+QCOW2_DISCARD_NEVER);
+return ret;
+} else if (refcount == s-refcount_max) {
+s-free_byte_offset = offset;
+goto redo;
+}
+
 ret = qcow2_update_cluster_refcount(bs, offset  s-cluster_bits,
 1, false, QCOW2_DISCARD_NEVER);
 if (ret  0) {
-- 
1.9.3




[Qemu-devel] [PATCH v4 07/26] qcow2: Respect error in qcow2_alloc_bytes()

2014-12-03 Thread Max Reitz
qcow2_update_cluster_refcount() may fail, and qcow2_alloc_bytes() should
mind that case.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 095ff9b..6166f7d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -778,8 +778,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t 
offset,
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 {
 BDRVQcowState *s = bs-opaque;
-int64_t offset, cluster_offset;
-int free_in_cluster;
+int64_t offset, cluster_offset, new_cluster;
+int free_in_cluster, ret;
 
 BLKDBG_EVENT(bs-file, BLKDBG_CLUSTER_ALLOC_BYTES);
 assert(size  0  size = s-cluster_size);
@@ -800,23 +800,32 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 free_in_cluster -= size;
 if (free_in_cluster == 0)
 s-free_byte_offset = 0;
-if (offset_into_cluster(s, offset) != 0)
-qcow2_update_cluster_refcount(bs, offset  s-cluster_bits, 1,
-  false, QCOW2_DISCARD_NEVER);
+if (offset_into_cluster(s, offset) != 0) {
+ret = qcow2_update_cluster_refcount(bs, offset  s-cluster_bits,
+1, false, QCOW2_DISCARD_NEVER);
+if (ret  0) {
+return ret;
+}
+}
 } else {
-offset = qcow2_alloc_clusters(bs, s-cluster_size);
-if (offset  0) {
-return offset;
+new_cluster = qcow2_alloc_clusters(bs, s-cluster_size);
+if (new_cluster  0) {
+return new_cluster;
 }
 cluster_offset = start_of_cluster(s, s-free_byte_offset);
-if ((cluster_offset + s-cluster_size) == offset) {
+if ((cluster_offset + s-cluster_size) == new_cluster) {
 /* we are lucky: contiguous data */
 offset = s-free_byte_offset;
-qcow2_update_cluster_refcount(bs, offset  s-cluster_bits, 1,
-  false, QCOW2_DISCARD_NEVER);
+ret = qcow2_update_cluster_refcount(bs, offset  s-cluster_bits,
+1, false, QCOW2_DISCARD_NEVER);
+if (ret  0) {
+qcow2_free_clusters(bs, new_cluster, s-cluster_size,
+QCOW2_DISCARD_NEVER);
+return ret;
+}
 s-free_byte_offset += size;
 } else {
-s-free_byte_offset = offset;
+s-free_byte_offset = new_cluster;
 goto redo;
 }
 }
-- 
1.9.3




[Qemu-devel] [PATCH v4 10/26] qcow2: Helper function for refcount modification

2014-12-03 Thread Max Reitz
Since refcounts do not always have to be a uint16_t, all refcount blocks
and arrays in memory should not have a specific type (thus they become
pointers to void) and for accessing them, two helper functions are used
(a getter and a setter). Those functions are called indirectly through
function pointers in the BDRVQcowState so they may later be exchanged
for different refcount orders.

With the check and repair functions using this function, the refcount
array they are creating will be in big endian byte order; additionally,
using realloc_refcount_array() makes the size of this refcount array
always cluster-aligned. Both combined allow rebuild_refcount_structure()
to drop the bounce buffer which was used to convert parts of the
refcount array to big endian byte order and store them on disk. Instead,
those parts can now be written directly.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 122 -
 block/qcow2.h  |   8 
 2 files changed, 79 insertions(+), 51 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 8bb5167..125ca12 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -32,6 +32,11 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 int64_t offset, int64_t length, uint64_t addend,
 bool decrease, enum qcow2_discard_type type);
 
+static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index);
+
+static void set_refcount_ro4(void *refcount_array, uint64_t index,
+ uint64_t value);
+
 
 /*/
 /* refcount handling */
@@ -42,6 +47,9 @@ int qcow2_refcount_init(BlockDriverState *bs)
 unsigned int refcount_table_size2, i;
 int ret;
 
+s-get_refcount = get_refcount_ro4;
+s-set_refcount = set_refcount_ro4;
+
 assert(s-refcount_table_size = INT_MAX / sizeof(uint64_t));
 refcount_table_size2 = s-refcount_table_size * sizeof(uint64_t);
 s-refcount_table = g_try_malloc(refcount_table_size2);
@@ -72,6 +80,19 @@ void qcow2_refcount_close(BlockDriverState *bs)
 }
 
 
+static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index)
+{
+return be16_to_cpu(((const uint16_t *)refcount_array)[index]);
+}
+
+static void set_refcount_ro4(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+assert(!(value  16));
+((uint16_t *)refcount_array)[index] = cpu_to_be16(value);
+}
+
+
 static int load_refcount_block(BlockDriverState *bs,
int64_t refcount_block_offset,
void **refcount_block)
@@ -97,7 +118,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t 
cluster_index,
 uint64_t refcount_table_index, block_index;
 int64_t refcount_block_offset;
 int ret;
-uint16_t *refcount_block;
+void *refcount_block;
 
 refcount_table_index = cluster_index  s-refcount_block_bits;
 if (refcount_table_index = s-refcount_table_size) {
@@ -119,16 +140,15 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t 
cluster_index,
 }
 
 ret = qcow2_cache_get(bs, s-refcount_block_cache, refcount_block_offset,
-(void**) refcount_block);
+  refcount_block);
 if (ret  0) {
 return ret;
 }
 
 block_index = cluster_index  (s-refcount_block_size - 1);
-*refcount = be16_to_cpu(refcount_block[block_index]);
+*refcount = s-get_refcount(refcount_block, block_index);
 
-ret = qcow2_cache_put(bs, s-refcount_block_cache,
-(void**) refcount_block);
+ret = qcow2_cache_put(bs, s-refcount_block_cache, refcount_block);
 if (ret  0) {
 return ret;
 }
@@ -172,7 +192,7 @@ static int in_same_refcount_block(BDRVQcowState *s, 
uint64_t offset_a,
  * Returns 0 on success or -errno in error case
  */
 static int alloc_refcount_block(BlockDriverState *bs,
-int64_t cluster_index, uint16_t **refcount_block)
+int64_t cluster_index, void **refcount_block)
 {
 BDRVQcowState *s = bs-opaque;
 unsigned int refcount_table_index;
@@ -199,7 +219,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 }
 
  return load_refcount_block(bs, refcount_block_offset,
- (void**) refcount_block);
+refcount_block);
 }
 }
 
@@ -259,7 +279,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 /* The block describes itself, need to update the cache */
 int block_index = (new_block  s-cluster_bits) 
 (s-refcount_block_size - 1);
-(*refcount_block)[block_index] = cpu_to_be16(1);
+s-set_refcount(*refcount_block, block_index, 1);
 } else {
 /* Described somewhere else. This can recurse at most twice before we
  * arrive 

[Qemu-devel] [PATCH v4 17/26] progress: Allow regressing progress

2014-12-03 Thread Max Reitz
Progress may regress; this should be displayed correctly by
qemu_progress_print().

While touching that area of code, drop the redundant parentheses in the
same condition.

Signed-off-by: Max Reitz mre...@redhat.com
---
 util/qemu-progress.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/qemu-progress.c b/util/qemu-progress.c
index 4ee5cd0..532333e 100644
--- a/util/qemu-progress.c
+++ b/util/qemu-progress.c
@@ -152,7 +152,8 @@ void qemu_progress_print(float delta, int max)
 state.current = current;
 
 if (current  (state.last_print + state.min_skip) ||
-(current == 100) || (current == 0)) {
+current  (state.last_print - state.min_skip) ||
+current == 100 || current == 0) {
 state.last_print = state.current;
 state.print();
 }
-- 
1.9.3




[Qemu-devel] [PATCH v4 09/26] qcow2: Helper for refcount array reallocation

2014-12-03 Thread Max Reitz
Add a helper function for reallocating a refcount array, independent of
the refcount order. The newly allocated space is zeroed and the function
handles failed reallocations gracefully.

The helper function will always align the buffer size to a cluster
boundary; if storing the refcounts in such an array in big endian byte
order, this makes it possible to write parts of the array directly as
refcount blocks into the image file.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 137 +++--
 1 file changed, 88 insertions(+), 49 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 152ca22..8bb5167 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1130,6 +1130,70 @@ fail:
 /* refcount checking functions */
 
 
+static size_t refcount_array_byte_size(BDRVQcowState *s, uint64_t entries)
+{
+if (s-refcount_order  3) {
+/* sub-byte width */
+int shift = 3 - s-refcount_order;
+return (entries + (1  shift) - 1)  shift;
+} else if (s-refcount_order == 3) {
+/* byte width */
+return entries;
+} else {
+/* multiple bytes wide */
+
+/* This assertion holds because there is no way we can address more 
than
+ * 2^(64 - 9) clusters at once (with cluster size 512 = 2^9, and 
because
+ * offsets have to be representable in bytes); due to every cluster
+ * corresponding to one refcount entry and because refcount_order has 
to
+ * be below 7, we are far below that limit */
+assert(!(entries  (64 - (s-refcount_order - 3;
+
+return entries  (s-refcount_order - 3);
+}
+}
+
+/**
+ * Reallocates *array so that it can hold new_size entries. *size must contain
+ * the current number of entries in *array. If the reallocation fails, *array
+ * and *size will not be modified and -errno will be returned. If the
+ * reallocation is successful, *array will be set to the new buffer and *size
+ * will be set to new_size. The size of the reallocated refcount array buffer
+ * will be aligned to a cluster boundary, and the newly allocated area will be
+ * zeroed.
+ */
+static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
+  int64_t *size, int64_t new_size)
+{
+/* Round to clusters so the array can be directly written to disk */
+size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
+s-cluster_size);
+size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
+s-cluster_size);
+uint16_t *new_ptr;
+
+if (new_byte_size == old_byte_size) {
+*size = new_size;
+return 0;
+}
+
+assert(new_byte_size  0);
+
+new_ptr = g_try_realloc(*array, new_byte_size);
+if (!new_ptr) {
+return -ENOMEM;
+}
+
+if (new_byte_size  old_byte_size) {
+memset((void *)((uintptr_t)new_ptr + old_byte_size), 0,
+   new_byte_size - old_byte_size);
+}
+
+*array = new_ptr;
+*size  = new_size;
+
+return 0;
+}
 
 /*
  * Increases the refcount for a range of clusters in a given refcount table.
@@ -1146,6 +1210,7 @@ static int inc_refcounts(BlockDriverState *bs,
 {
 BDRVQcowState *s = bs-opaque;
 uint64_t start, last, cluster_offset, k;
+int ret;
 
 if (size = 0) {
 return 0;
@@ -1157,23 +1222,12 @@ static int inc_refcounts(BlockDriverState *bs,
 cluster_offset += s-cluster_size) {
 k = cluster_offset  s-cluster_bits;
 if (k = *refcount_table_size) {
-int64_t old_refcount_table_size = *refcount_table_size;
-uint16_t *new_refcount_table;
-
-*refcount_table_size = k + 1;
-new_refcount_table = g_try_realloc(*refcount_table,
-   *refcount_table_size *
-   sizeof(**refcount_table));
-if (!new_refcount_table) {
-*refcount_table_size = old_refcount_table_size;
+ret = realloc_refcount_array(s, refcount_table,
+ refcount_table_size, k + 1);
+if (ret  0) {
 res-check_errors++;
-return -ENOMEM;
+return ret;
 }
-*refcount_table = new_refcount_table;
-
-memset(*refcount_table + old_refcount_table_size, 0,
-   (*refcount_table_size - old_refcount_table_size) *
-   sizeof(**refcount_table));
 }
 
 if (++(*refcount_table)[k] == 0) {
@@ -1542,8 +1596,7 @@ static int check_refblocks(BlockDriverState *bs, 
BdrvCheckResult *res,
 fix  BDRV_FIX_ERRORS ? Repairing : ERROR, i);
 
 if (fix  BDRV_FIX_ERRORS) {
-int64_t old_nb_clusters = *nb_clusters;
-uint16_t 

[Qemu-devel] [PATCH v4 13/26] qcow2: refcount_order parameter for qcow2_create2

2014-12-03 Thread Max Reitz
Add a refcount_order parameter to qcow2_create2(), use that value for
the image header and for calculating the size required for
preallocation.

For now, always pass 4.

This addition requires changes to the calculation of the file size for
the full and falloc preallocation modes. That in turn is a nice
opportunity to add a comment about that calculation not necessarily
being exact (and that being intentional).

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2.c | 46 +++---
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ff57566..766f79a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1769,7 +1769,7 @@ static int preallocate(BlockDriverState *bs)
 static int qcow2_create2(const char *filename, int64_t total_size,
  const char *backing_file, const char *backing_format,
  int flags, size_t cluster_size, PreallocMode prealloc,
- QemuOpts *opts, int version,
+ QemuOpts *opts, int version, int refcount_order,
  Error **errp)
 {
 /* Calculate cluster_bits */
@@ -1802,9 +1802,21 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
 int ret;
 
 if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
+/* Note: The following calculation does not need to be exact; if it is 
a
+ * bit off, either some bytes will be leaked (which is fine) or we
+ * will need to increase the file size by some bytes (which is fine,
+ * too, as long as the bulk is allocated here). Therefore, using
+ * floating point arithmetic is fine. */
 int64_t meta_size = 0;
 uint64_t nreftablee, nrefblocke, nl1e, nl2e;
 int64_t aligned_total_size = align_offset(total_size, cluster_size);
+int refblock_bits, refblock_size;
+/* refcount entry size in bytes */
+double rces = (1  refcount_order) / 8.;
+
+/* see qcow2_open() */
+refblock_bits = cluster_bits - (refcount_order - 3);
+refblock_size = 1  refblock_bits;
 
 /* header: 1 cluster */
 meta_size += cluster_size;
@@ -1829,20 +1841,20 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
  *   c = cluster size
  *   y1 = number of refcount blocks entries
  *   y2 = meta size including everything
+ *   rces = refcount entry size in bytes
  * then,
  *   y1 = (y2 + a)/c
- *   y2 = y1 * sizeof(u16) + y1 * sizeof(u16) * sizeof(u64) / c + m
+ *   y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
  * we can get y1:
- *   y1 = (a + m) / (c - sizeof(u16) - sizeof(u16) * sizeof(u64) / c)
+ *   y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
  */
-nrefblocke = (aligned_total_size + meta_size + cluster_size) /
-(cluster_size - sizeof(uint16_t) -
- 1.0 * sizeof(uint16_t) * sizeof(uint64_t) / cluster_size);
-nrefblocke = align_offset(nrefblocke, cluster_size / sizeof(uint16_t));
-meta_size += nrefblocke * sizeof(uint16_t);
+nrefblocke = (aligned_total_size + meta_size + cluster_size)
+   / (cluster_size - rces - rces * sizeof(uint64_t)
+ / cluster_size);
+meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
 
 /* total size of refcount tables */
-nreftablee = nrefblocke * sizeof(uint16_t) / cluster_size;
+nreftablee = nrefblocke / refblock_size;
 nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
 meta_size += nreftablee * sizeof(uint64_t);
 
@@ -1877,7 +1889,7 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
 .l1_size= cpu_to_be32(0),
 .refcount_table_offset  = cpu_to_be64(cluster_size),
 .refcount_table_clusters= cpu_to_be32(1),
-.refcount_order = cpu_to_be32(4),
+.refcount_order = cpu_to_be32(refcount_order),
 .header_length  = cpu_to_be32(sizeof(*header)),
 };
 
@@ -1997,6 +2009,7 @@ static int qcow2_create(const char *filename, QemuOpts 
*opts, Error **errp)
 size_t cluster_size = DEFAULT_CLUSTER_SIZE;
 PreallocMode prealloc;
 int version = 3;
+int refcount_bits = 16, refcount_order;
 Error *local_err = NULL;
 int ret;
 
@@ -2051,8 +2064,19 @@ static int qcow2_create(const char *filename, QemuOpts 
*opts, Error **errp)
 goto finish;
 }
 
+if (version  3  refcount_bits != 16) {
+error_setg(errp, Different refcount widths than 16 bits require 
+   compatibility level 1.1 or above (use compat=1.1 or 
+   greater));
+ret = -EINVAL;
+goto finish;
+}
+
+

[Qemu-devel] [PATCH v4 16/26] qcow2: Allow creation with refcount order != 4

2014-12-03 Thread Max Reitz
Add a creation option to qcow2 for setting the refcount order of images
to be created, and respect that option's value.

This breaks some test outputs, fix them.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2.c  |  20 
 include/block/block_int.h  |   1 +
 tests/qemu-iotests/049.out | 112 ++---
 tests/qemu-iotests/082.out |  41 ++---
 tests/qemu-iotests/085.out |  38 +++
 5 files changed, 130 insertions(+), 82 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 912680c..078c319 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2064,6 +2064,17 @@ static int qcow2_create(const char *filename, QemuOpts 
*opts, Error **errp)
 goto finish;
 }
 
+refcount_bits = qemu_opt_get_number_del(opts, BLOCK_OPT_REFCOUNT_BITS,
+refcount_bits);
+if (refcount_bits = 0 || refcount_bits  64 ||
+!is_power_of_2(refcount_bits))
+{
+error_setg(errp, Refcount width must be a power of two and may not 
+   exceed 64 bits);
+ret = -EINVAL;
+goto finish;
+}
+
 if (version  3  refcount_bits != 16) {
 error_setg(errp, Different refcount widths than 16 bits require 
compatibility level 1.1 or above (use compat=1.1 or 
@@ -2704,6 +2715,9 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 } else if (!strcmp(desc-name, BLOCK_OPT_LAZY_REFCOUNTS)) {
 lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
+} else if (!strcmp(desc-name, BLOCK_OPT_REFCOUNT_BITS)) {
+error_report(Cannot change refcount entry width);
+return -ENOTSUP;
 } else {
 /* if this assertion fails, this probably means a new option was
  * added without having it covered here */
@@ -2873,6 +2887,12 @@ static QemuOptsList qcow2_create_opts = {
 .help = Postpone refcount updates,
 .def_value_str = off
 },
+{
+.name = BLOCK_OPT_REFCOUNT_BITS,
+.type = QEMU_OPT_NUMBER,
+.help = Width of a reference count entry in bits,
+.def_value_str = 16
+},
 { /* end of list */ }
 }
 };
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 192fce8..cca4fe8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -56,6 +56,7 @@
 #define BLOCK_OPT_ADAPTER_TYPE  adapter_type
 #define BLOCK_OPT_REDUNDANCYredundancy
 #define BLOCK_OPT_NOCOW nocow
+#define BLOCK_OPT_REFCOUNT_BITS refcount_bits
 
 #define BLOCK_PROBE_BUF_SIZE512
 
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 09ca0ae..765afdd 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -4,90 +4,90 @@ QA output created by 049
 == 1. Traditional size parameter ==
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024b
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1k
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1K
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 1T
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off 
cluster_size=65536 lazy_refcounts=off 
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off 
cluster_size=65536 

[Qemu-devel] [PATCH v4 20/26] qcow2: Use abort() instead of assert(false)

2014-12-03 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 block/qcow2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ed86370..63e07bc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2718,9 +2718,9 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 error_report(Cannot change refcount entry width);
 return -ENOTSUP;
 } else {
-/* if this assertion fails, this probably means a new option was
+/* if this point is reached, this probably means a new option was
  * added without having it covered here */
-assert(false);
+abort();
 }
 
 desc++;
-- 
1.9.3




[Qemu-devel] [PATCH v4 22/26] qcow2: Use intermediate helper CB for amend

2014-12-03 Thread Max Reitz
If there is more than one time-consuming operation to be performed for
qcow2_amend_options(), we need an intermediate CB which coordinates the
progress of the individual operations and passes the result to the
original status callback.

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block/qcow2.c | 80 ++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 5b9016f..6caac05 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2654,6 +2654,75 @@ static int qcow2_downgrade(BlockDriverState *bs, int 
target_version,
 return 0;
 }
 
+typedef enum Qcow2AmendOperation {
+/* This is the value Qcow2AmendHelperCBInfo::last_operation will be
+ * statically initialized to so that the helper CB can discern the first
+ * invocation from an operation change */
+QCOW2_NO_OPERATION = 0,
+
+QCOW2_DOWNGRADING,
+} Qcow2AmendOperation;
+
+typedef struct Qcow2AmendHelperCBInfo {
+/* The code coordinating the amend operations should only modify
+ * these four fields; the rest will be managed by the CB */
+BlockDriverAmendStatusCB *original_status_cb;
+void *original_cb_opaque;
+
+Qcow2AmendOperation current_operation;
+
+/* Total number of operations to perform (only set once) */
+int total_operations;
+
+/* The following fields are managed by the CB */
+
+/* Number of operations completed */
+int operations_completed;
+
+/* Cumulative offset of all completed operations */
+int64_t offset_completed;
+
+Qcow2AmendOperation last_operation;
+int64_t last_work_size;
+} Qcow2AmendHelperCBInfo;
+
+static void qcow2_amend_helper_cb(BlockDriverState *bs,
+  int64_t operation_offset,
+  int64_t operation_work_size, void *opaque)
+{
+Qcow2AmendHelperCBInfo *info = opaque;
+int64_t current_work_size;
+int64_t projected_work_size;
+
+if (info-current_operation != info-last_operation) {
+if (info-last_operation != QCOW2_NO_OPERATION) {
+info-offset_completed += info-last_work_size;
+info-operations_completed++;
+}
+
+info-last_operation = info-current_operation;
+}
+
+assert(info-total_operations  0);
+assert(info-operations_completed  info-total_operations);
+
+info-last_work_size = operation_work_size;
+
+current_work_size = info-offset_completed + operation_work_size;
+
+/* current_work_size is the total work size for (operations_completed + 1)
+ * operations (which includes this one), so multiply it by the number of
+ * operations not covered and divide it by the number of operations
+ * covered to get a projection for the operations not covered */
+projected_work_size = current_work_size * (info-total_operations -
+   info-operations_completed - 1)
+/ (info-operations_completed + 1);
+
+info-original_status_cb(bs, info-offset_completed + operation_offset,
+ current_work_size + projected_work_size,
+ info-original_cb_opaque);
+}
+
 static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque)
@@ -2668,6 +2737,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 bool encrypt;
 int ret;
 QemuOptDesc *desc = opts-list-desc;
+Qcow2AmendHelperCBInfo helper_cb_info;
 
 while (desc  desc-name) {
 if (!qemu_opt_find(opts, desc-name)) {
@@ -2726,6 +2796,12 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 desc++;
 }
 
+helper_cb_info = (Qcow2AmendHelperCBInfo){
+.original_status_cb = status_cb,
+.original_cb_opaque = cb_opaque,
+.total_operations = (new_version  old_version)
+};
+
 /* Upgrade first (some features may require compat=1.1) */
 if (new_version  old_version) {
 s-qcow_version = new_version;
@@ -2784,7 +2860,9 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 
 /* Downgrade last (so unsupported features can be removed before) */
 if (new_version  old_version) {
-ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
+helper_cb_info.current_operation = QCOW2_DOWNGRADING;
+ret = qcow2_downgrade(bs, new_version, qcow2_amend_helper_cb,
+  helper_cb_info);
 if (ret  0) {
 return ret;
 }
-- 
1.9.3




[Qemu-devel] [PATCH v4 11/26] qcow2: More helpers for refcount modification

2014-12-03 Thread Max Reitz
Add helper functions for getting and setting refcounts in a refcount
array for any possible refcount order, and choose the correct one during
refcount initialization.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 121 -
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 125ca12..c0841a7 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -32,10 +32,49 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 int64_t offset, int64_t length, uint64_t addend,
 bool decrease, enum qcow2_discard_type type);
 
+static uint64_t get_refcount_ro0(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro1(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro2(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro3(const void *refcount_array, uint64_t index);
 static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro5(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro6(const void *refcount_array, uint64_t index);
 
+static void set_refcount_ro0(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro1(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro2(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro3(void *refcount_array, uint64_t index,
+ uint64_t value);
 static void set_refcount_ro4(void *refcount_array, uint64_t index,
  uint64_t value);
+static void set_refcount_ro5(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro6(void *refcount_array, uint64_t index,
+ uint64_t value);
+
+
+static Qcow2GetRefcountFunc *const get_refcount_funcs[] = {
+get_refcount_ro0,
+get_refcount_ro1,
+get_refcount_ro2,
+get_refcount_ro3,
+get_refcount_ro4,
+get_refcount_ro5,
+get_refcount_ro6
+};
+
+static Qcow2SetRefcountFunc *const set_refcount_funcs[] = {
+set_refcount_ro0,
+set_refcount_ro1,
+set_refcount_ro2,
+set_refcount_ro3,
+set_refcount_ro4,
+set_refcount_ro5,
+set_refcount_ro6
+};
 
 
 /*/
@@ -47,8 +86,10 @@ int qcow2_refcount_init(BlockDriverState *bs)
 unsigned int refcount_table_size2, i;
 int ret;
 
-s-get_refcount = get_refcount_ro4;
-s-set_refcount = set_refcount_ro4;
+assert(s-refcount_order = 0  s-refcount_order = 6);
+
+s-get_refcount = get_refcount_funcs[s-refcount_order];
+s-set_refcount = set_refcount_funcs[s-refcount_order];
 
 assert(s-refcount_table_size = INT_MAX / sizeof(uint64_t));
 refcount_table_size2 = s-refcount_table_size * sizeof(uint64_t);
@@ -80,6 +121,59 @@ void qcow2_refcount_close(BlockDriverState *bs)
 }
 
 
+static uint64_t get_refcount_ro0(const void *refcount_array, uint64_t index)
+{
+return (((const uint8_t *)refcount_array)[index / 8]  (index % 8))  0x1;
+}
+
+static void set_refcount_ro0(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+assert(!(value  1));
+((uint8_t *)refcount_array)[index / 8] = ~(0x1  (index % 8));
+((uint8_t *)refcount_array)[index / 8] |= value  (index % 8);
+}
+
+static uint64_t get_refcount_ro1(const void *refcount_array, uint64_t index)
+{
+return (((const uint8_t *)refcount_array)[index / 4]  (2 * (index % 4)))
+0x3;
+}
+
+static void set_refcount_ro1(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+assert(!(value  2));
+((uint8_t *)refcount_array)[index / 4] = ~(0x3  (2 * (index % 4)));
+((uint8_t *)refcount_array)[index / 4] |= value  (2 * (index % 4));
+}
+
+static uint64_t get_refcount_ro2(const void *refcount_array, uint64_t index)
+{
+return (((const uint8_t *)refcount_array)[index / 2]  (4 * (index % 2)))
+0xf;
+}
+
+static void set_refcount_ro2(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+assert(!(value  4));
+((uint8_t *)refcount_array)[index / 2] = ~(0xf  (4 * (index % 2)));
+((uint8_t *)refcount_array)[index / 2] |= value  (4 * (index % 2));
+}
+
+static uint64_t get_refcount_ro3(const void *refcount_array, uint64_t index)
+{
+return ((const uint8_t *)refcount_array)[index];
+}
+
+static void set_refcount_ro3(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+assert(!(value  8));
+((uint8_t *)refcount_array)[index] = value;
+}
+
 static uint64_t get_refcount_ro4(const 

[Qemu-devel] [PATCH v4 21/26] qcow2: Split upgrade/downgrade paths for amend

2014-12-03 Thread Max Reitz
If the image version should be upgraded, that is the first we should do;
if it should be downgraded, that is the last we should do. So split the
version change block into an upgrade part at the start and a downgrade
part at the end.

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 block/qcow2.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 63e07bc..5b9016f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2726,20 +2726,13 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 desc++;
 }
 
-if (new_version != old_version) {
-if (new_version  old_version) {
-/* Upgrade */
-s-qcow_version = new_version;
-ret = qcow2_update_header(bs);
-if (ret  0) {
-s-qcow_version = old_version;
-return ret;
-}
-} else {
-ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
-if (ret  0) {
-return ret;
-}
+/* Upgrade first (some features may require compat=1.1) */
+if (new_version  old_version) {
+s-qcow_version = new_version;
+ret = qcow2_update_header(bs);
+if (ret  0) {
+s-qcow_version = old_version;
+return ret;
 }
 }
 
@@ -2753,7 +2746,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 
 if (s-use_lazy_refcounts != lazy_refcounts) {
 if (lazy_refcounts) {
-if (s-qcow_version  3) {
+if (new_version  3) {
 error_report(Lazy refcounts only supported with compatibility 

  level 1.1 and above (use compat=1.1 or 
greater));
 return -EINVAL;
@@ -2789,6 +2782,14 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 }
 }
 
+/* Downgrade last (so unsupported features can be removed before) */
+if (new_version  old_version) {
+ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
+if (ret  0) {
+return ret;
+}
+}
+
 return 0;
 }
 
-- 
1.9.3




Re: [Qemu-devel] [Xen-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap

2014-12-03 Thread Don Slutz

On 12/03/14 07:20, Stefano Stabellini wrote:

On Wed, 3 Dec 2014, Wei Liu wrote:

On Tue, Dec 02, 2014 at 03:23:29PM -0500, Don Slutz wrote:
[...]

   hw_error(xc_domain_getinfo failed);
   }
-if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
-(nr_pfn * XC_PAGE_SIZE / 1024))  0) {
+max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE;
+free_pages = max_pages - info.nr_pages;
+real_free = free_pages;
+if (free_pages  VGA_HOLE_SIZE) {
+free_pages -= VGA_HOLE_SIZE;
+} else {
+free_pages = 0;
+}

I don't think we need to subtract VGA_HOLE_SIZE.

If you do not use some slack (like 32 here), xen does report:


(d5) [2014-11-29 17:07:21] Loaded SeaBIOS
(d5) [2014-11-29 17:07:21] Creating MP tables ...
(d5) [2014-11-29 17:07:21] Loading ACPI ...
(XEN) [2014-11-29 17:07:21] page_alloc.c:1568:d5 Over-allocation for domain
5: 1057417  1057416
(XEN) [2014-11-29 17:07:21] memory.c:158:d5 Could not allocate order=0

This message is a bit red herring.

It's hvmloader trying to populate ram for firmware data. The actual
amount of extra pages needed depends on the firmware.

In any case it's safe to disallow hvmloader from doing so, it will just
relocate some pages from ram (hence shrinking *mem_end).

That looks like a better solution



I went with a leave some slack so that the error message above is not 
output.


When a change to hvmloader is done so that the message does not appear 
during

normal usage, the extra pages in QEMU can be dropped.



extent: id=5 memflags=0 (0 of 1)
(d5) [2014-11-29 17:07:21] vm86 TSS at 00098c00
(d5) [2014-11-29 17:07:21] BIOS map:


However while QEMU startup ends with 32 free pages (maxmem - curmem)
this quickly changes to 23.  I.E. there are 7 more places that act like a
call
to xc_domain_populate_physmap_exact() but do not log errors if there is
no free pages.  And just to make sure I do not fully understand what is
happening here, after the message above, the domain appears to work
fine and ends up with 8 free pages (code I do not know about ends up
releasing populated pages).

So my current thinking is to have QEMU leave a few (9 to 32 (64?)) pages
free.


Unless we know before hand how many pages hvmloader needs this number is
estimation at best.
  
Right. It would be nice to get rid of any estimations by:

- making hvmloader use normal ram
- making qemu increase maxmem
- removing all the estimation from libxl


Sounds like a plan for 4.6

-Don Slutz



[Qemu-devel] [PATCH v4 12/26] qcow2: Open images with refcount order != 4

2014-12-03 Thread Max Reitz
No longer refuse to open images with a different refcount entry width
than 16 bits; only reject images with a refcount width larger than 64
bits (which is prohibited by the specification).

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 block/qcow2.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 25d6a66..ff57566 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -677,10 +677,10 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 /* Check support for various header values */
-if (header.refcount_order != 4) {
-report_unsupported(bs, errp, %d bit reference counts,
-   1  header.refcount_order);
-ret = -ENOTSUP;
+if (header.refcount_order  6) {
+error_setg(errp, Reference count entry width too large; may not 
+   exceed 64 bits);
+ret = -EINVAL;
 goto fail;
 }
 s-refcount_order = header.refcount_order;
-- 
1.9.3




[Qemu-devel] [PATCH v4 23/26] qcow2: Add function for refcount order amendment

2014-12-03 Thread Max Reitz
Add a function qcow2_change_refcount_order() which allows changing the
refcount order of a qcow2 image.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2-refcount.c | 452 +
 block/qcow2.h  |   4 +
 2 files changed, 456 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index c0841a7..92f01f3 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2491,3 +2491,455 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, 
int ign, int64_t offset,
 
 return 0;
 }
+
+/* A pointer to a function of this type is given to walk_over_reftable(). That
+ * function will create refblocks and pass them to a RefblockFinishOp once they
+ * are completed (@refblock). @refblock_empty is set if the refblock is
+ * completely empty.
+ *
+ * Along with the refblock, a corresponding reftable entry is passed, in the
+ * reftable @reftable (which may be reallocated) at @reftable_index.
+ *
+ * @allocated should be set to true if a new cluster has been allocated.
+ */
+typedef int (RefblockFinishOp)(BlockDriverState *bs, uint64_t **reftable,
+   uint64_t reftable_index, uint64_t 
*reftable_size,
+   void *refblock, bool refblock_empty,
+   bool *allocated, Error **errp);
+
+/**
+ * This operation for walk_over_reftable() allocates the refblock on disk (if
+ * it is not empty) and inserts its offset into the new reftable. The size of
+ * this new reftable is increased as required.
+ */
+static int alloc_refblock(BlockDriverState *bs, uint64_t **reftable,
+  uint64_t reftable_index, uint64_t *reftable_size,
+  void *refblock, bool refblock_empty, bool *allocated,
+  Error **errp)
+{
+BDRVQcowState *s = bs-opaque;
+int64_t offset;
+
+if (!refblock_empty  reftable_index = *reftable_size) {
+uint64_t *new_reftable;
+uint64_t new_reftable_size;
+
+new_reftable_size = ROUND_UP(reftable_index + 1,
+ s-cluster_size / sizeof(uint64_t));
+if (new_reftable_size  QCOW_MAX_REFTABLE_SIZE / sizeof(uint64_t)) {
+error_setg(errp,
+   This operation would make the refcount table grow 
+   beyond the maximum size supported by QEMU, aborting);
+return -ENOTSUP;
+}
+
+new_reftable = g_try_realloc(*reftable, new_reftable_size *
+sizeof(uint64_t));
+if (!new_reftable) {
+error_setg(errp, Failed to increase reftable buffer size);
+return -ENOMEM;
+}
+
+memset(new_reftable + *reftable_size, 0,
+   (new_reftable_size - *reftable_size) * sizeof(uint64_t));
+
+*reftable  = new_reftable;
+*reftable_size = new_reftable_size;
+}
+
+if (!refblock_empty  !(*reftable)[reftable_index]) {
+offset = qcow2_alloc_clusters(bs, s-cluster_size);
+if (offset  0) {
+error_setg_errno(errp, -offset, Failed to allocate refblock);
+return offset;
+}
+(*reftable)[reftable_index] = offset;
+*allocated = true;
+}
+
+return 0;
+}
+
+/**
+ * This operation for walk_over_reftable() writes the refblock to disk at the
+ * offset specified by the new reftable's entry. It does not modify the new
+ * reftable or change any refcounts.
+ */
+static int flush_refblock(BlockDriverState *bs, uint64_t **reftable,
+  uint64_t reftable_index, uint64_t *reftable_size,
+  void *refblock, bool refblock_empty, bool *allocated,
+  Error **errp)
+{
+BDRVQcowState *s = bs-opaque;
+int64_t offset;
+int ret;
+
+if (reftable_index  *reftable_size  (*reftable)[reftable_index]) {
+offset = (*reftable)[reftable_index];
+
+ret = qcow2_pre_write_overlap_check(bs, 0, offset, s-cluster_size);
+if (ret  0) {
+error_setg_errno(errp, -ret, Overlap check failed);
+return ret;
+}
+
+ret = bdrv_pwrite(bs-file, offset, refblock, s-cluster_size);
+if (ret  0) {
+error_setg_errno(errp, -ret, Failed to write refblock);
+return ret;
+}
+} else {
+assert(refblock_empty);
+}
+
+return 0;
+}
+
+/**
+ * This function walks over the existing reftable and every referenced 
refblock;
+ * if @new_set_refcount is non-NULL, it is called for every refcount entry to
+ * create an equal new entry in the passed @new_refblock. Once that
+ * @new_refblock is completely filled, @operation will be called.
+ *
+ * @status_cb and @cb_opaque are used for the amend operation's status 
callback.
+ * @index is the index of the walk_over_reftable() calls and @total is the 
total
+ * number of walk_over_reftable() 

[Qemu-devel] [PATCH v4 24/26] qcow2: Invoke refcount order amendment function

2014-12-03 Thread Max Reitz
Make use of qcow2_change_refcount_order() to support changing the
refcount order with qemu-img amend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2.c | 44 +++-
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6caac05..7df5af0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2606,13 +2606,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int 
target_version,
 }
 
 if (s-refcount_order != 4) {
-/* we would have to convert the image to a refcount_order == 4 image
- * here; however, since qemu (at the time of writing this) does not
- * support anything different than 4 anyway, there is no point in doing
- * so right now; however, we should error out (if qemu supports this in
- * the future and this code has not been adapted) */
-error_report(qcow2_downgrade: Image refcount orders other than 4 are 
- currently not supported.);
+error_report(compat=0.10 requires refcount_bits=16);
 return -ENOTSUP;
 }
 
@@ -2660,6 +2654,7 @@ typedef enum Qcow2AmendOperation {
  * invocation from an operation change */
 QCOW2_NO_OPERATION = 0,
 
+QCOW2_CHANGING_REFCOUNT_ORDER,
 QCOW2_DOWNGRADING,
 } Qcow2AmendOperation;
 
@@ -2735,6 +2730,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 const char *compat = NULL;
 uint64_t cluster_size = s-cluster_size;
 bool encrypt;
+int refcount_bits = s-refcount_bits;
 int ret;
 QemuOptDesc *desc = opts-list-desc;
 Qcow2AmendHelperCBInfo helper_cb_info;
@@ -2785,8 +2781,16 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
 } else if (!strcmp(desc-name, BLOCK_OPT_REFCOUNT_BITS)) {
-error_report(Cannot change refcount entry width);
-return -ENOTSUP;
+refcount_bits = qemu_opt_get_number(opts, BLOCK_OPT_REFCOUNT_BITS,
+refcount_bits);
+
+if (refcount_bits = 0 || refcount_bits  64 ||
+!is_power_of_2(refcount_bits))
+{
+error_report(Refcount width must be a power of two and may 
+ not exceed 64 bits);
+return -EINVAL;
+}
 } else {
 /* if this point is reached, this probably means a new option was
  * added without having it covered here */
@@ -2800,6 +2804,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 .original_status_cb = status_cb,
 .original_cb_opaque = cb_opaque,
 .total_operations = (new_version  old_version)
+  + (s-refcount_bits != refcount_bits)
 };
 
 /* Upgrade first (some features may require compat=1.1) */
@@ -2812,6 +2817,27 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 }
 }
 
+if (s-refcount_bits != refcount_bits) {
+int refcount_order = ffs(refcount_bits) - 1;
+Error *local_error = NULL;
+
+if (new_version  3  refcount_bits != 16) {
+error_report(Different refcount widths than 16 bits require 
+ compatibility level 1.1 or above (use compat=1.1 or 
+ greater));
+return -EINVAL;
+}
+
+helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;
+ret = qcow2_change_refcount_order(bs, refcount_order,
+  qcow2_amend_helper_cb,
+  helper_cb_info, local_error);
+if (ret  0) {
+qerror_report_err(local_error);
+return ret;
+}
+}
+
 if (backing_file || backing_format) {
 ret = qcow2_change_backing_file(bs, backing_file ?: bs-backing_file,
 backing_format ?: bs-backing_format);
-- 
1.9.3




[Qemu-devel] [PATCH v4 19/26] qcow2: Use error_report() in qcow2_amend_options()

2014-12-03 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
---
 block/qcow2.c  | 14 ++
 tests/qemu-iotests/061.out | 14 +++---
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b028a4a..ed86370 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2685,11 +2685,11 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 } else if (!strcmp(compat, 1.1)) {
 new_version = 3;
 } else {
-fprintf(stderr, Unknown compatibility level %s.\n, compat);
+error_report(Unknown compatibility level %s, compat);
 return -EINVAL;
 }
 } else if (!strcmp(desc-name, BLOCK_OPT_PREALLOC)) {
-fprintf(stderr, Cannot change preallocation mode.\n);
+error_report(Cannot change preallocation mode);
 return -ENOTSUP;
 } else if (!strcmp(desc-name, BLOCK_OPT_SIZE)) {
 new_size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
@@ -2701,16 +2701,14 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT,
 s-crypt_method);
 if (encrypt != !!s-crypt_method) {
-fprintf(stderr, Changing the encryption flag is not 
-supported.\n);
+error_report(Changing the encryption flag is not supported);
 return -ENOTSUP;
 }
 } else if (!strcmp(desc-name, BLOCK_OPT_CLUSTER_SIZE)) {
 cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
  cluster_size);
 if (cluster_size != s-cluster_size) {
-fprintf(stderr, Changing the cluster size is not 
-supported.\n);
+error_report(Changing the cluster size is not supported);
 return -ENOTSUP;
 }
 } else if (!strcmp(desc-name, BLOCK_OPT_LAZY_REFCOUNTS)) {
@@ -2756,8 +2754,8 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 if (s-use_lazy_refcounts != lazy_refcounts) {
 if (lazy_refcounts) {
 if (s-qcow_version  3) {
-fprintf(stderr, Lazy refcounts only supported with 
compatibility 
-level 1.1 and above (use compat=1.1 or greater)\n);
+error_report(Lazy refcounts only supported with compatibility 

+ level 1.1 and above (use compat=1.1 or 
greater));
 return -EINVAL;
 }
 s-compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 9045544..2fd92ca 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -281,19 +281,19 @@ No errors were found on the image.
 === Testing invalid configurations ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
-Lazy refcounts only supported with compatibility level 1.1 and above (use 
compat=1.1 or greater)
+qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above 
(use compat=1.1 or greater)
 qemu-img: Error while amending options: Invalid argument
-Lazy refcounts only supported with compatibility level 1.1 and above (use 
compat=1.1 or greater)
+qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above 
(use compat=1.1 or greater)
 qemu-img: Error while amending options: Invalid argument
-Unknown compatibility level 0.42.
+qemu-img: Unknown compatibility level 0.42
 qemu-img: Error while amending options: Invalid argument
 qemu-img: Invalid parameter 'foo'
 qemu-img: Invalid options for file format 'qcow2'
-Changing the cluster size is not supported.
+qemu-img: Changing the cluster size is not supported
 qemu-img: Error while amending options: Operation not supported
-Changing the encryption flag is not supported.
+qemu-img: Changing the encryption flag is not supported
 qemu-img: Error while amending options: Operation not supported
-Cannot change preallocation mode.
+qemu-img: Cannot change preallocation mode
 qemu-img: Error while amending options: Operation not supported
 
 === Testing correct handling of unset value ===
@@ -301,7 +301,7 @@ qemu-img: Error while amending options: Operation not 
supported
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 Should work:
 Should not work:
-Changing the cluster size is not supported.
+qemu-img: Changing the cluster size is not supported
 qemu-img: Error while amending options: Operation not supported
 
 === Testing zero expansion on inactive clusters ===
-- 
1.9.3




[Qemu-devel] [PATCH v4 25/26] qcow2: Point to amend function in check

2014-12-03 Thread Max Reitz
If a reference count is not representable with the current refcount
order, the image check should point to qemu-img amend for increasing the
refcount order. However, qemu-img amend needs write access to the image
which cannot be provided if the image is marked corrupt; and the image
check will not mark the image consistent unless everything actually is
consistent.

Therefore, if an image is marked corrupt and the image check encounters
a reference count overflow, it cannot be fixed by using qemu-img amend
to increase the refcount order. Instead, one has to use qemu-img convert
to create a completely new copy of the image in this case.

Alternatively, we may want to give the user a way of manually removing
the corrupt flag, maybe through qemu-img amend, but this is not part of
this patch.

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block/qcow2-refcount.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 92f01f3..a656324 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1370,6 +1370,9 @@ static int inc_refcounts(BlockDriverState *bs,
 if (refcount == s-refcount_max) {
 fprintf(stderr, ERROR: overflow cluster offset=0x% PRIx64
 \n, cluster_offset);
+fprintf(stderr, Use qemu-img amend to increase the refcount entry 

+width or qemu-img convert to create a clean copy if the 
+image cannot be opened for writing\n);
 res-corruptions++;
 continue;
 }
-- 
1.9.3




Re: [Qemu-devel] [PATCH v3 00/13] block: Various Coverity-spotted fixes

2014-12-03 Thread Kevin Wolf
Am 02.12.2014 um 18:32 hat Max Reitz geschrieben:
 This series fixes various issues spotted by Coverity. None of these is
 critical; most are just If you do something crazy, qemu-img crashes or
 But what if there is no qcow2 driver?.
 
 Also, none is security-relevant. The only crashes which are fixed here
 are sure to have resulted from dereferencing a NULL pointer.

Thanks, applied all to block-next.

Kevin



[Qemu-devel] [PATCH v4 26/26] iotests: Add test for different refcount widths

2014-12-03 Thread Max Reitz
Add a test for conversion between different refcount widths and errors
specific to certain widths (i.e. snapshots with refcount_bits=1).

Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/112 | 296 +
 tests/qemu-iotests/112.out | 155 
 tests/qemu-iotests/group   |   1 +
 3 files changed, 452 insertions(+)
 create mode 100755 tests/qemu-iotests/112
 create mode 100644 tests/qemu-iotests/112.out

diff --git a/tests/qemu-iotests/112 b/tests/qemu-iotests/112
new file mode 100755
index 000..74c75ab
--- /dev/null
+++ b/tests/qemu-iotests/112
@@ -0,0 +1,296 @@
+#!/bin/bash
+#
+# Test cases for different refcount_bits values
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq=$(basename $0)
+echo QA output created by $seq
+
+here=$PWD
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap _cleanup; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qcow2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+# This test will set refcount_bits on its own which would conflict with the
+# manual setting; compat will be overridden as well
+_unsupported_imgopts refcount_bits 'compat=0.10'
+
+function print_refcount_bits()
+{
+$QEMU_IMG info $TEST_IMG | sed -n '/refcount bits:/ s/^ *//p'
+}
+
+echo
+echo '=== refcount_bits limits ==='
+echo
+
+# Must be positive (non-zero)
+IMGOPTS=$IMGOPTS,refcount_bits=0 _make_test_img 64M
+# Must be positive (non-negative)
+IMGOPTS=$IMGOPTS,refcount_bits=-1 _make_test_img 64M
+# May not exceed 64
+IMGOPTS=$IMGOPTS,refcount_bits=128 _make_test_img 64M
+# Must be a power of two
+IMGOPTS=$IMGOPTS,refcount_bits=42 _make_test_img 64M
+
+# 1 is the minimum
+IMGOPTS=$IMGOPTS,refcount_bits=1 _make_test_img 64M
+print_refcount_bits
+
+# 64 is the maximum
+IMGOPTS=$IMGOPTS,refcount_bits=64 _make_test_img 64M
+print_refcount_bits
+
+# 16 is the default
+_make_test_img 64M
+print_refcount_bits
+
+echo
+echo '=== refcount_bits and compat=0.10 ==='
+echo
+
+# Should work
+IMGOPTS=$IMGOPTS,compat=0.10,refcount_bits=16 _make_test_img 64M
+print_refcount_bits
+
+# Should not work
+IMGOPTS=$IMGOPTS,compat=0.10,refcount_bits=1 _make_test_img 64M
+IMGOPTS=$IMGOPTS,compat=0.10,refcount_bits=64 _make_test_img 64M
+
+
+echo
+echo '=== Snapshot limit on refcount_bits=1 ==='
+echo
+
+IMGOPTS=$IMGOPTS,refcount_bits=1 _make_test_img 64M
+print_refcount_bits
+
+$QEMU_IO -c 'write 0 512' $TEST_IMG | _filter_qemu_io
+
+# Should fail for now; in the future, this might be supported by automatically
+# copying all clusters with overflowing refcount
+$QEMU_IMG snapshot -c foo $TEST_IMG
+
+# The new L1 table could/should be leaked
+_check_test_img
+
+echo
+echo '=== Snapshot limit on refcount_bits=2 ==='
+echo
+
+IMGOPTS=$IMGOPTS,refcount_bits=2 _make_test_img 64M
+print_refcount_bits
+
+$QEMU_IO -c 'write 0 512' $TEST_IMG | _filter_qemu_io
+
+# Should succeed
+$QEMU_IMG snapshot -c foo $TEST_IMG
+$QEMU_IMG snapshot -c bar $TEST_IMG
+# Should fail (4th reference)
+$QEMU_IMG snapshot -c baz $TEST_IMG
+
+# The new L1 table could/should be leaked
+_check_test_img
+
+echo
+echo '=== Compressed clusters with refcount_bits=1 ==='
+echo
+
+IMGOPTS=$IMGOPTS,refcount_bits=1 _make_test_img 64M
+print_refcount_bits
+
+# Both should fit into a single host cluster; instead of failing to increase 
the
+# refcount of that cluster, qemu should just allocate a new cluster and make
+# this operation succeed
+$QEMU_IO -c 'write -P 0 -c  0  64k' \
+ -c 'write -P 1 -c 64k 64k' \
+ $TEST_IMG | _filter_qemu_io
+
+_check_test_img
+
+echo
+echo '=== MSb set in 64 bit refcount ==='
+echo
+
+IMGOPTS=$IMGOPTS,refcount_bits=64 _make_test_img 64M
+print_refcount_bits
+
+$QEMU_IO -c 'write 0 512' $TEST_IMG | _filter_qemu_io
+
+# Set the MSb in the refblock entry of the data cluster
+poke_file $TEST_IMG $((0x20028)) \x80\x00\x00\x00\x00\x00\x00\x00
+
+# Clear OFLAG_COPIED in the L2 entry of the data cluster
+poke_file $TEST_IMG $((0x4)) \x00\x00\x00\x00\x00\x05\x00\x00
+
+# Try to write to that cluster (should work, even though the MSb is set)
+$QEMU_IO -c 'write 0 512' $TEST_IMG | _filter_qemu_io

Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface

2014-12-03 Thread Michael S. Tsirkin
On Wed, Dec 03, 2014 at 12:29:43PM +, Stefano Stabellini wrote:
 The second patch is already Acked.
 You just need a review on this one to move forward, right?
 
 Andreas, Michael?

Looks like a generic qdev thing, nothing to do with me.

 On Wed, 3 Dec 2014, Paul Durrant wrote:
  The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
  models explicitly register with Xen for config space accesses. This patch
  adds a listener interface into qdev-core which can be used by the Xen
  interface code to monitor for arrival and departure of PCI devices.
  
  Signed-off-by: Paul Durrant paul.durr...@citrix.com
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Andreas Faerber afaer...@suse.de
  Cc: Paolo Bonzini pbonz...@redhat.com
  Cc: Peter Crosthwaite peter.crosthwa...@xilinx.com
  Cc: Igor Mammedov imamm...@redhat.com
  Cc: Markus Armbruster arm...@redhat.com
  Cc: Thomas Huth th...@linux.vnet.ibm.com
  Cc: Christian Borntraeger borntrae...@de.ibm.com
  ---
   hw/core/qdev.c  |   54 
  +++
   include/hw/qdev-core.h  |   10 +
   include/qemu/typedefs.h |1 +
   3 files changed, 65 insertions(+)
  
  diff --git a/hw/core/qdev.c b/hw/core/qdev.c
  index fcb1638..4a9c1f6 100644
  --- a/hw/core/qdev.c
  +++ b/hw/core/qdev.c
  @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
   return 0;
   }
   
  +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
  += QTAILQ_HEAD_INITIALIZER(qdev_listeners);
  +
  +enum ListenerDirection { Forward, Reverse };
  +
  +#define QDEV_LISTENER_CALL(_callback, _direction, _args...) \
  +do {\
  +DeviceListener *_listener;  \
  +\
  +switch (_direction) {   \
  +case Forward:   \
  +QTAILQ_FOREACH(_listener, qdev_listeners, link) {  \
  +if (_listener-_callback) { \
  +_listener-_callback(_listener, ##_args);   \
  +}   \
  +}   \
  +break;  \
  +case Reverse:   \
  +QTAILQ_FOREACH_REVERSE(_listener, qdev_listeners,  \
  +   qdev_listeners, link) {  \
  +if (_listener-_callback) { \
  +_listener-_callback(_listener, ##_args);   \
  +}   \
  +}   \
  +break;  \
  +default:\
  +abort();\
  +}   \
  +} while (0)
  +
  +static int qdev_listener_add(DeviceState *dev, void *opaque)
  +{
  +QDEV_LISTENER_CALL(realize, Forward, dev);
  +
  +return 0;
  +}
  +
  +void qdev_listener_register(DeviceListener *listener)
  +{
  +QTAILQ_INSERT_TAIL(qdev_listeners, listener, link);
  +
  +qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
  +   NULL, NULL);
  +}
  +
  +void qdev_listener_unregister(DeviceListener *listener)
  +{
  +QTAILQ_REMOVE(qdev_listeners, listener, link);
  +}
  +
   static void device_realize(DeviceState *dev, Error **errp)
   {
   DeviceClass *dc = DEVICE_GET_CLASS(dev);
  @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error 
  **errp)
   return;
   }
   }
  +
  +QDEV_LISTENER_CALL(realize, Forward, dev);
   }
   
   static void device_unrealize(DeviceState *dev, Error **errp)
   {
   DeviceClass *dc = DEVICE_GET_CLASS(dev);
   
  +QDEV_LISTENER_CALL(unrealize, Reverse, dev);
  +
   if (dc-exit) {
   int rc = dc-exit(dev);
   if (rc  0) {
  diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
  index 178fee2..f2dc267 100644
  --- a/include/hw/qdev-core.h
  +++ b/include/hw/qdev-core.h
  @@ -167,6 +167,12 @@ struct DeviceState {
   int alias_required_for_version;
   };
   
  +struct DeviceListener {
  +void (*realize)(DeviceListener *listener, DeviceState *dev);
  +void (*unrealize)(DeviceListener *listener, DeviceState *dev);
  +QTAILQ_ENTRY(DeviceListener) link;
  +};
  +
   #define TYPE_BUS bus
   #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
   #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
  @@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState 
  *bus, DeviceState 

Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface

2014-12-03 Thread Paolo Bonzini


On 03/12/2014 14:40, Michael S. Tsirkin wrote:
 On Wed, Dec 03, 2014 at 12:29:43PM +, Stefano Stabellini wrote:
 The second patch is already Acked.
 You just need a review on this one to move forward, right?

 Andreas, Michael?
 
 Looks like a generic qdev thing, nothing to do with me.
 
 On Wed, 3 Dec 2014, Paul Durrant wrote:
 The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
 models explicitly register with Xen for config space accesses. This patch
 adds a listener interface into qdev-core which can be used by the Xen
 interface code to monitor for arrival and departure of PCI devices.

 Signed-off-by: Paul Durrant paul.durr...@citrix.com
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Andreas Faerber afaer...@suse.de
 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Peter Crosthwaite peter.crosthwa...@xilinx.com
 Cc: Igor Mammedov imamm...@redhat.com
 Cc: Markus Armbruster arm...@redhat.com
 Cc: Thomas Huth th...@linux.vnet.ibm.com
 Cc: Christian Borntraeger borntrae...@de.ibm.com
 ---
  hw/core/qdev.c  |   54 
 +++
  include/hw/qdev-core.h  |   10 +
  include/qemu/typedefs.h |1 +
  3 files changed, 65 insertions(+)

 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index fcb1638..4a9c1f6 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
  return 0;
  }
  
 +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
 += QTAILQ_HEAD_INITIALIZER(qdev_listeners);
 +
 +enum ListenerDirection { Forward, Reverse };
 +
 +#define QDEV_LISTENER_CALL(_callback, _direction, _args...) \
 +do {\
 +DeviceListener *_listener;  \
 +\
 +switch (_direction) {   \
 +case Forward:   \
 +QTAILQ_FOREACH(_listener, qdev_listeners, link) {  \
 +if (_listener-_callback) { \
 +_listener-_callback(_listener, ##_args);   \
 +}   \
 +}   \
 +break;  \
 +case Reverse:   \
 +QTAILQ_FOREACH_REVERSE(_listener, qdev_listeners,  \
 +   qdev_listeners, link) {  \
 +if (_listener-_callback) { \
 +_listener-_callback(_listener, ##_args);   \
 +}   \
 +}   \
 +break;  \
 +default:\
 +abort();\
 +}   \
 +} while (0)
 +
 +static int qdev_listener_add(DeviceState *dev, void *opaque)
 +{
 +QDEV_LISTENER_CALL(realize, Forward, dev);
 +
 +return 0;
 +}
 +
 +void qdev_listener_register(DeviceListener *listener)
 +{
 +QTAILQ_INSERT_TAIL(qdev_listeners, listener, link);
 +
 +qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
 +   NULL, NULL);
 +}
 +
 +void qdev_listener_unregister(DeviceListener *listener)
 +{
 +QTAILQ_REMOVE(qdev_listeners, listener, link);
 +}
 +
  static void device_realize(DeviceState *dev, Error **errp)
  {
  DeviceClass *dc = DEVICE_GET_CLASS(dev);
 @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error 
 **errp)
  return;
  }
  }
 +
 +QDEV_LISTENER_CALL(realize, Forward, dev);
  }
  
  static void device_unrealize(DeviceState *dev, Error **errp)
  {
  DeviceClass *dc = DEVICE_GET_CLASS(dev);
  
 +QDEV_LISTENER_CALL(unrealize, Reverse, dev);

These need to be in device_set_realized.  device_realize and
device_unrealize are just for backwards-compatibility to devices that
still use init/exit.

Also, this has to be call to be _after_ unrealization, i.e. after
setting dev-pending_deleted_event in device_set_realize.

Paolo

  if (dc-exit) {
  int rc = dc-exit(dev);
  if (rc  0) {
 diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
 index 178fee2..f2dc267 100644
 --- a/include/hw/qdev-core.h
 +++ b/include/hw/qdev-core.h
 @@ -167,6 +167,12 @@ struct DeviceState {
  int alias_required_for_version;
  };
  
 +struct DeviceListener {
 +void (*realize)(DeviceListener *listener, DeviceState *dev);
 +void (*unrealize)(DeviceListener *listener, DeviceState *dev);
 +QTAILQ_ENTRY(DeviceListener) link;
 +};
 +
  #define TYPE_BUS bus
  #define 

[Qemu-devel] [PATCH v2 2/2] iotests: Add test for vmdk JSON file names

2014-12-03 Thread Max Reitz
Add a test for vmdk files which use a file with a JSON file name, and
which then try to open extents. That should fail and the error message
should at least try to look helpful.

Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/059 | 6 ++
 tests/qemu-iotests/059.out | 4 
 2 files changed, 10 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 2ed1a7f..50ca5ce 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -111,6 +111,12 @@ $QEMU_IO -f qcow2 -c write -P 0xb 10240 512 
$TEST_IMG.qcow2 | _filter_qemu_i
 $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized 
$TEST_IMG.qcow2 $TEST_IMG 21
 
 echo
+echo === Testing monolithicFlat with internally generated JSON file name ===
+IMGOPTS=subformat=monolithicFlat _make_test_img 64M
+$QEMU_IO -c open -o 
driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio
 21 \
+| _filter_testdir | _filter_imgfmt
+
+echo
 echo === Testing version 3 ===
 _use_sample_img iotest-version3.vmdk.bz2
 _img_info
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 0dadba6..cbb0de4 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2053,6 +2053,10 @@ wrote 512/512 bytes at offset 0
 wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+=== Testing monolithicFlat with internally generated JSON file name ===
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor 
file 'json:{image: {driver: file, filename: TEST_DIR/t.IMGFMT}, 
driver: blkdebug, inject-error.0.event: read_aio}'
+
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
 file format: IMGFMT
-- 
1.9.3




[Qemu-devel] [PATCH v2 1/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Max Reitz
If vmdk blindly tries to use path_combine() using bs-file-filename as
the base file name, this will result in a bad error message for JSON
file names when calling bdrv_open(). It is better to only try
bs-file-exact_filename; if that is empty, bs-file-filename will be
useless for path_combine() and an error should be emitted (containing
bs-file-filename because desc_file_path (which is
bs-file-exact_filename) is empty).

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c   |  2 +-
 block/vmdk.c  | 10 +-
 include/block/block.h |  1 +
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 591fbe4..239ae5f 100644
--- a/block.c
+++ b/block.c
@@ -229,7 +229,7 @@ size_t bdrv_opt_mem_align(BlockDriverState *bs)
 }
 
 /* check if the path starts with protocol: */
-static int path_has_protocol(const char *path)
+int path_has_protocol(const char *path)
 {
 const char *p;
 
diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..65413a1 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -818,6 +818,14 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 goto next_line;
 }
 
+if (!path_is_absolute(fname)  !path_has_protocol(fname) 
+!desc_file_path[0])
+{
+error_setg(errp, Cannot use relative extent paths with VMDK 
+   descriptor file '%s', bs-file-filename);
+return -EINVAL;
+}
+
 path_combine(extent_path, sizeof(extent_path),
 desc_file_path, fname);
 extent_file = NULL;
@@ -894,7 +902,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags, char *buf,
 }
 s-create_type = g_strdup(ct);
 s-desc_offset = 0;
-ret = vmdk_parse_extents(buf, bs, bs-file-filename, errp);
+ret = vmdk_parse_extents(buf, bs, bs-file-exact_filename, errp);
 exit:
 return ret;
 }
diff --git a/include/block/block.h b/include/block/block.h
index 610be9f..919c8f5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -401,6 +401,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs,
 char *dest, size_t sz);
 int bdrv_is_snapshot(BlockDriverState *bs);
 
+int path_has_protocol(const char *path);
 int path_is_absolute(const char *path);
 void path_combine(char *dest, int dest_size,
   const char *base_path,
-- 
1.9.3




[Qemu-devel] [PATCH v4 14/26] qcow2: Use symbolic macros in qcow2_amend_options

2014-12-03 Thread Max Reitz
qcow2_amend_options() should not compare options against some inline
strings but rather use the symbolic macros available for each of the
creation options.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qcow2.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 766f79a..912680c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2664,8 +2664,8 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 continue;
 }
 
-if (!strcmp(desc-name, compat)) {
-compat = qemu_opt_get(opts, compat);
+if (!strcmp(desc-name, BLOCK_OPT_COMPAT_LEVEL)) {
+compat = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL);
 if (!compat) {
 /* preserve default */
 } else if (!strcmp(compat, 0.10)) {
@@ -2676,32 +2676,33 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 fprintf(stderr, Unknown compatibility level %s.\n, compat);
 return -EINVAL;
 }
-} else if (!strcmp(desc-name, preallocation)) {
+} else if (!strcmp(desc-name, BLOCK_OPT_PREALLOC)) {
 fprintf(stderr, Cannot change preallocation mode.\n);
 return -ENOTSUP;
-} else if (!strcmp(desc-name, size)) {
-new_size = qemu_opt_get_size(opts, size, 0);
-} else if (!strcmp(desc-name, backing_file)) {
-backing_file = qemu_opt_get(opts, backing_file);
-} else if (!strcmp(desc-name, backing_fmt)) {
-backing_format = qemu_opt_get(opts, backing_fmt);
-} else if (!strcmp(desc-name, encryption)) {
-encrypt = qemu_opt_get_bool(opts, encryption, s-crypt_method);
+} else if (!strcmp(desc-name, BLOCK_OPT_SIZE)) {
+new_size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+} else if (!strcmp(desc-name, BLOCK_OPT_BACKING_FILE)) {
+backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
+} else if (!strcmp(desc-name, BLOCK_OPT_BACKING_FMT)) {
+backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
+} else if (!strcmp(desc-name, BLOCK_OPT_ENCRYPT)) {
+encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT,
+s-crypt_method);
 if (encrypt != !!s-crypt_method) {
 fprintf(stderr, Changing the encryption flag is not 
 supported.\n);
 return -ENOTSUP;
 }
-} else if (!strcmp(desc-name, cluster_size)) {
-cluster_size = qemu_opt_get_size(opts, cluster_size,
+} else if (!strcmp(desc-name, BLOCK_OPT_CLUSTER_SIZE)) {
+cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
  cluster_size);
 if (cluster_size != s-cluster_size) {
 fprintf(stderr, Changing the cluster size is not 
 supported.\n);
 return -ENOTSUP;
 }
-} else if (!strcmp(desc-name, lazy_refcounts)) {
-lazy_refcounts = qemu_opt_get_bool(opts, lazy_refcounts,
+} else if (!strcmp(desc-name, BLOCK_OPT_LAZY_REFCOUNTS)) {
+lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
 } else {
 /* if this assertion fails, this probably means a new option was
-- 
1.9.3




Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name

2014-12-03 Thread Eric Blake
On 12/03/2014 03:30 AM, Kevin Wolf wrote:
 [ CCed Benoît and Max, this is blockdev work ]
 [ CCed Jeff, we're also talking about op blockers ]
 
 Not stripping quoted text for their convenience.

I still intend to go through this mail in more detail, but off of a
quick glance, I see you missed a command:

qapi-schema.json:
* change
  @device (sometimes) names a backend, with the further restriction that
no backend can be named 'vnc'

TODO: add new commands that de-multiplex this stupidity.  'change' is
not extensible, and management should not be using it once the new
commands are in place


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] qemu rdma live migration

2014-12-03 Thread Vasiliy Tolstov
2014-12-03 16:38 GMT+03:00 Dr. David Alan Gilbert dgilb...@redhat.com:
 The entire migration stream is passed over the RDMA; however
 I'm not clear how the address lookup works, what you pass to the
 migration command is the IP associated with the interface.

 Dave


Yes i see this. So as i understand working ethernet connection between
nodes needs to be established before live migration. Thanks

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru



[Qemu-devel] [PATCH] ARM64: support access to more performance registers in AA64 mode

2014-12-03 Thread Chengyu Song
In AA64 mode, certain system registers are access through MSR/MRS 
instructions instead of MCR/MRC. This patch added more such registers:

/* ARMv8 manual, D8.4.10 */
PMINTENCLR_EL1

/* ARMv8 manual, D8.4.11 */
PMINTENSET_EL1

/* ARMv8 manual, D8.4.12 */
PMOVSCLR_EL0

/* ARMv8 manual, D8.4.14 */
PMSELR_EL0

/* ARMv8 manual, D8.4.15 */
PMSWINC_EL0

/* ARMv8 manual, D8.4.16 */
PMUSERENR_EL0

/* ARMv8 manual, D8.4.17 */
PMXEVCNTR_EL0

/* ARMv8 manual, D8.4.18 */
PMXEVTYPER_EL0

Signed-off-by: Chengyu Song cson...@gatech.edu
---
 target-arm/helper.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index b74d348..43b5b06 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -843,15 +843,28 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   .accessfn = pmreg_access,
   .writefn = pmovsr_write,
   .raw_writefn = raw_write },
+{ .name = PMOVSCLR_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 12, .opc1 = 3, .opc2 = 3,
+  .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr),
+  .accessfn = pmreg_access,
+  .writefn = pmovsr_write,
+  .raw_writefn = raw_write },
 /* Unimplemented so WI. */
 { .name = PMSWINC, .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4,
   .access = PL0_W, .accessfn = pmreg_access, .type = ARM_CP_NOP },
+{ .name = PMSWINC_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 12, .opc1 = 3, .opc2 = 4,
+  .access = PL0_W, .accessfn = pmreg_access, .type = ARM_CP_NOP },
 /* Since we don't implement any events, writing to PMSELR is UNPREDICTABLE.
  * We choose to RAZ/WI.
  */
 { .name = PMSELR, .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
   .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
   .accessfn = pmreg_access },
+{ .name = PMSELR_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 12, .opc1 = 3, .opc2 = 5,
+  .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
+  .accessfn = pmreg_access },
 #ifndef CONFIG_USER_ONLY
 { .name = PMCCNTR, .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
   .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
@@ -875,24 +888,51 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
   .accessfn = pmreg_access, .writefn = pmxevtyper_write,
   .raw_writefn = raw_write },
+{ .name = PMXEVTYPER_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 13, .opc1 = 3, .opc2 = 1,
+  .access = PL0_RW,
+  .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
+  .accessfn = pmreg_access, .writefn = pmxevtyper_write,
+  .raw_writefn = raw_write },
 /* Unimplemented, RAZ/WI. */
 { .name = PMXEVCNTR, .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
   .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
   .accessfn = pmreg_access },
+{ .name = PMXEVCNTR_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 13, .opc1 = 3, .opc2 = 2,
+  .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
+  .accessfn = pmreg_access },
 { .name = PMUSERENR, .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0,
   .access = PL0_R | PL1_RW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr),
   .resetvalue = 0,
   .writefn = pmuserenr_write, .raw_writefn = raw_write },
+{ .name = PMUSERENR_EL0, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 14, .opc1 = 3, .opc2 = 0,
+  .access = PL0_R | PL1_RW,
+  .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr),
+  .resetvalue = 0,
+  .writefn = pmuserenr_write, .raw_writefn = raw_write },
 { .name = PMINTENSET, .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 
1,
   .access = PL1_RW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
   .resetvalue = 0,
   .writefn = pmintenset_write, .raw_writefn = raw_write },
+{ .name = PMINTENSET_EL1, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 1,
+  .access = PL1_RW,
+  .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
+  .resetvalue = 0,
+  .writefn = pmintenset_write, .raw_writefn = raw_write },
 { .name = PMINTENCLR, .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 
2,
   .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
   .resetvalue = 0, .writefn = pmintenclr_write, },
+{ .name = PMINTENCLR_EL1, .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 2,
+  .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
+  .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
+  .resetvalue = 0, .writefn = pmintenclr_write, },
 { .name = VBAR, .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 

Re: [Qemu-devel] qemu rdma live migration

2014-12-03 Thread Dr. David Alan Gilbert
* Vasiliy Tolstov (v.tols...@selfip.ru) wrote:
 2014-12-03 16:38 GMT+03:00 Dr. David Alan Gilbert dgilb...@redhat.com:
  The entire migration stream is passed over the RDMA; however
  I'm not clear how the address lookup works, what you pass to the
  migration command is the IP associated with the interface.
 
  Dave
 
 
 Yes i see this. So as i understand working ethernet connection between
 nodes needs to be established before live migration. Thanks

I don't think it needs ethernet; it needs an IP address but that
can be IPoIB.

Dave

 
 -- 
 Vasiliy Tolstov,
 e-mail: v.tols...@selfip.ru
 jabber: v...@selfip.ru
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PATCH v2 0/2] vmdk: Fix error for JSON descriptor file names

2014-12-03 Thread Max Reitz
This series improves the error message emitted when a vmdk file has a
JSON file name and the vmdk driver tries to read the extent files (which
will not work), and adds an appropriate test case.


v2:
- Patch 1: Only error out if the extent file name is not absolute
  [Kevin]
- Patch 2: Because I changed the error message in patch 1, the test
  output needs to be amended accordingly


git-backport-diff against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/2:[0018] [FC] 'vmdk: Fix error for JSON descriptor file names'
002/2:[0002] [FC] 'iotests: Add test for vmdk JSON file names'


Max Reitz (2):
  vmdk: Fix error for JSON descriptor file names
  iotests: Add test for vmdk JSON file names

 block.c|  2 +-
 block/vmdk.c   | 10 +-
 include/block/block.h  |  1 +
 tests/qemu-iotests/059 |  6 ++
 tests/qemu-iotests/059.out |  4 
 5 files changed, 21 insertions(+), 2 deletions(-)

-- 
1.9.3




  1   2   3   >