Re: [Qemu-devel] [PATCH V17 1/6] docs: document for add-cow file format
On Mon, Dec 10, 2012 at 11:39 PM, Kevin Wolf wrote:
> Am 06.12.2012 07:51, schrieb Dong Xu Wang:
>> Document for add-cow format, the usage and spec of add-cow are introduced.
>>
>> Signed-off-by: Dong Xu Wang
>> ---
>> docs/specs/add-cow.txt | 154
>>
>> 1 files changed, 154 insertions(+), 0 deletions(-)
>> create mode 100644 docs/specs/add-cow.txt
>>
>> diff --git a/docs/specs/add-cow.txt b/docs/specs/add-cow.txt
>> new file mode 100644
>> index 000..24e9a11
>> --- /dev/null
>> +++ b/docs/specs/add-cow.txt
>> @@ -0,0 +1,154 @@
>> +== General ==
>> +
>> +The raw file format does not support backing files or copy on write feature.
>> +The add-cow image format makes it possible to use backing files with a raw
>> +image by keeping a separate .add-cow metadata file. Once all sectors
>> +have been written into the raw image it is safe to discard the .add-cow
>> +and backing files, then we can use the raw image directly.
>> +
>> +An example usage of add-cow would look like::
>
> Double colon.
Okay.
>
>> +(ubuntu.img is a disk image which has an installed OS.)
>> +1) Create a raw image with the same size of ubuntu.img
>> +qemu-img create -f raw test.raw 8G
>> +2) Create an add-cow image which will store dirty bitmap
>> +qemu-img create -f add-cow test.add-cow \
>> +-o backing_file=ubuntu.img,image_file=test.raw
>> +3) Run qemu with add-cow image
>> +qemu -drive if=virtio,file=test.add-cow
>> +
>> +test.raw may be larger than ubuntu.img, in that case, the size of
>> test.add-cow
>> +will be calculated from the size of test.raw.
>> +
>> +image_fmt can be omitted, in that case image_fmt should be set as "raw".
>
> By "should be set as" you mean "is assumed to be"?
>
Okay. Will fix.
>> +backing_fmt can also be omitted, add-cow should do a probe operation and
>> determine
>
> This line takes more than 80 characters. More follow, I won't comment on
> each.
>
Okay, will fix.
>> +what the backing file's format is.
>> +
>> +=Specification=
>> +
>> +The file format looks like this:
>> +
>> + +---+---+
>> + | Header| COW bitmap |
>> + +---+---+
>> +
>> +All numbers in add-cow are stored in Little Endian byte order.
>> +
>> +== Header ==
>> +
>> +The Header is included in the first bytes:
>> +(HEADER_SIZE is defined in 44-47 bytes.)
>> +Byte0 - 3:magic
>> +add-cow magic string ("ACOW").
>> +
>> +4 - 7:version
>> +Version number (only valid value is 1 now).
>> +
>> +8 - 11:backing file name offset
>> +Offset in the add-cow file at which the backing file
>> +name is stored (NB: The string is not
>> NUL-terminated).
>> +If backing file name does NOT exist, this field
>> will be
>> +0. Must be between 80 and [HEADER_SIZE - 2](a file
>> name
>> +must be at least 1 byte).
>> +
>> +12 - 15:backing file name size
>> +Length of the backing file name in bytes. It will
>> be 0
>> +if the backing file name offset is 0. If backing
>> file
>> +name offset is non-zero, then it must be non-zero.
>> Must
>> +be less than [HEADER_SIZE - 80] to fit in the
>> reserved
>> +part of the header. Backing file name offset + size
>> +must be no more than HEADER_SIZE.
>> +
>> +16 - 19:image file name offset
>> +Offset in the add-cow file at which the image file
>> name
>> +is stored (NB: The string is not NUL-terminated). It
>> +must be between 80 and [HEADER_SIZE - 2]. Image file
>> +name size + offset must be no more than HEADER_SIZE.
>> +
>> +20 - 23:image file name size
>> +Length of the image file name in bytes.
>> +Must be less than [HEADER_SIZE - 80] to fit in the
>> reserved
>> +part of the header.
>> +
>> +24 - 27:cluster bits
>> +Number of bits that are used for addressing an
>> offset
>> +within a cluster (1 << cluster_bits is the cluster
>> size).
>> +Must not be less than 9 (i.e. 512 byte clusters).
>> +
>> +Note: qemu as of today has an implementation limit
>> of 2 MB
>> +as the maximum cluster size and won't be able to
>> open images
>> +with larger cluster sizes.
>> +
>> +28 - 35:features
>> +Bitmask of features. If a feature bit
Re: [Qemu-devel] [PATCH V17 6/6] qemu-iotests: add add-cow iotests support.
On Tue, Dec 11, 2012 at 1:15 AM, Kevin Wolf wrote: > Am 06.12.2012 07:51, schrieb Dong Xu Wang: >> This patch will use qemu-iotests to test add-cow file format. >> >> Signed-off-by: Dong Xu Wang >> --- >> tests/qemu-iotests/017 |2 +- >> tests/qemu-iotests/020 |2 +- >> tests/qemu-iotests/common|6 ++ >> tests/qemu-iotests/common.rc | 15 ++- >> 4 files changed, 22 insertions(+), 3 deletions(-) > > I think at least tests 037 and 038 would work as well. > Okay. > Kevin >
Re: [Qemu-devel] [Qemu-ppc] [PATCH 16/19] openpic: add Shared MSI support
On 11.12.2012, at 01:36, Scott Wood wrote: > On 12/08/2012 07:44:39 AM, Alexander Graf wrote: >> The OpenPIC allows MSI access through shared MSI registers. Implement >> them for the MPC8544 MPIC, so we can support MSIs. >> Signed-off-by: Alexander Graf >> --- >> hw/openpic.c | 150 >> ++ >> 1 files changed, 130 insertions(+), 20 deletions(-) >> diff --git a/hw/openpic.c b/hw/openpic.c >> index f2f152f..f71d668 100644 >> --- a/hw/openpic.c >> +++ b/hw/openpic.c >> @@ -38,6 +38,7 @@ >> #include "pci.h" >> #include "openpic.h" >> #include "sysbus.h" >> +#include "msi.h" >> //#define DEBUG_OPENPIC >> @@ -52,6 +53,7 @@ >> #define MAX_TMR 4 >> #define VECTOR_BITS 8 >> #define MAX_IPI 4 >> +#define MAX_MSI 8 >> #define VID 0x03 /* MPIC version ID */ >> /* OpenPIC capability flags */ >> @@ -62,6 +64,8 @@ >> #define OPENPIC_GLB_REG_SIZE 0x10F0 >> #define OPENPIC_TMR_REG_START0x10F0 >> #define OPENPIC_TMR_REG_SIZE 0x220 >> +#define OPENPIC_MSI_REG_START0x1600 >> +#define OPENPIC_MSI_REG_SIZE 0x200 >> #define OPENPIC_SRC_REG_START0x1 >> #define OPENPIC_SRC_REG_SIZE (MAX_IRQ * 0x20) >> #define OPENPIC_CPU_REG_START0x2 >> @@ -126,6 +130,12 @@ >> #define IDR_P1_SHIFT 1 >> #define IDR_P0_SHIFT 0 >> +#define MSIIR_OFFSET 0x140 >> +#define MSIIR_SRS_SHIFT29 >> +#define MSIIR_SRS_MASK (0x7 << MSIIR_SRS_SHIFT) >> +#define MSIIR_IBS_SHIFT24 >> +#define MSIIR_IBS_MASK (0x1f << MSIIR_IBS_SHIFT) > > FWIW, if you want to model newer MPICs such as on p4080, they have multiple > banks of MSIs, so you may not want to hardcode one bank. The OpenPIC code was suffering a lot from attempts to generalize different implementations without implementing them. If we want to add support for p4080 MPICs later, we add a new model to the emulation and make the nr of msi banks a parameter, like the patch set does for all the other raven/mpc8544 differences. That way we don't get into the current mess of a halfway accurate emulation unless we really want it. Alex
Re: [Qemu-devel] [PATCH V17 5/6] add-cow file format core code.
On Tue, Dec 11, 2012 at 1:54 AM, Kevin Wolf wrote:
> Am 06.12.2012 07:51, schrieb Dong Xu Wang:
>> add-cow file format core code. It use block-cache.c as cache code.
>> It lacks of snapshot_blkdev support.
>>
>> v16->v17:
>> 1) Use stringify.
>>
>> v15->v16:
>> 1) Judge if opts is null in add_cow_create function.
>>
>> Signed-off-by: Dong Xu Wang
>> ---
>> block/Makefile.objs |1 +
>> block/add-cow.c | 690
>> +++
>> block/add-cow.h | 83 ++
>> block/block-cache.c |4 +
>> block/block-cache.h |1 +
>> block_int.h |2 +
>> 6 files changed, 781 insertions(+), 0 deletions(-)
>> create mode 100644 block/add-cow.c
>> create mode 100644 block/add-cow.h
>>
>> diff --git a/block/Makefile.objs b/block/Makefile.objs
>> index d23c250..f0ff067 100644
>> --- a/block/Makefile.objs
>> +++ b/block/Makefile.objs
>> @@ -1,5 +1,6 @@
>> block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o
>> vvfat.o
>> block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>> +block-obj-y += add-cow.o
>> block-obj-y += block-cache.o
>> block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>> block-obj-y += qed-check.o
>> diff --git a/block/add-cow.c b/block/add-cow.c
>> new file mode 100644
>> index 000..8cd7305
>> --- /dev/null
>> +++ b/block/add-cow.c
>> @@ -0,0 +1,690 @@
>> +/*
>> + * QEMU ADD-COW Disk Format
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + * Dong Xu Wang
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or
>> later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "block_int.h"
>> +#include "module.h"
>> +#include "add-cow.h"
>
> You have only this source file, so there's no need to have a header.
>
Okay.
>> +s->image_hd = bdrv_new("");
>> +if (path_has_protocol(tmp_name)) {
>> +pstrcpy(image_filename, sizeof(image_filename), tmp_name);
>> +} else {
>> +path_combine(image_filename, sizeof(image_filename),
>> + bs->filename, tmp_name);
>> +}
>> +bdrv_get_full_backing_filename(bs, backing_filename,
>> + sizeof(backing_filename));
>> +if (!strcmp(backing_filename, image_filename)) {
>> +fprintf(stderr, "Error: backing file and image file are same: %s\n",
>> +backing_filename);
>
> error_report().
>
> Also, s->image_hd is leaked.
>
Okay.
>> +ret = EINVAL;
>> +goto fail;
>> +}
>> +ret = bdrv_open(s->image_hd, image_filename, flags,
>> +bdrv_find_format(s->header.image_fmt));
>> +if (ret < 0) {
>> +bdrv_delete(s->image_hd);
>> +goto fail;
>> +}
>> +
>> +bs->total_sectors = bdrv_getlength(s->image_hd) / BDRV_SECTOR_SIZE;
>> +s->cluster_size = 1 << s->header.cluster_bits;
>> +sector_per_byte = (s->cluster_size / BDRV_SECTOR_SIZE) * 8;
>> +s->bitmap_size =
>> +(bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
>> +s->bitmap_cache = block_cache_create(bs, ADD_COW_CACHE_SIZE,
>> + ADD_COW_CACHE_ENTRY_SIZE,
>> + BLOCK_TABLE_BITMAP);
>> +qemu_co_mutex_init(&s->lock);
>> +return 0;
>> +fail:
>> +if (s->bitmap_cache) {
>
> This is never true.
>
Okay.
>> +block_cache_destroy(bs, s->bitmap_cache);
>> +}
>> +return ret;
>> +}
>
> header.header_size isn't checked to be 4096 as required in the spec.
>
Okay, will check in newer version.
>> +
>> +static void add_cow_close(BlockDriverState *bs)
>> +{
>> +BDRVAddCowState *s = bs->opaque;
>> +if (s->bitmap_cache) {
>> +block_cache_destroy(bs, s->bitmap_cache);
>> +}
>> +bdrv_delete(s->image_hd);
>> +}
>> +
>> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
>> +{
>> +BDRVAddCowState *s = bs->opaque;
>> +BlockCache *c = s->bitmap_cache;
>> +int64_t cluster_num = sector_num / (s->cluster_size / BDRV_SECTOR_SIZE);
>
> s->cluster_size / BDRV_SECTOR_SIZE is so common that it could be worth
> having another field s->cluster_secs like in qcow2.
>
Okay. will add cluster_secs field.
>> +uint8_t *table = NULL;
>> +bool val = false;
>> +int ret;
>> +
>> +uint64_t offset = s->header.header_size
>> ++ (offset_in_bitmap(sector_num, s->cluster_size / BDRV_SECTOR_SIZE)
>> +& (~(c->cluster_size - 1)));
>
> It becomes very obvious here that your choice of the image format layout
> means that all bitmap blocks are not aligned to cluster boundaries, but
> offset by header_size.
>
> This isn't necessarily a big problem, but I'm not sure if it's a good
> idea either.
>
>> +ret = block_cache_get(bs, s->bitmap_cache, offset, (void **)&table);
>> +if (ret < 0) {
>> +return ret;
>> +}
>> +
>>
Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme
On 11.12.2012, at 00:34, Scott Wood wrote:
> On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
>> MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
>> because QEMU takes the sparseness of the IRQ number space into account.
>> Remove that cleverness and instead assume a flat number space. This makes
>> the code easier to understand, because we are actually aligned with Linux
>> on the view of our worlds.
>> Signed-off-by: Alexander Graf
>> ---
>> hw/openpic.c | 287
>> hw/ppc/e500.c |4 +-
>> 2 files changed, 43 insertions(+), 248 deletions(-)
>
> Thanks!
>
> This also helps accommodate Linux's (and probably other OSes') MPIC
> driver, which on boot will initialize all vectors, whether the actual
> interrupt is valid or not. Currently QEMU rejects those accesses as
> illegal MMIO; we only survive it now because we don't yet have support for
> injecting a machine check on e500.
>
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index b30c853..0d858cc 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -47,7 +47,7 @@
>> #endif
>> #define MAX_CPU15
>> -#define MAX_IRQ 128
>> +#define MAX_IRQ 256
>> #define MAX_TMR 4
>> #define VECTOR_BITS 8
>> #define MAX_IPI 4
>> @@ -82,32 +82,25 @@ enum {
>> #define MPIC_MAX_CPU 1
>> #define MPIC_MAX_EXT 12
>> #define MPIC_MAX_INT 64
>> -#define MPIC_MAX_MSG 4
>> -#define MPIC_MAX_MSI 8
>> -#define MPIC_MAX_TMR MAX_TMR
>> -#define MPIC_MAX_IPI MAX_IPI
>> -#define MPIC_MAX_IRQ (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR +
>> MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
>> +#define MPIC_MAX_IRQ MAX_IRQ
>> /* Interrupt definitions */
>> -#define MPIC_EXT_IRQ 0
>> -#define MPIC_INT_IRQ (MPIC_EXT_IRQ + MPIC_MAX_EXT)
>> -#define MPIC_TMR_IRQ (MPIC_INT_IRQ + MPIC_MAX_INT)
>> -#define MPIC_MSG_IRQ (MPIC_TMR_IRQ + MPIC_MAX_TMR)
>> -#define MPIC_MSI_IRQ (MPIC_MSG_IRQ + MPIC_MAX_MSG)
>> -#define MPIC_IPI_IRQ (MPIC_MSI_IRQ + MPIC_MAX_MSI)
>> +/* IRQs, accessible through the IRQ region */
>> +#define MPIC_EXT_IRQ 0x00
>> +#define MPIC_INT_IRQ 0x10
>> +#define MPIC_MSG_IRQ 0xb0
>> +#define MPIC_MSI_IRQ 0xe0
>
> Where are MPIC_EXT/INT/MSG/MSI_IRQ used now? Note that these are
> specific to Freescale's MPIC.
>
>> +/* These are available through separate regions, but
>> + for simplicity's sake mapped into the same number space */
>> +#define MPIC_TMR_IRQ 0xf3
>> +#define MPIC_IPI_IRQ 0xfb
>
> Please don't do this, or at least choose different numbers. 0xf3 is a
> valid MSI on p4080 (not to mention T4240 which goes beyond 256).
Ah, that's where I was wondering myself too. I copied the above logic from
Linux, which maps tmr and ipi to max_irq-x. But I agree that it sounds off.
I'll rework this one again to distinguish between src and internal interrupt
lines.
Alex
Re: [Qemu-devel] [Qemu-ppc] [PATCH 10/19] openpic: remove unused type variable
On 11.12.2012, at 00:42, Scott Wood wrote:
> On 12/08/2012 07:44:33 AM, Alexander Graf wrote:
>> The openpic source irqs are carrying around a type indicator that
>> is never accessed by anything. Remove it.
>> Signed-off-by: Alexander Graf
>> ---
>> hw/openpic.c | 27 ++-
>> 1 files changed, 2 insertions(+), 25 deletions(-)
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index e4ef23d..d252b2b 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -167,13 +167,6 @@ static uint32_t openpic_cpu_read_internal(void *opaque,
>> hwaddr addr,
>> static void openpic_cpu_write_internal(void *opaque, hwaddr addr,
>>uint32_t val, int idx);
>> -enum {
>> -IRQ_EXTERNAL = 0x01,
>> -IRQ_INTERNAL = 0x02,
>> -IRQ_TIMER= 0x04,
>> -IRQ_SPECIAL = 0x08,
>> -};
>
> We may want to distinguish based on something like this in the future -- for
> example, internal interrupts on FSL MPIC don't have a "sense" bit.
I would rather like to give irq lines flags than types then. An FSL MPIC could
set a 'has no sense bit' flag in the irq line which means the sense bit is
always masked out when written to.
Alex
Re: [Qemu-devel] [PATCH 14/19] openpic: convert to qdev
On 11.12.2012, at 00:47, Scott Wood wrote: > On 12/08/2012 07:44:37 AM, Alexander Graf wrote: >> This patch converts the OpenPIC device to qdev. Along the way it >> renames the "openpic" target to "raven" and the "mpic" target to >> "mpc8544", to better reflect the actual models they implement. >> This way we have a generic OpenPIC device now that can handle >> different flavors of the OpenPIC specification. > > I'd rather not see the expansion of "mpc8544" hardcoding, especially since > it's not really modelling an MPIC as found in an mpc8544, but rather > half-implementing a generic Freescale MPIC. That's what we've been doing wrong all the time now. We shouldn't implement a half-generic fsl mpic, because we will never get to a point where we're accurate enough or flexible enough for both definitions. If we want a pv style generic mpic (for -M e500), let's add such an mpic to the model list and make that one be really generic. But the MPIC in -M mpc8544ds should behave exactly like an mpc8544 mpic. Whenever we fail to do so, we better fix the emulation to be accurate ;) > Can we just call it a Freescale MPIC, and then work on parameterizing it > properly (starting with MPIC version)? We can add a generic fsl mpic if you like. We can also extract commonalities between the generic fsl mpic and the mpc8544 mpic into a function inside openpic.c. The MPIC version is a parameter after this patch set, but it's an internal parameter. My first version of the qdevification was setting all the openpic parameters from the machine file, so that you could assemble whatever openpic you like by setting the currect parameters. That approach is overkill though. We are working in an open source world, where you can always just modify openpic.c, addd your model and be happy :) Thanks a lot for the review btw! Alex
Re: [Qemu-devel] [PATCH V17 4/6] rename qcow2-cache.c to block-cache.c
On Tue, Dec 11, 2012 at 1:11 AM, Kevin Wolf wrote:
> Am 06.12.2012 07:51, schrieb Dong Xu Wang:
>> We will re-use qcow2-cache as block layer common cache code,
>> so change its name and made some changes, define a struct named
>> BlockTableType, pass BlockTableType and table size parameters to
>> block cache initialization function.
>>
>> Signed-off-by: Dong Xu Wang
>> ---
>> block/Makefile.objs|3 +-
>> block/block-cache.c| 317
>> +++
>> block/block-cache.h| 76 +++
>> block/qcow2-cache.c| 323
>>
>> block/qcow2-cluster.c | 53
>> block/qcow2-refcount.c | 66 ++-
>> block/qcow2.c | 21 ++--
>> block/qcow2.h | 24 +---
>> trace-events | 13 +-
>> 9 files changed, 481 insertions(+), 415 deletions(-)
>> create mode 100644 block/block-cache.c
>> create mode 100644 block/block-cache.h
>> delete mode 100644 block/qcow2-cache.c
>>
>> diff --git a/block/Makefile.objs b/block/Makefile.objs
>> index 7f01510..d23c250 100644
>> --- a/block/Makefile.objs
>> +++ b/block/Makefile.objs
>> @@ -1,5 +1,6 @@
>> block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o
>> vvfat.o
>> -block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>> qcow2-cache.o
>> +block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>> +block-obj-y += block-cache.o
>> block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>> block-obj-y += qed-check.o
>> block-obj-y += parallels.o blkdebug.o blkverify.o
>> diff --git a/block/block-cache.c b/block/block-cache.c
>> new file mode 100644
>> index 000..bf5c57c
>> --- /dev/null
>> +++ b/block/block-cache.c
>> @@ -0,0 +1,317 @@
>> +/*
>> + * QEMU Block Layer Cache
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + * Dong Xu Wang
>> + *
>> + * This file is based on qcow2-cache.c, see its copyrights below:
>> + *
>> + * L2/refcount table cache for the QCOW2 format
>> + *
>> + * Copyright (c) 2010 Kevin Wolf
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> copy
>> + * of this software and associated documentation files (the "Software"), to
>> deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "block_int.h"
>> +#include "qemu-common.h"
>> +#include "trace.h"
>> +#include "block-cache.h"
>> +
>> +BlockCache *block_cache_create(BlockDriverState *bs, int num_tables,
>> + size_t cluster_size, BlockTableType type)
>
> cluster_size should probably be called table_size. It just happens that
> qcow2 tables are always one cluster, but it may be different for other
> formats using this implementation in the future.
>
Okay.
>> +int block_cache_put(BlockDriverState *bs, BlockCache *c, void **table)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < c->size; i++) {
>> +if (c->entries[i].table == *table) {
>> +goto found;
>> +}
>> +}
>> +return -ENOENT;
>> +
>> +found:
>> +c->entries[i].ref--;
>> +assert(c->entries[i].ref >= 0);
>> +*table = NULL;
>> +return 0;
>> +}
>
> Why did you swap the assert() and *table = NULL?
>
I will use original code here. I have no reason to swap them..
>> diff --git a/block/block-cache.h b/block/block-cache.h
>> new file mode 100644
>> index 000..4efa06e
>> --- /dev/null
>> +++ b/block/block-cache.h
>> @@ -0,0 +1,76 @@
>> +/*
>> + * QEMU Block Layer Cache
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + * Dong Xu Wang
>> + *
>> + * This file is based on qcow2-cache.c, see its copyrights below:
>> + *
>> + * L2/refcount table cache for the QCOW2 format
>> + *
>> + * Copyright (c) 2010 Kevin Wolf
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> copy
>> + * of this software and associated documentation files (the "Software"), to
>> deal
>> + * in the Software witho
Re: [Qemu-devel] Network isn't clean when qemu exits
On Tue, Dec 11, 2012 at 01:57:04PM +0800, Amos Kong wrote:
> In vl.c:main(), tap device would be created when net_init_clients() is called.
> "device" option is parsed after calling net_init_clients() by:
> # "qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1)
>
> Qemu will exit if fails to parse device parameters without calling
> net_cleanup().
> I touch a problem, the tap device which is created by qemu-ifup script
> could not be removed by qemu-ifdown script.
>
> I did a quick fix, the tap device should be removed.
>
> But there are many "exit(1)" after calling net_init_clients() in vl.c,
> it's ugly to call net_cleanup() before each exit. Not sure if we have
> other exit(1) in other sub-functions, which is also called after calling
> net_init_clients().
>
> Any comments?
>
> =
> @@ -3882,8 +3889,11 @@ int main(int argc, char **argv, char **envp)
> }
>
> /* init generic devices */
> -if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func,
> NULL, 1) != 0)
> +if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func,
> + NULL, 1) != 0) {
> +net_cleanup();
> exit(1);
> +}
How about qemu_add_exit_notifier() or atexit(3)? Several other places
in QEMU use this to clean up.
Then you can also remove net_cleanup() from the end of vl.c:main() since
it gets called implicitly.
Stefan
Re: [Qemu-devel] [PATCH qom-cpu 3/4] Really finally kill cpudef config section support
于 2012-12-11 7:53, Eduardo Habkost 写道: On Tue, Dec 11, 2012 at 12:12:23AM +0100, Andreas Färber wrote: Am 10.12.2012 19:09, schrieb Eduardo Habkost: On Sun, Dec 09, 2012 at 08:45:52PM +0100, Andreas Färber wrote: Commit 511c68d3af626cb0a39034cb77e7ac64d3a26c0c (finally kill cpudef config section support) removed the cpudef parsing support but left cpudef_* hooks behind. Remove those. The cpudef_* functions have nothing to do with the cpudef config section since QEMU 1.2, it is just about initializing CPU-definition-related data structures, so the patch subject is a bit misleading. My memory tells me they were specifically added for the config file support... git-blame proves me wrong and shows they were added by Johan Cooper and refactored by Blue, explaining his sudden cpudef patch involvement. Sorry. Do you have a proposal for a better text? My reasoning is we should clean up before we forget about it and things stay behind. Something like "kill the cpudef_setup() hook that we don't need anymore [because only x86 uses it, and we can simply use class_init to initialize data structures]" sounds good enough to me. Hi, Eduardo I think we need uninstall rules which should delete the def files, otherwise we met error: qemu-system-x86_64:/usr/local/etc/qemu/target-x86_64.conf:3: There is no option group 'cpudef' -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH V7 00/10] replace QEMUOptionParameter with QemuOpts parser
On Thu, Dec 06, 2012 at 02:47:17PM +0800, Dong Xu Wang wrote: > Patch 1-3 are from Luiz, added Markus's comments, discussion could be found > here: > http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html > Patch 3 was changed according Paolo's comments. > > Patch 4-5: because qemu_opts_create can not fail while id is null, so create > function qemu_opts_create_nofail and use it. > > Patch 6: create function qemu_opt_set_number, like qemu_opt_set_bool. > > Patch 7: add def_value and use it in qemu_opts_print. > > Patch 8: Create functions to pair with QEMUOptionParameter parser. > > Patch 9: Use QemuOpts parser in Block. > > Patch 10: Remove QEMUOptionParameter parser related code. > > v6->v7: > 1) Fix typo: enouth->enough. > 2) use osdep.h:stringify(), not redefining new macro. > 3) preserve TODO comment. > 4) fix typo: BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. > 5) initialize disk_type even when opts is NULL. > > v5->v6: > 1) allocate enough space in append_opts_list function. > 2) judge if opts == NULL in block layer create functions. > 3) use bdrv_create_file(filename, NULL) in qcow_create funtion. > 4) made more readable while using qemu_opt_get_number funtion. > > v4->v5: > 1) Rewrite qemu_opts_create_nofail function based on Peter Maydell's comments. > 2) Use g_strdup_printf in qemu_opt_set_number. > 3) Rewrite qemu_opts_print. > 4) .bdrv_create_options returns pointer directly. Fix a bug about > "encryption". > 5) Check qemu_opt_get_number in raw-posix.c. > > v3->v4: > 1) Rebased to the newest source tree. > 2) Remove redundant "#include "block-cache.h" > 3) Other small changes. > > v2->v3: > 1) rewrite qemu_opt_set_bool and qemu_opt_set_number according Paolo's > coments. > 2) split patches to make review easier. > > v1->v2: > 1) add Luiz's patches. > 2) create qemu_opt_set_number() and qemu_opts_create_nofail() functions. > 3) add QemuOptsList map to drivers. > 4) use original opts parser, not creating new ones. > 5) fix other bugs. > > Dong Xu Wang (10): > qemu-option: opt_set(): split it up into more functions > qemu-option: qemu_opts_validate(): fix duplicated code > qemu-option: qemu_opt_set_bool(): fix code duplication > introduce qemu_opts_create_nofail function > use qemu_opts_create_nofail > create new function: qemu_opt_set_number > add def_print_str and use it in qemu_opts_print. > Create four opts list related functions > Use QemuOpts support in block layer > remove QEMUOptionParameter related functions and struct > > block.c | 91 +- > block.h |5 +- > block/cow.c | 46 +++--- > block/qcow.c | 60 +++--- > block/qcow2.c | 171 +- > block/qed.c | 86 +- > block/raw-posix.c | 65 > block/raw.c | 30 ++-- > block/sheepdog.c | 75 > block/vdi.c | 69 > block/vmdk.c | 74 > block/vpc.c | 67 > block/vvfat.c | 11 +- > block_int.h |6 +- > blockdev.c|2 +- > hw/watchdog.c |2 +- > qemu-config.c |4 +- > qemu-img.c| 63 +++ > qemu-option.c | 512 > ++--- > qemu-option.h | 39 + > qemu-sockets.c| 16 +- > vl.c | 12 +- > 22 files changed, 656 insertions(+), 850 deletions(-) Acked-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH v6 00/12] virtio: virtio-blk data plane
This morning I found the following unstaged change in my working tree,
it should have been part of "[PATCH v6 10/12] virtio-blk: restore
VirtIOBlkConf->config_wce flag":
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c60b89a..32cc910 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -895,6 +895,7 @@ static Property virtio_blk_properties[] = {
#ifdef __linux__
DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
#endif
+DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
Re: [Qemu-devel] [Bug 1087974] Re: [regression] vnc tight png produces garbled output
On Mon, Dec 10, 2012 at 03:54:58PM -, Tim Hardeck wrote: > 47683d669f993308c2b84bed4ce64aafb5d7ced4 is the first bad commit > commit 47683d669f993308c2b84bed4ce64aafb5d7ced4 > Author: Gerd Hoffmann > Date: Thu Oct 11 12:04:33 2012 +0200 > > pixman/vnc: remove rgb_prepare_row* functions > > Let pixman do it instead. > > Signed-off-by: Gerd Hoffmann > > :04 04 653d58e66bf3a2d8240b2f9176979c44ccd720e1 > 6b6e367a8522cb58b42ad8f204387a354d3b3d00 M ui > > > Just reverting this particular commit isn't enough thou but it is connected. This suggests the conversion to pixman has introduced the bug. Tim: can you provide steps to reproduce the bug? Stefan
Re: [Qemu-devel] [PATCH V17 4/6] rename qcow2-cache.c to block-cache.c
Am 11.12.2012 09:25, schrieb Dong Xu Wang:
> On Tue, Dec 11, 2012 at 1:11 AM, Kevin Wolf wrote:
>> Am 06.12.2012 07:51, schrieb Dong Xu Wang:
>>> We will re-use qcow2-cache as block layer common cache code,
>>> so change its name and made some changes, define a struct named
>>> BlockTableType, pass BlockTableType and table size parameters to
>>> block cache initialization function.
>>>
>>> Signed-off-by: Dong Xu Wang
>>> diff --git a/block/block-cache.h b/block/block-cache.h
>>> new file mode 100644
>>> index 000..4efa06e
>>> --- /dev/null
>>> +++ b/block/block-cache.h
>>> @@ -0,0 +1,76 @@
>>> +/*
>>> + * QEMU Block Layer Cache
>>> + *
>>> + * Copyright IBM, Corp. 2012
>>> + *
>>> + * Authors:
>>> + * Dong Xu Wang
>>> + *
>>> + * This file is based on qcow2-cache.c, see its copyrights below:
>>> + *
>>> + * L2/refcount table cache for the QCOW2 format
>>> + *
>>> + * Copyright (c) 2010 Kevin Wolf
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining a
>>> copy
>>> + * of this software and associated documentation files (the "Software"),
>>> to deal
>>> + * in the Software without restriction, including without limitation the
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be included
>>> in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>>> OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>> FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#ifndef BLOCK_CACHE_H
>>> +#define BLOCK_CACHE_H
>>> +
>>> +typedef enum {
>>> +BLOCK_TABLE_REF,
>>> +BLOCK_TABLE_L2,
>>> +} BlockTableType;
>>
>> I'm not excited about exposing these types, but it's okay for now. We
>> can always change the implementation later to be nicer.
>>
>>> +
>>> +typedef struct BlockCachedTable {
>>> +void*table;
>>> +int64_t offset;
>>> +booldirty;
>>> +int cache_hits;
>>> +int ref;
>>> +} BlockCachedTable;
>>> +
>>> +struct BlockCache {
>>> +BlockCachedTable*entries;
>>> +struct BlockCache *depends;
>>> +int size;
>>> +size_t cluster_size;
>>> +BlockTableType table_type;
>>> +booldepends_on_flush;
>>> +};
>>
>> Why have these definitions been moved to the header file? They are
>> supposed to be private to the implementation.
>>
> Both qcow2.h:BDRVQcowState and add-cow.c: BDRVAddCowState have a
> member typed BlockCache,
> such as:
> typedef struct BDRVAddCowState {
> BlockDriverState*image_hd;
> CoMutex lock;
> int cluster_size;
> BlockCache *bitmap_cache;
> uint64_tbitmap_size;
> AddCowHeaderheader;
> charbacking_fmt[16];
> charimage_fmt[16];
> } BDRVAddCowState;
>
> So I have to move the definitions to the header file, so that both
> add-cow.c and qcow2.h can use BlockCache.
> Is it Okay?
The fields are actually of the type BlockCache*, i.e. a pointer. They
don't need the full definition of the struct, a forward declaration is
sufficient. So it's enough to have these lines from qcow2.h in the header:
struct Qcow2Cache;
typedef struct Qcow2Cache Qcow2Cache;
Kevin
Re: [Qemu-devel] [PATCH V17 5/6] add-cow file format core code.
Am 11.12.2012 09:11, schrieb Dong Xu Wang:
>>> index bf5c57c..1a30462 100644
>>> --- a/block/block-cache.c
>>> +++ b/block/block-cache.c
>>> @@ -112,6 +112,8 @@ static int block_cache_entry_flush(BlockDriverState
>>> *bs, BlockCache *c, int i)
>>> BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_UPDATE_PART);
>>> } else if (c->table_type == BLOCK_TABLE_L2) {
>>> BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE);
>>> +} else if (c->table_type == BLOCK_TABLE_BITMAP) {
>>> +BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);
>>> }
>>>
>>> ret = bdrv_pwrite(bs->file, c->entries[i].offset,
>>> @@ -245,6 +247,8 @@ static int block_cache_do_get(BlockDriverState *bs,
>>> BlockCache *c,
>>> if (read_from_disk) {
>>> if (c->table_type == BLOCK_TABLE_L2) {
>>> BLKDBG_EVENT(bs->file, BLKDBG_L2_LOAD);
>>> +} else if (c->table_type == BLOCK_TABLE_BITMAP) {
>>> +BLKDBG_EVENT(bs->file, BLKDBG_COW_READ);
>>> }
>>
>> I must admit that I don't like this table_type stuff at all, even more
>> so if every new format adds new types to it. Not sure what to suggest here.
>>
>> But anyway, even if we leave it, aren't BLKDBG_COW_READ/WRITE something
>> completely different?
>
> Sorry, I have read enum BlkDebugEvent to find out which type would be
> suitable for this case. In the previous
> comments, you said existing DebugEvent types would be enough, do you
> think which types should be picked up
> for add-cow BLKDBG_EVENT?
Maybe BLKDBG_L2_LOAD/UPDATE?
Kevin
Re: [Qemu-devel] [RFC 0/4] virtio: stabilize migration format
On Mon, Dec 10, 2012 at 08:29:46AM -0600, Anthony Liguori wrote: > This series replaces: > > qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); > > With code that properly saves out each element of the structure using > a well defined endian format. Migration is broken today from big endian to > little endian hosts. > > There's no way to fix this problem without bumping the migration version > number and that's exactly what we do here. By bumping the migration version > number, we do break new->old migration but that's unavoidable right now. > > In order to support old->new, we assume that all incoming data is in little > endian. The final patch adds a check to the load routines to fail old->new > on big endian hosts where this may not have been true. Is there a way to detect the endianness of the source host - by peaking at a known multibyte value in the incoming stream? That way we could even support cross-endian migration. Not sure if this much magic makes sense since cross-endian migration is probably used rarely. Stefan
Re: [Qemu-devel] [Bug 1087974] Re: [regression] vnc tight png produces garbled output
On 12/11/12 09:57, Stefan Hajnoczi wrote: > On Mon, Dec 10, 2012 at 03:54:58PM -, Tim Hardeck wrote: >> 47683d669f993308c2b84bed4ce64aafb5d7ced4 is the first bad commit >> commit 47683d669f993308c2b84bed4ce64aafb5d7ced4 >> Author: Gerd Hoffmann >> Date: Thu Oct 11 12:04:33 2012 +0200 >> >> pixman/vnc: remove rgb_prepare_row* functions >> >> Let pixman do it instead. >> >> Signed-off-by: Gerd Hoffmann >> >> :04 04 653d58e66bf3a2d8240b2f9176979c44ccd720e1 >> 6b6e367a8522cb58b42ad8f204387a354d3b3d00 M ui >> >> >> Just reverting this particular commit isn't enough thou but it is connected. > > This suggests the conversion to pixman has introduced the bug. Quite possible. I've tested tight via 'vncviewer PreferredEncoding=Tight', but that doesn't force png, so there is a chance for issues I havn't seen. > Tim: can you provide steps to reproduce the bug? That will certainly help fixing. Also: How garbled? Totally messed up? Just colors wrong? Any other visible pattern? cheers, Gerd
Re: [Qemu-devel] [PATCH v6] Add compare subcommand for qemu-img
On Thu, Dec 06, 2012 at 07:24:04AM -0500, Miroslav Rezanina wrote: > This is second version of patch adding compare subcommand that > compares two images. Compare has following criteria: > - only data part is compared > - unallocated sectors are not read > - in case of different image size, exceeding part of bigger disk has > to be zeroed/unallocated to compare rest > - qemu-img returns: > - 0 if images are identical > - 1 if images differ > - 2 on error > > v6: > - added handling -?, -h options for compare subcommand > > v5 (only minor changes): > - removed redundant comment > - removed dead code (goto after help()) > - set final total_sectors on first assignment > > v4: > - Fixed various typos > - Added functions for empty sector check and sector-to-bytes offset > conversion > - Fixed command-line parameters processing > > v3: > - options -f/-F are orthogonal > - documentation updated to new syntax and behavior > - used byte offset instead of sector number for output > > v2: > - changed option for second image format to -F > - changed handling of -f and -F [1] > - added strict mode (-s) > - added quiet mode (-q) > - improved output messages [2] > - rename variables for larger image handling > - added man page content > > Signed-off-by: Miroslav Rezanina Acked-by: Stefan Hajnoczi
Re: [Qemu-devel] What is the current state of the various usb implementations?
On Mon, Dec 10, 2012 at 11:38:21AM -0500, tom289...@safe-mail.net wrote: > Hello, > > I sent this to qemu-discuss a week ago and got no responses, so I'm trying > here. > > Looking at `qemu-kvm -device ?`, there seem to be multiple usb > implementations: > ich9-usb-uhci2 > ich9-usb-uhci3 > ich9-usb-uhci1 > piix3-usb-uhci > piix4-usb-uhci > vt82c686b-usb-uhci > ich9-usb-ehci1 > usb-ehci > nec-usb-xhci > > Are they all fully implemented and stable? If not, what is the state of each? Here is a link to Gerd's USB Status presentation at KVM Forum 2012 in November: http://www.linux-kvm.org/wiki/images/b/be/2012-forum-qemu-usb-status-update.pdf Stefan
Re: [Qemu-devel] [PATCH] Fix error code checking for SetFilePointer() call
On Mon, Dec 10, 2012 at 12:56:22PM +0100, Fabien Chouteau wrote: > An error has occurred if the return value is invalid_set_file_pointer > and getlasterror doesn't return no_error. > > Signed-off-by: Fabien Chouteau > --- > block/raw-win32.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) The fprintf() is kind of iffy but we only return -EIO so I guess it helps to print the full error. Acked-by: Stefan Hajnoczi
Re: [Qemu-devel] What is the current state of the various usb implementations?
HI, Since the slides have been there, why would nobody like to export them on http://www.linux-kvm.org/page/KVM_Forum_2012? If so, we can download or review them. On Tue, Dec 11, 2012 at 5:03 PM, Stefan Hajnoczi wrote: > On Mon, Dec 10, 2012 at 11:38:21AM -0500, tom289...@safe-mail.net wrote: >> Hello, >> >> I sent this to qemu-discuss a week ago and got no responses, so I'm trying >> here. >> >> Looking at `qemu-kvm -device ?`, there seem to be multiple usb >> implementations: >> ich9-usb-uhci2 >> ich9-usb-uhci3 >> ich9-usb-uhci1 >> piix3-usb-uhci >> piix4-usb-uhci >> vt82c686b-usb-uhci >> ich9-usb-ehci1 >> usb-ehci >> nec-usb-xhci >> >> Are they all fully implemented and stable? If not, what is the state of each? > > Here is a link to Gerd's USB Status presentation at KVM Forum 2012 in > November: > > http://www.linux-kvm.org/wiki/images/b/be/2012-forum-qemu-usb-status-update.pdf > > Stefan > -- Regards, Zhi Yong Wu
Re: [Qemu-devel] [PATCH 1/1] seabios: update to e8a76b0f225bba5ba9d63ab227e0a37b3beb1059
Thanks Gerd,
This fix also my win2003 R2 SP2 x64 acpi bsod with hpet enabled
Regards,
Alexandre
- Mail original -
De: "Gerd Hoffmann"
À: qemu-devel@nongnu.org
Cc: "Gerd Hoffmann" , qemu-sta...@nongnu.org
Envoyé: Mardi 11 Décembre 2012 08:34:12
Objet: [Qemu-devel] [PATCH 1/1] seabios: update to
e8a76b0f225bba5ba9d63ab227e0a37b3beb1059
This patch updates seabios to latest git master. Changes:
(1) q35 patches merged.
(2) some acpi cleanups.
(3) fixes irq 8 conflict.
(3) makes this a candidate for the stable branch
Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann
---
pc-bios/acpi-dsdt.aml | Bin 4540 -> 4438 bytes
pc-bios/bios.bin | Bin 131072 -> 262144 bytes
pc-bios/q35-acpi-dsdt.aml | Bin 0 -> 7458 bytes
roms/seabios | 2 +-
4 files changed, 1 insertions(+), 1 deletions(-)
create mode 100644 pc-bios/q35-acpi-dsdt.aml
diff --git a/pc-bios/acpi-dsdt.aml b/pc-bios/acpi-dsdt.aml
index
bb3dd83a56f84131f9c9f968cbee9385931cecd3..18b4dc1aa53859593604abe598b05784b116a360
100644
GIT binary patch
delta 574
zcmdm^d`*eVCD`0uHizAVnY*n>jg-F>?BHKs32dzQI|{EXc(-IgqP>R~4=*x=C`fJg=xQ1J{26
zH&!HJPnOB&xFiju8`a|U0wN5J0}PFw85lhD8Q6UST>TgsEEt%b9YG9cXD1-V;pPG)
z1$?9@zv8l&Gxv6KiHA9nnOTH0J~+gYVF?#MN4#@T@MM2(N#^J#^~nX?5_Vt_K@Kif
zB?f^?hUg|Q7B?4XxcAr@L?pm^3qa;4EMS|Qz_5g2Aq(87Q`|E3(M@(f0U>bvSm1g<
zF2&R%D8R+*&RD@*!OX#-)W86;)Ro2A-`5c4U{*#*Xu_-p`4g8hKbU{;Ff(u{34qOU
zW0}03N6ZT9WKh81)W`IL38;+&s7=6|10L>^FR`0XuH^ONb)OB2Hv!iYP*iT5?7%+x
y6mL5t!{m#6+LLSfY}tI+ef+$gCOh$2PyWLfJoy5j&g5!-V94_8Z2rZ6h!Fs+D3!_p
delta 695
zcmcbnv`3lCCDfUsi(f<%p+S&?i&cq1pppTq$(_Z`#To7ib_OI}
zIuKn>p>76~7qG~rEdjBjo4i?l0z%;SurP>dfbA)Wi;L3>h%hvE2?};hs7?TSf}tT{
z0o&vRh9wLOSwPNEL>MS2z{Ts%SixMu%)z15zyNd(#BfKpNvxvvm?k?792`3QY3?f`Xgd2!(2N511!V^SzDG0bAOq+a;)r-wZ9h68W
zr|@=6&f?RVEWu~X=F9Hm=j}B4CZEmZX?($xGx&8Ti}D9EvP^!+
zw7gYTRwO11c>z)IRw-V>yOx)OsNfyR+TZhaR#xl#dH(}Hd_4EenR(4?Uh|rn*SzL(
z%*k=i$pRruvTi{2)IDoM=K=we!)&pMyrNH7?#(n?}0I$R`_5olY4_<>An+(hW
z77u3G>;wWAfXBe+Ll|=&%Gh7S7;_lT*oT051Y-k6GWHc9V;S236ap2%8Q>OR
z0o>9UJ2sxN1?h~f2DSq;CNQ=XuuNu5H-)ioKo;-`@WqsD#%dAJPG!sos0W;1W^C6q
z#%==l^(Z`xu>@ch@C5h=XgLq`KtI3)xX%a8BF0jIEFgRdV}pQ+z)aw4Ap9-HdIKYX
zJizcaV}EBGP^&zs1egKL1r`7sfNy}TWl%RD19r=yZeSZw2`qkxF)xX+uE2XM87l+U
z8X-8~u!^yP?=klB8Z;qi?Dz+ajr$P%ft|o$6JwKr?A9NlEdV31^JB)^u0w|PjP3Z0
zvEP6%HlRI$Ra?*mK;{>y$(N|zR>p<{?*g}gW?wVb5zv0aSZClf-~sUWw~TpiV=VML
z#)bg1zGv(mAh&?A)`e*Q8HJDn_!w~6!I(c#^b`8TPN?1t#>Ef_7!P~}xRfxa2hITe
zXXFElfxW;H;5cvr_!D>t`~?WR7;^%0f!~35cBAk;C~Gg;zhp0CCxOAGj0Kl7whXui
z^x4l?4saWAJiyr7Kq+ty7*WAk2CxXY0>u1+{J>
zpa*iZ@$)Y55%49j8#n^o0-7C1d=-Wo5OjjENZ<*eJ_(HgTY;Z}KY`d&=xjh1u&o-r
z&M>wII1j|1gZj^7Bmo~?z{mmi19L7y^Pk|S>=I))fX6`0Wn=&nfJ7h(C;;{Y>Z?#0
z@D$KqV{8I28^{Ab0=5CAz)3&`o&wtI70b77Gz)MRo
za)7H#k@gm20dHl)BqJ~lco#4O>)uA3fid4@j4cNW0oUb>O$UwxUETpbFmEMB2k;8)
z!6M)@;9H=3KHh<2K=8Y0ZeTXB4Y&w6zQTg+Cp2jC9l(jE8)*b2ik=UYrtKsy*0?RSig0^+~NWCYy(fiVpX
zOel~5%mj?UI$#^{2hjFM#zt<3jey-L+5ubh6NWUf7+4K#0Lp$s`_~|_6UL}T5!7YI
zBvTAk1KR+P64--XD0DYto%TX~KvF3*QO;N$5VxPPsRz*WE6|w^p~()zdIE!iu1C;0
zf5nIb3JAZU0>_{QVCr$$i7Li^s>()3LLe67J{4$%v91FS04IR#Q;c0d16zI;V;@)!
ztOK?KOD>>LVAe&nHSi}e^%D97@EOn^Lprd%z?N4C>_j^Z?*OPAICCDfz&%faHDu$*
zy_LXv0x`f~U_6ikyaDKeE4>AF8&E|GtQpV>@COzG>wzL*AMgus8n^`10B(H*7660-
z-GM*)A|4pu4}5_|Kw*sL9cPv90YB9GgzB;z|IEurb>5Bt>?QhKcGEC9#bU9HUskG5
z$zRR1j7drM(v^}h-ru{kYKJF()qA*axu+YGCg|+*J=~=+I_LaEoxgP1I7a6y&uZjV
z-rc%}rDzx-tGPJ1A6O#MVp+Rmp_(<`MW`>duqUh`S(
zr_}1uUlz-_Wu;kzGg&ce#Y#f)V+P?;U!6Mgfg2OwAZfNg;otctdMR15kp+s#YMH5(
z?ivnTh`NQJhgXt~y6Y1-9sAX_Ju2;W<(@p+&r5aV5g+TPQ^hp$rGC9s=}-7ienF~;
zC;Yr$fa-k1`)y~dS~c*wZA1Nzp%
z@t_I@;XxCn6t7dusgHTHc2k|skoc3I@Hy?;3kjUJ(}gRQC~L4-q_VVShq62}wLkh=
zs{zWdmH3H>Z;`n`w0}qn;Bvbgs&^jq?}>6RId5KVbw(q3$Lm<
z(ZG%E{XN5>JgE}3nz^F-`t}zY-PtO)
zj%566`dghv*dit-+1PKSJXZNN^6vZmNIlw85H8b5yjUos&J@wcVl7URpE!szmY&4z+5Vq7d_ww
zL9JDv-{*&de7ovwWxrl;?M~C>e1vL?R>td8QQzoWDmA)63=-?PdvItQVmmdws^EdV
z>mQ2+Dr_!#L?hX;o{tL-S5Hw|lN#_)9se+RD2?(f!L57iZSm=K){@!`tFYIyWmQ>^
zB`;~Vv$W6r%J*(ezIxYU8QkZex!=?}$m8mGw+>0F!S{J?hlHUtH02-a(WefGWF3m>
zhdPy#@F69{>zp&S#lBFhE&eTvulQFiZ`mx8gBF1oqGnC-F_~IY?b4RxR<(9s7;i$H~Epyr7HbjJU_J3Pf?Pt)~Y0t
z#3_ca^-NEEP8V+~a#fc=|5Z&z){ZbHdl^-R%fA>7sw9=U5#)0EE&f{<&mM0>>?S?f
zhNFjo@|En$Xi1Tks@3#Bv^+=Vt-Bsq<^08OcI~VRxn0=0+wX!3hPFPeRa?a#&Ap+>
z=3z|KX*EA2>U=Y`)QBL_g$HZ5G&L8jzFKy;RyaOM^UZ+KhFNpxNWfh@tc7!N68K|G<
zT;`X0ercVq?q8vTIyM#b5f#K96>jLI71}5T9lZR!Ajiy@XZ1PUyA9R+dhgC^u}Rr-
z?8QRA$Y7zBQl*yG2Ac4!fj~U2Pg`}Gjrja|Zs_x>qP{*}js;3Qck3G21FCEp&4%o(MC$+#sx+gncs=CW-r^Rljb@K`pK86j(t(KRueRC1+$S56
zo^{y}y$Uls)8xH`+{S1xjQqHcTP0fN#?aqEmxQ>D6hfo97`4l)^!C!R+DDsXQW14d
z-yz?KB@&a)T1{U7s54cW=zA1d9bP35dt#w!uyWARdN9d-?da7OFxw(j#f&44Qa6ND
zo#UQK)#DU;Y561!78fxHlGj{?_hG(@99?ENcVkH+NheZJjHinI2^Y46;&N|Nq`BI~
z4Na^yW*x0`+){NHir6!mex*y0>Hd}y
zCEI>&Z8M{ba7GXe`|NWY!yLh)PO5l(v4n!p(!!OrV<>iRT?suO>dI@FYD0bC68Y=;E`@~V^2vH
zwZh<>{w%98q++nqZ#7bDb@{{X!>gpJs+H>2v5|@VrNJHADYaBpku^$N
zX36r(j26fI)l`?mzwr+S$7L%s#D?RjaCt>+s8A8xg=JL6c5TKgW5c#XDHX91cPnCh
z)>pS_1Cxcj4!|8Up-hx;`MS*}>D)5e&N7&;Dei({zrn;lmUZw61OROMk8V`$K4
zJ??mX#qegS;_uWm-ot8bTzdMX!muI!LbDvShSB3;9xQG`HyY+E6ZGn*jZLch;V5?*
znyRryNGbc}VMqBJL+2_vn<+VO4(&iW
Re: [Qemu-devel] buildbot failure in qemu on virtfs_x86_64_debian_6_0
On Thu, Dec 06, 2012 at 08:19:24PM +0530, Aneesh Kumar K.V wrote: > q...@buildbot.b1-systems.de writes: > > > The Buildbot has detected a new failure on builder virtfs_x86_64_debian_6_0 > > while building qemu. > > Full details are available at: > > > > http://buildbot.b1-systems.de/qemu/builders/virtfs_x86_64_debian_6_0/builds/307 > > > > Buildbot URL: http://buildbot.b1-systems.de/qemu/ > > > > Buildslave for this Build: yuzuki > > > > Build Reason: The Nightly scheduler named 'nightly_virtfs' triggered this > > build > > Build Source Stamp: [branch for-upstream] HEAD > > Blamelist: > > > > BUILD FAILED: failed git > > Hmm, My github repo changed this week from QEMU.git to qemu.git. > > git://github.com/kvaneesh/qemu.git Daniel: The URL above is the new virtfs git repo. Is it possible to update the buildbot config? Thanks, Stefan
Re: [Qemu-devel] [PATCH] net, hub: fix the indent in the comments
On Fri, Dec 07, 2012 at 09:43:18AM +0800, zwu.ker...@gmail.com wrote: > From: Zhi Yong Wu > > Remove some redundant blanks in the comments of > net_hub_id_for_client(). > > Signed-off-by: Zhi Yong Wu > --- > net/hub.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] buildbot failure in qemu on virtfs_x86_64_debian_6_0
On Tuesday, December 11, 2012 10:34:34 AM Stefan Hajnoczi wrote: > On Thu, Dec 06, 2012 at 08:19:24PM +0530, Aneesh Kumar K.V wrote: > > q...@buildbot.b1-systems.de writes: > > > > > > > The Buildbot has detected a new failure on builder > > > virtfs_x86_64_debian_6_0 while building qemu.> > > > > Full details are available at: > > > http://buildbot.b1-systems.de/qemu/builders/virtfs_x86_64_debian_6_0 > > >/builds/307> > > > > Buildbot URL: http://buildbot.b1-systems.de/qemu/ > > > > > > Buildslave for this Build: yuzuki > > > > > > Build Reason: The Nightly scheduler named 'nightly_virtfs' triggered > > > this build Build Source Stamp: [branch for-upstream] HEAD > > > Blamelist: > > > > > > BUILD FAILED: failed git > > > > > > > > Hmm, My github repo changed this week from QEMU.git to qemu.git. > > > > > > > > git://github.com/kvaneesh/qemu.git > > Daniel: The URL above is the new virtfs git repo. Is it possible to > update the buildbot config? updated. Best Regards, Daniel -- Daniel Gollub Linux Consultant & Developer Tel.: +49-160 47 73 970 Mail: gol...@b1-systems.de B1 Systems GmbH Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537 signature.asc Description: This is a digitally signed message part.
Re: [Qemu-devel] [ [PATCH 1/2] cutils:change strtosz_suffix_unit function
On Fri, Dec 07, 2012 at 11:49:49AM +0800, liguang wrote:
> if value to be translated is larger than INT64_MAX,
> this function will not be convenient for caller to
> be aware of it, so change a little for this.
>
> Signed-off-by: liguang
> ---
> cutils.c |5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
I don't understand what this patch is supposed to do.
Why change the type of mul from double to int64_t?
Are you using retval = 0 as a special value to indicate overflow? Then
the new return value should be documented. But it would be better to
change the function to return -errno values instead of 0/-1 so the
caller knows the reason for specific failure cases (e.g. -E2BIG).
Should the val < 0 case be checked earlier in the function? It seems
like a different failure case then INT64_MAX overflow.
> diff --git a/cutils.c b/cutils.c
> index 4f0692f..8905b5e 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -219,11 +219,11 @@ static int64_t suffix_mul(char suffix, int64_t unit)
> int64_t strtosz_suffix_unit(const char *nptr, char **end,
> const char default_suffix, int64_t unit)
> {
> -int64_t retval = -1;
> +int64_t retval = -1, mul;
> char *endptr;
> unsigned char c;
> int mul_required = 0;
> -double val, mul, integral, fraction;
> +double val, integral, fraction;
>
> errno = 0;
> val = strtod(nptr, &endptr);
> @@ -246,6 +246,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end,
> goto fail;
> }
> if ((val * mul >= INT64_MAX) || val < 0) {
> +retval = 0;
> goto fail;
> }
> retval = val * mul;
> --
> 1.7.2.5
>
>
Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered
Hi Kevin,
I'm using the bdrv_pread() function during boot partition detection ...
In detail:
bdrv_pread() is called to read 32 bytes from a 2048 bytes formatted
disk. This results in setting up a read of 512 bytes (1 sector
multiplied by 512 current code in paio_submit()), which is wrong for a
O_DIRECT opened file, and produces the error.
Heinz
On Mon, 2012-12-10 at 09:55 +0100, Kevin Wolf wrote:
> Am 07.12.2012 21:26, schrieb Heinz Graalfs:
> > Hello Kevin,
> >
> > I'm resending my answer as of Nov 23rd.
> >
> > Is this still on your queue?
>
> No, it wasn't. I guess I was waiting for a new version of the patch.
>
> >>> }
> >>>
> >>> void *qemu_blockalign(BlockDriverState *bs, size_t size)
> >>> diff --git a/block/raw-posix.c b/block/raw-posix.c
> >>> index f2f0404..baebf1d 100644
> >>> --- a/block/raw-posix.c
> >>> +++ b/block/raw-posix.c
> >>> @@ -700,6 +700,12 @@ static BlockDriverAIOCB
> >>> *paio_submit(BlockDriverState *bs, int fd,
> >>> acb->aio_nbytes = nb_sectors * 512;
> >>> acb->aio_offset = sector_num * 512;
> >>>
> >>> +/* O_DIRECT also requires an aligned length */
> >>> +if (bs->open_flags & BDRV_O_NOCACHE) {
> >>> +acb->aio_nbytes += acb->bs->buffer_alignment - 1;
> >>> +acb->aio_nbytes &= ~(acb->bs->buffer_alignment - 1);
> >>> +}
> >>
> >> Modifying aio_nbytes, but not the iov looks wrong to me. This may work
> >> in the handle_aiocb_rw_linear() code path, but not with actual vectored
> >> I/O.
> >
> > Current coding ensures that read IO buffers always seem to be aligned
> > correctly. Whereas read length values are not always appropriate for an
> > O_DIRECT scenario.
> >
> > For a 2048 formatted disk I verified that
> >
> > 1. non vectored IO - the length needs to be adapted several times,
> >which is accomplished now by the patch.
> >
> > 2. vectored IO - the qiov's total length is always a multiple of the
> >logical block size
> > (which is also verified in virtio_blk_handle_read())
> >The particular iov length fields are already correctly setup as a
> >multiple of the logical block size when processed in
> >virtio_blk_handle_request().
>
> I must admit that I don't quite understand this. As far as I know,
> virtio-blk doesn't make any difference between requests with niov = 1
> and real vectored requests. So how can the length of the latter always
> be right, whereas the length of the former may be wrong?
>
> The other point is that requests may not even be coming from virtio-blk.
> They could be made by other device emulations or they could come from a
> block job. (They also could be the result of a merge in the block layer,
> though if the original requests were aligned, the result will stay aligned)
>
> Kevin
>
Re: [Qemu-devel] [PATCH 1/2] pseries: Don't allow TCE (iommu) tables to be registered with duplicate LIOBNs
On 10.12.2012, at 14:00, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2012 at 05:07:32PM +1100, David Gibson wrote:
>> The PAPR specification requires that every bus or device mediated by the
>> IOMMU have a unique Logical IO Bus Number (LIOBN). This patch adds a check
>> to enforce this, which will help catch errors in configuration earlier.
>>
>> Signed-off-by: David Gibson
>
> Acked-by: Michael S. Tsirkin
Thanks, applied to ppc-next.
Alex
>
>> ---
>> hw/spapr_iommu.c |6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/spapr_iommu.c b/hw/spapr_iommu.c
>> index 02d78cc..3011b25 100644
>> --- a/hw/spapr_iommu.c
>> +++ b/hw/spapr_iommu.c
>> @@ -120,6 +120,12 @@ DMAContext *spapr_tce_new_dma_context(uint32_t liobn,
>> size_t window_size)
>> {
>> sPAPRTCETable *tcet;
>>
>> +if (spapr_tce_find_by_liobn(liobn)) {
>> +fprintf(stderr, "Attempted to create TCE table with duplicate"
>> +" LIOBN 0x%x\n", liobn);
>> +return NULL;
>> +}
>> +
>> if (!window_size) {
>> return NULL;
>> }
>> --
>> 1.7.10.4
Re: [Qemu-devel] [PATCH 2/3 v4] vnc: added initial websocket protocol support
On Fri, Dec 07, 2012 at 03:56:34PM +0100, Tim Hardeck wrote:
Thanks for addressing my review comments.
> @@ -1328,13 +1358,14 @@ void vnc_client_read(void *opaque)
>
> void vnc_write(VncState *vs, const void *data, size_t len)
> {
> -buffer_reserve(&vs->output, len);
> +buffer_reserve(&vs->output, len);
>
> -if (vs->csock != -1 && buffer_empty(&vs->output)) {
> -qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read,
> vnc_client_write, vs);
> -}
> +if (vs->csock != -1 && buffer_empty(&vs->output)) {
> +qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read,
> +vnc_client_write, vs);
> +}
>
> -buffer_append(&vs->output, data, len);
> +buffer_append(&vs->output, data, len);
QEMU uses 4 spaces for indentation.
Stefan
Re: [Qemu-devel] KVM call agenda for 2012-12-11
Am 10.12.2012 14:59, schrieb Juan Quintela: > > Hi > > Please send in any agenda topics you are interested in. Can probably be answered on the list, but what is the status of libqos? Kevin
Re: [Qemu-devel] main-loop.c: About Select handling
On Thu, Dec 06, 2012 at 12:30:19AM +, Furukawa, Eiji wrote: > > The select() do not masked signal in os_host_main_loop_wait. > > For example, qemu-timer.c gives SIGALRM regularly. > Even if a buffer of select is empty, What does "a buffer of select is empty" mean? > I think that it is a problem that this select() does exit by this SIGALRM. The main loop will iterate again. Why is it a problem if select() returns? Stefan
[Qemu-devel] [PATCH 3/6] target-i386: explicitly set vendor for each built-in cpudef
it will help to get rid of setting default.
Signed-off-by: Igor Mammedov
---
target-i386/cpu.c | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 64b7637..1497980 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -387,6 +387,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "core2duo",
.level = 10,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 15,
.stepping = 11,
@@ -431,6 +434,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "qemu32",
.level = 4,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 3,
.stepping = 3,
@@ -441,6 +447,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "kvm32",
.level = 5,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 15,
.model = 6,
.stepping = 1,
@@ -455,6 +464,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "coreduo",
.level = 10,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 14,
.stepping = 8,
@@ -470,6 +482,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "486",
.level = 1,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 4,
.model = 0,
.stepping = 0,
@@ -479,6 +494,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "pentium",
.level = 1,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 5,
.model = 4,
.stepping = 3,
@@ -488,6 +506,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "pentium2",
.level = 2,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 5,
.stepping = 2,
@@ -497,6 +518,9 @@ static x86_def_t builtin_x86_defs[] = {
{
.name = "pentium3",
.level = 2,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 7,
.stepping = 3,
@@ -522,6 +546,9 @@ static x86_def_t builtin_x86_defs[] = {
.name = "n270",
/* original is on level 10 */
.level = 5,
+.vendor1 = CPUID_VENDOR_INTEL_1,
+.vendor2 = CPUID_VENDOR_INTEL_2,
+.vendor3 = CPUID_VENDOR_INTEL_3,
.family = 6,
.model = 28,
.stepping = 2,
--
1.7.1
[Qemu-devel] [PATCH 5/6] target-i386: move setting defaults out of cpu_x86_parse_featurestr()
No functional change, needed for simplifying convertion to properties.
Signed-off-by: Igor Mammedov
---
target-i386/cpu.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 99fd3f3..e534254 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1264,7 +1264,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
/* Features to be added */
uint32_t plus_features = 0, plus_ext_features = 0;
uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
-uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
+uint32_t plus_kvm_features = 0, plus_svm_features = 0;
uint32_t plus_7_0_ebx_features = 0;
/* Features to be removed */
uint32_t minus_features = 0, minus_ext_features = 0;
@@ -1273,10 +1273,6 @@ static int cpu_x86_parse_featurestr(x86_def_t
*x86_cpu_def, char *features)
uint32_t minus_7_0_ebx_features = 0;
uint32_t numvalue;
-add_flagname_to_bitmaps("hypervisor", &plus_features,
-&plus_ext_features, &plus_ext2_features, &plus_ext3_features,
-&plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features);
-
featurestr = features ? strtok(features, ",") : NULL;
while (featurestr) {
@@ -1536,6 +1532,12 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
goto error;
}
+def->kvm_features |= kvm_default_features;
+add_flagname_to_bitmaps("hypervisor", &def->features,
+&def->ext_features, &def->ext2_features,
+&def->ext3_features, &def->kvm_features,
+&def->svm_features, &def->cpuid_7_0_ebx_features);
+
if (cpu_x86_parse_featurestr(def, features) < 0) {
goto error;
}
--
1.7.1
[Qemu-devel] [Bug 1087974] Re: [regression] vnc tight png produces garbled output
* make sure that qemu is compiled with --enable-vnc-png * git clone git://github.com/kanaka/noVNC * edit include/rfb.js at line 50 and comment out or remove all encodings above "['TIGHT_PNG',-260 ]," * open vnc.html in Firefox or Chrome *apply either my patch to QEMU https://lists.nongnu.org/archive/html /qemu-devel/2012-12/msg00869.html or use Websockify https://github.com/kanaka/websockify to get Websocket support. * in case of my patch run QEMU with `-vnc :0,websocket` and connect with noVNC to port 5700. * in case of Websockify run QEMU with `./websockify.py 5900 -- qemu- system-x86_64 -vnc :0` and connect to port 5900 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1087974 Title: [regression] vnc tight png produces garbled output Status in QEMU: New Bug description: VNC Tight PNG compression did work fine two or three month ago but don't anymore. Now when Tight PNG is used parts of the desktop are shown but they are scrambled together. I have always tested this feature against QEMU git with noVNC by only allowing Tight PNG compression. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1087974/+subscriptions
Re: [Qemu-devel] [PATCH 4/8] s390: Add channel I/O instructions.
On 10.12.2012, at 10:18, Cornelia Huck wrote:
> On Mon, 10 Dec 2012 10:00:16 +0100
> Alexander Graf wrote:
>
>>
>>
>> On 07.12.2012, at 13:50, Cornelia Huck wrote:
>>
>>> Provide handlers for (most) channel I/O instructions.
>>>
>>> Signed-off-by: Cornelia Huck
>>> ---
>>> target-s390x/cpu.h| 87 +++
>>> target-s390x/ioinst.c | 694
>>> +-
>>> target-s390x/ioinst.h | 16 ++
>>> trace-events | 6 +
>>> 4 files changed, 796 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>> index 73bfc20..bc119f8 100644
>>> --- a/target-s390x/cpu.h
>>> +++ b/target-s390x/cpu.h
>>> @@ -127,6 +127,8 @@ typedef struct CPUS390XState {
>>>QEMUTimer *tod_timer;
>>>
>>>QEMUTimer *cpu_timer;
>>> +
>>> +void *chsc_page;
>>> } CPUS390XState;
>>>
>>> #include "cpu-qom.h"
>>> @@ -363,6 +365,91 @@ static inline unsigned
>>> s390_del_running_cpu(CPUS390XState *env)
>>> void cpu_lock(void);
>>> void cpu_unlock(void);
>>>
>>> +typedef struct SubchDev SubchDev;
>>> +typedef struct SCHIB SCHIB;
>>> +typedef struct ORB ORB;
>>> +
>>> +static inline SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t
>>> ssid,
>>> + uint16_t schid)
>>> +{
>>> +return NULL;
>>> +}
>>> +static inline bool css_subch_visible(SubchDev *sch)
>>> +{
>>> +return false;
>>> +}
>>> +static inline void css_conditional_io_interrupt(SubchDev *sch)
>>> +{
>>> +}
>>> +static inline int css_do_stsch(SubchDev *sch, uint64_t addr)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t
>>> schid)
>>> +{
>>> +return true;
>>> +}
>>> +static inline int css_do_msch(SubchDev *sch, SCHIB *schib)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_xsch(SubchDev *sch)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_csch(SubchDev *sch)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_hsch(SubchDev *sch)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_ssch(SubchDev *sch, ORB *orb)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_tsch(SubchDev *sch, uint64_t addr)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_stcrw(uint64_t addr)
>>> +{
>>> +return 1;
>>> +}
>>> +static inline int css_do_tpi(uint64_t addr, int lowcore)
>>> +{
>>> +return 0;
>>> +}
>>> +static inline int css_collect_chp_desc(int m, uint8_t cssid, uint8_t
>>> f_chpid,
>>> + int rfmt, uint8_t l_chpid, void
>>> *buf)
>>> +{
>>> +return 0;
>>> +}
>>> +static inline void css_do_schm(uint8_t mbk, int update, int dct, uint64_t
>>> mbo)
>>> +{
>>> +}
>>> +static inline int css_enable_mss(void)
>>> +{
>>> +return -EINVAL;
>>> +}
>>> +static inline int css_enable_mcsse(void)
>>> +{
>>> +return -EINVAL;
>>> +}
>>> +static inline int css_do_rsch(SubchDev *sch)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline int css_do_rchp(uint8_t cssid, uint8_t chpid)
>>> +{
>>> +return -ENODEV;
>>> +}
>>> +static inline bool css_present(uint8_t cssid)
>>> +{
>>> +return false;
>>> +}
>>> +
>>> static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
>>> {
>>>env->aregs[0] = newtls >> 32;
>>> diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
>>> index 8577b2c..60ce985 100644
>>> --- a/target-s390x/ioinst.c
>>> +++ b/target-s390x/ioinst.c
>>> @@ -15,14 +15,19 @@
>>>
>>> #include "cpu.h"
>>> #include "ioinst.h"
>>> +#include "trace.h"
>>>
>>> -#ifdef DEBUG_IOINST
>>> -#define dprintf(fmt, ...) \
>>> -do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
>>> -#else
>>> -#define dprintf(fmt, ...) \
>>> -do { } while (0)
>>> -#endif
>>> +/* Special handling for the prefix page. */
>>> +static void *s390_get_address(CPUS390XState *env, ram_addr_t guest_addr)
>>> +{
>>> +if (guest_addr < 8192) {
>>> +guest_addr += env->psa;
>>> +} else if ((env->psa <= guest_addr) && (guest_addr < env->psa + 8192))
>>> {
>>> +guest_addr -= env->psa;
>>> +}
>>> +
>>> +return qemu_get_ram_ptr(guest_addr);
>>
>> Do we actually need this?
>
> Yes. I've seen failures for I/O instructions using the lowcore (which
> the Linux kernel likes to do).
Then we want an s390 generic function that does this, not an io specific one
though, right? Also qemu_get_ram_ptr is a no-go, as it doesn't do boundary
checks.
So what we really want is something like s390_cpu_physical_memory_map(env, ...)
with a special case on the lowcore.
>
>>
>>> +}
>>>
>>> int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int
>>> *ssid,
>>> int *schid)
>>> @@ -44,3 +49,678 @@ int ioinst_disassemble_sch_ident(uint32_t value, int
>>> *m, int *cssid, int *ssid,
>>>*schid = value & IOINST_SCHID_NR;
>>>return 0;
>>> }
>>> +
>>> +in
[Qemu-devel] [PATCH 0/6] x86 CPU cleanup (wave 2)
This series is a several cleanups, moved out from CPU properties series, since they do not really depend on CPU properties refactoring and could simplify CPU subclasses work as well. git tree for testing: https://github.com/imammedo/qemu/tree/x86_cpu_cleanup.wave2 Igor Mammedov (6): target-i386: filter out not TCG features if running without kvm at realize time target-i386: sanitize AMD's ext2_features at realize time target-i386: explicitly set vendor for each built-in cpudef target-i386: setting default 'vendor' is obsolete, remove it target-i386: move setting defaults out of cpu_x86_parse_featurestr() target-i386: move out CPU features initialization in separate func target-i386/cpu.c | 147 1 files changed, 90 insertions(+), 57 deletions(-)
Re: [Qemu-devel] [Bug 1087114] [NEW] assertion "QLIST_EMPTY(&bs->tracked_requests)" failed
On Thu, Dec 06, 2012 at 04:02:57AM -, Brad Smith wrote:
> QEMU 1.3.0 on OpenBSD now crashes with an error as shown below and the
> command line params do not seem to matter.
Please use git-bisect(1) to identify the commit that caused the
regression.
I was unable to hit this code path with qemu-system-i386 with an IDE
disk. Please do share your command-line.
> assertion "QLIST_EMPTY(&bs->tracked_requests)" failed: file "block.c",
> line 1220, function "bdrv_drain_all"
bdrv_drain_all() waits until in-flight requests have completed. The
assertion verifies that all I/O requests are really done. Something is
wrong here.
> #1 0x030d1bce24aa in abort () at /usr/src/lib/libc/stdlib/abort.c:70
> p = (struct atexit *) 0x30d11897000
> mask = 4294967263
> cleanup_called = 1
> #2 0x030d1bc5ff44 in __assert2 (file=Variable "file" is not available.
> ) at /usr/src/lib/libc/gen/assert.c:52
> No locals.
> #3 0x030b0d383a03 in bdrv_drain_all () at block.c:1220
> bs = (BlockDriverState *) 0x30d13f3b630
> busy = false
> __func__ = "bdrv_drain_all"
> #4 0x030b0d43acfc in bmdma_cmd_writeb (bm=0x30d0f5f56a8, val=8) at
> hw/ide/pci.c:312
> __func__ = "bmdma_cmd_writeb"
> #5 0x030b0d43b450 in bmdma_write (opaque=0x30d0f5f56a8, addr=0, val=8,
> size=1) at hw/ide/piix.c:76
> bm = (BMDMAState *) 0x30d0f5f56a8
The device is an IDE disk.
> #6 0x030b0d5c2ce6 in memory_region_write_accessor (opaque=0x30d0f5f57d0,
> addr=0, value=0x30d18c288f0, size=1, shift=0, mask=255)
> at /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/memory.c:334
> mr = (MemoryRegion *) 0x30d0f5f57d0
> tmp = 8
> #7 0x030b0d5c2dc5 in access_with_adjusted_size (addr=0,
> value=0x30d18c288f0, size=1, access_size_min=1, access_size_max=4,
> access=0x30b0d5c2c6b ,
> opaque=0x30d0f5f57d0) at
> /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/memory.c:364
> access_mask = 255
> access_size = 1
> i = 0
> #8 0x030b0d5c3222 in memory_region_iorange_write (iorange=0x30d1d5e7400,
> offset=0, width=1, data=8)
> at /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/memory.c:439
> mrio = (MemoryRegionIORange *) 0x30d1d5e7400
> mr = (MemoryRegion *) 0x30d0f5f57d0
> __func__ = "memory_region_iorange_write"
> #9 0x030b0d5c019a in ioport_writeb_thunk (opaque=0x30d1d5e7400,
> addr=49216, data=8) at
> /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/ioport.c:212
> ioport = (IORange *) 0x30d1d5e7400
> #10 0x030b0d5bfb65 in ioport_write (index=0, address=49216, data=8) at
> /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/ioport.c:83
> func = (IOPortWriteFunc *) 0x30b0d5c0148
> default_func = {0x30b0d5bfbbc , 0x30b0d5bfc61
> , 0x30b0d5bfd0c }
> #11 0x030b0d5c0704 in cpu_outb (addr=49216, val=8 '\b') at
> /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/ioport.c:289
> No locals.
> #12 0x030b0d6067dd in helper_outb (port=49216, data=8) at
> /home/ports/pobj/qemu-1.3.0-debug/qemu-1.3.0/target-i386/misc_helper.c:72
> No locals.
Re: [Qemu-devel] [PATCH 2/8] s390: Channel I/O basic defintions.
On 10.12.2012, at 11:18, Cornelia Huck wrote:
> On Mon, 10 Dec 2012 09:07:57 +0100
> Alexander Graf wrote:
>
>>
>> On 07.12.2012, at 13:50, Cornelia Huck wrote:
>>
>>> Basic channel I/O structures and helper function.
>>>
>>> Signed-off-by: Cornelia Huck
>>> ---
>>> target-s390x/Makefile.objs | 2 +-
>>> target-s390x/ioinst.c | 46 ++
>>> target-s390x/ioinst.h | 207
>>> +
>>> 3 files changed, 254 insertions(+), 1 deletion(-)
>>> create mode 100644 target-s390x/ioinst.c
>>> create mode 100644 target-s390x/ioinst.h
>>>
>>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
>>> index e728abf..3afb0b7 100644
>>> --- a/target-s390x/Makefile.objs
>>> +++ b/target-s390x/Makefile.objs
>>> @@ -1,4 +1,4 @@
>>> obj-y += translate.o helper.o cpu.o interrupt.o
>>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>>> -obj-$(CONFIG_SOFTMMU) += machine.o
>>> +obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o
>>> obj-$(CONFIG_KVM) += kvm.o
>>> diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
>>> new file mode 100644
>>> index 000..8577b2c
>>> --- /dev/null
>>> +++ b/target-s390x/ioinst.c
>>> @@ -0,0 +1,46 @@
>>> +/*
>>> + * I/O instructions for S/390
>>> + *
>>> + * Copyright 2012 IBM Corp.
>>> + * Author(s): Cornelia Huck
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>>> + * your option) any later version. See the COPYING file in the top-level
>>> + * directory.
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include "cpu.h"
>>> +#include "ioinst.h"
>>> +
>>> +#ifdef DEBUG_IOINST
>>> +#define dprintf(fmt, ...) \
>>> +do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
>>> +#else
>>> +#define dprintf(fmt, ...) \
>>> +do { } while (0)
>>> +#endif
>>> +
>>> +int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int
>>> *ssid,
>>> + int *schid)
>>> +{
>>> +if (!(value & IOINST_SCHID_ONE)) {
>>> +return -EINVAL;
>>> +}
>>> +if (!(value & IOINST_SCHID_M)) {
>>> +if (value & IOINST_SCHID_CSSID) {
>>> +return -EINVAL;
>>> +}
>>> +*cssid = 0;
>>> +*m = 0;
>>> +} else {
>>> +*cssid = (value & IOINST_SCHID_CSSID) >> 24;
>>
>> (value & IOINST_SCHID_CSSID_MASK) >> IOINST_SCHID_CSSID_SHIFT
>
> I think that actually decreases readability.
I'm fine with doing it Jocelyn-style too:
#define IOINST_SCHID_CSSID(x) ((x & 0x...) >> 24)
if you prefer that for readability :)
Alex
Re: [Qemu-devel] [PATCH 3/8] s390: I/O interrupt and machine check injection.
On 10.12.2012, at 11:27, Cornelia Huck wrote:
> On Mon, 10 Dec 2012 09:20:57 +0100
> Alexander Graf wrote:
>
>>
>> On 07.12.2012, at 13:50, Cornelia Huck wrote:
>>
>>> I/O interrupts are queued per isc. Only crw pending machine checks
>>> are supported.
>>>
>>> Signed-off-by: Cornelia Huck
>>> ---
>>> target-s390x/cpu.h| 67 +++
>>> target-s390x/helper.c | 145
>>> ++
>>> 2 files changed, 212 insertions(+)
>>>
>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>> index 0f9a1f7..73bfc20 100644
>>> --- a/target-s390x/cpu.h
>>> +++ b/target-s390x/cpu.h
>>> @@ -47,6 +47,11 @@
>>> #define MMU_USER_IDX 1
>>>
>>> #define MAX_EXT_QUEUE 16
>>> +#define MAX_IO_QUEUE 16
>>> +#define MAX_MCHK_QUEUE 16
>>> +
>>> +#define PSW_MCHK_MASK 0x0004
>>> +#define PSW_IO_MASK 0x0200
>>>
>>> typedef struct PSW {
>>>uint64_t mask;
>>> @@ -59,6 +64,17 @@ typedef struct ExtQueue {
>>>uint32_t param64;
>>> } ExtQueue;
>>>
>>> +typedef struct IOQueue {
>>> +uint16_t id;
>>> +uint16_t nr;
>>> +uint32_t parm;
>>> +uint32_t word;
>>> +} IOQueue;
>>> +
>>> +typedef struct MchkQueue {
>>> +uint16_t type;
>>> +} MchkQueue;
>>> +
>>> typedef struct CPUS390XState {
>>>uint64_t regs[16]; /* GP registers */
>>>
>>> @@ -88,8 +104,16 @@ typedef struct CPUS390XState {
>>>
>>>int pending_int;
>>>ExtQueue ext_queue[MAX_EXT_QUEUE];
>>> +IOQueue io_queue[MAX_IO_QUEUE][8];
>>> +MchkQueue mchk_queue[MAX_MCHK_QUEUE];
>>>
>>>int ext_index;
>>> +int io_index[8];
>>> +int mchk_index;
>>> +
>>> +uint64_t ckc;
>>> +uint64_t cputm;
>>> +uint32_t todpr;
>>>
>>>CPU_COMMON
>>>
>>> @@ -364,12 +388,16 @@ static inline void cpu_set_tls(CPUS390XState *env,
>>> target_ulong newtls)
>>> #define EXCP_EXT 1 /* external interrupt */
>>> #define EXCP_SVC 2 /* supervisor call (syscall) */
>>> #define EXCP_PGM 3 /* program interruption */
>>> +#define EXCP_IO 7 /* I/O interrupt */
>>> +#define EXCP_MCHK 8 /* machine check */
>>>
>>> #endif /* CONFIG_USER_ONLY */
>>>
>>> #define INTERRUPT_EXT(1 << 0)
>>> #define INTERRUPT_TOD(1 << 1)
>>> #define INTERRUPT_CPUTIMER (1 << 2)
>>> +#define INTERRUPT_IO (1 << 3)
>>> +#define INTERRUPT_MCHK (1 << 4)
>>>
>>> /* Program Status Word. */
>>> #define S390_PSWM_REGNUM 0
>>> @@ -977,6 +1005,45 @@ static inline void cpu_inject_ext(CPUS390XState *env,
>>> uint32_t code, uint32_t pa
>>>cpu_interrupt(env, CPU_INTERRUPT_HARD);
>>> }
>>>
>>> +static inline void cpu_inject_io(CPUS390XState *env, uint16_t
>>> subchannel_id,
>>> + uint16_t subchannel_number,
>>> + uint32_t io_int_parm, uint32_t
>>> io_int_word)
>>> +{
>>> +int isc = ffs(io_int_word << 2) - 1;
>>> +
>>> +if (env->io_index[isc] == MAX_IO_QUEUE - 1) {
>>> +/* ugh - can't queue anymore. Let's drop. */
>>> +return;
>>> +}
>>> +
>>> +env->io_index[isc]++;
>>> +assert(env->io_index[isc] < MAX_IO_QUEUE);
>>> +
>>> +env->io_queue[env->io_index[isc]][isc].id = subchannel_id;
>>> +env->io_queue[env->io_index[isc]][isc].nr = subchannel_number;
>>> +env->io_queue[env->io_index[isc]][isc].parm = io_int_parm;
>>> +env->io_queue[env->io_index[isc]][isc].word = io_int_word;
>>> +
>>> +env->pending_int |= INTERRUPT_IO;
>>> +cpu_interrupt(env, CPU_INTERRUPT_HARD);
>>> +}
>>> +
>>> +static inline void cpu_inject_crw_mchk(CPUS390XState *env)
>>> +{
>>> +if (env->mchk_index == MAX_MCHK_QUEUE - 1) {
>>> +/* ugh - can't queue anymore. Let's drop. */
>>> +return;
>>> +}
>>> +
>>> +env->mchk_index++;
>>> +assert(env->mchk_index < MAX_MCHK_QUEUE);
>>> +
>>> +env->mchk_queue[env->mchk_index].type = 1;
>>> +
>>> +env->pending_int |= INTERRUPT_MCHK;
>>> +cpu_interrupt(env, CPU_INTERRUPT_HARD);
>>> +}
>>> +
>>> static inline bool cpu_has_work(CPUState *cpu)
>>> {
>>>CPUS390XState *env = &S390_CPU(cpu)->env;
>>> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
>>> index b7b812a..4ff148d 100644
>>> --- a/target-s390x/helper.c
>>> +++ b/target-s390x/helper.c
>>> @@ -574,12 +574,144 @@ static void do_ext_interrupt(CPUS390XState *env)
>>>load_psw(env, mask, addr);
>>> }
>>>
>>> +static void do_io_interrupt(CPUS390XState *env)
>>> +{
>>> +uint64_t mask, addr;
>>> +LowCore *lowcore;
>>> +hwaddr len = TARGET_PAGE_SIZE;
>>> +IOQueue *q;
>>> +uint8_t isc;
>>> +int disable = 1;
>>> +int found = 0;
>>> +
>>> +if (!(env->psw.mask & PSW_MASK_IO)) {
>>> +cpu_abort(env, "I/O int w/o I/O mask\n");
>>> +}
>>> +
>>> +for (isc = 0; isc < 8; isc++) {
>>> +if (env->io_index[isc] < 0) {
>>> +continue;
>>> +}
>>> +if (env->io_index[isc] > MAX_IO_QUEUE) {
>>> +cpu_abort(env, "I/O queue overrun for isc %d: %d\n",
>>
Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered
Am 11.12.2012 10:58, schrieb Heinz Graalfs: > Hi Kevin, > > I'm using the bdrv_pread() function during boot partition detection ... > > In detail: > bdrv_pread() is called to read 32 bytes from a 2048 bytes formatted > disk. This results in setting up a read of 512 bytes (1 sector > multiplied by 512 current code in paio_submit()), which is wrong for a > O_DIRECT opened file, and produces the error. So this sounds like the real problem: bdrv_pread/pwrite assume 512 byte sectors. May it's better to fix it there instead of just fixing one code path in one backend. In any case this patch as submitted is wrong as it overflows the buffer passed to paio_submit. Test it with this patch: --- a/qemu-io.c +++ b/qemu-io.c @@ -1718,6 +1718,8 @@ static int openfile(char *name, int flags, int growable) bs = NULL; return 1; } + +bdrv_set_buffer_alignment(bs, 4096); } return 0; $ ./qemu-io -n -c 'read -p 0 512' /tmp/foo read 512/512 bytes at offset 0 512 bytes, 1 ops; 0.0001 sec (3.727 MiB/sec and 7633.5878 ops/sec) *** glibc detected *** ./qemu-io: double free or corruption (out): 0x7fa22349b000 *** Kevin
Re: [Qemu-devel] What is the current state of the various usb implementations?
On 12/10/12 17:38, tom289...@safe-mail.net wrote: > Hello, > > I sent this to qemu-discuss a week ago and got no responses, so I'm trying > here. > > Looking at `qemu-kvm -device ?`, there seem to be multiple usb > implementations: > ich9-usb-uhci2 > ich9-usb-uhci3 > ich9-usb-uhci1 > piix3-usb-uhci > piix4-usb-uhci > vt82c686b-usb-uhci These are all UHCI, just variants with different pci ids for the different chipsets we are emulating. USB 1.1. Stable. > ich9-usb-ehci1 > usb-ehci Likewise these basially differ in the pci id only. USB 2.0. Needs UHCI/OHCI companion controllers for USB 1.1 support (see docs/ich9-ehci-uhci.cfg). Stable. > nec-usb-xhci XHCI. USB 3.0. Mostly stable. Has seen less testing that the others though, both because it is the youngest code base and because guest support isn't that good. cheers, Gerd
Re: [Qemu-devel] [PATCH 1/3] s390: Fix empty kernel command line
On 07.12.2012, at 14:55, Jens Freimann wrote:
> From: Christian Borntraeger
>
> Since commit 967c0da73a7b0da186baba6632301d83644a570c
>vl.c: Avoid segfault when started with no arguments
>
> the user can specify a kernel without a command line. Lets not
> overwrite the default command line with \0 in that case.
>
> Signed-off-by: Christian Borntraeger
> Signed-off-by: Jens Freimann
> ---
> hw/s390-virtio.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index ca1bb09..d77871a 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args)
> stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> }
>
> -if (rom_ptr(KERN_PARM_AREA)) {
> +if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) {
why strlen()? If no -append option was passed, kernel_cmdline should be NULL.
If -append "" was passed, the user wants to command line to be overwritten with
"\0".
Alex
Re: [Qemu-devel] [PATCH 2/3] s390: Move IPL code into a separate device
On 07.12.2012, at 14:55, Jens Freimann wrote:
> From: Christian Borntraeger
>
> Lets move the code to setup IPL for external kernel
> or via the zipl rom into a separate file. This allows to
>
> - define a reboot handler, setting up the PSW appropriately
> - enhance the boot code to IPL disks that contain a bootmap that
> was created with zipl under LPAR or z/VM (see additional patch)
> and thus disabling the bios code that only works for specifically
> prepared disks.
> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
>
> Signed-off-by: Christian Borntraeger
> Signed-off-by: Jens Freimann
Creating a separate device sounds like a good idea, but please do so
consistently. Please move everything that relates to booting into the device,
including -kernel.
Also, I don't think we need a "-default" ipl device. Just create a generic ipl
device that controls booting.
Alex
> ---
> hw/s390-virtio.c | 30 ++---
> hw/s390x/Makefile.objs |1 +
> hw/s390x/ipl.c | 80
> hw/s390x/ipl.h | 34
> 4 files changed, 119 insertions(+), 26 deletions(-)
> create mode 100644 hw/s390x/ipl.c
> create mode 100644 hw/s390x/ipl.h
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index d77871a..10a5fd7 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -33,6 +33,7 @@
>
> #include "hw/s390-virtio-bus.h"
> #include "hw/s390x/sclp.h"
> +#include "hw/s390x/ipl.h"
>
> //#define DEBUG_S390
>
> @@ -48,17 +49,6 @@
> #define KVM_S390_VIRTIO_RESET 1
> #define KVM_S390_VIRTIO_SET_STATUS 2
>
> -#define KERN_IMAGE_START0x01UL
> -#define KERN_PARM_AREA 0x010480UL
> -#define INITRD_START0x80UL
> -#define INITRD_PARM_START 0x010408UL
> -#define INITRD_PARM_SIZE0x010410UL
> -#define PARMFILE_START 0x001000UL
> -
> -#define ZIPL_START 0x009000UL
> -#define ZIPL_LOAD_ADDR 0x009000UL
> -#define ZIPL_FILENAME"s390-zipl.rom"
> -
> #define MAX_BLK_DEVS10
>
> static VirtIOS390Bus *s390_bus;
> @@ -185,6 +175,7 @@ static void s390_init(QEMUMachineInitArgs *args)
> /* get a BUS */
> s390_bus = s390_virtio_bus_init(&my_ram_size);
> s390_sclp_init();
> +s390_ipl_init();
>
> /* allocate RAM */
> memory_region_init_ram(ram, "s390.ram", my_ram_size);
> @@ -225,11 +216,7 @@ static void s390_init(QEMUMachineInitArgs *args)
> tmp_env->storage_keys = storage_keys;
> }
>
> -/* One CPU has to run */
> -s390_add_running_cpu(env);
> -
> if (kernel_filename) {
> -
> kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
>NULL, 1, ELF_MACHINE, 0);
> if (kernel_size == -1UL) {
> @@ -240,13 +227,6 @@ static void s390_init(QEMUMachineInitArgs *args)
> kernel_filename);
> exit(1);
> }
> -/*
> - * we can not rely on the ELF entry point, since up to 3.2 this
> - * value was 0x800 (the SALIPL loader) and it wont work. For
> - * all (Linux) cases 0x1 (KERN_IMAGE_START) should be fine.
> - */
> -env->psw.addr = KERN_IMAGE_START;
> -env->psw.mask = 0x00018000ULL;
> } else {
> ram_addr_t bios_size = 0;
> char *bios_filename;
> @@ -257,7 +237,7 @@ static void s390_init(QEMUMachineInitArgs *args)
> }
>
> bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> -bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
> +bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START,
> 4096);
> g_free(bios_filename);
>
> if ((long)bios_size < 0) {
> @@ -267,9 +247,6 @@ static void s390_init(QEMUMachineInitArgs *args)
> if (bios_size > 4096) {
> hw_error("stage1 bootloader is > 4k\n");
> }
> -
> -env->psw.addr = ZIPL_START;
> -env->psw.mask = 0x00018000ULL;
> }
>
> if (initrd_filename) {
> @@ -352,3 +329,4 @@ static void s390_machine_init(void)
> }
>
> machine_init(s390_machine_init);
> +
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 096dfcd..4a5a5d8 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> obj-y += sclpquiesce.o sclpconsole.o
> +obj-y += ipl.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> new file mode 100644
> index 000..1736fca
> --- /dev/null
> +++ b/hw/s390x/ipl.c
> @@ -0,0 +1,80 @@
> +/*
> + * bootloader support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Christian Borntraeger
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> your
>
[Qemu-devel] [PATCH 2/6] target-i386: sanitize AMD's ext2_features at realize time
when CPU properties are implemented, ext2_features may change
between object_new(CPU) and cpu_realize_fn(). Sanitizing
ext2_features for AMD based CPU at realize() time will keep
current behavior after CPU features are converted to properties.
Signed-off-by: Igor Mammedov
---
v2:
- style fix, make line shorter than 80 characters
amd san
---
target-i386/cpu.c | 21 +++--
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 63aae86..64b7637 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1539,16 +1539,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
"tsc-frequency", &error);
-/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
- * CPUID[1].EDX.
- */
-if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
-env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
-env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
-env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
-env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES);
-}
-
object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
if (error) {
fprintf(stderr, "%s\n", error_get_pretty(error));
@@ -2062,6 +2052,17 @@ void x86_cpu_realize(Object *obj, Error **errp)
env->cpuid_level = 7;
}
+/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
+ * CPUID[1].EDX.
+ */
+if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
+env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
+env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
+env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
+env->cpuid_ext2_features |= (env->cpuid_features
+ & CPUID_EXT2_AMD_ALIASES);
+}
+
if (!kvm_enabled()) {
env->cpuid_features &= TCG_FEATURES;
env->cpuid_ext_features &= TCG_EXT_FEATURES;
--
1.7.1
[Qemu-devel] [PATCH 4/6] target-i386: setting default 'vendor' is obsolete, remove it
since cpu_def config is not supported anymore and all remainig sources now
always set x86_def_t.vendor[123] fields remove setting default vendor to
simplify future refactoring.
Signed-off-by: Igor Mammedov
---
target-i386/cpu.c | 13 -
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1497980..99fd3f3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1539,15 +1539,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
if (cpu_x86_parse_featurestr(def, features) < 0) {
goto error;
}
-if (def->vendor1) {
-env->cpuid_vendor1 = def->vendor1;
-env->cpuid_vendor2 = def->vendor2;
-env->cpuid_vendor3 = def->vendor3;
-} else {
-env->cpuid_vendor1 = CPUID_VENDOR_INTEL_1;
-env->cpuid_vendor2 = CPUID_VENDOR_INTEL_2;
-env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
-}
+assert(def->vendor1);
+env->cpuid_vendor1 = def->vendor1;
+env->cpuid_vendor2 = def->vendor2;
+env->cpuid_vendor3 = def->vendor3;
env->cpuid_vendor_override = def->vendor_override;
object_property_set_int(OBJECT(cpu), def->level, "level", &error);
object_property_set_int(OBJECT(cpu), def->family, "family", &error);
--
1.7.1
Re: [Qemu-devel] [PATCH] Fix error code checking for SetFilePointer() call
Am 11.12.2012 10:21, schrieb Stefan Hajnoczi: > On Mon, Dec 10, 2012 at 12:56:22PM +0100, Fabien Chouteau wrote: >> An error has occurred if the return value is invalid_set_file_pointer >> and getlasterror doesn't return no_error. >> >> Signed-off-by: Fabien Chouteau >> --- >> block/raw-win32.c | 17 ++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) > > The fprintf() is kind of iffy but we only return -EIO so I guess it > helps to print the full error. > > Acked-by: Stefan Hajnoczi Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH 1/6] target-i386: filter out not TCG features if running without kvm at realize time
Signed-off-by: Igor Mammedov
Reviewed-by: Eduardo Habkost
Signed-off-by: Eduardo Habkost
---
target-i386/cpu.c | 31 ---
1 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7be3ad8..63aae86 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1549,21 +1549,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES);
}
-if (!kvm_enabled()) {
-env->cpuid_features &= TCG_FEATURES;
-env->cpuid_ext_features &= TCG_EXT_FEATURES;
-env->cpuid_ext2_features &= (TCG_EXT2_FEATURES
-#ifdef TARGET_X86_64
-| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
-#endif
-);
-env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
-env->cpuid_svm_features &= TCG_SVM_FEATURES;
-} else {
-#ifdef CONFIG_KVM
-filter_features_for_kvm(cpu);
-#endif
-}
object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
if (error) {
fprintf(stderr, "%s\n", error_get_pretty(error));
@@ -2077,6 +2062,22 @@ void x86_cpu_realize(Object *obj, Error **errp)
env->cpuid_level = 7;
}
+if (!kvm_enabled()) {
+env->cpuid_features &= TCG_FEATURES;
+env->cpuid_ext_features &= TCG_EXT_FEATURES;
+env->cpuid_ext2_features &= (TCG_EXT2_FEATURES
+#ifdef TARGET_X86_64
+| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
+#endif
+);
+env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
+env->cpuid_svm_features &= TCG_SVM_FEATURES;
+} else {
+#ifdef CONFIG_KVM
+filter_features_for_kvm(cpu);
+#endif
+}
+
#ifndef CONFIG_USER_ONLY
qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
--
1.7.1
Re: [Qemu-devel] [PATCH 8/8] s390: Add new channel I/O based virtio transport.
On 07.12.2012, at 13:50, Cornelia Huck wrote:
> Add a new virtio transport that uses channel commands to perform
> virtio operations.
>
> Add a new machine type s390-ccw that uses this virtio-ccw transport
> and make it the default machine for s390.
>
> Signed-off-by: Cornelia Huck
> ---
> hw/s390-virtio.c | 149 ++--
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/virtio-ccw.c | 909 +
> hw/s390x/virtio-ccw.h | 81 +
> trace-events | 4 +
> 5 files changed, 1124 insertions(+), 20 deletions(-)
> create mode 100644 hw/s390x/virtio-ccw.c
> create mode 100644 hw/s390x/virtio-ccw.h
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 9e1afb2..f29ff74 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -33,6 +33,8 @@
>
> #include "hw/s390-virtio-bus.h"
> #include "hw/s390x/sclp.h"
> +#include "hw/s390x/css.h"
> +#include "hw/s390x/virtio-ccw.h"
>
> //#define DEBUG_S390
>
> @@ -47,6 +49,7 @@
> #define KVM_S390_VIRTIO_NOTIFY 0
> #define KVM_S390_VIRTIO_RESET 1
> #define KVM_S390_VIRTIO_SET_STATUS 2
> +#define KVM_S390_VIRTIO_CCW_NOTIFY 3
>
> #define KERN_IMAGE_START0x01UL
> #define KERN_PARM_AREA 0x010480UL
> @@ -63,6 +66,7 @@
>
> static VirtIOS390Bus *s390_bus;
> static S390CPU **ipi_states;
> +VirtioCcwBus *ccw_bus;
>
> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> {
> @@ -76,15 +80,21 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t
> hypercall)
> {
> int r = 0, i;
> +int cssid, ssid, schid, m;
> +SubchDev *sch;
>
> dprintf("KVM hypercall: %ld\n", hypercall);
> switch (hypercall) {
> case KVM_S390_VIRTIO_NOTIFY:
> if (mem > ram_size) {
> -VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus,
> - mem, &i);
> -if (dev) {
> -virtio_queue_notify(dev->vdev, i);
> +if (s390_bus) {
> +VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus,
> + mem, &i);
> +if (dev) {
> +virtio_queue_notify(dev->vdev, i);
> +} else {
> +r = -EINVAL;
> +}
We really want to factor out the DIAG handling code similar to how spapr
handles its hypercalls. That way the legacy s390-virtio machine can register a
VIRTIO_NOTIFY hypercall that works for it here, while the s390-virtio-ccw
machine doesn't.
> } else {
> r = -EINVAL;
> }
> @@ -93,28 +103,49 @@ int s390_virtio_hypercall(CPUS390XState *env, uint64_t
> mem, uint64_t hypercall)
> }
> break;
> case KVM_S390_VIRTIO_RESET:
> -{
> -VirtIOS390Device *dev;
> -
> -dev = s390_virtio_bus_find_mem(s390_bus, mem);
> -virtio_reset(dev->vdev);
> -stb_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS, 0);
> -s390_virtio_device_sync(dev);
> -s390_virtio_reset_idx(dev);
> +if (s390_bus) {
> +VirtIOS390Device *dev;
> +
> +dev = s390_virtio_bus_find_mem(s390_bus, mem);
> +virtio_reset(dev->vdev);
> +stb_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS, 0);
> +s390_virtio_device_sync(dev);
> +s390_virtio_reset_idx(dev);
> +} else {
> +r = -EINVAL;
> +}
> break;
> -}
> case KVM_S390_VIRTIO_SET_STATUS:
> -{
> -VirtIOS390Device *dev;
> +if (s390_bus) {
> +VirtIOS390Device *dev;
>
> -dev = s390_virtio_bus_find_mem(s390_bus, mem);
> -if (dev) {
> -s390_virtio_device_update_status(dev);
> +dev = s390_virtio_bus_find_mem(s390_bus, mem);
> +if (dev) {
> +s390_virtio_device_update_status(dev);
> +} else {
> +r = -EINVAL;
> +}
> } else {
> r = -EINVAL;
> }
> break;
> -}
> +case KVM_S390_VIRTIO_CCW_NOTIFY:
> +if (ccw_bus) {
> +if (ioinst_disassemble_sch_ident(env->regs[2], &m, &cssid, &ssid,
> + &schid)) {
> +r = -EINVAL;
> +} else {
> +sch = css_find_subch(m, cssid, ssid, schid);
> +if (sch && css_subch_visible(sch)) {
> +virtio_queue_notify(virtio_ccw_get_vdev(sch),
> env->regs[3]);
> +} else {
> +r = -EINVAL;
> +}
> +}
> + } else {
> + r = -EINVAL;
> + }
> + break;
> default:
> r = -EINVAL;
> break;
> @@ -370,7 +401,6 @@ static QEMUMachine s390_machine = {
> .no_sdcard = 1,
> .
[Qemu-devel] [PATCH 6/6] target-i386: move out CPU features initialization in separate func
No functional change, a simple code movement to simplify following refactoring.
Signed-off-by: Igor Mammedov
---
v2:
- rebased on top of "i386: cpu: remove duplicate feature names"
http://www.mail-archive.com/qemu-devel@nongnu.org/msg129458.html
v3:
- rebased on top of 1.3-rc0 & splitted cpu_x86_find_by_name()
- AMD's ext2_features filtering are moved into separate patch
---
target-i386/cpu.c | 53 ++---
1 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e534254..3b9bbfe 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1235,6 +1235,34 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor
*v, void *opaque,
cpu->env.tsc_khz = value / 1000;
}
+static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
+{
+CPUX86State *env = &cpu->env;
+
+assert(def->vendor1);
+env->cpuid_vendor1 = def->vendor1;
+env->cpuid_vendor2 = def->vendor2;
+env->cpuid_vendor3 = def->vendor3;
+env->cpuid_vendor_override = def->vendor_override;
+object_property_set_int(OBJECT(cpu), def->level, "level", errp);
+object_property_set_int(OBJECT(cpu), def->family, "family", errp);
+object_property_set_int(OBJECT(cpu), def->model, "model", errp);
+object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
+env->cpuid_features = def->features;
+env->cpuid_ext_features = def->ext_features;
+env->cpuid_ext2_features = def->ext2_features;
+env->cpuid_ext3_features = def->ext3_features;
+object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
+env->cpuid_kvm_features = def->kvm_features;
+env->cpuid_svm_features = def->svm_features;
+env->cpuid_ext4_features = def->ext4_features;
+env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
+env->cpuid_xlevel2 = def->xlevel2;
+object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
+"tsc-frequency", errp);
+object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+}
+
static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
{
x86_def_t *def;
@@ -1513,7 +1541,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
{
-CPUX86State *env = &cpu->env;
x86_def_t def1, *def = &def1;
Error *error = NULL;
char *name, *features;
@@ -1541,29 +1568,9 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
if (cpu_x86_parse_featurestr(def, features) < 0) {
goto error;
}
-assert(def->vendor1);
-env->cpuid_vendor1 = def->vendor1;
-env->cpuid_vendor2 = def->vendor2;
-env->cpuid_vendor3 = def->vendor3;
-env->cpuid_vendor_override = def->vendor_override;
-object_property_set_int(OBJECT(cpu), def->level, "level", &error);
-object_property_set_int(OBJECT(cpu), def->family, "family", &error);
-object_property_set_int(OBJECT(cpu), def->model, "model", &error);
-object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
-env->cpuid_features = def->features;
-env->cpuid_ext_features = def->ext_features;
-env->cpuid_ext2_features = def->ext2_features;
-env->cpuid_ext3_features = def->ext3_features;
-object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
-env->cpuid_kvm_features = def->kvm_features;
-env->cpuid_svm_features = def->svm_features;
-env->cpuid_ext4_features = def->ext4_features;
-env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
-env->cpuid_xlevel2 = def->xlevel2;
-object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
-"tsc-frequency", &error);
-object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
+cpudef_2_x86_cpu(cpu, def, &error);
+
if (error) {
fprintf(stderr, "%s\n", error_get_pretty(error));
error_free(error);
--
1.7.1
Re: [Qemu-devel] [RFC/PATCH 0/2] ipl device followup
On 07.12.2012, at 16:14, Christian Borntraeger wrote: > Alex, > > here is were the IPL device code would move into. Some rough edges are still > there, but it can ipl almost anything that was zipled under LPAR/VM. Please try and check how nicely this can be done if we would map a firmware blob into the guest address space. I still believe that we can handle all boot strapping cases from within guest context. The great advantage would be that we get user interaction and consistent device state. And less code that runs under host privileges. Alex
Re: [Qemu-devel] [PATCH 1/3] s390: Fix empty kernel command line
On 11/12/12 11:34, Alexander Graf wrote:
>
> On 07.12.2012, at 14:55, Jens Freimann wrote:
>
>> From: Christian Borntraeger
>>
>> Since commit 967c0da73a7b0da186baba6632301d83644a570c
>>vl.c: Avoid segfault when started with no arguments
>>
>> the user can specify a kernel without a command line. Lets not
>> overwrite the default command line with \0 in that case.
>>
>> Signed-off-by: Christian Borntraeger
>> Signed-off-by: Jens Freimann
>> ---
>> hw/s390-virtio.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>> index ca1bb09..d77871a 100644
>> --- a/hw/s390-virtio.c
>> +++ b/hw/s390-virtio.c
>> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args)
>> stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
>> }
>>
>> -if (rom_ptr(KERN_PARM_AREA)) {
>> +if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) {
>
> why strlen()? If no -append option was passed, kernel_cmdline should be NULL.
> If -append "" was passed, the user wants to command line to be overwritten
> with "\0".
Nope. kernel_cmdline is always a valid pointer.
vl.c:
[..]
if (!kernel_cmdline) {
kernel_cmdline = "";
}
[..]
Re: [Qemu-devel] [PATCH 1/3] s390: Fix empty kernel command line
On 11.12.2012, at 12:15, Christian Borntraeger wrote:
> On 11/12/12 11:34, Alexander Graf wrote:
>>
>> On 07.12.2012, at 14:55, Jens Freimann wrote:
>>
>>> From: Christian Borntraeger
>>>
>>> Since commit 967c0da73a7b0da186baba6632301d83644a570c
>>> vl.c: Avoid segfault when started with no arguments
>>>
>>> the user can specify a kernel without a command line. Lets not
>>> overwrite the default command line with \0 in that case.
>>>
>>> Signed-off-by: Christian Borntraeger
>>> Signed-off-by: Jens Freimann
>>> ---
>>> hw/s390-virtio.c |2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>>> index ca1bb09..d77871a 100644
>>> --- a/hw/s390-virtio.c
>>> +++ b/hw/s390-virtio.c
>>> @@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args)
>>>stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
>>>}
>>>
>>> -if (rom_ptr(KERN_PARM_AREA)) {
>>> +if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) {
>>
>> why strlen()? If no -append option was passed, kernel_cmdline should be
>> NULL. If -append "" was passed, the user wants to command line to be
>> overwritten with "\0".
>
> Nope. kernel_cmdline is always a valid pointer.
>
> vl.c:
>
> [..]
>if (!kernel_cmdline) {
>kernel_cmdline = "";
>}
Then that's on purpose. Either we change the default for everyone or not at
all. But checking for "" only in the s390 machine sounds off.
Alex
Re: [Qemu-devel] [PATCH v7 00/10] i8254, i8259 and running Microport UNIX (ca 1987)
Matthew Ogilvie wrote:
> 2. Just fix it immediately, and don't worry about migration. Squash
>the last few patches together. A single missed periodic
>timer tick that only happens when migrating
>between versions of qemu is probably not a significant
>concern. (Unless someone knows of an OS that actually runs
>the i8254 in single shot mode 4, where a missed interrupt
>could cause a hang or something?)
Hi Matthew,
Such as Linux? 0x38 looks like mode 4 to me. I suspect it's used in
tickless mode when there isn't a better clock event source.
linux/drivers/clocksource/i8253.c:
#ifdef CONFIG_CLKEVT_I8253
/* ... */
case CLOCK_EVT_MODE_ONESHOT:
/* One shot setup */
outb_p(0x38, PIT_MODE);
/* ... */
/*
* Program the next event in oneshot mode
*
* Delta is given in PIT ticks
*/
static int pit_next_event(unsigned long delta, struct
clock_event_device *evt)
{
raw_spin_lock(&i8253_lock);
outb_p(delta & 0xff , PIT_CH0); /* LSB */
outb_p(delta >> 8 , PIT_CH0); /* MSB */
raw_spin_unlock(&i8253_lock);
return 0;
}
/* ... */
#endif
> 4. Support both old and fixed i8254 models, selectable at runtime
>with a command line option. (Question: What should such an
>option look like?) This may be the best way to actually
>change the 8254, but I'm not sure changes are even needed.
>It's certainly getting rather far afield from running Microport
>UNIX...
I can't see a reason to have the old behaviour, if every guest works
with the new one, except for this awkward cross-version migration
thing.
I guess ideally, device emulations would be versioned when their
behaviour changes, rather like shared libraries are, and the
appropriate old version kept around to be loaded for a particular
machine that's still running with it. Sounds a bit complicated though.
Best,
-- Jamie
[Qemu-devel] [Bug 1087974] Re: [regression] vnc tight png produces garbled output
** Attachment added: "screenshot of the issue" https://bugs.launchpad.net/qemu/+bug/1087974/+attachment/3457223/+files/tight-png-compression-issue.png -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1087974 Title: [regression] vnc tight png produces garbled output Status in QEMU: New Bug description: VNC Tight PNG compression did work fine two or three month ago but don't anymore. Now when Tight PNG is used parts of the desktop are shown but they are scrambled together. I have always tested this feature against QEMU git with noVNC by only allowing Tight PNG compression. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1087974/+subscriptions
Re: [Qemu-devel] [PATCH 1/3] s390: Fix empty kernel command line
On 11/12/12 12:19, Alexander Graf wrote:
>
> On 11.12.2012, at 12:15, Christian Borntraeger wrote:
>
>> On 11/12/12 11:34, Alexander Graf wrote:
>>>
>>> On 07.12.2012, at 14:55, Jens Freimann wrote:
>>>
From: Christian Borntraeger
Since commit 967c0da73a7b0da186baba6632301d83644a570c
vl.c: Avoid segfault when started with no arguments
the user can specify a kernel without a command line. Lets not
overwrite the default command line with \0 in that case.
Signed-off-by: Christian Borntraeger
Signed-off-by: Jens Freimann
---
hw/s390-virtio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index ca1bb09..d77871a 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -290,7 +290,7 @@ static void s390_init(QEMUMachineInitArgs *args)
stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
}
-if (rom_ptr(KERN_PARM_AREA)) {
+if (rom_ptr(KERN_PARM_AREA) && strlen(kernel_cmdline)) {
>>>
>>> why strlen()? If no -append option was passed, kernel_cmdline should be
>>> NULL. If -append "" was passed, the user wants to command line to be
>>> overwritten with "\0".
>>
>> Nope. kernel_cmdline is always a valid pointer.
>>
>> vl.c:
>>
>> [..]
>>if (!kernel_cmdline) {
>>kernel_cmdline = "";
>>}
>
> Then that's on purpose. Either we change the default for everyone or not at
> all. But checking for "" only in the s390 machine sounds off.
Ok, makes sense.
Re: [Qemu-devel] [RFC V3 01/24] qcow2: Add deduplication to the qcow2 specification.
On Mon, Nov 26, 2012 at 02:05:00PM +0100, Benoît Canet wrote: > Signed-off-by: Benoit Canet > --- > docs/specs/qcow2.txt | 33 - > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt > index 36a559d..16eafd7 100644 > --- a/docs/specs/qcow2.txt > +++ b/docs/specs/qcow2.txt > @@ -80,7 +80,10 @@ in the description of a field. > tables to repair refcounts before accessing > the > image. > > -Bits 1-63: Reserved (set to 0) > +Bit 1: Deduplication bit. If this bit is set then > +deduplication is used on this image. > + > +Bits 2-63: Reserved (set to 0) > > 80 - 87: compatible_features > Bitmask of compatible features. An implementation can This bit prevents programs that don't support dedup from opening the image file. What are the restrictions really - can a program without dedup support read the file? Can it write to the file (invalidating the dedup table)? > @@ -116,6 +119,7 @@ be stored. Each extension has a structure like the > following: > 0x - End of the header extension area > 0xE2792ACA - Backing file format name > 0x6803f857 - Feature name table > +0xCD8E819B - Deduplication > other - Unknown header extension, can be safely > ignored > > @@ -159,6 +163,33 @@ the header extension data. Each entry look like this: > terminated if it has full length) > > > +== Deduplication == > + > +The deduplication extension contains the offset and size of the deduplication > +table. > + > +Byte 0 - 7: Offset > + > + 8 - 11: Size Units? > + > +== Deduplication table == Before going into the layout please summarize the point of this table: The deduplication table maps a physical offset to a data hash and logical offset. ... > +The deduplication table contains 64 bits offsets to the level 2 deduplication > +table clusters. > +Each entry of these clusters contains a 32 bytes SHA256 hash followed by the > +64 bits logical offset of the first encountered block having this hash. At this point a diagram showing L1, L2, and dedup table entry would help. Or perhaps the entry structure can be presented like other structures in this spec to reduce the amount of English description and use a more formal reference: Each L2 deduplication table entry has the following structure: Byte 0 - 31: SHA256 hash of data cluster 32 - 39: Logical offset of first encountered block having this hash > +Entries in the deduplication table are orderered by physical cluster index. > + > +The number of entries in an l2 deduplication table cluster is : > +l2_dedup_cluster_entries = cluster_size / (32 + 8) > + > +The index in the level 1 deduplication table is : > +l1_dedup_index = physical_cluster_index / l2_dedup_cluster_entries > + > +The index in the level 2 deduplication table is: > +l2_dedup_index = physical_cluster_index % l2_dedup_cluster_entries > + > == Host cluster management == > > qcow2 manages the allocation of host clusters by maintaining a reference > count > -- > 1.7.10.4 > >
Re: [Qemu-devel] [RFC V3 01/24] qcow2: Add deduplication to the qcow2 specification.
On Mon, Nov 26, 2012 at 02:05:00PM +0100, Benoît Canet wrote: > +== Deduplication table == > + > +The deduplication table contains 64 bits offsets to the level 2 deduplication > +table clusters. > +Each entry of these clusters contains a 32 bytes SHA256 hash followed by the > +64 bits logical offset of the first encountered block having this hash. Can you foresee the need to use a different hash algorithm in the future and should we add a hash_algo enum field to the dedup QCOW2 header extension? Stefan
Re: [Qemu-devel] [RFC V3 02/24] qcow2: Add deduplication structures and fields.
On Mon, Nov 26, 2012 at 02:05:01PM +0100, Benoît Canet wrote:
> diff --git a/block/qcow2.h b/block/qcow2.h
> index b4eb654..e192001 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -58,6 +58,23 @@
>
> #define DEFAULT_CLUSTER_SIZE 65536
>
> +/* deduplication node */
> +typedef struct {
> +uint8_t *hash; /* 32 bytes hash of a given cluster */
Pointer to the hash value instead of storing the value inline? At this
point in the series I'm not sure yet why it's not stored inline. That
way we'd avoid a 4- or 8-byte pointer to a separately allocated 32-byte
blob. Maybe there is a reason later on...
Stefan
Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats
On Tue, 11 Dec 2012 05:12:29 + Dietmar Maurer wrote: > Can't we enable stat queries by default (10s interval), I'm not sure I like this for two reasons. First, there will be cases where the user doesn't want this to be enabled. Second, we'll be forcing an interval on users. > and simply return all stats with one API call (query-ballon)? We can't use query-balloon because this changes query-balloon from synchronous to asynchronous, and this is an incompatible change. Now, adding a new command to do this was my first proposal on this: http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg00983.html But Anthony suggested doing this through qom, which I agree that is a simpler & cleaner interface. > That qom-get interface forces me to do 6 API calls to get all information! Does this really matter for an application? > > > -Original Message- > > From: qemu-devel-bounces+dietmar=proxmox@nongnu.org > > [mailto:qemu-devel-bounces+dietmar=proxmox@nongnu.org] On > > Behalf Of Luiz Capitulino > > Sent: Montag, 10. Dezember 2012 20:36 > > To: qemu-devel@nongnu.org > > Cc: aligu...@us.ibm.com; mdr...@linux.vnet.ibm.com; a...@us.ibm.com > > Subject: [Qemu-devel] [PATCH 0/3] re-enable balloon stats > > > > This new try to re-enable the virtio-balloon driver stats uses QOM > > properties > > via a polling mechanism as suggested by Anthony here: > > > > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02390.html > > > > o Changes from the rfc > > > > - avoid balloon_stats_poll_cb() and virtio_balloon_handle_output() > >running in parallel > > - small renames and re-writes for better readability > > - update documentation > > > > Luiz Capitulino (3): > > virtio-balloon: drop old stats code > > virtio-balloon: re-enable balloon stats > > docs: document virtio-balloon stats > > > > docs/virtio-balloon-stats.txt | 87 +++ > > hw/virtio-balloon.c | 189 --- > > --- > > 2 files changed, 252 insertions(+), 24 deletions(-) create mode 100644 > > docs/virtio-balloon-stats.txt > > > > -- > > 1.8.0 > > > >
Re: [Qemu-devel] [PATCH] target-arm: fix target CPUs on ARM GIC
On 10 December 2012 03:32, wrote: > Fix a bug on the ARM GIC model where interrupts are not > set pending on the correct target CPUs when they are > triggered by writes to the Interrupt Set Enable or > Set Pending registers. > > Signed-off-by: Daniel Sangorrin Reviewed-by: Peter Maydell Thanks -- applied to the arm-devs.next tree. -- PMM
Re: [Qemu-devel] [RFC V3 03/24] qcow2: Add qcow2_dedup_read_missing_and_concatenate
On Mon, Nov 26, 2012 at 02:05:02PM +0100, Benoît Canet wrote:
> +/**
> + * Read some data from the QCOW2 file
> + *
> + * @data: the buffer where the data must be stored
> + * @sector_num: the sector number to read in the QCOW2 file
> + * @nb_sectors: the number of sectors to read
> + * @ret:negative on error
Dropping s->lock is important information to document - it means things
can change by the time this function returns and the caller needs to be
prepared.
Perhaps this function can be moved to qcow2.c and given a generic name.
It does nothing dedup-specific.
> + */
> +static int qcow2_dedup_read_missing_cluster_data(BlockDriverState *bs,
> + uint8_t *data,
> + uint64_t sector_num,
> + int nb_sectors)
> +{
> +BDRVQcowState *s = bs->opaque;
> +QEMUIOVector qiov;
> +struct iovec iov;
> +int ret;
> +
> +iov.iov_len = nb_sectors * BDRV_SECTOR_SIZE;
> +iov.iov_base = data;
> +qemu_iovec_init_external(&qiov, &iov, 1);
> +qemu_co_mutex_unlock(&s->lock);
> +ret = bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
> +qemu_co_mutex_lock(&s->lock);
> +if (ret < 0) {
> +error_report("failed to read %d sectors at offset %" PRIu64 "\n",
> + nb_sectors, sector_num);
> +}
> +
> +return ret;
> +}
> +
> +/*
> + * Prepare a buffer containing all the required data required to compute
> cluster
> + * sized deduplication hashes.
> + * If sector_num and nb_sectors are unaligned cluster wize it read the
> missing
> + * data before and after the qiov.
If sector_num or nb_sectors are not cluster-aligned, missing data
before/after the qiov will be read.
> + *
> + * @qiov: the qiov for which missing data must be read
> + * @sector_num: the first sectors that must be read into the qiov
> + * @nb_sectors: the number of sectors to read into the qiov
> + * @data: the place where the data will be concatenated and
> stored
> + * @nb_data_sectors:the resulting size of the contatenated data (in
> sectors)
> + * @ret:negative on error
> + */
> +int qcow2_dedup_read_missing_and_concatenate(BlockDriverState *bs,
> + QEMUIOVector *qiov,
> + uint64_t sector_num,
> + int nb_sectors,
> + uint8_t **data,
> + int *nb_data_sectors)
> +{
> +BDRVQcowState *s = bs->opaque;
> +int ret;
> +uint64_t cluster_beginning_sector;
> +uint64_t first_sector_after_qiov;
> +int cluster_beginning_nr;
> +int cluster_ending_nr;
> +int unaligned_ending_nr;
> +uint64_t max_cluster_ending_nr;
> +
> +/* compute how much and where to read at the beginning */
> +cluster_beginning_nr = sector_num & (s->cluster_sectors - 1);
> +cluster_beginning_sector = sector_num - cluster_beginning_nr;
> +
> +/* for the ending */
> +first_sector_after_qiov = sector_num + nb_sectors;
> +unaligned_ending_nr = first_sector_after_qiov & (s->cluster_sectors - 1);
> +cluster_ending_nr = unaligned_ending_nr ?
> +s->cluster_sectors - unaligned_ending_nr : 0;
> +
> +/* compute total size in sectors and allocate memory */
> +*nb_data_sectors = cluster_beginning_nr + nb_sectors + cluster_ending_nr;
> +*data = qemu_blockalign(bs, *nb_data_sectors * BDRV_SECTOR_SIZE);
> +memset(*data, 0, *nb_data_sectors * BDRV_SECTOR_SIZE);
Is memset necessary since we either read all data or return an error?
> +/* read beginning */
> +if (cluster_beginning_nr) {
> +ret = qcow2_dedup_read_missing_cluster_data(bs,
> +*data,
> +cluster_beginning_sector,
> +cluster_beginning_nr);
> +
> +if (ret < 0) {
> +goto fail;
> +}
> +}
> +
> +/* append qiov content */
> +qemu_iovec_to_buf(qiov, 0, *data + cluster_beginning_nr *
> BDRV_SECTOR_SIZE,
> + qiov->size);
> +
> +/* Fix cluster_ending_nr if we are at risk of reading outside the image
> + * (Cluster unaligned image size)
> + */
> +max_cluster_ending_nr = bs->total_sectors - first_sector_after_qiov;
> +cluster_ending_nr = max_cluster_ending_nr < (uint64_t) cluster_ending_nr
> ?
> +(int) max_cluster_ending_nr : cluster_ending_nr;
> +
> +/* read and add ending */
> +if (cluster_ending_nr) {
> +ret = qcow2_dedup_read_missing_cluster_data(bs,
> +*data +
> +(cluster_be
Re: [Qemu-devel] [PATCH v2] exynos4210/mct: Avoid infinite loop on non incremental timers
On 3 December 2012 22:55, Jean-Christophe DUBOIS wrote: > Check for a 0 "distance" value to avoid infinite loop when the > expired FCR timer was not programed with auto-increment. > > With this change the behavior is coherent with the same type > of code in the exynos4210_gfrc_restart() function in the same > file. > > Linux seems to mostly use this timer with auto-increment > which explain why it is not a problem most of the time. > > However other OS might have a problem with this if they > don't use the auto-increment feature. > > Signed-off-by: Jean-Christophe DUBOIS Thanks, applied to arm-devs.next. -- PMM
Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats
> > Can't we enable stat queries by default (10s interval), > > I'm not sure I like this for two reasons. First, there will be cases where the > user doesn't want this to be enabled. Second, we'll be forcing an interval on > users. OK. > > and simply return all stats with one API call (query-ballon)? > > We can't use query-balloon because this changes query-balloon from > synchronous to asynchronous, and this is an incompatible change. Why don't we simply cache the last received stats, and query-ballon returns the cached values?
Re: [Qemu-devel] [PATCH 8/8] s390: Add new channel I/O based virtio transport.
On 11/12/12 11:53, Alexander Graf wrote:
>
> On 07.12.2012, at 13:50, Cornelia Huck wrote:
>
>> Add a new virtio transport that uses channel commands to perform
>> virtio operations.
>>
>> Add a new machine type s390-ccw that uses this virtio-ccw transport
>> and make it the default machine for s390.
>>
>> Signed-off-by: Cornelia Huck
>> ---
>> hw/s390-virtio.c | 149 ++--
>> hw/s390x/Makefile.objs | 1 +
>> hw/s390x/virtio-ccw.c | 909
>> +
>> hw/s390x/virtio-ccw.h | 81 +
>> trace-events | 4 +
>> 5 files changed, 1124 insertions(+), 20 deletions(-)
>> create mode 100644 hw/s390x/virtio-ccw.c
>> create mode 100644 hw/s390x/virtio-ccw.h
>>
>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>> index 9e1afb2..f29ff74 100644
>> --- a/hw/s390-virtio.c
>> +++ b/hw/s390-virtio.c
>> @@ -33,6 +33,8 @@
>>
>> #include "hw/s390-virtio-bus.h"
>> #include "hw/s390x/sclp.h"
>> +#include "hw/s390x/css.h"
>> +#include "hw/s390x/virtio-ccw.h"
>>
>> //#define DEBUG_S390
>>
>> @@ -47,6 +49,7 @@
>> #define KVM_S390_VIRTIO_NOTIFY 0
>> #define KVM_S390_VIRTIO_RESET 1
>> #define KVM_S390_VIRTIO_SET_STATUS 2
>> +#define KVM_S390_VIRTIO_CCW_NOTIFY 3
>>
>> #define KERN_IMAGE_START0x01UL
>> #define KERN_PARM_AREA 0x010480UL
>> @@ -63,6 +66,7 @@
>>
>> static VirtIOS390Bus *s390_bus;
>> static S390CPU **ipi_states;
>> +VirtioCcwBus *ccw_bus;
>>
>> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>> {
>> @@ -76,15 +80,21 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>> int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t
>> hypercall)
>> {
>> int r = 0, i;
>> +int cssid, ssid, schid, m;
>> +SubchDev *sch;
>>
>> dprintf("KVM hypercall: %ld\n", hypercall);
>> switch (hypercall) {
>> case KVM_S390_VIRTIO_NOTIFY:
>> if (mem > ram_size) {
>> -VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus,
>> - mem, &i);
>> -if (dev) {
>> -virtio_queue_notify(dev->vdev, i);
>> +if (s390_bus) {
>> +VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus,
>> + mem, &i);
>> +if (dev) {
>> +virtio_queue_notify(dev->vdev, i);
>> +} else {
>> +r = -EINVAL;
>> +}
>
> We really want to factor out the DIAG handling code similar to how spapr
> handles its hypercalls. That way the legacy s390-virtio machine can register
> a VIRTIO_NOTIFY hypercall that works for it here, while the s390-virtio-ccw
> machine doesn't.
>
Agreed, but this has nothing to do with virtio-ccw and should be part of a
follow-up cleanup. no?
Christian
Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats
On Tue, 11 Dec 2012 12:05:32 + Dietmar Maurer wrote: > > > and simply return all stats with one API call (query-ballon)? > > > > We can't use query-balloon because this changes query-balloon from > > synchronous to asynchronous, and this is an incompatible change. > > Why don't we simply cache the last received stats, and query-ballon returns > the cached values? Giving that we're doing this through device properties, we'd duplicate the interface by also returning this info in query-balloon. However, it wouldn't be a problem to extend 'info balloon' (in HMP) with that info. That is, 'info balloon' could use the QOM interface to query balloon stats.
Re: [Qemu-devel] [PATCH 3/8] s390: I/O interrupt and machine check injection.
On Mon, 10 Dec 2012 18:26:28 -0600 Rob Landley wrote: > What do you actually use to run Linux under this target? There are some > leads at > http://virtuallyfun.superglobalmegacorp.com/?p=1206 which more or less > leads to > http://ftp.nl.debian.org/debian/dists/Debian6.0.6/main/installer-s390/current/images/generic/ > > but I dunno what qemu command line would go along with those files... > > Any hints? The easiest way is probably to use an existing Linux/390 installation on a disk attached to your system and an external monolithic kernel containing the virtio-ccw guest support patches and the virtio drivers: s390x-softmmu/qemu-system-s390x -machine s390-ccw -m 1024 -smp 2 -enable-kvm -kernel /path/to/your/kernel -drive file=/dev/sda,if=none,media=disk,id=drive-virtio-disk0 -device virtio-blk-ccw,devno=fe.0.0815,drive=drive-virtio-disk0,id=virtio-disk0 -append "root=/dev/vda1" -nographic
Re: [Qemu-devel] [PATCH v6] Add compare subcommand for qemu-img
Am 06.12.2012 13:24, schrieb Miroslav Rezanina:
> This is second version of patch adding compare subcommand that
> compares two images. Compare has following criteria:
> - only data part is compared
> - unallocated sectors are not read
> - in case of different image size, exceeding part of bigger disk has
> to be zeroed/unallocated to compare rest
> - qemu-img returns:
> - 0 if images are identical
> - 1 if images differ
> - 2 on error
>
> v6:
> - added handling -?, -h options for compare subcommand
>
> v5 (only minor changes):
> - removed redundant comment
> - removed dead code (goto after help())
> - set final total_sectors on first assignment
>
> v4:
> - Fixed various typos
> - Added functions for empty sector check and sector-to-bytes offset
> conversion
> - Fixed command-line parameters processing
>
> v3:
> - options -f/-F are orthogonal
> - documentation updated to new syntax and behavior
> - used byte offset instead of sector number for output
>
> v2:
> - changed option for second image format to -F
> - changed handling of -f and -F [1]
> - added strict mode (-s)
> - added quiet mode (-q)
> - improved output messages [2]
> - rename variables for larger image handling
> - added man page content
>
> Signed-off-by: Miroslav Rezanina
Please move the changelog below a "---" line so that git am ignores it
for the final commit message.
> diff --git a/block.c b/block.c
> index 854ebd6..fdc8c45 100644
> --- a/block.c
> +++ b/block.c
> @@ -2698,6 +2698,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
>
> typedef struct BdrvCoIsAllocatedData {
> BlockDriverState *bs;
> +BlockDriverState *base;
> int64_t sector_num;
> int nb_sectors;
> int *pnum;
> @@ -2828,6 +2829,44 @@ int coroutine_fn
> bdrv_co_is_allocated_above(BlockDriverState *top,
> return 0;
> }
>
> +/* Coroutine wrapper for bdrv_is_allocated_above() */
> +static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
> +{
> +BdrvCoIsAllocatedData *data = opaque;
> +BlockDriverState *top = data->bs;
> +BlockDriverState *base = data->base;
> +
> +data->ret = bdrv_co_is_allocated_above(top, base, data->sector_num,
> + data->nb_sectors, data->pnum);
> +data->done = true;
> +}
> +
> +/*
> + * Synchronous wrapper around bdrv_co_is_allocated_above().
> + *
> + * See bdrv_co_is_allocated_above() for details.
> + */
> +int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
> + int64_t sector_num, int nb_sectors, int *pnum)
> +{
> +Coroutine *co;
> +BdrvCoIsAllocatedData data = {
> +.bs = top,
> +.base = base,
> +.sector_num = sector_num,
> +.nb_sectors = nb_sectors,
> +.pnum = pnum,
> +.done = false,
> +};
> +
> +co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
> +qemu_coroutine_enter(co, &data);
> +while (!data.done) {
> +qemu_aio_wait();
> +}
> +return data.ret;
> +}
> +
> BlockInfo *bdrv_query_info(BlockDriverState *bs)
> {
> BlockInfo *info = g_malloc0(sizeof(*info));
> diff --git a/block.h b/block.h
> index 722c620..2cb8d71 100644
> --- a/block.h
> +++ b/block.h
> @@ -278,6 +278,8 @@ int bdrv_co_discard(BlockDriverState *bs, int64_t
> sector_num, int nb_sectors);
> int bdrv_has_zero_init(BlockDriverState *bs);
> int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int
> nb_sectors,
>int *pnum);
> +int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
> +int64_t sector_num, int nb_sectors, int *pnum);
>
> void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
> BlockdevOnError on_write_error);
This first part looks good. I think it could be a separate patch, however.
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index a181363..c076085 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -27,6 +27,12 @@ STEXI
> @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
> ETEXI
>
> +DEF("compare", img_compare,
> +"compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
> +STEXI
> +@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1}
> @var{filename2}
> +ETEXI
> +
> DEF("convert", img_convert,
> "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s
> snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
> STEXI
> diff --git a/qemu-img.c b/qemu-img.c
> index e29e01b..09abb3a 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -101,7 +101,13 @@ static void help(void)
> " '-a' applies a snapshot (revert disk to saved state)\n"
> " '-c' creates a snapshot\n"
> " '-d' deletes a snapshot\n"
> - " '-l' lists all snapshots in the given image\n";
> + " '-l' lists all snapshot
Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats
> > Can't we enable stat queries by default (10s interval), > > I'm not sure I like this for two reasons. First, there will be cases where the > user doesn't want this to be enabled. Second, we'll be forcing an interval on > users. So when should we set the stats-polling-interval? I first thought I simply set that at VM start, but that triggers an error: "guest doesn't support balloon stats" because the balloon driver is not loaded.
[Qemu-devel] [PULL 0/2] migration queue
Hi Anthony, this fixes two errors on the migration code, could you pull? Thanks, Juan. The following changes since commit 1c97e303d4ea80a2691334b0febe87a50660f99d: Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2012-12-10 08:35:15 -0600) are available in the git repository at: git://repo.or.cz/qemu/quintela.git migration.next for you to fetch changes up to 77db8657048f233edf21e1a9ebdc30a367fbdc36: migration: Fix madvise breakage if host and guest have different page sizes (2012-12-11 12:45:56 +0100) David Gibson (2): Fix off-by-1 error in RAM migration code migration: Fix madvise breakage if host and guest have different page sizes arch_init.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH 2/2] migration: Fix madvise breakage if host and guest have different page sizes
From: David Gibson
madvise(DONTNEED) will throw away the contents of the whole page at the
given address, even if the given length is less than the page size. One
can argue about whether that's the correct behaviour, but that's what it's
done for a long time in Linux at least.
That means that the madvise() in ram_load(), on a setup where
TARGET_PAGE_SIZE is smaller than the host page size, can throw away data
in guest pages adjacent to the one it's actually processing right now,
leading to guest memory corruption on an incoming migration.
This patch therefore, disables the madvise() if the host page size is
larger than TARGET_PAGE_SIZE. This means we don't get the benefits of that
madvise() in this case, but a more complete fix is more difficult to
accomplish. This at least fixes the guest memory corruption.
Signed-off-by: David Gibson
Reported-by: Alexey Kardashevskiy
Signed-off-by: Juan Quintela
---
arch_init.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch_init.c b/arch_init.c
index b75a4c5..83dcc53 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -840,7 +840,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
memset(host, ch, TARGET_PAGE_SIZE);
#ifndef _WIN32
if (ch == 0 &&
-(!kvm_enabled() || kvm_has_sync_mmu())) {
+(!kvm_enabled() || kvm_has_sync_mmu()) &&
+getpagesize() <= TARGET_PAGE_SIZE) {
qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
}
#endif
--
1.7.11.7
[Qemu-devel] [PATCH 1/2] Fix off-by-1 error in RAM migration code
From: David Gibson The code for migrating (or savevm-ing) memory pages starts off by creating a dirty bitmap and filling it with 1s. Except, actually, because bit addresses are 0-based it fills every bit except bit 0 with 1s and puts an extra 1 beyond the end of the bitmap, potentially corrupting unrelated memory. Oops. This patch fixes it. Signed-off-by: David Gibson Signed-off-by: Juan Quintela --- arch_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index e6effe8..b75a4c5 100644 --- a/arch_init.c +++ b/arch_init.c @@ -568,7 +568,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque) int64_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS; migration_bitmap = bitmap_new(ram_pages); -bitmap_set(migration_bitmap, 1, ram_pages); +bitmap_set(migration_bitmap, 0, ram_pages); migration_dirty_pages = ram_pages; bytes_transferred = 0; -- 1.7.11.7
[Qemu-devel] [PATCH 07/35] migration: make writes blocking
Move all the writes to the migration_thread, and make writings
blocking. Notice that are still using the iothread for everything
that we do.
Signed-off-by: Juan Quintela
---
migration-exec.c | 1 -
migration-fd.c | 1 -
migration-tcp.c | 1 +
migration-unix.c | 1 +
migration.c | 17 -
qemu-file.h | 5 -
savevm.c | 5 -
7 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/migration-exec.c b/migration-exec.c
index 2b6fcb4..df76606 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -70,7 +70,6 @@ void exec_start_outgoing_migration(MigrationState *s, const
char *command, Error
s->fd = fileno(f);
assert(s->fd != -1);
-socket_set_nonblock(s->fd);
s->opaque = qemu_popen(f, "w");
diff --git a/migration-fd.c b/migration-fd.c
index 5fe28e0..a161338 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -77,7 +77,6 @@ void fd_start_outgoing_migration(MigrationState *s, const
char *fdname, Error **
return;
}
-fcntl(s->fd, F_SETFL, O_NONBLOCK);
s->get_error = fd_errno;
s->write = fd_write;
s->close = fd_close;
diff --git a/migration-tcp.c b/migration-tcp.c
index 5e855fe..c0256ec 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -61,6 +61,7 @@ static void tcp_wait_for_connect(int fd, void *opaque)
} else {
DPRINTF("migrate connect success\n");
s->fd = fd;
+socket_set_block(s->fd);
migrate_fd_connect(s);
}
}
diff --git a/migration-unix.c b/migration-unix.c
index dba72b4..536cec8 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -61,6 +61,7 @@ static void unix_wait_for_connect(int fd, void *opaque)
} else {
DPRINTF("migrate connect success\n");
s->fd = fd;
+socket_set_block(s->fd);
migrate_fd_connect(s);
}
}
diff --git a/migration.c b/migration.c
index 9c34627..17ee872 100644
--- a/migration.c
+++ b/migration.c
@@ -297,18 +297,6 @@ static void migrate_fd_completed(MigrationState *s)
notifier_list_notify(&migration_state_notifiers, s);
}
-static void migrate_fd_put_notify(void *opaque)
-{
-MigrationState *s = opaque;
-int ret;
-
-qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-ret = qemu_file_put_notify(s->file);
-if (ret) {
-migrate_fd_error(s);
-}
-}
-
ssize_t migrate_fd_put_buffer(MigrationState *s, const void *data,
size_t size)
{
@@ -325,10 +313,6 @@ ssize_t migrate_fd_put_buffer(MigrationState *s, const
void *data,
if (ret == -1)
ret = -(s->get_error(s));
-if (ret == -EAGAIN) {
-qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
-}
-
return ret;
}
@@ -426,7 +410,6 @@ int migrate_fd_close(MigrationState *s)
{
int rc = 0;
if (s->fd != -1) {
-qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
rc = s->close(s);
s->fd = -1;
}
diff --git a/qemu-file.h b/qemu-file.h
index d64bdbb..68deefb 100644
--- a/qemu-file.h
+++ b/qemu-file.h
@@ -113,11 +113,6 @@ int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t
new_rate);
int64_t qemu_file_get_rate_limit(QEMUFile *f);
int qemu_file_get_error(QEMUFile *f);
-/* Try to send any outstanding data. This function is useful when output is
- * halted due to rate limiting or EAGAIN errors occur as it can be used to
- * resume output. */
-int qemu_file_put_notify(QEMUFile *f);
-
static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
{
qemu_put_be64(f, *pv);
diff --git a/savevm.c b/savevm.c
index 5d04d59..c4ee899 100644
--- a/savevm.c
+++ b/savevm.c
@@ -556,11 +556,6 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
-int qemu_file_put_notify(QEMUFile *f)
-{
-return f->ops->put_buffer(f->opaque, NULL, 0, 0);
-}
-
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.11.7
[Qemu-devel] [PATCH 15/35] migration-fd: remove duplicate include
Signed-off-by: Juan Quintela --- migration-fd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/migration-fd.c b/migration-fd.c index 487b17e..77aef6d 100644 --- a/migration-fd.c +++ b/migration-fd.c @@ -20,7 +20,6 @@ #include "qemu-char.h" #include "qemu-file.h" #include "block.h" -#include "qemu_socket.h" //#define DEBUG_MIGRATION_FD -- 1.7.11.7
[Qemu-devel] [PATCH 35/35] migration: print times for end phase
Signed-off-by: Juan Quintela
---
block.c | 6 ++
cpus.c | 17 +
migration.c | 12
savevm.c| 13 +
4 files changed, 48 insertions(+)
diff --git a/block.c b/block.c
index c05875f..2fa0827 100644
--- a/block.c
+++ b/block.c
@@ -2680,9 +2680,15 @@ int bdrv_get_flags(BlockDriverState *bs)
void bdrv_flush_all(void)
{
BlockDriverState *bs;
+int64_t start_time, end_time;
+
+start_time = qemu_get_clock_ms(rt_clock);
QTAILQ_FOREACH(bs, &bdrv_states, list) {
bdrv_flush(bs);
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time flush device %s: %ld\n", bs->filename,
+ end_time - start_time);
}
}
diff --git a/cpus.c b/cpus.c
index d9c332f..a920a06 100644
--- a/cpus.c
+++ b/cpus.c
@@ -437,14 +437,31 @@ bool cpu_is_stopped(CPUState *cpu)
static void do_vm_stop(RunState state)
{
+int64_t start_time, end_time;
+
if (runstate_is_running()) {
+start_time = qemu_get_clock_ms(rt_clock);
cpu_disable_ticks();
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time cpu_disable_ticks %ld\n", end_time - start_time);
pause_all_vcpus();
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time pause_all_vcpus %ld\n", end_time - start_time);
runstate_set(state);
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time runstate_set %ld\n", end_time - start_time);
vm_state_notify(0, state);
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time vmstate_notify %ld\n", end_time - start_time);
bdrv_drain_all();
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time bdrv_drain_all %ld\n", end_time - start_time);
bdrv_flush_all();
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time bdrv_flush_all %ld\n", end_time - start_time);
monitor_protocol_event(QEVENT_STOP, NULL);
+end_time = qemu_get_clock_ms(rt_clock);
+printf("time monitor_protocol_event %ld\n", end_time - start_time);
}
}
diff --git a/migration.c b/migration.c
index dd6a401..fc0548c 100644
--- a/migration.c
+++ b/migration.c
@@ -723,12 +723,17 @@ static void *buffered_file_thread(void *opaque)
DPRINTF("done iterating\n");
start_time = qemu_get_clock_ms(rt_clock);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+end_time = qemu_get_clock_ms(rt_clock);
+printf("wakeup_request %ld\n", end_time - start_time);
if (old_vm_running) {
vm_stop(RUN_STATE_FINISH_MIGRATE);
} else {
vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
}
+end_time = qemu_get_clock_ms(rt_clock);
+printf("vm_stop %ld\n", end_time - start_time);
+
/* 8 is the size of an end of section mark, so empty section */
while ((ret = qemu_savevm_state_iterate(m->file, free_space))
> 8) {
@@ -739,15 +744,20 @@ static void *buffered_file_thread(void *opaque)
}
free_space = s->buffer_capacity - s->buffer_size;
}
+end_time = qemu_get_clock_ms(rt_clock);
+printf("iterate phase %ld\n", end_time - start_time);
ret = qemu_savevm_state_complete(m->file);
if (ret < 0) {
qemu_mutex_unlock_iothread();
break;
} else {
+end_time = qemu_get_clock_ms(rt_clock);
+printf("complete without error 3a %ld\n", end_time -
start_time);
migrate_fd_completed(m);
}
end_time = qemu_get_clock_ms(rt_clock);
+printf("completed %ld\n", end_time - start_time);
m->total_time = end_time - m->total_time;
m->downtime = end_time - start_time;
if (m->state != MIG_STATE_COMPLETED) {
@@ -755,6 +765,8 @@ static void *buffered_file_thread(void *opaque)
vm_start();
}
}
+end_time = qemu_get_clock_ms(rt_clock);
+printf("end completed stage %ld\n", end_time - start_time);
last_round = true;
}
}
diff --git a/savevm.c b/savevm.c
index 4b7715a..c964b66 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1706,9 +1706,14 @@ int qemu_savevm_state_complete(QEMUFile *f)
{
SaveStateEntry *se;
int ret;
+int64_t t1;
+int64_t t0 = qemu_get_clock_ms(rt_clock);
cpu_synchronize_all_states();
+t1 = qemu_get_clock_ms(rt_clock);
+printf("synchronize_all_states %ld\n", t1 - t0);
+t0 = t1;
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
if (!se->ops || !se->ops->save_live_complete) {
co
[Qemu-devel] [PATCH 16/35] migration: move buffered_file.c code into migration.c
This only moves the code (also from buffered_file.h to migration.h).
Fix whitespace until checkpatch is happy.
Signed-off-by: Juan Quintela
---
Makefile.objs | 2 +-
buffered_file.c | 259
buffered_file.h | 22 -
migration.c | 233 +-
migration.h | 1 +
5 files changed, 234 insertions(+), 283 deletions(-)
delete mode 100644 buffered_file.c
delete mode 100644 buffered_file.h
diff --git a/Makefile.objs b/Makefile.objs
index 3c7abca..f0309ac 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -79,7 +79,7 @@ extra-obj-$(CONFIG_LINUX) += fsdev/
common-obj-y += tcg-runtime.o host-utils.o main-loop.o
common-obj-y += input.o
-common-obj-y += buffered_file.o migration.o migration-tcp.o
+common-obj-y += migration.o migration-tcp.o
common-obj-y += qemu-char.o #aio.o
common-obj-y += block-migration.o iohandler.o
common-obj-y += bitmap.o bitops.o
diff --git a/buffered_file.c b/buffered_file.c
deleted file mode 100644
index a6d4bd8..000
--- a/buffered_file.c
+++ /dev/null
@@ -1,259 +0,0 @@
-/*
- * QEMU buffered QEMUFile
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU GPL, version 2. See
- * the COPYING file in the top-level directory.
- *
- * Contributions after 2012-01-13 are licensed under the terms of the
- * GNU GPL, version 2 or (at your option) any later version.
- */
-
-#include "qemu-common.h"
-#include "hw/hw.h"
-#include "qemu-timer.h"
-#include "qemu-char.h"
-#include "buffered_file.h"
-#include "qemu-thread.h"
-
-//#define DEBUG_BUFFERED_FILE
-
-typedef struct QEMUFileBuffered
-{
-MigrationState *migration_state;
-QEMUFile *file;
-size_t bytes_xfer;
-size_t xfer_limit;
-uint8_t *buffer;
-size_t buffer_size;
-size_t buffer_capacity;
-QemuThread thread;
-} QEMUFileBuffered;
-
-#ifdef DEBUG_BUFFERED_FILE
-#define DPRINTF(fmt, ...) \
-do { printf("buffered-file: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do { } while (0)
-#endif
-
-static ssize_t buffered_flush(QEMUFileBuffered *s)
-{
-size_t offset = 0;
-ssize_t ret = 0;
-
-DPRINTF("flushing %zu byte(s) of data\n", s->buffer_size);
-
-while (s->bytes_xfer < s->xfer_limit && offset < s->buffer_size) {
-
-ret = migrate_fd_put_buffer(s->migration_state, s->buffer + offset,
-s->buffer_size - offset);
-if (ret <= 0) {
-DPRINTF("error flushing data, %zd\n", ret);
-break;
-} else {
-DPRINTF("flushed %zd byte(s)\n", ret);
-offset += ret;
-s->bytes_xfer += ret;
-}
-}
-
-DPRINTF("flushed %zu of %zu byte(s)\n", offset, s->buffer_size);
-memmove(s->buffer, s->buffer + offset, s->buffer_size - offset);
-s->buffer_size -= offset;
-
-if (ret < 0) {
-return ret;
-}
-return offset;
-}
-
-static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
int size)
-{
-QEMUFileBuffered *s = opaque;
-ssize_t error;
-
-DPRINTF("putting %d bytes at %" PRId64 "\n", size, pos);
-
-error = qemu_file_get_error(s->file);
-if (error) {
-DPRINTF("flush when error, bailing: %s\n", strerror(-error));
-return error;
-}
-
-if (size <= 0) {
-return size;
-}
-
-if (size > (s->buffer_capacity - s->buffer_size)) {
-DPRINTF("increasing buffer capacity from %zu by %zu\n",
-s->buffer_capacity, size + 1024);
-
-s->buffer_capacity += size + 1024;
-
-s->buffer = g_realloc(s->buffer, s->buffer_capacity);
-}
-
-memcpy(s->buffer + s->buffer_size, buf, size);
-s->buffer_size += size;
-
-return size;
-}
-
-static int buffered_close(void *opaque)
-{
-QEMUFileBuffered *s = opaque;
-ssize_t ret = 0;
-int ret2;
-
-DPRINTF("closing\n");
-
-s->xfer_limit = INT_MAX;
-while (!qemu_file_get_error(s->file) && s->buffer_size) {
-ret = buffered_flush(s);
-if (ret < 0) {
-break;
-}
-}
-
-ret2 = migrate_fd_close(s->migration_state);
-if (ret >= 0) {
-ret = ret2;
-}
-ret = migrate_fd_close(s->migration_state);
-s->migration_state->complete = true;
-return ret;
-}
-
-/*
- * The meaning of the return values is:
- * 0: We can continue sending
- * 1: Time to stop
- * negative: There has been an error
- */
-static int buffered_get_fd(void *opaque)
-{
-QEMUFileBuffered *s = opaque;
-
-return qemu_get_fd(s->file);
-}
-
-static int buffered_rate_limit(void *opaque)
-{
-QEMUFileBuffered *s = opaque;
-int ret;
-
-ret = qemu_file_get_error(s->file);
-if (ret) {
-return ret;
-}
-
-if (s->bytes_xfer > s->xfer_limit)
-return 1;
-
-return 0;
-}
-
-static int64_t buffered_set_rate_limit(void *opaqu
[Qemu-devel] [PATCH 23/35] migration: unfold rest of migrate_fd_put_ready() into thread
This will allow us finer control in next patches.
Signed-off-by: Juan Quintela
---
migration.c | 95 ++---
1 file changed, 41 insertions(+), 54 deletions(-)
diff --git a/migration.c b/migration.c
index 5c2f413..7a4a6f0 100644
--- a/migration.c
+++ b/migration.c
@@ -669,54 +669,6 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
-{
-int ret;
-uint64_t pending_size;
-bool last_round = false;
-
-qemu_mutex_lock_iothread();
-DPRINTF("iterate\n");
-pending_size = qemu_savevm_state_pending(s->file, max_size);
-DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
-if (pending_size >= max_size) {
-ret = qemu_savevm_state_iterate(s->file);
-if (ret < 0) {
-migrate_fd_error(s);
-}
-} else {
-int old_vm_running = runstate_is_running();
-int64_t start_time, end_time;
-
-DPRINTF("done iterating\n");
-start_time = qemu_get_clock_ms(rt_clock);
-qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
-if (old_vm_running) {
-vm_stop(RUN_STATE_FINISH_MIGRATE);
-} else {
-vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
-}
-
-if (qemu_savevm_state_complete(s->file) < 0) {
-migrate_fd_error(s);
-} else {
-migrate_fd_completed(s);
-}
-end_time = qemu_get_clock_ms(rt_clock);
-s->total_time = end_time - s->total_time;
-s->downtime = end_time - start_time;
-if (s->state != MIG_STATE_COMPLETED) {
-if (old_vm_running) {
-vm_start();
-}
-}
-last_round = true;
-}
-qemu_mutex_unlock_iothread();
-
-return last_round;
-}
-
/* 100ms xfer_limit is the limit that we should write each 100ms */
#define BUFFER_DELAY 100
@@ -741,6 +693,7 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
+uint64_t pending_size;
qemu_mutex_lock_iothread();
if (m->state != MIG_STATE_ACTIVE) {
@@ -752,6 +705,46 @@ static void *buffered_file_thread(void *opaque)
qemu_mutex_unlock_iothread();
break;
}
+if (s->bytes_xfer < s->xfer_limit) {
+DPRINTF("iterate\n");
+pending_size = qemu_savevm_state_pending(m->file, max_size);
+DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
+if (pending_size >= max_size) {
+ret = qemu_savevm_state_iterate(m->file);
+if (ret < 0) {
+qemu_mutex_unlock_iothread();
+break;
+}
+} else {
+int old_vm_running = runstate_is_running();
+int64_t start_time, end_time;
+
+DPRINTF("done iterating\n");
+start_time = qemu_get_clock_ms(rt_clock);
+qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+if (old_vm_running) {
+vm_stop(RUN_STATE_FINISH_MIGRATE);
+} else {
+vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+}
+ret = qemu_savevm_state_complete(m->file);
+if (ret < 0) {
+qemu_mutex_unlock_iothread();
+break;
+} else {
+migrate_fd_completed(m);
+}
+end_time = qemu_get_clock_ms(rt_clock);
+m->total_time = end_time - m->total_time;
+m->downtime = end_time - start_time;
+if (m->state != MIG_STATE_COMPLETED) {
+if (old_vm_running) {
+vm_start();
+}
+}
+last_round = true;
+}
+}
qemu_mutex_unlock_iothread();
if (current_time >= initial_time + BUFFER_DELAY) {
@@ -775,12 +768,6 @@ static void *buffered_file_thread(void *opaque)
if (ret < 0) {
break;
}
-
-DPRINTF("file is ready\n");
-if (s->bytes_xfer < s->xfer_limit) {
-DPRINTF("notifying client\n");
-last_round = migrate_fd_put_ready(m, max_size);
-}
}
out:
--
1.7.11.7
[Qemu-devel] [PATCH 28/35] migration: Only go to the iterate stage if there is anything to send
Signed-off-by: Orit Wasserman
Signed-off-by: Juan Quintela
---
migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration.c b/migration.c
index 7a4a6f0..216356d 100644
--- a/migration.c
+++ b/migration.c
@@ -709,7 +709,7 @@ static void *buffered_file_thread(void *opaque)
DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(m->file, max_size);
DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
-if (pending_size >= max_size) {
+if (pending_size && pending_size >= max_size) {
ret = qemu_savevm_state_iterate(m->file);
if (ret < 0) {
qemu_mutex_unlock_iothread();
--
1.7.11.7
[Qemu-devel] [PATCH 22/35] migration: move exit condition to migration thread
Signed-off-by: Juan Quintela
---
migration.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/migration.c b/migration.c
index 031302b..5c2f413 100644
--- a/migration.c
+++ b/migration.c
@@ -676,12 +676,6 @@ static bool migrate_fd_put_ready(MigrationState *s,
uint64_t max_size)
bool last_round = false;
qemu_mutex_lock_iothread();
-if (s->state != MIG_STATE_ACTIVE) {
-DPRINTF("put_ready returning because of non-active state\n");
-qemu_mutex_unlock_iothread();
-return false;
-}
-
DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(s->file, max_size);
DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
@@ -748,9 +742,18 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
-if (s->migration_state->complete) {
+qemu_mutex_lock_iothread();
+if (m->state != MIG_STATE_ACTIVE) {
+DPRINTF("put_ready returning because of non-active state\n");
+qemu_mutex_unlock_iothread();
break;
}
+if (m->complete) {
+qemu_mutex_unlock_iothread();
+break;
+}
+qemu_mutex_unlock_iothread();
+
if (current_time >= initial_time + BUFFER_DELAY) {
uint64_t transferred_bytes = s->bytes_xfer;
uint64_t time_spent = current_time - initial_time;
--
1.7.11.7
[Qemu-devel] [PATCH 26/35] memory: introduce memory_region_test_and_clear_dirty
This function avoids having to do two calls, one to test the dirty bit, and
other to reset it.
Signed-off-by: Juan Quintela
---
memory.c | 16
memory.h | 16
2 files changed, 32 insertions(+)
diff --git a/memory.c b/memory.c
index 7419853..c72a5e2 100644
--- a/memory.c
+++ b/memory.c
@@ -1081,6 +1081,22 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr
addr,
return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, -1);
}
+bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
+hwaddr size, unsigned client)
+{
+bool ret;
+assert(mr->terminates);
+ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size,
+1 << client);
+if (ret) {
+cpu_physical_memory_reset_dirty(mr->ram_addr + addr,
+mr->ram_addr + addr + size,
+1 << client);
+}
+return ret;
+}
+
+
void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
{
AddressSpace *as;
diff --git a/memory.h b/memory.h
index 9462bfd..bc63a87 100644
--- a/memory.h
+++ b/memory.h
@@ -454,6 +454,22 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr,
hwaddr size);
/**
+ * memory_region_test_and_clear_dirty: Check whether a range of bytes is dirty
+ * for a specified client. It clears them.
+ *
+ * Checks whether a range of bytes has been written to since the last
+ * call to memory_region_reset_dirty() with the same @client. Dirty logging
+ * must be enabled.
+ *
+ * @mr: the memory region being queried.
+ * @addr: the address (relative to the start of the region) being queried.
+ * @size: the size of the range being queried.
+ * @client: the user of the logging information; %DIRTY_MEMORY_MIGRATION or
+ * %DIRTY_MEMORY_VGA.
+ */
+bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr,
+hwaddr size, unsigned client);
+/**
* memory_region_sync_dirty_bitmap: Synchronize a region's dirty bitmap with
* any external TLBs (e.g. kvm)
*
--
1.7.11.7
[Qemu-devel] [PATCH 21/35] migration: Add buffered_flush error handling
Now that we have error handling we can do proper handling of
buffered_flush().
Signed-off-by: Juan Quintela
---
migration.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/migration.c b/migration.c
index 336941b..031302b 100644
--- a/migration.c
+++ b/migration.c
@@ -768,7 +768,8 @@ static void *buffered_file_thread(void *opaque)
/* usleep expects microseconds */
usleep((initial_time + BUFFER_DELAY - current_time)*1000);
}
-if (buffered_flush(s) < 0) {
+ret = buffered_flush(s);
+if (ret < 0) {
break;
}
--
1.7.11.7
Re: [Qemu-devel] [PATCH 2/8] s390: Channel I/O basic defintions.
On Tue, 11 Dec 2012 11:27:05 +0100
Alexander Graf wrote:
>
> On 10.12.2012, at 11:18, Cornelia Huck wrote:
>
> > On Mon, 10 Dec 2012 09:07:57 +0100
> > Alexander Graf wrote:
> >
> >>
> >> On 07.12.2012, at 13:50, Cornelia Huck wrote:
> >>
> >>> Basic channel I/O structures and helper function.
> >>>
> >>> Signed-off-by: Cornelia Huck
> >>> ---
> >>> target-s390x/Makefile.objs | 2 +-
> >>> target-s390x/ioinst.c | 46 ++
> >>> target-s390x/ioinst.h | 207
> >>> +
> >>> 3 files changed, 254 insertions(+), 1 deletion(-)
> >>> create mode 100644 target-s390x/ioinst.c
> >>> create mode 100644 target-s390x/ioinst.h
> >>>
> >>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
> >>> index e728abf..3afb0b7 100644
> >>> --- a/target-s390x/Makefile.objs
> >>> +++ b/target-s390x/Makefile.objs
> >>> @@ -1,4 +1,4 @@
> >>> obj-y += translate.o helper.o cpu.o interrupt.o
> >>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
> >>> -obj-$(CONFIG_SOFTMMU) += machine.o
> >>> +obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o
> >>> obj-$(CONFIG_KVM) += kvm.o
> >>> diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
> >>> new file mode 100644
> >>> index 000..8577b2c
> >>> --- /dev/null
> >>> +++ b/target-s390x/ioinst.c
> >>> @@ -0,0 +1,46 @@
> >>> +/*
> >>> + * I/O instructions for S/390
> >>> + *
> >>> + * Copyright 2012 IBM Corp.
> >>> + * Author(s): Cornelia Huck
> >>> + *
> >>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> >>> + * your option) any later version. See the COPYING file in the top-level
> >>> + * directory.
> >>> + */
> >>> +
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +
> >>> +#include "cpu.h"
> >>> +#include "ioinst.h"
> >>> +
> >>> +#ifdef DEBUG_IOINST
> >>> +#define dprintf(fmt, ...) \
> >>> +do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> >>> +#else
> >>> +#define dprintf(fmt, ...) \
> >>> +do { } while (0)
> >>> +#endif
> >>> +
> >>> +int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int
> >>> *ssid,
> >>> + int *schid)
> >>> +{
> >>> +if (!(value & IOINST_SCHID_ONE)) {
> >>> +return -EINVAL;
> >>> +}
> >>> +if (!(value & IOINST_SCHID_M)) {
> >>> +if (value & IOINST_SCHID_CSSID) {
> >>> +return -EINVAL;
> >>> +}
> >>> +*cssid = 0;
> >>> +*m = 0;
> >>> +} else {
> >>> +*cssid = (value & IOINST_SCHID_CSSID) >> 24;
> >>
> >> (value & IOINST_SCHID_CSSID_MASK) >> IOINST_SCHID_CSSID_SHIFT
> >
> > I think that actually decreases readability.
>
> I'm fine with doing it Jocelyn-style too:
>
> #define IOINST_SCHID_CSSID(x) ((x & 0x...) >> 24)
>
> if you prefer that for readability :)
It's worth a try.
[Qemu-devel] [PATCH 30/35] ram: account the amount of transferred ram better
Signed-off-by: Juan Quintela
---
arch_init.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 6c12a7d..3e82588 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -265,16 +265,21 @@ uint64_t xbzrle_mig_pages_overflow(void)
return acct_info.xbzrle_overflows;
}
-static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
-int cont, int flag)
+static size_t save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
+ int cont, int flag)
{
-qemu_put_be64(f, offset | cont | flag);
-if (!cont) {
-qemu_put_byte(f, strlen(block->idstr));
-qemu_put_buffer(f, (uint8_t *)block->idstr,
-strlen(block->idstr));
-}
+size_t size;
+
+qemu_put_be64(f, offset | cont | flag);
+size = 8;
+if (!cont) {
+qemu_put_byte(f, strlen(block->idstr));
+qemu_put_buffer(f, (uint8_t *)block->idstr,
+strlen(block->idstr));
+size += 1 + strlen(block->idstr);
+}
+return size;
}
#define ENCODING_FLAG_XBZRLE 0x1
@@ -321,11 +326,11 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t
*current_data,
}
/* Send XBZRLE based compressed page */
-save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
+bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
qemu_put_byte(f, ENCODING_FLAG_XBZRLE);
qemu_put_be16(f, encoded_len);
qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
-bytes_sent = encoded_len + 1 + 2;
+bytes_sent += encoded_len + 1 + 2;
acct_info.xbzrle_pages++;
acct_info.xbzrle_bytes += bytes_sent;
@@ -457,9 +462,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
if (is_dup_page(p)) {
acct_info.dup_pages++;
-save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS);
+bytes_sent = save_block_hdr(f, block, offset, cont,
+RAM_SAVE_FLAG_COMPRESS);
qemu_put_byte(f, *p);
-bytes_sent = 1;
+bytes_sent += 1;
} else if (migrate_use_xbzrle()) {
current_addr = block->offset + offset;
bytes_sent = save_xbzrle_page(f, p, current_addr, block,
@@ -471,9 +477,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
/* either we didn't send yet (we may have had XBZRLE overflow) */
if (bytes_sent == -1) {
-save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
+bytes_sent = save_block_hdr(f, block, offset, cont,
RAM_SAVE_FLAG_PAGE);
qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
-bytes_sent = TARGET_PAGE_SIZE;
+bytes_sent += TARGET_PAGE_SIZE;
acct_info.norm_pages++;
}
--
1.7.11.7
Re: [Qemu-devel] [PATCH 4/8] s390: Add channel I/O instructions.
On Tue, 11 Dec 2012 11:18:44 +0100
Alexander Graf wrote:
>
> On 10.12.2012, at 10:18, Cornelia Huck wrote:
>
> > On Mon, 10 Dec 2012 10:00:16 +0100
> > Alexander Graf wrote:
> >
> >>
> >>
> >> On 07.12.2012, at 13:50, Cornelia Huck wrote:
> >>> +/* Special handling for the prefix page. */
> >>> +static void *s390_get_address(CPUS390XState *env, ram_addr_t guest_addr)
> >>> +{
> >>> +if (guest_addr < 8192) {
> >>> +guest_addr += env->psa;
> >>> +} else if ((env->psa <= guest_addr) && (guest_addr < env->psa +
> >>> 8192)) {
> >>> +guest_addr -= env->psa;
> >>> +}
> >>> +
> >>> +return qemu_get_ram_ptr(guest_addr);
> >>
> >> Do we actually need this?
> >
> > Yes. I've seen failures for I/O instructions using the lowcore (which
> > the Linux kernel likes to do).
>
> Then we want an s390 generic function that does this, not an io specific one
> though, right? Also qemu_get_ram_ptr is a no-go, as it doesn't do boundary
> checks.
Oh, wasn't aware of that.
>
> So what we really want is something like s390_cpu_physical_memory_map(env,
> ...) with a special case on the lowcore.
Let's see how this works out.
> >>> +addr = ipb >> 28;
> >>> +if (addr > 0) {
> >>> +addr = env->regs[addr];
> >>> +}
> >>> +addr += (ipb & 0xfff) >> 16;
> >>
> >> This adds the upper bits twice. Are you sire that's correct?
> >
> > If addr was 0, it doesn't. If addr was > 0 before, we grabbed the
> > address from the corresponding register and want to add to it.
>
> This is a very confusing way of writing what you're trying to express then
> :). How about
>
> hwaddr addr = 0;
>
> reg = ipb >> 28;
> if (reg) {
> addr = env->regs[reg];
> }
> addr += (ipb >> 16) & 0xfff0;
I've moved this to a helper function anyway - but this looks a bit more
readable, yes.
Re: [Qemu-devel] Network isn't clean when qemu exits
On Tue, Dec 11, 2012 at 09:38:43AM +0100, Stefan Hajnoczi wrote:
> On Tue, Dec 11, 2012 at 01:57:04PM +0800, Amos Kong wrote:
> > In vl.c:main(), tap device would be created when net_init_clients() is
> > called.
> > "device" option is parsed after calling net_init_clients() by:
> > # "qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1)
> >
> > Qemu will exit if fails to parse device parameters without calling
> > net_cleanup().
> > I touch a problem, the tap device which is created by qemu-ifup script
> > could not be removed by qemu-ifdown script.
> >
> > I did a quick fix, the tap device should be removed.
> >
> > But there are many "exit(1)" after calling net_init_clients() in vl.c,
> > it's ugly to call net_cleanup() before each exit. Not sure if we have
> > other exit(1) in other sub-functions, which is also called after calling
> > net_init_clients().
> >
> > Any comments?
> >
> > =
> > @@ -3882,8 +3889,11 @@ int main(int argc, char **argv, char **envp)
> > }
> >
> > /* init generic devices */
> > -if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func,
> > NULL, 1) != 0)
> > +if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func,
> > + NULL, 1) != 0) {
> > +net_cleanup();
> > exit(1);
> > +}
>
> How about qemu_add_exit_notifier() or atexit(3)? Several other places
> in QEMU use this to clean up.
>
> Then you can also remove net_cleanup() from the end of vl.c:main() since
> it gets called implicitly.
>
> Stefan
Good idea, thanks a lot.
Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats
On Tue, 11 Dec 2012 12:29:01 + Dietmar Maurer wrote: > > > Can't we enable stat queries by default (10s interval), > > > > I'm not sure I like this for two reasons. First, there will be cases where > > the > > user doesn't want this to be enabled. Second, we'll be forcing an interval > > on > > users. > > So when should we set the stats-polling-interval? I first thought I simply > set that at VM start, but that triggers an error: > > "guest doesn't support balloon stats" > > because the balloon driver is not loaded. Yes, it's required to have the balloon driver loaded. The stats are reported by it.
Re: [Qemu-devel] [PATCH 8/8] s390: Add new channel I/O based virtio transport.
On Tue, 11 Dec 2012 11:53:18 +0100
Alexander Graf wrote:
>
> On 07.12.2012, at 13:50, Cornelia Huck wrote:
>
> > Add a new virtio transport that uses channel commands to perform
> > virtio operations.
> >
> > Add a new machine type s390-ccw that uses this virtio-ccw transport
> > and make it the default machine for s390.
> >
> > Signed-off-by: Cornelia Huck
> > ---
> > hw/s390-virtio.c | 149 ++--
> > hw/s390x/Makefile.objs | 1 +
> > hw/s390x/virtio-ccw.c | 909
> > +
> > hw/s390x/virtio-ccw.h | 81 +
> > trace-events | 4 +
> > 5 files changed, 1124 insertions(+), 20 deletions(-)
> > create mode 100644 hw/s390x/virtio-ccw.c
> > create mode 100644 hw/s390x/virtio-ccw.h
> >
> > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> > index 9e1afb2..f29ff74 100644
> > --- a/hw/s390-virtio.c
> > +++ b/hw/s390-virtio.c
> > @@ -33,6 +33,8 @@
> >
> > #include "hw/s390-virtio-bus.h"
> > #include "hw/s390x/sclp.h"
> > +#include "hw/s390x/css.h"
> > +#include "hw/s390x/virtio-ccw.h"
> >
> > //#define DEBUG_S390
> >
> > @@ -47,6 +49,7 @@
> > #define KVM_S390_VIRTIO_NOTIFY 0
> > #define KVM_S390_VIRTIO_RESET 1
> > #define KVM_S390_VIRTIO_SET_STATUS 2
> > +#define KVM_S390_VIRTIO_CCW_NOTIFY 3
> >
> > #define KERN_IMAGE_START0x01UL
> > #define KERN_PARM_AREA 0x010480UL
> > @@ -63,6 +66,7 @@
> >
> > static VirtIOS390Bus *s390_bus;
> > static S390CPU **ipi_states;
> > +VirtioCcwBus *ccw_bus;
> >
> > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> > {
> > @@ -76,15 +80,21 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> > int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t
> > hypercall)
> > {
> > int r = 0, i;
> > +int cssid, ssid, schid, m;
> > +SubchDev *sch;
> >
> > dprintf("KVM hypercall: %ld\n", hypercall);
> > switch (hypercall) {
> > case KVM_S390_VIRTIO_NOTIFY:
> > if (mem > ram_size) {
> > -VirtIOS390Device *dev = s390_virtio_bus_find_vring(s390_bus,
> > - mem, &i);
> > -if (dev) {
> > -virtio_queue_notify(dev->vdev, i);
> > +if (s390_bus) {
> > +VirtIOS390Device *dev =
> > s390_virtio_bus_find_vring(s390_bus,
> > + mem,
> > &i);
> > +if (dev) {
> > +virtio_queue_notify(dev->vdev, i);
> > +} else {
> > +r = -EINVAL;
> > +}
>
> We really want to factor out the DIAG handling code similar to how spapr
> handles its hypercalls. That way the legacy s390-virtio machine can register
> a VIRTIO_NOTIFY hypercall that works for it here, while the s390-virtio-ccw
> machine doesn't.
>
> > } else {
> > r = -EINVAL;
> > }
> > @@ -93,28 +103,49 @@ int s390_virtio_hypercall(CPUS390XState *env, uint64_t
> > mem, uint64_t hypercall)
> > }
> > break;
> > case KVM_S390_VIRTIO_RESET:
> > -{
> > -VirtIOS390Device *dev;
> > -
> > -dev = s390_virtio_bus_find_mem(s390_bus, mem);
> > -virtio_reset(dev->vdev);
> > -stb_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS, 0);
> > -s390_virtio_device_sync(dev);
> > -s390_virtio_reset_idx(dev);
> > +if (s390_bus) {
> > +VirtIOS390Device *dev;
> > +
> > +dev = s390_virtio_bus_find_mem(s390_bus, mem);
> > +virtio_reset(dev->vdev);
> > +stb_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS, 0);
> > +s390_virtio_device_sync(dev);
> > +s390_virtio_reset_idx(dev);
> > +} else {
> > +r = -EINVAL;
> > +}
> > break;
> > -}
> > case KVM_S390_VIRTIO_SET_STATUS:
> > -{
> > -VirtIOS390Device *dev;
> > +if (s390_bus) {
> > +VirtIOS390Device *dev;
> >
> > -dev = s390_virtio_bus_find_mem(s390_bus, mem);
> > -if (dev) {
> > -s390_virtio_device_update_status(dev);
> > +dev = s390_virtio_bus_find_mem(s390_bus, mem);
> > +if (dev) {
> > +s390_virtio_device_update_status(dev);
> > +} else {
> > +r = -EINVAL;
> > +}
> > } else {
> > r = -EINVAL;
> > }
> > break;
> > -}
> > +case KVM_S390_VIRTIO_CCW_NOTIFY:
> > +if (ccw_bus) {
> > +if (ioinst_disassemble_sch_ident(env->regs[2], &m, &cssid,
> > &ssid,
> > + &schid)) {
> > +r = -EINVAL;
> > +} else {
> > +sch = css_find_subch(m, cssid, ssid, schid);
> > +if (sch && css_subch_visible(sch)) {
> > +virtio_queue_notify(virtio_ccw_get_vde
Re: [Qemu-devel] [PATCH v6] Add compare subcommand for qemu-img
Hi Kevin,
thanks for review, comments inline.
- Original Message -
> From: "Kevin Wolf"
> To: "Miroslav Rezanina"
> Cc: qemu-devel@nongnu.org, "Paolo Bonzini" , "Stefan
> Hajnoczi"
> Sent: Tuesday, December 11, 2012 1:27:45 PM
> Subject: Re: [PATCH v6] Add compare subcommand for qemu-img
>
> Am 06.12.2012 13:24, schrieb Miroslav Rezanina:
> > This is second version of patch adding compare subcommand that
> > compares two images. Compare has following criteria:
> > - only data part is compared
> > - unallocated sectors are not read
> > - in case of different image size, exceeding part of bigger disk
> > has
> > to be zeroed/unallocated to compare rest
> > - qemu-img returns:
> > - 0 if images are identical
> > - 1 if images differ
> > - 2 on error
> >
> > v6:
> > - added handling -?, -h options for compare subcommand
> >
> > v5 (only minor changes):
> > - removed redundant comment
> > - removed dead code (goto after help())
> > - set final total_sectors on first assignment
> >
> > v4:
> > - Fixed various typos
> > - Added functions for empty sector check and sector-to-bytes
> > offset
> > conversion
> > - Fixed command-line parameters processing
> >
> > v3:
> > - options -f/-F are orthogonal
> > - documentation updated to new syntax and behavior
> > - used byte offset instead of sector number for output
> >
> > v2:
> > - changed option for second image format to -F
> > - changed handling of -f and -F [1]
> > - added strict mode (-s)
> > - added quiet mode (-q)
> > - improved output messages [2]
> > - rename variables for larger image handling
> > - added man page content
> >
> > Signed-off-by: Miroslav Rezanina
>
> Please move the changelog below a "---" line so that git am ignores
> it
> for the final commit message.
>
Ok, next version probably be as multipart so this will go into the header mail.
> > diff --git a/block.c b/block.c
> > index 854ebd6..fdc8c45 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2698,6 +2698,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
> >
> > typedef struct BdrvCoIsAllocatedData {
> > BlockDriverState *bs;
> > +BlockDriverState *base;
> > int64_t sector_num;
> > int nb_sectors;
> > int *pnum;
> > @@ -2828,6 +2829,44 @@ int coroutine_fn
> > bdrv_co_is_allocated_above(BlockDriverState *top,
> > return 0;
> > }
> >
> > +/* Coroutine wrapper for bdrv_is_allocated_above() */
> > +static void coroutine_fn bdrv_is_allocated_above_co_entry(void
> > *opaque)
> > +{
> > +BdrvCoIsAllocatedData *data = opaque;
> > +BlockDriverState *top = data->bs;
> > +BlockDriverState *base = data->base;
> > +
> > +data->ret = bdrv_co_is_allocated_above(top, base,
> > data->sector_num,
> > + data->nb_sectors,
> > data->pnum);
> > +data->done = true;
> > +}
> > +
> > +/*
> > + * Synchronous wrapper around bdrv_co_is_allocated_above().
> > + *
> > + * See bdrv_co_is_allocated_above() for details.
> > + */
> > +int bdrv_is_allocated_above(BlockDriverState *top,
> > BlockDriverState *base,
> > + int64_t sector_num, int nb_sectors, int
> > *pnum)
> > +{
> > +Coroutine *co;
> > +BdrvCoIsAllocatedData data = {
> > +.bs = top,
> > +.base = base,
> > +.sector_num = sector_num,
> > +.nb_sectors = nb_sectors,
> > +.pnum = pnum,
> > +.done = false,
> > +};
> > +
> > +co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
> > +qemu_coroutine_enter(co, &data);
> > +while (!data.done) {
> > +qemu_aio_wait();
> > +}
> > +return data.ret;
> > +}
> > +
> > BlockInfo *bdrv_query_info(BlockDriverState *bs)
> > {
> > BlockInfo *info = g_malloc0(sizeof(*info));
> > diff --git a/block.h b/block.h
> > index 722c620..2cb8d71 100644
> > --- a/block.h
> > +++ b/block.h
> > @@ -278,6 +278,8 @@ int bdrv_co_discard(BlockDriverState *bs,
> > int64_t sector_num, int nb_sectors);
> > int bdrv_has_zero_init(BlockDriverState *bs);
> > int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
> > int nb_sectors,
> >int *pnum);
> > +int bdrv_is_allocated_above(BlockDriverState *top,
> > BlockDriverState *base,
> > +int64_t sector_num, int nb_sectors,
> > int *pnum);
> >
> > void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError
> > on_read_error,
> > BlockdevOnError on_write_error);
>
> This first part looks good. I think it could be a separate patch,
> however.
>
Agree with this, was lazy to split.
> > diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> > index a181363..c076085 100644
> > --- a/qemu-img-cmds.hx
> > +++ b/qemu-img-cmds.hx
> > @@ -27,6 +27,12 @@ STEXI
> > @item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
> > ETEXI
> >
> > +DEF("compare", img_compare,
> > +"compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1
> > filename2")
> >
[Qemu-devel] [PATCH 05/35] migration: make qemu_fopen_ops_buffered() return void
We want the file assignment to happen before the thread is created to
avoid locking, so we just do it before creating the thread.
Signed-off-by: Juan Quintela
Reviewed-by: Orit Wasserman
---
buffered_file.c | 13 ++---
buffered_file.h | 2 +-
migration.c | 2 +-
migration.h | 1 +
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 418d22a..27610b4 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -33,7 +33,6 @@ typedef struct QEMUFileBuffered
size_t buffer_size;
size_t buffer_capacity;
QemuThread thread;
-bool complete;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -163,7 +162,7 @@ static int buffered_close(void *opaque)
ret = ret2;
}
ret = migrate_fd_close(s->migration_state);
-s->complete = true;
+s->migration_state->complete = true;
return ret;
}
@@ -232,7 +231,7 @@ static void *buffered_file_thread(void *opaque)
while (true) {
int64_t current_time = qemu_get_clock_ms(rt_clock);
-if (s->complete) {
+if (s->migration_state->complete) {
break;
}
if (s->freeze_output) {
@@ -264,7 +263,7 @@ static const QEMUFileOps buffered_file_ops = {
.set_rate_limit = buffered_set_rate_limit,
};
-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
+void qemu_fopen_ops_buffered(MigrationState *migration_state)
{
QEMUFileBuffered *s;
@@ -272,12 +271,12 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState
*migration_state)
s->migration_state = migration_state;
s->xfer_limit = migration_state->bandwidth_limit / 10;
-s->complete = false;
+s->migration_state->complete = false;
s->file = qemu_fopen_ops(s, &buffered_file_ops);
+migration_state->file = s->file;
+
qemu_thread_create(&s->thread, buffered_file_thread, s,
QEMU_THREAD_DETACHED);
-
-return s->file;
}
diff --git a/buffered_file.h b/buffered_file.h
index ef010fe..8a246fd 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -17,6 +17,6 @@
#include "hw/hw.h"
#include "migration.h"
-QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state);
+void qemu_fopen_ops_buffered(MigrationState *migration_state);
#endif
diff --git a/migration.c b/migration.c
index 73ce170..d4b8406 100644
--- a/migration.c
+++ b/migration.c
@@ -448,7 +448,7 @@ void migrate_fd_connect(MigrationState *s)
int ret;
s->state = MIG_STATE_ACTIVE;
-s->file = qemu_fopen_ops_buffered(s);
+qemu_fopen_ops_buffered(s);
DPRINTF("beginning savevm\n");
ret = qemu_savevm_state_begin(s->file, &s->params);
diff --git a/migration.h b/migration.h
index c3a23cc..b66fd60 100644
--- a/migration.h
+++ b/migration.h
@@ -45,6 +45,7 @@ struct MigrationState
int64_t dirty_pages_rate;
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
int64_t xbzrle_cache_size;
+bool complete;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.11.7
Re: [Qemu-devel] [RFC V3 06/24] qcow2: Add qcow2_dedup and related functions.
On Mon, Nov 26, 2012 at 02:05:05PM +0100, Benoît Canet wrote:
> +/*
> + * Compute the hash of a given cluster
> + *
> + * @data: a buffer containing the cluster data
> + * @ret: a HASH_LENGTH long dynamically allocated array containing the hash
> + */
> +static uint8_t *qcow2_compute_cluster_hash(BlockDriverState *bs,
> + uint8_t *data)
> +{
> +return NULL;
> +}
> +
> +/* Try to find the offset of a given cluster if it's duplicated
> + * Exceptionally we cast return value to int64_t to use as error code.
I don't see an int64_t return value, this comment is probably outdated.
> + *
> + * @data:a buffer containing the cluster
> + * @skip_cluster_nr: the number of cluster to skip in the buffer
> + * @hash:if hash is provided it's used else it's computed
> + * @ret: QCowHashNode of the duplicated cluster or NULL
This function does several things:
1. Allocating and computing *hash if not given.
2. Returning existing dedup_tree_by_hash node or NULL if the node wasn't
already in the tree.
3. Inserting the node into the tree if not present.
I wonder if it can be simplified or split to do less work.
> +/*
> + * Helper used to link a deduplicated cluster in the l2
> + *
> + * @logical_cluster_offset: the cluster offset seen by the guest (in
> sectors)
> + * @physical_cluster_offset: the cluster offset in the QCOW2 file (in
> sectors)
Perhaps s/_offset/_sect/ because usually offset is in bytes.
[Qemu-devel] compile tcm_vhost kernel module as built-in
Hi, is there any virtio-scsi developer here? I take a look at the tcm_vhost module of kernel 3.7 and want to compile it as built-in. However, CONFIG_TCM_VHOST only allows user to build as module. I wonder why this restriction exists? thx a lot. ching
Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cpu.c
On Mon, Dec 10, 2012 at 10:30:42PM -0200, Eduardo Habkost wrote:
> As we will need to create the CPU object after splitting the CPU model
> string (because we're going to use different subclasses for each CPU
> model), move the CPU object creation to cpu_x86_register(), and at the
> same time rename cpu_x86_register() to cpu_x86_create().
>
> This will also simplify the CPU creation code to a trivial
> cpu_x86_create()+cpu_x86_realize() sequence. This will be useful for
> code that have to set additional properties before cpu_x86_realize() is
> called (e.g. the PC CPU initialization code, that needs to set APIC IDs
> depending on the CPU cores/threads topology).
>
> Signed-off-by: Eduardo Habkost
Igor just sent "[PATCH 0/6] x86 CPU cleanup (wave 2)", that conflicts
a little with this series.
As the series he sent is larger and some of the patches got some reviews
and comments previously, I will rebase this on top of his series and
resubmit.
> ---
> target-i386/cpu.c| 17 +
> target-i386/cpu.h| 2 +-
> target-i386/helper.c | 9 ++---
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 7be3ad8..044e2d9 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1488,14 +1488,22 @@ static void filter_features_for_kvm(X86CPU *cpu)
> }
> #endif
>
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> +/* Create and initialize a X86CPU object, based on the full CPU model string
> + * (that may include "+feature,-feature,feature=xxx" feature strings)
> + */
> +X86CPU *cpu_x86_create(const char *cpu_model)
> {
> -CPUX86State *env = &cpu->env;
> +X86CPU *cpu;
> +CPUX86State *env;
> x86_def_t def1, *def = &def1;
> Error *error = NULL;
> char *name, *features;
> gchar **model_pieces;
>
> +cpu = X86_CPU(object_new(TYPE_X86_CPU));
> +env = &cpu->env;
> +env->cpu_model_str = cpu_model;
> +
> memset(def, 0, sizeof(*def));
>
> model_pieces = g_strsplit(cpu_model, ",", 2);
> @@ -1572,10 +1580,11 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> }
>
> g_strfreev(model_pieces);
> -return 0;
> +return cpu;
> error:
> +object_delete(OBJECT(cpu));
> g_strfreev(model_pieces);
> -return -1;
> +return NULL;
> }
>
> #if !defined(CONFIG_USER_ONLY)
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 386c4f6..3ebaae9 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -980,7 +980,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
> void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> uint32_t *eax, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx);
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
> +X86CPU *cpu_x86_create(const char *cpu_model);
> void cpu_clear_apic_feature(CPUX86State *env);
> void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf206cf..23af4a8 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1243,15 +1243,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env,
> unsigned int selector,
> X86CPU *cpu_x86_init(const char *cpu_model)
> {
> X86CPU *cpu;
> -CPUX86State *env;
> Error *error = NULL;
>
> -cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -env = &cpu->env;
> -env->cpu_model_str = cpu_model;
> -
> -if (cpu_x86_register(cpu, cpu_model) < 0) {
> -object_delete(OBJECT(cpu));
> +cpu = cpu_x86_create(cpu_model);
> +if (!cpu) {
> return NULL;
> }
>
> --
> 1.7.11.7
>
>
--
Eduardo
Re: [Qemu-devel] [PATCH 1/6] target-i386: filter out not TCG features if running without kvm at realize time
On Tue, Dec 11, 2012 at 11:11:01AM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov
> Reviewed-by: Eduardo Habkost
> Signed-off-by: Eduardo Habkost
Just to confirm that this submission still looks good to me:
Reviewed-by: Eduardo Habkost
> ---
> target-i386/cpu.c | 31 ---
> 1 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 7be3ad8..63aae86 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1549,21 +1549,6 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES);
> }
>
> -if (!kvm_enabled()) {
> -env->cpuid_features &= TCG_FEATURES;
> -env->cpuid_ext_features &= TCG_EXT_FEATURES;
> -env->cpuid_ext2_features &= (TCG_EXT2_FEATURES
> -#ifdef TARGET_X86_64
> -| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
> -#endif
> -);
> -env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
> -env->cpuid_svm_features &= TCG_SVM_FEATURES;
> -} else {
> -#ifdef CONFIG_KVM
> -filter_features_for_kvm(cpu);
> -#endif
> -}
> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
> if (error) {
> fprintf(stderr, "%s\n", error_get_pretty(error));
> @@ -2077,6 +2062,22 @@ void x86_cpu_realize(Object *obj, Error **errp)
> env->cpuid_level = 7;
> }
>
> +if (!kvm_enabled()) {
> +env->cpuid_features &= TCG_FEATURES;
> +env->cpuid_ext_features &= TCG_EXT_FEATURES;
> +env->cpuid_ext2_features &= (TCG_EXT2_FEATURES
> +#ifdef TARGET_X86_64
> +| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
> +#endif
> +);
> +env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
> +env->cpuid_svm_features &= TCG_SVM_FEATURES;
> +} else {
> +#ifdef CONFIG_KVM
> +filter_features_for_kvm(cpu);
> +#endif
> +}
> +
> #ifndef CONFIG_USER_ONLY
> qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
>
> --
> 1.7.1
>
--
Eduardo
[Qemu-devel] [PATCH 04/35] buffered_file: Move from using a timer to use a thread
We still protect everything except the wait with the iothread lock.
But we moved from a timer to a thread. Steps one by one.
We also need to detect when we have finished with a variable "complete".
Signed-off-by: Juan Quintela
---
buffered_file.c | 58 +++--
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index bd0f61d..418d22a 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -18,6 +18,7 @@
#include "qemu-timer.h"
#include "qemu-char.h"
#include "buffered_file.h"
+#include "qemu-thread.h"
//#define DEBUG_BUFFERED_FILE
@@ -31,7 +32,8 @@ typedef struct QEMUFileBuffered
uint8_t *buffer;
size_t buffer_size;
size_t buffer_capacity;
-QEMUTimer *timer;
+QemuThread thread;
+bool complete;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -160,11 +162,8 @@ static int buffered_close(void *opaque)
if (ret >= 0) {
ret = ret2;
}
-qemu_del_timer(s->timer);
-qemu_free_timer(s->timer);
-g_free(s->buffer);
-g_free(s);
-
+ret = migrate_fd_close(s->migration_state);
+s->complete = true;
return ret;
}
@@ -222,23 +221,38 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static void buffered_rate_tick(void *opaque)
+/* 10ms xfer_limit is the limit that we should write each 10ms */
+#define BUFFER_DELAY 100
+
+static void *buffered_file_thread(void *opaque)
{
QEMUFileBuffered *s = opaque;
+int64_t expire_time = qemu_get_clock_ms(rt_clock) + BUFFER_DELAY;
-if (qemu_file_get_error(s->file)) {
-buffered_close(s);
-return;
-}
-
-qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
-
-if (s->freeze_output)
-return;
-
-s->bytes_xfer = 0;
+while (true) {
+int64_t current_time = qemu_get_clock_ms(rt_clock);
-buffered_put_buffer(s, NULL, 0, 0);
+if (s->complete) {
+break;
+}
+if (s->freeze_output) {
+continue;
+}
+if (current_time >= expire_time) {
+s->bytes_xfer = 0;
+expire_time = current_time + BUFFER_DELAY;
+}
+if (s->bytes_xfer >= s->xfer_limit) {
+/* usleep expects microseconds */
+usleep((expire_time - current_time)*1000);
+}
+qemu_mutex_lock_iothread();
+buffered_put_buffer(s, NULL, 0, 0);
+qemu_mutex_unlock_iothread();
+}
+g_free(s->buffer);
+g_free(s);
+return NULL;
}
static const QEMUFileOps buffered_file_ops = {
@@ -258,12 +272,12 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState
*migration_state)
s->migration_state = migration_state;
s->xfer_limit = migration_state->bandwidth_limit / 10;
+s->complete = false;
s->file = qemu_fopen_ops(s, &buffered_file_ops);
-s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
-
-qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+qemu_thread_create(&s->thread, buffered_file_thread, s,
+ QEMU_THREAD_DETACHED);
return s->file;
}
--
1.7.11.7
Re: [Qemu-devel] [RFC V3 08/24] qcow2: Implement qcow2_compute_cluster_hash.
On Mon, Nov 26, 2012 at 02:05:07PM +0100, Benoît Canet wrote:
> Signed-off-by: Benoit Canet
> ---
> Makefile|3 +++
> Makefile.target |2 +-
> block/qcow2-dedup.c | 10 --
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 88285a4..c79b2da 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -168,6 +168,9 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
> $(block-obj-y) $(qapi-obj-y) \
>qapi-visit.o qapi-types.o
> qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
> qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
> +qemu-img$(EXESUF): LIBS+=-lcrypto
> +qemu-nbd$(EXESUF): LIBS+=-lcrypto
> +qemu-io$(EXESUF): LIBS+=-lcrypto
>
> qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>
> diff --git a/Makefile.target b/Makefile.target
> index 3822bc5..f9a988a 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -119,7 +119,7 @@ obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
> obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
> obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
> obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
> -LIBS+=-lz
> +LIBS+=-lz -lcrypto
Need a ./configure check for openssl?
VNC can already use gnutls so perhaps we should support that?
http://gnutls.org/manual/gnutls.html#Hash-and-HMAC-functions
>
> QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
> diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
> index 83ad61e..37e8266 100644
> --- a/block/qcow2-dedup.c
> +++ b/block/qcow2-dedup.c
> @@ -25,11 +25,13 @@
> * THE SOFTWARE.
> */
>
> +#include
> +#include
> #include "block_int.h"
> #include "qemu-common.h"
> #include "qcow2.h"
>
> -#define HASH_LENGTH 32
> +#define HASH_LENGTH SHA256_DIGEST_LENGTH
>
> static int qcow2_dedup_read_write_hash(BlockDriverState *bs,
> uint8_t **hash,
> @@ -188,7 +190,11 @@ static QCowHashNode
> *qcow2_dedup_build_qcow_hash_node(uint8_t *hash,
> static uint8_t *qcow2_compute_cluster_hash(BlockDriverState *bs,
> uint8_t *data)
> {
> -return NULL;
> +BDRVQcowState *s = bs->opaque;
> +uint8_t *hash = g_malloc0(HASH_LENGTH);
> +EVP_Digest(data, s->cluster_size,
> + hash, NULL, EVP_sha256(), NULL);
> +return hash;
> }
Not sure if it's worth allocating these relatively small objects on the
heap and worrying about their lifecycle.
It's simpler to pass references to the hashes and copy the entire object
to pass ownership.
This function would become:
void qcow2_compute_cluster_hash(BlockDriverState *bs, uint8_t *data, QcowHash
*hash);
typedef struct {
uint8_t data[SHA256_DIGEST_LENGTH];
} QcowHash;
The caller needs to decide whether a stack-allocated variable is
appropriate or if the hash should live inside a QcowHashNode, etc.
Stefan
Re: [Qemu-devel] [PATCH 4/6] target-i386: setting default 'vendor' is obsolete, remove it
On Tue, Dec 11, 2012 at 11:11:04AM +0100, Igor Mammedov wrote:
> since cpu_def config is not supported anymore and all remainig sources now
> always set x86_def_t.vendor[123] fields remove setting default vendor to
> simplify future refactoring.
>
> Signed-off-by: Igor Mammedov
Reviewed-by: Eduardo Habkost
> ---
> target-i386/cpu.c | 13 -
> 1 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1497980..99fd3f3 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1539,15 +1539,10 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> if (cpu_x86_parse_featurestr(def, features) < 0) {
> goto error;
> }
> -if (def->vendor1) {
> -env->cpuid_vendor1 = def->vendor1;
> -env->cpuid_vendor2 = def->vendor2;
> -env->cpuid_vendor3 = def->vendor3;
> -} else {
> -env->cpuid_vendor1 = CPUID_VENDOR_INTEL_1;
> -env->cpuid_vendor2 = CPUID_VENDOR_INTEL_2;
> -env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
> -}
> +assert(def->vendor1);
> +env->cpuid_vendor1 = def->vendor1;
> +env->cpuid_vendor2 = def->vendor2;
> +env->cpuid_vendor3 = def->vendor3;
> env->cpuid_vendor_override = def->vendor_override;
> object_property_set_int(OBJECT(cpu), def->level, "level", &error);
> object_property_set_int(OBJECT(cpu), def->family, "family", &error);
> --
> 1.7.1
>
--
Eduardo
Re: [Qemu-devel] [PATCH 5/6] target-i386: move setting defaults out of cpu_x86_parse_featurestr()
On Tue, Dec 11, 2012 at 11:11:05AM +0100, Igor Mammedov wrote:
> No functional change, needed for simplifying convertion to properties.
>
> Signed-off-by: Igor Mammedov
Reviewed-by: Eduardo Habkost
> ---
> target-i386/cpu.c | 12 +++-
> 1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 99fd3f3..e534254 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1264,7 +1264,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
> *x86_cpu_def, char *features)
> /* Features to be added */
> uint32_t plus_features = 0, plus_ext_features = 0;
> uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
> -uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
> +uint32_t plus_kvm_features = 0, plus_svm_features = 0;
> uint32_t plus_7_0_ebx_features = 0;
> /* Features to be removed */
> uint32_t minus_features = 0, minus_ext_features = 0;
> @@ -1273,10 +1273,6 @@ static int cpu_x86_parse_featurestr(x86_def_t
> *x86_cpu_def, char *features)
> uint32_t minus_7_0_ebx_features = 0;
> uint32_t numvalue;
>
> -add_flagname_to_bitmaps("hypervisor", &plus_features,
> -&plus_ext_features, &plus_ext2_features, &plus_ext3_features,
> -&plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features);
> -
> featurestr = features ? strtok(features, ",") : NULL;
>
> while (featurestr) {
> @@ -1536,6 +1532,12 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> goto error;
> }
>
> +def->kvm_features |= kvm_default_features;
> +add_flagname_to_bitmaps("hypervisor", &def->features,
> +&def->ext_features, &def->ext2_features,
> +&def->ext3_features, &def->kvm_features,
> +&def->svm_features,
> &def->cpuid_7_0_ebx_features);
> +
> if (cpu_x86_parse_featurestr(def, features) < 0) {
> goto error;
> }
> --
> 1.7.1
>
--
Eduardo
[Qemu-devel] [PATCH 19/35] migration: move migration notifier
At this point, it is waranteed that state is ACTIVE. Old position didn't assured hat. Signed-off-by: Juan Quintela --- migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/migration.c b/migration.c index 371ff0c..913f3bc 100644 --- a/migration.c +++ b/migration.c @@ -450,8 +450,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, error_propagate(errp, local_err); return; } - -notifier_list_notify(&migration_state_notifiers, s); } void qmp_migrate_cancel(Error **errp) @@ -812,4 +810,5 @@ void migrate_fd_connect(MigrationState *migration_state) qemu_thread_create(&s->thread, buffered_file_thread, s, QEMU_THREAD_DETACHED); +notifier_list_notify(&migration_state_notifiers, s); } -- 1.7.11.7
Re: [Qemu-devel] [PATCH 2/6] target-i386: sanitize AMD's ext2_features at realize time
On Tue, Dec 11, 2012 at 11:11:02AM +0100, Igor Mammedov wrote:
> when CPU properties are implemented, ext2_features may change
> between object_new(CPU) and cpu_realize_fn(). Sanitizing
> ext2_features for AMD based CPU at realize() time will keep
> current behavior after CPU features are converted to properties.
>
> Signed-off-by: Igor Mammedov
> ---
> v2:
> - style fix, make line shorter than 80 characters
>
> amd san
Incomplete sentence?
> ---
> target-i386/cpu.c | 21 +++--
> 1 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 63aae86..64b7637 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1539,16 +1539,6 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
> "tsc-frequency", &error);
>
> -/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> - * CPUID[1].EDX.
> - */
> -if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
> -env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
> -env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
> -env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
> -env->cpuid_ext2_features |= (def->features & CPUID_EXT2_AMD_ALIASES);
> -}
> -
> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
> if (error) {
> fprintf(stderr, "%s\n", error_get_pretty(error));
> @@ -2062,6 +2052,17 @@ void x86_cpu_realize(Object *obj, Error **errp)
> env->cpuid_level = 7;
> }
>
> +/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> + * CPUID[1].EDX.
> + */
> +if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
> +env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
> +env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
I would add extra indentation space here, to make it not align with the
statements below, making the condition visually distinct from the body,
like in the original code you are moving.
> +env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
> +env->cpuid_ext2_features |= (env->cpuid_features
> + & CPUID_EXT2_AMD_ALIASES);
Weird spacing around the "&" above (3 spaces indent, 2 spaces after the
"&").
I would align this as:
env->cpuid_ext2_features |= (env->cpuid_features &
CPUID_EXT2_AMD_ALIASES);
As the above issues are only cosmetic:
Reviewed-by: Eduardo Habkost
> +}
> +
> if (!kvm_enabled()) {
> env->cpuid_features &= TCG_FEATURES;
> env->cpuid_ext_features &= TCG_EXT_FEATURES;
> --
> 1.7.1
>
--
Eduardo
Re: [Qemu-devel] [PATCH 6/6] target-i386: move out CPU features initialization in separate func
On Tue, Dec 11, 2012 at 11:11:06AM +0100, Igor Mammedov wrote:
> No functional change, a simple code movement to simplify following
> refactoring.
>
> Signed-off-by: Igor Mammedov
Reviewed-by: Eduardo Habkost
> ---
> v2:
> - rebased on top of "i386: cpu: remove duplicate feature names"
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg129458.html
> v3:
> - rebased on top of 1.3-rc0 & splitted cpu_x86_find_by_name()
> - AMD's ext2_features filtering are moved into separate patch
> ---
> target-i386/cpu.c | 53
> ++---
> 1 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e534254..3b9bbfe 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1235,6 +1235,34 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
> Visitor *v, void *opaque,
> cpu->env.tsc_khz = value / 1000;
> }
>
> +static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
> +{
> +CPUX86State *env = &cpu->env;
> +
> +assert(def->vendor1);
> +env->cpuid_vendor1 = def->vendor1;
> +env->cpuid_vendor2 = def->vendor2;
> +env->cpuid_vendor3 = def->vendor3;
> +env->cpuid_vendor_override = def->vendor_override;
> +object_property_set_int(OBJECT(cpu), def->level, "level", errp);
> +object_property_set_int(OBJECT(cpu), def->family, "family", errp);
> +object_property_set_int(OBJECT(cpu), def->model, "model", errp);
> +object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
> +env->cpuid_features = def->features;
> +env->cpuid_ext_features = def->ext_features;
> +env->cpuid_ext2_features = def->ext2_features;
> +env->cpuid_ext3_features = def->ext3_features;
> +object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
> +env->cpuid_kvm_features = def->kvm_features;
> +env->cpuid_svm_features = def->svm_features;
> +env->cpuid_ext4_features = def->ext4_features;
> +env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> +env->cpuid_xlevel2 = def->xlevel2;
> +object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
> +"tsc-frequency", errp);
> +object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
> +}
> +
> static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> {
> x86_def_t *def;
> @@ -1513,7 +1541,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
>
> int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> {
> -CPUX86State *env = &cpu->env;
> x86_def_t def1, *def = &def1;
> Error *error = NULL;
> char *name, *features;
> @@ -1541,29 +1568,9 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model)
> if (cpu_x86_parse_featurestr(def, features) < 0) {
> goto error;
> }
> -assert(def->vendor1);
> -env->cpuid_vendor1 = def->vendor1;
> -env->cpuid_vendor2 = def->vendor2;
> -env->cpuid_vendor3 = def->vendor3;
> -env->cpuid_vendor_override = def->vendor_override;
> -object_property_set_int(OBJECT(cpu), def->level, "level", &error);
> -object_property_set_int(OBJECT(cpu), def->family, "family", &error);
> -object_property_set_int(OBJECT(cpu), def->model, "model", &error);
> -object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
> -env->cpuid_features = def->features;
> -env->cpuid_ext_features = def->ext_features;
> -env->cpuid_ext2_features = def->ext2_features;
> -env->cpuid_ext3_features = def->ext3_features;
> -object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
> -env->cpuid_kvm_features = def->kvm_features;
> -env->cpuid_svm_features = def->svm_features;
> -env->cpuid_ext4_features = def->ext4_features;
> -env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> -env->cpuid_xlevel2 = def->xlevel2;
> -object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
> -"tsc-frequency", &error);
>
> -object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
> +cpudef_2_x86_cpu(cpu, def, &error);
> +
> if (error) {
> fprintf(stderr, "%s\n", error_get_pretty(error));
> error_free(error);
> --
> 1.7.1
>
--
Eduardo
Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cpu.c
On Mon, 10 Dec 2012 22:30:42 -0200
Eduardo Habkost wrote:
> As we will need to create the CPU object after splitting the CPU model
> string (because we're going to use different subclasses for each CPU
> model), move the CPU object creation to cpu_x86_register(), and at the
> same time rename cpu_x86_register() to cpu_x86_create().
perhaps it would be better to move cpu_x86_init() inside cpu.c and embed
cpu_x86_register() in it, as you have done in one of yours series, and avoid
creating intermediate cpu_x86_create() when the end result of cpu subclasses
& properties should become a simple cpu_x86_init():
1. lookup class name, 2. get cpu instance , 3. set properties, 4. realize()
>
> This will also simplify the CPU creation code to a trivial
> cpu_x86_create()+cpu_x86_realize() sequence. This will be useful for
> code that have to set additional properties before cpu_x86_realize() is
> called (e.g. the PC CPU initialization code, that needs to set APIC IDs
> depending on the CPU cores/threads topology).
>
> Signed-off-by: Eduardo Habkost
> ---
> target-i386/cpu.c| 17 +
> target-i386/cpu.h| 2 +-
> target-i386/helper.c | 9 ++---
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 7be3ad8..044e2d9 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1488,14 +1488,22 @@ static void filter_features_for_kvm(X86CPU *cpu)
> }
> #endif
>
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> +/* Create and initialize a X86CPU object, based on the full CPU model
> string
> + * (that may include "+feature,-feature,feature=xxx" feature strings)
> + */
> +X86CPU *cpu_x86_create(const char *cpu_model)
> {
> -CPUX86State *env = &cpu->env;
> +X86CPU *cpu;
> +CPUX86State *env;
> x86_def_t def1, *def = &def1;
> Error *error = NULL;
> char *name, *features;
> gchar **model_pieces;
>
> +cpu = X86_CPU(object_new(TYPE_X86_CPU));
> +env = &cpu->env;
> +env->cpu_model_str = cpu_model;
> +
> memset(def, 0, sizeof(*def));
>
> model_pieces = g_strsplit(cpu_model, ",", 2);
> @@ -1572,10 +1580,11 @@ int cpu_x86_register(X86CPU *cpu, const char
> *cpu_model) }
>
> g_strfreev(model_pieces);
> -return 0;
> +return cpu;
> error:
> +object_delete(OBJECT(cpu));
> g_strfreev(model_pieces);
> -return -1;
> +return NULL;
> }
>
> #if !defined(CONFIG_USER_ONLY)
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 386c4f6..3ebaae9 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -980,7 +980,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
> void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> uint32_t *eax, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx);
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
> +X86CPU *cpu_x86_create(const char *cpu_model);
> void cpu_clear_apic_feature(CPUX86State *env);
> void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t
> *edx); diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf206cf..23af4a8 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1243,15 +1243,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env,
> unsigned int selector, X86CPU *cpu_x86_init(const char *cpu_model)
> {
> X86CPU *cpu;
> -CPUX86State *env;
> Error *error = NULL;
>
> -cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -env = &cpu->env;
> -env->cpu_model_str = cpu_model;
> -
> -if (cpu_x86_register(cpu, cpu_model) < 0) {
> -object_delete(OBJECT(cpu));
> +cpu = cpu_x86_create(cpu_model);
> +if (!cpu) {
> return NULL;
> }
>
