Re: [Qemu-devel] [PATCH v25 11/31] change block layer to support both QemuOpts and QEMUOptionParamter
On 4/22/2014 at 07:17 AM, in message 5355a71e.2010...@redhat.com, Eric
Blake
ebl...@redhat.com wrote:
On 04/10/2014 11:54 AM, Chunyan Liu wrote:
Change block layer to support both QemuOpts and QEMUOptionParameter.
After this patch, it will change backend drivers one by one. At the end,
QEMUOptionParameter will be removed and only QemuOpts is kept.
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
@@ -419,8 +420,27 @@ static void coroutine_fn bdrv_create_co_entry(void
*opaque)
CreateCo *cco = opaque;
+if (cco-drv-bdrv_create2) {
+QemuOptsList *opts_list = NULL;
+QemuOpts *opts = NULL;
+if (!cco-opts) {
Isn't your conversion pair-wise per driver, in that you always pair
bdrv_create2 with options, and bdrv_create with opts? That is, won't
cco-opts always be false if cco-drv-bdrv_create2 is non-NULL, since
we already guaranteed that there is at most one of the two creation
function callbacks defined? Maybe you have a missing assertion at the
point where the callbacks are registered? [1]
To handle both QEMUOptionParameter and QemuOpts, we convert
QEMUOptionParameter to QemuOpts first for unified processing.
And according to v22 discussion, it's better to convert back to
QEMUOptionParameter at the last moment.
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02181.html.
For bdrv_create, either do this in bdrv_create(), or here in
bdrv_create_co_entry(). bdrv_create() just calls bdrv_create_co_entry
to do the work. Do you think we should finish conversion in bdrv_create()
before calling bdrv_create_co_entry()?
+opts_list = params_to_opts(cco-options);
+cco-opts = opts =
+qemu_opts_create(opts_list, NULL, 0, error_abort);
Ouch. This assigns to cco-opts,...
+}
+ret = cco-drv-bdrv_create2(cco-filename, cco-opts,
local_err);
+qemu_opts_del(opts);
...but this frees the memory. You have modified the caller's opaque
pointer to point to what is now stale memory. Is this intentional? [2]
Yes. My intension is to avoid deleting cco-opts errorly, only the converted
one from cco-options should be deleted, so I use a temp variable to retrieve
the new allocated opts, just new and delete the temp variable is OK.
The work could also be done as:
if (cco-options) {
opts_list = params_to_opts(cco-options);
cco-opts =
qemu_opts_create(opts_list, NULL, 0, error_abort);
}
ret = cco-drv-bdrv_create2(cco-filename, cco-opts, local_err);
if (cco-options) {
qemu_opts_del(cco-opts);
qemu_opts_free(opts_list);
}
+qemu_opts_free(opts_list);
+} else {
+QEMUOptionParameter *options = NULL;
+if (!cco-options) {
+cco-options = options = opts_to_params(cco-opts);
+}
+ret = cco-drv-bdrv_create(cco-filename, cco-options,
local_err);
+free_option_parameters(options);
Same question [2] in the opposite direction for cco-options.
@@ -5296,28 +5328,25 @@ void bdrv_img_create(const char *filename, const
char *fmt,
/* Parse -o options */
if (options) {
-param = parse_option_parameters(options, create_options, param);
-if (param == NULL) {
+if (qemu_opts_do_parse(opts, options, NULL) != 0) {
error_setg(errp, Invalid options for file format '%s'.,
fmt);
Pre-existing (and probably worth an independent patch to qemu-trivial),
but error messages typically should not end in '.'
Will add new patch for that.
+int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options,
+ QemuOpts *opts)
{
-if (bs-drv-bdrv_amend_options == NULL) {
+int ret;
+assert(!(options opts));
+
+if (!bs-drv-bdrv_amend_options !bs-drv-bdrv_amend_options2) {
Hmm, should we also guarantee that at most one of these callback
functions exist? [1]
return -ENOTSUP;
}
-return bs-drv-bdrv_amend_options(bs, options);
+if (bs-drv-bdrv_amend_options2) {
+QemuOptsList *tmp_opts_list = NULL;
+QemuOpts *tmp_opts = NULL;
+if (options) {
+tmp_opts_list = params_to_opts(options);
+opts = tmp_opts =
+qemu_opts_create(tmp_opts_list, NULL, 0, error_abort);
+}
+ret = bs-drv-bdrv_amend_options2(bs, opts);
+qemu_opts_del(tmp_opts);
Why do you need tmp_opts? Isn't manipulation of 'opts' sufficient here?
Same as answer to [2]. The intension is to avoid deleting opts errorly.
+++ b/include/block/block_int.h
@@ -118,6 +118,8 @@ struct BlockDriver {
void (*bdrv_rebind)(BlockDriverState *bs);
Re: [Qemu-devel] [PATCH v5 12/12] iotests: Omit length/offset test in 040 and 041
On Thu, 04/17 23:59, Max Reitz wrote:
As the length of a mirror block job no longer directly depends on the
size of the block device, drop the related checks from this test.
As 041 uses the wait_until_completed function from iotests.py, that
check has to be dropped there as well which in turn affects test 055. On
the other hand, a block job's length does not have to be related to the
length of the image file in the first place, so that check was
questionable anyway.
Signed-off-by: Max Reitz mre...@redhat.com
---
tests/qemu-iotests/040| 3 ---
tests/qemu-iotests/041| 2 --
tests/qemu-iotests/iotests.py | 2 --
3 files changed, 7 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 734b6a6..437af2b 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -46,13 +46,10 @@ class ImageCommitTestCase(iotests.QMPTestCase):
if event['event'] == 'BLOCK_JOB_COMPLETED':
self.assert_qmp(event, 'data/type', 'commit')
self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
completed = True
elif event['event'] == 'BLOCK_JOB_READY':
self.assert_qmp(event, 'data/type', 'commit')
self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/len', self.image_len)
self.vm.qmp('block-job-complete', device='drive0')
self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index ec470b2..8bb7ec3 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -46,8 +46,6 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
event = self.cancel_and_wait()
self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
self.assert_qmp(event, 'data/type', 'mirror')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
def complete_and_wait(self, drive='drive0', wait_ready=True):
'''Complete a block job and wait for it to finish'''
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e4fa9af..0d3ff24 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -265,8 +265,6 @@ class QMPTestCase(unittest.TestCase):
if event['event'] == 'BLOCK_JOB_COMPLETED':
self.assert_qmp(event, 'data/device', drive)
self.assert_qmp_absent(event, 'data/error')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
completed = True
self.assert_no_active_block_jobs()
--
1.9.2
Since you are touching this code as well, do you mind take this in your
next revision? :)
https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00274.html
Thanks,
Fam
Re: [Qemu-devel] [PATCH v3 3/4] pc q35: Add new object pc-memory-layout.
On Tue, 2014-04-22 at 20:13 -0400, Don Slutz wrote:
On 04/21/14 08:27, Paolo Bonzini wrote:
Il 24/03/2014 19:55, Don Slutz ha scritto:
This new object has the property max-ram-below-4g.
If you add enough PCI devices then all mmio for them will not fit
below 4G which may not be the layout the user wanted. This allows
you to increase the below 4G address space that PCI devices can use
(aka decrease ram below 4G) and therefore in more cases not have any
mmio that is above 4G.
For example adding -global pc-memory-layout.max-ram-below-4g=2G to
the command line will limit the amount of ram that is below 4G to
2G.
Does Xen's firmware allow 64-bit BARs?
Yes. But not all supported devices do.
I'm wondering why this is not a problem on KVM.
I suspect that it is. It only shows up when you add a lot of
PCI devices either directly or via pci pass through. Also it
is more Xen focused because the default ram below 4G is
much larger so that 32 bit only guests can have a lot of ram.
Also, overloading -global like this isn't the best long term choice. We
should get custom -machine properties in 2.1, we should use them.
I may have missed this. Is this
[PATCH V3 0/5] remove QEMUMachine indirection from MachineClass
Or some patch that is yet to be posted?
Hi Don,
I posted a patch some time ago and I am going to post it again soon with some
modifications:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00041.html
Basically what matters are these lines:
machine_opts = qemu_get_machine_opts();
+if (qemu_opt_foreach(machine_opts, object_set_property, current_machine,
1) 0) {
+object_unref(OBJECT(current_machine));
+exit(1);
+}
You create a subtype of MACHINE which has this property and it will be
automatically
assigned. The positive thing here is that the property is not global anymore.
Thanks,
Marcel
(I did add this to qemu_machine_opts and changed to the current
way to handle:
Subject: Re: [Qemu-devel] [PATCH 1/1] vl.c: Add pci_hole_min_size
machine option.
Date: Wed, 5 Mar 2014 08:46:33 +0200
From: Michael S. Tsirkin m...@redhat.com
To: Don Slutz dsl...@verizon.com
CC: xen-de...@lists.xensource.com, qemu-devel@nongnu.org, Anthony
Liguori aligu...@amazon.com, Stefano Stabellini
stefano.stabell...@eu.citrix.com
On Tue, Mar 04, 2014 at 06:51:11PM -0500, Don Slutz wrote:
On 03/04/14 17:20, Michael S. Tsirkin wrote:
On Tue, Mar 04, 2014 at 02:18:49PM -0500, Don Slutz wrote:
On 03/04/14 11:58, Michael S. Tsirkin wrote:
On Tue, Mar 04, 2014 at 11:36:44AM -0500, Don Slutz wrote:
On 03/04/14 02:34, Michael S. Tsirkin wrote:
On Thu, Feb 27, 2014 at 05:32:23PM -0500, Don Slutz wrote:
This allows growing the pci_hole to the size needed.
...
Yes. Also, please find a way to only add it to
machine types that support it, instead of ignoring it
silently for those that don't.
)
Paolo
Re: [Qemu-devel] [PATCH V3 0/5] remove QEMUMachine indirection from MachineClass
On Wed, 2014-04-09 at 20:34 +0300, Marcel Apfelbaum wrote: Cc: Andreas Färber afaer...@suse.de V2 - V3: - Addressed Andreas's comments: - Dropped QEMUMachineInitArgs's 'next' obsoleted field in a separate patch - Revision the separation into patches: - Started using MachineClass for .machine early (3/5). - Merged hw/ppc changes with QEMUMachine indirection removal - Ensured that git bisect is not affected - Rebased to master. Ping Thanks, Marcel V1 - V2: - Addressed Paolo's comments: - replaced commas by semicolons on patch 4/5. - Rebased to master. This is a continuation of 'QEMU Machine as QOM object' effort. The scope of this series is to allow machine QOM-ification of all machines gradually, by removing the need for QEMUMachine registration through vl.c . Now we will have 2 paths: 1. Non QOM-ified machines will be converted to QOM on the fly in vl.c by qemu machine registration. 2. QOM-ified machines will behave as regular QOM classes setting MachineClass fields in class_init. - Patch 4/5 demonstrates this. Next steps: - Replace QemuOpts queries by MachineState fields. - Follow Paolo's suggestions to get rid of QEMUMachineInitArgs. Comments are appreciated, Thanks, Marcel Marcel Apfelbaum (5): hw/boards.h: remove obsoleted field from QEMUMachine vl.c: copy QEMUMachine's fields to MachineClass vl.c: Replace QEMUMachine with MachineClass in QEMUMachineInitArgs machine: replace QEMUMachine by MachineClass in accelerator configuration machine: remove QEMUMachine indirection from MachineClass device-hotplug.c| 2 +- hw/ppc/spapr.c | 26 +-- include/hw/boards.h | 30 +++-- include/hw/xen/xen.h| 2 +- include/qemu/typedefs.h | 1 + include/sysemu/kvm.h| 2 +- include/sysemu/qtest.h | 2 +- kvm-all.c | 6 +-- kvm-stub.c | 2 +- qmp.c | 4 +- qtest.c | 2 +- vl.c| 114 +++- xen-all.c | 2 +- xen-stub.c | 2 +- 14 files changed, 116 insertions(+), 81 deletions(-)
Re: [Qemu-devel] [PATCH v8 0/4] qemu-img: add preallocation=full
ping... On Wed, Apr 09, 2014 at 03:12:33PM +0800, Hu Tao wrote: The purpose of this series is to use posix_fallocate() when creating img file to ensure there are disk space for it which is way fast than acturally writing to disk. But this only works in file system level. For cases like thin provisioning, an option full preallocation is added to write zeros to storage to ensure disk space. Hu Tao (4): qapi: introduce PreallocMode and a new PreallocMode full. raw, qcow2: don't convert file size to sector size raw-posix: Add full image preallocation option qcow2: Add full image preallocation option block/qcow2.c | 95 -- block/raw-posix.c | 63 ++ block/raw-win32.c | 4 +- qapi-schema.json | 14 +++ tests/qemu-iotests/082.out | 54 +- 5 files changed, 182 insertions(+), 48 deletions(-) -- 1.8.5.2.229.g4448466
Re: [Qemu-devel] [PATCH 008/124] vmstate: Reduce code duplication
On Tue, Apr 22, 2014 at 10:59:33AM +0100, Dr. David Alan Gilbert wrote:
* Juan Quintela (quint...@redhat.com) wrote:
From: Michael S. Tsirkin m...@redhat.com
Move size offset and number of elements math out
to functions, to reduce code duplication.
In my original review of Michael's patch I said I was OK with it, but I'd
prefer if we had something better than 'int' for vmstate_n_elems, but
didn't want to hold up his fix series; if this is part of the huge patch
series then we might as well tidy this up.
How about:
1) Make vmstate_n_elems return uint32_t or unsigned int
2) Make it check in the int32_t case for a -ve number print a warning and
return 0.
at the moment it's broken in the corner case of VMS_VARRAY_UINT32 and a huge
value.
Dave
OK just to record what we discussed on IRC, it's probably OK
to address this comment in a follow-up patch, so that
we don't all get this patchbomb again.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Cc: Dr. David Alan Gilbert dgilb...@redhat.com
Signed-off-by: Juan Quintela quint...@redhat.com
---
vmstate.c | 100
--
1 file changed, 52 insertions(+), 48 deletions(-)
diff --git a/vmstate.c b/vmstate.c
index bcf1cde..e0debfa 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -10,6 +10,50 @@ static void vmstate_subsection_save(QEMUFile *f, const
VMStateDescription *vmsd,
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
*vmsd,
void *opaque);
+static int vmstate_n_elems(void *opaque, VMStateField *field)
+{
+int n_elems = 1;
+
+if (field-flags VMS_ARRAY) {
+n_elems = field-num;
+} else if (field-flags VMS_VARRAY_INT32) {
+n_elems = *(int32_t *)(opaque+field-num_offset);
+} else if (field-flags VMS_VARRAY_UINT32) {
+n_elems = *(uint32_t *)(opaque+field-num_offset);
+} else if (field-flags VMS_VARRAY_UINT16) {
+n_elems = *(uint16_t *)(opaque+field-num_offset);
+} else if (field-flags VMS_VARRAY_UINT8) {
+n_elems = *(uint8_t *)(opaque+field-num_offset);
+}
+
+return n_elems;
+}
+
+static int vmstate_size(void *opaque, VMStateField *field)
+{
+int size = field-size;
+
+if (field-flags VMS_VBUFFER) {
+size = *(int32_t *)(opaque+field-size_offset);
+if (field-flags VMS_MULTIPLY) {
+size *= field-size;
+}
+}
+
+return size;
+}
+
+static void *vmstate_base_addr(void *opaque, VMStateField *field)
+{
+void *base_addr = opaque + field-offset;
+
+if (field-flags VMS_POINTER) {
+base_addr = *(void **)base_addr + field-start;
+}
+
+return base_addr;
+}
+
int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id)
{
@@ -37,30 +81,10 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
field-field_exists(opaque, version_id)) ||
(!field-field_exists
field-version_id = version_id)) {
-void *base_addr = opaque + field-offset;
-int i, n_elems = 1;
-int size = field-size;
-
-if (field-flags VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field-size_offset);
-if (field-flags VMS_MULTIPLY) {
-size *= field-size;
-}
-}
-if (field-flags VMS_ARRAY) {
-n_elems = field-num;
-} else if (field-flags VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field-num_offset);
-} else if (field-flags VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field-num_offset);
-} else if (field-flags VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field-num_offset);
-} else if (field-flags VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field-num_offset);
-}
-if (field-flags VMS_POINTER) {
-base_addr = *(void **)base_addr + field-start;
-}
+void *base_addr = vmstate_base_addr(opaque, field);
+int i, n_elems = vmstate_n_elems(opaque, field);
+int size = vmstate_size(opaque, field);
+
for (i = 0; i n_elems; i++) {
void *addr = base_addr + size * i;
@@ -109,30 +133,10 @@ void vmstate_save_state(QEMUFile *f, const
VMStateDescription *vmsd,
while (field-name) {
if (!field-field_exists ||
field-field_exists(opaque, vmsd-version_id)) {
-void *base_addr = opaque + field-offset;
-int i, n_elems = 1;
-int size =
Re: [Qemu-devel] [PATCH 1/3] virtio: Introduce VirtIODevice.broken
On Tue, Apr 22, 2014 at 04:55:15PM +0800, Fam Zheng wrote:
If guest driver behaves abnormally, emulation code could mark the device
as broken.
Once broken is set, device emulation will typically wait for a reset
command and ignore any other operations, but it could also return error
responds. In other words, whether and how does guest know about this
error status is device specific.
Signed-off-by: Fam Zheng f...@redhat.com
We really need a flag to notify guest about this state though.
We should add this in virtio 1.0.
For now, how about clearing DRIVER_OK?
This way we don't need to touch so much code.
---
hw/virtio/virtio.c | 12
include/hw/virtio/virtio.h | 3 +++
2 files changed, 15 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aeabf3a..222bb73 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -538,6 +538,16 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val)
vdev-status = val;
}
+bool virtio_broken(VirtIODevice *vdev)
+{
+return vdev-broken;
+}
+
+void virtio_set_broken(VirtIODevice *vdev)
+{
+vdev-broken = true;
+}
+
void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
@@ -554,6 +564,7 @@ void virtio_reset(void *opaque)
vdev-queue_sel = 0;
vdev-status = 0;
vdev-isr = 0;
+vdev-broken = false;
vdev-config_vector = VIRTIO_NO_VECTOR;
virtio_notify_vector(vdev, vdev-config_vector);
@@ -995,6 +1006,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
vdev-status = 0;
vdev-isr = 0;
vdev-queue_sel = 0;
+vdev-broken = 0;
vdev-config_vector = VIRTIO_NO_VECTOR;
vdev-vq = g_malloc0(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
vdev-vm_running = runstate_is_running();
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 3e54e90..5b16faa 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -121,6 +121,7 @@ struct VirtIODevice
bool vm_running;
VMChangeStateEntry *vmstate;
char *bus_name;
+bool broken;
};
typedef struct VirtioDeviceClass {
@@ -211,6 +212,8 @@ void virtio_queue_notify(VirtIODevice *vdev, int n);
uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
void virtio_set_status(VirtIODevice *vdev, uint8_t val);
+void virtio_set_broken(VirtIODevice *vdev);
+bool virtio_broken(VirtIODevice *vdev);
void virtio_reset(void *opaque);
void virtio_update_irq(VirtIODevice *vdev);
int virtio_set_features(VirtIODevice *vdev, uint32_t val);
--
1.9.2
[Qemu-devel] [PATCH] doc: add -drive rerror=, werror= to qemu --help output
These options are already documented on the man page but missing from qemu --help. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- qemu-options.hx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815..664ad87 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -408,7 +408,8 @@ DEF(drive, HAS_ARG, QEMU_OPTION_drive, -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n - [,serial=s][,addr=A][,id=name][,aio=threads|native]\n + [,serial=s][,addr=A][,rerror=ignore|stop|report|enospc]\n + [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n [,readonly=on|off][,copy-on-read=on|off]\n [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n -- 1.9.0
Re: [Qemu-devel] [PATCH v25 02/31] QemuOpts: add def_value_str to QemuOptDesc
On 4/22/2014 at 02:22 AM, in message 535561e2.3000...@redhat.com, Eric Blake ebl...@redhat.com wrote: On 04/10/2014 11:53 AM, Chunyan Liu wrote: Add def_value_str (default value) to QemuOptDesc, to replace function of the default value in QEMUOptionParameter. Improve qemu_opts_get_* functions: if find opt, return opt-str; otherwise, if desc-def_value_str is set, return desc-def_value_str; otherwise, return input defval. Improve qemu_opts_print: if option is set, print opt-str; otherwise, if desc-def_value_str is set, also print it. It will replace print_option_parameters. To avoid print info changes, change qemu_opts_print from fprintf stderr to printf, and remove last printf. This still feels like a bunch to be squashing into one patch. Two possibilities that would have made it nicer to review (either one could be done in isolation, or even doing both if you have a reason to respin): 1. Clearly document in the commit message that there are NO current callers of qemu_opts_print - it is dead code in this patch but later in the series will make use of it, so we are free to change it however we'd like to make it useful when it is no longer dead code 2. This is a lot of change in one patch. Splitting into two parts (repurpose qemu_opts_print, but without default value handling; then add default handling along with the change toe qemu_opts_print to print a default) splits the functionality of the two patches OK. I'll split it into two parts as in v24, and improve description: patch 1 (print default value, same as v22) patch2 (repurpose qemu_opts_print, to replace print_options function, as in v24). But as this series is already gone through so many revisions, and was small enough for me to look at again... Reviewed-by: Leandro Dorileo l...@dorileo.org Reviewed-by: Eric Blake ebl...@redhat.com ...I'm fine with keeping my review. Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Chunyan Liu cy...@suse.com --- changes: * Following Leandro's comment: merge v24 0002-QemuOpts-add-def_value_str-to-QemuOptDesc.patch and v24 0011-qemu_opts_print-change-fprintf-stderr-to-printf.patch into one. Normally, when you make non-trivial changes based on a previous review, it is wise to drop the Reviewed-by for anyone that you want to re-review those changes. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org
[Qemu-devel] [PATCH] block/cloop: use PRIu32 format specifier for uint32_t
PRIu32 is the format string specifier for uint32_t, let's use it.
Variables -block_size, -n_blocks, and i are all uint32_t.
Suggested-by: Max Reitz mre...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/cloop.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/cloop.c b/block/cloop.c
index b6ad50f..8457737 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
}
s-block_size = be32_to_cpu(s-block_size);
if (s-block_size % 512) {
-error_setg(errp, block_size %u must be a multiple of 512,
+error_setg(errp, block_size % PRIu32 must be a multiple of 512,
s-block_size);
return -EINVAL;
}
@@ -86,7 +86,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
* need a buffer this big.
*/
if (s-block_size MAX_BLOCK_SIZE) {
-error_setg(errp, block_size %u must be %u MB or less,
+error_setg(errp, block_size % PRIu32 must be %u MB or less,
s-block_size,
MAX_BLOCK_SIZE / (1024 * 1024));
return -EINVAL;
@@ -101,7 +101,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
/* read offsets */
if (s-n_blocks (UINT32_MAX - 1) / sizeof(uint64_t)) {
/* Prevent integer overflow */
-error_setg(errp, n_blocks %u must be %zu or less,
+error_setg(errp, n_blocks % PRIu32 must be %zu or less,
s-n_blocks,
(UINT32_MAX - 1) / sizeof(uint64_t));
return -EINVAL;
@@ -133,7 +133,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
if (s-offsets[i] s-offsets[i - 1]) {
error_setg(errp, offsets not monotonically increasing at
- index %u, image file is corrupt, i);
+ index % PRIu32 , image file is corrupt, i);
ret = -EINVAL;
goto fail;
}
@@ -146,8 +146,8 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
* ridiculous s-compressed_block allocation.
*/
if (size 2 * MAX_BLOCK_SIZE) {
-error_setg(errp, invalid compressed block size at index %u,
- image file is corrupt, i);
+error_setg(errp, invalid compressed block size at index % PRIu32
+ , image file is corrupt, i);
ret = -EINVAL;
goto fail;
}
--
1.9.0
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi, Gerd and Juan.
Thanks for your guides about the confuse live migration about changing the
keyboard buffer size.
According your suggestion, I got two solutions to address the issue:
- Keep the data array 256 bytes long, change the rptr/wptr/count/data array at
post_load(), both
Ps/2 keyboard and mouse. This solution can be compatible with older qemu
versions, which can
do live migration each other.
-Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the
rptr/wptr/count at
post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct
vmstate_ps2_common.
This solution just save the 16 bytes buffer and drop the rest, So we can't
migrate vm to older qemu versions.
But migration from old qemu to new qemu is ok.
I have tested the two solutions, but which one is better? Expect your reply.
Thanks!
Best regards,
-Gonglei
-Original Message-
From: Juan Quintela [mailto:quint...@redhat.com]
Sent: Tuesday, April 22, 2014 8:05 PM
To: Gerd Hoffmann
Cc: Gonglei (Arei); qemu-devel@nongnu.org; aligu...@amazon.com;
Huangweidong (C)
Subject: Re: [PATCH RFC] ps2: set the keybord output buffer size as the same
as
kernel
Gerd Hoffmann kra...@redhat.com wrote:
On Di, 2014-04-22 at 08:16 +, Gonglei (Arei) wrote:
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 3412079..a754fef 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -71,7 +71,7 @@
#define MOUSE_STATUS_ENABLED0x20
#define MOUSE_STATUS_SCALE210x10
-#define PS2_QUEUE_SIZE 256
+#define PS2_QUEUE_SIZE 16 /* Keyboard output buffer size */
typedef struct {
uint8_t data[PS2_QUEUE_SIZE];
This changes ps2 vmstate and breaks live migration.
Good catch, Gerd.
I got the information in the destination of live migration:
Unknown savevm section type 24
load of migration failed
I'm not familiar with the situation of cross-version live migration,
could you give me
some guide ? Thanks.
Keep the data array 256 bytes long, best with a comment that
compatibility with older qemu versions requires this.
Also the post_load function must handle the case that rptr, wptr count
variables have values which used to be valid for the older qemu versions
but are not valid any more with the smaller queue. In the (unlikely)
case that count is larger than 16 the best you can do is probably simply
throw away the queue. 16 and less queue elements you can move to the
start of the data array (so they are within the 16 bytes still used
after your patch is merged) and adjust rptr+wptr accordingly.
Cc'ing Juan for additional insights.
HTH,
Gerd
static int ps2_common_post_load(void *opaque, int version_id)
{
PS2State *s = opaque;
/* Here goes the code that resets rptr/wptr/count if it is bigger
than p16
Gerd said that dropping the queue is a good idea.
*/
return 0;
}
static const VMStateDescription vmstate_ps2_common = {
.name = PS2 Common State,
.version_id = 3,
.minimum_version_id = 2,
.minimum_version_id_old = 2,
.post_load = ps2_common_post_load,
.fields = (VMStateField[]) {
VMSTATE_INT32(write_cmd, PS2State),
VMSTATE_INT32(queue.rptr, PS2State),
VMSTATE_INT32(queue.wptr, PS2State),
VMSTATE_INT32(queue.count, PS2State),
VMSTATE_BUFFER(queue.data, PS2State),
/* A coment here explaining why we changed the queue from 256 to
16 could be a good idea */
VMSTATE_UNUSED_BUFFER(256-16 );
VMSTATE_END_OF_LIST()
}
};
Hope it helps.
Later, Juan.
[Qemu-devel] [PULL v2 0/2] usb: mtp filesharing
Hi, Here comes the usb patch queue, featuring the MTP (media transfer protocol) device usable for file sharing. For now it is supported on linux hosts and read-only. v2: clarify copyright and licensing as suggested by Eric Blake. please pull, Gerd The following changes since commit 2d03b49c3f225994c4b0b46146437d8c887d6774: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140417-1' into staging (2014-04-17 21:37:26 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-usb-5 for you to fetch changes up to 840a178c94dbd3f5b5550fb8621620cb761de72d: usb: mtp filesharing (2014-04-23 10:28:14 +0200) usb: mtp filesharing Gerd Hoffmann (2): usb: add CompatibleID support to msos usb: mtp filesharing default-configs/usb.mak |1 + hw/usb/Makefile.objs|4 + hw/usb/desc-msos.c |6 +- hw/usb/desc.h |1 + hw/usb/dev-mtp.c| 1103 +++ trace-events| 21 + 6 files changed, 1135 insertions(+), 1 deletion(-) create mode 100644 hw/usb/dev-mtp.c
[Qemu-devel] [PULL v2 1/2] usb: add CompatibleID support to msos
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/desc-msos.c | 6 +-
hw/usb/desc.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/usb/desc-msos.c b/hw/usb/desc-msos.c
index ed8d62c..334d1ae 100644
--- a/hw/usb/desc-msos.c
+++ b/hw/usb/desc-msos.c
@@ -44,7 +44,7 @@ typedef struct msos_compat_hdr {
typedef struct msos_compat_func {
uint8_t bFirstInterfaceNumber;
uint8_t reserved_1;
-uint8_t compatibleId[8];
+char compatibleId[8];
uint8_t subCompatibleId[8];
uint8_t reserved_2[6];
} QEMU_PACKED msos_compat_func;
@@ -59,6 +59,10 @@ static int usb_desc_msos_compat(const USBDesc *desc, uint8_t
*dest)
func = (void *)(dest + length);
func-bFirstInterfaceNumber = 0;
func-reserved_1 = 0x01;
+if (desc-msos-CompatibleID) {
+snprintf(func-compatibleId, sizeof(func-compatibleId),
+ %s, desc-msos-CompatibleID);
+}
length += sizeof(*func);
count++;
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 2b4fcda..8e8db03 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -184,6 +184,7 @@ struct USBDescOther {
};
struct USBDescMSOS {
+const char*CompatibleID;
const wchar_t *Label;
bool SelectiveSuspendEnabled;
};
--
1.8.3.1
[Qemu-devel] [PULL v2 2/2] usb: mtp filesharing
Implementation of a USB Media Transfer Device device for easy
filesharing. Read-only. No access control inside qemu, it will
happily export any file it is able to open to the guest, i.e.
standard unix access rights for the qemu process apply.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
default-configs/usb.mak |1 +
hw/usb/Makefile.objs|4 +
hw/usb/dev-mtp.c| 1103 +++
trace-events| 21 +
4 files changed, 1129 insertions(+)
create mode 100644 hw/usb/dev-mtp.c
diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index 1bf9075..73d8489 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -1,6 +1,7 @@
CONFIG_USB_TABLET_WACOM=y
CONFIG_USB_STORAGE_BOT=y
CONFIG_USB_STORAGE_UAS=y
+CONFIG_USB_STORAGE_MTP=y
CONFIG_USB_SMARTCARD=y
CONFIG_USB_AUDIO=y
CONFIG_USB_SERIAL=y
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 97b4575..17d460c 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -26,6 +26,10 @@ common-obj-y += ccid-card-passthru.o
common-obj-$(CONFIG_SMARTCARD_NSS)+= ccid-card-emulated.o
endif
+ifeq ($(CONFIG_POSIX),y)
+common-obj-$(CONFIG_USB_STORAGE_MTP) += dev-mtp.o
+endif
+
# usb redirection
common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
new file mode 100644
index 000..8b44032
--- /dev/null
+++ b/hw/usb/dev-mtp.c
@@ -0,0 +1,1103 @@
+/*
+ * Media Transfer Protocol implementation, backed by host filesystem.
+ *
+ * Copyright Red Hat, Inc 2014
+ *
+ * Author:
+ * Gerd Hoffmann kra...@redhat.com
+ *
+ * This code is licensed under the GPL v2 or later.
+ */
+
+#include wchar.h
+#include dirent.h
+#include unistd.h
+
+#include sys/stat.h
+#include sys/statvfs.h
+
+#include qemu-common.h
+#include qemu/iov.h
+#include trace.h
+#include hw/usb.h
+#include hw/usb/desc.h
+
+/* --- */
+
+enum mtp_container_type {
+TYPE_COMMAND = 1,
+TYPE_DATA = 2,
+TYPE_RESPONSE = 3,
+TYPE_EVENT= 4,
+};
+
+enum mtp_code {
+/* command codes */
+CMD_GET_DEVICE_INFO= 0x1001,
+CMD_OPEN_SESSION = 0x1002,
+CMD_CLOSE_SESSION = 0x1003,
+CMD_GET_STORAGE_IDS= 0x1004,
+CMD_GET_STORAGE_INFO = 0x1005,
+CMD_GET_NUM_OBJECTS= 0x1006,
+CMD_GET_OBJECT_HANDLES = 0x1007,
+CMD_GET_OBJECT_INFO= 0x1008,
+CMD_GET_OBJECT = 0x1009,
+CMD_GET_PARTIAL_OBJECT = 0x101b,
+
+/* response codes */
+RES_OK = 0x2001,
+RES_SESSION_NOT_OPEN = 0x2003,
+RES_INVALID_TRANSACTION_ID = 0x2004,
+RES_OPERATION_NOT_SUPPORTED= 0x2005,
+RES_PARAMETER_NOT_SUPPORTED= 0x2006,
+RES_INVALID_STORAGE_ID = 0x2008,
+RES_INVALID_OBJECT_HANDLE = 0x2009,
+RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
+RES_INVALID_PARENT_OBJECT = 0x201a,
+RES_INVALID_PARAMETER = 0x201d,
+RES_SESSION_ALREADY_OPEN = 0x201e,
+
+/* format codes */
+FMT_UNDEFINED_OBJECT = 0x3000,
+FMT_ASSOCIATION= 0x3001,
+};
+
+typedef struct {
+uint32_t length;
+uint16_t type;
+uint16_t code;
+uint32_t trans;
+} QEMU_PACKED mtp_container;
+
+/* --- */
+
+typedef struct MTPState MTPState;
+typedef struct MTPControl MTPControl;
+typedef struct MTPData MTPData;
+typedef struct MTPObject MTPObject;
+
+enum {
+EP_DATA_IN = 1,
+EP_DATA_OUT,
+EP_EVENT,
+};
+
+struct MTPControl {
+uint16_t code;
+uint32_t trans;
+int argc;
+uint32_t argv[5];
+};
+
+struct MTPData {
+uint16_t code;
+uint32_t trans;
+uint32_t offset;
+uint32_t length;
+uint32_t alloc;
+uint8_t *data;
+bool first;
+int fd;
+};
+
+struct MTPObject {
+uint32_t handle;
+uint16_t format;
+char *name;
+char *path;
+struct stat stat;
+MTPObject*parent;
+MTPObject**children;
+int32_t nchildren;
+QTAILQ_ENTRY(MTPObject) next;
+};
+
+struct MTPState {
+USBDevicedev;
+char *root;
+char *desc;
+uint32_t flags;
+
+MTPData *data_in;
+MTPData *data_out;
+MTPControl *result;
+uint32_t session;
+uint32_t next_handle;
+
+QTAILQ_HEAD(, MTPObject) objects;
+};
+
+#define QEMU_STORAGE_ID 0x00010001
+
+#define MTP_FLAG_WRITABLE 0
+
+#define FLAG_SET(_mtp, _flag) ((_mtp)-flags (1 (_flag)))
+
+/* --- */
+
+#define MTP_MANUFACTURER QEMU
+#define MTP_PRODUCT QEMU filesharing
+
+enum {
+STR_MANUFACTURER = 1,
+
Re: [Qemu-devel] [PATCH] ivshmem: fix potential OOB r/w access
On Wed, Apr 16, 2014 at 09:33:02AM +0200, Sebastian Krahmer wrote:
Fix OOB access via malformed incoming_posn parameters
and check that requested memory is actually alloced.
Signed-off-by: Sebastian Krahmer krah...@suse.de
---
Please use scripts/checkpatch.pl to check coding style. QEMU always
uses {} even for a one-line if body.
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 8d144ba..f58356e 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -28,6 +28,7 @@
#include sys/mman.h
#include sys/types.h
+#include limits.h
#define PCI_VENDOR_ID_IVSHMEM PCI_VENDOR_ID_REDHAT_QUMRANET
#define PCI_DEVICE_ID_IVSHMEM 0x1110
@@ -401,23 +402,34 @@ static void close_guest_eventfds(IVShmemState *s, int
posn)
/* this function increase the dynamic storage need to store data about other
* guests */
-static void increase_dynamic_storage(IVShmemState *s, int new_min_size) {
+static int increase_dynamic_storage(IVShmemState *s, int new_min_size) {
int j, old_nb_alloc;
+/* check for integer overflow */
+if (new_min_size = INT_MAX/sizeof(Peer) - 1 || new_min_size = 0)
+return -1;
+
old_nb_alloc = s-nb_peers;
-while (new_min_size = s-nb_peers)
-s-nb_peers = s-nb_peers * 2;
+/* heap allocators already have good alloc strategies, no need
+ * to re-implement here. +1 because #new_min_size is used as last array
index */
+if (new_min_size = s-nb_peers)
+s-nb_peers = new_min_size + 1;
+else
+return 0;
IVSHMEM_DPRINTF(bumping storage to %d guests\n, s-nb_peers);
s-peers = g_realloc(s-peers, s-nb_peers * sizeof(Peer));
+if (!s-peers)
+return -1;
This is not needed because g_realloc() does not return NULL on
out-of-memory:
https://developer.gnome.org/glib/unstable/glib-Memory-Allocation.html#g-realloc
/* zero out new pointers */
for (j = old_nb_alloc; j s-nb_peers; j++) {
s-peers[j].eventfds = NULL;
s-peers[j].nb_eventfds = 0;
}
+return 0;
}
static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
@@ -428,13 +440,19 @@ static void ivshmem_read(void *opaque, const uint8_t *
buf, int flags)
long incoming_posn;
memcpy(incoming_posn, buf, sizeof(long));
+if (incoming_posn -1) {
+fprintf(stderr, invalid posn index\n);
+return;
+}
/* pick off s-server_chr-msgfd and store it, posn should accompany msg
*/
tmp_fd = qemu_chr_fe_get_msgfd(s-server_chr);
IVSHMEM_DPRINTF(posn is %ld, fd is %d\n, incoming_posn, tmp_fd);
-
/* make sure we have enough space for this guest */
if (incoming_posn = s-nb_peers) {
-increase_dynamic_storage(s, incoming_posn);
+if (increase_dynamic_storage(s, incoming_posn) 0) {
+fprintf(stderr, could not allocate memory\n);
This error message is wrong because the failure could also indicate
out-of-range incoming_posn.
+return;
If tmp_fd != -1 this return leaks it.
Re: [Qemu-devel] [PATCH] Unnecessary comma.
On Wed, Apr 16, 2014 at 05:43:07PM +0400, Igor Ryzhov wrote: Signed-off-by: Igor Ryzhov iryz...@arccn.ru --- net/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) No need to resend, I fixed up the commit message. Thanks, applied to my net-next tree: https://github.com/stefanha/qemu/commits/net-next Stefan
Re: [Qemu-devel] [PATCH v5 02/12] qcow2: Implement bdrv_make_empty()
Am 22.04.2014 um 18:02 hat Max Reitz geschrieben: On 22.04.2014 16:23, Kevin Wolf wrote: QCOW2_DISCARD_REQUEST is for requests that come from the guest. SNAPSHOT would be a nice fit if we reinterpreted it so that it doesn't only refer to internal snapshots but also to external ones. I would be okay with OTHER as well if you prefer. Perhaps a look at the defaults can help us: SNAPSHOT defaults to on, OTHER to off. I think it totally makes sense to physically discard clusters in qcow2_make_empty() in almost every case, so that might be a good reason to use SNAPSHOT here. My problem with using SNAPSHOT would be that this function is that there is no indication of this function being meant for committing snapshots only; however, in practice it is obviously only used for that purpose, thus I can accept it. It all depends on what you call a snapshot. I think an image with a backing file constitutes a snapshot pretty much by definition. But I know that some people differentiate snapshots and templates, or things like that. But I do like reasoning with the defaults. ;-) :-) I'll go for SNAPSHOT then and include an explanatory comment. Great, thanks. Kevin
[Qemu-devel] Google Summer of Code and Outreach Program for Women accepted projects
Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. Accepted projects have now been announced: Intel IOMMU (VT-d) Emulation - Candidate: Le Tan Mentor: Jan Kiszka Device driver framework for low-level testing - Candidate: Marc Marí Mentors: Paolo Bonzini and Stefan Hajnoczi Disk image fuzz testing - Candidate: Maria Kustova (maxa) Mentors: Fam Zheng and Stefan Hajnoczi TianoCore support for Apple's boot.efi - Candidate: Reza Jelveh Mentors: Alexander Graf and Gabriel Somlo Continuous vmstate testing and dirty bitmap logging - Candidate: sanidhya Mentor: Juan Quintela Rewriting code of vbox driver for libvirt - Candidate: T A Mahadevan Mentor: Michal Privoznik Libvirt: Job Control for the Storage Driver - Candidate: TuckerD Mentor: Martin Kletzander Applying for the idea rewrite virtual box's driver - Candidate: uaedante Mentor: Michal Privoznik Congratulations to our accepted candidates and welcome to the QEMU, libvirt, and KVM communities! Stefan
Re: [Qemu-devel] [PATCH v5 07/12] qemu-img: Empty images after commit
Am 22.04.2014 um 18:22 hat Max Reitz geschrieben: On 22.04.2014 17:19, Eric Blake wrote: On 04/17/2014 03:59 PM, Max Reitz wrote: After the top image has been committed into an image in its backing chain, all images above that base image should be emptied to restore the old qemu-img commit behavior. Signed-off-by: Max Reitz mre...@redhat.com --- qemu-img.c | 87 +++--- 1 file changed, 84 insertions(+), 3 deletions(-) Does emptying an image take significant time? If so, does that need to be reflected in the progress meter? For a 16 GB image I have here (should be nearly full) it took 1:22 min. Copying it took six minutes, so I guess committing it would take even more. I think the ratio is small enough not to include it in the progress meter. Did you check why it took that long? Sounds like we're issuing a lot of independent discard requests instead of few big ones. Is the image heavily fragmented? Furthermore, I don't see a reasonable implementation for a make_empty progress output: As a general implementation for all image formats implementing discard will not work (the qcow2 implementation clearly states that discarded sectors should read back as zero) We can always add new flags or a separate new callback if it improves things. Kevin
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. I have tested the two solutions, but which one is better? Expect your reply. Thanks! Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] block/cloop: use PRIu32 format specifier for uint32_t
Am 23.04.2014 um 10:05 hat Stefan Hajnoczi geschrieben: PRIu32 is the format string specifier for uint32_t, let's use it. Variables -block_size, -n_blocks, and i are all uint32_t. Suggested-by: Max Reitz mre...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
On Mi, 2014-04-23 at 08:06 +, Gonglei (Arei) wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. I think long term we want #2, but using #1 as step inbetween for a few releases until 16 byte buffer size is widely used will might be useful. Completely separate question: Have you figured what the root cause for the bug is? While wading through the code I've figured the queue size isn't (directly) exposed to the guest. And it's used for both mouse and kbd. For the kbd 16 byte buffer is probably ok, but for mouse events we probably want keep some more space to buffer events ... cheers, Gerd
Re: [Qemu-devel] [PATCH] doc: add -drive rerror=, werror= to qemu --help output
Am 23.04.2014 um 09:36 hat Stefan Hajnoczi geschrieben: These options are already documented on the man page but missing from qemu --help. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- qemu-options.hx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815..664ad87 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -408,7 +408,8 @@ DEF(drive, HAS_ARG, QEMU_OPTION_drive, -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n - [,serial=s][,addr=A][,id=name][,aio=threads|native]\n + [,serial=s][,addr=A][,rerror=ignore|stop|report|enospc]\n enospc doesn't exist for rerror. + [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n [,readonly=on|off][,copy-on-read=on|off]\n [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n Kevin
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Gonglei (Arei) arei.gong...@huawei.com wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. Second solution is better in the sense that it allows forward and backward migration. The first one only allows forward migration. Please use second one. Later, Juan. I have tested the two solutions, but which one is better? Expect your reply. Thanks! Best regards, -Gonglei
[Qemu-devel] [PULL 01/16] block: Fix nb_sectors check in bdrv_check_byte_request()
nb_sectors is signed, check for negative values.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Max Reitz mre...@redhat.com
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 990a754..3b7951e 100644
--- a/block.c
+++ b/block.c
@@ -2601,7 +2601,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs,
int64_t offset,
static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
-if (nb_sectors INT_MAX / BDRV_SECTOR_SIZE) {
+if (nb_sectors 0 || nb_sectors INT_MAX / BDRV_SECTOR_SIZE) {
return -EIO;
}
--
1.8.3.1
[Qemu-devel] [PULL 12/16] block: Catch duplicate IDs in bdrv_new()
Since commit f298d071, block devices added with blockdev-add don't have
a QemuOpts around in dinfo-opts. Consequently, we can't rely any more
on QemuOpts catching duplicate IDs for block devices.
This patch adds a new check for duplicate IDs to bdrv_new(), and moves
the existing check that the ID isn't already taken for a node-name there
as well.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
block.c| 11 +++
blockdev.c | 6 --
tests/qemu-iotests/087 | 33 +
tests/qemu-iotests/087.out | 13 +
4 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index f3b93c9..fc2edd3 100644
--- a/block.c
+++ b/block.c
@@ -336,6 +336,17 @@ BlockDriverState *bdrv_new(const char *device_name, Error
**errp)
{
BlockDriverState *bs;
+if (bdrv_find(device_name)) {
+error_setg(errp, Device with id '%s' already exists,
+ device_name);
+return NULL;
+}
+if (bdrv_find_node(device_name)) {
+error_setg(errp, Device with node-name '%s' already exists,
+ device_name);
+return NULL;
+}
+
bs = g_malloc0(sizeof(BlockDriverState));
QLIST_INIT(bs-dirty_bitmaps);
pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
diff --git a/blockdev.c b/blockdev.c
index 3a11a62..09826f1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -452,12 +452,6 @@ static DriveInfo *blockdev_init(const char *file, QDict
*bs_opts,
}
}
-if (bdrv_find_node(qemu_opts_id(opts))) {
-error_setg(errp, device id=%s is conflicting with a node-name,
- qemu_opts_id(opts));
-goto early_err;
-}
-
/* init */
dinfo = g_malloc0(sizeof(*dinfo));
dinfo-id = g_strdup(qemu_opts_id(opts));
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index a38bb70..37d82fc 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -73,6 +73,39 @@ run_qemu EOF
EOF
echo
+echo === Duplicate ID ===
+echo
+
+run_qemu EOF
+{ execute: qmp_capabilities }
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: disk,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: disk,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
+{ execute: quit }
+EOF
+
+echo
echo === aio=native without O_DIRECT ===
echo
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index e65dcdf..479bf86 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -13,6 +13,19 @@ QMP_VERSION
{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
DEVICE_TRAY_MOVED, data: {device: floppy0, tray-open: true}}
+=== Duplicate ID ===
+
+Testing:
+QMP_VERSION
+{return: {}}
+{return: {}}
+{error: {class: GenericError, desc: Device with id 'disk' already
exists}}
+{return: {}}
+{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
SHUTDOWN}
+{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
DEVICE_TRAY_MOVED, data: {device: ide1-cd0, tray-open: true}}
+{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
DEVICE_TRAY_MOVED, data: {device: floppy0, tray-open: true}}
+
+
=== aio=native without O_DIRECT ===
Testing:
--
1.8.3.1
[Qemu-devel] [PULL 05/16] vmdk: Fix %d and %lld to PRI* in format strings
From: Fam Zheng f...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/vmdk.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index b69988d..938a183 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -640,7 +640,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (le32_to_cpu(header.version) 3) {
char buf[64];
-snprintf(buf, sizeof(buf), VMDK version %d,
+snprintf(buf, sizeof(buf), VMDK version % PRId32,
le32_to_cpu(header.version));
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
bs-device_name, vmdk, buf);
@@ -671,8 +671,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
}
if (bdrv_getlength(file)
le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE) {
-error_setg(errp, File truncated, expecting at least %lld bytes,
- le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE);
+error_setg(errp, File truncated, expecting at least % PRId64
bytes,
+ (int64_t)(le64_to_cpu(header.grain_offset)
+ * BDRV_SECTOR_SIZE));
return -EINVAL;
}
@@ -1720,7 +1721,7 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options,
\n
ddb.virtualHWVersion = \%d\\n
ddb.geometry.cylinders = \% PRId64 \\n
-ddb.geometry.heads = \%d\\n
+ddb.geometry.heads = \% PRIu32 \\n
ddb.geometry.sectors = \63\\n
ddb.adapterType = \%s\\n;
@@ -1780,9 +1781,9 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options,
strcmp(fmt, twoGbMaxExtentFlat));
compress = !strcmp(fmt, streamOptimized);
if (flat) {
-desc_extent_line = RW %lld FLAT \%s\ 0\n;
+desc_extent_line = RW % PRId64 FLAT \%s\ 0\n;
} else {
-desc_extent_line = RW %lld SPARSE \%s\\n;
+desc_extent_line = RW % PRId64 SPARSE \%s\\n;
}
if (flat backing_file) {
error_setg(errp, Flat image can't have backing file);
--
1.8.3.1
[Qemu-devel] [PULL 13/16] qemu-iotests: Check common namespace for id and node-name
A name that is taken by an ID can't be taken by a node-name at the same
time. Check that conflicts are correctly detected.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Fam Zheng f...@redhat.com
---
tests/qemu-iotests/087 | 52 ++
tests/qemu-iotests/087.out | 5 +
2 files changed, 57 insertions(+)
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 37d82fc..82c56b1 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -83,6 +83,7 @@ run_qemu EOF
options: {
driver: $IMGFMT,
id: disk,
+node-name: test-node,
file: {
driver: file,
filename: $TEST_IMG
@@ -102,6 +103,57 @@ run_qemu EOF
}
}
}
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: test-node,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: disk2,
+node-name: disk,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: disk2,
+node-name: test-node,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
+{ execute: blockdev-add,
+ arguments: {
+ options: {
+driver: $IMGFMT,
+id: disk3,
+node-name: disk3,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+ }
+}
+ }
{ execute: quit }
EOF
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 479bf86..7fbee3f 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -20,6 +20,11 @@ QMP_VERSION
{return: {}}
{return: {}}
{error: {class: GenericError, desc: Device with id 'disk' already
exists}}
+{error: {class: GenericError, desc: Device with node-name 'test-node'
already exists}}
+main-loop: WARNING: I/O thread spun for 1000 iterations
+{error: {class: GenericError, desc: could not open disk image disk2:
node-name=disk is conflicting with a device id}}
+{error: {class: GenericError, desc: could not open disk image disk2:
Duplicate node name}}
+{error: {class: GenericError, desc: could not open disk image disk3:
node-name=disk3 is conflicting with a device id}}
{return: {}}
{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
SHUTDOWN}
{timestamp: {seconds: TIMESTAMP, microseconds: TIMESTAMP}, event:
DEVICE_TRAY_MOVED, data: {device: ide1-cd0, tray-open: true}}
--
1.8.3.1
[Qemu-devel] [PULL 02/16] block: Limit size to INT_MAX in bdrv_check_byte_request()
Commit 8f4754ed intended to protect against integer overflow bugs in
block drivers by making sure that a single request that is passed to
drivers is no longer than INT_MAX bytes.
However, meanwhile there are some callers that don't use that code path
any more but call bdrv_check_byte_request() directy, so let's add a
check there as well.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Max Reitz mre...@redhat.com
---
block.c | 4
1 file changed, 4 insertions(+)
diff --git a/block.c b/block.c
index 3b7951e..5a0b421 100644
--- a/block.c
+++ b/block.c
@@ -2581,6 +2581,10 @@ static int bdrv_check_byte_request(BlockDriverState *bs,
int64_t offset,
{
int64_t len;
+if (size INT_MAX) {
+return -EIO;
+}
+
if (!bdrv_is_inserted(bs))
return -ENOMEDIUM;
--
1.8.3.1
[Qemu-devel] [PULL 14/16] qemu-img: Improve error messages
From: Fam Zheng f...@redhat.com
Previously, when there is a user error in argv parsing, qemu-img prints
help text and exits.
Add an error_exit function to print a helpful error message and a hint
to run 'qemu-img --help' for more information.
As a bonus, qemu-img cmd --help now has a more reasonable exit code
0.
In the future the help text should be split by sub command, and only
print the information for the specified command.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
qemu-img.c | 74 ++
1 file changed, 46 insertions(+), 28 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index fb626ac..4dae84a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -57,8 +57,22 @@ static void format_print(void *opaque, const char *name)
printf( %s, name);
}
+static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
+{
+va_list ap;
+
+error_printf(qemu-img: );
+
+va_start(ap, fmt);
+error_vprintf(fmt, ap);
+va_end(ap);
+
+error_printf(\nTry 'qemu-img --help' for more information\n);
+exit(EXIT_FAILURE);
+}
+
/* Please keep in synch with qemu-img.texi */
-static void help(void)
+static void QEMU_NORETURN help(void)
{
const char *help_msg =
qemu-img version QEMU_VERSION , Copyright (c) 2004-2008 Fabrice
Bellard\n
@@ -129,7 +143,7 @@ static void help(void)
printf(%s\nSupported formats:, help_msg);
bdrv_iterate_format(format_print, NULL);
printf(\n);
-exit(1);
+exit(EXIT_SUCCESS);
}
static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
@@ -399,7 +413,7 @@ static int img_create(int argc, char **argv)
}
if (optind = argc) {
-help();
+error_exit(Expecting image file name);
}
optind++;
@@ -422,7 +436,7 @@ static int img_create(int argc, char **argv)
img_size = (uint64_t)sval;
}
if (optind != argc) {
-help();
+error_exit(Unexpected argument: %s, argv[optind]);
}
bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -591,7 +605,8 @@ static int img_check(int argc, char **argv)
} else if (!strcmp(optarg, all)) {
fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
} else {
-help();
+error_exit(Unknown option value for -r
+ (expecting 'leaks' or 'all'): %s, optarg);
}
break;
case OPTION_OUTPUT:
@@ -603,7 +618,7 @@ static int img_check(int argc, char **argv)
}
}
if (optind != argc - 1) {
-help();
+error_exit(Expecting one image file name);
}
filename = argv[optind++];
@@ -714,7 +729,7 @@ static int img_commit(int argc, char **argv)
}
}
if (optind != argc - 1) {
-help();
+error_exit(Expecting one image file name);
}
filename = argv[optind++];
@@ -960,7 +975,7 @@ static int img_compare(int argc, char **argv)
if (optind != argc - 2) {
-help();
+error_exit(Expecting two image file names);
}
filename1 = argv[optind++];
filename2 = argv[optind++];
@@ -1276,7 +1291,7 @@ static int img_convert(int argc, char **argv)
}
if (bs_n 1) {
-help();
+error_exit(Must specify image file name);
}
@@ -1886,7 +1901,7 @@ static int img_info(int argc, char **argv)
}
}
if (optind != argc - 1) {
-help();
+error_exit(Expecting one image file name);
}
filename = argv[optind++];
@@ -2050,10 +2065,10 @@ static int img_map(int argc, char **argv)
break;
}
}
-if (optind = argc) {
-help();
+if (optind != argc - 1) {
+error_exit(Expecting one image file name);
}
-filename = argv[optind++];
+filename = argv[optind];
if (output !strcmp(output, json)) {
output_format = OFORMAT_JSON;
@@ -2142,7 +2157,7 @@ static int img_snapshot(int argc, char **argv)
return 0;
case 'l':
if (action) {
-help();
+error_exit(Cannot mix '-l', '-a', '-c', '-d');
return 0;
}
action = SNAPSHOT_LIST;
@@ -2150,7 +2165,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'a':
if (action) {
-help();
+error_exit(Cannot mix '-l', '-a', '-c', '-d');
return 0;
}
action = SNAPSHOT_APPLY;
@@ -2158,7 +2173,7 @@ static int img_snapshot(int argc, char **argv)
break;
case 'c':
if (action) {
-help();
+error_exit(Cannot mix '-l', '-a', '-c', '-d');
return 0;
}
action = SNAPSHOT_CREATE;
@@ -2166,7 +2181,7 @@ static int img_snapshot(int argc, char
[Qemu-devel] [PULL 07/16] curl: Replaced old error handling with error reporting API.
From: Maria Kustova m...@catit.be Signed-off-by: Maria Kustova mari...@catit.be Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Signed-off-by: Kevin Wolf kw...@redhat.com --- block/curl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/curl.c b/block/curl.c index 1b9b1f6..6731d28 100644 --- a/block/curl.c +++ b/block/curl.c @@ -543,7 +543,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, return 0; out: -fprintf(stderr, CURL: Error opening file: %s\n, state-errmsg); +error_setg(errp, CURL: Error opening file: %s, state-errmsg); curl_easy_cleanup(state-curl); state-curl = NULL; out_noclean: -- 1.8.3.1
[Qemu-devel] [PULL 08/16] block: Remove -errno return value from bdrv_assign_node_name
It takes an errp argument. That's enough for error handling.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 0c81747..0ff5764 100644
--- a/block.c
+++ b/block.c
@@ -788,38 +788,36 @@ static int bdrv_open_flags(BlockDriverState *bs, int
flags)
return open_flags;
}
-static int bdrv_assign_node_name(BlockDriverState *bs,
- const char *node_name,
- Error **errp)
+static void bdrv_assign_node_name(BlockDriverState *bs,
+ const char *node_name,
+ Error **errp)
{
if (!node_name) {
-return 0;
+return;
}
/* empty string node name is invalid */
if (node_name[0] == '\0') {
error_setg(errp, Empty node name);
-return -EINVAL;
+return;
}
/* takes care of avoiding namespaces collisions */
if (bdrv_find(node_name)) {
error_setg(errp, node-name=%s is conflicting with a device id,
node_name);
-return -EINVAL;
+return;
}
/* takes care of avoiding duplicates node names */
if (bdrv_find_node(node_name)) {
error_setg(errp, Duplicate node name);
-return -EINVAL;
+return;
}
/* copy node name into the bs and insert it into the graph list */
pstrcpy(bs-node_name, sizeof(bs-node_name), node_name);
QTAILQ_INSERT_TAIL(graph_bdrv_states, bs, node_list);
-
-return 0;
}
/*
@@ -854,9 +852,10 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
trace_bdrv_open_common(bs, filename ?: , flags, drv-format_name);
node_name = qdict_get_try_str(options, node-name);
-ret = bdrv_assign_node_name(bs, node_name, errp);
-if (ret 0) {
-return ret;
+bdrv_assign_node_name(bs, node_name, local_err);
+if (error_is_set(local_err)) {
+error_propagate(errp, local_err);
+return -EINVAL;
}
qdict_del(options, node-name);
--
1.8.3.1
[Qemu-devel] [PULL 15/16] vmdk: Fix %x to PRIx32 in format strings for cid
From: Fam Zheng f...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/vmdk.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 938a183..06a1f9f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -262,7 +262,7 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int
parent)
p_name = strstr(desc, cid_str);
if (p_name != NULL) {
p_name += cid_str_size;
-sscanf(p_name, %x, cid);
+sscanf(p_name, % SCNx32, cid);
}
return cid;
@@ -290,7 +290,7 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t
cid)
p_name = strstr(desc, CID);
if (p_name != NULL) {
p_name += sizeof(CID);
-snprintf(p_name, sizeof(desc) - (p_name - desc), %x\n, cid);
+snprintf(p_name, sizeof(desc) - (p_name - desc), % PRIx32 \n, cid);
pstrcat(desc, sizeof(desc), tmp_desc);
}
@@ -1708,8 +1708,8 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options,
const char desc_template[] =
# Disk DescriptorFile\n
version=1\n
-CID=%x\n
-parentCID=%x\n
+CID=% PRIx32 \n
+parentCID=% PRIx32 \n
createType=\%s\\n
%s
\n
@@ -1851,7 +1851,7 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options,
}
/* generate descriptor file */
desc = g_strdup_printf(desc_template,
- (unsigned int)time(NULL),
+ (uint32_t)time(NULL),
parent_cid,
fmt,
parent_desc_line,
--
1.8.3.1
[Qemu-devel] [PULL 10/16] block: Add errp to bdrv_new()
This patch adds an errp parameter to bdrv_new() and updates all its
callers. The next patches will make use of this in order to check for
duplicate IDs. Most of the callers know that their ID is fine, so they
can simply assert that there is no error.
Behaviour doesn't change with this patch yet as bdrv_new() doesn't
actually assign errors to errp.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
block.c | 6 +++---
block/iscsi.c | 2 +-
block/vvfat.c | 2 +-
blockdev.c| 9 +++--
hw/block/xen_disk.c | 7 +--
include/block/block.h | 2 +-
qemu-img.c| 6 +++---
qemu-io.c | 2 +-
qemu-nbd.c| 3 ++-
9 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index 0ff5764..f3b93c9 100644
--- a/block.c
+++ b/block.c
@@ -332,7 +332,7 @@ void bdrv_register(BlockDriver *bdrv)
}
/* create a new block device (by default it is empty) */
-BlockDriverState *bdrv_new(const char *device_name)
+BlockDriverState *bdrv_new(const char *device_name, Error **errp)
{
BlockDriverState *bs;
@@ -1220,7 +1220,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
Error **errp)
qdict_put(snapshot_options, file.filename,
qstring_from_str(tmp_filename));
-bs_snapshot = bdrv_new();
+bs_snapshot = bdrv_new(, error_abort);
bs_snapshot-is_temporary = 1;
ret = bdrv_open(bs_snapshot, NULL, NULL, snapshot_options,
@@ -1287,7 +1287,7 @@ int bdrv_open(BlockDriverState **pbs, const char
*filename,
if (*pbs) {
bs = *pbs;
} else {
-bs = bdrv_new();
+bs = bdrv_new(, error_abort);
}
/* NULL means an empty set of options */
diff --git a/block/iscsi.c b/block/iscsi.c
index f425573..a636ea4 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1401,7 +1401,7 @@ static int iscsi_create(const char *filename,
QEMUOptionParameter *options,
IscsiLun *iscsilun = NULL;
QDict *bs_options;
-bs = bdrv_new();
+bs = bdrv_new(, error_abort);
/* Read out options */
while (options options-name) {
diff --git a/block/vvfat.c b/block/vvfat.c
index 1978c9e..c3af7ff 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2947,7 +2947,7 @@ static int enable_write_target(BDRVVVFATState *s)
unlink(s-qcow_filename);
#endif
-s-bs-backing_hd = bdrv_new();
+s-bs-backing_hd = bdrv_new(, error_abort);
s-bs-backing_hd-drv = vvfat_write_target;
s-bs-backing_hd-opaque = g_malloc(sizeof(void*));
*(void**)s-bs-backing_hd-opaque = s;
diff --git a/blockdev.c b/blockdev.c
index 5dd01ea..3a11a62 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -461,7 +461,11 @@ static DriveInfo *blockdev_init(const char *file, QDict
*bs_opts,
/* init */
dinfo = g_malloc0(sizeof(*dinfo));
dinfo-id = g_strdup(qemu_opts_id(opts));
-dinfo-bdrv = bdrv_new(dinfo-id);
+dinfo-bdrv = bdrv_new(dinfo-id, error);
+if (error) {
+error_propagate(errp, error);
+goto bdrv_new_err;
+}
dinfo-bdrv-open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
dinfo-bdrv-read_only = ro;
dinfo-refcount = 1;
@@ -523,8 +527,9 @@ static DriveInfo *blockdev_init(const char *file, QDict
*bs_opts,
err:
bdrv_unref(dinfo-bdrv);
-g_free(dinfo-id);
QTAILQ_REMOVE(drives, dinfo, next);
+bdrv_new_err:
+g_free(dinfo-id);
g_free(dinfo);
early_err:
QDECREF(bs_opts);
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index bc061e6..a8fea72 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -817,11 +817,14 @@ static int blk_connect(struct XenDevice *xendev)
index = (blkdev-xendev.dev - 202 * 256) / 16;
blkdev-dinfo = drive_get(IF_XEN, 0, index);
if (!blkdev-dinfo) {
+Error *local_err = NULL;
/* setup via xenbus - create new block driver instance */
xen_be_printf(blkdev-xendev, 2, create new bdrv (xenbus setup)\n);
-blkdev-bs = bdrv_new(blkdev-dev);
+blkdev-bs = bdrv_new(blkdev-dev, local_err);
+if (local_err) {
+blkdev-bs = NULL;
+}
if (blkdev-bs) {
-Error *local_err = NULL;
BlockDriver *drv = bdrv_find_whitelisted_format(blkdev-fileproto,
readonly);
if (bdrv_open(blkdev-bs, blkdev-filename, NULL, NULL, qflags,
diff --git a/include/block/block.h b/include/block/block.h
index 2b51eec..c12808a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -180,7 +180,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
QEMUOptionParameter *options, Error **errp);
int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
Error **errp);
-BlockDriverState *bdrv_new(const char *device_name);
+BlockDriverState *bdrv_new(const char *device_name, Error **errp);
void bdrv_make_anon(BlockDriverState *bs);
void
[Qemu-devel] [PULL 16/16] block/cloop: use PRIu32 format specifier for uint32_t
From: Stefan Hajnoczi stefa...@redhat.com
PRIu32 is the format string specifier for uint32_t, let's use it.
Variables -block_size, -n_blocks, and i are all uint32_t.
Suggested-by: Max Reitz mre...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/cloop.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/cloop.c b/block/cloop.c
index b6ad50f..8457737 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
}
s-block_size = be32_to_cpu(s-block_size);
if (s-block_size % 512) {
-error_setg(errp, block_size %u must be a multiple of 512,
+error_setg(errp, block_size % PRIu32 must be a multiple of 512,
s-block_size);
return -EINVAL;
}
@@ -86,7 +86,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
* need a buffer this big.
*/
if (s-block_size MAX_BLOCK_SIZE) {
-error_setg(errp, block_size %u must be %u MB or less,
+error_setg(errp, block_size % PRIu32 must be %u MB or less,
s-block_size,
MAX_BLOCK_SIZE / (1024 * 1024));
return -EINVAL;
@@ -101,7 +101,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
/* read offsets */
if (s-n_blocks (UINT32_MAX - 1) / sizeof(uint64_t)) {
/* Prevent integer overflow */
-error_setg(errp, n_blocks %u must be %zu or less,
+error_setg(errp, n_blocks % PRIu32 must be %zu or less,
s-n_blocks,
(UINT32_MAX - 1) / sizeof(uint64_t));
return -EINVAL;
@@ -133,7 +133,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
if (s-offsets[i] s-offsets[i - 1]) {
error_setg(errp, offsets not monotonically increasing at
- index %u, image file is corrupt, i);
+ index % PRIu32 , image file is corrupt, i);
ret = -EINVAL;
goto fail;
}
@@ -146,8 +146,8 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
* ridiculous s-compressed_block allocation.
*/
if (size 2 * MAX_BLOCK_SIZE) {
-error_setg(errp, invalid compressed block size at index %u,
- image file is corrupt, i);
+error_setg(errp, invalid compressed block size at index % PRIu32
+ , image file is corrupt, i);
ret = -EINVAL;
goto fail;
}
--
1.8.3.1
Re: [Qemu-devel] [QEMU v7 PATCH 0/7] SMBIOS: build full tables in QEMU
Hi,
Thanks again for any feedback and comments (or for just applying the patch) !
Series looks good to me.
/me wonders though how you've tested the final revision and the compat
stuff. Patch #6 doesn't actually add a new machine type where the
non-legacy mode is active ...
Incremental patch attached.
I'll go wait for a few days for additional feedback, then go squash the
fix into patch #6 and send out a pull request.
cheers,
Gerd
From fa45a8993706fe7c31a5a2f38f5c90d061275203 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Wed, 23 Apr 2014 11:56:32 +0200
Subject: [PATCH] 2.1 machine type fixup
---
hw/i386/pc_piix.c | 11 +--
hw/i386/pc_q35.c | 9 -
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f3e2c4e..7de9507 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -400,12 +400,18 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
PC_I440FX_MACHINE_OPTIONS, \
.default_machine_opts = firmware=bios-256k.bin
+static QEMUMachine pc_i440fx_machine_v2_1 = {
+PC_I440FX_2_0_MACHINE_OPTIONS,
+.name = pc-i440fx-2.1,
+.alias = pc,
+.init = pc_init_pci,
+.is_default = 1,
+};
+
static QEMUMachine pc_i440fx_machine_v2_0 = {
PC_I440FX_2_0_MACHINE_OPTIONS,
.name = pc-i440fx-2.0,
-.alias = pc,
.init = pc_init_pci_2_0,
-.is_default = 1,
};
#define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
@@ -830,6 +836,7 @@ static QEMUMachine xenfv_machine = {
static void pc_machine_init(void)
{
+qemu_register_machine(pc_i440fx_machine_v2_1);
qemu_register_machine(pc_i440fx_machine_v2_0);
qemu_register_machine(pc_i440fx_machine_v1_7);
qemu_register_machine(pc_i440fx_machine_v1_6);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2b565cb..4c57b91 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -314,10 +314,16 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
PC_Q35_MACHINE_OPTIONS, \
.default_machine_opts = firmware=bios-256k.bin
+static QEMUMachine pc_q35_machine_v2_1 = {
+PC_Q35_2_0_MACHINE_OPTIONS,
+.name = pc-q35-2.1,
+.alias = q35,
+.init = pc_q35_init,
+};
+
static QEMUMachine pc_q35_machine_v2_0 = {
PC_Q35_2_0_MACHINE_OPTIONS,
.name = pc-q35-2.0,
-.alias = q35,
.init = pc_q35_init_2_0,
};
@@ -371,6 +377,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
static void pc_q35_machine_init(void)
{
+qemu_register_machine(pc_q35_machine_v2_1);
qemu_register_machine(pc_q35_machine_v2_0);
qemu_register_machine(pc_q35_machine_v1_7);
qemu_register_machine(pc_q35_machine_v1_6);
--
1.8.3.1
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
On Mi, 2014-04-23 at 09:32 +, Gonglei (Arei) wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. But you loose anything in the buffer when migrating from old - new. cheers, Gerd
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
On Mi, 2014-04-23 at 11:45 +0200, Juan Quintela wrote: Gonglei (Arei) arei.gong...@huawei.com wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. Second solution is better in the sense that it allows forward and backward migration. The first one only allows forward migration. No, first goes both ways too, and unlike the second it doesn't loose the buffer content as long as less than 16 bytes are actually used. cheers, Gerd
[Qemu-devel] [PULL 00/16] Block patches
The following changes since commit 2d03b49c3f225994c4b0b46146437d8c887d6774: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140417-1' into staging (2014-04-17 21:37:26 +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 370e681629da587af7592a7b83ebc7ec4955461e: block/cloop: use PRIu32 format specifier for uint32_t (2014-04-23 11:34:10 +0200) Block patches Aakriti Gupta (1): convert fprintf() calls to error_setg() in block/qed.c:bdrv_qed_create() Fam Zheng (4): vmdk: Fix %d and %lld to PRI* in format strings block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap qemu-img: Improve error messages vmdk: Fix %x to PRIx32 in format strings for cid Kevin Wolf (9): block: Fix nb_sectors check in bdrv_check_byte_request() block: Limit size to INT_MAX in bdrv_check_byte_request() block: Catch integer overflow in bdrv_rw_co() block: Check bdrv_getlength() return value in bdrv_make_zero() block: Remove -errno return value from bdrv_assign_node_name block: Add errp to bdrv_new() qemu-img: Avoid duplicate block device IDs block: Catch duplicate IDs in bdrv_new() qemu-iotests: Check common namespace for id and node-name Maria Kustova (1): curl: Replaced old error handling with error reporting API. Stefan Hajnoczi (1): block/cloop: use PRIu32 format specifier for uint32_t block-migration.c | 30 ++-- block.c| 69 +++ block/cloop.c | 12 ++--- block/curl.c | 2 +- block/iscsi.c | 2 +- block/mirror.c | 5 +- block/qed.c| 16 --- block/vmdk.c | 23 - block/vvfat.c | 2 +- blockdev.c | 15 +++--- hw/block/xen_disk.c| 7 ++- include/block/block.h | 5 +- qemu-img.c | 116 +++-- qemu-io.c | 2 +- qemu-nbd.c | 3 +- tests/qemu-iotests/084.out | 5 +- tests/qemu-iotests/087 | 85 + tests/qemu-iotests/087.out | 18 +++ 18 files changed, 302 insertions(+), 115 deletions(-)
[Qemu-devel] [PULL 06/16] block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap
From: Fam Zheng f...@redhat.com
bdrv_getlength could fail, check the return value before using it.
Return NULL and set errno if it fails. Callers are updated to handle
the error case.
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Max Reitz mre...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block-migration.c | 30 ++
block.c | 11 +--
block/mirror.c| 5 -
include/block/block.h | 3 ++-
4 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 897fdba..56951e0 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -310,13 +310,28 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
/* Called with iothread lock taken. */
-static void set_dirty_tracking(void)
+static int set_dirty_tracking(void)
{
BlkMigDevState *bmds;
+int ret;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-bmds-dirty_bitmap = bdrv_create_dirty_bitmap(bmds-bs, BLOCK_SIZE);
+bmds-dirty_bitmap = bdrv_create_dirty_bitmap(bmds-bs, BLOCK_SIZE,
+ NULL);
+if (!bmds-dirty_bitmap) {
+ret = -errno;
+goto fail;
+}
}
+return 0;
+
+fail:
+QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
+if (bmds-dirty_bitmap) {
+bdrv_release_dirty_bitmap(bmds-bs, bmds-dirty_bitmap);
+}
+}
+return ret;
}
static void unset_dirty_tracking(void)
@@ -611,10 +626,17 @@ static int block_save_setup(QEMUFile *f, void *opaque)
block_mig_state.submitted, block_mig_state.transferred);
qemu_mutex_lock_iothread();
-init_blk_migration(f);
/* start track dirty blocks */
-set_dirty_tracking();
+ret = set_dirty_tracking();
+
+if (ret) {
+qemu_mutex_unlock_iothread();
+return ret;
+}
+
+init_blk_migration(f);
+
qemu_mutex_unlock_iothread();
ret = flush_blks(f);
diff --git a/block.c b/block.c
index da55877..0c81747 100644
--- a/block.c
+++ b/block.c
@@ -5110,7 +5110,8 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs,
QEMUIOVector *qiov)
return true;
}
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int
granularity)
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int
granularity,
+ Error **errp)
{
int64_t bitmap_size;
BdrvDirtyBitmap *bitmap;
@@ -5119,7 +5120,13 @@ BdrvDirtyBitmap
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity)
granularity = BDRV_SECTOR_BITS;
assert(granularity);
-bitmap_size = (bdrv_getlength(bs) BDRV_SECTOR_BITS);
+bitmap_size = bdrv_getlength(bs);
+if (bitmap_size 0) {
+error_setg_errno(errp, -bitmap_size, could not get length of device);
+errno = -bitmap_size;
+return NULL;
+}
+bitmap_size = BDRV_SECTOR_BITS;
bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
bitmap-bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
QLIST_INSERT_HEAD(bs-dirty_bitmaps, bitmap, list);
diff --git a/block/mirror.c b/block/mirror.c
index 0ef41f9..2618c37 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -605,7 +605,10 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
s-granularity = granularity;
s-buf_size = MAX(buf_size, granularity);
-s-dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity);
+s-dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+if (!s-dirty_bitmap) {
+return;
+}
bdrv_set_enable_write_cache(s-target, true);
bdrv_set_on_error(s-target, on_target_error, on_target_error);
bdrv_iostatus_enable(s-target);
diff --git a/include/block/block.h b/include/block/block.h
index b3230a2..2b51eec 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -429,7 +429,8 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs,
QEMUIOVector *qiov);
struct HBitmapIter;
typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int
granularity);
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int
granularity,
+ Error **errp);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t
sector);
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2 0/2] HMP: support specifying dump format for dump-guest-memory
On 17/04/14 10:15, Qiao Nuohan wrote: The last version is here: http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg00018.html ChangLog: Changes from v7 to v8: 1. add a patch to fix doc of dump-guest-memory Qiao Nuohan (2): HMP: fix doc of dump-guest-memory HMP: support specifying dump format for dump-guest-memory hmp-commands.hx | 30 +++--- hmp.c | 25 ++--- 2 files changed, 41 insertions(+), 14 deletions(-) Luiz, are you going to pick these patches for your HMP tree? Thanks Christian
[Qemu-devel] [PULL 04/16] block: Check bdrv_getlength() return value in bdrv_make_zero()
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Max Reitz mre...@redhat.com
---
block.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index ec3fa50..da55877 100644
--- a/block.c
+++ b/block.c
@@ -2749,10 +2749,16 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t
sector_num,
*/
int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
{
-int64_t target_size = bdrv_getlength(bs) / BDRV_SECTOR_SIZE;
+int64_t target_size;
int64_t ret, nb_sectors, sector_num = 0;
int n;
+target_size = bdrv_getlength(bs);
+if (target_size 0) {
+return target_size;
+}
+target_size /= BDRV_SECTOR_SIZE;
+
for (;;) {
nb_sectors = target_size - sector_num;
if (nb_sectors = 0) {
--
1.8.3.1
[Qemu-devel] [PULL 11/16] qemu-img: Avoid duplicate block device IDs
qemu-img used to use image as ID for all block devices. This means
that e.g. img_convert() ended up with potentially multiple source images
and one target image, all with the same ID. The next patch will catch
this and fail to open the block device.
This patch makes sure that qemu-img uses meaningful unique IDs for the
block devices it uses.
Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
qemu-img.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 3e8bd80..fb626ac 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -262,7 +262,8 @@ static int print_block_option_help(const char *filename,
const char *fmt)
return 0;
}
-static BlockDriverState *bdrv_new_open(const char *filename,
+static BlockDriverState *bdrv_new_open(const char *id,
+ const char *filename,
const char *fmt,
int flags,
bool require_io,
@@ -274,7 +275,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
Error *local_err = NULL;
int ret;
-bs = bdrv_new(image, error_abort);
+bs = bdrv_new(id, error_abort);
if (fmt) {
drv = bdrv_find_format(fmt);
@@ -615,7 +616,7 @@ static int img_check(int argc, char **argv)
return 1;
}
-bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+bs = bdrv_new_open(image, filename, fmt, flags, true, quiet);
if (!bs) {
return 1;
}
@@ -724,7 +725,7 @@ static int img_commit(int argc, char **argv)
return -1;
}
-bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+bs = bdrv_new_open(image, filename, fmt, flags, true, quiet);
if (!bs) {
return 1;
}
@@ -967,14 +968,14 @@ static int img_compare(int argc, char **argv)
/* Initialize before goto out */
qemu_progress_init(progress, 2.0);
-bs1 = bdrv_new_open(filename1, fmt1, BDRV_O_FLAGS, true, quiet);
+bs1 = bdrv_new_open(image 1, filename1, fmt1, BDRV_O_FLAGS, true, quiet);
if (!bs1) {
error_report(Can't open file %s, filename1);
ret = 2;
goto out3;
}
-bs2 = bdrv_new_open(filename2, fmt2, BDRV_O_FLAGS, true, quiet);
+bs2 = bdrv_new_open(image 2, filename2, fmt2, BDRV_O_FLAGS, true, quiet);
if (!bs2) {
error_report(Can't open file %s, filename2);
ret = 2;
@@ -1292,8 +1293,11 @@ static int img_convert(int argc, char **argv)
total_sectors = 0;
for (bs_i = 0; bs_i bs_n; bs_i++) {
-bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true,
- quiet);
+char *id = bs_n 1 ? g_strdup_printf(source %d, bs_i)
+: g_strdup(source);
+bs[bs_i] = bdrv_new_open(id, argv[optind + bs_i], fmt, BDRV_O_FLAGS,
+ true, quiet);
+g_free(id);
if (!bs[bs_i]) {
error_report(Could not open '%s', argv[optind + bs_i]);
ret = -1;
@@ -1416,7 +1420,7 @@ static int img_convert(int argc, char **argv)
return -1;
}
-out_bs = bdrv_new_open(out_filename, out_fmt, flags, true, quiet);
+out_bs = bdrv_new_open(target, out_filename, out_fmt, flags, true,
quiet);
if (!out_bs) {
ret = -1;
goto out;
@@ -1799,8 +1803,8 @@ static ImageInfoList *collect_image_info_list(const char
*filename,
}
g_hash_table_insert(filenames, (gpointer)filename, NULL);
-bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
- false, false);
+bs = bdrv_new_open(image, filename, fmt,
+ BDRV_O_FLAGS | BDRV_O_NO_BACKING, false, false);
if (!bs) {
goto err;
}
@@ -2060,7 +2064,7 @@ static int img_map(int argc, char **argv)
return 1;
}
-bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS, true, false);
+bs = bdrv_new_open(image, filename, fmt, BDRV_O_FLAGS, true, false);
if (!bs) {
return 1;
}
@@ -2180,7 +2184,7 @@ static int img_snapshot(int argc, char **argv)
filename = argv[optind++];
/* Open the image */
-bs = bdrv_new_open(filename, NULL, bdrv_oflags, true, quiet);
+bs = bdrv_new_open(image, filename, NULL, bdrv_oflags, true, quiet);
if (!bs) {
return 1;
}
@@ -2309,7 +2313,7 @@ static int img_rebase(int argc, char **argv)
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
-bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+bs = bdrv_new_open(image, filename, fmt, flags, true, quiet);
if (!bs) {
return 1;
}
@@ -2606,7 +2610,8 @@ static int img_resize(int argc, char **argv)
Re: [Qemu-devel] [PATCH] armv7m_nvic: fix CPUID Base Register
On 21 April 2014 00:25, Rabin Vincent ra...@rab.in wrote: cp15.c0_cpuid is never initialized for ARMv7-M; take the value directly from cpu-midr instead. Signed-off-by: Rabin Vincent ra...@rab.in Hmm, I wonder how long that's been broken for... Reviewed-by: Peter Maydell peter.mayd...@linaro.org thanks -- PMM
[Qemu-devel] [PULL 09/16] convert fprintf() calls to error_setg() in block/qed.c:bdrv_qed_create()
From: Aakriti Gupta aakri...@gmail.com
This patch converts fprintf() calls to error_setg() in
block/qed.c:bdrv_qed_create()
(error_setg() is part of error reporting API in include/qapi/error.h)
Signed-off-by: Aakriti Gupta aakri...@gmail.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/qed.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index 3bd9db9..c130e42 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -650,19 +650,21 @@ static int bdrv_qed_create(const char *filename,
QEMUOptionParameter *options,
}
if (!qed_is_cluster_size_valid(cluster_size)) {
-fprintf(stderr, QED cluster size must be within range [%u, %u] and
power of 2\n,
-QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
+error_setg(errp, QED cluster size must be within range [%u, %u]
+ and power of 2,
+ QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
return -EINVAL;
}
if (!qed_is_table_size_valid(table_size)) {
-fprintf(stderr, QED table size must be within range [%u, %u] and
power of 2\n,
-QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
+error_setg(errp, QED table size must be within range [%u, %u]
+ and power of 2,
+ QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
return -EINVAL;
}
if (!qed_is_image_size_valid(image_size, cluster_size, table_size)) {
-fprintf(stderr, QED image size must be a non-zero multiple of
-cluster size and less than % PRIu64 bytes\n,
-qed_max_image_size(cluster_size, table_size));
+error_setg(errp, QED image size must be a non-zero multiple of
+ cluster size and less than % PRIu64 bytes,
+ qed_max_image_size(cluster_size, table_size));
return -EINVAL;
}
--
1.8.3.1
[Qemu-devel] [PULL 03/16] block: Catch integer overflow in bdrv_rw_co()
Insanely large requests could cause an integer overflow in
bdrv_rw_co() while converting sectors to bytes. This patch catches the
problem and returns an error (if we hadn't overflown the integer here,
bdrv_check_byte_request() would have rejected the request, so we're not
breaking anything that was supposed to work before).
We actually do have a test case that triggers behaviour where we
accidentally let such a request pass, so that it would return success,
but read 0 bytes instead of the requested 4 GB. It fails now like it
should.
If the vdi block driver wants to be able to deal with huge images, it
can't read the whole block bitmap at once into memory like it does
today, but needs to use a metadata cache like qcow2 does.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block.c| 4
tests/qemu-iotests/084.out | 5 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 5a0b421..ec3fa50 100644
--- a/block.c
+++ b/block.c
@@ -2690,6 +2690,10 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t
sector_num, uint8_t *buf,
.iov_len = nb_sectors * BDRV_SECTOR_SIZE,
};
+if (nb_sectors 0 || nb_sectors INT_MAX / BDRV_SECTOR_SIZE) {
+return -EINVAL;
+}
+
qemu_iovec_init_external(qiov, iov, 1);
return bdrv_prwv_co(bs, sector_num BDRV_SECTOR_BITS,
qiov, is_write, flags);
diff --git a/tests/qemu-iotests/084.out b/tests/qemu-iotests/084.out
index e681924..c7120d9 100644
--- a/tests/qemu-iotests/084.out
+++ b/tests/qemu-iotests/084.out
@@ -4,10 +4,7 @@ QA output created by 084
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Test 1: Maximum size (1024 TB):
-image: TEST_DIR/t.IMGFMT
-file format: IMGFMT
-virtual size: 1024T (1125899905794048 bytes)
-cluster_size: 1048576
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open
'TEST_DIR/t.IMGFMT': Invalid argument
Test 2: Size too large (1024TB + 1)
qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported VDI image size (size
is 0x3fff1, max supported is 0x3fff0)
--
1.8.3.1
[Qemu-devel] [Bug 1311614] [NEW] qemu-arm segfaults with gcc 4.9.0
Public bug reported: I have an ARM chroot that working with qemu-arm emulation [root@filzbach fedya]# cat /proc/sys/fs/binfmt_misc/arm enabled interpreter /usr/bin/qemu-arm-binfmt flags: P offset 0 magic 7f454c460101010002002800 mask ff00feff In chroot installed gcc dependencies with 4.9.0 version sudo rpm --root /home/fedya/root/ -qa | grep 4.9.0 libgcc1-4.9.0_2014.04-1-omv2013.0.armv7hl libgomp1-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++6-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-cpp-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++-devel-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-c++-4.9.0_2014.04-1-omv2013.0.armv7hl When i try to run rpm , rpmbuild, rpm2cpiocommand i always see qemu segfault message example: [root@filzbach /]# uname -a Linux filzbach.lindev.ch 3.13.6-nrjQL-desktop-70omv #1 SMP PREEMPT Wed Mar 12 21:40:00 UTC 2014 armv7l armv7l armv7l GNU/Linux [root@filzbach /]# rpm qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segfault became apparent only after gcc upgrade from 4.8.3 to 4.9.0. When i downgrade it to 4.8.3 all working fine again. It looks like a qemu bug with gcc. P.S. I tried to rebuild qemu with gcc 4.9.0 I tried to build qemu from git sources, from fedora sources, from suse sources etc. ** Affects: qemu Importance: Undecided Status: New ** Tags: arm armv7hl qemu segfault -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1311614 Title: qemu-arm segfaults with gcc 4.9.0 Status in QEMU: New Bug description: I have an ARM chroot that working with qemu-arm emulation [root@filzbach fedya]# cat /proc/sys/fs/binfmt_misc/arm enabled interpreter /usr/bin/qemu-arm-binfmt flags: P offset 0 magic 7f454c460101010002002800 mask ff00feff In chroot installed gcc dependencies with 4.9.0 version sudo rpm --root /home/fedya/root/ -qa | grep 4.9.0 libgcc1-4.9.0_2014.04-1-omv2013.0.armv7hl libgomp1-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++6-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-cpp-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++-devel-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-c++-4.9.0_2014.04-1-omv2013.0.armv7hl When i try to run rpm , rpmbuild, rpm2cpiocommand i always see qemu segfault message example: [root@filzbach /]# uname -a Linux filzbach.lindev.ch 3.13.6-nrjQL-desktop-70omv #1 SMP PREEMPT Wed Mar 12 21:40:00 UTC 2014 armv7l armv7l armv7l GNU/Linux [root@filzbach /]# rpm qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segfault became apparent only after gcc upgrade from 4.8.3 to 4.9.0. When i downgrade it to 4.8.3 all working fine again. It looks like a qemu bug with gcc. P.S. I tried to rebuild qemu with gcc 4.9.0 I tried to build qemu from git sources, from fedora sources, from suse sources etc. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1311614/+subscriptions
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Gerd Hoffmann kra...@redhat.com wrote: On Mi, 2014-04-23 at 09:32 +, Gonglei (Arei) wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. But you loose anything in the buffer when migrating from old - new. Anything bigger than 16bytes, no? And that is the whole point that we are talking about? Or the 16bytes that we are using can be at any place on the buffer? If that is the case, I agree that the first option is better for some versions. Sorry for my missunderstanding. Later, Juan. PD. No, I don't claim to understand how PS2 work at all.
[Qemu-devel] [Bug 1311614] Re: qemu-arm segfaults with gcc 4.9.0
And of course i rebuilt rpm package with latest gcc 4.9.0 Btw all working fine on a real hardware. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1311614 Title: qemu-arm segfaults with gcc 4.9.0 Status in QEMU: New Bug description: I have an ARM chroot that working with qemu-arm emulation [root@filzbach fedya]# cat /proc/sys/fs/binfmt_misc/arm enabled interpreter /usr/bin/qemu-arm-binfmt flags: P offset 0 magic 7f454c460101010002002800 mask ff00feff In chroot installed gcc dependencies with 4.9.0 version sudo rpm --root /home/fedya/root/ -qa | grep 4.9.0 libgcc1-4.9.0_2014.04-1-omv2013.0.armv7hl libgomp1-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++6-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-cpp-4.9.0_2014.04-1-omv2013.0.armv7hl libstdc++-devel-4.9.0_2014.04-1-omv2013.0.armv7hl gcc-c++-4.9.0_2014.04-1-omv2013.0.armv7hl When i try to run rpm , rpmbuild, rpm2cpiocommand i always see qemu segfault message example: [root@filzbach /]# uname -a Linux filzbach.lindev.ch 3.13.6-nrjQL-desktop-70omv #1 SMP PREEMPT Wed Mar 12 21:40:00 UTC 2014 armv7l armv7l armv7l GNU/Linux [root@filzbach /]# rpm qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segfault became apparent only after gcc upgrade from 4.8.3 to 4.9.0. When i downgrade it to 4.8.3 all working fine again. It looks like a qemu bug with gcc. P.S. I tried to rebuild qemu with gcc 4.9.0 I tried to build qemu from git sources, from fedora sources, from suse sources etc. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1311614/+subscriptions
[Qemu-devel] [PATCH v2] doc: add -drive rerror=, werror= to qemu --help output
These options are already documented on the man page but missing from qemu --help. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- qemu-options.hx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815..a1333c2 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -408,7 +408,8 @@ DEF(drive, HAS_ARG, QEMU_OPTION_drive, -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n - [,serial=s][,addr=A][,id=name][,aio=threads|native]\n + [,serial=s][,addr=A][,rerror=ignore|stop|report]\n + [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n [,readonly=on|off][,copy-on-read=on|off]\n [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n -- 1.9.0
Re: [Qemu-devel] Who signed gemu-1.7.1.tar.bz2?
On Tue, Apr 22, 2014 at 09:35:07AM -0500, Michael Roth wrote: Quoting Stefan Hajnoczi (2014-04-22 08:31:08) On Wed, Apr 02, 2014 at 05:40:23PM -0700, Alex Davis wrote: and where is their gpg key? Michael Roth mdr...@linux.vnet.ibm.com is doing releases: http://pgp.mit.edu/pks/lookup?op=vindexsearch=0x3353C9CEF108B584 $ gpg --verify qemu-2.0.0.tar.bz2.sig gpg: Signature made Thu 17 Apr 2014 03:49:55 PM CEST using RSA key ID F108B584 gpg: Good signature from Michael Roth fluks...@gmail.com gpg: aka Michael Roth mdr...@utexas.edu gpg: aka Michael Roth mdr...@linux.vnet.ibm.com Missed the context, but if this is specifically about 1.7.1: 1.7.1 was prior to me handling the release tarballs, Anthony actually did the signing and uploading for that one. I'm a bit confused though, as the key ID on that tarball is: mdroth@loki:~/Downloads$ gpg --verify qemu-1.7.1.tar.bz2.sig gpg: Signature made Tue 25 Mar 2014 09:03:24 AM CDT using RSA key ID ADF0D2D9 gpg: Can't check signature: public key not found I can't seem to locate ADF0D2D9 though: http://pgp.mit.edu/pks/lookup?search=0xADF0D2D9op=vindex Anthony's normal key (for 1.6.0 and 1.7.0 at least) was 7C18C076: http://pgp.mit.edu/pks/lookup?search=0x7C18C076op=vindex I think maybe Anthony might've signed it with a separate local key? This is a mess :). We need a page like this explaining how QEMU releases are signed: https://www.kernel.org/category/signatures.html Mike: as release manager, can you post a page like that to the QEMU wiki? Thanks, Stefan
Re: [Qemu-devel] networking stalls in the guest -- backlog in the host
On Tue, Apr 01, 2014 at 09:43:42AM -0600, David Ahern wrote:
On 4/1/14, 9:09 AM, Stefan Hajnoczi wrote:
On Thu, Mar 27, 2014 at 04:13:15PM -0600, David Ahern wrote:
We are hitting a networking problem and hoping someone has an idea
-- perhaps a known bug.
After a couple of hours of runtime with low level traffic (e.g., 1
sec pings) the VM stops receiving packets. In the host running tc on
the tap device shows a full backlog and packets getting dropped:
tc -s qdisc show dev vnet0
qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1
1 1 1 1 1 1 1
Sent 5806496634 bytes 4163358 pkt (dropped 116079, overlimits 0 requeues 4)
backlog 33834b 500p requeues 4
The tap device is passed to qemu as fd=24. Running strace on the IO
thread does not show the fd in the list passed to select. e.g.,
select(55, [7 8 11 18 52 53 54], [], [], {1, 0}) = 1 (in [8], left
{0, 872402})
That would explain why the packets are not pulled from the tap
device into the VM. When networking is functioning properly, you do
see fd=24 in the list followed by read(24, ...).
Why would qemu stop adding the fd to the list passed to select?
This is qemu-kvm-1.0 (upgrading is not an option), started by
libvirt (libvirt 1.0.2). The command line is rather long. Snippets:
/usr/bin/kvm -M pc-1.0 -cpu host -enable-kvm -m 4096 -smp 4
...
-netdev tap,fd=24,id=hostnet0 -device
virtio-net-pci,netdev=hostnet0,id=net0,mac=00:16:3e:ba:55:60,bus=pci.0,addr=0xa
...
Host kernel: 3.2.0-60-generic
Guest kernel: 3.8.0-29-generic
Are you using vhost_net or the userspace virtio-net emulation?
virtio-net.
There have been bugs in the past, like the recent:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=68e5ec64009812dbaa03ed9cfded9344986f5304
So it's not totally surprising. I'm afraid you either need to bisect
against a newer, working QEMU version or debug your old QEMU from first
principles (using gdb it should be possible to figure out more about why
the tap file descriptor is not in the select(2) fdset).
Stefan
Re: [Qemu-devel] Guest bandwidth setting
On Sun, Mar 30, 2014 at 02:35:01PM +0200, Pradeep Kiruvale wrote: I am implementing some functionality where in I need to set the bandwidth control for my guest vms.Please let me know how can I do it. I am looking into net/net.c file,I dint find any leads.I tried two ways. I am trying to find how to set virtual nics(in my case e1000) speed,I did not find any ways to do that. If I devide the packet into smaller packets in qemu_deliver_packet() function I am not able to transfer the other half of the packet. Please let me know how can I resolve this. The simplest way is to use libvirt's network Quality-of-Service features. You can specify bandwidth limits: http://libvirt.org/formatdomain.html#elementQoS Stefan
Re: [Qemu-devel] [PATCH v2] doc: add -drive rerror=, werror= to qemu --help output
Am 23.04.2014 um 13:55 hat Stefan Hajnoczi geschrieben: These options are already documented on the man page but missing from qemu --help. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com
[Qemu-devel] where can I find the latest memory hot plugging patch for Qemu?
Dear Qemu developers, I'm doing a masters thesis project which involves Qemu. Especially cpu and memory hot plugging. But I have a hard time finding the right git repository which contains the latest memory hot plugging work. Can anyone please point me in the right direction? Regards, Gerrit van der Kolk
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
On Mi, 2014-04-23 at 08:06 +, Gonglei (Arei) wrote:
Hi, Gerd and Juan.
Thanks for your guides about the confuse live migration about changing the
keyboard buffer size.
According your suggestion, I got two solutions to address the issue:
- Keep the data array 256 bytes long, change the rptr/wptr/count/data array
at post_load(), both
Ps/2 keyboard and mouse. This solution can be compatible with older qemu
versions, which can
do live migration each other.
-Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the
rptr/wptr/count at
post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16)
in struct vmstate_ps2_common.
This solution just save the 16 bytes buffer and drop the rest, So we can't
migrate vm to older qemu versions.
But migration from old qemu to new qemu is ok.
I think long term we want #2, but using #1 as step inbetween for a few
releases until 16 byte buffer size is widely used will might be useful.
Completely separate question: Have you figured what the root cause for
the bug is?
When the linux kernel booting, will init the i8042 controller
(drivers/input/serio/i8042.c)
i8042_init()
|- i8042_controller_check()
static int i8042_controller_check(void)
{
if (i8042_flush() == I8042_BUFFER_SIZE) {
pr_err(No controller found\n);
return -ENODEV;
}
return 0;
}
With this 16 byte buffer in drivers/input/serio/i8042.h:
#define I8042_BUFFER_SIZE 16
and this piece of code in drivers/input/serio/i8042.c:
/*
* i8042_flush() flushes all data that may be in the *keyboard* and *mouse*
buffers
* of the i8042 down the toilet.
*/
static int i8042_flush(void)
{
unsigned long flags;
unsigned char data, str;
int i = 0;
spin_lock_irqsave(i8042_lock, flags);
while (((str = i8042_read_status()) I8042_STR_OBF) (i
I8042_BUFFER_SIZE)) {
udelay(50);
data = i8042_read_data();
i++;
dbg(%02x - i8042 (flush, %s), data,
str I8042_STR_AUXDATA ? aux : kbd);
}
spin_unlock_irqrestore(i8042_lock, flags);
return i;
}
So, if we type anything on keyboard (or move mouse) over 16 bytes while
Linux kernel is still booting, Linux kernel will get confused. And as a result,
decides there is no controller found.
While wading through the code I've figured the queue size
isn't (directly) exposed to the guest.
When we type the keyboard or move mouse, the ps2_queue() will be called,
Account the wptr and count value, and fill the data array. Then the qemu
will update irq. In kbd_update_irq() the kbd(i8042) state will be set:
s-status |= KBD_STAT_OBF;
s-outport |= KBD_OUT_OBF;
then send irq to announce Guest OS.
The process correspond with the above linux kernel code, i8042_flush(void).
And it's used for both mouse and
kbd. For the kbd 16 byte buffer is probably ok,
Yes, the i8042 controller used by both ps/2 keyboard and ps/2 mouse. So,
The buffer size is shared by ps/2 kbd and mouse.
but for mouse events we
probably want keep some more space to buffer events ...
Maybe you are right, and I worry about it too. At present,
but I haven't feel uncomfortable by VNC client then before.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH v8 2/4] raw, qcow2: don't convert file size to sector size
On 04/09/2014 01:12 AM, Hu Tao wrote:
and avoid converting it back later.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
block/qcow2.c | 8
block/raw-posix.c | 4 ++--
block/raw-win32.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
@@ -1777,7 +1777,7 @@ static int qcow2_create(const char *filename,
QEMUOptionParameter *options,
/* Read out options */
while (options options-name) {
if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-sectors = options-value.n / 512;
+size = options-value.n BDRV_SECTOR_MASK;
The old code rounds down, which in my opinion is a bug. The qcow2 file
format has a size parameter that is recorded in bytes, not sectors, so I
see no technical reason why we can't support a file where the last
sector is partial. If I request a size 1025 bytes, I should get a file
with 3 sectors (where the last sector is partial), and not a file with 2
sectors (where the last byte that I requested cannot be accessed). Your
patch preserves the pre-existing behavior, but I think this series
should consider fixing the bug and allowing for the creation of a qcow2
file whose size exposed to the guest is NOT a multiple of sector size,
or at least, at a bare minimum, round my request UP to the nearest
sector size rather than giving me something smaller than what I asked for.
+++ b/block/raw-posix.c
@@ -1246,7 +1246,7 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options,
/* Read out options */
while (options options-name) {
if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / BDRV_SECTOR_SIZE;
+total_size = options-value.n BDRV_SECTOR_MASK;
Another case of incorrectly rounding down.
+++ b/block/raw-win32.c
@@ -486,7 +486,7 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options,
/* Read out options */
while (options options-name) {
if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / 512;
+total_size = options-value.n BDRV_SECTOR_MASK;
and another
--
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] Patch for shrinking qcow2 disk image
On Tue, Apr 01, 2014 at 02:06:10PM +0800, Jun Li wrote:
Signed-off-by: Jun Li junm...@gmail.com
This patch can make sure the data still existing after shrinking. And only
discard the unused (guest) clusters. If shrinking to the size which stored
data, It will return an error and will not do any change.
As this patch can support shrinking, so changed the func name of
qcow2_grow_l1_table to qcow2_truncate_l1_table.
Please follow the patch formatting conventions:
qcow2: add qcow2_truncate() shrinking support
...patch description...
Signed-off-by: ...
-int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
bool exact_size)
{
BDRVQcowState *s = bs-opaque;
@@ -39,9 +39,6 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
int64_t new_l1_table_offset, new_l1_size;
uint8_t data[12];
-if (min_size = s-l1_size)
-return 0;
-
if (exact_size) {
new_l1_size = min_size;
} else {
@@ -66,7 +63,18 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size2 = sizeof(uint64_t) * new_l1_size;
new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
-memcpy(new_l1_table, s-l1_table, s-l1_size * sizeof(uint64_t));
+
+/* shrinking the image */
+if (min_size = s-l1_size) {
+if (s-l1_table[new_l1_size] != 0) {
+error_report(Could not shrink to this size,
+it will destory image data);
+return -ENOTSUP;
+}
+memcpy(new_l1_table, s-l1_table, new_l1_size2);
+}
+
+ memcpy(new_l1_table, s-l1_table, s-l1_size * sizeof(uint64_t));
In the shrink case the second memcpy() overflows new_l1_table.
But the bigger issue I see with this patch is that bdrv_truncate() is
supposed to shrink the file, whether there is data present or not.
Please drop the if (s-l1_table[new_l1_size] != 0) test.
/* write new table (align to cluster) */
BLKDBG_EVENT(bs-file, BLKDBG_L1_GROW_ALLOC_TABLE);
@@ -559,7 +567,7 @@ static int get_cluster_table(BlockDriverState *bs,
uint64_t offset,
l1_index = offset (s-l2_bits + s-cluster_bits);
if (l1_index = s-l1_size) {
-ret = qcow2_grow_l1_table(bs, l1_index + 1, false);
+ret = qcow2_truncate_l1_table(bs, l1_index + 1, false);
if (ret 0) {
return ret;
}
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 2fc6320..ab16c52 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -491,7 +491,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
*snapshot_id)
* L1 table of the snapshot. If the snapshot L1 table is smaller, the
* current one must be padded with zeros.
*/
-ret = qcow2_grow_l1_table(bs, sn-l1_size, true);
+ret = qcow2_truncate_l1_table(bs, sn-l1_size, true);
if (ret 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index b9dc960..4797879 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1764,14 +1764,8 @@ static int qcow2_truncate(BlockDriverState *bs,
int64_t offset)
return -ENOTSUP;
}
-/* shrinking is currently not supported */
-if (offset bs-total_sectors * 512) {
-error_report(qcow2 doesn't support shrinking images yet);
-return -ENOTSUP;
-}
-
new_l1_size = size_to_l1(s, offset);
-ret = qcow2_grow_l1_table(bs, new_l1_size, true);
+ret = qcow2_truncate_l1_table(bs, new_l1_size, true);
if (ret 0) {
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 0b0eac8..298d84e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -455,7 +455,7 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs,
int ign, int64_t offset,
int64_t size);
/* qcow2-cluster.c functions */
-int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
bool exact_size);
Please indent bool exact_size up to the '(':
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
+bool exact_size);
Re: [Qemu-devel] [PATCH v8 3/4] raw-posix: Add full image preallocation option
On 04/09/2014 01:12 AM, Hu Tao wrote:
This patch adds a new option preallocation for raw format, and implements
full preallocation.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
block/raw-posix.c | 61
---
1 file changed, 54 insertions(+), 7 deletions(-)
+if (prealloc == PREALLOC_MODE_METADATA) {
+/* posix_fallocate() doesn't set errno. */
+result = -posix_fallocate(fd, 0, total_size);
+if (result != 0) {
+error_setg_errno(errp, -result,
+ Could not preallocate data for the new file);
}
-if (qemu_close(fd) != 0) {
-result = -errno;
-error_setg_errno(errp, -result, Could not close the new file);
+} else if (prealloc == PREALLOC_MODE_FULL) {
+char *buf = g_malloc0(65536);
+int64_t num = 0, left = total_size;
+
+while (left 0) {
+num = MIN(left, 65536);
+result = write(fd, buf, num);
+if (result 0) {
+result = -errno;
+error_setg_errno(errp, -result,
+ Could not write to the new file);
+g_free(buf);
+goto out_close;
+}
+left -= num;
}
+fsync(fd);
+g_free(buf);
+}
+out_close:
+if (qemu_close(fd) != 0) {
+result = -errno;
This overwrites any earlier failure with the failure of close, which may
less informative. I think you want to use 'if (!result) { result =
-errno; }' rather than blindly losing information.
--
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 v3 2/4] GlobalProperty: Display warning about unused -global
On 04/22/14 20:28, Paolo Bonzini wrote: Il 22/04/2014 19:13, Don Slutz ha scritto: Even then, I suspect sooner or later machines other than PC and Q35 will be versioned. At that point we'll probably want QEMU-global compat_props that automatically apply to all machines, even if a type is not missing. I think we should already approximate this behavior by allowing machine .compat_props where the type doesn't exist. If I am reading this correctly, this is asking that .compat_props to not be reported. This currently happens because of not_used being false in all C99 struct initializations. Oh, I see. That's obvious in retrospect, but perhaps a comment can be useful around the definition of the not_used field. Will add a comment: /** * GlobalProperty: * @not_used: Track use of a global property. Defaults to false in all C99 struct initializations. * * This prevents reports of .compat_props when they are not used. */ (Not sure if this is the correct formatting.) -Don Slutz Paolo
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Gerd Hoffmann kra...@redhat.com wrote: On Mi, 2014-04-23 at 09:32 +, Gonglei (Arei) wrote: Hi, Gerd and Juan. Thanks for your guides about the confuse live migration about changing the keyboard buffer size. According your suggestion, I got two solutions to address the issue: - Keep the data array 256 bytes long, change the rptr/wptr/count/data array at post_load(), both Ps/2 keyboard and mouse. This solution can be compatible with older qemu versions, which can do live migration each other. -Change the data array to 16 bytes, still save as PS2_QUEUE_SIZE. Reset the rptr/wptr/count at post_load(), both ps/2 keyboard and mouse. Add VMSTATE_UNUSED(256-16) in struct vmstate_ps2_common. This solution just save the 16 bytes buffer and drop the rest, So we can't migrate vm to older qemu versions. But migration from old qemu to new qemu is ok. Sorry, the second solution also support cross-version live migration each other. But you loose anything in the buffer when migrating from old - new. Anything bigger than 16bytes, no? And that is the whole point that we are talking about? Or the 16bytes that we are using can be at any place on the buffer? If that is the case, I agree that the first option is better for some versions. Sorry for my missunderstanding. Actually, in my hundreds of times testing, about the queue buffer value, at the migration dest end the rptr always equal with wptr, the value range is [0, 256), and count value always is 0. So, in the old qemu versions, the 16 bytes data maybe at any place on the buffer. The queue just be realized to a lined container by data array, the rptr/wptr will fill it cyclically. So, I want to post a normal patch about the first solution, can I? Thanks. Best regards, -Gonglei
Re: [Qemu-devel] drive_del vs. device_del: what should come first?
On Fri, Apr 11, 2014 at 02:47:20PM +0200, Heinz Graalfs wrote: Hello Markus, I finally managed to reproduce the problem, at least once ... The scenario was: dd if=/dev/vdx1 of=partitionone followed by a virsh detach... (with the device_del() under the cover) during active dd processing dmesg shows: [79026.220718] User process fault: interruption code 0x40009 in qemu-system-s390x[8000+43c000] [79026.220737] CPU: 3 PID: 20117 Comm: qemu-system-s39 Not tainted 3.10.33+ #4 [79026.220742] task: 8593 ti: e0b74000 task.ti: e0b74000 [79026.220746] User PSW : 070500018000 80269722 (0x80269722) [79026.220774]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:1 AS:0 CC:0 PM:0 EA:3 User GPRS: [79026.220827] 803761dc 809001d0 [79026.220831]03fffd0391a8 808ff560 808eb020 03c03838 [79026.220834] 80375e88 802696f6 03c03838 [79026.220847] User Code: 80269716: b9040034 lgr %r3,%r4 8026971a: a729 lghi%r2,0 #8026971e: b9870021 dlgr%r2,%r1 80269722: b9040012 lgr %r1,%r2 80269726: 5010b0a0 st %r1,160(%r11) 8026972a: 5820b0a0 l %r2,160(%r11) 8026972e: e310b0a80004 lg %r1,168(%r11) 80269734: 58101000 l %r1,0(%r1) [79026.220875] Last Breaking-Event-Address: [79026.220878] [8026904c] 0x8026904c with PSW addr: 80269722 pointing into virtio_queue_empty ... 8026a67c t virtio_notify_vector 802694a4 T virtio_queue_empty 8026b440 T virtio_queue_get_addr ... and coming from: R14 802696f6 virtqueue_fill ... 80269f74 T virtqueue_avail_bytes 80269540 T virtqueue_fill 8026979c T virtqueue_flush 80269c2c T virtqueue_get_avail_bytes ... see below the complete core_backtrace ... On 02/04/14 19:40, Markus Armbruster wrote: Heinz Graalfs graa...@linux.vnet.ibm.com writes: On 01/04/14 17:48, Markus Armbruster wrote: Heinz Graalfs graa...@linux.vnet.ibm.com writes: Hi Kevin, doing a virsh detach-device ... ends up in the following QEMU monitor commands: 1. device_del ... 2. drive_del ... qmp_device_del() performs the device unplug path. In case of a block device do_drive_del() tries to prevent further IO against the host device. However, bdrv_find() during drive_del() results in an error, because the device is already gone. Due to this error all the bdrv_xxx calls to quiesce the block driver as well as all other processing is skipped. Is the sequence that libvirt triggers OK? Shouldn't drive_del be executed first? No. OK, I see. The drive is deleted implicitly (release_drive()). Doing a device_del() requires another drive_add() AND device_add(). Yes. The automatic drive delete on unplug is a wart that will not be preserved in the new interfaces we're building now. (Doing just a device_add() complains about the missing drive. A subsequent info qtree lets QEMU abort.) Really? Got a reproducer for me? drive_del is nasty. Its purpose is to revoke access to an image even when the guest refuses to cooperate. To the guest, this looks like hardware failure. Deleting a device during active IO is nasty and it should look like a hardware failure. I would expect lots of errors. If you drive_del before device_del, even a perfectly well-behaved guest guest is exposed to a terminally broken device between drive_del and completion of unplug. The early drive_del() would mean that no further IO would be possible. A polite way to describe the effect of drive_del on the guest. drive_del makes all attempts at actual I/O error out without any forewarning whatsoever. If you do that to your guest, and something breaks, you get to keep the pieces. Even if you only do it for a short period of time. Always try a device_del first, and only if that does not complete within reasonable time, and you absolutely must revoke access to the image, then whack it over the head with drive_del. What is this reasonable time? Depends on how heavily loaded host and guest are. Suggest to measure with a suitably thrashing guest, then shift left to taste. On 390 we experience problems (QEMU abort) when asynch block IO completes and the virtqueues are already gone. I suppose the bdrv_drain_all() in bdrv_close() is a little late. I don't see such problems with an early bdrv_drain_all() (drive_del) and an unplug (device_del) afterwards. Sounds like a bug. Got a reproducer? here is the complete core_backtrace:
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi, Anything bigger than 16bytes, no? And that is the whole point that we are talking about? Or the 16bytes that we are using can be at any place on the buffer? Yes. It's a ring buffer, with rptr pointing to the first used element and wptr pointing to the first free element. So, what we need is a function to move the content we are interested (between rptr and wptr) in to the head of the ring buffer, set rptr to 0, set wptr to count. We obviously need to do that in post_load(), but as I've noticed meanwhile also in pre_save, otherwise old qemu will get things wrong in case the buffer is wrapped (rptr wptr). cheers, Gerd
Re: [Qemu-devel] [Bug 1288620] Re: memory leak with default NIC model
On Sat, Mar 29, 2014 at 03:02:23AM -, Aidan Gauland wrote: I have been able to consistently reproduce the bug again, and have run QEMU with Valgrind until OOM. It is unrelated to networking; it is caused by loading a config file. I ran QEMU from Git commit 7f6613cedc59fa849105668ae971dc31004bca1c under valgrind via... valgrind qemu-system-x86_64 -readconfig windows8_throwaway_VM.conf -m 1G -vga std 21 | tee valgrind.log ...where the contents of windows8_throwaway_VM.conf is... [drive] file = windows8_throwaway_HDD.img index = 0 media = disk if = virtio [net] type = nic vlan = 0 model = virtio [net] type = user vlan = 0 [rtc] base = localtime [machine] accel = kvm (I will attach the file in a separate comment, because launchpad appears to only allow at most one attachment per comment.) It does not seem to matter whether VirtIO is used, as I have had this problem when not using any VirtIO devices, but the Windows guest I had on-hand was already using it. If I invoke QEMU with the equivalent settings all via the command line, it does not gobble memory (again, regardless of VirtIO). qemu-system-x86_64 -drive file=windows8_throwaway_HDD.img,index=0,media=disk,if=virtio -enable-kvm -m 1G -vga std -net nic,vlan=0,model=virtio -net user,vlan=0 -rtc localtime So this is a problem that only happens under Valgrind? Perhaps this is a valgrind bug. Stefan
[Qemu-devel] [PATCH 12/13] remove __get_user return check from PPC do_setcontext
From: Riku Voipio riku.voi...@linaro.org The last remaining check for return value of __get_user. Signed-off-by: Riku Voipio riku.voi...@linaro.org Cc: Alexander Graf ag...@suse.de --- linux-user/signal.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 487fa2f..424b5d8 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -4839,8 +4839,7 @@ static int do_setcontext(struct target_ucontext *ucp, CPUPPCState *env, int sig) fprintf (stderr, do_setcontext: not implemented\n); return 0; #else -if (__get_user(mcp_addr, ucp-tuc_regs)) -return 1; +__get_user(mcp_addr, ucp-tuc_regs); if (!lock_user_struct(VERIFY_READ, mcp, mcp_addr, 1)) return 1; -- 1.9.2
[Qemu-devel] [PATCH 02/13] signal.c setup_frame/x86: __put_user cleanup
From: Riku Voipio riku.voi...@linaro.org
Remove the remaining check for __put_user return
value, and all the checks for err variable which
is no longer set anywhere.
Now we can only end up in give_sigsegv due to failed
lock_user_struct - thus we remove the unlock_user_struct
to avoid unlocking a region never locked.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 4155cac..f261383 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -911,7 +911,7 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
{
abi_ulong frame_addr;
struct sigframe *frame;
- int i, err = 0;
+ int i;
frame_addr = get_sigframe(ka, env, sizeof(*frame));
@@ -920,18 +920,13 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
__put_user(current_exec_domain_sig(sig),
frame-sig);
- if (err)
- goto give_sigsegv;
setup_sigcontext(frame-sc, frame-fpstate, env, set-sig[0],
frame_addr + offsetof(struct sigframe, fpstate));
- if (err)
- goto give_sigsegv;
-for(i = 1; i TARGET_NSIG_WORDS; i++) {
-if (__put_user(set-sig[i], frame-extramask[i - 1]))
-goto give_sigsegv;
-}
+for(i = 1; i TARGET_NSIG_WORDS; i++) {
+__put_user(set-sig[i], frame-extramask[i - 1]);
+}
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
@@ -950,8 +945,6 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
__put_user(val16, (uint16_t *)(frame-retcode+6));
}
- if (err)
- goto give_sigsegv;
/* Set up registers for signal handler */
env-regs[R_ESP] = frame_addr;
@@ -968,7 +961,6 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
return;
give_sigsegv:
- unlock_user_struct(frame, frame_addr, 1);
if (sig == TARGET_SIGSEGV)
ka-_sa_handler = TARGET_SIG_DFL;
force_sig(TARGET_SIGSEGV /* , current */);
--
1.9.2
[Qemu-devel] [PATCH 03/13] signal.c: remove return value from copy_siginfo_to_user
From: Riku Voipio riku.voi...@linaro.org
Since copy_siginfo_to_user always returns 0, make it void
and remove any checks for return value from calling functions.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 28 +---
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index f261383..4da9c26 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -721,11 +721,10 @@ int do_sigaction(int sig, const struct target_sigaction
*act,
return ret;
}
-static inline int copy_siginfo_to_user(target_siginfo_t *tinfo,
+static inline void copy_siginfo_to_user(target_siginfo_t *tinfo,
const target_siginfo_t *info)
{
tswap_siginfo(tinfo, info);
-return 0;
}
static inline int current_exec_domain_sig(int sig)
@@ -985,9 +984,7 @@ static void setup_rt_frame(int sig, struct target_sigaction
*ka,
__put_user(addr, frame-pinfo);
addr = frame_addr + offsetof(struct rt_sigframe, uc);
__put_user(addr, frame-puc);
- err |= copy_siginfo_to_user(frame-info, info);
- if (err)
- goto give_sigsegv;
+ copy_siginfo_to_user(frame-info, info);
/* Create the ucontext. */
__put_user(0, frame-uc.tuc_flags);
@@ -1362,9 +1359,7 @@ static void target_setup_frame(int usig, struct
target_sigaction *ka,
env-pc = ka-_sa_handler;
env-xregs[30] = return_addr;
if (info) {
-if (copy_siginfo_to_user(frame-info, info)) {
-goto give_sigsegv;
-}
+copy_siginfo_to_user(frame-info, info);
env-xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info);
env-xregs[2] = frame_addr + offsetof(struct target_rt_sigframe, uc);
}
@@ -3362,7 +3357,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
signal = current_exec_domain_sig(sig);
-err |= copy_siginfo_to_user(frame-info, info);
+copy_siginfo_to_user(frame-info, info);
/* Create the ucontext. */
__put_user(0, frame-uc.tuc_flags);
@@ -4042,10 +4037,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
__put_user(uc_addr, frame-puc);
if (ka-sa_flags SA_SIGINFO) {
-err |= copy_siginfo_to_user(frame-info, info);
-}
-if (err) {
-goto give_sigsegv;
+copy_siginfo_to_user(frame-info, info);
}
/*err |= __clear_user(frame-uc, offsetof(struct ucontext,
uc_mcontext));*/
@@ -4305,9 +4297,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
}
qemu_log(%s: 1\n, __FUNCTION__);
-if (copy_siginfo_to_user(frame-info, info)) {
-goto give_sigsegv;
-}
+copy_siginfo_to_user(frame-info, info);
/* Create the ucontext. */
__put_user(0, frame-uc.tuc_flags);
@@ -4881,7 +4871,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
signal = current_exec_domain_sig(sig);
-err |= copy_siginfo_to_user(rt_sf-info, info);
+copy_siginfo_to_user(rt_sf-info, info);
__put_user(0, rt_sf-uc.tuc_flags);
__put_user(0, rt_sf-uc.tuc_link);
@@ -5300,7 +5290,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
uc_addr = frame_addr + offsetof(struct target_rt_sigframe, uc);
__put_user(uc_addr, frame-puc);
-err |= copy_siginfo_to_user(frame-info, info);
+copy_siginfo_to_user(frame-info, info);
/* Create the ucontext */
@@ -5586,7 +5576,7 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
goto give_sigsegv;
}
-err |= copy_siginfo_to_user(frame-info, info);
+copy_siginfo_to_user(frame-info, info);
__put_user(0, frame-uc.tuc_flags);
__put_user(0, frame-uc.tuc_link);
--
1.9.2
[Qemu-devel] [PATCH 06/13] RFC comment out restore_fpu_state (sparc)
From: Riku Voipio riku.voi...@linaro.org
A function never called from anywhere, half-complete code
never finished. ifdef completly out. Alternatively we could
just kick it out as the half-ready code could be digged from
git if needed.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 8d2b6c9..2fb7fd1 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2367,11 +2367,11 @@ sigsegv:
unlock_user(sf, sf_addr, sizeof(struct target_signal_frame));
force_sig(TARGET_SIGSEGV);
}
+#if 0
static inline int
restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu)
{
int err;
-#if 0
#ifdef CONFIG_SMP
if (current-flags PF_USEDFPU)
regs-psr = ~PSR_EF;
@@ -2383,27 +2383,22 @@ restore_fpu_state(CPUSPARCState *env,
qemu_siginfo_fpu_t *fpu)
#endif
current-used_math = 1;
current-flags = ~PF_USEDFPU;
-#endif
-#if 0
if (verify_area (VERIFY_READ, fpu, sizeof(*fpu)))
return -EFAULT;
-#endif
/* XXX: incorrect */
err = copy_from_user(env-fpr[0], fpu-si_float_regs[0],
(sizeof(abi_ulong) * 32));
err |= __get_user(env-fsr, fpu-si_fsr);
-#if 0
err |= __get_user(current-thread.fpqdepth, fpu-si_fpqdepth);
if (current-thread.fpqdepth != 0)
err |= __copy_from_user(current-thread.fpqueue[0],
fpu-si_fpqueue[0],
((sizeof(unsigned long) +
(sizeof(unsigned long *)))*16));
-#endif
return err;
}
-
+#endif
static void setup_rt_frame(int sig, struct target_sigaction *ka,
target_siginfo_t *info,
--
1.9.2
[Qemu-devel] [PATCH 07/13] do_sigaltstack: remove __get_user value check
From: Riku Voipio riku.voi...@linaro.org
Access is already checked in the lock_user_struct
call before.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2fb7fd1..9118597 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -619,11 +619,12 @@ abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong
uoss_addr, abi_ulong sp)
struct target_sigaltstack ss;
ret = -TARGET_EFAULT;
-if (!lock_user_struct(VERIFY_READ, uss, uss_addr, 1)
- || __get_user(ss.ss_sp, uss-ss_sp)
- || __get_user(ss.ss_size, uss-ss_size)
- || __get_user(ss.ss_flags, uss-ss_flags))
+if (!lock_user_struct(VERIFY_READ, uss, uss_addr, 1)) {
goto out;
+}
+__get_user(ss.ss_sp, uss-ss_sp);
+__get_user(ss.ss_size, uss-ss_size);
+__get_user(ss.ss_flags, uss-ss_flags);
unlock_user_struct(uss, uss_addr, 0);
ret = -TARGET_EPERM;
--
1.9.2
[Qemu-devel] [PATCH 09/13] signal.c: setup_frame remove __put_user checks
From: Riku Voipio riku.voi...@linaro.org
Remove if(__put_user checks and their related error paths
for all architecture's setup_frame, setup_rt_frame and similar.
Remove the unlock_user_struct when the only way to end up there is
from failed lock_user_struct.
Remove err variable if there are no users for it in the function
anymore.
---
linux-user/signal.c | 97 ++---
1 file changed, 25 insertions(+), 72 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2c717b8..c3e5545 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -971,7 +971,7 @@ static void setup_rt_frame(int sig, struct target_sigaction
*ka,
{
abi_ulong frame_addr, addr;
struct rt_sigframe *frame;
- int i, err = 0;
+ int i;
frame_addr = get_sigframe(ka, env, sizeof(*frame));
@@ -996,10 +996,9 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
setup_sigcontext(frame-uc.tuc_mcontext, frame-fpstate, env,
set-sig[0], frame_addr + offsetof(struct rt_sigframe, fpstate));
-for(i = 0; i TARGET_NSIG_WORDS; i++) {
-if (__put_user(set-sig[i], frame-uc.tuc_sigmask.sig[i]))
-goto give_sigsegv;
-}
+for(i = 0; i TARGET_NSIG_WORDS; i++) {
+__put_user(set-sig[i], frame-uc.tuc_sigmask.sig[i]);
+}
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
@@ -1016,9 +1015,6 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
__put_user(val16, (uint16_t *)(frame-retcode+5));
}
- if (err)
- goto give_sigsegv;
-
/* Set up registers for signal handler */
env-regs[R_ESP] = frame_addr;
env-eip = ka-_sa_handler;
@@ -1034,7 +1030,6 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
return;
give_sigsegv:
- unlock_user_struct(frame, frame_addr, 1);
if (sig == TARGET_SIGSEGV)
ka-_sa_handler = TARGET_SIG_DFL;
force_sig(TARGET_SIGSEGV /* , current */);
@@ -1593,7 +1588,7 @@ get_sigframe(struct target_sigaction *ka, CPUARMState
*regs, int framesize)
return (sp - framesize) ~7;
}
-static int
+static void
setup_return(CPUARMState *env, struct target_sigaction *ka,
abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
{
@@ -1617,8 +1612,7 @@ setup_return(CPUARMState *env, struct target_sigaction
*ka,
if (ka-sa_flags TARGET_SA_SIGINFO)
idx += 2;
- if (__put_user(retcodes[idx], rc))
- return 1;
+__put_user(retcodes[idx], rc);
retcode = rc_addr + thumb;
}
@@ -1628,8 +1622,6 @@ setup_return(CPUARMState *env, struct target_sigaction
*ka,
env-regs[14] = retcode;
env-regs[15] = handler (thumb ? ~1 : ~3);
cpsr_write(env, cpsr, 0x);
-
- return 0;
}
static abi_ulong *setup_sigframe_v2_vfp(abi_ulong *regspace, CPUARMState *env)
@@ -1716,15 +1708,13 @@ static void setup_frame_v1(int usig, struct
target_sigaction *ka,
setup_sigcontext(frame-sc, regs, set-sig[0]);
-for(i = 1; i TARGET_NSIG_WORDS; i++) {
-if (__put_user(set-sig[i], frame-extramask[i - 1]))
-goto end;
- }
+for(i = 1; i TARGET_NSIG_WORDS; i++) {
+__put_user(set-sig[i], frame-extramask[i - 1]);
+}
setup_return(regs, ka, frame-retcode, frame_addr, usig,
frame_addr + offsetof(struct sigframe_v1, retcode));
-end:
unlock_user_struct(frame, frame_addr, 1);
}
@@ -1786,8 +1776,7 @@ static void setup_rt_frame_v1(int usig, struct
target_sigaction *ka,
setup_sigcontext(frame-uc.tuc_mcontext, env, set-sig[0]);
for(i = 0; i TARGET_NSIG_WORDS; i++) {
-if (__put_user(set-sig[i], frame-uc.tuc_sigmask.sig[i]))
-goto end;
+__put_user(set-sig[i], frame-uc.tuc_sigmask.sig[i]);
}
setup_return(env, ka, frame-retcode, frame_addr, usig,
@@ -1796,7 +1785,6 @@ static void setup_rt_frame_v1(int usig, struct
target_sigaction *ka,
env-regs[1] = info_addr;
env-regs[2] = uc_addr;
-end:
unlock_user_struct(frame, frame_addr, 1);
}
@@ -2952,8 +2940,7 @@ static void setup_frame(int sig, struct target_sigaction
* ka,
setup_sigcontext(regs, frame-sf_sc);
for(i = 0; i TARGET_NSIG_WORDS; i++) {
- if(__put_user(set-sig[i], frame-sf_mask.sig[i]))
- goto give_sigsegv;
+__put_user(set-sig[i], frame-sf_mask.sig[i]);
}
/*
@@ -2980,7 +2967,6 @@ static void setup_frame(int sig, struct target_sigaction
* ka,
return;
give_sigsegv:
-unlock_user_struct(frame, frame_addr, 1);
force_sig(TARGET_SIGSEGV/*, current*/);
}
@@ -3560,7 +3546,6 @@ static void setup_frame(int sig,
[Qemu-devel] [PATCH 00/13] __{get,put}_user return value cleanup
From: Riku Voipio riku.voi...@linaro.org
This series is primarily motivated to have a gcc-4.9 buildfix:
linux-user/syscall.c: In function ‘host_to_target_stat64’:
linux-user/qemu.h:301:19: error: right-hand operand of comma expression has no
effect [-Werror=unused-value]
((hptr), (x)), 0)
removing the unused 0 moves the bar:
linux-user/main.c: In function ‘arm_kernel_cmpxchg64_helper’:
linux-user/qemu.h:330:15: error: void value not ignored as it ought to be
__ret = __put_user((x), __hptr);\
And after fixing that, we see there is a lot of reading the return
value of __put_user and __get_user in signal.c - apparently without
much of consistency as other functions do check and others don't...
I'm not 100% sure that simply removing the checks is right way
and signal.c is quite a mess, so push this set early for comments
before spending more time on this approach.
Riku Voipio (13):
signal.c: remove __get/__put_user return value reading
signal.c setup_frame/x86: __put_user cleanup
signal.c: remove return value from copy_siginfo_to_user
signal.c: remove return value from setup_sigcontext
signal.c: remove return value from restore_sigcontext
RFC comment out restore_fpu_state (sparc)
do_sigaltstack: remove __get_user value check
do_sigreturn - remove __get_user checks
signal.c: setup_frame remove __put_user checks
remove __put/get error checks from ppc {save,restore}_user_regs
sparc64_set_context: remove __get_user checks
remove __get_user return check from PPC do_setcontext
fix gcc-4.9 compiler error on __{get,put]}_user
linux-user/qemu.h | 12 +-
linux-user/signal.c | 1221 +--
2 files changed, 515 insertions(+), 718 deletions(-)
--
1.9.2
[Qemu-devel] [PATCH 04/13] signal.c: remove return value from setup_sigcontext
From: Riku Voipio riku.voi...@linaro.org
Make all implementations of setup_sigcontext void and
remove checking it's return value from functions calling
setup_sigcontext.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 65 +
1 file changed, 21 insertions(+), 44 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 4da9c26..6427e6e 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -836,12 +836,11 @@ struct rt_sigframe
*/
/* XXX: save x87 state */
-static int
-setup_sigcontext(struct target_sigcontext *sc, struct target_fpstate *fpstate,
-CPUX86State *env, abi_ulong mask, abi_ulong fpstate_addr)
+static void setup_sigcontext(struct target_sigcontext *sc,
+struct target_fpstate *fpstate, CPUX86State *env, abi_ulong mask,
+abi_ulong fpstate_addr)
{
CPUState *cs = CPU(x86_env_get_cpu(env));
-int err = 0;
uint16_t magic;
/* already locked in setup_frame() */
@@ -874,7 +873,6 @@ setup_sigcontext(struct target_sigcontext *sc, struct
target_fpstate *fpstate,
/* non-iBCS2 extensions.. */
__put_user(mask, sc-oldmask);
__put_user(env-cr[2], sc-cr2);
- return err;
}
/*
@@ -994,9 +992,9 @@ static void setup_rt_frame(int sig, struct target_sigaction
*ka,
frame-uc.tuc_stack.ss_flags);
__put_user(target_sigaltstack_used.ss_size,
frame-uc.tuc_stack.ss_size);
- err |= setup_sigcontext(frame-uc.tuc_mcontext, frame-fpstate,
- env, set-sig[0],
-frame_addr + offsetof(struct rt_sigframe,
fpstate));
+setup_sigcontext(frame-uc.tuc_mcontext, frame-fpstate, env,
+set-sig[0], frame_addr + offsetof(struct rt_sigframe, fpstate));
+
for(i = 0; i TARGET_NSIG_WORDS; i++) {
if (__put_user(set-sig[i], frame-uc.tuc_sigmask.sig[i]))
goto give_sigsegv;
@@ -2856,10 +2854,9 @@ static inline int install_sigtramp(unsigned int *tramp,
unsigned int syscall)
return err;
}
-static inline int
-setup_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
+static inline void setup_sigcontext(CPUMIPSState *regs,
+struct target_sigcontext *sc)
{
-int err = 0;
int i;
__put_user(exception_resume_pc(regs), sc-sc_pc);
@@ -2891,8 +2888,6 @@ setup_sigcontext(CPUMIPSState *regs, struct
target_sigcontext *sc)
for (i = 0; i 32; ++i) {
__put_user(regs-active_fpu.fpr[i].d, sc-sc_fpregs[i]);
}
-
-return err;
}
static inline int
@@ -2979,8 +2974,7 @@ static void setup_frame(int sig, struct target_sigaction
* ka,
install_sigtramp(frame-sf_code, TARGET_NR_sigreturn);
-if(setup_sigcontext(regs, frame-sf_sc))
- goto give_sigsegv;
+setup_sigcontext(regs, frame-sf_sc);
for(i = 0; i TARGET_NSIG_WORDS; i++) {
if(__put_user(set-sig[i], frame-sf_mask.sig[i]))
@@ -3227,10 +3221,9 @@ static abi_ulong get_sigframe(struct target_sigaction
*ka,
return (sp - frame_size) -8ul;
}
-static int setup_sigcontext(struct target_sigcontext *sc,
+static void setup_sigcontext(struct target_sigcontext *sc,
CPUSH4State *regs, unsigned long mask)
{
-int err = 0;
int i;
#define COPY(x) __put_user(regs-x, sc-sc_##x)
@@ -3255,8 +3248,6 @@ static int setup_sigcontext(struct target_sigcontext *sc,
/* non-iBCS2 extensions.. */
__put_user(mask, sc-oldmask);
-
-return err;
}
static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
@@ -3305,7 +3296,7 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
signal = current_exec_domain_sig(sig);
-err |= setup_sigcontext(frame-sc, regs, set-sig[0]);
+setup_sigcontext(frame-sc, regs, set-sig[0]);
for (i = 0; i TARGET_NSIG_WORDS - 1; i++) {
__put_user(set-sig[i + 1], frame-extramask[i]);
@@ -3955,11 +3946,10 @@ badframe:
/* Set up a signal frame. */
-static int setup_sigcontext(struct target_sigcontext *sc,
+static void setup_sigcontext(struct target_sigcontext *sc,
CPUOpenRISCState *regs,
unsigned long mask)
{
-int err = 0;
unsigned long usp = regs-gpr[1];
/* copy the regs. they are first in sc so we can use sc directly */
@@ -3973,7 +3963,7 @@ static int setup_sigcontext(struct target_sigcontext *sc,
/* then some other stuff */
__put_user(mask, sc-oldmask);
-__put_user(usp, sc-usp); return err;
+__put_user(usp, sc-usp);
}
static inline unsigned long align_sigframe(unsigned long sp)
@@ -4048,14 +4038,10 @@ static void setup_rt_frame(int sig, struct
target_sigaction *ka,
__put_user(sas_ss_flags(env-gpr[1]), frame-uc.tuc_stack.ss_flags);
__put_user(target_sigaltstack_used.ss_size,
frame-uc.tuc_stack.ss_size);
-err
[Qemu-devel] [PATCH 01/13] signal.c: remove __get/__put_user return value reading
From: Riku Voipio riku.voi...@linaro.org
Remove all the simple cases or reading the return value
of __get_user and __put_user.
We set err = 0 in sparc versions of do_sigreturn and
sparc64_set_context to avoid compile error, but else this patch is
just the removal of err |= __get_user ... idiom.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 731 ++--
1 file changed, 363 insertions(+), 368 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7d6246f..4155cac 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -846,35 +846,35 @@ setup_sigcontext(struct target_sigcontext *sc, struct
target_fpstate *fpstate,
uint16_t magic;
/* already locked in setup_frame() */
- err |= __put_user(env-segs[R_GS].selector, (unsigned int *)sc-gs);
- err |= __put_user(env-segs[R_FS].selector, (unsigned int *)sc-fs);
- err |= __put_user(env-segs[R_ES].selector, (unsigned int *)sc-es);
- err |= __put_user(env-segs[R_DS].selector, (unsigned int *)sc-ds);
- err |= __put_user(env-regs[R_EDI], sc-edi);
- err |= __put_user(env-regs[R_ESI], sc-esi);
- err |= __put_user(env-regs[R_EBP], sc-ebp);
- err |= __put_user(env-regs[R_ESP], sc-esp);
- err |= __put_user(env-regs[R_EBX], sc-ebx);
- err |= __put_user(env-regs[R_EDX], sc-edx);
- err |= __put_user(env-regs[R_ECX], sc-ecx);
- err |= __put_user(env-regs[R_EAX], sc-eax);
-err |= __put_user(cs-exception_index, sc-trapno);
- err |= __put_user(env-error_code, sc-err);
- err |= __put_user(env-eip, sc-eip);
- err |= __put_user(env-segs[R_CS].selector, (unsigned int *)sc-cs);
- err |= __put_user(env-eflags, sc-eflags);
- err |= __put_user(env-regs[R_ESP], sc-esp_at_signal);
- err |= __put_user(env-segs[R_SS].selector, (unsigned int *)sc-ss);
+__put_user(env-segs[R_GS].selector, (unsigned int *)sc-gs);
+__put_user(env-segs[R_FS].selector, (unsigned int *)sc-fs);
+__put_user(env-segs[R_ES].selector, (unsigned int *)sc-es);
+__put_user(env-segs[R_DS].selector, (unsigned int *)sc-ds);
+__put_user(env-regs[R_EDI], sc-edi);
+__put_user(env-regs[R_ESI], sc-esi);
+__put_user(env-regs[R_EBP], sc-ebp);
+__put_user(env-regs[R_ESP], sc-esp);
+__put_user(env-regs[R_EBX], sc-ebx);
+__put_user(env-regs[R_EDX], sc-edx);
+__put_user(env-regs[R_ECX], sc-ecx);
+__put_user(env-regs[R_EAX], sc-eax);
+__put_user(cs-exception_index, sc-trapno);
+__put_user(env-error_code, sc-err);
+__put_user(env-eip, sc-eip);
+__put_user(env-segs[R_CS].selector, (unsigned int *)sc-cs);
+__put_user(env-eflags, sc-eflags);
+__put_user(env-regs[R_ESP], sc-esp_at_signal);
+__put_user(env-segs[R_SS].selector, (unsigned int *)sc-ss);
cpu_x86_fsave(env, fpstate_addr, 1);
fpstate-status = fpstate-sw;
magic = 0x;
-err |= __put_user(magic, fpstate-magic);
-err |= __put_user(fpstate_addr, sc-fpstate);
+__put_user(magic, fpstate-magic);
+__put_user(fpstate_addr, sc-fpstate);
/* non-iBCS2 extensions.. */
- err |= __put_user(mask, sc-oldmask);
- err |= __put_user(env-cr[2], sc-cr2);
+__put_user(mask, sc-oldmask);
+__put_user(env-cr[2], sc-cr2);
return err;
}
@@ -918,8 +918,8 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
goto give_sigsegv;
- err |= __put_user(current_exec_domain_sig(sig),
- frame-sig);
+__put_user(current_exec_domain_sig(sig),
+ frame-sig);
if (err)
goto give_sigsegv;
@@ -936,18 +936,18 @@ static void setup_frame(int sig, struct target_sigaction
*ka,
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
if (ka-sa_flags TARGET_SA_RESTORER) {
- err |= __put_user(ka-sa_restorer, frame-pretcode);
+__put_user(ka-sa_restorer, frame-pretcode);
} else {
uint16_t val16;
abi_ulong retcode_addr;
retcode_addr = frame_addr + offsetof(struct sigframe, retcode);
- err |= __put_user(retcode_addr, frame-pretcode);
+__put_user(retcode_addr, frame-pretcode);
/* This is popl %eax ; movl $,%eax ; int $0x80 */
val16 = 0xb858;
- err |= __put_user(val16, (uint16_t *)(frame-retcode+0));
- err |= __put_user(TARGET_NR_sigreturn, (int
*)(frame-retcode+2));
+__put_user(val16, (uint16_t *)(frame-retcode+0));
+__put_user(TARGET_NR_sigreturn, (int *)(frame-retcode+2));
val16 = 0x80cd;
- err |= __put_user(val16, (uint16_t *)(frame-retcode+6));
+__put_user(val16, (uint16_t *)(frame-retcode+6));
}
[Qemu-devel] [PATCH 05/13] signal.c: remove return value from restore_sigcontext
From: Riku Voipio riku.voi...@linaro.org
make most implementations of restore_sigcontext void and
remove checking it's return value from functions calling
restore_sigcontext.
The exception is the X86 version of the function that is
too different from others to deal in this way.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 65 +
1 file changed, 16 insertions(+), 49 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 6427e6e..8d2b6c9 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1547,12 +1547,6 @@ static const abi_ulong retcodes[4] = {
SWI_SYS_RT_SIGRETURN, SWI_THUMB_RT_SIGRETURN
};
-
-static inline int valid_user_regs(CPUARMState *regs)
-{
-return 1;
-}
-
static void
setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
CPUARMState *env, abi_ulong mask)
@@ -1843,10 +1837,9 @@ static void setup_rt_frame(int usig, struct
target_sigaction *ka,
}
}
-static int
+static void
restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
{
- int err = 0;
uint32_t cpsr;
__get_user(env-regs[0], sc-arm_r0);
@@ -1869,10 +1862,6 @@ restore_sigcontext(CPUARMState *env, struct
target_sigcontext *sc)
__get_user(cpsr, sc-arm_cpsr);
cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
#endif
-
- err |= !valid_user_regs(env);
-
- return err;
}
static long do_sigreturn_v1(CPUARMState *env)
@@ -1906,8 +1895,7 @@ static long do_sigreturn_v1(CPUARMState *env)
target_to_host_sigset_internal(host_set, set);
do_sigprocmask(SIG_SETMASK, host_set, NULL);
- if (restore_sigcontext(env, frame-sc))
- goto badframe;
+restore_sigcontext(env, frame-sc);
#if 0
/* Send SIGTRAP if we're single-stepping */
@@ -1987,8 +1975,7 @@ static int do_sigframe_return_v2(CPUARMState *env,
target_ulong frame_addr,
target_to_host_sigset(host_set, uc-tuc_sigmask);
do_sigprocmask(SIG_SETMASK, host_set, NULL);
-if (restore_sigcontext(env, uc-tuc_mcontext))
-return 1;
+restore_sigcontext(env, uc-tuc_mcontext);
/* Restore coprocessor signal frame */
regspace = uc-tuc_regspace;
@@ -2078,8 +2065,7 @@ static long do_rt_sigreturn_v1(CPUARMState *env)
target_to_host_sigset(host_set, frame-uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, host_set, NULL);
- if (restore_sigcontext(env, frame-uc.tuc_mcontext))
- goto badframe;
+restore_sigcontext(env, frame-uc.tuc_mcontext);
if (do_sigaltstack(frame_addr + offsetof(struct rt_sigframe_v1,
uc.tuc_stack), 0, get_sp_from_cpustate(env)) == -EFAULT)
goto badframe;
@@ -2890,10 +2876,9 @@ static inline void setup_sigcontext(CPUMIPSState *regs,
}
}
-static inline int
+static inline void
restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
{
-int err = 0;
int i;
__get_user(regs-CP0_EPC, sc-sc_pc);
@@ -2920,8 +2905,6 @@ restore_sigcontext(CPUMIPSState *regs, struct
target_sigcontext *sc)
for (i = 0; i 32; ++i) {
__get_user(regs-active_fpu.fpr[i].d, sc-sc_fpregs[i]);
}
-
-return err;
}
/*
@@ -3032,8 +3015,7 @@ long do_sigreturn(CPUMIPSState *regs)
target_to_host_sigset_internal(blocked, target_set);
do_sigprocmask(SIG_SETMASK, blocked, NULL);
-if (restore_sigcontext(regs, frame-sf_sc))
- goto badframe;
+restore_sigcontext(regs, frame-sf_sc);
#if 0
/*
@@ -3136,8 +3118,7 @@ long do_rt_sigreturn(CPUMIPSState *env)
target_to_host_sigset(blocked, frame-rs_uc.tuc_sigmask);
do_sigprocmask(SIG_SETMASK, blocked, NULL);
-if (restore_sigcontext(env, frame-rs_uc.tuc_mcontext))
-goto badframe;
+restore_sigcontext(env, frame-rs_uc.tuc_mcontext);
if (do_sigaltstack(frame_addr +
offsetof(struct target_rt_sigframe, rs_uc.tuc_stack),
@@ -3250,10 +3231,9 @@ static void setup_sigcontext(struct target_sigcontext
*sc,
__put_user(mask, sc-oldmask);
}
-static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
+static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
target_ulong *r0_p)
{
-unsigned int err = 0;
int i;
#define COPY(x) __get_user(regs-x, sc-sc_##x)
@@ -3278,7 +3258,6 @@ static int restore_sigcontext(CPUSH4State *regs, struct
target_sigcontext *sc,
regs-tra = -1; /* disable syscall checks */
__get_user(*r0_p, sc-sc_gregs[0]);
-return err;
}
static void setup_frame(int sig, struct target_sigaction *ka,
@@ -3423,8 +3402,7 @@ long do_sigreturn(CPUSH4State *regs)
target_to_host_sigset_internal(blocked, target_set);
do_sigprocmask(SIG_SETMASK, blocked, NULL);
-if (restore_sigcontext(regs, frame-sc, r0))
-goto badframe;
+
[Qemu-devel] [PATCH 08/13] do_sigreturn - remove __get_user checks
From: Riku Voipio riku.voi...@linaro.org
Remove if(__get_user checks and their related error paths
for all architecture's do_sigreturn. Remove the unlock_user_struct
when the only way to end up there is from failed lock_user_struct.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 62 +
1 file changed, 20 insertions(+), 42 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9118597..2c717b8 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1094,13 +1094,11 @@ long do_sigreturn(CPUX86State *env)
fprintf(stderr, do_sigreturn\n);
#endif
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
-goto badframe;
+goto sigsegv;
/* set blocked signals */
-if (__get_user(target_set.sig[0], frame-sc.oldmask))
-goto badframe;
+__get_user(target_set.sig[0], frame-sc.oldmask);
for(i = 1; i TARGET_NSIG_WORDS; i++) {
-if (__get_user(target_set.sig[i], frame-extramask[i - 1]))
-goto badframe;
+__get_user(target_set.sig[i], frame-extramask[i - 1]);
}
target_to_host_sigset_internal(set, target_set);
@@ -1114,6 +1112,7 @@ long do_sigreturn(CPUX86State *env)
badframe:
unlock_user_struct(frame, frame_addr, 0);
+sigsegv:
force_sig(TARGET_SIGSEGV);
return 0;
}
@@ -1886,12 +1885,10 @@ static long do_sigreturn_v1(CPUARMState *env)
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
goto badframe;
- if (__get_user(set.sig[0], frame-sc.oldmask))
-goto badframe;
-for(i = 1; i TARGET_NSIG_WORDS; i++) {
-if (__get_user(set.sig[i], frame-extramask[i - 1]))
-goto badframe;
-}
+__get_user(set.sig[0], frame-sc.oldmask);
+for(i = 1; i TARGET_NSIG_WORDS; i++) {
+__get_user(set.sig[i], frame-extramask[i - 1]);
+}
target_to_host_sigset_internal(host_set, set);
do_sigprocmask(SIG_SETMASK, host_set, NULL);
@@ -1907,7 +1904,6 @@ static long do_sigreturn_v1(CPUARMState *env)
return env-regs[0];
badframe:
- unlock_user_struct(frame, frame_addr, 0);
force_sig(TARGET_SIGSEGV /* , current */);
return 0;
}
@@ -3004,8 +3000,7 @@ long do_sigreturn(CPUMIPSState *regs)
goto badframe;
for(i = 0; i TARGET_NSIG_WORDS; i++) {
- if(__get_user(target_set.sig[i], frame-sf_mask.sig[i]))
- goto badframe;
+__get_user(target_set.sig[i], frame-sf_mask.sig[i]);
}
target_to_host_sigset_internal(blocked, target_set);
@@ -3646,11 +3641,9 @@ long do_sigreturn(CPUMBState *env)
goto badframe;
/* Restore blocked signals */
-if (__get_user(target_set.sig[0], frame-uc.tuc_mcontext.oldmask))
-goto badframe;
+__get_user(target_set.sig[0], frame-uc.tuc_mcontext.oldmask);
for(i = 1; i TARGET_NSIG_WORDS; i++) {
-if (__get_user(target_set.sig[i], frame-extramask[i - 1]))
-goto badframe;
+ __get_user(target_set.sig[i], frame-extramask[i - 1]);
}
target_to_host_sigset_internal(set, target_set);
do_sigprocmask(SIG_SETMASK, set, NULL);
@@ -3663,7 +3656,6 @@ long do_sigreturn(CPUMBState *env)
unlock_user_struct(frame, frame_addr, 0);
return env-regs[10];
badframe:
-unlock_user_struct(frame, frame_addr, 0);
force_sig(TARGET_SIGSEGV);
}
@@ -3821,11 +3813,9 @@ long do_sigreturn(CPUCRISState *env)
goto badframe;
/* Restore blocked signals */
- if (__get_user(target_set.sig[0], frame-sc.oldmask))
- goto badframe;
+__get_user(target_set.sig[0], frame-sc.oldmask);
for(i = 1; i TARGET_NSIG_WORDS; i++) {
- if (__get_user(target_set.sig[i], frame-extramask[i - 1]))
- goto badframe;
+__get_user(target_set.sig[i], frame-extramask[i - 1]);
}
target_to_host_sigset_internal(set, target_set);
do_sigprocmask(SIG_SETMASK, set, NULL);
@@ -3834,7 +3824,6 @@ long do_sigreturn(CPUCRISState *env)
unlock_user_struct(frame, frame_addr, 0);
return env-regs[10];
badframe:
- unlock_user_struct(frame, frame_addr, 0);
force_sig(TARGET_SIGSEGV);
}
@@ -4342,9 +4331,7 @@ long do_sigreturn(CPUS390XState *env)
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) {
goto badframe;
}
-if (__get_user(target_set.sig[0], frame-sc.oldmask[0])) {
-goto badframe;
-}
+__get_user(target_set.sig[0], frame-sc.oldmask[0]);
target_to_host_sigset_internal(set, target_set);
do_sigprocmask(SIG_SETMASK, set, NULL); /* ~_BLOCKABLE? */
@@ -4357,7 +4344,6 @@ long do_sigreturn(CPUS390XState *env)
return env-regs[2];
badframe:
-unlock_user_struct(frame, frame_addr, 0);
force_sig(TARGET_SIGSEGV);
return 0;
}
@@ -4898,15 +4884,13 @@ long
[Qemu-devel] [PATCH 13/13] fix gcc-4.9 compiler error on __{get, put]}_user
From: Riku Voipio riku.voi...@linaro.org
gcc-4.9 finds unused operand:
linux-user/syscall.c: In function ‘host_to_target_stat64’:
linux-user/qemu.h:301:19: error: right-hand operand of comma expression
has no effect [-Werror=unused-value]
((hptr), (x)), 0)
Just removing the rh operand is no good, it will error in later:
linux-user/main.c: In function ‘arm_kernel_cmpxchg64_helper’:
linux-user/qemu.h:330:15: error: void value not ignored as it ought to be
__ret = __put_user((x), __hptr);\
Thus, remove setting __ret from __get_user and __put_user, as and
set the right hand operand to (void)0 to make it clear that these
return never nothing.
This commit depends on the signal.c cleanup, to ensure bisectable
version history.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
Cc: Richard Henderson r...@twiddle.net
---
linux-user/qemu.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 36d4a73..772e180 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -298,7 +298,7 @@ static inline int access_ok(int type, abi_ulong addr,
abi_ulong size)
__builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p, \
__builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p, \
__builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p, abort \
- ((hptr), (x)), 0)
+ ((hptr), (x)), (void)0)
#define __get_user_e(x, hptr, e)\
((x) = (typeof(*hptr))( \
@@ -306,7 +306,7 @@ static inline int access_ok(int type, abi_ulong addr,
abi_ulong size)
__builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,\
__builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p, \
__builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p, abort \
- (hptr)), 0)
+ (hptr)), (void)0)
#ifdef TARGET_WORDS_BIGENDIAN
# define __put_user(x, hptr) __put_user_e(x, hptr, be)
@@ -325,9 +325,9 @@ static inline int access_ok(int type, abi_ulong addr,
abi_ulong size)
({ \
abi_ulong __gaddr = (gaddr); \
target_type *__hptr; \
-abi_long __ret;\
+abi_long __ret = 0;
\
if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) {
\
-__ret = __put_user((x), __hptr); \
+__put_user((x), __hptr); \
unlock_user(__hptr, __gaddr, sizeof(target_type)); \
} else \
__ret = -TARGET_EFAULT;
\
@@ -338,9 +338,9 @@ static inline int access_ok(int type, abi_ulong addr,
abi_ulong size)
({ \
abi_ulong __gaddr = (gaddr); \
target_type *__hptr; \
-abi_long __ret;\
+abi_long __ret = 0;
\
if ((__hptr = lock_user(VERIFY_READ, __gaddr, sizeof(target_type), 1))) { \
-__ret = __get_user((x), __hptr); \
+__get_user((x), __hptr); \
unlock_user(__hptr, __gaddr, 0); \
} else { \
/* avoid warning */\
--
1.9.2
[Qemu-devel] [PATCH 10/13] remove __put/get error checks from ppc {save, restore}_user_regs
From: Riku Voipio riku.voi...@linaro.org
As __get_user and __put_user do not return errors, remove the
if checks from around them. This allows making the save/restore
functions void.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
Cc: Alexander Graf ag...@suse.de
---
linux-user/signal.c | 126 +---
1 file changed, 41 insertions(+), 85 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index c3e5545..efb3562 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4506,7 +4506,7 @@ static target_ulong get_sigframe(struct target_sigaction
*ka,
return newsp;
}
-static int save_user_regs(CPUPPCState *env, struct target_mcontext *frame,
+static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame,
int sigret)
{
target_ulong msr = env-msr;
@@ -4519,21 +4519,17 @@ static int save_user_regs(CPUPPCState *env, struct
target_mcontext *frame,
/* Save general registers. */
for (i = 0; i ARRAY_SIZE(env-gpr); i++) {
-if (__put_user(env-gpr[i], frame-mc_gregs[i])) {
-return 1;
-}
+ __put_user(env-gpr[i], frame-mc_gregs[i]);
}
-if (__put_user(env-nip, frame-mc_gregs[TARGET_PT_NIP])
-|| __put_user(env-ctr, frame-mc_gregs[TARGET_PT_CTR])
-|| __put_user(env-lr, frame-mc_gregs[TARGET_PT_LNK])
-|| __put_user(env-xer, frame-mc_gregs[TARGET_PT_XER]))
-return 1;
+__put_user(env-nip, frame-mc_gregs[TARGET_PT_NIP]);
+__put_user(env-ctr, frame-mc_gregs[TARGET_PT_CTR]);
+__put_user(env-lr, frame-mc_gregs[TARGET_PT_LNK]);
+__put_user(env-xer, frame-mc_gregs[TARGET_PT_XER]);
for (i = 0; i ARRAY_SIZE(env-crf); i++) {
ccr |= env-crf[i] (32 - ((i + 1) * 4));
}
-if (__put_user(ccr, frame-mc_gregs[TARGET_PT_CCR]))
-return 1;
+__put_user(ccr, frame-mc_gregs[TARGET_PT_CCR]);
/* Save Altivec registers if necessary. */
if (env-insns_flags PPC_ALTIVEC) {
@@ -4541,69 +4537,53 @@ static int save_user_regs(CPUPPCState *env, struct
target_mcontext *frame,
ppc_avr_t *avr = env-avr[i];
ppc_avr_t *vreg = frame-mc_vregs.altivec[i];
-if (__put_user(avr-u64[0], vreg-u64[0]) ||
-__put_user(avr-u64[1], vreg-u64[1])) {
-return 1;
-}
+__put_user(avr-u64[0], vreg-u64[0]);
+__put_user(avr-u64[1], vreg-u64[1]);
}
/* Set MSR_VR in the saved MSR value to indicate that
frame-mc_vregs contains valid data. */
msr |= MSR_VR;
-if (__put_user((uint32_t)env-spr[SPR_VRSAVE],
- frame-mc_vregs.altivec[32].u32[3]))
-return 1;
+__put_user((uint32_t)env-spr[SPR_VRSAVE],
+ frame-mc_vregs.altivec[32].u32[3]);
}
/* Save floating point registers. */
if (env-insns_flags PPC_FLOAT) {
for (i = 0; i ARRAY_SIZE(env-fpr); i++) {
-if (__put_user(env-fpr[i], frame-mc_fregs[i])) {
-return 1;
-}
+__put_user(env-fpr[i], frame-mc_fregs[i]);
}
-if (__put_user((uint64_t) env-fpscr, frame-mc_fregs[32]))
-return 1;
+__put_user((uint64_t) env-fpscr, frame-mc_fregs[32]);
}
/* Save SPE registers. The kernel only saves the high half. */
if (env-insns_flags PPC_SPE) {
#if defined(TARGET_PPC64)
for (i = 0; i ARRAY_SIZE(env-gpr); i++) {
-if (__put_user(env-gpr[i] 32, frame-mc_vregs.spe[i])) {
-return 1;
-}
+__put_user(env-gpr[i] 32, frame-mc_vregs.spe[i]);
}
#else
for (i = 0; i ARRAY_SIZE(env-gprh); i++) {
-if (__put_user(env-gprh[i], frame-mc_vregs.spe[i])) {
-return 1;
-}
+__put_user(env-gprh[i], frame-mc_vregs.spe[i]);
}
#endif
/* Set MSR_SPE in the saved MSR value to indicate that
frame-mc_vregs contains valid data. */
msr |= MSR_SPE;
-if (__put_user(env-spe_fscr, frame-mc_vregs.spe[32]))
-return 1;
+__put_user(env-spe_fscr, frame-mc_vregs.spe[32]);
}
/* Store MSR. */
-if (__put_user(msr, frame-mc_gregs[TARGET_PT_MSR]))
-return 1;
+__put_user(msr, frame-mc_gregs[TARGET_PT_MSR]);
/* Set up the sigreturn trampoline: li r0,sigret; sc. */
if (sigret) {
-if (__put_user(0x3800UL | sigret, frame-tramp[0]) ||
-__put_user(0x4402UL, frame-tramp[1])) {
-return 1;
-}
+__put_user(0x3800UL | sigret, frame-tramp[0]);
+__put_user(0x4402UL, frame-tramp[1]);
}
-
-return 0;
}
-static int restore_user_regs(CPUPPCState *env,
- struct target_mcontext *frame, int sig)
+static void restore_user_regs(CPUPPCState *env,
+
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi,
Anything bigger than 16bytes, no? And that is the whole point that we
are talking about? Or the 16bytes that we are using can be at any place
on the buffer?
Yes. It's a ring buffer, with rptr pointing to the first used element
and wptr pointing to the first free element.
So, what we need is a function to move the content we are interested
(between rptr and wptr) in to the head of the ring buffer, set rptr to
0, set wptr to count. We obviously need to do that in post_load(), but
as I've noticed meanwhile also in pre_save, otherwise old qemu will get
things wrong in case the buffer is wrapped (rptr wptr).
Hi, Gerd. Maybe we just need to do in post_load(). Such as the follow code:
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
PS2State *ps2 = s-common;
PS2Queue *q = ps2-queue;
int size;
int i;
if (version_id == 2)
s-scancode_set=2;
/* the new version id for this patch */
if (version_id == 4) {
return 0;
}
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
printf( ==kbd: rptr: %d, wptr: %d, count: %d, size: %d\n, q-rptr,
q-wptr, q-count, size);
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = q-data[q-rptr];
if (++q-rptr == 256) {
q-rptr = 0;
}
}
/* reset rptr/wptr/count */
q-rptr = 0;
q-wptr = size;
q-count = size;
ps2-update_irq(ps2-update_arg, q-count != 0);
return 0;
}
Any problem? Thanks.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH] block: Prevent coroutine stack overflow when recursing in bdrv_open_backing_file.
[ Cc: qemu-devel ] Patches should never be sent to qemu-stable only. Am 23.04.2014 um 13:45 hat Stefan Hajnoczi geschrieben: On Tue, Apr 22, 2014 at 05:05:27PM +0200, Benoît Canet wrote: In 1.7.1 qcow2_create2 reopen the file for flushing without the BDRV_O_NO_BACKING flags. As a consequence the code would recursively open the whole backing chain. These three stack arrays would pile up through the recursion and lead to a coroutine stack overflow. Convert these array to malloced buffers in order to streamline the coroutine footprint. Symptoms where freezes or segfaults on production machines while taking QMP externals snapshots. The overflow disturbed coroutine switching. Signed-off-by: Benoit Canet benoit.ca...@gmail.com --- block.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) This is patch hints at dropping the PATH_MAX constant completely: Let's use g_strdup()/g_strdup_printf()/etc for filenames and stop using hard-coded limits. But as a bug fix this patch is good. I had to resolve conflicts against the block tree - I guess you wrote the patch against v1.7.1. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH 00/13] __{get, put}_user return value cleanup
On 23 April 2014 14:11, riku.voi...@linaro.org wrote:
From: Riku Voipio riku.voi...@linaro.org
This series is primarily motivated to have a gcc-4.9 buildfix:
linux-user/syscall.c: In function ‘host_to_target_stat64’:
linux-user/qemu.h:301:19: error: right-hand operand of comma expression has
no effect [-Werror=unused-value]
((hptr), (x)), 0)
removing the unused 0 moves the bar:
linux-user/main.c: In function ‘arm_kernel_cmpxchg64_helper’:
linux-user/qemu.h:330:15: error: void value not ignored as it ought to be
__ret = __put_user((x), __hptr);\
And after fixing that, we see there is a lot of reading the return
value of __put_user and __get_user in signal.c - apparently without
much of consistency as other functions do check and others don't...
I'm not 100% sure that simply removing the checks is right way
and signal.c is quite a mess, so push this set early for comments
before spending more time on this approach.
So the reason for this inconsistency is that QEMU's approach
to checking that memory is accessible differs from the Linux kernel.
In Linux, the check effectively happens on each get/put access
(for free courtesy of the MMU), so the __get/__put_user accessors
can each individually succeed or fail, and the return value must
always be checked.
In QEMU, we do an access check for an entire region via
lock_user (or lock_user_struct) and catch the failure case there.
So our __get/__put_user accessors can never fail. However we
retained the return value because a lot of the code in signal.c
is pretty much copy-and-pasted from the kernel sources with
minor tweaks, and so it includes the gather up and check
return values at each step code.
Generally in code review or if I've been tidying up or fixing the
signal handling code for some other reason I've removed the
pointless accumulation of return values. But a bunch of our
less-maintained architectures still have it.
signal.c: remove __get/__put_user return value reading
signal.c setup_frame/x86: __put_user cleanup
signal.c: remove return value from copy_siginfo_to_user
signal.c: remove return value from setup_sigcontext
signal.c: remove return value from restore_sigcontext
RFC comment out restore_fpu_state (sparc)
do_sigaltstack: remove __get_user value check
do_sigreturn - remove __get_user checks
signal.c: setup_frame remove __put_user checks
remove __put/get error checks from ppc {save,restore}_user_regs
sparc64_set_context: remove __get_user checks
remove __get_user return check from PPC do_setcontext
fix gcc-4.9 compiler error on __{get,put]}_user
Given that signal.c is a mix of code for all our supported
architectures, it would be nice if these patch summary lines
were a bit more consistent:
linux-user/signal.c: Generic change affecting all archs
linux-user/signal.c: PPC: Change affecting only PPC
and so on.
I should probably have another go at a tidyup I was
looking at ages ago which splits linux-user/signal.c
into linux-user/$arch/signal.c...
thanks
-- PMM
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Gonglei (Arei) arei.gong...@huawei.com wrote:
Hi,
Anything bigger than 16bytes, no? And that is the whole point that we
are talking about? Or the 16bytes that we are using can be at any place
on the buffer?
Yes. It's a ring buffer, with rptr pointing to the first used element
and wptr pointing to the first free element.
So, what we need is a function to move the content we are interested
(between rptr and wptr) in to the head of the ring buffer, set rptr to
0, set wptr to count. We obviously need to do that in post_load(), but
as I've noticed meanwhile also in pre_save, otherwise old qemu will get
things wrong in case the buffer is wrapped (rptr wptr).
Hi, Gerd. Maybe we just need to do in post_load(). Such as the follow code:
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
cast not needed.
PS2State *ps2 = s-common;
PS2Queue *q = ps2-queue;
int size;
int i;
if (version_id == 2)
s-scancode_set=2;
/* the new version id for this patch */
if (version_id == 4) {
return 0;
}
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
printf( ==kbd: rptr: %d, wptr: %d, count: %d, size: %d\n, q-rptr,
q-wptr, q-count, size);
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = q-data[q-rptr];
It is me, or this don't work if the destination and source regions
overlap? Yes, it depens on how it overlaps.
if (++q-rptr == 256) {
q-rptr = 0;
}
}
/* reset rptr/wptr/count */
q-rptr = 0;
q-wptr = size;
q-count = size;
ps2-update_irq(ps2-update_arg, q-count != 0);
return 0;
}
Any problem? Thanks.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global
On 04/22/14 20:28, Paolo Bonzini wrote: Il 22/04/2014 16:23, Don Slutz ha scritto: Currently I have issues running tests: dcs-xen-50:~/qemu/outmake test Use make check, not make test. make test is old and suffered some bitrot. Paolo Thanks, make check is working. -Don Slutz
[Qemu-devel] [PATCH 11/13] sparc64_set_context: remove __get_user checks
From: Riku Voipio riku.voi...@linaro.org
Remove checks of __get_user and the err variable
used to control flow with it.
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/signal.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index efb3562..487fa2f 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2547,7 +2547,6 @@ void sparc64_set_context(CPUSPARCState *env)
target_mc_gregset_t *grp;
abi_ulong pc, npc, tstate;
abi_ulong fp, i7, w_addr;
-int err = 0;
unsigned int i;
ucp_addr = env-regwptr[UREG_I0];
@@ -2556,15 +2555,14 @@ void sparc64_set_context(CPUSPARCState *env)
grp = ucp-tuc_mcontext.mc_gregs;
__get_user(pc, ((*grp)[MC_PC]));
__get_user(npc, ((*grp)[MC_NPC]));
-if (err || ((pc | npc) 3))
+if ((pc | npc) 3)
goto do_sigsegv;
if (env-regwptr[UREG_I1]) {
target_sigset_t target_set;
sigset_t set;
if (TARGET_NSIG_WORDS == 1) {
-if (__get_user(target_set.sig[0], ucp-tuc_sigmask.sig[0]))
-goto do_sigsegv;
+__get_user(target_set.sig[0], ucp-tuc_sigmask.sig[0]);
} else {
abi_ulong *src, *dst;
src = ucp-tuc_sigmask.sig;
@@ -2572,8 +2570,6 @@ void sparc64_set_context(CPUSPARCState *env)
for (i = 0; i TARGET_NSIG_WORDS; i++, dst++, src++) {
__get_user(*dst, src);
}
-if (err)
-goto do_sigsegv;
}
target_to_host_sigset_internal(set, target_set);
do_sigprocmask(SIG_SETMASK, set, NULL);
@@ -2616,7 +2612,7 @@ void sparc64_set_context(CPUSPARCState *env)
* is only restored if fenab is non-zero in:
* __get_user(fenab, (ucp-tuc_mcontext.mc_fpregs.mcfpu_enab));
*/
-err |= __get_user(env-fprs, (ucp-tuc_mcontext.mc_fpregs.mcfpu_fprs));
+__get_user(env-fprs, (ucp-tuc_mcontext.mc_fpregs.mcfpu_fprs));
{
uint32_t *src = ucp-tuc_mcontext.mc_fpregs.mcfpu_fregs.sregs;
for (i = 0; i 64; i++, src++) {
@@ -2631,8 +2627,6 @@ void sparc64_set_context(CPUSPARCState *env)
(ucp-tuc_mcontext.mc_fpregs.mcfpu_fsr));
__get_user(env-gsr,
(ucp-tuc_mcontext.mc_fpregs.mcfpu_gsr));
-if (err)
-goto do_sigsegv;
unlock_user_struct(ucp, ucp_addr, 0);
return;
do_sigsegv:
--
1.9.2
Re: [Qemu-devel] [PATCH v2 0/2] HMP: support specifying dump format for dump-guest-memory
On Wed, 23 Apr 2014 12:38:12 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: On 17/04/14 10:15, Qiao Nuohan wrote: The last version is here: http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg00018.html ChangLog: Changes from v7 to v8: 1. add a patch to fix doc of dump-guest-memory Qiao Nuohan (2): HMP: fix doc of dump-guest-memory HMP: support specifying dump format for dump-guest-memory hmp-commands.hx | 30 +++--- hmp.c | 25 ++--- 2 files changed, 41 insertions(+), 14 deletions(-) Luiz, are you going to pick these patches for your HMP tree? Yes, but I'm late with QMP review.
[Qemu-devel] [PATCH] ivshmem: fix potential OOB r/w access (#2)
Fix OOB access via malformed incoming_posn parameters
and check that requested memory is actually alloced.
tmp_fd does not leak on error; see following dup() call.
According to docu g_realloc() may return NULL so we need
to check that. Passes checkpatch.pl, after also fixing wrong
ivshmem.c style itself.
Signed-off-by: Sebastian Krahmer krah...@suse.de
---
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 8d144ba..5c0f116 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -28,6 +28,7 @@
#include sys/mman.h
#include sys/types.h
+#include limits.h
#define PCI_VENDOR_ID_IVSHMEM PCI_VENDOR_ID_REDHAT_QUMRANET
#define PCI_DEVICE_ID_IVSHMEM 0x1110
@@ -401,23 +402,41 @@ static void close_guest_eventfds(IVShmemState *s, int
posn)
/* this function increase the dynamic storage need to store data about other
* guests */
-static void increase_dynamic_storage(IVShmemState *s, int new_min_size) {
+static int increase_dynamic_storage(IVShmemState *s, int new_min_size)
+{
int j, old_nb_alloc;
+/* check for integer overflow */
+if (new_min_size = INT_MAX/sizeof(Peer) - 1 || new_min_size = 0) {
+return -1;
+}
+
old_nb_alloc = s-nb_peers;
-while (new_min_size = s-nb_peers)
-s-nb_peers = s-nb_peers * 2;
+/* heap allocators already have good alloc strategies, no need
+ * to re-implement here. +1 because #new_min_size is used as last array
+ * index */
+if (new_min_size = s-nb_peers) {
+s-nb_peers = new_min_size + 1;
+} else {
+return 0;
+}
IVSHMEM_DPRINTF(bumping storage to %d guests\n, s-nb_peers);
s-peers = g_realloc(s-peers, s-nb_peers * sizeof(Peer));
+/* g_realloc() may return NULL */
+if (!s-peers) {
+return -1;
+}
+
/* zero out new pointers */
for (j = old_nb_alloc; j s-nb_peers; j++) {
s-peers[j].eventfds = NULL;
s-peers[j].nb_eventfds = 0;
}
+return 0;
}
static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
@@ -428,13 +447,20 @@ static void ivshmem_read(void *opaque, const uint8_t *
buf, int flags)
long incoming_posn;
memcpy(incoming_posn, buf, sizeof(long));
+if (incoming_posn -1) {
+fprintf(stderr, invalid posn index\n);
+return;
+}
/* pick off s-server_chr-msgfd and store it, posn should accompany msg */
tmp_fd = qemu_chr_fe_get_msgfd(s-server_chr);
IVSHMEM_DPRINTF(posn is %ld, fd is %d\n, incoming_posn, tmp_fd);
/* make sure we have enough space for this guest */
if (incoming_posn = s-nb_peers) {
-increase_dynamic_storage(s, incoming_posn);
+if (increase_dynamic_storage(s, incoming_posn) 0) {
+fprintf(stderr, increase_dynamic_storage() failed\n);
+return;
+}
}
if (tmp_fd == -1) {
--
~ perl self.pl
~ $_='print\$_=\47$_\47;eval';eval
~ krah...@suse.de - SuSE Security Team
Re: [Qemu-devel] vm disk blockio and fileio
On Mon, Mar 31, 2014 at 10:09:13PM +0800, longguang.yue wrote: from the aspect of qemu, there are file and block disk type, what is the final difference between the two types ? if its type is block and source is volume or physical disk, who finally read/write the disk? i thought block is raw format, so raw driver will do the actual r/w, am i right? if so qemu r/w using fileio no matter what disk type it is ? what is the call flows when disk type is block when vm r/w? QEMU is a host userspace application and it performs disk I/O on behalf of the guest. The difference between raw image files and host block devices (e.g. LVM logical volumes or host partitions) is very small - the code for both is shared in block/raw-posix.c. If you are using the default -drive aio=threads option then a thread pool will perform pread()/pwrite()/preadv()/pwritev()/fdatasync(). If you are using -drive aio=native then Linux AIO APIs are used (io_submit() and io_getevents()). Stefan
Re: [Qemu-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global
Am 23.04.2014 01:13, schrieb Don Slutz: And that Andreas Färber would like me to extend qom-test to check that at least 1 instance of each machine does use each .compat_props so that a typo there gets checked. No, the machines are already being checked. I was saying, if we are going to check correctness of types for .compat_props, we can easily test for typos using make check (or make check-qtest). Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Gonglei (Arei) arei.gong...@huawei.com wrote:
Hi,
Anything bigger than 16bytes, no? And that is the whole point that we
are talking about? Or the 16bytes that we are using can be at any place
on the buffer?
Yes. It's a ring buffer, with rptr pointing to the first used element
and wptr pointing to the first free element.
So, what we need is a function to move the content we are interested
(between rptr and wptr) in to the head of the ring buffer, set rptr to
0, set wptr to count. We obviously need to do that in post_load(), but
as I've noticed meanwhile also in pre_save, otherwise old qemu will get
things wrong in case the buffer is wrapped (rptr wptr).
Hi, Gerd. Maybe we just need to do in post_load(). Such as the follow code:
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
cast not needed.
This is the original code, I don't change it. Maybe I should post the diff
file, Sorry.
PS2State *ps2 = s-common;
PS2Queue *q = ps2-queue;
int size;
int i;
if (version_id == 2)
s-scancode_set=2;
/* the new version id for this patch */
if (version_id == 4) {
return 0;
}
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
printf( ==kbd: rptr: %d, wptr: %d, count: %d, size: %d\n, q-rptr,
q-wptr, q-count, size);
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = q-data[q-rptr];
It is me, or this don't work if the destination and source regions
overlap? Yes, it depens on how it overlaps.
Good catch, Juan, Thanks. It should use a temp data array to save the q-data[]
value,
Such as below:
int size;
int i;
int tmp_data[PS2_QUEUE_SIZE];
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
printf( ==kbd: rptr: %d, wptr: %d, count: %d, size: %d\n, q-rptr,
q-wptr, q-count, size);
for (i = 0; i size; i++) {
/* move the queue elements to the tmp data array */
tmp_data[i] = q-data[q-rptr];
if (++q-rptr == 256) {
q-rptr = 0;
}
}
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = tmp_data[i];
}
/* reset rptr/wptr/count */
q-rptr = 0;
q-wptr = size;
q-count = size;
s-update_irq(s-update_arg, q-count != 0);
return 0;
}
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi,
/* the new version id for this patch */
if (version_id == 4) {
return 0;
}
I don't think we need a new version.
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
I'd rather do size = q-count PS2_QUEUE_SIZE ? 0 : q-count;
i.e. if it doesn't fit better throw away everything, which reduces the
chance that we'll deliver a incomplete message to the guest.
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = q-data[q-rptr];
if (++q-rptr == 256) {
q-rptr = 0;
}
}
That fails for the wraparound (rptr wptr) case.
cheers,
Gerd
[Qemu-devel] [QEMU v8 PATCH 0/7] SMBIOS: build full tables in QEMU
On Wed, Apr 23, 2014 at 12:13:48PM +0200, Gerd Hoffmann wrote: Series looks good to me. Awesome, thanks! Here's one more iteration, rearranging things to start with the most benign and uncontroversial patches, and progressing toward the payload at the end :) /me wonders though how you've tested the final revision and the compat stuff. Patch #6 doesn't actually add a new machine type where the non-legacy mode is active ... Why, by commenting and uncommenting smbios_legacy_mode = true; in the *compat_2_0() functions :) I realized I was being silly after sending out the v7 series, and added something more or less similar to what you wrote, and was saving it for v8. I squashed it on top of the compat functions, see patch 2/7. I'll go wait for a few days for additional feedback, then [...] send out a pull request. Sounds like a plan! BTW, would it make sense to have Kevin apply this: http://www.seabios.org/pipermail/seabios/2014-April/007856.html to SeaBIOS and then pull a new blob from there around the same time (or before ?) this patch set is applied to QEMU ? Thanks much, Gabriel Gabriel L. Somlo (7): E820: Add interface for accessing e820 table PC: Add machine version 2.1 for piix and q35 SMBIOS: Rename symbols to better reflect future use SMBIOS: Update header file definitions SMBIOS: Use macro to set smbios defaults SMBIOS: Use bitmaps to prevent incompatible comand line options SMBIOS: Build aggregate smbios tables and entry point hw/i386/pc.c | 39 ++- hw/i386/pc_piix.c| 43 ++- hw/i386/pc_q35.c | 37 ++- hw/i386/smbios.c | 789 +++ include/hw/i386/pc.h | 2 + include/hw/i386/smbios.h | 99 -- 6 files changed, 902 insertions(+), 107 deletions(-) -- 1.9.0
[Qemu-devel] [QEMU v8 PATCH 1/7] E820: Add interface for accessing e820 table
Add the following two functions:
- e820_get_num_entries() - query the size of the e820 table
- e820_get_entry() - grab an entry matching a given set of criteria
This interface is currently necessary for creating type 19
(memory array mapped address) structures in smbios.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/pc.c | 15 +++
include/hw/i386/pc.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 14f0d91..aefb315 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -612,6 +612,21 @@ int e820_add_entry(uint64_t address, uint64_t length,
uint32_t type)
return e820_entries;
}
+int e820_get_num_entries(void)
+{
+return e820_entries;
+}
+
+bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t
*length)
+{
+if (idx e820_entries e820_table[idx].type == cpu_to_le32(type)) {
+*address = le64_to_cpu(e820_table[idx].address);
+*length = le64_to_cpu(e820_table[idx].length);
+return true;
+}
+return false;
+}
+
/* Calculates the limit to CPU APIC ID values
*
* This function returns the limit for the APIC ID value, so that all
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9010246..9f26e14 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -239,6 +239,8 @@ uint16_t pvpanic_port(void);
#define E820_UNUSABLE 5
int e820_add_entry(uint64_t, uint64_t, uint32_t);
+int e820_get_num_entries(void);
+bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
#define PC_Q35_COMPAT_1_7 \
PC_COMPAT_1_7, \
--
1.9.0
[Qemu-devel] [QEMU v8 PATCH 7/7] SMBIOS: Build aggregate smbios tables and entry point
Build an aggregate set of smbios tables and an entry point structure.
Insert tables and entry point into fw_cfg respectively under
etc/smbios/smbios-tables and etc/smbios/smbios-anchor.
Machine types = 2.0 will for now continue using field-by-field
overrides to SeaBIOS defaults, but for machine types 2.1 and up we
expect the BIOS to look for and use the aggregate tables generated
by this patch.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/pc.c | 24 +-
hw/i386/pc_piix.c| 4 +-
hw/i386/pc_q35.c | 4 +-
hw/i386/smbios.c | 708 +--
include/hw/i386/smbios.h | 5 +-
5 files changed, 720 insertions(+), 25 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7155269..07de238 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -642,8 +642,8 @@ static unsigned int pc_apic_id_limit(unsigned int max_cpus)
static FWCfgState *bochs_bios_init(void)
{
FWCfgState *fw_cfg;
-uint8_t *smbios_table;
-size_t smbios_len;
+uint8_t *smbios_tables, *smbios_anchor;
+size_t smbios_tables_len, smbios_anchor_len;
uint64_t *numa_fw_cfg;
int i, j;
unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
@@ -670,10 +670,21 @@ static FWCfgState *bochs_bios_init(void)
acpi_tables, acpi_tables_len);
fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
-smbios_table = smbios_get_table_legacy(smbios_len);
-if (smbios_table)
+smbios_tables = smbios_get_table_legacy(smbios_tables_len);
+if (smbios_tables) {
fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
- smbios_table, smbios_len);
+ smbios_tables, smbios_tables_len);
+}
+
+smbios_get_tables(smbios_tables, smbios_tables_len,
+ smbios_anchor, smbios_anchor_len);
+if (smbios_anchor) {
+fw_cfg_add_file(fw_cfg, etc/smbios/smbios-tables,
+smbios_tables, smbios_tables_len);
+fw_cfg_add_file(fw_cfg, etc/smbios/smbios-anchor,
+smbios_anchor, smbios_anchor_len);
+}
+
fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
e820_reserve, sizeof(e820_reserve));
fw_cfg_add_file(fw_cfg, etc/e820, e820_table,
@@ -1042,6 +1053,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState
*icc_bridge)
sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
APIC_DEFAULT_ADDRESS, 0x1000);
}
+
+/* tell smbios about cpuid version and features */
+smbios_set_cpuid(cpu-env.cpuid_version, cpu-env.features[FEAT_1_EDX]);
}
/* pci-info ROM file. Little endian format */
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d149f81..5b3594b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_pci_info;
static bool has_acpi_build = true;
static bool smbios_defaults = true;
+static bool smbios_legacy_mode;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
* host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte
* pages in the host.
@@ -146,7 +147,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
if (smbios_defaults) {
/* These values are guest ABI, do not change */
smbios_set_defaults(QEMU, Standard PC (i440FX + PIIX, 1996),
-args-machine-name);
+args-machine-name, smbios_legacy_mode);
}
/* allocate ram and load rom/bios */
@@ -264,6 +265,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
static void pc_compat_2_0(QEMUMachineInitArgs *args)
{
+smbios_legacy_mode = true;
}
static void pc_compat_1_7(QEMUMachineInitArgs *args)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1ede53c..5b48231 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -51,6 +51,7 @@
static bool has_pci_info;
static bool has_acpi_build = true;
static bool smbios_defaults = true;
+static bool smbios_legacy_mode;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
* host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte
* pages in the host.
@@ -133,7 +134,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
if (smbios_defaults) {
/* These values are guest ABI, do not change */
smbios_set_defaults(QEMU, Standard PC (Q35 + ICH9, 2009),
-args-machine-name);
+args-machine-name, smbios_legacy_mode);
}
/* allocate ram and load rom/bios */
@@ -242,6 +243,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
static void pc_compat_2_0(QEMUMachineInitArgs *args)
{
+smbios_legacy_mode = true;
}
static void pc_compat_1_7(QEMUMachineInitArgs *args)
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index
[Qemu-devel] [QEMU v8 PATCH 5/7] SMBIOS: Use macro to set smbios defaults
The function smbios_set_defaults() uses a repeating code pattern
for each field. This patch replaces that pattern with a macro.
This patch contains no functional changes.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/smbios.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e734d4c..9f83bfb 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -260,20 +260,6 @@ static void smbios_build_type_1_fields(void)
}
}
-void smbios_set_defaults(const char *manufacturer, const char *product,
- const char *version)
-{
-if (!type1.manufacturer) {
-type1.manufacturer = manufacturer;
-}
-if (!type1.product) {
-type1.product = product;
-}
-if (!type1.version) {
-type1.version = version;
-}
-}
-
uint8_t *smbios_get_table_legacy(size_t *length)
{
if (!smbios_immutable) {
@@ -288,6 +274,19 @@ uint8_t *smbios_get_table_legacy(size_t *length)
/* end: legacy setup functions for = 2.0 machines */
+#define SMBIOS_SET_DEFAULT(field, value) \
+if (!field) { \
+field = value;\
+}
+
+void smbios_set_defaults(const char *manufacturer, const char *product,
+ const char *version)
+{
+SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
+SMBIOS_SET_DEFAULT(type1.product, product);
+SMBIOS_SET_DEFAULT(type1.version, version);
+}
+
static void save_opt(const char **dest, QemuOpts *opts, const char *name)
{
const char *val = qemu_opt_get(opts, name);
--
1.9.0
[Qemu-devel] [QEMU v8 PATCH 3/7] SMBIOS: Rename symbols to better reflect future use
Rename the following symbols:
- smbios_set_type1_defaults() to the more general smbios_set_defaults();
- bool smbios_type1_defaults to the more general smbios_defaults;
- smbios_get_table() to smbios_get_table_legacy();
This patch contains no functional changes.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/pc.c | 2 +-
hw/i386/pc_piix.c| 14 +++---
hw/i386/pc_q35.c | 10 +-
hw/i386/smbios.c | 18 --
include/hw/i386/smbios.h | 6 +++---
5 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index aefb315..7155269 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -670,7 +670,7 @@ static FWCfgState *bochs_bios_init(void)
acpi_tables, acpi_tables_len);
fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
-smbios_table = smbios_get_table(smbios_len);
+smbios_table = smbios_get_table_legacy(smbios_len);
if (smbios_table)
fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
smbios_table, smbios_len);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f13d0ac..d149f81 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,7 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_pci_info;
static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
* host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte
* pages in the host.
@@ -143,10 +143,10 @@ static void pc_init1(QEMUMachineInitArgs *args,
guest_info-has_pci_info = has_pci_info;
guest_info-isapc_ram_fw = !pci_enabled;
-if (smbios_type1_defaults) {
+if (smbios_defaults) {
/* These values are guest ABI, do not change */
-smbios_set_type1_defaults(QEMU, Standard PC (i440FX + PIIX, 1996),
- args-machine-name);
+smbios_set_defaults(QEMU, Standard PC (i440FX + PIIX, 1996),
+args-machine-name);
}
/* allocate ram and load rom/bios */
@@ -269,7 +269,7 @@ static void pc_compat_2_0(QEMUMachineInitArgs *args)
static void pc_compat_1_7(QEMUMachineInitArgs *args)
{
pc_compat_2_0(args);
-smbios_type1_defaults = false;
+smbios_defaults = false;
gigabyte_align = false;
option_rom_has_mr = true;
x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
@@ -356,7 +356,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs
*args)
{
has_pci_info = false;
has_acpi_build = false;
-smbios_type1_defaults = false;
+smbios_defaults = false;
x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
enable_compat_apic_id_mode();
pc_init1(args, 1, 0);
@@ -366,7 +366,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
{
has_pci_info = false;
has_acpi_build = false;
-smbios_type1_defaults = false;
+smbios_defaults = false;
if (!args-cpu_model) {
args-cpu_model = 486;
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1622700..1ede53c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,7 +50,7 @@
static bool has_pci_info;
static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
* host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte
* pages in the host.
@@ -130,10 +130,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
guest_info-isapc_ram_fw = false;
guest_info-has_acpi_build = has_acpi_build;
-if (smbios_type1_defaults) {
+if (smbios_defaults) {
/* These values are guest ABI, do not change */
-smbios_set_type1_defaults(QEMU, Standard PC (Q35 + ICH9, 2009),
- args-machine-name);
+smbios_set_defaults(QEMU, Standard PC (Q35 + ICH9, 2009),
+args-machine-name);
}
/* allocate ram and load rom/bios */
@@ -247,7 +247,7 @@ static void pc_compat_2_0(QEMUMachineInitArgs *args)
static void pc_compat_1_7(QEMUMachineInitArgs *args)
{
pc_compat_2_0(args);
-smbios_type1_defaults = false;
+smbios_defaults = false;
gigabyte_align = false;
option_rom_has_mr = true;
x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e8f41ad..e734d4c 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -21,9 +21,8 @@
#include hw/i386/smbios.h
#include hw/loader.h
-/*
- * Structures shared with the BIOS
- */
+
+/* legacy structures and constants for = 2.0 machines */
struct smbios_header {
uint16_t length;
uint8_t type;
@@ -46,6 +45,9 @@ struct smbios_table {
static
[Qemu-devel] [QEMU v8 PATCH 6/7] SMBIOS: Use bitmaps to prevent incompatible comand line options
Replace existing smbios_check_collision() functionality with
a pair of bitmaps: have_binfile_bitmap and have_fields_bitmap.
Bits corresponding to each smbios type are set by smbios_entry_add(),
which also uses the bitmaps to ensure that binary blobs and field
values are never accepted for the same type.
These bitmaps will also be used in the future to decide whether
or not to build a full table for a given smbios type.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/smbios.c | 50 +++-
include/hw/i386/smbios.h | 2 ++
2 files changed, 22 insertions(+), 30 deletions(-)
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 9f83bfb..6bbfd15 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -51,11 +51,8 @@ static size_t smbios_entries_len;
static int smbios_type4_count = 0;
static bool smbios_immutable;
-static struct {
-bool seen;
-int headertype;
-Location loc;
-} first_opt[2];
+static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
+static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
static struct {
const char *vendor, *version, *date;
@@ -166,29 +163,6 @@ static void smbios_validate_table(void)
}
}
-/*
- * To avoid unresolvable overlaps in data, don't allow both
- * tables and fields for the same smbios type.
- */
-static void smbios_check_collision(int type, int entry)
-{
-if (type ARRAY_SIZE(first_opt)) {
-if (first_opt[type].seen) {
-if (first_opt[type].headertype != entry) {
-error_report(Can't mix file= and type= for same type);
-loc_push_restore(first_opt[type].loc);
-error_report(This is the conflicting setting);
-loc_pop(first_opt[type].loc);
-exit(1);
-}
-} else {
-first_opt[type].seen = true;
-first_opt[type].headertype = entry;
-loc_save(first_opt[type].loc);
-}
-}
-}
-
/* legacy setup functions for = 2.0 machines */
static void smbios_add_field(int type, int offset, const void *data, size_t
len)
@@ -337,7 +311,14 @@ void smbios_entry_add(QemuOpts *opts)
}
header = (struct smbios_structure_header *)(table-data);
-smbios_check_collision(header-type, SMBIOS_TABLE_ENTRY);
+
+if (test_bit(header-type, have_fields_bitmap)) {
+error_report(can't load type %d struct, fields already
specified!,
+ header-type);
+exit(1);
+}
+set_bit(header-type, have_binfile_bitmap);
+
if (header-type == 4) {
smbios_type4_count++;
}
@@ -352,7 +333,16 @@ void smbios_entry_add(QemuOpts *opts)
if (val) {
unsigned long type = strtoul(val, NULL, 0);
-smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+if (type SMBIOS_MAX_TYPE) {
+error_report(out of range!);
+exit(1);
+}
+
+if (test_bit(type, have_binfile_bitmap)) {
+error_report(can't add fields, binary file already loaded!);
+exit(1);
+}
+set_bit(type, have_fields_bitmap);
switch (type) {
case 0:
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 777e025..3a9361d 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -15,6 +15,8 @@
#include qemu/option.h
+#define SMBIOS_MAX_TYPE 127
+
void smbios_entry_add(QemuOpts *opts);
void smbios_set_defaults(const char *manufacturer, const char *product,
const char *version);
--
1.9.0
[Qemu-devel] [QEMU v8 PATCH 4/7] SMBIOS: Update header file definitions
Add definitions for smbios entry point (anchor), and for type 2
(base board) structure which is required by some versions of OS X.
Remove definition for type 20 (memory device mapped address)
structure, which is no longer required as of smbios spec v2.5.
Update all other structure definitions to bring them into
compliance with smbios spec v2.8.
This patch contains no functional changes.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
include/hw/i386/smbios.h | 88
1 file changed, 66 insertions(+), 22 deletions(-)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index f808199..777e025 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -24,6 +24,26 @@ uint8_t *smbios_get_table_legacy(size_t *length);
* SMBIOS spec defined tables
*/
+/* SMBIOS entry point (anchor).
+ * BIOS must place this at a 16-bit-aligned address between 0xf and
0xf.
+ */
+struct smbios_entry_point {
+uint8_t anchor_string[4];
+uint8_t checksum;
+uint8_t length;
+uint8_t smbios_major_version;
+uint8_t smbios_minor_version;
+uint16_t max_structure_size;
+uint8_t entry_point_revision;
+uint8_t formatted_area[5];
+uint8_t intermediate_anchor_string[5];
+uint8_t intermediate_checksum;
+uint16_t structure_table_length;
+uint32_t structure_table_address;
+uint16_t number_of_structures;
+uint8_t smbios_bcd_revision;
+} QEMU_PACKED;
+
/* This goes at the beginning of every SMBIOS structure. */
struct smbios_structure_header {
uint8_t type;
@@ -60,7 +80,23 @@ struct smbios_type_1 {
uint8_t family_str;
} QEMU_PACKED;
-/* SMBIOS type 3 - System Enclosure (v2.3) */
+/* SMBIOS type 2 - Base Board */
+struct smbios_type_2 {
+struct smbios_structure_header header;
+uint8_t manufacturer_str;
+uint8_t product_str;
+uint8_t version_str;
+uint8_t serial_number_str;
+uint8_t asset_tag_number_str;
+uint8_t feature_flags;
+uint8_t location_str;
+uint16_t chassis_handle;
+uint8_t board_type;
+uint8_t contained_element_count;
+/* contained elements follow */
+} QEMU_PACKED;
+
+/* SMBIOS type 3 - System Enclosure (v2.7) */
struct smbios_type_3 {
struct smbios_structure_header header;
uint8_t manufacturer_str;
@@ -76,10 +112,11 @@ struct smbios_type_3 {
uint8_t height;
uint8_t number_of_power_cords;
uint8_t contained_element_count;
-// contained elements follow
+uint8_t sku_number_str;
+/* contained elements follow */
} QEMU_PACKED;
-/* SMBIOS type 4 - Processor Information (v2.0) */
+/* SMBIOS type 4 - Processor Information (v2.6) */
struct smbios_type_4 {
struct smbios_structure_header header;
uint8_t socket_designation_str;
@@ -97,11 +134,17 @@ struct smbios_type_4 {
uint16_t l1_cache_handle;
uint16_t l2_cache_handle;
uint16_t l3_cache_handle;
+uint8_t serial_number_str;
+uint8_t asset_tag_number_str;
+uint8_t part_number_str;
+uint8_t core_count;
+uint8_t core_enabled;
+uint8_t thread_count;
+uint16_t processor_characteristics;
+uint16_t processor_family2;
} QEMU_PACKED;
-/* SMBIOS type 16 - Physical Memory Array
- * Associated with one type 17 (Memory Device).
- */
+/* SMBIOS type 16 - Physical Memory Array (v2.7) */
struct smbios_type_16 {
struct smbios_structure_header header;
uint8_t location;
@@ -110,10 +153,10 @@ struct smbios_type_16 {
uint32_t maximum_capacity;
uint16_t memory_error_information_handle;
uint16_t number_of_memory_devices;
+uint64_t extended_maximum_capacity;
} QEMU_PACKED;
-/* SMBIOS type 17 - Memory Device
- * Associated with one type 19
- */
+
+/* SMBIOS type 17 - Memory Device (v2.8) */
struct smbios_type_17 {
struct smbios_structure_header header;
uint16_t physical_memory_array_handle;
@@ -127,27 +170,28 @@ struct smbios_type_17 {
uint8_t bank_locator_str;
uint8_t memory_type;
uint16_t type_detail;
+uint16_t speed;
+uint8_t manufacturer_str;
+uint8_t serial_number_str;
+uint8_t asset_tag_number_str;
+uint8_t part_number_str;
+uint8_t attributes;
+uint32_t extended_size;
+uint32_t configured_clock_speed;
+uint32_t minimum_voltage;
+uint32_t maximum_voltage;
+uint32_t configured_voltage;
} QEMU_PACKED;
-/* SMBIOS type 19 - Memory Array Mapped Address */
+/* SMBIOS type 19 - Memory Array Mapped Address (v2.7) */
struct smbios_type_19 {
struct smbios_structure_header header;
uint32_t starting_address;
uint32_t ending_address;
uint16_t memory_array_handle;
uint8_t partition_width;
-} QEMU_PACKED;
-
-/* SMBIOS type 20 - Memory Device Mapped Address */
-struct smbios_type_20 {
-struct smbios_structure_header header;
-uint32_t starting_address;
-uint32_t ending_address;
-uint16_t memory_device_handle;
-uint16_t memory_array_mapped_address_handle;
-uint8_t
[Qemu-devel] [QEMU v8 PATCH 2/7] PC: Add machine version 2.1 for piix and q35
Add v2.1 machine types and (empty) compat functions for v2.0,
to both piix and q35. For now, the new machines will behave
identically to their 2.0 counterparts.
Signed-off-by: Gabriel Somlo so...@cmu.edu
---
hw/i386/pc_piix.c | 27 ---
hw/i386/pc_q35.c | 25 +++--
2 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7930a26..f13d0ac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -262,8 +262,13 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
pc_init1(args, 1, 1);
}
+static void pc_compat_2_0(QEMUMachineInitArgs *args)
+{
+}
+
static void pc_compat_1_7(QEMUMachineInitArgs *args)
{
+pc_compat_2_0(args);
smbios_type1_defaults = false;
gigabyte_align = false;
option_rom_has_mr = true;
@@ -303,6 +308,12 @@ static void pc_compat_1_2(QEMUMachineInitArgs *args)
x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
}
+static void pc_init_pci_2_0(QEMUMachineInitArgs *args)
+{
+pc_compat_2_0(args);
+pc_init_pci(args);
+}
+
static void pc_init_pci_1_7(QEMUMachineInitArgs *args)
{
pc_compat_1_7(args);
@@ -387,14 +398,23 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
PC_I440FX_MACHINE_OPTIONS, \
.default_machine_opts = firmware=bios-256k.bin
-static QEMUMachine pc_i440fx_machine_v2_0 = {
-PC_I440FX_2_0_MACHINE_OPTIONS,
-.name = pc-i440fx-2.0,
+#define PC_I440FX_2_1_MACHINE_OPTIONS \
+PC_I440FX_2_0_MACHINE_OPTIONS
+
+static QEMUMachine pc_i440fx_machine_v2_1 = {
+PC_I440FX_2_1_MACHINE_OPTIONS,
+.name = pc-i440fx-2.1,
.alias = pc,
.init = pc_init_pci,
.is_default = 1,
};
+static QEMUMachine pc_i440fx_machine_v2_0 = {
+PC_I440FX_2_0_MACHINE_OPTIONS,
+.name = pc-i440fx-2.0,
+.init = pc_init_pci_2_0,
+};
+
#define PC_I440FX_1_7_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
static QEMUMachine pc_i440fx_machine_v1_7 = {
@@ -817,6 +837,7 @@ static QEMUMachine xenfv_machine = {
static void pc_machine_init(void)
{
+qemu_register_machine(pc_i440fx_machine_v2_1);
qemu_register_machine(pc_i440fx_machine_v2_0);
qemu_register_machine(pc_i440fx_machine_v1_7);
qemu_register_machine(pc_i440fx_machine_v1_6);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c844dc2..1622700 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -240,8 +240,13 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
}
}
+static void pc_compat_2_0(QEMUMachineInitArgs *args)
+{
+}
+
static void pc_compat_1_7(QEMUMachineInitArgs *args)
{
+pc_compat_2_0(args);
smbios_type1_defaults = false;
gigabyte_align = false;
option_rom_has_mr = true;
@@ -268,6 +273,12 @@ static void pc_compat_1_4(QEMUMachineInitArgs *args)
x86_cpu_compat_set_features(Westmere, FEAT_1_ECX, 0,
CPUID_EXT_PCLMULQDQ);
}
+static void pc_q35_init_2_0(QEMUMachineInitArgs *args)
+{
+pc_compat_2_0(args);
+pc_q35_init(args);
+}
+
static void pc_q35_init_1_7(QEMUMachineInitArgs *args)
{
pc_compat_1_7(args);
@@ -301,11 +312,20 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
PC_Q35_MACHINE_OPTIONS, \
.default_machine_opts = firmware=bios-256k.bin
+#define PC_Q35_2_1_MACHINE_OPTIONS \
+PC_Q35_2_0_MACHINE_OPTIONS
+
+static QEMUMachine pc_q35_machine_v2_1 = {
+PC_Q35_2_1_MACHINE_OPTIONS,
+.name = pc-q35-2.1,
+.alias = q35,
+.init = pc_q35_init,
+};
+
static QEMUMachine pc_q35_machine_v2_0 = {
PC_Q35_2_0_MACHINE_OPTIONS,
.name = pc-q35-2.0,
-.alias = q35,
-.init = pc_q35_init,
+.init = pc_q35_init_2_0,
};
#define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
@@ -358,6 +378,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
static void pc_q35_machine_init(void)
{
+qemu_register_machine(pc_q35_machine_v2_1);
qemu_register_machine(pc_q35_machine_v2_0);
qemu_register_machine(pc_q35_machine_v1_7);
qemu_register_machine(pc_q35_machine_v1_6);
--
1.9.0
Re: [Qemu-devel] [PATCH 00/13] __{get, put}_user return value cleanup
On Wed, Apr 23, 2014 at 02:22:32PM +0100, Peter Maydell wrote:
On 23 April 2014 14:11, riku.voi...@linaro.org wrote:
From: Riku Voipio riku.voi...@linaro.org
This series is primarily motivated to have a gcc-4.9 buildfix:
linux-user/syscall.c: In function ‘host_to_target_stat64’:
linux-user/qemu.h:301:19: error: right-hand operand of comma expression has
no effect [-Werror=unused-value]
((hptr), (x)), 0)
removing the unused 0 moves the bar:
linux-user/main.c: In function ‘arm_kernel_cmpxchg64_helper’:
linux-user/qemu.h:330:15: error: void value not ignored as it ought to be
__ret = __put_user((x), __hptr);\
And after fixing that, we see there is a lot of reading the return
value of __put_user and __get_user in signal.c - apparently without
much of consistency as other functions do check and others don't...
I'm not 100% sure that simply removing the checks is right way
and signal.c is quite a mess, so push this set early for comments
before spending more time on this approach.
So the reason for this inconsistency is that QEMU's approach
to checking that memory is accessible differs from the Linux kernel.
In Linux, the check effectively happens on each get/put access
(for free courtesy of the MMU), so the __get/__put_user accessors
can each individually succeed or fail, and the return value must
always be checked.
Right. I guess this will be good to put the commit message
or code comment in the 12/12 patch.
In QEMU, we do an access check for an entire region via
lock_user (or lock_user_struct) and catch the failure case there.
So our __get/__put_user accessors can never fail. However we
retained the return value because a lot of the code in signal.c
is pretty much copy-and-pasted from the kernel sources with
minor tweaks, and so it includes the gather up and check
return values at each step code.
Ok, this explains. Thanks for quick answer!
Generally in code review or if I've been tidying up or fixing the
signal handling code for some other reason I've removed the
pointless accumulation of return values. But a bunch of our
less-maintained architectures still have it.
signal.c: remove __get/__put_user return value reading
signal.c setup_frame/x86: __put_user cleanup
signal.c: remove return value from copy_siginfo_to_user
signal.c: remove return value from setup_sigcontext
signal.c: remove return value from restore_sigcontext
RFC comment out restore_fpu_state (sparc)
do_sigaltstack: remove __get_user value check
do_sigreturn - remove __get_user checks
signal.c: setup_frame remove __put_user checks
remove __put/get error checks from ppc {save,restore}_user_regs
sparc64_set_context: remove __get_user checks
remove __get_user return check from PPC do_setcontext
fix gcc-4.9 compiler error on __{get,put]}_user
Given that signal.c is a mix of code for all our supported
architectures, it would be nice if these patch summary lines
were a bit more consistent:
linux-user/signal.c: Generic change affecting all archs
linux-user/signal.c: PPC: Change affecting only PPC
Ok, no problem. As said, I wanted to see that the principle,
patch split etc is acceptable before going to detailed polishing
like summary lines.
and so on.
I should probably have another go at a tidyup I was
looking at ages ago which splits linux-user/signal.c
into linux-user/$arch/signal.c...
Yes, it was in my mind when repeatedly compiling all archs
for any single change in signal.c...
Riku
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi,
/* the new version id for this patch */
if (version_id == 4) {
return 0;
}
I don't think we need a new version.
OK.
/* set the useful data buffer queue size, PS2_QUEUE_SIZE */
size = MIN(q-count, PS2_QUEUE_SIZE);
I'd rather do size = q-count PS2_QUEUE_SIZE ? 0 : q-count;
i.e. if it doesn't fit better throw away everything, which reduces the
chance that we'll deliver a incomplete message to the guest.
Sounds good, Thanks, Gerd.
for (i = 0; i size; i++) {
/* move the queue elements to the start of data array */
q-data[i] = q-data[q-rptr];
if (++q-rptr == 256) {
q-rptr = 0;
}
}
That fails for the wraparound (rptr wptr) case.
Yep, it should use a temporary data array to transfer, which I have written
in the previous email.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel
Hi, Completely separate question: Have you figured what the root cause for the bug is? While wading through the code I've figured the queue size isn't (directly) exposed to the guest. The process correspond with the above linux kernel code, i8042_flush(void). So linux tries to flush the buffer, then is confused in case it can read more than 16 bytes (which is the buffer size of real hardware) without seeing the status register signaling that the buffer is empty. So the qemu internal buffer size is indirectly visible to the guest. Hmm. We could try sending the data to the guest in chunks, i.e. signal buffer empty, then set up a (short) timer when we'll go make it appear to the guest as if new data arrived (which in reality just comes from out large buffer). Not that easy to do, as we should better avoid splitting messages in the middle. but for mouse events we probably want keep some more space to buffer events ... Maybe you are right, and I worry about it too. At present, but I haven't feel uncomfortable by VNC client then before. Do you have a usb-tablet attached, for more comfortable absolute coordinates? If so, then the ps/2 mouse will not be used to deliver events to the guest. And as this is a pretty common setup we maybe should not worry too much about the ps/2 mouse queue depth ... cheers, Gerd
