Re: [Qemu-devel] [PATCH v2 12/13] intel_iommu: ioapic: IR support for emulated IOAPIC

[Peter Xu]

On Tue, Apr 12, 2016 at 08:39:21PM -0700, Jan Kiszka wrote:
> On 2016-04-12 20:33, Peter Xu wrote:
> > On Tue, Apr 12, 2016 at 08:39:02AM -0700, Jan Kiszka wrote:
> >> On 2016-04-12 02:02, Peter Xu wrote:
> > 
> > [...]
> > 
> >>> Yes, I should consider other x86 platforms like AMD. Thanks to point
> >>> out. It seems that there are many places in the patchset that lacks
> >>> thorough consideration about this. Will try to fix them in next
> >>> version.
> >>>
> >>> Regarding to the above MSI solution: I'd say it is a good way to
> >>> hide everything else behind.  However, since we introduced one extra
> >>> layer (MSI) which actually does not exist, not sure there would be
> >>> problem too.  Also, I feel it a little bit hacky if we "create" one
> >>> MSI out of the air...  For example, if someone tries to capture MSIs
> >>> from QEMU inside in the APIC memory writes, he will see something he
> >>> cannot explain if he never knows this hack's there.  Considering the
> >>> above, I would prefer hooks, or better to provide a callback (a
> >>> function pointer that others like AMD can override) to do the
> >>> translation.  How do you think?
> >>
> >> The HPET does send MSIs, and I'm not sure how much different the
> >> IOAPIC's message actually is. In any case, modelling it as MSI is
> >> neither adding incorrectness nor making the code more complex (in fact,
> >> the contrary is true!). Last but not least, it would be trivial to
> >> filter out non-PCI MSI sources if we wanted to trace only PCI - because
> >> we need to identify the origin anyway for remapping purposes. So,
> >> explicit hooking looks like the wrong way to me.
> > 
> > I am just not sure about the difference between IOAPIC's messages
> > and MSI ones. For now, they seems very alike. However, I am not sure
> > whether it would be not alike in the future. E.g., if one day, we
> > extend APIC bus to support more than 255 CPUs (could it? I do not
> > know for sure), here if we are with this "MSI layer", we would not
> > be able to do that, since MSI only support 8 bits for destination ID
> > field. That's my only worry now. If you (or Radim? or anyone more
> > experienced on this than me) can confirm that this would never be a
> > problem, I'd be glad to take the MSI way.
> 
> That's one of the reason why we need IR: >255 is only possible this way,
> because it requires x2APIC and that requires IR (see Intel spec). So,
> IOAPIC messages will then always travel via VT-d. No need to worry at all.

Ah, right. When we deliver the MSI, it's in remappable format, so
there is no destination ID at all... Okay, I can take this in
v3. Thanks.

-- peterx

Re: [Qemu-devel] [PATCH] docs/atomics.txt: Update pointer to linux macro

[Pranith Kumar]

On Tue, Apr 12, 2016 at 5:20 PM, Paolo Bonzini  wrote:

> FWIW I'll be mostly offline this week and on vacation starting from the
> next, so it's probably best if you send the patch at the beginning of
> May.  It will be fixed _and_ act as a reminder. :)

Sure, I will do so in May.

-- 
Pranith

[Qemu-devel] [PATCH] qemu-iotests: 041: More robust assertion on quorum node

[Fam Zheng]

Block nodes are now assigned names automatically, therefore the test
case is fragile in using fixed indices in result. Introduce a method in
iotests.py and do the matching more sensibly.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/041| 12 
 tests/qemu-iotests/iotests.py | 14 ++
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index c7da95d..b1c542f 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -810,8 +810,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
 self.assert_qmp(result, 'return', {})
 
 self.complete_and_wait(drive="quorum0")
-result = self.vm.qmp('query-named-block-nodes')
-self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+self.assert_has_block_node("repair0", quorum_repair_img)
 # TODO: a better test requiring some QEMU infrastructure will be added
 #   to check that this file is really driven by quorum
 self.vm.shutdown()
@@ -833,8 +832,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
 self.cancel_and_wait(drive="quorum0", force=True)
 # here we check that the last registered quorum file has not been
 # swapped out and unref
-result = self.vm.qmp('query-named-block-nodes')
-self.assert_qmp(result, 'return[1]/file', quorum_img3)
+self.assert_has_block_node(None, quorum_img3)
 self.vm.shutdown()
 
 def test_cancel_after_ready(self):
@@ -850,10 +848,9 @@ class TestRepairQuorum(iotests.QMPTestCase):
 self.assert_qmp(result, 'return', {})
 
 self.wait_ready_and_cancel(drive="quorum0")
-result = self.vm.qmp('query-named-block-nodes')
 # here we check that the last registered quorum file has not been
 # swapped out and unref
-self.assert_qmp(result, 'return[1]/file', quorum_img3)
+self.assert_has_block_node(None, quorum_img3)
 self.vm.shutdown()
 self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
 'target image does not match source after mirroring')
@@ -974,8 +971,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
 self.assert_qmp(result, 'return', {})
 
 self.complete_and_wait(drive="quorum0")
-result = self.vm.qmp('query-named-block-nodes')
-self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+self.assert_has_block_node("repair0", quorum_repair_img)
 # TODO: a better test requiring some QEMU infrastructure will be added
 #   to check that this file is really driven by quorum
 self.vm.shutdown()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0c0b533..d9ef60e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -348,6 +348,20 @@ class QMPTestCase(unittest.TestCase):
 result = self.vm.qmp('query-block-jobs')
 self.assert_qmp(result, 'return', [])
 
+def assert_has_block_node(self, node_name=None, file_name=None):
+"""Issue a query-named-block-nodes and assert node_name and/or
+file_name is present in the result"""
+def check_equal_or_none(a, b):
+return a == None or b == None or a == b
+assert node_name or file_name
+result = self.vm.qmp('query-named-block-nodes')
+for x in result["return"]:
+if check_equal_or_none(x.get("node-name"), node_name) and \
+check_equal_or_none(x.get("file"), file_name):
+return
+self.assertTrue(False, "Cannot find %s %s in result:\n%s" % \
+(node_name, file_name, result))
+
 def cancel_and_wait(self, drive='drive0', force=False, resume=False):
 '''Cancel a block job and wait for it to finish, returning the event'''
 result = self.vm.qmp('block-job-cancel', device=drive, force=force)
-- 
2.8.0

Re: [Qemu-devel] [PATCH v2 12/13] intel_iommu: ioapic: IR support for emulated IOAPIC

[Jan Kiszka]

On 2016-04-12 20:33, Peter Xu wrote:
> On Tue, Apr 12, 2016 at 08:39:02AM -0700, Jan Kiszka wrote:
>> On 2016-04-12 02:02, Peter Xu wrote:
> 
> [...]
> 
>>> Yes, I should consider other x86 platforms like AMD. Thanks to point
>>> out. It seems that there are many places in the patchset that lacks
>>> thorough consideration about this. Will try to fix them in next
>>> version.
>>>
>>> Regarding to the above MSI solution: I'd say it is a good way to
>>> hide everything else behind.  However, since we introduced one extra
>>> layer (MSI) which actually does not exist, not sure there would be
>>> problem too.  Also, I feel it a little bit hacky if we "create" one
>>> MSI out of the air...  For example, if someone tries to capture MSIs
>>> from QEMU inside in the APIC memory writes, he will see something he
>>> cannot explain if he never knows this hack's there.  Considering the
>>> above, I would prefer hooks, or better to provide a callback (a
>>> function pointer that others like AMD can override) to do the
>>> translation.  How do you think?
>>
>> The HPET does send MSIs, and I'm not sure how much different the
>> IOAPIC's message actually is. In any case, modelling it as MSI is
>> neither adding incorrectness nor making the code more complex (in fact,
>> the contrary is true!). Last but not least, it would be trivial to
>> filter out non-PCI MSI sources if we wanted to trace only PCI - because
>> we need to identify the origin anyway for remapping purposes. So,
>> explicit hooking looks like the wrong way to me.
> 
> I am just not sure about the difference between IOAPIC's messages
> and MSI ones. For now, they seems very alike. However, I am not sure
> whether it would be not alike in the future. E.g., if one day, we
> extend APIC bus to support more than 255 CPUs (could it? I do not
> know for sure), here if we are with this "MSI layer", we would not
> be able to do that, since MSI only support 8 bits for destination ID
> field. That's my only worry now. If you (or Radim? or anyone more
> experienced on this than me) can confirm that this would never be a
> problem, I'd be glad to take the MSI way.

That's one of the reason why we need IR: >255 is only possible this way,
because it requires x2APIC and that requires IR (see Intel spec). So,
IOAPIC messages will then always travel via VT-d. No need to worry at all.

Jan

Re: [Qemu-devel] [PATCH v2 12/13] intel_iommu: ioapic: IR support for emulated IOAPIC

[Peter Xu]

On Tue, Apr 12, 2016 at 08:39:02AM -0700, Jan Kiszka wrote:
> On 2016-04-12 02:02, Peter Xu wrote:

[...]

> > Yes, I should consider other x86 platforms like AMD. Thanks to point
> > out. It seems that there are many places in the patchset that lacks
> > thorough consideration about this. Will try to fix them in next
> > version.
> > 
> > Regarding to the above MSI solution: I'd say it is a good way to
> > hide everything else behind.  However, since we introduced one extra
> > layer (MSI) which actually does not exist, not sure there would be
> > problem too.  Also, I feel it a little bit hacky if we "create" one
> > MSI out of the air...  For example, if someone tries to capture MSIs
> > from QEMU inside in the APIC memory writes, he will see something he
> > cannot explain if he never knows this hack's there.  Considering the
> > above, I would prefer hooks, or better to provide a callback (a
> > function pointer that others like AMD can override) to do the
> > translation.  How do you think?
> 
> The HPET does send MSIs, and I'm not sure how much different the
> IOAPIC's message actually is. In any case, modelling it as MSI is
> neither adding incorrectness nor making the code more complex (in fact,
> the contrary is true!). Last but not least, it would be trivial to
> filter out non-PCI MSI sources if we wanted to trace only PCI - because
> we need to identify the origin anyway for remapping purposes. So,
> explicit hooking looks like the wrong way to me.

I am just not sure about the difference between IOAPIC's messages
and MSI ones. For now, they seems very alike. However, I am not sure
whether it would be not alike in the future. E.g., if one day, we
extend APIC bus to support more than 255 CPUs (could it? I do not
know for sure), here if we are with this "MSI layer", we would not
be able to do that, since MSI only support 8 bits for destination ID
field. That's my only worry now. If you (or Radim? or anyone more
experienced on this than me) can confirm that this would never be a
problem, I'd be glad to take the MSI way.

Thanks.

-- peterx

Re: [Qemu-devel] [PATCH 18/18] test: add shutdown support vubr test

[Yuanhan Liu]

On Fri, Apr 01, 2016 at 01:16:28PM +0200, marcandre.lur...@redhat.com wrote:
> +static void
> +vubr_handle_slave_reply(VhostUserMsg *vmsg)
> +{
> +DPRINT(
> +"==   Vhost slave reply from QEMU   
> ==\n");
> +DPRINT("Request: %s (%d)\n", vubr_slave_request_str[vmsg->request],
> +   vmsg->request);
> +DPRINT("Flags:   0x%x\n", vmsg->flags);
> +DPRINT("Size:%d\n", vmsg->size);
> +
> +switch (vmsg->request) {
> +case VHOST_USER_SLAVE_SHUTDOWN:
> +DPRINT("Shutdown success: 0x%016"PRIx64"\n", vmsg->payload.u64);
> +if (vmsg->payload.u64 == 0) {
> +exit(0);
> +}
> +default:
> +DPRINT("Invalid slave reply");
> +};
   ^^

Minor nit: redundant ';'.

--yliu

Re: [Qemu-devel] [PATCH 11/18] vhost-user: add shutdown support

[Yuanhan Liu]

Hi Marc,

First of all, sorry again for late response!

Last time I tried with your first version, I found few issues related
with reconnect, mainly on the acked_feautres lost. While checking your
new code, I found that you've already solved that, which is great.

So, I tried harder this time, your patches work great, except that I
found few nits.

On Fri, Apr 01, 2016 at 01:16:21PM +0200, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
...
> +Slave message types
> +---
> +
> + * VHOST_USER_SLAVE_SHUTDOWN:
> +  Id: 1
> +  Master payload: N/A
> +  Slave payload: u64
> +
> +  Request the master to shutdown the slave. A 0 reply is for
> +  success, in which case the slave may close all connections
> +  immediately and quit.

Assume we are using ovs + dpdk here, that we could have two
vhost-user connections. While ovs tries to initiate a restart,
it might unregister the two connections one by one. In such
case, two VHOST_USER_SLAVE_SHUTDOWN request will be sent,
and two replies will get. Therefore, I don't think it's a
proper ask here to let the backend implementation to do quit
here.


>  
>  switch (msg.request) {
> +case VHOST_USER_SLAVE_SHUTDOWN: {
> +uint64_t success = 1; /* 0 is for success */
> +if (dev->stop) {
> +dev->stop(dev);
> +success = 0;
> +}
> +msg.payload.u64 = success;
> +msg.size = sizeof(msg.payload.u64);
> +size = send(u->slave_fd, , VHOST_USER_HDR_SIZE + msg.size, 0);
> +if (size != VHOST_USER_HDR_SIZE + msg.size) {
> +error_report("Failed to write reply.");
> +}
> +break;

You might want to remove the slave_fd from watch list? We
might also need to close slave_fd here, assuming that we
will no longer use it when VHOST_USER_SLAVE_SHUTDOWN is
received?

I'm asking because I found a seg fault issue sometimes,
due to opaque is NULL.


--yliu

Re: [Qemu-devel] post-copy is broken?

[Li, Liang Z]

> > > I used the latest qemu code (commit id: 4e71220387e88a22) and kernel
> > (v4.5) to test the post-copy, and find the guest get crashed after
> > live migration, no matter I did a local live migration or live
> > migration between two hosts. I just ran the stress as the workload in
> > guest. It seems the post-copy is broken?
> > >
> > > Stress parameters:  stress --vm 2 --vm-hang 1 --vm-bytes 2048M
> > > --vm-keep QEMU parameters: ./qemu-system-x86_64 --enable-kvm -
> smp
> > 4 -m
> > > 8192  -monitor stdio -drive file=/share/centos6u6.qcow
> >
> > My test seems to be working here (4.4.6-301.fc23 kernel) same qemu
> > version.
> > This is with an f20 guest running google stressapptest.
> >
> > What's your last working version?
> >
> 
> This is my first try of post-copy after the related patches been merged.
> I will double check and get back to you.
> 
> Thanks!
> Liang
> 
> > Dave

I tried the v4.4 upstream kernel, the issue was disappeared. It must be some 
changes between kernel v4.4 and v4.5
breaks post-copy.  

Liang.

> >
> > > Liang
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH V2 1/3] colo-compare: introduce colo compare initlization

[Zhang Chen]

+if (!size) {
+return 0;
+}
+
+ret = qemu_chr_fe_write_all(out, (uint8_t *), sizeof(len));
+if (ret != sizeof(len)) {
+goto err;
+}
+
+ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
+if (ret != size) {
+goto err;
+}
+

You can make this slightly simpler and save the return 0;


If we want to save the return 0 , the code will be changed like that:

err:
return (ret < 0 || ret == size) ? ret : -EIO;

I think it too complex to be understood, so should we keep the original ?


+return 0;
+
+err:
+return ret < 0 ? ret : -EIO;

err:
return ret <= 0 ? ret : -EIO;


This is wrong, if qemu_chr_fe_write_all success, ret will equal size.
return -EIO.


+}
+
+static int compare_chr_can_read(void *opaque)
+{
+return COMPARE_READ_LEN_MAX;
+}



--
Thanks
zhangchen

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Ryan Harper]

Here's an update.

The Xenial kernel doesn;t like the emulated POWER7 cpu that the command
line being used generates by default.

processor   : 0
cpu : POWER7 (raw), altivec supported
clock   : 1000.00MHz
revision: 2.3 (pvr 003f 0203)

timebase: 51200
platform: pSeries
model   : IBM pSeries (emulated by qemu)
machine : CHRP IBM pSeries (emulated by qemu)

We can boot a Wily image (kernel 4.2.0-35) just fine with the POWER7
cpu.


When booting Xenial's kernel with POWER7 cpu, it produces a stacktrace during 
module load:

[9.885165] Loaded X.509 cert 'Build time autogenerated kernel key: 
6687eed33bf99302166296c3e5cafe31ef38ad41'
[9.886507] zswap: loaded using pool lzo/zbud
[9.916000] modprobe[74]: unhandled signal 4 at 3fffb5a4d03c nip 
3fffb5a4d03c lr 3fffb5a25e24 code 30001
[9.925819] modprobe[76]: unhandled signal 4 at 3fff85b9d03c nip 
3fff85b9d03c lr 3fff85b75e24 code 30001
[9.928401] Key type trusted registered
[9.930762] modprobe[79]: unhandled signal 4 at 3fff7d05d03c nip 
3fff7d05d03c lr 3fff7d035e24 code 30001
[9.933360] modprobe[80]: unhandled signal 4 at 3fff8820d03c nip 
3fff8820d03c lr 3fff881e5e24 code 30001
[9.936240] modprobe[83]: unhandled signal 4 at 3fffb4fbd03c nip 
3fffb4fbd03c lr 3fffb4f95e24 code 30001
[9.938873] modprobe[84]: unhandled signal 4 at 3fff92d4d03c nip 
3fff92d4d03c lr 3fff92d25e24 code 30001
[9.940335] Key type encrypted registered
[9.940461] AppArmor: AppArmor sha1 policy hashing enabled
[9.941005] ima: No TPM chip found, activating TPM-bypass!
[9.942985] evm: HMAC attrs: 0x1
[9.947081] hctosys: unable to open rtc device (rtc0)
[9.987867] Freeing unused kernel memory: 6144K (c0ea - 
c14a)
[9.991123] init[1]: unhandled signal 4 at 3fff8edfd03c nip 
3fff8edfd03c lr 3fff8edd5e24 code 30001
[9.994581] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0004
[9.994581] 
[9.994889] CPU: 0 PID: 1 Comm: init Not tainted 4.4.0-18-generic #34-Ubuntu
[9.995054] Call Trace:
[9.995216] [c0001e4c3a50] [c0aed6fc] dump_stack+0xb0/0xf0 
(unreliable)
[9.995336] [c0001e4c3a90] [c0ae9930] panic+0x100/0x2c0
[9.995398] [c0001e4c3b20] [c00bd554] do_exit+0xc24/0xc30
[9.995443] [c0001e4c3be0] [c00bd644] do_group_exit+0x64/0x100
[9.995490] [c0001e4c3c20] [c00ceaac] get_signal+0x55c/0x7b0
[9.995534] [c0001e4c3d10] [c0017424] do_signal+0x54/0x2b0
[9.995578] [c0001e4c3e00] [c001787c] do_notify_resume+0xbc/0xd0
[9.995677] [c0001e4c3e30] [c0009838] 
ret_from_except_lite+0x64/0x68
[   10.011069] ---[ end Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0004
[   10.011069] 


When we use -enable-kvm, this bypasses the tcg POWER7 cpu, and uses the host 
cpu type (POWER8) which is why we can boot the Xenial kernel with KVM.

We need to open a linux task to help track down that issue; also if
someone is testing Xenial on POWER7 hardware, that may help determine if
there is a lurking qemu tcg issue, though given that Wily kernels boot
fine in tcg mode; it's more likely there's something that changed/broke
in the kernels since 4.2.0-35.

I'm marking the qemu task invalid, and will open the linux task.


** Changed in: qemu (Ubuntu)
   Status: Confirmed => Invalid

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  Invalid
Status in qemu package in Ubuntu:
  Invalid

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating 

Re: [Qemu-devel] post-copy is broken?

[Li, Liang Z]

> > I used the latest qemu code (commit id: 4e71220387e88a22) and kernel
> (v4.5) to test the post-copy, and find the guest get crashed after live
> migration, no matter I did a local live migration or live migration between 
> two
> hosts. I just ran the stress as the workload in guest. It seems the post-copy 
> is
> broken?
> >
> > Stress parameters:  stress --vm 2 --vm-hang 1 --vm-bytes 2048M
> > --vm-keep QEMU parameters: ./qemu-system-x86_64 --enable-kvm -smp
> 4 -m
> > 8192  -monitor stdio -drive file=/share/centos6u6.qcow
> 
> My test seems to be working here (4.4.6-301.fc23 kernel) same qemu
> version.
> This is with an f20 guest running google stressapptest.
> 
> What's your last working version?
> 

This is my first try of post-copy after the related patches been merged.
I will double check and get back to you.

Thanks!
Liang

> Dave
> 
> > Liang
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PULL 1/4] xen: Fix IDE unplug

[John Snow]

From: Anthony PERARD 

After commit e5e7855 (blockdev: Separate BB name management), starting a
guest with PVHVM support result in this assert:
qemu-system-i386: block/block-backend.c:173: blk_delete: Assertion `!blk->name' 
failed.

A backtrace show that a caller is pci_piix3_xen_ide_unplug().

This patch fix it.

Signed-off-by: Anthony PERARD 
Message-id: 1460382666-29885-1-git-send-email-anthony.per...@citrix.com
Signed-off-by: John Snow 
---
 hw/ide/piix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 0a4cbcb..6d76ce9 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -189,6 +189,7 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
 idedev = pci_ide->bus[di->bus].slave;
 }
 idedev->conf.blk = NULL;
+monitor_remove_blk(blk);
 blk_unref(blk);
 }
 }
-- 
2.4.11

[Qemu-devel] [PULL 2/4] ide: don't lose pending dma state

[John Snow]

From: Pavel Butsykin 

If the migration occurs after the IDE DMA has been set up but before it
has been initiated, the state gets lost upon save/restore. Specifically,
->dma_cb callback gets cleared, so, when the guest eventually starts bus
mastering, the DMA never completes, causing the guest to time out the
operation.

OTOH all the infrastructure is already in place to restart the DMA if
the migration happens while the DMA is in progress.

So reuse that infrastructure, by setting bus->error_status based on
->dma_cmd in pre_save if ->dma_cb callback is already set but DMAING is
clear. This will indicate the need for restart and make sure ->dma_cb
is restored in ide_restart_bh(); howeover since DMAING is clear the state
upon restore will be exactly "ready for DMA" as before the save.

Signed-off-by: Pavel Butsykin 
Reviewed-by: Roman Kagan 
Signed-off-by: Denis V. Lunev 
Reviewed-by: John Snow 
Message-id: 1459924806-306-2-git-send-email-...@openvz.org
Signed-off-by: John Snow 
---
 hw/ide/core.c |  9 +
 hw/ide/internal.h | 15 +++
 hw/ide/pci.c  |  4 
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 90524d5..58d0687 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -804,14 +804,7 @@ static void ide_dma_cb(void *opaque, int ret)
 return;
 }
 if (ret < 0) {
-int op = IDE_RETRY_DMA;
-
-if (s->dma_cmd == IDE_DMA_READ)
-op |= IDE_RETRY_READ;
-else if (s->dma_cmd == IDE_DMA_TRIM)
-op |= IDE_RETRY_TRIM;
-
-if (ide_handle_rw_error(s, -ret, op)) {
+if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
 return;
 }
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 86bde26..68c7d0d 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -513,6 +513,21 @@ struct IDEDevice {
 #define IDE_RETRY_TRIM 0x80
 #define IDE_RETRY_HBA  0x100
 
+static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd)
+{
+switch (dma_cmd) {
+case IDE_DMA_READ:
+return IDE_RETRY_DMA | IDE_RETRY_READ;
+case IDE_DMA_WRITE:
+return IDE_RETRY_DMA;
+case IDE_DMA_TRIM:
+return IDE_RETRY_DMA | IDE_RETRY_TRIM;
+default:
+break;
+}
+return 0;
+}
+
 static inline IDEState *idebus_active_if(IDEBus *bus)
 {
 return bus->ifs + bus->unit;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 92ffee7..8d56a00 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -308,6 +308,10 @@ static void ide_bmdma_pre_save(void *opaque)
 BMDMAState *bm = opaque;
 uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
 
+if (!(bm->status & BM_STATUS_DMAING) && bm->dma_cb) {
+bm->bus->error_status =
+ide_dma_cmd_to_retry(bmdma_active_if(bm)->dma_cmd);
+}
 bm->migration_retry_unit = bm->bus->retry_unit;
 bm->migration_retry_sector_num = bm->bus->retry_sector_num;
 bm->migration_retry_nsector = bm->bus->retry_nsector;
-- 
2.4.11

[Qemu-devel] [PULL 4/4] ide: really restart pending and in-flight atapi dma

[John Snow]

From: Pavel Butsykin 

Restart of ATAPI DMA used to be unreachable, because the request to do
so wasn't indicated in bus->error_status due to the lack of spare bits, and
ide_restart_bh() would return early doing nothing.

This patch makes use of the observation that not all bit combinations were
possible in ->error_status. In particular, IDE_RETRY_READ only made sense
together with IDE_RETRY_DMA or IDE_RETRY_PIO. This allows to re-use
IDE_RETRY_READ alone as an indicator of ATAPI DMA restart request.

To makes things more uniform, ATAPI DMA gets its own value for ->dma_cmd.
As a means against confusion, macros are added to test the state of
->error_status.

The patch fixes the restart of both in-flight and pending ATAPI DMA,
following the scheme similar to that of IDE DMA.

[Including a fixup patch:
Message-id: 1460465594-15777-1-git-send-email-pbutsy...@virtuozzo.com
--js]

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
Reviewed-by: Roman Kagan 
Reviewed-by: John Snow 
Message-id: 1459924806-306-4-git-send-email-...@openvz.org
Signed-off-by: John Snow 
---
 hw/ide/atapi.c| 13 ++---
 hw/ide/core.c | 30 +++---
 hw/ide/internal.h | 20 
 hw/ide/macio.c|  2 ++
 4 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index acc52cd..2bb606c 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -375,15 +375,18 @@ static void ide_atapi_cmd_check_status(IDEState *s)
 }
 /* ATAPI DMA support */
 
-/* XXX: handle read errors */
 static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 int data_offset, n;
 
 if (ret < 0) {
-ide_atapi_io_error(s, ret);
-goto eot;
+if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
+if (s->bus->error_status) {
+return;
+}
+goto eot;
+}
 }
 
 if (s->io_buffer_size > 0) {
@@ -481,10 +484,6 @@ static void ide_atapi_cmd_read(IDEState *s, int lba, int 
nb_sectors,
 }
 }
 
-
-/* Called by *_restart_bh when the transfer function points
- * to ide_atapi_cmd
- */
 void ide_atapi_dma_restart(IDEState *s)
 {
 /*
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 58d0687..41e6a2d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -57,7 +57,6 @@ static const int smart_attributes[][12] = {
 { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
 };
 
-static int ide_handle_rw_error(IDEState *s, int error, int op);
 static void ide_dummy_transfer_stop(IDEState *s);
 
 static void padstr(char *str, const char *src, int len)
@@ -773,7 +772,7 @@ void ide_dma_error(IDEState *s)
 ide_set_irq(s->bus);
 }
 
-static int ide_handle_rw_error(IDEState *s, int error, int op)
+int ide_handle_rw_error(IDEState *s, int error, int op)
 {
 bool is_read = (op & IDE_RETRY_READ) != 0;
 BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);
@@ -783,8 +782,10 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 s->bus->error_status = op;
 } else if (action == BLOCK_ERROR_ACTION_REPORT) {
 block_acct_failed(blk_get_stats(s->blk), >acct);
-if (op & IDE_RETRY_DMA) {
+if (IS_IDE_RETRY_DMA(op)) {
 ide_dma_error(s);
+} else if (IS_IDE_RETRY_ATAPI(op)) {
+ide_atapi_io_error(s, -error);
 } else {
 ide_rw_error(s);
 }
@@ -872,6 +873,8 @@ static void ide_dma_cb(void *opaque, int ret)
 ide_issue_trim, ide_dma_cb, s,
 DMA_DIRECTION_TO_DEVICE);
 break;
+default:
+abort();
 }
 return;
 
@@ -1634,6 +1637,9 @@ static bool cmd_packet(IDEState *s, uint8_t cmd)
 
 s->status = READY_STAT | SEEK_STAT;
 s->atapi_dma = s->feature & 1;
+if (s->atapi_dma) {
+s->dma_cmd = IDE_DMA_ATAPI;
+}
 s->nsector = 1;
 ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,
ide_atapi_cmd);
@@ -2518,15 +2524,13 @@ static void ide_restart_bh(void *opaque)
 if (s->bus->dma->ops->restart) {
 s->bus->dma->ops->restart(s->bus->dma);
 }
-}
-
-if (error_status & IDE_RETRY_DMA) {
+} else if (IS_IDE_RETRY_DMA(error_status)) {
 if (error_status & IDE_RETRY_TRIM) {
 ide_restart_dma(s, IDE_DMA_TRIM);
 } else {
 ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
 }
-} else if (error_status & IDE_RETRY_PIO) {
+} else if (IS_IDE_RETRY_PIO(error_status)) {
 if (is_read) {
 ide_sector_read(s);
 } else {
@@ -2534,15 +2538,11 @@ static void ide_restart_bh(void *opaque)
 }
 } else if (error_status & IDE_RETRY_FLUSH) {
 

[Qemu-devel] [PULL 0/4] Ide patches

[John Snow]

The following changes since commit d44122ecd0fa62d20762bdd8f214f077cb8e011b:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2016-04-12 17:47:15 +0100)

are available in the git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to 502356eeeb5fd2bdd92b2d5156e511626c1c3814:

  ide: really restart pending and in-flight atapi dma (2016-04-12 18:48:15 
-0400)





Anthony PERARD (1):
  xen: Fix IDE unplug

Pavel Butsykin (3):
  ide: don't lose pending dma state
  ide: restart atapi dma by re-evaluating command packet
  ide: really restart pending and in-flight atapi dma

 hw/ide/atapi.c| 26 --
 hw/ide/core.c | 39 ---
 hw/ide/internal.h | 35 +++
 hw/ide/macio.c|  2 ++
 hw/ide/pci.c  |  4 
 hw/ide/piix.c |  1 +
 6 files changed, 70 insertions(+), 37 deletions(-)

-- 
2.4.11

[Qemu-devel] [PULL 3/4] ide: restart atapi dma by re-evaluating command packet

[John Snow]

From: Pavel Butsykin 

ide_atapi_dma_restart() used to just complete the DMA with an error,
under the assumption that there isn't enough information to restart it.

However, as the contents of the ->io_buffer is preserved, it looks safe to
just re-evaluate it and dispatch the ATAPI command again.

Signed-off-by: Pavel Butsykin 
Reviewed-by: Roman Kagan 
Signed-off-by: Denis V. Lunev 
Reviewed-by: John Snow 
Message-id: 1459924806-306-3-git-send-email-...@openvz.org
Signed-off-by: John Snow 
---
 hw/ide/atapi.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 1fe58ab..acc52cd 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -488,14 +488,13 @@ static void ide_atapi_cmd_read(IDEState *s, int lba, int 
nb_sectors,
 void ide_atapi_dma_restart(IDEState *s)
 {
 /*
- * I'm not sure we have enough stored to restart the command
- * safely, so give the guest an error it should recover from.
- * I'm assuming most guests will try to recover from something
- * listed as a medium error on a CD; it seems to work on Linux.
- * This would be more of a problem if we did any other type of
- * DMA operation.
+ * At this point we can just re-evaluate the packet command and start over.
+ * The presence of ->dma_cb callback in the pre_save ensures that the 
packet
+ * command has been completely sent and we can safely restart command.
  */
-ide_atapi_cmd_error(s, MEDIUM_ERROR, ASC_NO_SEEK_COMPLETE);
+s->unit = s->bus->retry_unit;
+s->bus->dma->ops->restart_dma(s->bus->dma);
+ide_atapi_cmd(s);
 }
 
 static inline uint8_t ide_atapi_set_profile(uint8_t *buf, uint8_t *index,
-- 
2.4.11

Re: [Qemu-devel] [Bug 1450881] Re: qemu-system-sparc MUTEX_HELD assert and libC lock errors

[jdery]

re:
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 58572c3..7998ff5 100644
...
-tcg_gen_qemu_ld8s(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx);

I confirmed this patch resolves MUTEX_HELD errors on a minimal testcase
solaris 6 image.
Thank you!

On Mon, Apr 11, 2016 at 7:03 AM, Mark Cave-Ayland <
mark.cave-ayl...@ilande.co.uk> wrote:

> Proposed patch posted to mailing list:
> https://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg01645.html -
> please test and report back.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1450881
>
> Title:
>   qemu-system-sparc MUTEX_HELD assert and libC lock errors
>
> Status in QEMU:
>   New
>
> Bug description:
>   Here I am cross-posting a comment I made on Artyom's blog.  Atar
>   responded that he "fixed these issues for some customers".  I hoped
>   that opening a bug to the opensource project might help develop the
>   solution for the public domain.
>
>   I now have a mostly-working Solaris 6 emulation, with great thanks to
>   the valuable information in Artyom's blog, brezular.com, and the
>   QEMU/Solaris 4.14 wikibook.
>
>   setup detail;
>   QEMU (present git snapshot, reports --version 2.2.92)
>   -M SS-20, openboot/proprietary prom
>
>   # uname -a
>   SunOS emu0 5.6 Generic_105181-33 sun4m sparc SUNW,SPARCstation-20
>
>   I continue to have a problem, which I have found others posted in blog
>   comments, but have not seen a resolution yet.
>
>   # /etc/init.d/init.dmi start
>   Run-time error, libC:
>   Trying to release a lock that was not acquired in this thread
>   (repeat above 1x)
>   Abort - core dumped
>
>   as well as:
>   Assertion failed: MUTEX_HELD(_mutex), file rpc/svc_run.c, line 766
>
>   which prints to the console periodically when "dmispd" is running.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1450881/+subscriptions
>

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

Title:
  qemu-system-sparc MUTEX_HELD assert and libC lock errors

Status in QEMU:
  New

Bug description:
  Here I am cross-posting a comment I made on Artyom's blog.  Atar
  responded that he "fixed these issues for some customers".  I hoped
  that opening a bug to the opensource project might help develop the
  solution for the public domain.

  I now have a mostly-working Solaris 6 emulation, with great thanks to
  the valuable information in Artyom's blog, brezular.com, and the
  QEMU/Solaris 4.14 wikibook.

  setup detail;
  QEMU (present git snapshot, reports --version 2.2.92)
  -M SS-20, openboot/proprietary prom

  # uname -a
  SunOS emu0 5.6 Generic_105181-33 sun4m sparc SUNW,SPARCstation-20

  I continue to have a problem, which I have found others posted in blog
  comments, but have not seen a resolution yet.

  # /etc/init.d/init.dmi start
  Run-time error, libC:
  Trying to release a lock that was not acquired in this thread
  (repeat above 1x)
  Abort - core dumped

  as well as:
  Assertion failed: MUTEX_HELD(_mutex), file rpc/svc_run.c, line 766

  which prints to the console periodically when "dmispd" is running.

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

Re: [Qemu-devel] [PATCH v2] exec.c: Ensure right alignment also for file backed ram

[Paolo Bonzini]

On 07/04/2016 23:31, Dominik Dingel wrote:
> diff --git a/exec.c b/exec.c
> index c4f9036..1ae98e4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1241,6 +1241,7 @@ static void *file_ram_alloc(RAMBlock *block,
>  void *area;
>  int fd = -1;
>  int64_t page_size;
> +int64_t alignment;
>  
>  if (kvm_enabled() && !kvm_has_sync_mmu()) {
>  error_setg(errp,
> @@ -1296,7 +1297,8 @@ static void *file_ram_alloc(RAMBlock *block,
>  }
>  
>  page_size = qemu_fd_getpagesize(fd);
> -block->mr->align = page_size;
> +alignment = MAX(page_size, QEMU_VMALLOC_ALIGN);
> +block->mr->align = alignment;
>  
>  if (memory < page_size) {
>  error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
> @@ -1305,7 +1307,7 @@ static void *file_ram_alloc(RAMBlock *block,
>  goto error;
>  }
>  
> -memory = ROUND_UP(memory, page_size);
> +memory = ROUND_UP(memory, alignment);

I think this change is not necessary either; it is enough to change the
qemu_ram_mmap below.

Paolo

>  /*
>   * ftruncate is not supported by hugetlbfs in older
> @@ -1317,7 +1319,7 @@ static void *file_ram_alloc(RAMBlock *block,
>  perror("ftruncate");
>  }
>  
> -area = qemu_ram_mmap(fd, memory, page_size, block->flags & RAM_SHARED);
> +area = qemu_ram_mmap(fd, memory, alignment, block->flags & RAM_SHARED);
>  if (area == MAP_FAILED) {
>  error_setg_errno(errp, errno,
>   "unable to map backing store for guest RAM");
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 408783f..a472372 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -247,6 +247,18 @@ void qemu_anon_ram_free(void *ptr, size_t size);
>  
>  #endif
>  
> +#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
> +   /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
> +  Valgrind does not support alignments larger than 1 MiB,
> +  therefore we need special code which handles running on Valgrind. */
> +#  define QEMU_VMALLOC_ALIGN (512 * 4096)
> +#elif defined(__linux__) && defined(__s390x__)
> +   /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
> +#  define QEMU_VMALLOC_ALIGN (256 * 4096)
> +#else
> +#  define QEMU_VMALLOC_ALIGN getpagesize()
> +#endif
> +
>  int qemu_madvise(void *addr, size_t len, int advice);
>  
>  int qemu_open(const char *name, int flags, ...);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 20ca141..4adde93 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -26,18 +26,6 @@
>   * THE SOFTWARE.
>   */
>  
> -#if defined(__linux__) && (defined(__x86_64__) || defined(__arm__))
> -   /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
> -  Valgrind does not support alignments larger than 1 MiB,
> -  therefore we need special code which handles running on Valgrind. */
> -#  define QEMU_VMALLOC_ALIGN (512 * 4096)
> -#elif defined(__linux__) && defined(__s390x__)
> -   /* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
> -#  define QEMU_VMALLOC_ALIGN (256 * 4096)
> -#else
> -#  define QEMU_VMALLOC_ALIGN getpagesize()
> -#endif
> -
>  #include "qemu/osdep.h"
>  #include 
>  #include 
> 

Re: [Qemu-devel] work needed on QEMU 2.6 ChangeLog

[Paolo Bonzini]

On 12/04/2016 21:56, Andrew Baumann wrote:
> * New partial Raspberry Pi 2 emulation with "raspi2" machine type.
> For now, it can boot older releases of Windows and Raspbian, but
> lacks a number of devices including USB.
> 
> Basically, it has already bit-rotted with respect to the most recent
> Windows and Raspbian images, and I haven't had time to debug them.
> Ubuntu also has problems with the missing power management device.
> Windows build 10556 and Raspbian 2015-09-24 are known to work. You
> can decide how much of that to include in the changelog :)

I've added the exact sentence you wrote above. :)

Paolo

Re: [Qemu-devel] Fwd: [PATCH v2 2/5] Change return type of functions that are named *_exit or *_exitfn in hw/char from int to void

[Paolo Bonzini]

On 12/04/2016 20:17, Nutan Shinde wrote:
> 
> -static int console_exit(SCLPEvent *event)
> +static console_exit(SCLPEvent *event)
>  {
> -return 0;

This patch doesn't even compile:

  CChw/char/sclpconsole.o
/home/pbonzini/work/upstream/qemu/hw/char/sclpconsole.c:245:8: error: return 
type defaults to ‘int’ [-Werror=implicit-int]
 static console_exit(SCLPEvent *event)
^
cc1: all warnings being treated as errors


Paolo

Re: [Qemu-devel] [PATCH] docs/atomics.txt: Update pointer to linux macro

[Paolo Bonzini]

On 12/04/2016 18:08, Pranith Kumar wrote:
> On Tue, Apr 12, 2016 at 7:42 AM, Marc-André Lureau
>  wrote:
>> Hi
>>
>> On Mon, Apr 11, 2016 at 7:30 PM, Pranith Kumar  wrote:
>>> Add a missing end brace and update doc to point to the latest access
>>> macro. ACCESS_ONE() is deprecated.
>>
>> ONE/ONCE
> 
> Right, I missed this one. Should I fix and send a new patch?

FWIW I'll be mostly offline this week and on vacation starting from the
next, so it's probably best if you send the patch at the beginning of
May.  It will be fixed _and_ act as a reminder. :)

Paolo

Re: [Qemu-devel] [PATCH] configure: Check if struct fsxattr is available from linux header

[Jan Vesely]

On Sat, 2016-03-19 at 21:18 -0400, Jan Vesely wrote:
> Fixes build with new linux headers.
> 
> Signed-off-by: Jan Vesely 
> ---
> linux-headers-4.5 and xfsprogs-3.2.4 in my case
> 
>  configure | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/configure b/configure
> index b88d0db..bb64d6c 100755
> --- a/configure
> +++ b/configure
> @@ -4474,6 +4474,21 @@ if test "$fortify_source" != "no"; then
>    fi
>  fi
>  
> +
> +# check if struct fsxattr is available
> +
> +have_fsxattr=no
> +cat > $TMPC << EOF
> +#include 
> +struct fsxattr foo;
> +int main(void) {
> +  return 0;
> +}
> +EOF
> +if compile_prog "" "" ; then
> +have_fsxattr=yes
> +fi
> +
>  ##
>  # End of CC checks
>  # After here, no more $cc or $ld runs
> @@ -5137,6 +5152,9 @@ fi
>  if test "$have_ifaddrs_h" = "yes" ; then
>  echo "HAVE_IFADDRS_H=y" >> $config_host_mak
>  fi
> +if test "$have_fsxattr" = "yes" ; then
> +echo "HAVE_FSXATTR=y" >> $config_host_mak
> +fi
>  if test "$vte" = "yes" ; then
>    echo "CONFIG_VTE=y" >> $config_host_mak
>    echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak

gentle ping.
the docs say trivial patches are collected every week. yet the last one
was in Feb.



signature.asc
Description: This is a digitally signed message part

[Qemu-devel] [PATCH] qmp-commands.hx: document minimum speed for block jobs

[Sascha Silbe]

The current rate limit implementation for block jobs is ineffective
below a certain minimum rate. It will permit writes at least once per
time slice. The resulting minimum write speed (assuming source and
sink are fast enough in the first place) is high enough that it may
surprise some users, so document it. Mention that this will be fixed
in the future, otherwise some users might misguidedly rely on it or
clamp their configuration settings to the documented value.

Signed-off-by: Sascha Silbe 
---
Noticed this while figuring out why qemu-iotests #141 failed on one of
my systems. I for one was quite surprised, so I went ahead and
documented it.


 qmp-commands.hx | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index de896a5..e0e519a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1107,7 +1107,10 @@ Arguments:
   obvious choice.  Care should be taken when specifying the
   string, to specify a valid filename or protocol.
   (json-string, optional) (Since 2.1)
-- "speed":  the maximum speed, in bytes per second (json-int, optional)
+- "speed": The maximum speed, in bytes per second. Note: In the current
+   implementation, at least 5MiB/s will be written, even if a lower
+   speed has been set. This will be fixed in the future. (json-int,
+   optional)
 - "on-error": the action to take on an error (default 'report').  'stop' and
   'enospc' can only be used if the block device supports io-status.
   (json-string, optional) (Since 2.1)
@@ -1172,7 +1175,10 @@ Arguments:
   size of the smaller top, you can safely truncate it
   yourself once the commit operation successfully completes.
   (json-string)
-- "speed":  the maximum speed, in bytes per second (json-int, optional)
+- "speed": The maximum speed, in bytes per second. Note: In the current
+   implementation, at least 5MiB/s will be written, even if a lower
+   speed has been set. This will be fixed in the future. (json-int,
+   optional)
 
 
 Example:
@@ -1219,7 +1225,10 @@ Arguments:
 is "incremental", must NOT be present otherwise.
 - "mode": whether and how QEMU should create a new image
   (NewImageMode, optional, default 'absolute-paths')
-- "speed": the maximum speed, in bytes per second (json-int, optional)
+- "speed": The maximum speed, in bytes per second. Note: The current
+   implementation will write at a minimum speed that depends on device
+   and format, even if a lower speed is configured. This will be fixed
+   in the future. (json-int, optional)
 - "on-source-error": the action to take on an error on the source, default
  'report'.  'stop' and 'enospc' can only be used
  if the block device supports io-status.
@@ -1260,7 +1269,10 @@ Arguments:
   possibilities include "full" for all the disk, "top" for only the
   sectors allocated in the topmost image, or "none" to only replicate
   new I/O (MirrorSyncMode).
-- "speed": the maximum speed, in bytes per second (json-int, optional)
+- "speed": The maximum speed, in bytes per second. Note: The current
+   implementation will write at a minimum speed that depends on device
+   and format, even if a lower speed is configured. This will be fixed
+   in the future. (json-int, optional)
 - "on-source-error": the action to take on an error on the source, default
  'report'.  'stop' and 'enospc' can only be used
  if the block device supports io-status.
@@ -1659,8 +1671,12 @@ Arguments:
   (json-string, optional)
 - "mode": how an image file should be created into the target
   file/device (NewImageMode, optional, default 'absolute-paths')
-- "speed": maximum speed of the streaming job, in bytes per second
-  (json-int)
+- "speed": The maximum speed, in bytes per second. Note: In the
+   current implementation, the buffer will be written at least
+   once per 100ms. So with the default buffer size of 10MiB,
+   at least 10MiB/s will be written, even if a lower speed is
+   configured. This will be fixed in the future. (json-int,
+   optional)
 - "granularity": granularity of the dirty bitmap, in bytes (json-int, optional)
 - "buf-size": maximum amount of data in flight from source to target, in bytes
   (json-int, default 10M)
@@ -1712,8 +1728,12 @@ Arguments:
 - "target": device name to mirror to (json-string)
 - "replaces": the block driver node name to replace when finished
   (json-string, optional)
-- "speed": maximum speed of the streaming job, in bytes per second
-  (json-int)
+- "speed": The maximum speed, in bytes per second. Note: In the
+   current implementation, the buffer will 

Re: [Qemu-devel] [PATCH] correct comment of MemoryHotplugState

[Eduardo Habkost]

On Sat, Apr 09, 2016 at 05:18:01PM +0800, Cao jin wrote:
> ping?

I will add this to the machine queue for 2.7.

> On 03/16/2016 04:00 AM, Eduardo Habkost wrote:
> >On Mon, Mar 14, 2016 at 01:42:06PM +0800, Cao jin wrote:
> >>Hi,
> >> Is anyone gonna take this one?
> >
> >Not sure which tree this should go. Michael, Igor, if you expect
> >this to go through the Machine Core tree, please let me know.
> >
> 
> -- 
> Yours Sincerely,
> 
> Cao jin
> 
> 

-- 
Eduardo

[Qemu-devel] work needed on QEMU 2.6 ChangeLog

[Paolo Bonzini]

Hi all, if you are CCed there is a feature you wrote or maintain that
needs further work in the ChangeLog
(http://wiki.qemu.org/ChangeLog/2.6).  In particular:

Andrew/Peter:
* New partial Raspberry Pi 1 and 2 emulation with "raspi" and "raspi2"
machine types. For now the "raspi2" machine type can boot Windows.
(FIXME: confirm)

Alex/Peter:
* TCG supports a new "-dfilter" option to limit exec, out_asm, op and
op_opt logging to a range of guest addresses (FIXME: physical or
virtual?). ARM also applies the filter to in_asm logging; this will be
extended to other targets in future releases (FIXME: probably should do
it now instead...)

Michael:
* FIXME: what's the state of nvdimm?

Daniel:
* FIXME: Support for TLS encryption in the TCP backend
* FIXME: -chardev logfile
* FIXME: qemu-io --object [...]
* FIXME: qemu-img --object [...]
* FIXME: document new secret passing system

Jason:
* FIXME: network filters?

Gerd:
* FIXME: document -input-linux when QOM-based syntax lands

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v4 1/9] softfloat: Implement run-time-configurable meaning of signaling NaN bit

[Eduardo Habkost]

On Tue, Apr 12, 2016 at 02:58:03PM +0200, Aleksandar Markovic wrote:
[...]
>   4) Updated code for all platforms to reflect changes in SoftFloat library.
>  This change is twofolds: it includes modifications of SoftFloat library
>  functions invocations, and an addition of invocation of function
>  set_snan_bit_is_one() during CPU initialization, with arguments that
>  are appropriate for each particular platform.
> 
>  In order to be at the same time accurate in relation to floating point
>  arithmetics and consistent with a particular platform code, following
>  principle is adopted related to invocations of new function
>  set_snan_bit_is_one():
> 
>  1. If a target platform doesn't use SoftFloat library, nothing is added.
> (cris, lm32, moxie)
>  2. Else, if a target platform's signaling bit meaning is "1 is
> signaling", explicit invocation of set_snan_bit_is_one(1) is added.
> (mips, sh4, unicore32)
>   (for mips this will change in the second patch of this
>   series, but this very patch leaves mips features as
>   they currently are.)
>  3. Else, if a target platform explicitly sets other fields of its
> float_status structure(s), explicit invocation of
> set_snan_bit_is_one(0) is added.
> (ppc, s390x, tricore)
>  4. Else, if a target platform doesn't explicitly set its structure
> CPUXXXState, explicit invocation of set_snan_bit_is_one(0) is added.
> (alpha, xtensa)

The CPU struct is zeroed by object_new() when the CPU object is
created, so the field is already set to 0 by default on all
architectures. You shouldn't need set_snan_bit_is_one(0) calls on
CPU initialization on any architecture.


>  5. For remaining cases, nothing is added. (Those cases explicitly set
> their CPUXXXState/float_status structures to 0, and thus implicitly
> perform set_snan_bit_is_one(0).)
> (arm, i386, m68k, microblaze, openrisc, sparc)
> 

-- 
Eduardo

[Qemu-devel] Fwd: [PATCH v2 5/5] Change return type of functions that are named *_exit or *_exitfn in hw/usb from int to void

[Nutan Shinde]

This change is required because the return type of exit(error) function is
not used.
This patch only has changes for files in hw/usb package.

Signed-off-by: Nutan Shinde 
---
 hw/usb/ccid-card-emulated.c   |  3 +--
 hw/usb/ccid.h |  2 +-
 hw/usb/dev-smartcard-reader.c | 11 ---
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 9ddd5ad..9962786 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -547,7 +547,7 @@ static int emulated_initfn(CCIDCardState *base)
 return 0;
 }

-static int emulated_exitfn(CCIDCardState *base)
+static void emulated_exitfn(CCIDCardState *base)
 {
 EmulatedState *card = EMULATED_CCID_CARD(base);
 VEvent *vevent = vevent_new(VEVENT_LAST, NULL, NULL);
@@ -564,7 +564,6 @@ static int emulated_exitfn(CCIDCardState *base)
 qemu_mutex_destroy(>handle_apdu_mutex);
 qemu_mutex_destroy(>vreader_mutex);
 qemu_mutex_destroy(>event_list_mutex);
-return 0;
 }

 static Property emulated_card_properties[] = {
diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h
index 9334da8..1f07011 100644
--- a/hw/usb/ccid.h
+++ b/hw/usb/ccid.h
@@ -33,7 +33,7 @@ typedef struct CCIDCardClass {
 void (*apdu_from_guest)(CCIDCardState *card,
 const uint8_t *apdu,
 uint32_t len);
-int (*exitfn)(CCIDCardState *card);
+void (*exitfn)(CCIDCardState *card);
 int (*initfn)(CCIDCardState *card);
 } CCIDCardClass;

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index af4b851..aeb357f 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -508,14 +508,13 @@ static void ccid_card_apdu_from_guest(CCIDCardState
*card,
 }
 }

-static int ccid_card_exitfn(CCIDCardState *card)
+static void ccid_card_exitfn(CCIDCardState *card)
 {
 CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);

 if (cc->exitfn) {
-return cc->exitfn(card);
+cc->exitfn(card);
 }
-return 0;
 }

 static int ccid_card_initfn(CCIDCardState *card)
@@ -1277,9 +1276,8 @@ void ccid_card_card_inserted(CCIDCardState *card)
 ccid_on_slot_change(s, true);
 }

-static int ccid_card_exit(DeviceState *qdev)
+static void ccid_card_exit(DeviceState *qdev)
 {
-int ret = 0;
 CCIDCardState *card = CCID_CARD(qdev);
 USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
 USBCCIDState *s = USB_CCID_DEV(dev);
@@ -1287,9 +1285,8 @@ static int ccid_card_exit(DeviceState *qdev)
 if (ccid_card_inserted(s)) {
 ccid_card_card_removed(card);
 }
-ret = ccid_card_exitfn(card);
+ccid_card_exitfn(card);
 s->card = NULL;
-return ret;
 }

 static int ccid_card_init(DeviceState *qdev)
--
1.9.1

[Qemu-devel] Fwd: [PATCH v2 4/5] Change return type of functions that are named *_exit or *_exitfn in hw/s390x from int to void

[Nutan Shinde]

This change is required because the return type of exit(error) function is
not used.
This patch only has changes for files in hw/s390x package.

Signed-off-by: Nutan Shinde 
---
 hw/s390x/virtio-ccw.c | 7 +++
 hw/s390x/virtio-ccw.h | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d51642d..e8b5a38 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -878,7 +878,7 @@ out_err:
 g_free(sch);
 }

-static int virtio_ccw_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_exit(VirtioCcwDevice *dev)
 {
 SubchDev *sch = dev->sch;

@@ -890,7 +890,6 @@ static int virtio_ccw_exit(VirtioCcwDevice *dev)
 release_indicator(>routes.adapter, dev->indicators);
 dev->indicators = NULL;
 }
-return 0;
 }

 static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
@@ -1735,12 +1734,12 @@ static void virtio_ccw_busdev_realize(DeviceState
*dev, Error **errp)
 virtio_ccw_device_realize(_dev, errp);
 }

-static int virtio_ccw_busdev_exit(DeviceState *dev)
+static void virtio_ccw_busdev_exit(DeviceState *dev)
 {
 VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
 VirtIOCCWDeviceClass *_info = VIRTIO_CCW_DEVICE_GET_CLASS(dev);

-return _info->exit(_dev);
+_info->exit(_dev);
 }

 static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev,
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 66c831b..854602f 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -68,7 +68,7 @@ typedef struct VirtioCcwDevice VirtioCcwDevice;
 typedef struct VirtIOCCWDeviceClass {
 DeviceClass parent_class;
 void (*realize)(VirtioCcwDevice *dev, Error **errp);
-int (*exit)(VirtioCcwDevice *dev);
+void (*exit)(VirtioCcwDevice *dev);
 } VirtIOCCWDeviceClass;

 /* Performance improves when virtqueue kick processing is decoupled from
the
--
1.9.1

Re: [Qemu-devel] [PATCHv9] Improve documentation for TLS

[Alex Bligh]

Eric,

Thanks for these - both in v10.

On 12 Apr 2016, at 16:15, Eric Blake  wrote:

>> +There is a degenerate case of SELECTIVETLS where all
>> +exports are TLS-only. This is permitted in part to make programming
>> +of servers easier. Operation is a little different from FORCEDTLS,
>> +as the client is not forced to upgrade to TLS prior to any options
>> +being processed, and the server MAY choose to give information on
>> +non-existent exports via NBD_OPT_INFO exports prior to an upgrade
> 
> s/exports prior/responses/

Thanks. Think you mean s/exports prior/responses prior/

>> +
>> +## Client-side requirements
> 
> ...but this should also be H3. s/##/###/

Thanks.

--
Alex Bligh






signature.asc
Description: Message signed with OpenPGP using GPGMail

[Qemu-devel] Fwd: [PATCH v2 3/5] Change return type of functions that are named *_exit or *_exitfn in hw/core from int to void

[Nutan Shinde]

This change is required because the return type of exit(error) function is
not used.
This patch only has changes for files in hw/core package.

Signed-off-by: Nutan Shinde 
---
 hw/core/qdev.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index db41aa1..7495fcf 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -237,11 +237,7 @@ static void device_unrealize(DeviceState *dev, Error
**errp)
 DeviceClass *dc = DEVICE_GET_CLASS(dev);

 if (dc->exit) {
-int rc = dc->exit(dev);
-if (rc < 0) {
-error_setg(errp, "Device exit failed.");
-return;
-}
+dc->exit(dev);
 }
 }

--
1.9.1

[Qemu-devel] [PATCHv10] Improve documentation for TLS

[Alex Bligh]

* Call out TLS into a separate section

* Add details of the TLS protocol itself

* Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
  be initiated from either side (as required by the TLS standard I believe
  and as actually works in practice)

* Clarify what is a requirement on servers, and what is a requirement on
  clients, separately, specifying their behaviour in a single place
  in the document.

* Document the three possible modes of operation of a server.

* Add text defining what 'terminate the session' means during
  negotiation, and when it is available.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 352 +--
 1 file changed, 318 insertions(+), 34 deletions(-)

Changes since v9

* Two further nits from Eric Blake.

Changes since v8:

* Reword section on disconnection, as per mail to list save with Eric Blake's
 change of 're' to 'regarding'.

Changes since v7

* I missed committing the changes re consistent use of 'option' rather than 
'command'
in v7. They are here now.

Changes from v6:

* Introduced language mandating a server to reply with NBD_ERR_INVALID
to NBD_OPT_STARTTLS if TLS is already negotiatied.

* Removed some duplication in SELECTIVETLS over the prohibition on
servers not returning NBD_ERR_TLSREQD to options other than
NBD_OPT_EXPORTNAME, NBD_OPT_INFO and NBD_OPT_GO. The same thing
was said a different way a couple of paragraphs below.

* Consistently refer to 'options' rather than 'commands' in the
negotiation phase.

* Eric Blake's nits

Changes from v5:

* Delete OPTIONALTLS (RIP)

* Add NBD_REP_ERR_POLICY

* s/NBD_ERR_REP/NBD_REP_ERR/ in one place

* Consistently use the phrase 'terminate the session' to mean dropping
the connection, as per Wouter. Note there are other inconsistent
uses of 'dropping the connection', 'disconnecting' etc. elsewhere
which I haven't touched.

* Similarly refer to the connection as a 'session' when it doesn't
explicitly mean the L3 TCP connection (TLS section only).

* Introduce a paragraph under newstyle negotiation emphasising that
terminating the session is legal and sometimes required, and defining
it.

Changes from v4

* Minor grammar nit

Changes from v3:

* Delete confusing text about server omitting entries from NBD_OPT_LIST
if TLS is not negotiated and FORCETLS is used, as that (of course)
requires NBD_REP_ERR_TLS_REQD elsewhere in the text.

* Further nits from Eric Blake

Changes from v2:

* The response to a command is a response, not a NBD_REP_ACK

* Make it clear that the response can be errored

* Nits from Eric Blake

Changes from v1:

* Make a NBD_CMD_CLOSE imply a flush

* Nits from Eric Blake

diff --git a/doc/proto.md b/doc/proto.md
index f117394..05fef3c 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -195,6 +195,23 @@ request before sending the next one of the same type. The 
server MAY
 send replies in the order that the requests were received, but is not
 required to.
 
+There is no requirement for the client or server to complete a
+negotiation if it does not wish to do so. Either end may simply
+close the TCP connection (though see below regarding prior use
+of NBD_OPT_ABORT). Under certain circumstances either
+the client or the server may be required by this document to close
+the TCP connection. In each case, this is referred to as 'terminate
+the session'.
+
+If the client wishes to terminate the session in the negotiation
+phase, and is not doing so because it is required to do so
+by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
+permits. There are instances where this is impossible, such as after
+an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
+negotiation of TLS.  For instance, if the client does not find an
+export it is looking for, it may simply send an NBD_OPT_ABORT
+and close the TCP connection.
+
 ### Transmission
 
 There are three message types in the transmission phase: the request,
@@ -286,6 +303,287 @@ S: (*length* bytes of data if the request is of type 
`NBD_CMD_READ`)
 This reply type MUST NOT be used except as documented by the
 experimental `STRUCTURED_REPLY` extension; see below.
 
+## TLS support
+
+The NBD protocol supports Transport Layer Security (TLS) (see
+[RFC5246](https://tools.ietf.org/html/rfc5246)
+as updated by
+[RFC6176](https://tools.ietf.org/html/rfc6176)
+).
+
+TLS is negotiated with the `NBD_OPT_STARTTLS`
+option. This is performed as an in-session upgrade. Below the term
+'negotiation' is used to refer to the sending and receiving of
+NBD options and option replies, and the term 'initiation' of TLS
+is used to refer to the actual upgrade to TLS.
+
+### Certificates, authentication and authorisation
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authorisation (for instance
+whether client and/or server certificates are required and
+what they 

[Qemu-devel] Fwd: [PATCH v2 2/5] Change return type of functions that are named *_exit or *_exitfn in hw/char from int to void

[Nutan Shinde]

This change is required because the return type of exit(error) function is
not used.
This patch only has changes for files in hw/char package.

Signed-off-by: Nutan Shinde 
---
 hw/char/sclpconsole-lm.c | 3 +--
 hw/char/sclpconsole.c| 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 7d4ff81..a9a9703 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -328,9 +328,8 @@ static int console_init(SCLPEvent *event)
 return 0;
 }

-static int console_exit(SCLPEvent *event)
+static console_exit(SCLPEvent *event)
 {
-return 0;
 }

 static void console_reset(DeviceState *dev)
diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index 45997ff..d179404 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -242,9 +242,8 @@ static void console_reset(DeviceState *dev)
scon->notify = false;
 }

-static int console_exit(SCLPEvent *event)
+static console_exit(SCLPEvent *event)
 {
-return 0;
 }

 static Property console_properties[] = {
--
1.9.1

[Qemu-devel] [Bug 1569491] [NEW] qemu system i386 poor performance on e5500 core

[luigiburdo]

Public bug reported:

I had been tested with generic core net building or with mtune e5500 but i have 
the same result: performances 
are extremly low compared with other classes of powerpc cpu.
The strange is the 5020 2ghz in all emulators been tested by me is comparable 
with a 970MP 2.7 ghz in speed and benchmarks but im facing the half of 
performance in i386-soft-mmu compared with a 2.5 ghz 970MP.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu system i386 poor performance on e5500 core

Status in QEMU:
  New

Bug description:
  I had been tested with generic core net building or with mtune e5500 but i 
have the same result: performances 
  are extremly low compared with other classes of powerpc cpu.
  The strange is the 5020 2ghz in all emulators been tested by me is comparable 
with a 970MP 2.7 ghz in speed and benchmarks but im facing the half of 
performance in i386-soft-mmu compared with a 2.5 ghz 970MP.

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

[Qemu-devel] Fwd: [PATCH v2 1/5] Change return type of functions that are named *_exit or *_exitfn in hw/ from int to void

[Nutan Shinde]

This changes is required because the return type of exit(error) function is
not used.
This patch only has changes for files in hw/audio package.

Signed-off-by: Nutan Shinde 
---
 hw/audio/hda-codec.c   | 3 +--
 hw/audio/intel-hda.c   | 3 +--
 hw/audio/intel-hda.h   | 2 +-
 include/hw/qdev-core.h | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const
struct desc_codec *desc)
 return 0;
 }

-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
 }
 }
 AUD_remove_card(>card);
-return 0;
 }

 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d372d4a..404cfcf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -66,7 +66,7 @@ static int hda_codec_dev_init(DeviceState *qdev)
 return cdc->init(dev);
 }

-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_exit(DeviceState *qdev)
 {
 HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
 HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -74,7 +74,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 if (cdc->exit) {
 cdc->exit(dev);
 }
-return 0;
 }

 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
 DeviceClass parent_class;

 int (*init)(HDACodecDevice *dev);
-int (*exit)(HDACodecDevice *dev);
+void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool
output);
 } HDACodecDeviceClass;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1ce02b2..0b635bd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -30,7 +30,7 @@ typedef enum DeviceCategory {
 } DeviceCategory;

 typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
--
1.9.1

[Qemu-devel] [PATCH v2 1/5] Change return type of functions that are named *_exit or *_exitfn in hw/ from int to void

[Nutan Shinde]

This changes is required because the return type of exit(error) function is not 
used.
This patch only has changes for files in hw/audio package.

Signed-off-by: Nutan Shinde 
---
 hw/audio/hda-codec.c   | 3 +--
 hw/audio/intel-hda.c   | 3 +--
 hw/audio/intel-hda.h   | 2 +-
 include/hw/qdev-core.h | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 return 0;
 }
 
-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
 }
 }
 AUD_remove_card(>card);
-return 0;
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d372d4a..404cfcf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -66,7 +66,7 @@ static int hda_codec_dev_init(DeviceState *qdev)
 return cdc->init(dev);
 }
 
-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_exit(DeviceState *qdev)
 {
 HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
 HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -74,7 +74,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 if (cdc->exit) {
 cdc->exit(dev);
 }
-return 0;
 }
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
 DeviceClass parent_class;
 
 int (*init)(HDACodecDevice *dev);
-int (*exit)(HDACodecDevice *dev);
+void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool 
output);
 } HDACodecDeviceClass;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1ce02b2..0b635bd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -30,7 +30,7 @@ typedef enum DeviceCategory {
 } DeviceCategory;
 
 typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
-- 
1.9.1

Re: [Qemu-devel] [PATCH] Change return type of functions that are named *_exit or *_exitfn in hw/ from int to void

[Nutan Shinde]

Hi,

Please ignore this patch, I have sent a new patch with version v2, in
subject line, which has updated changes.

Regards,
Nutan.

On Tue, Apr 12, 2016 at 11:24 PM, Nutan Shinde 
wrote:

> This changes is required because the return type of exit(error) function
> is not used.
> This patch only has changes for files in hw/audio package.
>
> Signed-off-by: Nutan Shinde 
> ---
>  hw/audio/hda-codec.c   | 3 +--
>  hw/audio/intel-hda.c   | 3 +--
>  hw/audio/intel-hda.h   | 2 +-
>  include/hw/qdev-core.h | 2 +-
>  4 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
> index 52d4640..5402cd1 100644
> --- a/hw/audio/hda-codec.c
> +++ b/hw/audio/hda-codec.c
> @@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const
> struct desc_codec *desc)
>  return 0;
>  }
>
> -static int hda_audio_exit(HDACodecDevice *hda)
> +static void hda_audio_exit(HDACodecDevice *hda)
>  {
>  HDAAudioState *a = HDA_AUDIO(hda);
>  HDAAudioStream *st;
> @@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
>  }
>  }
>  AUD_remove_card(>card);
> -return 0;
>  }
>
>  static int hda_audio_post_load(void *opaque, int version)
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index d372d4a..404cfcf 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -66,7 +66,7 @@ static int hda_codec_dev_init(DeviceState *qdev)
>  return cdc->init(dev);
>  }
>
> -static int hda_codec_dev_exit(DeviceState *qdev)
> +static void hda_codec_dev_exit(DeviceState *qdev)
>  {
>  HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
>  HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
> @@ -74,7 +74,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
>  if (cdc->exit) {
>  cdc->exit(dev);
>  }
> -return 0;
>  }
>
>  HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
> diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
> index d784bcf..53b78da 100644
> --- a/hw/audio/intel-hda.h
> +++ b/hw/audio/intel-hda.h
> @@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
>  DeviceClass parent_class;
>
>  int (*init)(HDACodecDevice *dev);
> -int (*exit)(HDACodecDevice *dev);
> +void (*exit)(HDACodecDevice *dev);
>  void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
>  void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool
> output);
>  } HDACodecDeviceClass;
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 1ce02b2..0b635bd 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -30,7 +30,7 @@ typedef enum DeviceCategory {
>  } DeviceCategory;
>
>  typedef int (*qdev_initfn)(DeviceState *dev);
> -typedef int (*qdev_event)(DeviceState *dev);
> +typedef void (*qdev_event)(DeviceState *dev);
>  typedef void (*qdev_resetfn)(DeviceState *dev);
>  typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
>  typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
> --
> 1.9.1
>
>

[Qemu-devel] [PATCH v2 1/5] Change return type of functions that are named *_exit or *_exitfn in hw/ from int to void

[Nutan Shinde]

This changes is required because the return type of exit(error) function is not 
used.
This patch only has changes for files in hw/audio package.

Signed-off-by: Nutan Shinde 
---
 hw/audio/hda-codec.c   | 3 +--
 hw/audio/intel-hda.c   | 3 +--
 hw/audio/intel-hda.h   | 2 +-
 include/hw/qdev-core.h | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 return 0;
 }
 
-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
 }
 }
 AUD_remove_card(>card);
-return 0;
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d372d4a..404cfcf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -66,7 +66,7 @@ static int hda_codec_dev_init(DeviceState *qdev)
 return cdc->init(dev);
 }
 
-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_exit(DeviceState *qdev)
 {
 HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
 HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -74,7 +74,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 if (cdc->exit) {
 cdc->exit(dev);
 }
-return 0;
 }
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
 DeviceClass parent_class;
 
 int (*init)(HDACodecDevice *dev);
-int (*exit)(HDACodecDevice *dev);
+void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool 
output);
 } HDACodecDeviceClass;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1ce02b2..0b635bd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -30,7 +30,7 @@ typedef enum DeviceCategory {
 } DeviceCategory;
 
 typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
-- 
1.9.1

Re: [Qemu-devel] post-copy is broken?

[Dr. David Alan Gilbert]

* Li, Liang Z (liang.z...@intel.com) wrote:
> Hi David,
> 
> I used the latest qemu code (commit id: 4e71220387e88a22) and kernel (v4.5) 
> to test the post-copy, and find the guest get crashed after live migration, 
> no matter I did a local live migration or live migration between two hosts. I 
> just ran the stress as the workload in guest. It seems the post-copy is 
> broken?
> 
> Stress parameters:  stress --vm 2 --vm-hang 1 --vm-bytes 2048M --vm-keep
> QEMU parameters: ./qemu-system-x86_64 --enable-kvm -smp 4 -m 8192  -monitor 
> stdio -drive file=/share/centos6u6.qcow

My test seems to be working here (4.4.6-301.fc23 kernel) same qemu version.
This is with an f20 guest running google stressapptest.

What's your last working version?

Dave

> Liang
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH] Change return type of functions that are named *_exit or *_exitfn in hw/ from int to void

[Nutan Shinde]

This changes is required because the return type of exit(error) function is not 
used.
This patch only has changes for files in hw/audio package.

Signed-off-by: Nutan Shinde 
---
 hw/audio/hda-codec.c   | 3 +--
 hw/audio/intel-hda.c   | 3 +--
 hw/audio/intel-hda.h   | 2 +-
 include/hw/qdev-core.h | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 52d4640..5402cd1 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -520,7 +520,7 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 return 0;
 }
 
-static int hda_audio_exit(HDACodecDevice *hda)
+static void hda_audio_exit(HDACodecDevice *hda)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -539,7 +539,6 @@ static int hda_audio_exit(HDACodecDevice *hda)
 }
 }
 AUD_remove_card(>card);
-return 0;
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d372d4a..404cfcf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -66,7 +66,7 @@ static int hda_codec_dev_init(DeviceState *qdev)
 return cdc->init(dev);
 }
 
-static int hda_codec_dev_exit(DeviceState *qdev)
+static void hda_codec_dev_exit(DeviceState *qdev)
 {
 HDACodecDevice *dev = DO_UPCAST(HDACodecDevice, qdev, qdev);
 HDACodecDeviceClass *cdc = HDA_CODEC_DEVICE_GET_CLASS(dev);
@@ -74,7 +74,6 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 if (cdc->exit) {
 cdc->exit(dev);
 }
-return 0;
 }
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index d784bcf..53b78da 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -38,7 +38,7 @@ typedef struct HDACodecDeviceClass
 DeviceClass parent_class;
 
 int (*init)(HDACodecDevice *dev);
-int (*exit)(HDACodecDevice *dev);
+void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool 
output);
 } HDACodecDeviceClass;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1ce02b2..0b635bd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -30,7 +30,7 @@ typedef enum DeviceCategory {
 } DeviceCategory;
 
 typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 3/3] xenfb: remove out_cons in xenfb_handle_events

[Stefano Stabellini]

On Tue, 12 Apr 2016, Wei Liu wrote:
> The variable out_cons was only used to temporarily hold the consumer
> index. Use cons directly to simplify code a bit.
> 
> No functional change introduced.
> 
> Signed-off-by: Wei Liu 

Except for the fact that it is based on patch #2, which is wrong, this
looks OK.

Acked-by: Stefano Stabellini 


> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> ---
>  hw/display/xenfb.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 7f4fad7..7d50efb 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -770,16 +770,16 @@ static void xenfb_invalidate(void *opaque)
>  
>  static void xenfb_handle_events(struct XenFB *xenfb)
>  {
> -uint32_t prod, cons, out_cons;
> +uint32_t prod, cons;
>  struct xenfb_page *page = xenfb->c.page;
>  
>  prod = page->out_prod;
> -out_cons = page->out_cons;
> +cons = page->out_cons;
>  xen_rmb();
> -if (prod - out_cons > XENFB_OUT_RING_LEN) {
> +if (prod - cons > XENFB_OUT_RING_LEN) {
>  return;
>  }
> -for (cons = out_cons; cons != prod; cons++) {
> +for ( ; cons != prod; cons++) {
>   union xenfb_out_event *event = _OUT_RING_REF(page, cons);
>  uint8_t type = event->type;
>   int x, y, w, h;
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH for-2.6 0/4] qemu-iotests: don't use /tmp

[Max Reitz]

On 12.04.2016 16:56, Sascha Silbe wrote:
> During review of my other qemu-iotests fixes, Max Reitz noticed a
> couple of additional places where qemu-iotests hardcode /tmp. This is
> both a security issue and cumbersome when running multiple instances
> of qemu-iotests (e.g. different users on a shared development machine
> like tuxmaker). Fix them up.
> 
> Checked (/var)/tmp usage using "sudo chown 000 /tmp /var/tmp". "tac"
> and "qemu -snapshot" fail in that case. I'll presume they create the
> temporary files in a secure manner.
> 
> There is one place left that may use /var/tmp in theory; will address
> that in a separate patch.
> 
> This series mostly removes dead code and addresses a potential
> security issue, all of that in the test suite rather than in
> production code. It should be applicable even during hard freeze.
>  
> 
> Sascha Silbe (4):
>   qemu-iotests: drop unused _within_tolerance() filter
>   qemu-iotests: common.rc: drop unused _do()
>   qemu-iotests: tests: do not set unused tmp variable
>   qemu-iotests: place valgrind log file in scratch dir

Thanks Sascha, I applied the series to my block tree:

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

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Xen-devel] [PATCH v2 2/3] xenfb: move xen_rmb to the correct location

[Stefano Stabellini]

On Tue, 12 Apr 2016, Wei Liu wrote:
> On Tue, Apr 12, 2016 at 02:38:13PM +0100, Andrew Cooper wrote:
> > On 12/04/16 13:57, David Vrabel wrote:
> > > On 12/04/16 11:43, Wei Liu wrote:
> > >> It should be placed before first time producer and consumer are used.
> > > This change isn't necessary and is confusing as this is not what this
> > > barrier is for.
> > >
> > > The barrier needs to be between the load of prod and the load of the
> > > ring contents (there's even a comment that says this).  This pairs with
> > > the corresponding write barrier between the store of the ring contents
> > > and the store of prod (in the other end).
> > 
> > Looking further, this code will compile to multiple reads of the page,
> > because there is no ACCESS_ONCE().  This code is still vulnerable to
> > XSA-155.

There is no ACCESS_ONCE in QEMU, the closest thing to it is atomic_read.


> Oops, accidentally kicked over a can of worms. Should have just sent
> patch 1. :-)
> 
> Jokes aside, more time is needed to fix this properly. So maybe we
> should just upstream patch #1 first. Stefano? Anthony?

Sure

Re: [Qemu-devel] [PATCH 4/4] qemu-iotests: place valgrind log file in scratch dir

[Max Reitz]

On 12.04.2016 16:56, Sascha Silbe wrote:
> Do not place the valgrind log file at a predictable path in a
> world-writable location. Use the common scratch directory (${TEST_DIR})
> instead.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/common.config | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/4] qemu-iotests: tests: do not set unused tmp variable

[Max Reitz]

On 12.04.2016 16:56, Sascha Silbe wrote:
> The previous commit removed the last usage of ${tmp} inside the tests
> themselves; the only remaining users are sourced by check. So we can now
> drop this variable from the tests.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/001 | 1 -
>  tests/qemu-iotests/002 | 1 -
>  tests/qemu-iotests/003 | 1 -
>  tests/qemu-iotests/004 | 1 -
>  tests/qemu-iotests/005 | 1 -
>  tests/qemu-iotests/007 | 1 -
>  tests/qemu-iotests/008 | 1 -
>  tests/qemu-iotests/009 | 1 -
>  tests/qemu-iotests/010 | 1 -
>  tests/qemu-iotests/011 | 1 -
>  tests/qemu-iotests/012 | 1 -
>  tests/qemu-iotests/013 | 1 -
>  tests/qemu-iotests/014 | 1 -
>  tests/qemu-iotests/015 | 1 -
>  tests/qemu-iotests/017 | 1 -
>  tests/qemu-iotests/018 | 1 -
>  tests/qemu-iotests/019 | 1 -
>  tests/qemu-iotests/020 | 1 -
>  tests/qemu-iotests/021 | 1 -
>  tests/qemu-iotests/022 | 1 -
>  tests/qemu-iotests/023 | 1 -
>  tests/qemu-iotests/024 | 1 -
>  tests/qemu-iotests/025 | 1 -
>  tests/qemu-iotests/026 | 1 -
>  tests/qemu-iotests/027 | 1 -
>  tests/qemu-iotests/028 | 1 -
>  tests/qemu-iotests/029 | 1 -
>  tests/qemu-iotests/031 | 1 -
>  tests/qemu-iotests/032 | 1 -
>  tests/qemu-iotests/033 | 1 -
>  tests/qemu-iotests/034 | 1 -
>  tests/qemu-iotests/035 | 1 -
>  tests/qemu-iotests/036 | 1 -
>  tests/qemu-iotests/037 | 1 -
>  tests/qemu-iotests/038 | 1 -
>  tests/qemu-iotests/039 | 1 -
>  tests/qemu-iotests/042 | 1 -
>  tests/qemu-iotests/043 | 1 -
>  tests/qemu-iotests/046 | 1 -
>  tests/qemu-iotests/047 | 1 -
>  tests/qemu-iotests/049 | 1 -
>  tests/qemu-iotests/050 | 1 -
>  tests/qemu-iotests/051 | 1 -
>  tests/qemu-iotests/052 | 1 -
>  tests/qemu-iotests/053 | 1 -
>  tests/qemu-iotests/054 | 1 -
>  tests/qemu-iotests/058 | 1 -
>  tests/qemu-iotests/059 | 1 -
>  tests/qemu-iotests/060 | 1 -
>  tests/qemu-iotests/061 | 1 -
>  tests/qemu-iotests/062 | 1 -
>  tests/qemu-iotests/063 | 1 -
>  tests/qemu-iotests/064 | 1 -
>  tests/qemu-iotests/066 | 1 -
>  tests/qemu-iotests/067 | 1 -
>  tests/qemu-iotests/068 | 1 -
>  tests/qemu-iotests/069 | 1 -
>  tests/qemu-iotests/070 | 1 -
>  tests/qemu-iotests/071 | 1 -
>  tests/qemu-iotests/072 | 1 -
>  tests/qemu-iotests/073 | 1 -
>  tests/qemu-iotests/075 | 1 -
>  tests/qemu-iotests/076 | 1 -
>  tests/qemu-iotests/077 | 1 -
>  tests/qemu-iotests/078 | 1 -
>  tests/qemu-iotests/079 | 1 -
>  tests/qemu-iotests/080 | 1 -
>  tests/qemu-iotests/081 | 1 -
>  tests/qemu-iotests/082 | 1 -
>  tests/qemu-iotests/083 | 1 -
>  tests/qemu-iotests/084 | 1 -
>  tests/qemu-iotests/086 | 1 -
>  tests/qemu-iotests/087 | 1 -
>  tests/qemu-iotests/088 | 1 -
>  tests/qemu-iotests/089 | 1 -
>  tests/qemu-iotests/090 | 1 -
>  tests/qemu-iotests/092 | 1 -
>  tests/qemu-iotests/094 | 1 -
>  tests/qemu-iotests/097 | 1 -
>  tests/qemu-iotests/098 | 1 -
>  tests/qemu-iotests/099 | 1 -
>  tests/qemu-iotests/100 | 1 -
>  tests/qemu-iotests/101 | 1 -
>  tests/qemu-iotests/102 | 1 -
>  tests/qemu-iotests/103 | 1 -
>  tests/qemu-iotests/104 | 1 -
>  tests/qemu-iotests/105 | 1 -
>  tests/qemu-iotests/107 | 1 -
>  tests/qemu-iotests/108 | 1 -
>  tests/qemu-iotests/109 | 1 -
>  tests/qemu-iotests/110 | 1 -
>  tests/qemu-iotests/111 | 1 -
>  tests/qemu-iotests/112 | 1 -
>  tests/qemu-iotests/113 | 1 -
>  tests/qemu-iotests/114 | 1 -
>  tests/qemu-iotests/115 | 1 -
>  tests/qemu-iotests/116 | 1 -
>  tests/qemu-iotests/117 | 1 -
>  tests/qemu-iotests/119 | 1 -
>  tests/qemu-iotests/120 | 1 -
>  tests/qemu-iotests/121 | 1 -
>  tests/qemu-iotests/122 | 1 -
>  tests/qemu-iotests/123 | 1 -
>  tests/qemu-iotests/128 | 1 -
>  tests/qemu-iotests/130 | 1 -
>  tests/qemu-iotests/131 | 1 -
>  tests/qemu-iotests/133 | 1 -
>  tests/qemu-iotests/134 | 1 -
>  tests/qemu-iotests/135 | 1 -
>  tests/qemu-iotests/137 | 1 -
>  tests/qemu-iotests/138 | 1 -
>  tests/qemu-iotests/140 | 1 -
>  tests/qemu-iotests/141 | 1 -
>  tests/qemu-iotests/142 | 1 -
>  tests/qemu-iotests/143 | 1 -
>  tests/qemu-iotests/145 | 1 -
>  tests/qemu-iotests/150 | 1 -
>  117 files changed, 117 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: update Xen mailing list address

[Stefano Stabellini]

On Tue, 12 Apr 2016, Stefano Stabellini wrote:
> On Tue, 12 Apr 2016, Wei Liu wrote:
> > Signed-off-by: Wei Liu 
> 
> Acked-by: Stefano Stabellini 

I need to update my scripts. I meant:

Acked-by: Stefano Stabellini 

> 
> > Cc: Stefano Stabellini 
> > Cc: Anthony Perard 
> > 
> > This is the canonical address. It also seems that this is the most
> > reliable address.
> > ---
> >  MAINTAINERS | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9277fbf..df2aa2a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -280,7 +280,7 @@ Guest CPU Cores (Xen):
> >  X86
> >  M: Stefano Stabellini 
> >  M: Anthony Perard 
> > -L: xen-de...@lists.xensource.com
> > +L: xen-de...@lists.xenproject.org
> >  S: Supported
> >  F: xen-*
> >  F: */xen*
> > -- 
> > 2.1.4
> > 
> 

Re: [Qemu-devel] [PATCH v2 1/3] xenfb: use the correct condition to avoid excessive looping

[Stefano Stabellini]

On Tue, 12 Apr 2016, Wei Liu wrote:
> In commit ac0487e1 ("xenfb.c: avoid expensive loops when prod <=
> out_cons"), ">=" was used. In fact, a full ring is a legit state.
> Correct the test to use ">".
> 
> Reported-by: "Hao, Xudong" 
> Signed-off-by: Wei Liu 
> Tested-by: "Hao, Xudong" 
> Acked-by: Anthony Perard 

Acked-by: Stefano Stabellini 

I'll add it to my queue


> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> 
> Backport candidate to our own tree.
> ---
>  hw/display/xenfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 40b096a..9866dfd 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -775,7 +775,7 @@ static void xenfb_handle_events(struct XenFB *xenfb)
>  
>  prod = page->out_prod;
>  out_cons = page->out_cons;
> -if (prod - out_cons >= XENFB_OUT_RING_LEN) {
> +if (prod - out_cons > XENFB_OUT_RING_LEN) {
>  return;
>  }
>  xen_rmb();   /* ensure we see ring contents up to prod */
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 2/4] qemu-iotests: common.rc: drop unused _do()

[Max Reitz]

On 12.04.2016 16:56, Sascha Silbe wrote:
> _do() was never used and possibly creates temporary files at
> predictable, world-writable locations. Get rid of it.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/common.rc | 46 
> 
>  1 file changed, 46 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/4] qemu-iotests: drop unused _within_tolerance() filter

[Max Reitz]

On 12.04.2016 16:56, Sascha Silbe wrote:
> _within_tolerance() isn't used anymore and possibly creates temporary
> files at predictable, world-writable locations. Get rid of it.
> 
> If it's needed again in the future it can be revived easily and fixed up
> to use TEST_DIR and / or safely created temporary files.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/common.filter | 101 
> ---
>  1 file changed, 101 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: update Xen mailing list address

[Stefano Stabellini]

On Tue, 12 Apr 2016, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Acked-by: Stefano Stabellini 


> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> 
> This is the canonical address. It also seems that this is the most
> reliable address.
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9277fbf..df2aa2a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -280,7 +280,7 @@ Guest CPU Cores (Xen):
>  X86
>  M: Stefano Stabellini 
>  M: Anthony Perard 
> -L: xen-de...@lists.xensource.com
> +L: xen-de...@lists.xenproject.org
>  S: Supported
>  F: xen-*
>  F: */xen*
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCHv9] Improve documentation for TLS

[Eric Blake]

On 04/12/2016 07:27 AM, Alex Bligh wrote:
> * Call out TLS into a separate section
> 
> * Add details of the TLS protocol itself
> 
> * Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
>   be initiated from either side (as required by the TLS standard I believe
>   and as actually works in practice)
> 
> * Clarify what is a requirement on servers, and what is a requirement on
>   clients, separately, specifying their behaviour in a single place
>   in the document.
> 
> * Document the three possible modes of operation of a server.
> 
> * Add text defining what 'terminate the session' means during
>   negotiation, and when it is available.
> 
> Signed-off-by: Alex Bligh 
> ---

> @@ -286,6 +303,287 @@ S: (*length* bytes of data if the request is of type 
> `NBD_CMD_READ`)
>  This reply type MUST NOT be used except as documented by the
>  experimental `STRUCTURED_REPLY` extension; see below.
>  
> +## TLS support
> +

This is appropriate as an H2 header...


> +
> +### Server-side requirements

...this is appropriate as H3...


> +
> +## Client-side requirements

...but this should also be H3. s/##/###/

> +
> +If the client supports TLS at all, it MUST be prepared
> +to deal with servers operating in any of the above modes.
> +Notwithstanding, a client MAY always terminate the session or
> +refuse to connect to a particular export if TLS is
> +not available and the user requires TLS.
> +
> +The client MUST NOT issue `NBD_OPT_STARTTLS` unless the server
> +set flag NBD_FLAG_FIXED_NEWSTYLE and the client replied
> +with NBD_FLAG_C_FIXED_NEWSTYLE in the fixed newstyle
> +negotiation.
> +
> +The client MUST NOT issue `NBD_OPT_STARTTLS` if TLS has already
> +been initiated.
> +
> +Subject to the above two limitations, the client MAY send
> +`NBD_OPT_STARTTLS` at any time to initiate a TLS session. If the
> +client receives `NBD_REP_ACK` in response, it MUST immediately
> +upgrade the session to TLS. If it receives `NBD_REP_ERR_UNSUP`,
> +`NBD_REP_ERR_POLICY` or any other error in response, it indicates
> +that the server cannot or will not upgrade the session to TLS,
> +and therefore the client MUST either continue the session
> +without TLS, or terminate the session.
> +
> +A client that prefers to use TLS irrespective of whether
> +the server makes TLS mandatory SHOULD send `NBD_OPT_STARTTLS`
> +as the first option. This will ensure option haggling is subject
> +to TLS, and will thus prevent the possibility of options being
> +compromised by a Man-in-the-Middle attack. Note that the
> +`NBD_OPT_STARTTLS` itself may be compromised - see 'downgrade
> +attacks' for more details. For this reason, a client which only
> +wishes to use TLS SHOULD terminate the session if the
> +`NBD_OPT_STARTTLS` replies with an error.
> +
> +If the TLS handshake is unsuccessful (for instance the server's
> +certificate does not validate) the client MUST terminate the
> +session as by this stage it is too late to continue without TLS.
> +
> +If the client receives an `NBD_REP_ERR_TLS_REQD` in response
> +to any option, it implies that this option cannot be executed
> +unless a TLS upgrade is performed. If the option is any
> +option other than `NBD_OPT_INFO` or `NBD_OPT_GO`, this
> +indicates that no option will succeed unless a TLS upgrade
> +is performed; the client MAY therefore choose to issue
> +an `NBD_OPT_STARTTLS`, or MAY terminate the session (if
> +for instance it does not support TLS or does not have
> +appropriate credentials for this server). If the client
> +receives `NBD_REP_ERR_TLS_REQD` in response to
> +`NBD_OPT_INFO` or `NBD_OPT_GO` this indicates that the
> +export referred to within the option is either non-existent
> +or requires TLS; the client MAY therefore choose to issue
> +an `NBD_OPT_STARTTLS`, MAY terminate the session (if
> +for instance it does not support TLS or does not have
> +appropriate credentials for this server), or MAY continue
> +in another manner without TLS, for instance by querying
> +or using other exports.
> +
> +If a client supports TLS, it SHOULD also support the INFO
> +extension, and SHOULD use `NBD_OPT_GO` if available in place
> +of `NBD_OPT_EXPORT_NAME`. The reason for this is set out in
> +the final paragraphs of the sections under 'FORCEDTLS'
> +and 'SELECTIVETLS': this gives an opportunity for the
> +server to transmit that an error going into transmission
> +mode is due to the client's failure to initiate TLS,
> +and the fact that the client may obtain information about
> +which exports are TLS-only through `NBD_OPT_INFO`.
> +
> +### Security considerations
> +
> + TLS versions
> +
> +NBD implementations supporting TLS MUST support TLS version 1.2,
> +SHOULD support any later versions. NBD implementations
> +MAY support older versions but SHOULD NOT do so by default
> +(i.e. they SHOULD only be available by a configuration change).
> +Older versions SHOULD NOT be used where there is a risk of security
> +problems with those older 

Re: [Qemu-devel] [PATCH 3/3] ide: really restart pending and in-flight atapi dma

[John Snow]

On 04/12/2016 08:17 AM, Pavel Butsykin wrote:
> On 12.04.2016 01:18, Eric Blake wrote:
>> On 04/06/2016 12:40 AM, Denis V. Lunev wrote:
>>> From: Pavel Butsykin 
>>>
>>> Restart of ATAPI DMA used to be unreachable, because the request to do
>>> so wasn't indicated in bus->error_status due to the lack of spare
>>> bits, and
>>> ide_restart_bh() would return early doing nothing.
>>>
>>> This patch makes use of the observation that not all bit combinations
>>> were
>>> possible in ->error_status. In particular, IDE_RETRY_READ only made
>>> sense
>>> together with IDE_RETRY_DMA or IDE_RETRY_PIO. This allows to re-use
>>> IDE_RETRY_READ alone as an indicator of ATAPI DMA restart request.
>>>
>>> To makes things more uniform, ATAPI DMA gets its own value for
>>> ->dma_cmd.
>>> As a means against confusion, macros are added to test the state of
>>> ->error_status.
>>>
>>> The patch fixes the restart of both in-flight and pending ATAPI DMA,
>>> following the scheme similar to that of IDE DMA.
>>>
>>> Signed-off-by: Pavel Butsykin >

and these seem prone to false positives; where it might be better to do:

>>> Signed-off-by: Denis V. Lunev 
>>> ---
>>
>> I'll leave the technical feasibility of this to others, but have some
>> coding style comments:
>>
>>
>>> @@ -783,8 +782,10 @@ static int ide_handle_rw_error(IDEState *s, int
>>> error, int op)
>>>   s->bus->error_status = op;
>>>   } else if (action == BLOCK_ERROR_ACTION_REPORT) {
>>>   block_acct_failed(blk_get_stats(s->blk), >acct);
>>> -if (op & IDE_RETRY_DMA) {
>>> +if (IS_IDE_RETRY_DMA(op)) {
>>>   ide_dma_error(s);
>>
>> I'd probably have split this into two patches; one adding the accessor
>> macros for existing access, and the other adding the new bit pattern
>> (mixing a conversion along with new code is a bit trickier to review in
>> one patch).
>>
>>
>>> +++ b/hw/ide/internal.h
>>> @@ -338,6 +338,7 @@ enum ide_dma_cmd {
>>>   IDE_DMA_READ,
>>>   IDE_DMA_WRITE,
>>>   IDE_DMA_TRIM,
>>> +IDE_DMA_ATAPI
>>
>> Please keep the trailing comma, so that the next addition to this enum
>> won't have to adjust an existing line.
>>
> ok.
> 
>>>   };
>>>
>>>   #define ide_cmd_is_read(s) \
>>> @@ -508,11 +509,27 @@ struct IDEDevice {
>>>   /* These are used for the error_status field of IDEBus */
>>>   #define IDE_RETRY_DMA  0x08
>>>   #define IDE_RETRY_PIO  0x10
>>> +#define IDE_RETRY_ATAPI 0x20 /* reused IDE_RETRY_READ bit */
>>>   #define IDE_RETRY_READ  0x20
>>>   #define IDE_RETRY_FLUSH 0x40
>>>   #define IDE_RETRY_TRIM 0x80
>>>   #define IDE_RETRY_HBA  0x100
>>
>> Seems rather sparse on the comments about which bit patterns are valid
>> together.  If IDE_RETRY_READ is always used with at least one other bit,
>> it might make more sense to have an IDE_RETRY_MASK that selects the set
>> of bits being multiplexed, and/or macros that define the bits used in
>> combination.  Something along the lines of:
>>
>> #define IDE_RETRY_MASK0x38
>> #define IDE_RETRY_READ_DMA0x28
>> #define IDE_RETRY_READ_PIO0x30
>> #define IDE_RETRY_ATAPI   0x20
>>
>>>
>>> +#define IS_IDE_RETRY_DMA(_status) \
>>> +((_status) & IDE_RETRY_DMA)
>>> +
>>> +#define IS_IDE_RETRY_PIO(_status) \
>>> +((_status) & IDE_RETRY_PIO)
>>
>> and these seem prone to false positives; where it might be better to do:
>>
>> #define IS_IDE_RETRY_DMA(_status) \
>>  (((_status) & IDE_RETRY_MASK) == IDE_RETRY_READ_DMA)
>>
> This is not entirely true, because IDE_RETRY_DMA can be used for READ or
> WRITE operation.
> 
>>> +
>>> +/*
>>> + * The method of the IDE_RETRY_ATAPI determination is to use a
>>> previously
>>> + * impossible bit combination as a new status value.
>>> + */
>>> +#define IS_IDE_RETRY_ATAPI(_status)   \
>>> +(((_status) & IDE_RETRY_ATAPI) && \
>>> + !IS_IDE_RETRY_DMA(_status) &&\
>>> + !IS_IDE_RETRY_PIO(_status))
>>> +
>>
>> And this evaluates _status more than once, compared to:
>>
>> #define IS_IDE_RETRY_ATAPI(_status) \
>>  (((_status) & IDE_RETRY_MASK) == IDE_RETRY_ATAPI)
>>
>>
> Yes, it looks much nicer. I can make the change as a follow-up patch.
> 

I can squash the patch in staging.

Thanks,
--js

[Qemu-devel] [PULL 13/13] qemu-iotests: iotests.py: get rid of __all__

[Kevin Wolf]

From: Sascha Silbe 

The __all__ list contained a typo for as long as the iotests module
existed. That typo prevented "from iotests import *" (which is the
only case where iotests.__all__ is used at all) from ever working.

The names used by iotests are highly prone to name collisions, so
importing them all unconditionally is a bad idea anyway. Since __all__
is not adding any value, let's just get rid of it.

Fixes: f345cfd0 ("qemu-iotests: add iotests Python module")
Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-8-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 4 
 1 file changed, 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index bf31ec8..0c0b533 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -29,10 +29,6 @@ import qmp
 import qtest
 import struct
 
-__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
-   'VM', 'QMPTestCase', 'notrun', 'main', 'verify_image_format',
-   'verify_platform', 'filter_test_dir', 'filter_win32',
-   'filter_qemu_io', 'filter_chown', 'log']
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
-- 
1.8.3.1

[Qemu-devel] [PULL 12/13] qemu-iotests: 068: don't require KVM

[Kevin Wolf]

From: Sascha Silbe 

None of the other test cases explicitly enable KVM and there's no
obvious reason for 068 to require it. Drop this so all test cases can be
executed in environments where KVM is not available (e.g. because the
user doesn't have sufficient permissions to access /dev/kvm).

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-6-git-send-email-si...@linux.vnet.ibm.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/068 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 58d1d80..7562dd7 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -53,7 +53,7 @@ _make_test_img $IMG_SIZE
 
 case "$QEMU_DEFAULT_MACHINE" in
   s390-ccw-virtio)
-  platform_parm="-no-shutdown -machine accel=kvm"
+  platform_parm="-no-shutdown"
   ;;
   *)
   platform_parm=""
-- 
1.8.3.1

[Qemu-devel] [PULL 08/13] qemu-iotests: check: don't place files with predictable names in /tmp

[Kevin Wolf]

From: Sascha Silbe 

Placing files with predictable or even hard-coded names in /tmp is a
security risk and can prevent or disturb operation on a multi-user
machine. Place them inside the "scratch" directory instead, as we
already do for most other test-related files.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-2-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/check | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index c350f16..4cba215 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -19,7 +19,6 @@
 # Control script for QA
 #
 
-tmp=/tmp/$$
 status=0
 needwrap=true
 try=0
@@ -130,6 +129,8 @@ fi
 #exit 1
 #fi
 
+tmp="${TEST_DIR}"/$$
+
 _wallclock()
 {
 date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
@@ -146,8 +147,8 @@ _wrapup()
 # for hangcheck ...
 # remove files that were used by hangcheck
 #
-[ -f /tmp/check.pid ] && rm -rf /tmp/check.pid
-[ -f /tmp/check.sts ] && rm -rf /tmp/check.sts
+[ -f "${TEST_DIR}"/check.pid ] && rm -rf "${TEST_DIR}"/check.pid
+[ -f "${TEST_DIR}"/check.sts ] && rm -rf "${TEST_DIR}"/check.sts
 
 if $showme
 then
@@ -197,8 +198,8 @@ END{ if (NR > 0) {
 needwrap=false
 fi
 
-rm -f /tmp/*.out /tmp/*.err /tmp/*.time
-rm -f /tmp/check.pid /tmp/check.sts
+rm -f "${TEST_DIR}"/*.out "${TEST_DIR}"/*.err "${TEST_DIR}"/*.time
+rm -f "${TEST_DIR}"/check.pid "${TEST_DIR}"/check.sts
 rm -f $tmp.*
 }
 
@@ -208,16 +209,16 @@ trap "_wrapup; exit \$status" 0 1 2 3 15
 # Save pid of check in a well known place, so that hangcheck can be sure it
 # has the right pid (getting the pid from ps output is not reliable enough).
 #
-rm -rf /tmp/check.pid
-echo $$ >/tmp/check.pid
+rm -rf "${TEST_DIR}"/check.pid
+echo $$ > "${TEST_DIR}"/check.pid
 
 # for hangcheck ...
 # Save the status of check in a well known place, so that hangcheck can be
 # sure to know where check is up to (getting test number from ps output is
 # not reliable enough since the trace stuff has been introduced).
 #
-rm -rf /tmp/check.sts
-echo "preamble" >/tmp/check.sts
+rm -rf "${TEST_DIR}"/check.sts
+echo "preamble" > "${TEST_DIR}"/check.sts
 
 # don't leave old full output behind on a clean run
 rm -f check.full
@@ -285,7 +286,7 @@ do
 rm -f core $seq.notrun
 
 # for hangcheck ...
-echo "$seq" >/tmp/check.sts
+echo "$seq" > "${TEST_DIR}"/check.sts
 
 start=`_wallclock`
 $timestamp && echo -n "["`date "+%T"`"]"
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for 2.6 v4 0/3] ide: fix loss of the dma/atapi state during migration

[John Snow]

On 04/12/2016 01:33 AM, Denis V. Lunev wrote:
> On 04/12/2016 12:47 AM, John Snow wrote:
>>
>> On 04/11/2016 10:12 AM, Denis V. Lunev wrote:
>>> On 04/06/2016 09:40 AM, Denis V. Lunev wrote:
 This patch set fixes bugs in the IDE DMA and the IDE ATAPI on
 operations to
 save/restore the state.

   From the user point of view this results in IDE timeouts in the guest
 when the user reads from the DVD like the following:

 [424332.169229] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action
 0x6 frozen
 [424332.170423] sr 0:0:0:0: [sr0] CDB:
 [424332.171234] Read(10): 28 00 00 00 02 e4 00 00 01 00
 [424332.172418] ata1.00: cmd a0/01:00:00:00:08/00:00:00:00:00/a0 tag 0
 dma 2048 in
res 40/00:02:00:0c:00/00:00:00:00:00/a0 Emask 0x4 (timeout)
 [424332.174877] ata1.00: status: { DRDY }
 [424337.212099] ata1: link is slow to respond, please be patient
 (ready=0)
 [424342.220084] ata1: device not ready (errno=-16), forcing hardreset
 [424342.222700] ata1: soft resetting link
 [424342.381059] ata1.00: configured for MWDMA2
 [424342.383693] ata1: EH complete

 Another similar nasty effects are possible.

 Signed-off-by: Pavel Butsykin 
 Reviewed-by: Roman Kagan 
 Signed-off-by: Denis V. Lunev 
 CC: John Snow 

 Changes from v1:
 - added converter of IDE_DMA_* to IDE_RETRY_* (1)
 - fixed handling of the IDE_RETRY_HBA at the ide_restart_bh function
 (3)

 Changes from v2:
 - fixed enumeration value ‘IDE_DMA_ATAPI’ not handled in switch for
 macio.c (3)

 Changes from v3:
 - move the IDE_DMA_ATAPI setting in the cmd_packet func (3)

 Pavel Butsykin (3):
 ide: don't lose pending dma state
 ide: restart atapi dma by re-evaluating command packet
 ide: really restart pending and in-flight atapi dma

hw/ide/atapi.c| 26 --
hw/ide/core.c | 39 ---
hw/ide/internal.h | 36 
hw/ide/macio.c|  2 ++
hw/ide/pci.c  |  4 
5 files changed, 70 insertions(+), 37 deletions(-)

>>> ping
>> Sorry, I don't appear to have been CC'd on the actual patch emails, so I
>> missed them. The individual patch emails also miss the "v4" tag which
>> makes them hard to spot and it's hard to figure out what's been
>> changed or not since my reviewed-by was added to all three patches, even
>> though I only acknowledged the first 2/3.an reaction time of about 265 
>> millise
>>
> this is strange for me - you R-b: was added only to patches 1-2, here is
> a quote
> 
> "The patch fixes the restart of both in-flight and pending ATAPI DMA,
> following the scheme similar to that of IDE DMA.
> 
> Signed-off-by: Pavel Butsykin
> Signed-off-by: Denis V. Lunev
> ---
>  hw/ide/atapi.c| 13 ++---
>  hw/ide/core.c | 30 +++---
>  hw/ide/internal.h | 21 +
>  hw/ide/macio.c|  2 ++
>  4 files changed, 44 insertions(+), 22 deletions(-)"
> 
> this letter was sent at "04/06/2016 09:40 AM"
> 
> Den

Odd. Maybe because the "V4" tags were missing, it confused our patches
tool? You're right, I don't see it in the mail itself, sorry.

--js

[Qemu-devel] [PULL 01/13] iotests: fix the broken 026.nocache output

[Kevin Wolf]

From: Pavel Butsykin 

This patch fixes longstanding issue with 026 iotest. Unfortunately,
this test contains 2 versions of the correct output, one for cached
writes and one for non-cached ones. People tends to fix only one
version of output of the test and thus noncached version becomes
broken. Unfortunately, it is default in tests/check-block.sh

The following problematic commits were made:
commit 3b5e14c76a6bb142bf250ddf99e24a0ac8c7bc12
Author: Max Reitz 
Date:   Tue Dec 2 18:32:51 2014 +0100
qcow2: Flushing the caches in qcow2_close may fail

commit a069e2f1372a0a823ab506fc019852a2a652aa54
Author: John Snow 
Date:   Fri Feb 6 16:26:17 2015 -0500
blkdebug: fix "once" rule

commit b106ad9185f35fc4ad669555ad0e79e276083bd7
Author: Kevin Wolf 
Date:   Fri Mar 28 18:06:31 2014 +0100
qcow2: Don't rely on free_cluster_index in alloc_refcount_block()

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Max Reitz 
CC: John Snow 
CC: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/026.out.nocache | 142 -
 1 file changed, 123 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/026.out.nocache 
b/tests/qemu-iotests/026.out.nocache
index a5000d5..9c2c8a9 100644
--- a/tests/qemu-iotests/026.out.nocache
+++ b/tests/qemu-iotests/026.out.nocache
@@ -14,6 +14,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 1 leaked clusters were found on the image.
@@ -21,6 +23,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 1 leaked clusters were found on the image.
@@ -38,6 +42,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 1 leaked clusters were found on the image.
@@ -45,6 +51,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 1 leaked clusters were found on the image.
@@ -70,7 +78,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -78,7 +90,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write -b
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -102,7 +118,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 28; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 read failed: No space left on device
 No errors were found on the image.
 

[Qemu-devel] [PULL 06/13] qcow2: Prevent backing file names longer than 1023

[Kevin Wolf]

From: Max Reitz 

We reject backing file names with a length of more than 1023 characters
when opening a qcow2 file, so we should not produce such files
ourselves.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/qcow2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 056525c..470734b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1986,6 +1986,10 @@ static int qcow2_change_backing_file(BlockDriverState 
*bs,
 {
 BDRVQcow2State *s = bs->opaque;
 
+if (backing_file && strlen(backing_file) > 1023) {
+return -EINVAL;
+}
+
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_file ?: "");
 pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_fmt ?: "");
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v12 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()

[Max Reitz]

On 11.04.2016 07:18, Changlong Xie wrote:
> On 03/30/2016 11:07 PM, Max Reitz wrote:
>> On 30.03.2016 13:39, Alberto Garcia wrote:
>>> On Tue 29 Mar 2016 05:51:22 PM CEST, Max Reitz wrote:
> It sounds like the argument here, and in Max's thread on
> query-block-node-tree, is that we DO have cases where order
> matters, and
> so we need a way for the hot-add operation to explicitly specify where
> in the list a child is inserted (whether it is being inserted as
> the new
> primary image, or explicitly as the last resort, or somewhere in the
> middle).  An optional parameter, that defaults to appending, may be
> ok,
> but we definitely need to consider how the order of children is
> affected
> by hot-add.

 However, the order should be queriable after the fact, and there are
 three ways I see to accomplish this:

 (1) Make this information queriable as driver-specific BDS information.
  I personally don't like it very much, but it would be fine.
 (2) Implement query-block-node-tree, make the order of child nodes
  significant and thus represent the FIFO order there. I don't like
  this because it would mean returning two orders through that child
  node list: One is the numeric order (children.0, children.1, ...)
  and another is the FIFO order, which are not necessarily equal.
 (3) Fix FIFO order to the child name (its role). I'm very much in favor
  of this.

 While I don't have good arguments against (1), I think I have good
 arguments for (3) instead: It just doesn't make sense to have a numeric
 order of children if this order doesn't mean anything; especially if
 you
 suddenly do need the list of child nodes to be ordered. To me, it
 doesn't make any sense to introduce a new hidden order which takes
 precedence over this obvious user-visible order.
>>>
>>> I'm not sure if I understand correctly what you mean in (3). The
>>> user-visible FIFO order is the one specified when the Quorum is created:
>>>
>>> children.0.file.filename=hd0.qcow2,
>>> children.1.file.filename=hd1.qcow2,
>>> ...
>>>
>>> Would you then call those BDS children.0, children.1, etc
>>
>> They are already called that way; it's not their node name but their
>> "child role" name.
>>
>>>and make
>>> those
>>> names be the ones that actually define how they are ordered internally?
>>
>> Yes, that's what I meant.
>>
> Hi Max
> 
> I think you just mean what i draw in below chart:
> 
> 1) Insert 4 children.
> 0   1  2  3
> +
> |children.0|children.1|children.2|children.3|
> +
> 
> 2) Remove the 2th child (s->children[1])
> 
> { "execute": "x-blockdev-change",
> 
>   "arguments": { "parent": "xxx",
> 
>  "child": "children.1" } }
> 
> 0   1  2  3
> +
> |children.0|children.1|children.2|children.3|
> +++--
>  ||
>   +--+   ++
> 0   1 |2 |
> +v--v
> |children.0|children.1|children.2|
> +

No, what I meant, is:

 0  1  2  3
+--+--+--+--+
|children.0|children.1|children.2|children.3|
+--+--+--+--+

|
v

 0  1  2  3
+--+--+--+--+
|children.0|  |children.2|children.3|
+--+--+--+--+

I.e., children.1 simply ceases to exist.

Max

> Remove children.1 and shorten the array, then rename children.{2,3} to
> children.{1.2}
> 
> 3) Insert a new child
> 
> 0   1  2 3
> +
> |children.0|children.1|children.2|children.3|
> +
> 
> But as Wen said:
> http://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg00898.html
> 
> Everytime we try to remove a children.i (i < n-1, so it's not the last
> element of the array[n]), we have to rename children.{i+1, n-1} to
> children.{i, n-2}.
> 
> Thanks
> -Xie



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 09/13] qemu-iotests: fix 051 on non-PC architectures

[Kevin Wolf]

From: Sascha Silbe 

Commit 61de4c68 [block: Remove BDRV_O_CACHE_WB] updated the reference
output for PCs, but neglected to do the same for the generic reference
output file. Fix 051 on all non-PC architectures by applying the same
change to the generic output file.

Fixes: 61de4c68 ("block: Remove BDRV_O_CACHE_WB")
Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-3-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/051.out | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index c1291ff..408d613 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -145,7 +145,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 Testing: -drive driver=null-co,cache=invalid_value
 QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
-Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
+Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -165,7 +165,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
 Cache mode:   writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
+Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -185,7 +185,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
 Cache mode:   writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive 
file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
+Testing: -drive 
file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -205,8 +205,8 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
 Cache mode:   writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive 
file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
-QEMU_PROG: -drive 
file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0:
 invalid cache option
+Testing: -drive 
file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
+QEMU_PROG: -drive 

[Qemu-devel] [PULL 10/13] qemu-iotests: iotests.VM: remove qtest socket on error

[Kevin Wolf]

From: Sascha Silbe 

On error, VM.launch() cleaned up the monitor unix socket, but left the
qtest unix socket behind. This caused the remaining sub-tests to fail
with EADDRINUSE:

+==
+ERROR: testQuorum (__main__.TestFifoQuorumEvents)
+--
+Traceback (most recent call last):
+  File "148", line 63, in setUp
+self.vm.launch()
+  File "/home6/silbe/qemu/tests/qemu-iotests/iotests.py", line 247, in launch
+self._qmp.accept()
+  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 
141, in accept
+return self.__negotiate_capabilities()
+  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 
57, in __negotiate_capabilities
+raise QMPConnectError
+QMPConnectError
+
+==
+ERROR: testQuorum (__main__.TestQuorumEvents)
+--
+Traceback (most recent call last):
+  File "148", line 63, in setUp
+self.vm.launch()
+  File "/home6/silbe/qemu/tests/qemu-iotests/iotests.py", line 244, in launch
+self._qtest = qtest.QEMUQtestProtocol(self._qtest_path, server=True)
+  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qtest.py", line 33, 
in __init__
+self._sock.bind(self._address)
+  File "/usr/lib64/python2.7/socket.py", line 224, in meth
+return getattr(self._sock,name)(*args)
+error: [Errno 98] Address already in use

Fix this by cleaning up both the monitor socket and the qtest socket iff
they exist.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-4-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8499e1b..fb5c482 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -16,6 +16,7 @@
 # along with this program.  If not, see .
 #
 
+import errno
 import os
 import re
 import subprocess
@@ -247,7 +248,8 @@ class VM(object):
 self._qmp.accept()
 self._qtest.accept()
 except:
-os.remove(self._monitor_path)
+_remove_if_exists(self._monitor_path)
+_remove_if_exists(self._qtest_path)
 raise
 
 def shutdown(self):
@@ -409,6 +411,15 @@ class QMPTestCase(unittest.TestCase):
 event = self.wait_until_completed(drive=drive)
 self.assert_qmp(event, 'data/type', 'mirror')
 
+def _remove_if_exists(path):
+'''Remove file object at path if it exists'''
+try:
+os.remove(path)
+except OSError as exception:
+if exception.errno == errno.ENOENT:
+   return
+raise
+
 def notrun(reason):
 '''Skip this test suite'''
 # Each test in qemu-iotests has a number ("seq")
-- 
1.8.3.1

[Qemu-devel] [PULL 11/13] qemu-iotests: 148: properly skip test if quorum support is missing

[Kevin Wolf]

From: Sascha Silbe 

qemu-iotests test case 148 already had some code for skipping the test
if quorum support is missing, but it didn't work in all
cases. TestQuorumEvents.setUp() gets run before the actual test class
(which contains the skipping code) and tries to start qemu with a drive
using the quorum driver. For some reason this works fine when using
qcow2, but fails for raw.

As the entire test case requires quorum, just check for availability
before even starting the test suite. Introduce a verify_quorum()
function in iotests.py for this purpose so future test cases can make
use of it.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
Message-id: 1459848109-29756-5-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/148| 4 +---
 tests/qemu-iotests/iotests.py | 5 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/148 b/tests/qemu-iotests/148
index d066ec3..e01b061 100644
--- a/tests/qemu-iotests/148
+++ b/tests/qemu-iotests/148
@@ -79,9 +79,6 @@ sector = "%d"
 self.assert_qmp(event, 'data/sector-num', sector)
 
 def testQuorum(self):
-if not 'quorum' in iotests.qemu_img_pipe('--help'):
-return
-
 # Generate an error and get an event
 self.vm.hmp_qemu_io("drive0", "aio_read %d %d" %
 (offset * sector_size, sector_size))
@@ -139,4 +136,5 @@ class TestFifoQuorumEvents(TestQuorumEvents):
 read_pattern = 'fifo'
 
 if __name__ == '__main__':
+iotests.verify_quorum()
 iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fb5c482..bf31ec8 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -437,6 +437,11 @@ def verify_platform(supported_oses=['linux']):
 if True not in [sys.platform.startswith(x) for x in supported_oses]:
 notrun('not suitable for this OS: %s' % sys.platform)
 
+def verify_quorum():
+'''Skip test suite if quorum support is not available'''
+if 'quorum' not in qemu_img_pipe('--help'):
+notrun('quorum support missing')
+
 def main(supported_fmts=[], supported_oses=['linux']):
 '''Run tests'''
 
-- 
1.8.3.1

[Qemu-devel] [PULL 07/13] MAINTAINERS: Block layer core, qcow2 and blkdebug

[Kevin Wolf]

From: Max Reitz 

As agreed with Kevin and already practiced for a while, I am adding
myself as co-maintainer of the block layer core, qcow2 and blkdebug.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d261412..1c30dfa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -985,6 +985,7 @@ F: tests/intel-hda-test.c
 
 Block layer core
 M: Kevin Wolf 
+M: Max Reitz 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block*
@@ -1569,6 +1570,7 @@ F: block/win32-aio.c
 
 qcow2
 M: Kevin Wolf 
+M: Max Reitz 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block/qcow2*
@@ -1581,6 +1583,7 @@ F: block/qcow.c
 
 blkdebug
 M: Kevin Wolf 
+M: Max Reitz 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block/blkdebug.c
-- 
1.8.3.1

[Qemu-devel] [PULL 03/13] block: initialize qcrypto API at startup

[Kevin Wolf]

From: "Daniel P. Berrange" 

Any programs which call the qcrypto APIs should ensure that
qcrypto_init() has been called before anything else which
can use crypto. Essentially this means right at the start
of the main method before initializing anything else.

This is important because some versions of gnutls/gcrypt
require explicit initialization before use.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Alex Bligh 
Tested-by: Alex Bligh 
Signed-off-by: Kevin Wolf 
---
 qemu-img.c | 6 ++
 qemu-io.c  | 6 ++
 qemu-nbd.c | 7 +++
 3 files changed, 19 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 17c5cfd..1697762 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -37,6 +37,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "block/qapi.h"
+#include "crypto/init.h"
 #include 
 
 #define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION QEMU_PKGVERSION \
@@ -3486,6 +3487,11 @@ int main(int argc, char **argv)
 exit(EXIT_FAILURE);
 }
 
+if (qcrypto_init(_error) < 0) {
+error_reportf_err(local_error, "cannot initialize crypto: ");
+exit(1);
+}
+
 module_call_init(MODULE_INIT_QOM);
 bdrv_init();
 if (argc < 2) {
diff --git a/qemu-io.c b/qemu-io.c
index 0a738f1..288bba8 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -23,6 +23,7 @@
 #include "sysemu/block-backend.h"
 #include "block/block_int.h"
 #include "trace/control.h"
+#include "crypto/init.h"
 
 #define CMD_NOFILE_OK   0x01
 
@@ -443,6 +444,11 @@ int main(int argc, char **argv)
 progname = basename(argv[0]);
 qemu_init_exec_dir(argv[0]);
 
+if (qcrypto_init(_error) < 0) {
+error_reportf_err(local_error, "cannot initialize crypto: ");
+exit(1);
+}
+
 module_call_init(MODULE_INIT_QOM);
 qemu_add_opts(_object_opts);
 bdrv_init();
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c2e4d3f..b5751f8 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -31,6 +31,7 @@
 #include "qapi/qmp/qstring.h"
 #include "qom/object_interfaces.h"
 #include "io/channel-socket.h"
+#include "crypto/init.h"
 
 #include 
 #include 
@@ -519,6 +520,12 @@ int main(int argc, char **argv)
 memset(_sigterm, 0, sizeof(sa_sigterm));
 sa_sigterm.sa_handler = termsig_handler;
 sigaction(SIGTERM, _sigterm, NULL);
+
+if (qcrypto_init(_err) < 0) {
+error_reportf_err(local_err, "cannot initialize crypto: ");
+exit(1);
+}
+
 module_call_init(MODULE_INIT_QOM);
 qemu_add_opts(_object_opts);
 qemu_init_exec_dir(argv[0]);
-- 
1.8.3.1

[Qemu-devel] [PULL 04/13] iotests: Make 150 use qemu-img map instead of du

[Kevin Wolf]

From: Max Reitz 

The actual on-disk size of a file does not only depend on factors qemu
can control. Thus, we should not depend on this to determine whether a
file has indeed been fully allocated. Instead, use qemu-img map and hope
that if an area is referenced, it is indeed allocated, too.

Also, limit the supported image formats to raw and qcow2 because the
actual qemu-img map output may depend on the image format.

Signed-off-by: Max Reitz 
Tested-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/150 | 41 +
 tests/qemu-iotests/150.out | 13 +
 2 files changed, 10 insertions(+), 44 deletions(-)

diff --git a/tests/qemu-iotests/150 b/tests/qemu-iotests/150
index 97d2a35..665373d 100755
--- a/tests/qemu-iotests/150
+++ b/tests/qemu-iotests/150
@@ -38,65 +38,34 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-_supported_fmt generic
+_supported_fmt raw qcow2
 _supported_proto file
 _supported_os Linux
 
 
-on_disk_size()
-{
-du "$@" | sed -e 's/\t\+.*//'
-}
-
-
 img_size=1048576
 
 
 echo
-echo '=== Comparing empty image against sparse conversion ==='
+echo '=== Mapping sparse conversion ==='
 echo
 
-_make_test_img $img_size
-
-empty_size=$(on_disk_size "$TEST_IMG")
-
-
 $QEMU_IMG_PROG convert -O "$IMGFMT" -S 512 \
 "json:{ 'driver': 'null-co', 'size': $img_size, 'read-zeroes': true }" \
 "$TEST_IMG"
 
-sparse_convert_size=$(on_disk_size "$TEST_IMG")
-
-
-if [ "$empty_size" -eq "$sparse_convert_size" ]; then
-echo 'Equal image size'
-else
-echo 'Different image size'
-fi
+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
 
 
 echo
-echo '=== Comparing full image against non-sparse conversion ==='
+echo '=== Mapping non-sparse conversion ==='
 echo
 
-_make_test_img $img_size
-$QEMU_IO -c "write 0 $img_size" "$TEST_IMG" | _filter_qemu_io
-
-full_size=$(on_disk_size "$TEST_IMG")
-
-
 $QEMU_IMG convert -O "$IMGFMT" -S 0 \
 "json:{ 'driver': 'null-co', 'size': $img_size, 'read-zeroes': true }" \
 "$TEST_IMG"
 
-non_sparse_convert_size=$(on_disk_size "$TEST_IMG")
-
-
-if [ "$full_size" -eq "$non_sparse_convert_size" ]; then
-echo 'Equal image size'
-else
-echo 'Different image size'
-fi
+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
 
 
 # success, all done
diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out
index 2d29da1..2a54e8d 100644
--- a/tests/qemu-iotests/150.out
+++ b/tests/qemu-iotests/150.out
@@ -1,14 +1,11 @@
 QA output created by 150
 
-=== Comparing empty image against sparse conversion ===
+=== Mapping sparse conversion ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-Equal image size
+Offset  Length  File
 
-=== Comparing full image against non-sparse conversion ===
+=== Mapping non-sparse conversion ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-wrote 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Equal image size
+Offset  Length  File
+0   0x10TEST_DIR/t.IMGFMT
 *** done
-- 
1.8.3.1

[Qemu-devel] [PULL 02/13] qemu-img: fix formatting of error message

[Kevin Wolf]

From: "Daniel P. Berrange" 

The error_reportf_err() will not automatically append a
': ' before adding its suffix, so we must include that
in the message we pass it, otherwise we get a badly
formatted message lacking whitespace:

qemu-img: Could not open 
'driver=nbd,host=127.0.0.1,port=,tls-creds=tls0'Failed to connect socket: 
Connection refused

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 06264d9..17c5cfd 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -256,7 +256,7 @@ static BlockBackend *img_open_opts(const char *optstr,
 options = qemu_opts_to_qdict(opts, NULL);
 blk = blk_new_open(NULL, NULL, options, flags, _err);
 if (!blk) {
-error_reportf_err(local_err, "Could not open '%s'", optstr);
+error_reportf_err(local_err, "Could not open '%s': ", optstr);
 return NULL;
 }
 blk_set_enable_write_cache(blk, !writethrough);
-- 
1.8.3.1

[Qemu-devel] [PULL 05/13] vpc: fix return value check for blk_pwrite

[Kevin Wolf]

From: Paolo Bonzini 

bdrv_pwrite_sync used to return zero or negative error, while blk_pwrite returns
the number of written bytes when successful.  This caused VPC image creation
to fail spectacularly: it wrote the first 512 bytes, and then exited immediately
because of the non-zero answer from blk_pwrite.  But the truly spectacular part
is that it returns a positive value (the 512 that blk_pwrite returned) causing
everyone to believe that it succeeded.

This fixes qemu-iotests with vpc format.

Fixes: b8f45cdf7827e39f9a1e6cc446f5972cc6144237
Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block/vpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 8830b5b..3713ec3 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -775,7 +775,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t 
*buf,
 num_bat_entries = (total_sectors + block_size / 512) / (block_size / 512);
 
 ret = blk_pwrite(blk, offset, buf, HEADER_SIZE);
-if (ret) {
+if (ret < 0) {
 goto fail;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 00/13] Block patches for 2.6

[Kevin Wolf]

The following changes since commit 42bb626f7ebc9197d2943b897a99e127315275ab:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2016-04-12 09:34:52 +0100)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 5158ac58306b3e8644fc020730f651fe74aa6674:

  Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-04-12' 
into queue-block (2016-04-12 18:09:16 +0200)



Block layer patches for 2.6


Daniel P. Berrange (2):
  qemu-img: fix formatting of error message
  block: initialize qcrypto API at startup

Kevin Wolf (1):
  Merge remote-tracking branch 
'mreitz/tags/pull-block-for-kevin-2016-04-12' into queue-block

Max Reitz (3):
  iotests: Make 150 use qemu-img map instead of du
  qcow2: Prevent backing file names longer than 1023
  MAINTAINERS: Block layer core, qcow2 and blkdebug

Paolo Bonzini (1):
  vpc: fix return value check for blk_pwrite

Pavel Butsykin (1):
  iotests: fix the broken 026.nocache output

Sascha Silbe (6):
  qemu-iotests: check: don't place files with predictable names in /tmp
  qemu-iotests: fix 051 on non-PC architectures
  qemu-iotests: iotests.VM: remove qtest socket on error
  qemu-iotests: 148: properly skip test if quorum support is missing
  qemu-iotests: 068: don't require KVM
  qemu-iotests: iotests.py: get rid of __all__

 MAINTAINERS|   3 +
 block/qcow2.c  |   4 ++
 block/vpc.c|   2 +-
 qemu-img.c |   8 ++-
 qemu-io.c  |   6 ++
 qemu-nbd.c |   7 ++
 tests/qemu-iotests/026.out.nocache | 142 -
 tests/qemu-iotests/051.out |  10 +--
 tests/qemu-iotests/068 |   2 +-
 tests/qemu-iotests/148 |   4 +-
 tests/qemu-iotests/150 |  41 ++-
 tests/qemu-iotests/150.out |  13 ++--
 tests/qemu-iotests/check   |  21 +++---
 tests/qemu-iotests/iotests.py  |  22 --
 14 files changed, 196 insertions(+), 89 deletions(-)

Re: [Qemu-devel] [PATCH] docs/atomics.txt: Update pointer to linux macro

[Pranith Kumar]

On Tue, Apr 12, 2016 at 7:42 AM, Marc-André Lureau
 wrote:
> Hi
>
> On Mon, Apr 11, 2016 at 7:30 PM, Pranith Kumar  wrote:
>> Add a missing end brace and update doc to point to the latest access
>> macro. ACCESS_ONE() is deprecated.
>
> ONE/ONCE

Right, I missed this one. Should I fix and send a new patch?

Thanks,
-- 
Pranith

Re: [Qemu-devel] [PATCH v2 12/13] intel_iommu: ioapic: IR support for emulated IOAPIC

[Jan Kiszka]

On 2016-04-12 02:02, Peter Xu wrote:
> On Mon, Apr 11, 2016 at 10:22:18PM -0700, Jan Kiszka wrote:
>> On 2016-04-11 02:19, Peter Xu wrote:
>>> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
>>> index 378e663..d963d45 100644
>>> --- a/hw/intc/ioapic.c
>>> +++ b/hw/intc/ioapic.c
>>> @@ -57,6 +57,8 @@ static void ioapic_service(IOAPICCommonState *s)
>>>  uint64_t entry;
>>>  uint8_t dest;
>>>  uint8_t dest_mode;
>>> +IntelIOMMUState *iommu = s->iommu;
>>> +VTDIrq irq = {0};
>>>  
>>>  for (i = 0; i < IOAPIC_NUM_PINS; i++) {
>>>  mask = 1 << i;
>>> @@ -65,11 +67,33 @@ static void ioapic_service(IOAPICCommonState *s)
>>>  
>>>  entry = s->ioredtbl[i];
>>>  if (!(entry & IOAPIC_LVT_MASKED)) {
>>> -trig_mode = ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1);
>>> -dest = entry >> IOAPIC_LVT_DEST_SHIFT;
>>> -dest_mode = (entry >> IOAPIC_LVT_DEST_MODE_SHIFT) & 1;
>>> -delivery_mode =
>>> -(entry >> IOAPIC_LVT_DELIV_MODE_SHIFT) & 
>>> IOAPIC_DM_MASK;
>>> +
>>> +if (iommu && iommu->intr_enabled) {
>>> +/*
>>> +* Interrupt remapping is enabled in owner IOMMU,
>>> +* we need to fetch the real IRQ information via
>>> +* IRTE of the root mapping table
>>> +*/
>>> +if (vtd_interrupt_remap_ioapic(iommu, , )) {
>>
>> VT-d is only one of the possible IOMMUs on x86. Please introduce a
>> generic interface.
>>
>> Look at Rita's and my patches: they translate the IOAPIC (and HPET...)
>> interrupts into MSI messages that are then - in a generic way -
>> intercepted by the respective IOMMU or directly dispatched to the APICs.
>> We may no longer need new memory regions for this, thanks to the region
>> attributes, but we also need no hard-coded hooks here.
> 
> Yes, I should consider other x86 platforms like AMD. Thanks to point
> out. It seems that there are many places in the patchset that lacks
> thorough consideration about this. Will try to fix them in next
> version.
> 
> Regarding to the above MSI solution: I'd say it is a good way to
> hide everything else behind.  However, since we introduced one extra
> layer (MSI) which actually does not exist, not sure there would be
> problem too.  Also, I feel it a little bit hacky if we "create" one
> MSI out of the air...  For example, if someone tries to capture MSIs
> from QEMU inside in the APIC memory writes, he will see something he
> cannot explain if he never knows this hack's there.  Considering the
> above, I would prefer hooks, or better to provide a callback (a
> function pointer that others like AMD can override) to do the
> translation.  How do you think?

The HPET does send MSIs, and I'm not sure how much different the
IOAPIC's message actually is. In any case, modelling it as MSI is
neither adding incorrectness nor making the code more complex (in fact,
the contrary is true!). Last but not least, it would be trivial to
filter out non-PCI MSI sources if we wanted to trace only PCI - because
we need to identify the origin anyway for remapping purposes. So,
explicit hooking looks like the wrong way to me.

Jan

Re: [Qemu-devel] [PATCHv9] Improve documentation for TLS

[Eric Blake]

On 04/12/2016 07:27 AM, Alex Bligh wrote:
> * Call out TLS into a separate section
> 
> * Add details of the TLS protocol itself
> 
> * Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
>   be initiated from either side (as required by the TLS standard I believe
>   and as actually works in practice)
> 
> * Clarify what is a requirement on servers, and what is a requirement on
>   clients, separately, specifying their behaviour in a single place
>   in the document.
> 
> * Document the three possible modes of operation of a server.
> 
> * Add text defining what 'terminate the session' means during
>   negotiation, and when it is available.
> 
> Signed-off-by: Alex Bligh 
> ---

> + SELECTIVETLS mode
> +

> +
> +There is a degenerate case of SELECTIVETLS where all
> +exports are TLS-only. This is permitted in part to make programming
> +of servers easier. Operation is a little different from FORCEDTLS,
> +as the client is not forced to upgrade to TLS prior to any options
> +being processed, and the server MAY choose to give information on
> +non-existent exports via NBD_OPT_INFO exports prior to an upgrade

s/exports prior/responses/

> +to TLS.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCHv2 for-2.6] ivshmem: fix ivshmem-{plain, doorbell} crash without arg

[marcandre . lureau]

From: Marc-André Lureau 

"qemu -device ivshmem-{plain,doorbell}" will crash, because the device
doesn't check that the required argument is provided.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 2eb8668..e40f23b 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -872,6 +872,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
**errp)
 s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem,
  _abort);
 } else {
+assert(s->server_chr);
+
 IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
 s->server_chr->filename);
 
@@ -1051,10 +1053,24 @@ static void ivshmem_plain_init(Object *obj)
  _abort);
 }
 
+static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
+{
+IVShmemState *s = IVSHMEM_COMMON(dev);
+
+if (!s->hostmem) {
+error_setg(errp, "You must specify a 'memdev'");
+return;
+}
+
+ivshmem_common_realize(dev, errp);
+}
+
 static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+k->realize = ivshmem_plain_realize;
 dc->props = ivshmem_plain_properties;
 dc->vmsd = _plain_vmsd;
 }
@@ -1099,10 +1115,24 @@ static void ivshmem_doorbell_init(Object *obj)
 s->legacy_size = SIZE_MAX;  /* whatever the server sends */
 }
 
+static void ivshmem_doorbell_realize(PCIDevice *dev, Error **errp)
+{
+IVShmemState *s = IVSHMEM_COMMON(dev);
+
+if (!s->server_chr) {
+error_setg(errp, "You must specify a 'chardev'");
+return;
+}
+
+ivshmem_common_realize(dev, errp);
+}
+
 static void ivshmem_doorbell_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+k->realize = ivshmem_doorbell_realize;
 dc->props = ivshmem_doorbell_properties;
 dc->vmsd = _doorbell_vmsd;
 }
-- 
2.5.5

[Qemu-devel] [PATCH 3/4] qemu-iotests: tests: do not set unused tmp variable

[Sascha Silbe]

The previous commit removed the last usage of ${tmp} inside the tests
themselves; the only remaining users are sourced by check. So we can now
drop this variable from the tests.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
---
 tests/qemu-iotests/001 | 1 -
 tests/qemu-iotests/002 | 1 -
 tests/qemu-iotests/003 | 1 -
 tests/qemu-iotests/004 | 1 -
 tests/qemu-iotests/005 | 1 -
 tests/qemu-iotests/007 | 1 -
 tests/qemu-iotests/008 | 1 -
 tests/qemu-iotests/009 | 1 -
 tests/qemu-iotests/010 | 1 -
 tests/qemu-iotests/011 | 1 -
 tests/qemu-iotests/012 | 1 -
 tests/qemu-iotests/013 | 1 -
 tests/qemu-iotests/014 | 1 -
 tests/qemu-iotests/015 | 1 -
 tests/qemu-iotests/017 | 1 -
 tests/qemu-iotests/018 | 1 -
 tests/qemu-iotests/019 | 1 -
 tests/qemu-iotests/020 | 1 -
 tests/qemu-iotests/021 | 1 -
 tests/qemu-iotests/022 | 1 -
 tests/qemu-iotests/023 | 1 -
 tests/qemu-iotests/024 | 1 -
 tests/qemu-iotests/025 | 1 -
 tests/qemu-iotests/026 | 1 -
 tests/qemu-iotests/027 | 1 -
 tests/qemu-iotests/028 | 1 -
 tests/qemu-iotests/029 | 1 -
 tests/qemu-iotests/031 | 1 -
 tests/qemu-iotests/032 | 1 -
 tests/qemu-iotests/033 | 1 -
 tests/qemu-iotests/034 | 1 -
 tests/qemu-iotests/035 | 1 -
 tests/qemu-iotests/036 | 1 -
 tests/qemu-iotests/037 | 1 -
 tests/qemu-iotests/038 | 1 -
 tests/qemu-iotests/039 | 1 -
 tests/qemu-iotests/042 | 1 -
 tests/qemu-iotests/043 | 1 -
 tests/qemu-iotests/046 | 1 -
 tests/qemu-iotests/047 | 1 -
 tests/qemu-iotests/049 | 1 -
 tests/qemu-iotests/050 | 1 -
 tests/qemu-iotests/051 | 1 -
 tests/qemu-iotests/052 | 1 -
 tests/qemu-iotests/053 | 1 -
 tests/qemu-iotests/054 | 1 -
 tests/qemu-iotests/058 | 1 -
 tests/qemu-iotests/059 | 1 -
 tests/qemu-iotests/060 | 1 -
 tests/qemu-iotests/061 | 1 -
 tests/qemu-iotests/062 | 1 -
 tests/qemu-iotests/063 | 1 -
 tests/qemu-iotests/064 | 1 -
 tests/qemu-iotests/066 | 1 -
 tests/qemu-iotests/067 | 1 -
 tests/qemu-iotests/068 | 1 -
 tests/qemu-iotests/069 | 1 -
 tests/qemu-iotests/070 | 1 -
 tests/qemu-iotests/071 | 1 -
 tests/qemu-iotests/072 | 1 -
 tests/qemu-iotests/073 | 1 -
 tests/qemu-iotests/075 | 1 -
 tests/qemu-iotests/076 | 1 -
 tests/qemu-iotests/077 | 1 -
 tests/qemu-iotests/078 | 1 -
 tests/qemu-iotests/079 | 1 -
 tests/qemu-iotests/080 | 1 -
 tests/qemu-iotests/081 | 1 -
 tests/qemu-iotests/082 | 1 -
 tests/qemu-iotests/083 | 1 -
 tests/qemu-iotests/084 | 1 -
 tests/qemu-iotests/086 | 1 -
 tests/qemu-iotests/087 | 1 -
 tests/qemu-iotests/088 | 1 -
 tests/qemu-iotests/089 | 1 -
 tests/qemu-iotests/090 | 1 -
 tests/qemu-iotests/092 | 1 -
 tests/qemu-iotests/094 | 1 -
 tests/qemu-iotests/097 | 1 -
 tests/qemu-iotests/098 | 1 -
 tests/qemu-iotests/099 | 1 -
 tests/qemu-iotests/100 | 1 -
 tests/qemu-iotests/101 | 1 -
 tests/qemu-iotests/102 | 1 -
 tests/qemu-iotests/103 | 1 -
 tests/qemu-iotests/104 | 1 -
 tests/qemu-iotests/105 | 1 -
 tests/qemu-iotests/107 | 1 -
 tests/qemu-iotests/108 | 1 -
 tests/qemu-iotests/109 | 1 -
 tests/qemu-iotests/110 | 1 -
 tests/qemu-iotests/111 | 1 -
 tests/qemu-iotests/112 | 1 -
 tests/qemu-iotests/113 | 1 -
 tests/qemu-iotests/114 | 1 -
 tests/qemu-iotests/115 | 1 -
 tests/qemu-iotests/116 | 1 -
 tests/qemu-iotests/117 | 1 -
 tests/qemu-iotests/119 | 1 -
 tests/qemu-iotests/120 | 1 -
 tests/qemu-iotests/121 | 1 -
 tests/qemu-iotests/122 | 1 -
 tests/qemu-iotests/123 | 1 -
 tests/qemu-iotests/128 | 1 -
 tests/qemu-iotests/130 | 1 -
 tests/qemu-iotests/131 | 1 -
 tests/qemu-iotests/133 | 1 -
 tests/qemu-iotests/134 | 1 -
 tests/qemu-iotests/135 | 1 -
 tests/qemu-iotests/137 | 1 -
 tests/qemu-iotests/138 | 1 -
 tests/qemu-iotests/140 | 1 -
 tests/qemu-iotests/141 | 1 -
 tests/qemu-iotests/142 | 1 -
 tests/qemu-iotests/143 | 1 -
 tests/qemu-iotests/145 | 1 -
 tests/qemu-iotests/150 | 1 -
 117 files changed, 117 deletions(-)

diff --git a/tests/qemu-iotests/001 b/tests/qemu-iotests/001
index 4e16469..ffd14e2 100755
--- a/tests/qemu-iotests/001
+++ b/tests/qemu-iotests/001
@@ -25,7 +25,6 @@ seq=`basename $0`
 echo "QA output created by $seq"
 
 here=`pwd`
-tmp=/tmp/$$
 status=1   # failure is the default!
 
 _cleanup()
diff --git a/tests/qemu-iotests/002 b/tests/qemu-iotests/002
index 6a865aa..d4f8e91 100755
--- a/tests/qemu-iotests/002
+++ b/tests/qemu-iotests/002
@@ -25,7 +25,6 @@ seq=`basename $0`
 echo "QA output created by $seq"
 
 here=`pwd`
-tmp=/tmp/$$
 status=1   # failure is the default!
 
 _cleanup()
diff --git a/tests/qemu-iotests/003 b/tests/qemu-iotests/003
index 98638d4..19889b9 100755
--- a/tests/qemu-iotests/003
+++ b/tests/qemu-iotests/003
@@ -25,7 +25,6 @@ seq=`basename $0`
 echo "QA output created by $seq"
 
 here=`pwd`
-tmp=/tmp/$$
 status=1   # failure is the default!
 
 _cleanup()
diff --git a/tests/qemu-iotests/004 b/tests/qemu-iotests/004
index 2ad77ed..67e1beb 100755
--- a/tests/qemu-iotests/004
+++ b/tests/qemu-iotests/004
@@ -25,7 +25,6 @@ seq=`basename $0`
 echo "QA output created by $seq"
 
 here=`pwd`
-tmp=/tmp/$$
 status=1   

[Qemu-devel] [PATCH] iSCSI: start moving options also for -drive

[Pino Toscano]

Hi,

to overcome the limitations of the options handling [1], I'm planning
to move more options for iSCSI also as block options, so it is possible
to specify them with -drive.

The only patch in this series is for initiator-target, as I want to be
sure the approach is correct, and wanted.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg06501.html

Thanks,

Pino Toscano (1):
  iscsi: allow "initiator-name" as block option

 block/iscsi.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH for-2.6 0/4] qemu-iotests: don't use /tmp

[Sascha Silbe]

During review of my other qemu-iotests fixes, Max Reitz noticed a
couple of additional places where qemu-iotests hardcode /tmp. This is
both a security issue and cumbersome when running multiple instances
of qemu-iotests (e.g. different users on a shared development machine
like tuxmaker). Fix them up.

Checked (/var)/tmp usage using "sudo chown 000 /tmp /var/tmp". "tac"
and "qemu -snapshot" fail in that case. I'll presume they create the
temporary files in a secure manner.

There is one place left that may use /var/tmp in theory; will address
that in a separate patch.

This series mostly removes dead code and addresses a potential
security issue, all of that in the test suite rather than in
production code. It should be applicable even during hard freeze.
 

Sascha Silbe (4):
  qemu-iotests: drop unused _within_tolerance() filter
  qemu-iotests: common.rc: drop unused _do()
  qemu-iotests: tests: do not set unused tmp variable
  qemu-iotests: place valgrind log file in scratch dir

 tests/qemu-iotests/001   |   1 -
 tests/qemu-iotests/002   |   1 -
 tests/qemu-iotests/003   |   1 -
 tests/qemu-iotests/004   |   1 -
 tests/qemu-iotests/005   |   1 -
 tests/qemu-iotests/007   |   1 -
 tests/qemu-iotests/008   |   1 -
 tests/qemu-iotests/009   |   1 -
 tests/qemu-iotests/010   |   1 -
 tests/qemu-iotests/011   |   1 -
 tests/qemu-iotests/012   |   1 -
 tests/qemu-iotests/013   |   1 -
 tests/qemu-iotests/014   |   1 -
 tests/qemu-iotests/015   |   1 -
 tests/qemu-iotests/017   |   1 -
 tests/qemu-iotests/018   |   1 -
 tests/qemu-iotests/019   |   1 -
 tests/qemu-iotests/020   |   1 -
 tests/qemu-iotests/021   |   1 -
 tests/qemu-iotests/022   |   1 -
 tests/qemu-iotests/023   |   1 -
 tests/qemu-iotests/024   |   1 -
 tests/qemu-iotests/025   |   1 -
 tests/qemu-iotests/026   |   1 -
 tests/qemu-iotests/027   |   1 -
 tests/qemu-iotests/028   |   1 -
 tests/qemu-iotests/029   |   1 -
 tests/qemu-iotests/031   |   1 -
 tests/qemu-iotests/032   |   1 -
 tests/qemu-iotests/033   |   1 -
 tests/qemu-iotests/034   |   1 -
 tests/qemu-iotests/035   |   1 -
 tests/qemu-iotests/036   |   1 -
 tests/qemu-iotests/037   |   1 -
 tests/qemu-iotests/038   |   1 -
 tests/qemu-iotests/039   |   1 -
 tests/qemu-iotests/042   |   1 -
 tests/qemu-iotests/043   |   1 -
 tests/qemu-iotests/046   |   1 -
 tests/qemu-iotests/047   |   1 -
 tests/qemu-iotests/049   |   1 -
 tests/qemu-iotests/050   |   1 -
 tests/qemu-iotests/051   |   1 -
 tests/qemu-iotests/052   |   1 -
 tests/qemu-iotests/053   |   1 -
 tests/qemu-iotests/054   |   1 -
 tests/qemu-iotests/058   |   1 -
 tests/qemu-iotests/059   |   1 -
 tests/qemu-iotests/060   |   1 -
 tests/qemu-iotests/061   |   1 -
 tests/qemu-iotests/062   |   1 -
 tests/qemu-iotests/063   |   1 -
 tests/qemu-iotests/064   |   1 -
 tests/qemu-iotests/066   |   1 -
 tests/qemu-iotests/067   |   1 -
 tests/qemu-iotests/068   |   1 -
 tests/qemu-iotests/069   |   1 -
 tests/qemu-iotests/070   |   1 -
 tests/qemu-iotests/071   |   1 -
 tests/qemu-iotests/072   |   1 -
 tests/qemu-iotests/073   |   1 -
 tests/qemu-iotests/075   |   1 -
 tests/qemu-iotests/076   |   1 -
 tests/qemu-iotests/077   |   1 -
 tests/qemu-iotests/078   |   1 -
 tests/qemu-iotests/079   |   1 -
 tests/qemu-iotests/080   |   1 -
 tests/qemu-iotests/081   |   1 -
 tests/qemu-iotests/082   |   1 -
 tests/qemu-iotests/083   |   1 -
 tests/qemu-iotests/084   |   1 -
 tests/qemu-iotests/086   |   1 -
 tests/qemu-iotests/087   |   1 -
 tests/qemu-iotests/088   |   1 -
 tests/qemu-iotests/089   |   1 -
 tests/qemu-iotests/090   |   1 -
 tests/qemu-iotests/092   |   1 -
 tests/qemu-iotests/094   |   1 -
 tests/qemu-iotests/097   |   1 -
 tests/qemu-iotests/098   |   1 -
 tests/qemu-iotests/099   |   1 -
 tests/qemu-iotests/100   |   1 -
 tests/qemu-iotests/101   |   1 -
 tests/qemu-iotests/102   |   1 -
 tests/qemu-iotests/103   |   1 -
 tests/qemu-iotests/104   |   1 -
 tests/qemu-iotests/105   |   1 -
 tests/qemu-iotests/107   |   1 -
 tests/qemu-iotests/108   |   1 -
 tests/qemu-iotests/109   |   1 -
 tests/qemu-iotests/110   |   1 -
 tests/qemu-iotests/111   |   1 -
 tests/qemu-iotests/112   |   1 -
 tests/qemu-iotests/113   |   1 -
 tests/qemu-iotests/114   |   1 -
 

[Qemu-devel] [PATCH 4/4] qemu-iotests: place valgrind log file in scratch dir

[Sascha Silbe]

Do not place the valgrind log file at a predictable path in a
world-writable location. Use the common scratch directory (${TEST_DIR})
instead.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
---
 tests/qemu-iotests/common.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 60bfabf..f824651 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -122,7 +122,7 @@ _qemu_img_wrapper()
 
 _qemu_io_wrapper()
 {
-local VALGRIND_LOGFILE=/tmp/$$.valgrind
+local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
 local RETVAL
 (
 if [ "${VALGRIND_QEMU}" == "y" ]; then
-- 
1.9.1

[Qemu-devel] [PATCH] iscsi: allow "initiator-name" as block option

[Pino Toscano]

Allow the "initiator-name" for both the -iscsi and the block options:
this way it is possible to set it directly as option in the -drive
specification.
The current way to specify the initiator name for a certain iSCSI
target is:
  -iscsi id=TARGET,initiator-name=IQN
which cannot be actually done when TARGET has the optional part, as
colon is not accepted as id for QemuOpts [1].

Hence, allow the "initiator-name" also in block options: this way
it is possible to set it directly as option in -drive, e.g.:
  -drive file=URI,driver=iscsi,initiator-name=IQN

[1] http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg06501.html

Signed-off-by: Pino Toscano 
---
 block/iscsi.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 302baf8..4a1c300 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1161,7 +1161,7 @@ static void parse_header_digest(struct iscsi_context 
*iscsi, const char *target,
 }
 }
 
-static char *parse_initiator_name(const char *target)
+static char *parse_initiator_name(QDict *options, const char *target)
 {
 QemuOptsList *list;
 QemuOpts *opts;
@@ -1169,6 +1169,11 @@ static char *parse_initiator_name(const char *target)
 char *iscsi_name;
 UuidInfo *uuid_info;
 
+name = qdict_get_try_str(options, "initiator-name");
+if (name != NULL) {
+return g_strdup(name);
+}
+
 list = qemu_find_opts("iscsi");
 if (list) {
 opts = qemu_opts_find(list, target);
@@ -1304,11 +1309,19 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, 
Error **errp)
 }
 }
 
+#define COMMON_ISCSI_OPTS \
+{ \
+.name = "initiator-name", \
+.type = QEMU_OPT_STRING, \
+.help = "Initiator iqn name to use when connecting", \
+}
+
 /* TODO Convert to fine grained options */
 static QemuOptsList runtime_opts = {
 .name = "iscsi",
 .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
 .desc = {
+COMMON_ISCSI_OPTS,
 {
 .name = "filename",
 .type = QEMU_OPT_STRING,
@@ -1473,7 +1486,7 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 memset(iscsilun, 0, sizeof(IscsiLun));
 
-initiator_name = parse_initiator_name(iscsi_url->target);
+initiator_name = parse_initiator_name(bs->options, iscsi_url->target);
 
 iscsi = iscsi_create_context(initiator_name);
 if (iscsi == NULL) {
@@ -1864,6 +1877,7 @@ static QemuOptsList qemu_iscsi_opts = {
 .name = "iscsi",
 .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
 .desc = {
+COMMON_ISCSI_OPTS,
 {
 .name = "user",
 .type = QEMU_OPT_STRING,
@@ -1883,10 +1897,6 @@ static QemuOptsList qemu_iscsi_opts = {
 .help = "HeaderDigest setting. "
 "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
 },{
-.name = "initiator-name",
-.type = QEMU_OPT_STRING,
-.help = "Initiator iqn name to use when connecting",
-},{
 .name = "timeout",
 .type = QEMU_OPT_NUMBER,
 .help = "Request timeout in seconds (default 0 = no timeout)",
-- 
2.5.5

[Qemu-devel] [PATCH 1/4] qemu-iotests: drop unused _within_tolerance() filter

[Sascha Silbe]

_within_tolerance() isn't used anymore and possibly creates temporary
files at predictable, world-writable locations. Get rid of it.

If it's needed again in the future it can be revived easily and fixed up
to use TEST_DIR and / or safely created temporary files.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
---
 tests/qemu-iotests/common.filter | 101 ---
 1 file changed, 101 deletions(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 84b7434..8a6e1b5 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -19,107 +19,6 @@
 # standard filters
 #
 
-# Checks that given_value is in range of correct_value +/- tolerance.
-# Tolerance can be an absolute value or a percentage of the correct value
-# (see examples with tolerances below).
-# Outputs suitable message to stdout if it's not in range.
-#
-# A verbose option, -v, may be used as the LAST argument
-#
-# e.g.
-# foo: 0.0298 = 0.03 +/- 5%
-# _within_tolerance "foo" 0.0298 0.03 5%
-#
-# foo: 0.0298 = 0.03 +/- 0.01
-# _within_tolerance "foo" 0.0298 0.03 0.01
-#
-# foo: 0.0298 = 0.03 -0.01 +0.002
-# _within_tolerance "foo" 0.0298 0.03 0.01 0.002
-#
-# foo: verbose output of 0.0298 = 0.03 +/- 5%
-# _within_tolerance "foo" 0.0298 0.03 5% -v
-_within_tolerance()
-{
-  _name=$1
-  _given_val=$2
-  _correct_val=$3
-  _mintol=$4
-  _maxtol=$_mintol
-  _verbose=0
-  _debug=false
-
-  # maxtol arg is optional
-  # verbose arg is optional
-  if [ $# -ge 5 ]
-  then
- if [ "$5" = "-v" ]
- then
-_verbose=1
- else
-_maxtol=$5
- fi
-  fi
-  if [ $# -ge 6 ]
-  then
- [ "$6" = "-v" ] && _verbose=1
-  fi
-
-  # find min with or without %
-  _mintolerance=`echo $_mintol | sed -e 's/%//'`
-  if [ $_mintol = $_mintolerance ]
-  then
-  _min=`echo "scale=5; $_correct_val-$_mintolerance" | bc`
-  else
-  _min=`echo "scale=5; $_correct_val-$_mintolerance*0.01*$_correct_val" | 
bc`
-  fi
-
-  # find max with or without %
-  _maxtolerance=`echo $_maxtol | sed -e 's/%//'`
-  if [ $_maxtol = $_maxtolerance ]
-  then
-  _max=`echo "scale=5; $_correct_val+$_maxtolerance" | bc`
-  else
-  _max=`echo "scale=5; $_correct_val+$_maxtolerance*0.01*$_correct_val" | 
bc`
-  fi
-
-  $_debug && echo "min = $_min"
-  $_debug && echo "max = $_max"
-
-  cat <$tmp.bc.1
-scale=5;
-if ($_min <= $_given_val) 1;
-if ($_min > $_given_val) 0;
-EOF
-
-  cat <$tmp.bc.2
-scale=5;
-if ($_given_val <= $_max) 1;
-if ($_given_val > $_max) 0;
-EOF
-
-  _above_min=`bc <$tmp.bc.1`
-  _below_max=`bc <$tmp.bc.2`
-
-  rm -f $tmp.bc.[12]
-
-  _in_range=`expr $_above_min \& $_below_max`
-
-  # fix up min, max precision for output
-  # can vary for 5.3, 6.2
-  _min=`echo $_min | sed -e 's/0*$//'` # get rid of trailling zeroes
-  _max=`echo $_max | sed -e 's/0*$//'` # get rid of trailling zeroes
-
-  if [ $_in_range -eq 1 ]
-  then
-[ $_verbose -eq 1 ] && echo $_name is in range
-return 0
-  else
-[ $_verbose -eq 1 ] && echo $_name has value of $_given_val
-[ $_verbose -eq 1 ] && echo $_name is NOT in range $_min .. $_max
-return 1
-  fi
-}
-
 # ctime(3) dates
 #
 _filter_date()
-- 
1.9.1

[Qemu-devel] [PATCH 2/4] qemu-iotests: common.rc: drop unused _do()

[Sascha Silbe]

_do() was never used and possibly creates temporary files at
predictable, world-writable locations. Get rid of it.

Signed-off-by: Sascha Silbe 
Reviewed-by: Bo Tu 
---
 tests/qemu-iotests/common.rc | 46 
 1 file changed, 46 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index d9913f8..5249ec5 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -287,52 +287,6 @@ _need_to_be_root()
 fi
 }
 
-
-# Do a command, log it to $seq.full, optionally test return status
-# and die if command fails. If called with one argument _do executes the
-# command, logs it, and returns its exit status. With two arguments _do
-# first prints the message passed in the first argument, and then "done"
-# or "fail" depending on the return status of the command passed in the
-# second argument. If the command fails and the variable _do_die_on_error
-# is set to "always" or the two argument form is used and _do_die_on_error
-# is set to "message_only" _do will print an error message to
-# $seq.out and exit.
-
-_do()
-{
-if [ $# -eq 1 ]; then
-_cmd=$1
-elif [ $# -eq 2 ]; then
-_note=$1
-_cmd=$2
-echo -n "$_note... "
-else
-echo "Usage: _do [note] cmd" 1>&2
-status=1; exit
-fi
-
-(eval "echo '---' \"$_cmd\"") >>"$OUTPUT_DIR/$seq.full"
-(eval "$_cmd") >$tmp._out 2>&1; ret=$?
-cat $tmp._out >>"$OUTPUT_DIR/$seq.full"
-if [ $# -eq 2 ]; then
-if [ $ret -eq 0 ]; then
-echo "done"
-else
-echo "fail"
-fi
-fi
-if [ $ret -ne 0  ] \
-&& [ "$_do_die_on_error" = "always" \
--o \( $# -eq 2 -a "$_do_die_on_error" = "message_only" \) ]
-then
-[ $# -ne 2 ] && echo
-eval "echo \"$_cmd\" failed \(returned $ret\): see $seq.full"
-status=1; exit
-fi
-
-return $ret
-}
-
 # bail out, setting up .notrun file
 #
 _notrun()
-- 
1.9.1

Re: [Qemu-devel] [PATCH] move const_le{16, 23} to qemu/bswap.h, add comment

[Eric Blake]

On 04/12/2016 12:07 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/hw/virtio/virtio-input.h | 14 --
>  include/qemu/bswap.h | 19 +++
>  2 files changed, 19 insertions(+), 14 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] virtio-input: add parenthesis to const_le{16, 32}

[Eric Blake]

On 04/11/2016 11:51 PM, Gerd Hoffmann wrote:
> "_x" must be "(_x)" otherwise things fail if you pass in expressions.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/hw/virtio/virtio-input.h | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Eric Blake 


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.6] ivshmem: fix crash with ivshmem-plain

[Markus Armbruster]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> "qemu -device ivshmem-plain" will crash, because it doesn't check that
> 'memdev' was provided.
>
> Signed-off-by: Marc-André Lureau 

Oww!

> ---
>  hw/misc/ivshmem.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 08bf5c6..fae4d08 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -868,7 +868,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
> **errp)
>  
>  s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem,
>   _abort);
> -} else {
> +} else if (s->server_chr) {

I'd simply stick

   assert(s->server_chr)

here.

>  IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
>  s->server_chr->filename);
>  
> @@ -899,6 +899,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
> **errp)
>  error_setg(errp, "failed to initialize interrupts");
>  return;
>  }
> +} else {
> +g_assert_not_reached();
>  }
>  
>  vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> @@ -1048,10 +1050,25 @@ static void ivshmem_plain_init(Object *obj)
>   _abort);
>  }
>  
> +static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
> +{
> +IVShmemState *s = IVSHMEM_COMMON(dev);
> +
> +if (!s->hostmem) {
> +error_setg(errp, "You must specify a 'memdev'");
> +return;
> +}
> +
> +ivshmem_common_realize(dev, errp);
> +}
> +
> +
>  static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> +k->realize = ivshmem_plain_realize;
>  dc->props = ivshmem_plain_properties;
>  dc->vmsd = _plain_vmsd;
>  }

Same bug in ivshmem-doorbell.  Care to fix that, too?

[Qemu-devel] windows displays an error msg each time i run qemu " qemu machine emulators and tools stop working "

[Safa Hamza]

hello
i configure qemu on windows using mingw and msys , and im trying to run it
so i use kernel zimage and a file system working perfectly on ubuntu
this is the command

export DTB="-dtb /vexpress-v2p-ca9.dtb"

./arm-softmmu/qemu-system-arm -M vexpress-a9 -kernel /zImage -initrd
/rootfs_affiche.img -append "root=/dev/mmcblk0 rw physmap.enabled=0
console=ttyAMA0  rdinit=/sbin/init " -serial stdio $DTB -cpu cortex-a9
each time i run this command , windows display a window msg  " qemu machine
emulators and tools stop working " and prevent qemu to run

any suggestion please
[image: Inline image 1]

Re: [Qemu-devel] [RFC v2 08/11] tcg: add options for enabling MTTCG

[Alex Bennée]

Sergey Fedorov  writes:

> On 05/04/16 18:32, Alex Bennée wrote:
>> diff --git a/cpus.c b/cpus.c
>> index 46732a5..8d27fb0 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>
> (snip)
>
>> @@ -146,6 +147,48 @@ typedef struct TimersState {
>>  } TimersState;
>>
>>  static TimersState timers_state;
>> +static bool mttcg_enabled;
>> +
>> +static QemuOptsList qemu_tcg_opts = {
>> +.name = "tcg",
>> +.head = QTAILQ_HEAD_INITIALIZER(qemu_tcg_opts.head),
>> +.desc = {
>> +{
>> +.name = "mttcg",
>> +.type = QEMU_OPT_BOOL,
>> +.help = "Enable/disable multi-threaded TCG",
>> +},
>> +{ /* end of list */ }
>> +},
>> +};
>> +
>> +static void tcg_register_config(void)
>> +{
>> +qemu_add_opts(_tcg_opts);
>> +}
>> +
>> +opts_init(tcg_register_config);
>> +
>> +static bool default_mttcg_enabled(void)
>> +{
>> +/*
>> + * TODO: Check if we have a chance to have MTTCG working on this 
>> guest/host.
>> + *   Basically is the atomic instruction implemented? Is there any
>> + *   memory ordering issue?
>> + */
>
> I think this could be decided in configure/makefiles.

I was think we might have other interactions, like if the user enabled
replay/playback mode. There is also an argument that by having the logic
in the code it's easier for developers to see the logic as people don't
generally grok Makefiles.

>
>> +return false;
>> +}
>> +
>> +void qemu_tcg_configure(QemuOpts *opts)
>> +{
>> +mttcg_enabled = qemu_opt_get_bool(opts, "mttcg", 
>> default_mttcg_enabled());
>> +}
>> +
>> +bool qemu_tcg_mttcg_enabled(void)
>> +{
>> +return mttcg_enabled;
>> +}
>> +
>>
>>  int64_t cpu_get_icount_raw(void)
>>  {
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 13eeaae..5e3826c 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -369,6 +369,20 @@ extern struct CPUTailQ cpus;
>>  extern __thread CPUState *current_cpu;
>>
>>  /**
>> + * qemu_tcg_enable_mttcg:
>> + * Enable the MultiThread TCG support.
>> + */
>> +void qemu_tcg_enable_mttcg(void);
>
> Seems to be an orphaned declaration.
>
> Kind regards,
> Sergey


--
Alex Bennée

Re: [Qemu-devel] [RFC v2 08/11] tcg: add options for enabling MTTCG

[Alex Bennée]

Pavel Dovgalyuk  writes:

>> From: mttcg-requ...@listserver.greensocs.com 
>> [mailto:mttcg-requ...@listserver.greensocs.com]
>> On 12/04/16 15:50, KONRAD Frederic wrote:
>> >> Yes, but users may like to have an option to disable MTTCG for some
>> >> reason. I'm also concerned about icount mode: not sure how to account
>> >> virtual time when all vCPUs run in parallel.
>> >
>> > I'm thinking the same, we don't have a solution for icount yet.
>> > The reverse execution support is probably badly broken as well.
>>
>> Reverse execution doesn't even seem to support more than a single core.
>> As of icount, it looks like to be incompatible with MTTCG.
>
> It doesn't, because there is no multicore support for tcg yet.
> I think, that we have to find some solution for icount (even if it
> will be slow).

Well one of the reasons I'm keeping single thread behaviour is so things
like icount/replay support can continue to work. The two options will be
incompatible to start with.

I'm also expecting people will want to rule out mttcg as a problem at
least in the early days.

--
Alex Bennée

Re: [Qemu-devel] [Xen-devel] [PATCH v2 2/3] xenfb: move xen_rmb to the correct location

[Wei Liu]

On Tue, Apr 12, 2016 at 02:38:13PM +0100, Andrew Cooper wrote:
> On 12/04/16 13:57, David Vrabel wrote:
> > On 12/04/16 11:43, Wei Liu wrote:
> >> It should be placed before first time producer and consumer are used.
> > This change isn't necessary and is confusing as this is not what this
> > barrier is for.
> >
> > The barrier needs to be between the load of prod and the load of the
> > ring contents (there's even a comment that says this).  This pairs with
> > the corresponding write barrier between the store of the ring contents
> > and the store of prod (in the other end).
> 
> Looking further, this code will compile to multiple reads of the page,
> because there is no ACCESS_ONCE().  This code is still vulnerable to
> XSA-155.
> 

Oops, accidentally kicked over a can of worms. Should have just sent
patch 1. :-)

Jokes aside, more time is needed to fix this properly. So maybe we
should just upstream patch #1 first. Stefano? Anthony?

Wei.

Re: [Qemu-devel] [Xen-devel] [PATCH v2 2/3] xenfb: move xen_rmb to the correct location

[Andrew Cooper]

On 12/04/16 13:57, David Vrabel wrote:
> On 12/04/16 11:43, Wei Liu wrote:
>> It should be placed before first time producer and consumer are used.
> This change isn't necessary and is confusing as this is not what this
> barrier is for.
>
> The barrier needs to be between the load of prod and the load of the
> ring contents (there's even a comment that says this).  This pairs with
> the corresponding write barrier between the store of the ring contents
> and the store of prod (in the other end).

Looking further, this code will compile to multiple reads of the page,
because there is no ACCESS_ONCE().  This code is still vulnerable to
XSA-155.

~Andrew

>
> David
>
>> Signed-off-by: Wei Liu 
>> ---
>> Cc: Stefano Stabellini 
>> Cc: Anthony Perard 
>>
>> Backport candidate to our own tree.
>> ---
>>  hw/display/xenfb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
>> index 9866dfd..7f4fad7 100644
>> --- a/hw/display/xenfb.c
>> +++ b/hw/display/xenfb.c
>> @@ -775,10 +775,10 @@ static void xenfb_handle_events(struct XenFB *xenfb)
>>  
>>  prod = page->out_prod;
>>  out_cons = page->out_cons;
>> +xen_rmb();
>>  if (prod - out_cons > XENFB_OUT_RING_LEN) {
>>  return;
>>  }
>> -xen_rmb();  /* ensure we see ring contents up to prod */
>>  for (cons = out_cons; cons != prod; cons++) {
>>  union xenfb_out_event *event = _OUT_RING_REF(page, cons);
>>  uint8_t type = event->type;
>>
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel

[Qemu-devel] [PATCH v2 REPOST 1/2] Add dynamic module loading for block drivers

[Richard W.M. Jones]

From: Marc Marí 

Extend the current module interface to allow for block drivers to be loaded
dynamically on request.

The only block drivers that can be converted into modules are the drivers
that don't perform any init operation except for registering themselves. This
is why libiscsi has been disabled as a module.

All the necessary module information is located in a new structure found in
include/qemu/module_block.h

Signed-off-by: Marc Marí 
---
 block.c | 70 +++
 configure   |  2 +-
 include/qemu/module.h   |  3 ++
 include/qemu/module_block.h | 90 +
 util/module.c   | 38 ++-
 5 files changed, 175 insertions(+), 28 deletions(-)
 create mode 100644 include/qemu/module_block.h

diff --git a/block.c b/block.c
index d4939b4..ccd9e57 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/error-report.h"
+#include "qemu/module_block.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
@@ -252,11 +253,30 @@ BlockDriverState *bdrv_new(void)
 BlockDriver *bdrv_find_format(const char *format_name)
 {
 BlockDriver *drv1;
+size_t i;
+
 QLIST_FOREACH(drv1, _drivers, list) {
 if (!strcmp(drv1->format_name, format_name)) {
 return drv1;
 }
 }
+
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (!strcmp(block_driver_modules[i].format_name, format_name)) {
+block_module_load_one(block_driver_modules[i].library_name);
+/* Copying code is not nice, but this way the current discovery is
+ * not modified. Calling recursively could fail if the library
+ * has been deleted.
+ */
+QLIST_FOREACH(drv1, _drivers, list) {
+if (!strcmp(drv1->format_name, format_name)) {
+return drv1;
+}
+}
+}
+}
+
+
 return NULL;
 }
 
@@ -457,8 +477,15 @@ int get_tmp_filename(char *filename, int size)
 static BlockDriver *find_hdev_driver(const char *filename)
 {
 int score_max = 0, score;
+size_t i;
 BlockDriver *drv = NULL, *d;
 
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (block_driver_modules[i].has_probe_device) {
+block_module_load_one(block_driver_modules[i].library_name);
+}
+}
+
 QLIST_FOREACH(d, _drivers, list) {
 if (d->bdrv_probe_device) {
 score = d->bdrv_probe_device(filename);
@@ -480,6 +507,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 char protocol[128];
 int len;
 const char *p;
+size_t i;
 
 /* TODO Drivers without bdrv_file_open must be specified explicitly */
 
@@ -506,6 +534,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 len = sizeof(protocol) - 1;
 memcpy(protocol, filename, len);
 protocol[len] = '\0';
+
 QLIST_FOREACH(drv1, _drivers, list) {
 if (drv1->protocol_name &&
 !strcmp(drv1->protocol_name, protocol)) {
@@ -513,6 +542,23 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 }
 }
 
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (block_driver_modules[i].protocol_name &&
+!strcmp(block_driver_modules[i].protocol_name, protocol)) {
+block_module_load_one(block_driver_modules[i].library_name);
+/* Copying code is not nice, but this way the current discovery is
+ * not modified. Calling recursively could fail if the library
+ * has been deleted.
+ */
+QLIST_FOREACH(drv1, _drivers, list) {
+if (drv1->protocol_name &&
+!strcmp(drv1->protocol_name, protocol)) {
+return drv1;
+}
+}
+}
+}
+
 error_setg(errp, "Unknown protocol '%s'", protocol);
 return NULL;
 }
@@ -535,8 +581,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int 
buf_size,
 const char *filename)
 {
 int score_max = 0, score;
+size_t i;
 BlockDriver *drv = NULL, *d;
 
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (block_driver_modules[i].has_probe) {
+block_module_load_one(block_driver_modules[i].library_name);
+}
+}
+
 QLIST_FOREACH(d, _drivers, list) {
 if (d->bdrv_probe) {
 score = d->bdrv_probe(buf, buf_size, filename);
@@ -2794,6 +2847,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
char *name),
 BlockDriver *drv;
 int count = 0;
 int i;
+size_t n;
 const char **formats = NULL;
 
 QLIST_FOREACH(drv, _drivers, list) {
@@ -2811,6 +2865,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 

[Qemu-devel] [PATCH v2 REPOST 2/2] Add dynamic generation of module_block.h

[Richard W.M. Jones]

From: Marc Marí 

To simplify the addition of new block modules, add a script that generates
include/qemu/module_block.h automatically from the modules' source code.

This script assumes that the QEMU coding style rules are followed.

Signed-off-by: Marc Marí 
---
 .gitignore  |   1 +
 Makefile|  10 ++-
 include/qemu/module_block.h |  90 ---
 scripts/modules/module_block.py | 134 
 4 files changed, 142 insertions(+), 93 deletions(-)
 delete mode 100644 include/qemu/module_block.h
 create mode 100644 scripts/modules/module_block.py

diff --git a/.gitignore b/.gitignore
index 88a80ff..e87b09a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,3 +109,4 @@ cscope.*
 tags
 TAGS
 *~
+/include/qemu/module_block.h
diff --git a/Makefile b/Makefile
index 1d076a9..27bde8b 100644
--- a/Makefile
+++ b/Makefile
@@ -75,6 +75,8 @@ GENERATED_HEADERS += trace/generated-ust-provider.h
 GENERATED_SOURCES += trace/generated-ust.c
 endif
 
+GENERATED_HEADERS += include/qemu/module_block.h
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -227,9 +229,6 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 libqemustub.a: $(stub-obj-y)
 libqemuutil.a: $(util-obj-y)
 
-block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
-util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
-
 ##
 
 qemu-img.o: qemu-img-cmds.h
@@ -334,6 +333,11 @@ ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) 
libqemuutil.a libqemustub.a
 ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+include/qemu/module_block.h: $(SRC_PATH)/scripts/modules/module_block.py
+   $(call quiet-command,$(PYTHON) 
$(SRC_PATH)/scripts/modules/module_block.py \
+   "./include/qemu/" $(patsubst %.mo,%.c,$(block-obj-m)), \
+   "  GEN   $@")
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
diff --git a/include/qemu/module_block.h b/include/qemu/module_block.h
deleted file mode 100644
index d725db8..000
--- a/include/qemu/module_block.h
+++ /dev/null
@@ -1,90 +0,0 @@
-/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
-/*
- * QEMU Block Module Infrastructure
- *
- * Copyright Red Hat, Inc. 2015
- *
- * Authors:
- *  Marc Mari   
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_MODULE_BLOCK_H
-#define QEMU_MODULE_BLOCK_H
-
-#include "qemu-common.h"
-
-static const struct {
-const char *format_name;
-const char *protocol_name;
-const char *library_name;
-bool has_probe;
-bool has_probe_device;
-} block_driver_modules[] = {
-   {
-   .library_name = "curl",
-   .format_name = "http",
-   .protocol_name = "http",
-   },
-   {
-   .library_name = "curl",
-   .format_name = "https",
-   .protocol_name = "https",
-   },
-   {
-   .library_name = "curl",
-   .format_name = "ftp",
-   .protocol_name = "ftp",
-   },
-   {
-   .library_name = "curl",
-   .format_name = "ftps",
-   .protocol_name = "ftps",
-   },
-   {
-   .library_name = "curl",
-   .format_name = "tftp",
-   .protocol_name = "tftp",
-   },
-   {
-   .library_name = "rbd",
-   .format_name = "rbd",
-   .protocol_name = "rbd",
-   },
-   {
-   .library_name = "gluster",
-   .format_name = "gluster",
-   .protocol_name = "gluster",
-   },
-   {
-   .library_name = "gluster",
-   .format_name = "gluster",
-   .protocol_name = "gluster+tcp",
-   },
-   {
-   .library_name = "gluster",
-   .format_name = "gluster",
-   .protocol_name = "gluster+unix",
-   },
-   {
-   .library_name = "gluster",
-   .format_name = "gluster",
-   .protocol_name = "gluster+rdma",
-   },
-   {
-   .library_name = "ssh",
-   .format_name = "ssh",
-   .protocol_name = "ssh",
-   },
-   {
-   .library_name = "dmg",
-   .format_name = "dmg",
-   .has_probe = true,
-   },
-};
-
-#endif
-
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
new file mode 100644
index 000..0846362
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,134 @@
+#!/usr/bin/python
+#
+# Module information generator
+#
+# Copyright Red Hat, Inc. 2015
+#
+# Authors:
+#  Marc Mari 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+from __future__ import print_function
+import sys

[Qemu-devel] [PATCH v3 06/10] virtio: handle virtqueue_get_avail_bytes() errors

[Stefan Hajnoczi]

If the vring is invalid, tell the caller no bytes are available and mark
the device broken.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 0c0d333..48f759e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -402,14 +402,14 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 
 if (desc.flags & VRING_DESC_F_INDIRECT) {
 if (desc.len % sizeof(VRingDesc)) {
-error_report("Invalid size for indirect buffer table");
-exit(1);
+virtio_error(vdev, "Invalid size for indirect buffer table");
+goto err;
 }
 
 /* If we've got too many, that implies a descriptor loop. */
 if (num_bufs >= max) {
-error_report("Looped descriptor");
-exit(1);
+virtio_error(vdev, "Looped descriptor");
+goto err;
 }
 
 /* loop over the indirect descriptor table */
@@ -423,8 +423,8 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 do {
 /* If we've got too many, that implies a descriptor loop. */
 if (++num_bufs > max) {
-error_report("Looped descriptor");
-exit(1);
+virtio_error(vdev, "Looped descriptor");
+goto err;
 }
 
 if (desc.flags & VRING_DESC_F_WRITE) {
@@ -449,6 +449,11 @@ done:
 if (out_bytes) {
 *out_bytes = out_total;
 }
+return;
+
+err:
+in_total = out_total = 0;
+goto done;
 }
 
 int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
-- 
2.5.5

[Qemu-devel] [PATCH v2 REPOST 0/2] Add dynamic module loading for block drivers

[Richard W.M. Jones]

This is a repost of the support for dynamically loaded block drivers.
It is identical to how it was posted last summer, except that I have
rebased it and checked that it still works.  It was last posted here:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/threads.html#01995

Last time this was reviewed there were two strands of comments/
objections:

(1) Fam Zheng objected to parsing C structs using the Python generator
code and wanted special macros to be used instead:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02201.html

My objection to that is that it means the same information is defined
in two places, with the usual opportunities for the information to get
out of synch, which could cause crashes or modules not to be loaded.

(2) Denis Lunev wanted module loading to work more like Linux modules:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05331.html

That makes the changes much larger.

Rich.

[Qemu-devel] [PATCH v3 07/10] virtio: use unsigned int for virtqueue_get_avail_bytes() index

[Stefan Hajnoczi]

The virtio code uses int, unsigned int, and uint16_t for virtqueue
indices.  The uint16_t is used for the low-level descriptor layout in
virtio_ring.h while code that isn't concerned with descriptor layout can
use unsigned int.

Use of int is problematic because it can result in signed/unsigned
comparison and incompatible int*/unsigned int* pointer types.

Make the virtqueue_get_avail_bytes() 'i' variable unsigned int.  This
eliminates the need to introduce casts and modify code further in the
patches that follow.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 48f759e..493d6a7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -392,7 +392,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 unsigned int max, num_bufs, indirect = 0;
 VRingDesc desc;
 hwaddr desc_pa;
-int i;
+unsigned int i;
 
 max = vq->vring.num;
 num_bufs = total_bufs;
-- 
2.5.5

[Qemu-devel] [PATCH for-2.6] ivshmem: fix crash with ivshmem-plain

[marcandre . lureau]

From: Marc-André Lureau 

"qemu -device ivshmem-plain" will crash, because it doesn't check that
'memdev' was provided.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 08bf5c6..fae4d08 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -868,7 +868,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
**errp)
 
 s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem,
  _abort);
-} else {
+} else if (s->server_chr) {
 IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
 s->server_chr->filename);
 
@@ -899,6 +899,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error 
**errp)
 error_setg(errp, "failed to initialize interrupts");
 return;
 }
+} else {
+g_assert_not_reached();
 }
 
 vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
@@ -1048,10 +1050,25 @@ static void ivshmem_plain_init(Object *obj)
  _abort);
 }
 
+static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
+{
+IVShmemState *s = IVSHMEM_COMMON(dev);
+
+if (!s->hostmem) {
+error_setg(errp, "You must specify a 'memdev'");
+return;
+}
+
+ivshmem_common_realize(dev, errp);
+}
+
+
 static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+k->realize = ivshmem_plain_realize;
 dc->props = ivshmem_plain_properties;
 dc->vmsd = _plain_vmsd;
 }
-- 
2.5.5

[Qemu-devel] [PATCH v3 09/10] virtio: handle virtqueue_num_heads() errors

[Stefan Hajnoczi]

If the avail ring index is bogus virtqueue_num_heads() must return
-EINVAL.

The only caller is virtqueue_get_avail_bytes().  Return saying no bytes
are available when virtqueue_num_heads() fails.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 58599cf..4fe342a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -323,9 +323,9 @@ static int virtqueue_num_heads(VirtQueue *vq, unsigned int 
idx)
 
 /* Check it isn't doing very strange things with descriptor numbers. */
 if (num_heads > vq->vring.num) {
-error_report("Guest moved used index from %u to %u",
+virtio_error(vq->vdev, "Guest moved used index from %u to %u",
  idx, vq->shadow_avail_idx);
-exit(1);
+return -EINVAL;
 }
 /* On success, callers read a descriptor at vq->last_avail_idx.
  * Make sure descriptor read does not bypass avail index read. */
@@ -393,7 +393,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 idx = vq->last_avail_idx;
 
 total_bufs = in_total = out_total = 0;
-while (virtqueue_num_heads(vq, idx)) {
+while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
 VirtIODevice *vdev = vq->vdev;
 unsigned int max, num_bufs, indirect = 0;
 VRingDesc desc;
@@ -454,6 +454,11 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 else
 total_bufs++;
 }
+
+if (rc < 0) {
+goto err;
+}
+
 done:
 if (in_bytes) {
 *in_bytes = in_total;
-- 
2.5.5

[Qemu-devel] [PATCH v3 05/10] virtio: handle virtqueue_map_desc() errors

[Stefan Hajnoczi]

Errors can occur during virtqueue_pop(), especially in
virtqueue_map_desc().  In order to handle this we must unmap iov[]
before returning NULL.  The caller will consider the virtqueue empty and
the virtio_error() call will have marked the device broken.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 62 ++
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 6e6b968..0c0d333 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -460,10 +460,12 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int 
in_bytes,
 return in_bytes <= in_total && out_bytes <= out_total;
 }
 
-static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct 
iovec *iov,
+static bool virtqueue_map_desc(VirtIODevice *vdev, unsigned int *p_num_sg,
+   hwaddr *addr, struct iovec *iov,
unsigned int max_num_sg, bool is_write,
hwaddr pa, size_t sz)
 {
+bool ok = false;
 unsigned num_sg = *p_num_sg;
 assert(num_sg <= max_num_sg);
 
@@ -471,8 +473,9 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, 
hwaddr *addr, struct iove
 hwaddr len = sz;
 
 if (num_sg == max_num_sg) {
-error_report("virtio: too many write descriptors in indirect 
table");
-exit(1);
+virtio_error(vdev, "virtio: too many write descriptors in "
+   "indirect table");
+goto out;
 }
 
 iov[num_sg].iov_base = cpu_physical_memory_map(pa, , is_write);
@@ -483,7 +486,28 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, 
hwaddr *addr, struct iove
 pa += len;
 num_sg++;
 }
+ok = true;
+
+out:
 *p_num_sg = num_sg;
+return ok;
+}
+
+/* Only used by error code paths before we have a VirtQueueElement (therefore
+ * virtqueue_unmap_sg() can't be used).  Assumes buffers weren't written to
+ * yet.
+ */
+static void virtqueue_undo_map_desc(unsigned out_num, unsigned in_num,
+struct iovec *iov)
+{
+unsigned i;
+
+for (i = 0; i < out_num + in_num; i++) {
+int is_write = i >= out_num;
+
+cpu_physical_memory_unmap(iov->iov_base, iov->iov_len, is_write, 0);
+iov++;
+}
 }
 
 static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr,
@@ -582,8 +606,8 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 vring_desc_read(vdev, , desc_pa, i);
 if (desc.flags & VRING_DESC_F_INDIRECT) {
 if (desc.len % sizeof(VRingDesc)) {
-error_report("Invalid size for indirect buffer table");
-exit(1);
+virtio_error(vdev, "Invalid size for indirect buffer table");
+return NULL;
 }
 
 /* loop over the indirect descriptor table */
@@ -595,22 +619,30 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 
 /* Collect all the descriptors */
 do {
+bool map_ok;
+
 if (desc.flags & VRING_DESC_F_WRITE) {
-virtqueue_map_desc(_num, addr + out_num, iov + out_num,
-   VIRTQUEUE_MAX_SIZE - out_num, true, desc.addr, 
desc.len);
+map_ok = virtqueue_map_desc(vdev, _num, addr + out_num,
+iov + out_num,
+VIRTQUEUE_MAX_SIZE - out_num, true,
+desc.addr, desc.len);
 } else {
 if (in_num) {
-error_report("Incorrect order for descriptors");
-exit(1);
+virtio_error(vdev, "Incorrect order for descriptors");
+goto err_undo_map;
 }
-virtqueue_map_desc(_num, addr, iov,
-   VIRTQUEUE_MAX_SIZE, false, desc.addr, desc.len);
+map_ok = virtqueue_map_desc(vdev, _num, addr, iov,
+VIRTQUEUE_MAX_SIZE, false,
+desc.addr, desc.len);
+}
+if (!map_ok) {
+goto err_undo_map;
 }
 
 /* If we've got too many, that implies a descriptor loop. */
 if ((in_num + out_num) > max) {
-error_report("Looped descriptor");
-exit(1);
+virtio_error(vdev, "Looped descriptor");
+goto err_undo_map;
 }
 } while ((i = virtqueue_read_next_desc(vdev, , desc_pa, max)) != max);
 
@@ -630,6 +662,10 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 
 trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
 return elem;
+
+err_undo_map:
+virtqueue_undo_map_desc(out_num, in_num, iov);
+return NULL;
 }
 
 /* Reading and writing a structure directly to QEMUFile is *awful*, but
-- 
2.5.5

[Qemu-devel] [PATCH v3 08/10] virtio: handle virtqueue_read_next_desc() errors

[Stefan Hajnoczi]

Stop processing the vring if an avail ring index is invalid.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 45 -
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 493d6a7..58599cf 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -353,28 +353,33 @@ static unsigned int virtqueue_get_head(VirtQueue *vq, 
unsigned int idx)
 return head;
 }
 
-static unsigned virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
- hwaddr desc_pa, unsigned int max)
-{
-unsigned int next;
+enum {
+VIRTQUEUE_READ_DESC_ERROR = -1,
+VIRTQUEUE_READ_DESC_DONE = 0,   /* end of chain */
+VIRTQUEUE_READ_DESC_MORE = 1,   /* more buffers in chain */
+};
 
+static int virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
+hwaddr desc_pa, unsigned int max,
+unsigned int *next)
+{
 /* If this descriptor says it doesn't chain, we're done. */
 if (!(desc->flags & VRING_DESC_F_NEXT)) {
-return max;
+return VIRTQUEUE_READ_DESC_DONE;
 }
 
 /* Check they're not leading us off end of descriptors. */
-next = desc->next;
+*next = desc->next;
 /* Make sure compiler knows to grab that: we don't want it changing! */
 smp_wmb();
 
-if (next >= max) {
-error_report("Desc next is %u", next);
-exit(1);
+if (*next >= max) {
+virtio_error(vdev, "Desc next is %u", *next);
+return VIRTQUEUE_READ_DESC_ERROR;
 }
 
-vring_desc_read(vdev, desc, desc_pa, next);
-return next;
+vring_desc_read(vdev, desc, desc_pa, *next);
+return VIRTQUEUE_READ_DESC_MORE;
 }
 
 void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
@@ -383,6 +388,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 {
 unsigned int idx;
 unsigned int total_bufs, in_total, out_total;
+int rc;
 
 idx = vq->last_avail_idx;
 
@@ -435,7 +441,13 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
 goto done;
 }
-} while ((i = virtqueue_read_next_desc(vdev, , desc_pa, max)) != 
max);
+
+rc = virtqueue_read_next_desc(vdev, , desc_pa, max, );
+} while (rc == VIRTQUEUE_READ_DESC_MORE);
+
+if (rc == VIRTQUEUE_READ_DESC_ERROR) {
+goto err;
+}
 
 if (!indirect)
 total_bufs = num_bufs;
@@ -587,6 +599,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 hwaddr addr[VIRTQUEUE_MAX_SIZE];
 struct iovec iov[VIRTQUEUE_MAX_SIZE];
 VRingDesc desc;
+int rc;
 
 if (unlikely(vdev->broken)) {
 return NULL;
@@ -649,7 +662,13 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 virtio_error(vdev, "Looped descriptor");
 goto err_undo_map;
 }
-} while ((i = virtqueue_read_next_desc(vdev, , desc_pa, max)) != max);
+
+rc = virtqueue_read_next_desc(vdev, , desc_pa, max, );
+} while (rc == VIRTQUEUE_READ_DESC_MORE);
+
+if (rc == VIRTQUEUE_READ_DESC_ERROR) {
+goto err_undo_map;
+}
 
 /* Now copy what we have collected and mapped */
 elem = virtqueue_alloc_element(sz, out_num, in_num);
-- 
2.5.5

Re: [Qemu-devel] [Nbd] [PATCHv8] Improve documentation for TLS

[Alex Bligh]

Wouter,

On 12 Apr 2016, at 14:01, Wouter Verhelst  wrote:

> It's not being applied because of this ;-)

OK, I've sent a PATCHv9 with my suggested wording (so at least
it doesn't make anything worse), but I really think we should
avoid addressing further nits in the disconnection regime within
a change which is meant to be documenting TLS.

Alex
 

> I can probably apply it and then work on the clarification of that
> paragraph, I suppose, but I'd prefer doing it right from the get go.

-- 
Alex Bligh

Re: [Qemu-devel] [PATCH 8/9] Add memfd based hostmem

[Marc-André Lureau]

- Original Message -
> On Tue, 12 Apr 2016 13:55:28 +0200
> marcandre.lur...@redhat.com wrote:
> 
> > From: Marc-André Lureau 
> > 
> > Add a new memory backend, similar to hostmem-file, except that it
> > doesn't need a file path, or hugepages directory permissions. It also
> > try to enforce memory sealing if available.
> > 
> > This backend is mainly useful for easily sharing the memory with other
> > processes, for example with vhost-user backend, when hugepages aren't
> > available (for security reasons and other limitations).
> > 
> > Usage:
> > -object memory-backend-memfd,id=mem1,size=1G
> Does it make sense if it's possible to reuse/extend memory-backend-file,
> i.e. make 'mem-path' optional and behave like this patch
> and also add 'memfd' property so that parent process could
> pass its handle to QEMU as well?

I don't mind, I would be a bit worried by the combination of options and the 
expected result though.

> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  backends/Makefile.objs   |  1 +
> >  backends/hostmem-memfd.c | 85
> >  
> >  qemu-options.hx  | 12 +++
> >  3 files changed, 98 insertions(+)
> >  create mode 100644 backends/hostmem-memfd.c
> > 
> > diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> > index 31a3a89..32cfcdb 100644
> > --- a/backends/Makefile.objs
> > +++ b/backends/Makefile.objs
> > @@ -9,3 +9,4 @@ common-obj-$(CONFIG_TPM) += tpm.o
> >  
> >  common-obj-y += hostmem.o hostmem-ram.o
> >  common-obj-$(CONFIG_LINUX) += hostmem-file.o
> > +common-obj-$(CONFIG_POSIX) += hostmem-memfd.o
> > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > new file mode 100644
> > index 000..e0e18d7
> > --- /dev/null
> > +++ b/backends/hostmem-memfd.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * QEMU host memfd memory backend
> > + *
> > + * Copyright (C) 2016 Red Hat Inc
> > + *
> > + * Authors:
> > + *   Marc-André Lureau 
> > + *
> > + * 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 "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "sysemu/hostmem.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qom/object_interfaces.h"
> > +#include "qemu/memfd.h"
> > +#include "qapi/error.h"
> > +
> > +/**
> > + * @TYPE_MEMORY_BACKEND_MEMFD:
> > + * name of the memory backend that uses memfd mmap
> > + */
> > +#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
> > +
> > +#define MEMORY_BACKEND_MEMFD(obj) \
> > +OBJECT_CHECK(HostMemoryBackendMemfd, (obj), TYPE_MEMORY_BACKEND_MEMFD)
> > +
> > +typedef struct HostMemoryBackendMemfd HostMemoryBackendMemfd;
> > +
> > +struct HostMemoryBackendMemfd {
> > +HostMemoryBackend parent_obj;
> > +
> > +int fd;
> > +};
> > +
> > +static void
> > +memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > +{
> > +int fd;
> > +
> > +if (!backend->size) {
> > +error_setg(errp, "can't create backend with size 0");
> > +return;
> > +}
> > +#ifndef CONFIG_LINUX
> > +error_setg(errp, "memfd not supported on this host");
> > +#else
> > +if (!memory_region_size(>mr)) {
> > +backend->force_prealloc = mem_prealloc;
> > +fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD,
> > +   backend->size,
> > +   F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL);
> > +if (fd == -1) {
> > +error_setg(errp, "can't allocate memfd backend");
> > +return;
> > +}
> > +memory_region_init_ram_from_fd(>mr, OBJECT(backend),
> > +object_get_canonical_path(OBJECT(backend)),
> > +backend->size, true, fd, errp);
> > +}
> > +#endif
> > +}
> > +
> > +static void
> > +memfd_backend_class_init(ObjectClass *oc, void *data)
> > +{
> > +HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> > +
> > +bc->alloc = memfd_backend_memory_alloc;
> > +}
> > +
> > +static const TypeInfo memfd_backend_info = {
> > +.name = TYPE_MEMORY_BACKEND_MEMFD,
> > +.parent = TYPE_MEMORY_BACKEND,
> > +.class_init = memfd_backend_class_init,
> > +.instance_size = sizeof(HostMemoryBackendMemfd),
> > +};
> > +
> > +static void register_types(void)
> > +{
> > +type_register_static(_backend_info);
> > +}
> > +
> > +type_init(register_types);
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 587de8f..a54af0f 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3756,6 +3756,18 @@ The @option{share} boolean option determines whether
> > the memory
> >  region is marked as private to QEMU, or shared. The latter allows
> >  a co-operating external process to access the QEMU memory region.
> >  
> > +@item -object memory-backend-memfd,id=@var{id},size=@var{size}
> > +
> > 

[Qemu-devel] [PATCHv9] Improve documentation for TLS

[Alex Bligh]

* Call out TLS into a separate section

* Add details of the TLS protocol itself

* Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
  be initiated from either side (as required by the TLS standard I believe
  and as actually works in practice)

* Clarify what is a requirement on servers, and what is a requirement on
  clients, separately, specifying their behaviour in a single place
  in the document.

* Document the three possible modes of operation of a server.

* Add text defining what 'terminate the session' means during
  negotiation, and when it is available.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 352 +--
 1 file changed, 318 insertions(+), 34 deletions(-)

Changes since v8:

* Reword section on disconnection, as per mail to list save with Eric Blake's
  change of 're' to 'regarding'.

Changes since v7

* I missed committing the changes re consistent use of 'option' rather than 
'command'
 in v7. They are here now.

Changes from v6:

* Introduced language mandating a server to reply with NBD_ERR_INVALID
to NBD_OPT_STARTTLS if TLS is already negotiatied.

* Removed some duplication in SELECTIVETLS over the prohibition on
servers not returning NBD_ERR_TLSREQD to options other than
NBD_OPT_EXPORTNAME, NBD_OPT_INFO and NBD_OPT_GO. The same thing
was said a different way a couple of paragraphs below.

* Consistently refer to 'options' rather than 'commands' in the
negotiation phase.

* Eric Blake's nits

Changes from v5:

* Delete OPTIONALTLS (RIP)

* Add NBD_REP_ERR_POLICY

* s/NBD_ERR_REP/NBD_REP_ERR/ in one place

* Consistently use the phrase 'terminate the session' to mean dropping
the connection, as per Wouter. Note there are other inconsistent
uses of 'dropping the connection', 'disconnecting' etc. elsewhere
which I haven't touched.

* Similarly refer to the connection as a 'session' when it doesn't
explicitly mean the L3 TCP connection (TLS section only).

* Introduce a paragraph under newstyle negotiation emphasising that
terminating the session is legal and sometimes required, and defining
it.

Changes from v4

* Minor grammar nit

Changes from v3:

* Delete confusing text about server omitting entries from NBD_OPT_LIST
if TLS is not negotiated and FORCETLS is used, as that (of course)
requires NBD_REP_ERR_TLS_REQD elsewhere in the text.

* Further nits from Eric Blake

Changes from v2:

* The response to a command is a response, not a NBD_REP_ACK

* Make it clear that the response can be errored

* Nits from Eric Blake

Changes from v1:

* Make a NBD_CMD_CLOSE imply a flush

* Nits from Eric Blake

diff --git a/doc/proto.md b/doc/proto.md
index f117394..255fd11 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -195,6 +195,23 @@ request before sending the next one of the same type. The 
server MAY
 send replies in the order that the requests were received, but is not
 required to.
 
+There is no requirement for the client or server to complete a
+negotiation if it does not wish to do so. Either end may simply
+close the TCP connection (though see below regarding prior use
+of NBD_OPT_ABORT). Under certain circumstances either
+the client or the server may be required by this document to close
+the TCP connection. In each case, this is referred to as 'terminate
+the session'.
+
+If the client wishes to terminate the session in the negotiation
+phase, and is not doing so because it is required to do so
+by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
+permits. There are instances where this is impossible, such as after
+an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
+negotiation of TLS.  For instance, if the client does not find an
+export it is looking for, it may simply send an NBD_OPT_ABORT
+and close the TCP connection.
+
 ### Transmission
 
 There are three message types in the transmission phase: the request,
@@ -286,6 +303,287 @@ S: (*length* bytes of data if the request is of type 
`NBD_CMD_READ`)
 This reply type MUST NOT be used except as documented by the
 experimental `STRUCTURED_REPLY` extension; see below.
 
+## TLS support
+
+The NBD protocol supports Transport Layer Security (TLS) (see
+[RFC5246](https://tools.ietf.org/html/rfc5246)
+as updated by
+[RFC6176](https://tools.ietf.org/html/rfc6176)
+).
+
+TLS is negotiated with the `NBD_OPT_STARTTLS`
+option. This is performed as an in-session upgrade. Below the term
+'negotiation' is used to refer to the sending and receiving of
+NBD options and option replies, and the term 'initiation' of TLS
+is used to refer to the actual upgrade to TLS.
+
+### Certificates, authentication and authorisation
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authorisation (for instance
+whether client and/or server certificates are required and
+what they should contain); this is implementation dependent.
+

[Qemu-devel] [PATCH] ide: coding style fix

[Pavel Butsykin]

Signed-off-by: Pavel Butsykin 
---
 hw/ide/internal.h | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index eb006c2..d2c458f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -338,7 +338,7 @@ enum ide_dma_cmd {
 IDE_DMA_READ,
 IDE_DMA_WRITE,
 IDE_DMA_TRIM,
-IDE_DMA_ATAPI
+IDE_DMA_ATAPI,
 };
 
 #define ide_cmd_is_read(s) \
@@ -507,6 +507,7 @@ struct IDEDevice {
 };
 
 /* These are used for the error_status field of IDEBus */
+#define IDE_RETRY_MASK 0xf8
 #define IDE_RETRY_DMA  0x08
 #define IDE_RETRY_PIO  0x10
 #define IDE_RETRY_ATAPI 0x20 /* reused IDE_RETRY_READ bit */
@@ -526,9 +527,7 @@ struct IDEDevice {
  * impossible bit combination as a new status value.
  */
 #define IS_IDE_RETRY_ATAPI(_status)   \
-(((_status) & IDE_RETRY_ATAPI) && \
- !IS_IDE_RETRY_DMA(_status) &&\
- !IS_IDE_RETRY_PIO(_status))
+(((_status) & IDE_RETRY_MASK) == IDE_RETRY_ATAPI)
 
 static inline uint8_t ide_dma_cmd_to_retry(uint8_t dma_cmd)
 {
-- 
2.7.4

[Qemu-devel] [PATCH v3 04/10] virtio: migrate vdev->broken flag

[Stefan Hajnoczi]

Send a subsection if the vdev->broken flag is set.  This allows live
migration of broken virtio devices.

The subsection is only sent if vdev->broken has been set.  In most cases
the flag will be clear and no subsection will be sent.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index cedda4b..6e6b968 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1291,6 +1291,13 @@ static bool virtio_extra_state_needed(void *opaque)
 k->has_extra_state(qbus->parent);
 }
 
+static bool virtio_broken_needed(void *opaque)
+{
+VirtIODevice *vdev = opaque;
+
+return vdev->broken;
+}
+
 static const VMStateDescription vmstate_virtqueue = {
 .name = "virtqueue_state",
 .version_id = 1,
@@ -1405,6 +1412,17 @@ static const VMStateDescription 
vmstate_virtio_64bit_features = {
 }
 };
 
+static const VMStateDescription vmstate_virtio_broken = {
+.name = "virtio/broken",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = _broken_needed,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(broken, VirtIODevice),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_virtio = {
 .name = "virtio",
 .version_id = 1,
@@ -1418,6 +1436,7 @@ static const VMStateDescription vmstate_virtio = {
 _virtio_64bit_features,
 _virtio_virtqueues,
 _virtio_ringsize,
+_virtio_broken,
 _virtio_extra_state,
 NULL
 }
-- 
2.5.5