Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

2014-08-19 Thread Eric Auger
On 08/18/2014 11:54 PM, Joel Schopp wrote:
> 
> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque)
> +{
> +PlatformDevtreeData *data = opaque;
> +void *fdt = data->fdt;
> +const char *parent_node = data->node;
> +int compat_str_len;
> +char *nodename;
> +int i, ret;
> +uint32_t *irq_attr;
> +uint64_t *reg_attr;
> +uint64_t mmio_base;
> +uint64_t irq_number;
> +gchar mmio_base_prop[8];
> +gchar irq_number_prop[8];
> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> +VFIODevice *vbasedev = &vdev->vbasedev;
> +Object *obj = OBJECT(sbdev);
> +
> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
> +
> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
> +   vbasedev->name,
> +   mmio_base);
> +
> +qemu_fdt_add_subnode(fdt, nodename);
> +
> +compat_str_len = strlen(vdev->compat) + 1;
> +qemu_fdt_setprop(fdt, nodename, "compatible",
> +vdev->compat, compat_str_len);
> +
> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
> +
> +for (i = 0; i < vbasedev->num_regions; i++) {
> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i);
> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL);
> +reg_attr[2*i] = 1;
> +reg_attr[2*i+1] = mmio_base;
> +reg_attr[2*i+2] = 1;
> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem);
> +}
> 
> This should be 4 instead of 2. 
Hi Joel,

Yes definitively! Forgot to restore the original value after trying
different qemu_fdt_setprop_* functions. sorry for that.

Best Regards

Eric
> Also, to support 64 bit systems I think this should be 2 instead of 1.
> 




Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *

2014-08-19 Thread Markus Armbruster
Max Reitz  writes:

> On 18.08.2014 18:10, Markus Armbruster wrote:
>> They clutter the code.  Unfortunately, I can't figure out how to make
>> Coccinelle drop all of them, so I have to settle for common special
>> cases:
>>
>>  @@
>>  type T;
>>  T *pt;
>>  void *pv;
>>  @@
>>  - pt = (T *)pv;
>>  + pt = pv;
>>  @@
>>  type T;
>>  @@
>>  - (T *)
>>(\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\|
>>   g_try_malloc\|g_try_malloc0\|g_try_realloc\|
>>   g_try_new\|g_try_new0\|g_try_renew\)(...))
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>   block/vhdx-log.c| 2 +-
>>   block/vvfat.c   | 8 
>>   hw/ide/microdrive.c | 2 +-
>>   3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
>> index eb5c7a0..4267e60 100644
>> --- a/block/vhdx-log.c
>> +++ b/block/vhdx-log.c
>> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, 
>> BDRVVHDXState *s,
>>   buffer = qemu_blockalign(bs, total_length);
>>   memcpy(buffer, &new_hdr, sizeof(new_hdr));
>>   -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr));
>> +new_desc = (buffer + sizeof(new_hdr));
>>   data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE);
>>   data_tmp = data;
>
> You could drop the parantheses here. Also, I still don't like void
> pointer arithmetic, but well... ;-)

Separate issue, related because it would put the cast right back.

We use plenty of GCCisms.  If we ever want to port to a toolchain that
doesn't support them, adding the cast clutter to avoid arithmetic on
void * will be the least of our worries.

If we decide to hunt down this particular GCCism anyway, there's
-Wpointer-arith.  Without that, it's a game of whack-a-mole.

> Reviewed-by: Max Reitz 

Thanks!



Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *

2014-08-19 Thread Markus Armbruster
Jeff Cody  writes:

> On Mon, Aug 18, 2014 at 06:10:43PM +0200, Markus Armbruster wrote:
>> They clutter the code.  Unfortunately, I can't figure out how to make
>> Coccinelle drop all of them, so I have to settle for common special
>> cases:
>> 
>> @@
>> type T;
>> T *pt;
>> void *pv;
>> @@
>> - pt = (T *)pv;
>> + pt = pv;
>> @@
>> type T;
>> @@
>> - (T *)
>>   (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\|
>>   g_try_malloc\|g_try_malloc0\|g_try_realloc\|
>>   g_try_new\|g_try_new0\|g_try_renew\)(...))
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/vhdx-log.c| 2 +-
>>  block/vvfat.c   | 8 
>>  hw/ide/microdrive.c | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
>> index eb5c7a0..4267e60 100644
>> --- a/block/vhdx-log.c
>> +++ b/block/vhdx-log.c
>> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, 
>> BDRVVHDXState *s,
>>  buffer = qemu_blockalign(bs, total_length);
>>  memcpy(buffer, &new_hdr, sizeof(new_hdr));
>>  
>> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr));
>> +new_desc = (buffer + sizeof(new_hdr));
>
> Agree with Max, in that the parenthesis could be removed.  Not worthy
> of a respin normally, but since the point of this patch is to
> unclutter the code, I guess it makes sense to fix it.

Max and you are right.  I neglected to clean up the patch produced by
spatch.

>>  data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE);
>>  data_tmp = data;
>>  
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index f877e85..401539d 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int 
>> mapping_index)
>>  if(first_cluster == 0 && (is_dotdot || is_dot))
>>  continue;
>>  
>> -buffer=(char*)g_malloc(length);
>> +buffer=g_malloc(length);
>
> You missed a spot to put spaces around the '=' here.  Nothing worthy
> of a respin, of course - just a note in case you decide to respin.

Right again.  Although perhaps vvfat should remain ugly, to warn unwary
travellers ;)

> [...]
>
> With or without the changes above:
>
> Reviewed-by: Jeff Cody 

Thanks!



Re: [Qemu-devel] [PATCH 1/3] docs: List all image elements currently supported by the fuzzer

2014-08-19 Thread Fam Zheng
On Mon, 08/11 15:55, Maria Kustova wrote:
> Signed-off-by: Maria Kustova 
> ---
>  docs/image-fuzzer.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt
> index 0d0005d..f707269 100644
> --- a/docs/image-fuzzer.txt
> +++ b/docs/image-fuzzer.txt
> @@ -125,7 +125,8 @@ If a fuzzer configuration is specified, then it has the 
> next interpretation:
>  will be always fuzzed for every test. This case is useful for regression
>  testing.
>  
> -For now only header fields, header extensions and L1/L2 tables are generated.
> +The generator can create header fields, header extensions, L1/L2 tables and
> +refcount blocks and table.

"refcount table and blocks" might read better, but it doesn't hurt.

Reviewed-by: Fam Zheng 

>  
>  Module interfaces
>  -
> -- 
> 1.9.3
> 



Re: [Qemu-devel] [Qemu-trivial] [PATCH v6 03/10] virtio-blk: fix reference a pointer which might be freed

2014-08-19 Thread Michael Tokarev
19.08.2014 00:17, Michael S. Tsirkin wrote:
[]
> By the way, could you please add Cc qemu-stable on bugfixes
> you have queued?
> These are likely appopriate for 2.1.1.

Actually I've added Cc: qemu-stable@ in the commit message.
So it will go to stable (or should) once I'll send a pull
request.

Thanks,

/mjt



[Qemu-devel] [PATCH] pcihp: fix possible array out of bounds

2014-08-19 Thread arei.gonglei
From: Gonglei 

When 'bsel == ACPI_PCIHP_MAX_HOTPLUG_BUS', the
s->acpi_pcihp_pci_status[bsel] array will out of bounds.

Add check for this.

Signed-off-by: Gonglei 
---
 hw/acpi/pcihp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index fae663a..34dedf1 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -231,7 +231,7 @@ static uint64_t pci_read(void *opaque, hwaddr addr, 
unsigned int size)
 uint32_t val = 0;
 int bsel = s->hotplug_select;
 
-if (bsel < 0 || bsel > ACPI_PCIHP_MAX_HOTPLUG_BUS) {
+if (bsel < 0 || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
 return 0;
 }
 
-- 
1.7.12.4





Re: [Qemu-devel] [PATCH] configure: no need to mkdir QMP

2014-08-19 Thread Michael Tokarev
Applied to -trivial, thank you!

/mjt



Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

2014-08-19 Thread Eric Auger
On 08/19/2014 12:11 AM, Peter Maydell wrote:
> On 18 August 2014 22:54, Joel Schopp  wrote:
>>
>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque)
>> +{
>> +PlatformDevtreeData *data = opaque;
>> +void *fdt = data->fdt;
>> +const char *parent_node = data->node;
>> +int compat_str_len;
>> +char *nodename;
>> +int i, ret;
>> +uint32_t *irq_attr;
>> +uint64_t *reg_attr;
>> +uint64_t mmio_base;
>> +uint64_t irq_number;
>> +gchar mmio_base_prop[8];
>> +gchar irq_number_prop[8];
>> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
>> +VFIODevice *vbasedev = &vdev->vbasedev;
>> +Object *obj = OBJECT(sbdev);
>> +
>> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>> +
>> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
>> +   vbasedev->name,
>> +   mmio_base);
>> +
>> +qemu_fdt_add_subnode(fdt, nodename);
>> +
>> +compat_str_len = strlen(vdev->compat) + 1;
> 
> At this point you've already substituted the NULs in,
> so you can't call strlen(), I think.
Hi Peter,

yes you're right. Thanks
> 
>> +qemu_fdt_setprop(fdt, nodename, "compatible",
>> +vdev->compat, compat_str_len);
>> +
>> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>> +
>> +for (i = 0; i < vbasedev->num_regions; i++) {
>> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i);
>> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL);
>> +reg_attr[2*i] = 1;
>> +reg_attr[2*i+1] = mmio_base;
>> +reg_attr[2*i+2] = 1;
>> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem);
>> +}
>>
>> This should be 4 instead of 2.
>> Also, to support 64 bit systems I think this should be 2 instead of 1.
> 
> Actually it depends entirely on what the board has done to
> create the device tree node that we're inserting this child
> node into. For ARM boot.c sets both #address-cells and
> #size-cells to 2 regardless of whether the system is 32
> or 64 bits, for simplicity. I imagine PPC does something
> different. If we're editing a dtb that the user passed in (which
> I think would be pretty lunatic so we shouldn't do this)
> we'd have to actually walk the dtb to try to figure out what
> the semantics of the reg property should be.

Putting size=1 was the only solution I found to use an offset relative
to the parent bus instead of an absolute base address. I would explain
this because, in platform_bus_create_devtree, the function that creates
the "platform bus" node, #address-cells and #size-cells currently are
set to 1. I assume the motivation was that bus size was supposed to be
smaller than 4GB. Then I guess the problem is shifted to the inclusion
of the platform bus in any ARM platform.

Thanks

Eric
> 
> thanks
> -- PMM
> 




Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] slirp/misc: Use g_malloc() instead of malloc()

2014-08-19 Thread zhanghailiang

On 2014/8/18 19:32, Michael Tokarev wrote:

18.08.2014 11:51, zhanghailiang пишет:

Here we don't check the return value of malloc() which may fail.
Use the g_malloc() instead, which will abort the program when
there is not enough memory.

Signed-off-by: zhanghailiang
Reviewed-by: Alex Bennée
---
  slirp/misc.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index b8eb74c..f7fe497 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -54,7 +54,7 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char *exec,
}

tmp_ptr = *ex_ptr;
-   *ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
+   *ex_ptr = (struct ex_list *)g_malloc(sizeof(struct ex_list));


There's a convinient macro in glib, g_new(typename, numelts).  Also
there's a less commonly used g_renew() which is like realloc, but it
is not applicable here.


Hmm, it is a good idea to use g_new instead of g_malloc,
we have to perform type cast for g_malloc.(BTW, i found in qemu there
are several places use g_malloc but not perform appropriate type
coercions)
I will modify this.Thanks :)


(*ex_ptr)->ex_fport = port;
(*ex_ptr)->ex_addr = addr;
(*ex_ptr)->ex_pty = do_pty;
@@ -235,7 +235,7 @@ strdup(str)
  {
char *bptr;

-   bptr = (char *)malloc(strlen(str)+1);
+   bptr = (char *)g_malloc(strlen(str)+1);
strcpy(bptr, str);

return bptr;


Oh.  And this one should be removed completely.  It is a reimplementation
of strdup() for system which lacks it.  This code should go, we don't build


I couldn't agree more, i will removed it.


on such a system anyway and we always have g_strdup().  There's one more
usage of strdup() in this file, btw.



OK, Thanks.


I'm sorry for being so picky, and you're already at v7, but heck.. We should
be more active at reviewing patches :)



Aha! ;),i really appreciate your help. I learned a lot. Thanks.


Thanks,

/mjt

.







Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] slirp/misc: Use g_malloc() instead of malloc()

2014-08-19 Thread Michael Tokarev
19.08.2014 11:30, zhanghailiang wrote:
[]
> Hmm, it is a good idea to use g_new instead of g_malloc,
> we have to perform type cast for g_malloc.(BTW, i found in qemu there
> are several places use g_malloc but not perform appropriate type
> coercions)

There's no need to perform explicit type conversion from void* to type*.

Thanks,

/mjt



Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

2014-08-19 Thread Eric Auger
On 08/19/2014 12:26 AM, Joel Schopp wrote:
> 
> On 08/18/2014 05:11 PM, Peter Maydell wrote:
>> On 18 August 2014 22:54, Joel Schopp  wrote:
>>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque)
>>> +{
>>> +PlatformDevtreeData *data = opaque;
>>> +void *fdt = data->fdt;
>>> +const char *parent_node = data->node;
>>> +int compat_str_len;
>>> +char *nodename;
>>> +int i, ret;
>>> +uint32_t *irq_attr;
>>> +uint64_t *reg_attr;
>>> +uint64_t mmio_base;
>>> +uint64_t irq_number;
>>> +gchar mmio_base_prop[8];
>>> +gchar irq_number_prop[8];
>>> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
>>> +VFIODevice *vbasedev = &vdev->vbasedev;
>>> +Object *obj = OBJECT(sbdev);
>>> +
>>> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>>> +
>>> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
>>> +   vbasedev->name,
>>> +   mmio_base);
>>> +
>>> +qemu_fdt_add_subnode(fdt, nodename);
>>> +
>>> +compat_str_len = strlen(vdev->compat) + 1;
>> At this point you've already substituted the NULs in,
>> so you can't call strlen(), I think.
>>
>>> +qemu_fdt_setprop(fdt, nodename, "compatible",
>>> +vdev->compat, compat_str_len);
>>> +
>>> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>>> +
>>> +for (i = 0; i < vbasedev->num_regions; i++) {
>>> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i);
>>> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL);
>>> +reg_attr[2*i] = 1;
>>> +reg_attr[2*i+1] = mmio_base;
>>> +reg_attr[2*i+2] = 1;
>>> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem);
>>> +}
>>>
>>> This should be 4 instead of 2.
>>> Also, to support 64 bit systems I think this should be 2 instead of 1.
>> Actually it depends entirely on what the board has done to
>> create the device tree node that we're inserting this child
>> node into. For ARM boot.c sets both #address-cells and
>> #size-cells to 2 regardless of whether the system is 32
>> or 64 bits, for simplicity. I imagine PPC does something
>> different. If we're editing a dtb that the user passed in (which
>> I think would be pretty lunatic so we shouldn't do this)
>> we'd have to actually walk the dtb to try to figure out what
>> the semantics of the reg property should be.
> For the index [2*i],[2*i+1], etc is clearly a bug as when i = 1 it will
> overwrite two of the values.  Changing that to [4*i],[4*i+1],etc fixes it.
> 
> I think you are right on the size.  I also wonder if the user doesn't
> pass in a dtb if qemu should try to recreate the device-tree entry from
> the platform device entry in the host kernel?  If so would that best be
> done by recreating the values from /proc/device-tree ?
Antonios recently submitted a patch to retrieve dt info from the vfio
platform device.
[RFC 0/4] VFIO: PLATFORM: Return device tree info for a platform device node
https://www.mail-archive.com/kvm@vger.kernel.org/msg106282.html

Best Regards

Eric
> 
> I also wish that qemu had a flag to output the generated dtb to a file
> much like lkvm (kvmtool) has.
> 




Re: [Qemu-devel] [PATCH 2/3] fuzz: Add fuzzing functions for entries of refcount table and blocks

2014-08-19 Thread Fam Zheng
On Mon, 08/11 15:55, Maria Kustova wrote:
> Signed-off-by: Maria Kustova 
> ---
>  tests/image-fuzzer/qcow2/fuzz.py | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/image-fuzzer/qcow2/fuzz.py 
> b/tests/image-fuzzer/qcow2/fuzz.py
> index 57527f9..5852b4d 100644
> --- a/tests/image-fuzzer/qcow2/fuzz.py
> +++ b/tests/image-fuzzer/qcow2/fuzz.py
> @@ -18,8 +18,8 @@
>  
>  import random
>  
> -
>  UINT8 = 0xff
> +UINT16 = 0x
>  UINT32 = 0x
>  UINT64 = 0x
>  # Most significant bit orders
> @@ -28,6 +28,8 @@ UINT64_M = 63
>  # Fuzz vectors
>  UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1,
> UINT8]
> +UINT16_V = [0, 0x100, 0x1000, UINT16/4, UINT16/2 - 1, UINT16/2, UINT16/2 + 1,
> +UINT16 - 1, UINT16]
>  UINT32_V = [0, 0x100, 0x1000, 0x1, 0x10, UINT32/4, UINT32/2 - 1,
>  UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32]
>  UINT64_V = UINT32_V + [0x100, 0x1000, 0x1, UINT64/4,

Seeing some pattern in the vectors, but since the types are very few, we can
just construct it like this for now.

> @@ -353,3 +355,15 @@ def l2_entry(current):
>  value = offset + (is_cow << UINT64_M) + \
>  (is_compressed << UINT64_M - 1) + is_zero
>  return value
> +
> +
> +def refcount_table_entry(current):
> +"""Fuzz an entry of the refcount table."""
> +constraints = UINT64_V
> +return selector(current, constraints)
> +
> +
> +def refcount_block_entry(current):
> +"""Fuzz an entry of a refcount block."""
> +constraints = UINT16_V
> +return selector(current, constraints)
> -- 
> 1.9.3
> 

Reviewed-by: Fam Zheng 



Re: [Qemu-devel] [RFC 0/4] Adding -cdrom, -hd[abcd] and -drive file=... to Q35

2014-08-19 Thread Markus Armbruster
John Snow  writes:

> Currently, the drive definitions created by drive_new() when using
> the -drive file=...[,if=ide] or -cdrom or -hd[abcd] options are not
> picked up by the Q35 initialization routine.
>
> To fix this, we have to add hooks to search for these drives using
> something like pc_piix's ide_drive_get and then add them using
> something like pci_ide_create_devs.

ide_drive_get() isn't pc_piix's, it's a helper function in the IDE core
which boards (not just pc_piix) use to find the if=ide drives.  It fills
in an array of DriveInfo.

pci_ide_create_devs() is a helper function in the IDE PCI code which PCI
IDE controllers (not just piix3-ide) use to create IDE devices for an
array of DriveInfo.

> Where it gets slightly wonky is the fact that if=ide is specified
> to use two devices per bus, whereas AHCI does not utilize that
> same master/slave mechanic. Therefore, many places inside of
> blockdev.c where we add and define new drives use incorrect math
> for AHCI devices and try to place them on impossible buses.
> Notably -hdb and -hdd would become inaccessible.

Yes.

> To remedy this, I added a new interface type, IF_AHCI. Corresponding
> to this change, I modified the default machine properties for Q35
> to use this interface as a default.
>
> The changes appear to work well, but where I'd like some feedback
> is what should happen if people do something like:
>
> qemu -M q35 -drive if=ide,file=fedora.qcow2
>
> The code as presented here is not going to look for or attempt to
> connect IDE devices, because it is now looking for /AHCI/ devices.
>
> At worst, this may break a few existing scripts, but I actually think
> that since the if=ide,file=... shorthand never worked to begin with,
> the impact might actually be minimal.
>
> But since the legacy IDE interface of the ICH9 is as of yet unemulated,
> the if=ide drives don't have a reasonable place to go yet. I am also
> not sure what a reasonable way to handle people specifying BOTH
> if=ide and if=ahci drives would be.

We've been through IF_AHCI before, more than once, but that was before
you got involved :)

The problem at hand is that "-drive if=ide" and its sugared forms -hda,
-hdb, -cdrom, ... don't work with Q35.

You provide a solution for the sugared forms, you leave the problem
unsolved for the unsugared form, and you add a new problem: -drive
if=ahci doesn't work with i440FX.  Progress, sort of.

Let's take a step back, and recap previous discussion.  There are two
defensible point of views, in my opinion.

One is that IDE and AHCI should be separate interface types, just like
IDE and SCSI are.

Attempts to define an if=X drive with a board that doesn't provide a
controller for X should fail[*].  Only onboard controllers matter,
add-ons plugged in with -device don't.  An i440FX board provides only
IDE.  A Q35 board provides only AHCI, not IDE.  If we implement an
ich9-ahci legacy mode, and switch it on, then it provides only IDE, not
AHCI.  Or maybe both, depending on how we do it.

The other point of view is that IDE and AHCI are no more different than
the different kinds of SCSI HBAs.  This is certainly true from a qdev
point of view: just like SCSI devices can connect to any SCSI qbus,
regardless of the HBA providing it, so can IDE devices connect to any
IDE qbus, regardless of the controller providing it.

There's a wrinkle: the mapping between index to (bus, unit).  This
mapping is ABI.  The current mapping makes sense for the first
generation of controllers: PATA (two devices per bus, thus
if_max_devs[IF_IDE] = 2), and 8-bit SCSI (seven per bus, thus
if_max_devs[IF_SCSI = 7).

The mapping is silly for newer SCSI HBAs.  Commit 622b520f tried to make
it less silly, but had to be reverted in 27d6bf4 because the silliness
was ABI.

The mapping is also silly for ich9-ahci.  You side-step that silliness
only, by adding a new interface type for it.  But shouldn't we add a
number of SCSI interface types then, too?  Where does that end?

Can we do better?  I think we can, by making this part of the ABI
board-specific.  The general form of the mapping remains

(bus, unit) = (index / N, index % N)

but N now depends on board and interface type, not just the latter.

If the board connects if=scsi to an lsi53c895a, then N = 7.

If the board connects if=ide to an piix3-ide, then N = 2.

If the board connects if=ide to an ich9-ahci, then N = 1.

I trust you get the idea :)


[*] Currently, they're silently ignored with most boards for most X, but
I regard that as implementation defect.



Re: [Qemu-devel] [Question] Why doesn't PCIe hotplug work for Q35 machine?

2014-08-19 Thread Paolo Bonzini
Il 19/08/2014 08:25, Gonglei (Arei) ha scritto:
> 
> 1. Does qemu support ARI Forwarding for PCIe at present? If yes, how to 
> enable it ?

No, not yet.

> 2. If not, we should add some check for PCIe root ports and downstream ports,
>  meanwhile add explaining document.
> 3. Those check should add in general code level, both hotplug and coldplug.

That would be possible, yes.

Paolo



Re: [Qemu-devel] [Question] Why doesn't PCIe hotplug work for Q35 machine?

2014-08-19 Thread Gonglei (Arei)
Hi,

> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Tuesday, August 19, 2014 4:06 PM
> To: Gonglei (Arei); Marcel Apfelbaum; Michael S. Tsirkin
> Cc: qemu-devel@nongnu.org; imamm...@redhat.com; arm...@redhat.com;
> Huangweidong (C)
> Subject: Re: [Question] Why doesn't PCIe hotplug work for Q35 machine?
> 
> Il 19/08/2014 08:25, Gonglei (Arei) ha scritto:
> >
> > 1. Does qemu support ARI Forwarding for PCIe at present? If yes, how to
> enable it ?
> 
> No, not yet.
> 
> > 2. If not, we should add some check for PCIe root ports and downstream
> ports,
> >  meanwhile add explaining document.
> > 3. Those check should add in general code level, both hotplug and coldplug.
> 
> That would be possible, yes.
> 
OK, I got it. Some work in progress for this. 

Thanks again, Paolo.

Best regards,
-Gonglei


Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

2014-08-19 Thread Peter Maydell
On 19 August 2014 08:24, Eric Auger  wrote:
> Putting size=1 was the only solution I found to use an offset relative
> to the parent bus instead of an absolute base address. I would explain
> this because, in platform_bus_create_devtree, the function that creates
> the "platform bus" node, #address-cells and #size-cells currently are
> set to 1. I assume the motivation was that bus size was supposed to be
> smaller than 4GB. Then I guess the problem is shifted to the inclusion
> of the platform bus in any ARM platform.

Ah, I see. Yes, if the containing node is setting addr/size to 1
then 1 is correct, and the limitation then is just the 4GB max.

thanks
-- PMM



Re: [Qemu-devel] [PATCH V3] spapr: Fix stale HTAB during live migration

2014-08-19 Thread Dr. David Alan Gilbert
* Samuel Mendoza-Jonas (sam...@au1.ibm.com) wrote:
> If a guest reboots during a running migration, changes to the
> hash page table are not necessarily updated on the destination.
> Opening a new file descriptor to the HTAB forces the migration
> handler to resend the entire table.

Yes I think that's safe.

> Signed-off-by: Samuel Mendoza-Jonas 
> ---
> Changes in v3: Pointed out by David, htab_save_iterate could
>   potentially try to read before htab_fd is open again.
>   Leave opening the fd to the functions trying to read.
> Changes in v2: Forgot check on kvmppc_get_htab_fd return value
>  hw/ppc/spapr.c | 25 +
>  include/hw/ppc/spapr.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3a6d26d..5b41318 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -997,6 +997,10 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>  /* Kernel handles htab, we don't need to allocate one */
>  spapr->htab_shift = shift;
>  kvmppc_kern_htab = true;
> +
> +/* Check if we are overlapping a migration */
> +if (spapr->htab_fd > 0)
> +spapr->need_reset = true;
>  } else {
>  if (!spapr->htab) {
>  /* Allocate an htab if we don't yet have one */
> @@ -1156,6 +1160,7 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
>  } else {
>  assert(kvm_enabled());
>  
> +spapr->need_reset = false;
>  spapr->htab_fd = kvmppc_get_htab_fd(false);
>  if (spapr->htab_fd < 0) {
>  fprintf(stderr, "Unable to open fd for reading hash table from 
> KVM: %s\n",
> @@ -1309,6 +1314,16 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
>  if (!spapr->htab) {
>  assert(kvm_enabled());
>  
> +if (atomic_cmpxchg(&spapr->need_reset, true, false) == true) {
> +close(spapr->htab_fd);
> +spapr->htab_fd = kvmppc_get_htab_fd(false);
> +if (spapr->htab_fd < 0) {
> +fprintf(stderr, "Unable to open fd for reading hash table 
> from KVM: %s\n",
> +strerror(errno));

Either perror or error_report() with the strerror would seem better.

> +return -1;
> +}
> +}
> +

Why not make a little function for this; it seems a bad idea to have two copies 
of
it.
Also, add a comment saying why you're reopening it.

Dave

>  rc = kvmppc_save_htab(f, spapr->htab_fd,
>MAX_KVM_BUF_SIZE, MAX_ITERATION_NS);
>  if (rc < 0) {
> @@ -1340,6 +1355,16 @@ static int htab_save_complete(QEMUFile *f, void 
> *opaque)
>  
>  assert(kvm_enabled());
>  
> +if (atomic_cmpxchg(&spapr->need_reset, true, false) == true) {
> +close(spapr->htab_fd);
> +spapr->htab_fd = kvmppc_get_htab_fd(false);
> +if (spapr->htab_fd < 0) {
> +fprintf(stderr, "Unable to open fd for reading hash table 
> from KVM: %s\n",
> +strerror(errno));
> +return -1;
> +}
> +}
> +
>  rc = kvmppc_save_htab(f, spapr->htab_fd, MAX_KVM_BUF_SIZE, -1);
>  if (rc < 0) {
>  return rc;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 0c2e3c5..9ab9827 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -71,6 +71,7 @@ typedef struct sPAPREnvironment {
>  int htab_save_index;
>  bool htab_first_pass;
>  int htab_fd;
> +bool need_reset;
>  
>  /* state for Dynamic Reconfiguration Connectors */
>  sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE];
> -- 
> 1.9.3
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PATCH v8] slirp/misc: Use the GLib memory allocation APIs

2014-08-19 Thread zhanghailiang
Here we don't check the return value of malloc() which may fail.
Use the g_new() instead, which will abort the program when
there is not enough memory.

Also, use g_strdup instead of strdup and remove the unnecessary
strdup function.

Signed-off-by: zhanghailiang 
Reviewed-by: Alex Bennée 
---
 slirp/misc.c | 20 +++-
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index b8eb74c..6543dc7 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -54,11 +54,11 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char 
*exec,
}
 
tmp_ptr = *ex_ptr;
-   *ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
+   *ex_ptr = g_new(struct ex_list, 1);
(*ex_ptr)->ex_fport = port;
(*ex_ptr)->ex_addr = addr;
(*ex_ptr)->ex_pty = do_pty;
-   (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : strdup(exec);
+   (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : g_strdup(exec);
(*ex_ptr)->ex_next = tmp_ptr;
return 0;
 }
@@ -187,7 +187,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
   bptr++;
c = *bptr;
*bptr++ = (char)0;
-   argv[i++] = strdup(curarg);
+   argv[i++] = g_strdup(curarg);
   } while (c);
 
 argv[i] = NULL;
@@ -228,20 +228,6 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 }
 #endif
 
-#ifndef HAVE_STRDUP
-char *
-strdup(str)
-   const char *str;
-{
-   char *bptr;
-
-   bptr = (char *)malloc(strlen(str)+1);
-   strcpy(bptr, str);
-
-   return bptr;
-}
-#endif
-
 void slirp_connection_info(Slirp *slirp, Monitor *mon)
 {
 const char * const tcpstates[] = {
-- 
1.7.12.4





[Qemu-devel] [PATCH v3 4/4] block: Drop some superfluous casts from void *

2014-08-19 Thread Markus Armbruster
They clutter the code.  Unfortunately, I can't figure out how to make
Coccinelle drop all of them, so I have to settle for common special
cases:

@@
type T;
T *pt;
void *pv;
@@
- pt = (T *)pv;
+ pt = pv;
@@
type T;
@@
- (T *)
  (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\|
 g_try_malloc\|g_try_malloc0\|g_try_realloc\|
 g_try_new\|g_try_new0\|g_try_renew\)(...))

Topped off with minor manual style cleanups.

Signed-off-by: Markus Armbruster 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
---
 block/vhdx-log.c| 2 +-
 block/vvfat.c   | 8 
 hw/ide/microdrive.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index eb5c7a0..6547bec 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, 
BDRVVHDXState *s,
 buffer = qemu_blockalign(bs, total_length);
 memcpy(buffer, &new_hdr, sizeof(new_hdr));
 
-new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr));
+new_desc = buffer + sizeof(new_hdr);
 data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE);
 data_tmp = data;
 
diff --git a/block/vvfat.c b/block/vvfat.c
index f877e85..e56f766 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int 
mapping_index)
if(first_cluster == 0 && (is_dotdot || is_dot))
continue;
 
-   buffer=(char*)g_malloc(length);
+   buffer = g_malloc(length);
snprintf(buffer,length,"%s/%s",dirname,entry->d_name);
 
if(stat(buffer,&st)<0) {
@@ -767,7 +767,7 @@ static int read_directory(BDRVVVFATState* s, int 
mapping_index)
 
/* create mapping for this file */
if(!is_dot && !is_dotdot && (S_ISDIR(st.st_mode) || st.st_size)) {
-   s->current_mapping=(mapping_t*)array_get_next(&(s->mapping));
+   s->current_mapping = array_get_next(&(s->mapping));
s->current_mapping->begin=0;
s->current_mapping->end=st.st_size;
/*
@@ -811,12 +811,12 @@ static int read_directory(BDRVVVFATState* s, int 
mapping_index)
 }
 
  /* reget the mapping, since s->mapping was possibly realloc()ed */
-mapping = (mapping_t*)array_get(&(s->mapping), mapping_index);
+mapping = array_get(&(s->mapping), mapping_index);
 first_cluster += (s->directory.next - mapping->info.dir.first_dir_index)
* 0x20 / s->cluster_size;
 mapping->end = first_cluster;
 
-direntry = (direntry_t*)array_get(&(s->directory), mapping->dir_index);
+direntry = array_get(&(s->directory), mapping->dir_index);
 set_begin_of_direntry(direntry, mapping->begin);
 
 return 0;
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 2d70ddb..15671b8 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -567,7 +567,7 @@ PCMCIACardState *dscm1_init(DriveInfo *dinfo)
 }
 md->bus.ifs[0].drive_kind = IDE_CFATA;
 md->bus.ifs[0].mdata_size = METADATA_SIZE;
-md->bus.ifs[0].mdata_storage = (uint8_t *) g_malloc0(METADATA_SIZE);
+md->bus.ifs[0].mdata_storage = g_malloc0(METADATA_SIZE);
 
 return PCMCIA_CARD(md);
 }
-- 
1.9.3




[Qemu-devel] [PATCH v3 1/4] block: Use g_new() & friends where that makes obvious sense

2014-08-19 Thread Markus Armbruster
g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

Patch created with Coccinelle, with two manual changes on top:

* Add const to bdrv_iterate_format() to keep the types straight

* Convert the allocation in bdrv_drop_intermediate(), which Coccinelle
  inexplicably misses

Coccinelle semantic patch:

@@
type T;
@@
-g_malloc(sizeof(T))
+g_new(T, 1)
@@
type T;
@@
-g_try_malloc(sizeof(T))
+g_try_new(T, 1)
@@
type T;
@@
-g_malloc0(sizeof(T))
+g_new0(T, 1)
@@
type T;
@@
-g_try_malloc0(sizeof(T))
+g_try_new0(T, 1)
@@
type T;
expression n;
@@
-g_malloc(sizeof(T) * (n))
+g_new(T, n)
@@
type T;
expression n;
@@
-g_try_malloc(sizeof(T) * (n))
+g_try_new(T, n)
@@
type T;
expression n;
@@
-g_malloc0(sizeof(T) * (n))
+g_new0(T, n)
@@
type T;
expression n;
@@
-g_try_malloc0(sizeof(T) * (n))
+g_try_new0(T, n)
@@
type T;
expression p, n;
@@
-g_realloc(p, sizeof(T) * (n))
+g_renew(T, p, n)
@@
type T;
expression p, n;
@@
-g_try_realloc(p, sizeof(T) * (n))
+g_try_renew(T, p, n)

Signed-off-by: Markus Armbruster 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
---
 block-migration.c  |  6 +++---
 block.c| 14 +++---
 block/archipelago.c|  6 +++---
 block/gluster.c|  8 
 block/iscsi.c  |  2 +-
 block/nfs.c|  2 +-
 block/qcow.c   |  2 +-
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c |  8 
 block/qcow2-snapshot.c |  8 
 block/raw-posix.c  |  2 +-
 block/rbd.c|  4 ++--
 block/sheepdog.c   |  4 ++--
 block/vdi.c|  2 +-
 block/vhdx.c   |  4 ++--
 block/vmdk.c   |  7 +++
 block/vvfat.c  |  2 +-
 blockdev-nbd.c |  2 +-
 blockdev.c |  2 +-
 hw/ide/ahci.c  |  2 +-
 qemu-io-cmds.c |  2 +-
 qemu-io.c  |  2 +-
 22 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index ba3ed36..3ad31a2 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -283,7 +283,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState 
*bmds)
 nr_sectors = total_sectors - cur_sector;
 }
 
-blk = g_malloc(sizeof(BlkMigBlock));
+blk = g_new(BlkMigBlock, 1);
 blk->buf = g_malloc(BLOCK_SIZE);
 blk->bmds = bmds;
 blk->sector = cur_sector;
@@ -354,7 +354,7 @@ static void init_blk_migration_it(void *opaque, 
BlockDriverState *bs)
 return;
 }
 
-bmds = g_malloc0(sizeof(BlkMigDevState));
+bmds = g_new0(BlkMigDevState, 1);
 bmds->bs = bs;
 bmds->bulk_completed = 0;
 bmds->total_sectors = sectors;
@@ -465,7 +465,7 @@ static int mig_save_device_dirty(QEMUFile *f, 
BlkMigDevState *bmds,
 } else {
 nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK;
 }
-blk = g_malloc(sizeof(BlkMigBlock));
+blk = g_new(BlkMigBlock, 1);
 blk->buf = g_malloc(BLOCK_SIZE);
 blk->bmds = bmds;
 blk->sector = sector;
diff --git a/block.c b/block.c
index 6fa0201..712f5db 100644
--- a/block.c
+++ b/block.c
@@ -351,7 +351,7 @@ BlockDriverState *bdrv_new(const char *device_name, Error 
**errp)
 return NULL;
 }
 
-bs = g_malloc0(sizeof(BlockDriverState));
+bs = g_new0(BlockDriverState, 1);
 QLIST_INIT(&bs->dirty_bitmaps);
 pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
 if (device_name[0] != '\0') {
@@ -2628,7 +2628,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
  * into our deletion queue, until we hit the 'base'
  */
 while (intermediate) {
-intermediate_state = g_malloc0(sizeof(BlkIntermediateStates));
+intermediate_state = g_new0(BlkIntermediateStates, 1);
 intermediate_state->bs = intermediate;
 QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry);
 
@@ -3755,7 +3755,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
char *name),
 }
 
 if (!found) {
-formats = g_realloc(formats, (count + 1) * sizeof(char *));
+formats = g_renew(const char *, formats, count + 1);
 formats[count++] = drv->format_name;
 it(opaque, drv->format_name);
 }
@@ -5330,7 +5330,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
 errno = -bitmap_size;
 return NULL;
 }
-bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
+bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_all

[Qemu-devel] [PATCH v3 0/4] block: Use g_new() & friends more

2014-08-19 Thread Markus Armbruster
PATCH 1+2 convert some allocations.  While preparing them, I stumbled
over dead error handling and some useless casts, which led to PATCH
3+4.

I posted a tree-wide version of PATCH 1 some time ago, and was told to
split it up.  This is the block part, redone from scratch.  Other
parts available on request.

v3:
* Manual style cleanups in PATCH 4 [Max, Jeff]
* PATCH 1-3 unchanged.
v2:
* Regenerated with spatch.
* Because of that, I decided not to carry Max's R-by forward.

Markus Armbruster (4):
  block: Use g_new() & friends where that makes obvious sense
  block: Use g_new() & friends to avoid multiplying sizes
  qemu-io-cmds: g_renew() can't fail, bury dead error handling
  block: Drop some superfluous casts from void *

 block-migration.c  |  6 +++---
 block.c| 14 +++---
 block/archipelago.c|  6 +++---
 block/bochs.c  |  2 +-
 block/gluster.c|  8 
 block/iscsi.c  |  2 +-
 block/nfs.c|  2 +-
 block/parallels.c  |  2 +-
 block/qcow.c   |  2 +-
 block/qcow2-cache.c|  2 +-
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c |  8 
 block/qcow2-snapshot.c |  8 
 block/qed-check.c  |  3 +--
 block/raw-posix.c  |  2 +-
 block/rbd.c|  6 +++---
 block/sheepdog.c   |  6 +++---
 block/vdi.c|  2 +-
 block/vhdx-log.c   |  2 +-
 block/vhdx.c   |  4 ++--
 block/vmdk.c   |  7 +++
 block/vvfat.c  | 10 +-
 blockdev-nbd.c |  2 +-
 blockdev.c |  2 +-
 hw/block/nvme.c|  8 
 hw/ide/ahci.c  |  2 +-
 hw/ide/microdrive.c|  2 +-
 qemu-io-cmds.c | 21 ++---
 qemu-io.c  |  2 +-
 29 files changed, 67 insertions(+), 78 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH v3 2/4] block: Use g_new() & friends to avoid multiplying sizes

2014-08-19 Thread Markus Armbruster
g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons.
One, it catches multiplication overflowing size_t.  Two, it returns
T * rather than void *, which lets the compiler catch more type
errors.

Perhaps a conversion to g_malloc_n() would be neater in places, but
that's merely four years old, and we can't use such newfangled stuff.

This commit only touches allocations with size arguments of the form
sizeof(T), plus two that use 4 instead of sizeof(uint32_t).  We can
make the others safe by converting to g_malloc_n() when it becomes
available to us in a couple of years.

Signed-off-by: Markus Armbruster 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
---
 block/bochs.c   |  2 +-
 block/parallels.c   |  2 +-
 block/qcow2-cache.c |  2 +-
 block/qed-check.c   |  3 +--
 block/rbd.c |  2 +-
 block/sheepdog.c|  2 +-
 hw/block/nvme.c |  8 
 qemu-io-cmds.c  | 10 +-
 8 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 6674b27..199ac2b 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -131,7 +131,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EFBIG;
 }
 
-s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size);
 if (s->catalog_size && s->catalog_bitmap == NULL) {
 error_setg(errp, "Could not allocate memory for catalog");
 return -ENOMEM;
diff --git a/block/parallels.c b/block/parallels.c
index 1774ab8..2a814f3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -121,7 +121,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 ret = -EFBIG;
 goto fail;
 }
-s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size);
 if (s->catalog_size && s->catalog_bitmap == NULL) {
 ret = -ENOMEM;
 goto fail;
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 5353b44..fe0615a 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -50,7 +50,7 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int 
num_tables)
 
 c = g_malloc0(sizeof(*c));
 c->size = num_tables;
-c->entries = g_malloc0(sizeof(*c->entries) * num_tables);
+c->entries = g_new0(Qcow2CachedTable, num_tables);
 
 for (i = 0; i < c->size; i++) {
 c->entries[i].table = qemu_try_blockalign(bs->file, s->cluster_size);
diff --git a/block/qed-check.c b/block/qed-check.c
index 40a882c..36ecd29 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -227,8 +227,7 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, 
bool fix)
 };
 int ret;
 
-check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) *
-sizeof(check.used_clusters[0]));
+check.used_clusters = g_try_new0(uint32_t, (check.nclusters + 31) / 32);
 if (check.nclusters && check.used_clusters == NULL) {
 return -ENOMEM;
 }
diff --git a/block/rbd.c b/block/rbd.c
index 3aaf855..ea969e7 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -862,7 +862,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs,
 int max_snaps = RBD_MAX_SNAPS;
 
 do {
-snaps = g_malloc(sizeof(*snaps) * max_snaps);
+snaps = g_new(rbd_snap_info_t, max_snaps);
 snap_count = rbd_snap_list(s->image, snaps, &max_snaps);
 if (snap_count <= 0) {
 g_free(snaps);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index ba1ef43..12cbd9d 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2357,7 +2357,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 goto out;
 }
 
-sn_tab = g_malloc0(nr * sizeof(*sn_tab));
+sn_tab = g_new0(QEMUSnapshotInfo, nr);
 
 /* calculate a vdi id with hash function */
 hval = fnv_64a_buf(s->name, strlen(s->name), FNV1A_64_INIT);
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fd8f89..47577ec 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -319,7 +319,7 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, 
uint64_t dma_addr,
 sq->size = size;
 sq->cqid = cqid;
 sq->head = sq->tail = 0;
-sq->io_req = g_malloc(sq->size * sizeof(*sq->io_req));
+sq->io_req = g_new(NvmeRequest, sq->size);
 
 QTAILQ_INIT(&sq->req_list);
 QTAILQ_INIT(&sq->out_req_list);
@@ -773,9 +773,9 @@ static int nvme_init(PCIDevice *pci_dev)
 n->reg_size = 1 << qemu_fls(0x1004 + 2 * (n->num_queues + 1) * 4);
 n->ns_size = bs_size / (uint64_t)n->num_namespaces;
 
-n->namespaces = g_malloc0(sizeof(*n->namespaces)*n->num_namespaces);
-n->sq = g_malloc0(sizeof(*n->sq)*n->num_queues);
-n->cq = g_malloc0(sizeof(*n->cq)*n->num_queues);
+n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
+n->sq = g_new0(NvmeSQueue *, n->num_queues);
+n->cq = g_new0(NvmeCQueue *, n->num_queues);
 
 

[Qemu-devel] [PULL v2 03/23] linux-user: Fix syscall instruction usermode emulation on X86_64

2014-08-19 Thread riku . voipio
From: Jincheng Miao 

Currently syscall instruction is buggy on user mode X86_64,
the EIP is updated after do_syscall(), that is too late for
clone(). Because clone() will create a thread at the env->EIP
(the address of syscall insn), and then child thread enters
do_syscall() again, that is not expected. Sometimes it is tragic.

User mode syscall insn emulation is not used MSR, so the
action should be same to INT 0x80. INT 0x80 will update EIP in
do_interrupt(), ditto for syscall() for consistency.

Signed-off-by: Jincheng Miao 
Reviewed-by: Richard Henderson 
Signed-off-by: Riku Voipio 
---
 linux-user/main.c| 1 -
 target-i386/seg_helper.c | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index b453a39..472a16d 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -309,7 +309,6 @@ void cpu_loop(CPUX86State *env)
   env->regs[8],
   env->regs[9],
   0, 0);
-env->eip = env->exception_next_eip;
 break;
 #endif
 case EXCP0B_NOSEG:
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 2d970d0..13eefba 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -1127,8 +1127,8 @@ static void do_interrupt_user(CPUX86State *env, int 
intno, int is_int,
 
 /* Since we emulate only user space, we cannot do more than
exiting the emulation with the suitable exception and error
-   code */
-if (is_int) {
+   code. So update EIP for INT 0x80 and EXCP_SYSCALL. */
+if (is_int || intno == EXCP_SYSCALL) {
 env->eip = next_eip;
 }
 }
-- 
2.0.1




[Qemu-devel] [PULL v2 05/23] linux-user: fix readlink handling with magic exe symlink

2014-08-19 Thread riku . voipio
From: Mike Frysinger 

The current code always returns the length of the path when it should
be returning the number of bytes it wrote to the output string.

Further, readlink is not supposed to append a NUL byte, but the current
snprintf logic will always do just that.

Even further, if you pass in a length of 0, you're suppoesd to get back
an error (EINVAL), but the current logic just returns 0.

Further still, if there was an error reading the symlink, we should not
go ahead and try to read the target buffer as it is garbage.

Simple test for the first two issues:
$ cat test.c
int main() {
char buf[50];
size_t len;
for (len = 0; len < 10; ++len) {
memset(buf, '!', sizeof(buf));
ssize_t ret = readlink("/proc/self/exe", buf, len);
buf[20] = '\0';
printf("readlink(/proc/self/exe, {%s}, %zu) = %zi\n", buf, len, ret);
}
return 0;
}

Now compare the output of the native:
$ gcc test.c -o /tmp/x
$ /tmp/x
$ strace /tmp/x

With what qemu does:
$ armv7a-cros-linux-gnueabi-gcc test.c -o /tmp/x -static
$ qemu-arm /tmp/x
$ qemu-arm -strace /tmp/x

Signed-off-by: Mike Frysinger 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index fccf9f0..7c108ab 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6636,11 +6636,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0);
 if (!p || !p2) {
 ret = -TARGET_EFAULT;
+} else if (!arg3) {
+/* Short circuit this for the magic exe check. */
+ret = -TARGET_EINVAL;
 } else if (is_proc_myself((const char *)p, "exe")) {
 char real[PATH_MAX], *temp;
 temp = realpath(exec_path, real);
-ret = temp == NULL ? get_errno(-1) : strlen(real) ;
-snprintf((char *)p2, arg3, "%s", real);
+/* Return value is # of bytes that we wrote to the buffer. */
+if (temp == NULL) {
+ret = get_errno(-1);
+} else {
+/* Don't worry about sign mismatch as earlier mapping
+ * logic would have thrown a bad address error. */
+ret = MIN(strlen(real), arg3);
+/* We cannot NUL terminate the string. */
+memcpy(p2, real, ret);
+}
 } else {
 ret = get_errno(readlink(path(p), p2, arg3));
 }
-- 
2.0.1




[Qemu-devel] [PATCH v3 3/4] qemu-io-cmds: g_renew() can't fail, bury dead error handling

2014-08-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
---
 qemu-io-cmds.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index afd8867..b224ede 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -115,22 +115,13 @@ static char **breakline(char *input, int *count)
 int c = 0;
 char *p;
 char **rval = g_new0(char *, 1);
-char **tmp;
 
 while (rval && (p = qemu_strsep(&input, " ")) != NULL) {
 if (!*p) {
 continue;
 }
 c++;
-tmp = g_renew(char *, rval, (c + 1));
-if (!tmp) {
-g_free(rval);
-rval = NULL;
-c = 0;
-break;
-} else {
-rval = tmp;
-}
+rval = g_renew(char *, rval, (c + 1));
 rval[c - 1] = p;
 rval[c] = NULL;
 }
-- 
1.9.3




[Qemu-devel] [PULL v2 07/23] linux-user: support ioprio_{get, set} syscalls

2014-08-19 Thread riku . voipio
From: Paul Burton 

Add support for the ioprio_get & ioprio_set syscalls, allowing their
use by target programs.

Signed-off-by: Paul Burton 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 44853d0..f1c182b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -252,6 +252,12 @@ _syscall2(int, capget, struct __user_cap_header_struct *, 
header,
   struct __user_cap_data_struct *, data);
 _syscall2(int, capset, struct __user_cap_header_struct *, header,
   struct __user_cap_data_struct *, data);
+#if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get)
+_syscall2(int, ioprio_get, int, which, int, who)
+#endif
+#if defined(TARGET_NR_ioprio_set) && defined(__NR_ioprio_set)
+_syscall3(int, ioprio_set, int, which, int, who, int, ioprio)
+#endif
 
 static bitmask_transtbl fcntl_flags_tbl[] = {
   { TARGET_O_ACCMODE,   TARGET_O_WRONLY,O_ACCMODE,   O_WRONLY,},
@@ -9592,6 +9598,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 break;
 #endif
 
+#if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get)
+case TARGET_NR_ioprio_get:
+ret = get_errno(ioprio_get(arg1, arg2));
+break;
+#endif
+
+#if defined(TARGET_NR_ioprio_set) && defined(__NR_ioprio_set)
+case TARGET_NR_ioprio_set:
+ret = get_errno(ioprio_set(arg1, arg2, arg3));
+break;
+#endif
+
 default:
 unimplemented:
 gemu_log("qemu: Unsupported syscall: %d\n", num);
-- 
2.0.1




[Qemu-devel] [PULL v2 11/23] linux-user: Dereference Pointer Argument to ipc/semctl Sys Call

2014-08-19 Thread riku . voipio
From: Tom Musta 

When the ipc system call is used to wrap a semctl system call,
the ptr argument to ipc needs to be dereferenced prior to passing
it to the semctl handler.  This is because the fourth argument to
semctl is a union and not a pointer to a union.

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 239e682..0113250 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3140,9 +3140,15 @@ static abi_long do_ipc(unsigned int call, int first,
 ret = get_errno(semget(first, second, third));
 break;
 
-case IPCOP_semctl:
-ret = do_semctl(first, second, third, (union target_semun)(abi_ulong) 
ptr);
+case IPCOP_semctl: {
+/* The semun argument to semctl is passed by value, so dereference the
+ * ptr argument. */
+abi_ulong atptr;
+get_user_ual(atptr, (abi_ulong)ptr);
+ret = do_semctl(first, second, third,
+(union target_semun)(abi_ulong) atptr);
 break;
+}
 
 case IPCOP_msgget:
 ret = get_errno(msgget(first, second));
-- 
2.0.1




[Qemu-devel] [PULL v2 00/23] linux-user updates

2014-08-19 Thread riku . voipio
From: Riku Voipio 

The same as previous series, except the patch "make binfmt flag O require P"
has been dropped.

The following changes since commit 142f4ac5d5e024670ef4725e8943702b027e4218:

  Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-2014-08-15' 
into staging (2014-08-15 18:44:48 +0100)

are available in the git repository at:

  git://git.linaro.org/people/riku.voipio/qemu.git linux-user-for-upstream

for you to fetch changes up to 8349027af60e861fcc401c928eb76f9f418815d7:

  linux-user: check return value of malloc() (2014-08-18 14:03:29 +0300)

Jincheng Miao (1):
  linux-user: Fix syscall instruction usermode emulation on X86_64

Mike Frysinger (1):
  linux-user: fix readlink handling with magic exe symlink

Mikhail Ilyin (1):
  linux-user: /proc/self/maps content

Paul Burton (1):
  linux-user: support ioprio_{get, set} syscalls

Peter Maydell (1):
  linux-user: Fix conversion of sigevent argument to timer_create

Riku Voipio (4):
  linux-user: redirect openat calls
  linux-user: support timerfd_{create, gettime, settime} syscalls
  linux-user: support {name_to,open_by}_handle_at syscalls
  linux-user: add setns and unshare

Tom Musta (13):
  linux-user: PPC64 semid_ds Doesnt Include _unused1 and _unused2
  linux-user: Dereference Pointer Argument to ipc/semctl Sys Call
  linux-user: Properly Handle semun Structure In Cross-Endian Situations
  linux-user: Make ipc syscall's third argument an abi_long
  linux-user: Conditionally Pass Attribute Pointer to mq_open()
  linux-user: Detect Negative Message Sizes in msgsnd System Call
  linux-user: Handle NULL sched_param argument to sched_*
  linux-user: Detect fault in sched_rr_get_interval
  linux-user: Move get_ppc64_abi
  linux-user: Minimum Sig Handler Stack Size for PPC64 ELF V2
  linux-user: clock_nanosleep errno Handling on PPC
  linux-user: Support target-to-host translation of mlockall argument
  linux-user: writev Partial Writes

zhanghailiang (1):
  linux-user: check return value of malloc()

 configure   |  37 +
 include/exec/cpu-all.h  |   2 +
 linux-user/aarch64/syscall.h|   3 +
 linux-user/alpha/syscall.h  |   3 +
 linux-user/arm/syscall.h|   4 +
 linux-user/cris/syscall.h   |   3 +
 linux-user/elfload.c|   9 --
 linux-user/i386/syscall.h   |   3 +
 linux-user/m68k/syscall.h   |   4 +
 linux-user/main.c   |   1 -
 linux-user/microblaze/syscall.h |   3 +
 linux-user/mips/syscall.h   |   3 +
 linux-user/mips64/syscall.h |   3 +
 linux-user/openrisc/syscall.h   |   4 +
 linux-user/ppc/syscall.h|   4 +
 linux-user/ppc/target_cpu.h |  10 ++
 linux-user/s390x/syscall.h  |   3 +
 linux-user/sh4/syscall.h|   4 +
 linux-user/signal.c |  12 +-
 linux-user/sparc/syscall.h  |   3 +
 linux-user/sparc64/syscall.h|   3 +
 linux-user/strace.c |  30 
 linux-user/strace.list  |  18 +++
 linux-user/syscall.c| 347 ++--
 linux-user/unicore32/syscall.h  |   4 +
 linux-user/x86_64/syscall.h |   3 +
 target-i386/seg_helper.c|   4 +-
 27 files changed, 462 insertions(+), 65 deletions(-)

-- 
2.0.1




[Qemu-devel] [PULL v2 08/23] linux-user: support {name_to, open_by}_handle_at syscalls

2014-08-19 Thread riku . voipio
From: Riku Voipio 

Implement support for the name_to_handle_at and open_by_handle_at
syscalls, allowing their use by the target program.

Modified by Riku - move syscalls to functions and put behind
the already existing CONFIG_OPEN_BY_HANDLE to avoid build failure
with old glibc's.

Signed-off-by: Paul Burton 
Signed-off-by: Riku Voipio 
---
 linux-user/strace.c| 30 ++
 linux-user/strace.list |  6 +
 linux-user/syscall.c   | 70 ++
 3 files changed, 106 insertions(+)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index ea6c1d2..c20ddf1 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1552,6 +1552,36 @@ print_kill(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_name_to_handle_at
+static void
+print_name_to_handle_at(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_at_dirfd(arg0, 0);
+print_string(arg1, 0);
+print_pointer(arg2, 0);
+print_pointer(arg3, 0);
+print_raw_param("0x%x", arg4, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
+#ifdef TARGET_NR_open_by_handle_at
+static void
+print_open_by_handle_at(const struct syscallname *name,
+abi_long arg0, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+print_syscall_prologue(name);
+print_raw_param("%d", arg0, 0);
+print_pointer(arg2, 0);
+print_open_flags(arg3, 1);
+print_syscall_epilogue(name);
+}
+#endif
+
 /*
  * An array of all of the syscalls we know about
  */
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 8de972a..147f579 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -582,6 +582,9 @@
 #ifdef TARGET_NR_munmap
 { TARGET_NR_munmap, "munmap" , NULL, print_munmap, NULL },
 #endif
+#ifdef TARGET_NR_name_to_handle_at
+{ TARGET_NR_name_to_handle_at, "name_to_handle_at" , NULL, 
print_name_to_handle_at, NULL },
+#endif
 #ifdef TARGET_NR_nanosleep
 { TARGET_NR_nanosleep, "nanosleep" , NULL, NULL, NULL },
 #endif
@@ -624,6 +627,9 @@
 #ifdef TARGET_NR_openat
 { TARGET_NR_openat, "openat" , NULL, print_openat, NULL },
 #endif
+#ifdef TARGET_NR_open_by_handle_at
+{ TARGET_NR_open_by_handle_at, "open_by_handle_at" , NULL, 
print_open_by_handle_at, NULL },
+#endif
 #ifdef TARGET_NR_osf_adjtime
 { TARGET_NR_osf_adjtime, "osf_adjtime" , NULL, NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f1c182b..74c5d49 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5362,6 +5362,66 @@ static int do_openat(void *cpu_env, int dirfd, const 
char *pathname, int flags,
 
 return get_errno(sys_openat(dirfd, path(pathname), flags, mode));
 }
+#if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
+static abi_long do_name_to_handle_at(abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5)
+{
+struct file_handle *fh;
+uint32_t sz;
+int mount_id;
+abi_long ret;
+char *p;
+
+if (get_user_u32(sz, arg3)) {
+return -TARGET_EFAULT;
+}
+
+p = lock_user_string(arg2);
+if (!p) {
+return -TARGET_EFAULT;
+}
+
+fh = lock_user(VERIFY_WRITE, arg3, sizeof(*fh) + sz, 1);
+if (!fh) {
+unlock_user(p, arg2, 0);
+return -TARGET_EFAULT;
+}
+
+ret = get_errno(name_to_handle_at(arg1, path(p), fh, &mount_id, arg5));
+unlock_user(p, arg2, 0);
+unlock_user(p, arg3, sizeof(*fh) + sz);
+
+if (put_user_s32(mount_id, arg4)) {
+return -TARGET_EFAULT;
+}
+return ret;
+
+}
+#endif
+#if defined(TARGET_NR_open_by_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
+static abi_long do_open_by_handle_at(abi_long arg1, abi_long arg2, abi_long 
arg3)
+{
+struct file_handle *fh;
+uint32_t sz;
+abi_long ret;
+char *p;
+
+if (get_user_u32(sz, arg2)) {
+return -TARGET_EFAULT;
+}
+
+fh = lock_user(VERIFY_WRITE, arg2, sizeof(*fh) + sz, 1);
+if (!fh) {
+return -TARGET_EFAULT;
+}
+
+ret = get_errno(open_by_handle_at(arg1, fh,
+target_to_host_bitmask(arg3, fcntl_flags_tbl)));
+
+unlock_user(p, arg2, sizeof(*fh) + sz);
+return ret;
+}
+#endif
 
 /* do_syscall() should always have a single exit point at the end so
that actions, such as logging of syscall results, can be performed.
@@ -5448,6 +5508,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
   arg4));
 unlock_user(p, arg2, 0);
 break;
+#if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
+case TARGET_NR_name_to_handle_at:
+ret = do_name_to_handle_at(arg1, arg2, arg3, arg4, arg5);
+break;
+#endif
+#if defined(TARGET_NR_open_by_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
+case TARGET_NR_open_by_handle_at:
+ret = do_ope

[Qemu-devel] [PULL v2 01/23] linux-user: /proc/self/maps content

2014-08-19 Thread riku . voipio
From: Mikhail Ilyin 

Build /proc/self/maps doing a match against guest memory translation table.
Output only that map records which are valid for guest memory layout.

Signed-off-by: Mikhail Ilyin 
Signed-off-by: Riku Voipio 
---
 include/exec/cpu-all.h |  2 ++
 linux-user/syscall.c   | 25 ++---
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..f9d132f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
 #define RESERVED_VA 0ul
 #endif
 
+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
+(1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
 #endif
 
 /* page related stuff */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..c8c2b4c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
 
 static int open_self_maps(void *cpu_env, int fd)
 {
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
 CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
 TaskState *ts = cpu->opaque;
-#endif
 FILE *fp;
 char *line = NULL;
 size_t len = 0;
@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
 if ((fields < 10) || (fields > 11)) {
 continue;
 }
-if (!strncmp(path, "[stack]", 7)) {
-continue;
-}
-if (h2g_valid(min) && h2g_valid(max)) {
+if (h2g_valid(min)) {
+int flags = page_get_flags(h2g(min));
+max = h2g_valid(max - 1) ? max : (uintptr_t)g2h(GUEST_ADDR_MAX);
+if (page_check_range(h2g(min), max - min, flags) == -1) {
+continue;
+}
+if (h2g(min) == ts->info->stack_limit) {
+pstrcpy(path, sizeof(path), "  [stack]");
+}
 dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
 " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-h2g(min), h2g(max), flag_r, flag_w,
+h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
 flag_x, flag_p, offset, dev_maj, dev_min, inode,
 path[0] ? " " : "", path);
 }
@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
 free(line);
 fclose(fp);
 
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
-dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0  [stack]\n",
-(unsigned long long)ts->info->stack_limit,
-(unsigned long long)(ts->info->start_stack +
- (TARGET_PAGE_SIZE - 1)) & 
TARGET_PAGE_MASK,
-(unsigned long long)0);
-#endif
-
 return 0;
 }
 
-- 
2.0.1




[Qemu-devel] [PULL v2 02/23] linux-user: redirect openat calls

2014-08-19 Thread riku . voipio
From: Riku Voipio 

While Mikhail fixed /proc/self/maps, it was noticed openat calls are
not redirected currently. Some archs don't have open at all, so
openat needs to be redirected.

Fix this by consolidating open/openat code to do_openat - open
is implemented using openat(AT_FDCWD, ... ), which according
to open(2) man page is identical.

Since all targets now have openat, remove the ifdef around sys_openat
and openat: case in do_syscall.

Cc: Mikhail Ilin 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c8c2b4c..dd77673 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -294,7 +294,6 @@ static int sys_getcwd1(char *buf, size_t size)
   return strlen(buf)+1;
 }
 
-#ifdef TARGET_NR_openat
 static int sys_openat(int dirfd, const char *pathname, int flags, mode_t mode)
 {
   /*
@@ -306,7 +305,6 @@ static int sys_openat(int dirfd, const char *pathname, int 
flags, mode_t mode)
   }
   return (openat(dirfd, pathname, flags));
 }
-#endif
 
 #ifdef TARGET_NR_utimensat
 #ifdef CONFIG_UTIMENSAT
@@ -5274,7 +5272,7 @@ static int open_net_route(void *cpu_env, int fd)
 }
 #endif
 
-static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode)
+static int do_openat(void *cpu_env, int dirfd, const char *pathname, int 
flags, mode_t mode)
 {
 struct fake_open {
 const char *filename;
@@ -5295,7 +5293,7 @@ static int do_open(void *cpu_env, const char *pathname, 
int flags, mode_t mode)
 
 if (is_proc_myself(pathname, "exe")) {
 int execfd = qemu_getauxval(AT_EXECFD);
-return execfd ? execfd : get_errno(open(exec_path, flags, mode));
+return execfd ? execfd : get_errno(sys_openat(dirfd, exec_path, flags, 
mode));
 }
 
 for (fake_open = fakes; fake_open->filename; fake_open++) {
@@ -5329,7 +5327,7 @@ static int do_open(void *cpu_env, const char *pathname, 
int flags, mode_t mode)
 return fd;
 }
 
-return get_errno(open(path(pathname), flags, mode));
+return get_errno(sys_openat(dirfd, path(pathname), flags, mode));
 }
 
 /* do_syscall() should always have a single exit point at the end so
@@ -5404,22 +5402,19 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 case TARGET_NR_open:
 if (!(p = lock_user_string(arg1)))
 goto efault;
-ret = get_errno(do_open(cpu_env, p,
-target_to_host_bitmask(arg2, fcntl_flags_tbl),
-arg3));
+ret = get_errno(do_openat(cpu_env, AT_FDCWD, p,
+  target_to_host_bitmask(arg2, 
fcntl_flags_tbl),
+  arg3));
 unlock_user(p, arg1, 0);
 break;
-#if defined(TARGET_NR_openat) && defined(__NR_openat)
 case TARGET_NR_openat:
 if (!(p = lock_user_string(arg2)))
 goto efault;
-ret = get_errno(sys_openat(arg1,
-   path(p),
-   target_to_host_bitmask(arg3, 
fcntl_flags_tbl),
-   arg4));
+ret = get_errno(do_openat(cpu_env, arg1, p,
+  target_to_host_bitmask(arg3, 
fcntl_flags_tbl),
+  arg4));
 unlock_user(p, arg2, 0);
 break;
-#endif
 case TARGET_NR_close:
 ret = get_errno(close(arg1));
 break;
-- 
2.0.1




[Qemu-devel] [PULL v2 12/23] linux-user: Properly Handle semun Structure In Cross-Endian Situations

2014-08-19 Thread riku . voipio
From: Tom Musta 

The semun union used in the semctl system call contains both an int (val) and
pointers.  In cross-endian situations on 64 bit targets, the value passed to
semctl is an 8 byte (abi_long) value and thus does not have the 4-byte val
field in the correct location.  In order to rectify this, the other half
of the union must be accessed.  This is achieved in code by performing
a byte swap on the entire 8 byte union, followed by a 4-byte swap of the
first half.

Also, eliminate an extraneous (dead) line of code that sets target_su.val in
the IPC_SET/IPC_GET case.

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0113250..ba9dfc5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2652,9 +2652,18 @@ static inline abi_long do_semctl(int semid, int semnum, 
int cmd,
 switch( cmd ) {
case GETVAL:
case SETVAL:
-arg.val = tswap32(target_su.val);
+/* In 64 bit cross-endian situations, we will erroneously pick up
+ * the wrong half of the union for the "val" element.  To rectify
+ * this, the entire 8-byte structure is byteswapped, followed by
+* a swap of the 4 byte val field. In other cases, the data is
+* already in proper host byte order. */
+   if (sizeof(target_su.val) != (sizeof(target_su.buf))) {
+   target_su.buf = tswapal(target_su.buf);
+   arg.val = tswap32(target_su.val);
+   } else {
+   arg.val = target_su.val;
+   }
 ret = get_errno(semctl(semid, semnum, cmd, arg));
-target_su.val = tswap32(arg.val);
 break;
case GETALL:
case SETALL:
-- 
2.0.1




[Qemu-devel] [PULL v2 09/23] linux-user: add setns and unshare

2014-08-19 Thread riku . voipio
From: Riku Voipio 

Add support for the setns and unshare syscalls, trivially passed through to
the host. Based on patches by Paul Burton, added configure check.

Signed-off-by: Paul Burton 
Signed-off-by: Riku Voipio 
---
 configure  | 20 
 linux-user/strace.list |  3 +++
 linux-user/syscall.c   | 11 +++
 3 files changed, 34 insertions(+)

diff --git a/configure b/configure
index 3d0ab1b..c4e47e1 100755
--- a/configure
+++ b/configure
@@ -3470,6 +3470,23 @@ if compile_prog "" "" ; then
   timerfd=yes
 fi
 
+# check for setns and unshare support
+setns=no
+cat > $TMPC << EOF
+#include 
+
+int main(void)
+{
+int ret;
+ret = setns(0, 0);
+ret = unshare(0);
+return ret;
+}
+EOF
+if compile_prog "" "" ; then
+  setns=yes
+fi
+
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
   if has makeinfo && has pod2man; then
@@ -4541,6 +4558,9 @@ fi
 if test "$timerfd" = "yes" ; then
   echo "CONFIG_TIMERFD=y" >> $config_host_mak
 fi
+if test "$setns" = "yes" ; then
+  echo "CONFIG_SETNS=y" >> $config_host_mak
+fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 147f579..d5b8033 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1191,6 +1191,9 @@
 #ifdef TARGET_NR_set_mempolicy
 { TARGET_NR_set_mempolicy, "set_mempolicy" , NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_setns
+{ TARGET_NR_setns, "setns" , NULL, NULL, NULL },
+#endif
 #ifdef TARGET_NR_setpgid
 { TARGET_NR_setpgid, "setpgid" , NULL, NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 74c5d49..e5c3ebb 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9680,6 +9680,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 break;
 #endif
 
+#if defined(TARGET_NR_setns) && defined(CONFIG_SETNS)
+case TARGET_NR_setns:
+ret = get_errno(setns(arg1, arg2));
+break;
+#endif
+#if defined(TARGET_NR_unshare) && defined(CONFIG_SETNS)
+case TARGET_NR_unshare:
+ret = get_errno(unshare(arg1));
+break;
+#endif
+
 default:
 unimplemented:
 gemu_log("qemu: Unsupported syscall: %d\n", num);
-- 
2.0.1




[Qemu-devel] [PULL v2 06/23] linux-user: support timerfd_{create, gettime, settime} syscalls

2014-08-19 Thread riku . voipio
From: Riku Voipio 

Adds support for the timerfd_create, timerfd_gettime & timerfd_settime
syscalls, allowing use of timerfds by target programs.

v2: By Riku - added configure check for timerfd and ifdefs
for benefit of old distributions like RHEL5.

Signed-off-by: Paul Burton 
Signed-off-by: Riku Voipio 
---
 configure  | 17 +
 linux-user/strace.list |  9 +
 linux-user/syscall.c   | 45 +
 3 files changed, 71 insertions(+)

diff --git a/configure b/configure
index 283c71c..3d0ab1b 100755
--- a/configure
+++ b/configure
@@ -3456,6 +3456,20 @@ if compile_prog "" "" ; then
   sendfile=yes
 fi
 
+# check for timerfd support (glibc 2.8 and newer)
+timerfd=no
+cat > $TMPC << EOF
+#include 
+
+int main(void)
+{
+return(timerfd_create(CLOCK_REALTIME, 0));
+}
+EOF
+if compile_prog "" "" ; then
+  timerfd=yes
+fi
+
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
   if has makeinfo && has pod2man; then
@@ -4524,6 +4538,9 @@ fi
 if test "$sendfile" = "yes" ; then
   echo "CONFIG_SENDFILE=y" >> $config_host_mak
 fi
+if test "$timerfd" = "yes" ; then
+  echo "CONFIG_TIMERFD=y" >> $config_host_mak
+fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
diff --git a/linux-user/strace.list b/linux-user/strace.list
index fcb258d..8de972a 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1404,6 +1404,15 @@
 #ifdef TARGET_NR_timer_settime
 { TARGET_NR_timer_settime, "timer_settime" , NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_timerfd_create
+{ TARGET_NR_timerfd_create, "timerfd_create" , NULL, NULL, NULL },
+#endif
+#ifdef TARGET_NR_timerfd_gettime
+{ TARGET_NR_timerfd_gettime, "timerfd_gettime" , NULL, NULL, NULL },
+#endif
+#ifdef TARGET_NR_timerfd_settime
+{ TARGET_NR_timerfd_settime, "timerfd_settime" , NULL, NULL, NULL },
+#endif
 #ifdef TARGET_NR_times
 { TARGET_NR_times, "times" , NULL, NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 7c108ab..44853d0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -58,6 +58,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 //#include 
@@ -9547,6 +9548,50 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 }
 #endif
 
+#if defined(TARGET_NR_timerfd_create) && defined(CONFIG_TIMERFD)
+case TARGET_NR_timerfd_create:
+ret = get_errno(timerfd_create(arg1,
+target_to_host_bitmask(arg2, fcntl_flags_tbl)));
+break;
+#endif
+
+#if defined(TARGET_NR_timerfd_gettime) && defined(CONFIG_TIMERFD)
+case TARGET_NR_timerfd_gettime:
+{
+struct itimerspec its_curr;
+
+ret = get_errno(timerfd_gettime(arg1, &its_curr));
+
+if (arg2 && host_to_target_itimerspec(arg2, &its_curr)) {
+goto efault;
+}
+}
+break;
+#endif
+
+#if defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD)
+case TARGET_NR_timerfd_settime:
+{
+struct itimerspec its_new, its_old, *p_new;
+
+if (arg3) {
+if (target_to_host_itimerspec(&its_new, arg3)) {
+goto efault;
+}
+p_new = &its_new;
+} else {
+p_new = NULL;
+}
+
+ret = get_errno(timerfd_settime(arg1, arg2, p_new, &its_old));
+
+if (arg4 && host_to_target_itimerspec(arg4, &its_old)) {
+goto efault;
+}
+}
+break;
+#endif
+
 default:
 unimplemented:
 gemu_log("qemu: Unsupported syscall: %d\n", num);
-- 
2.0.1




[Qemu-devel] [PULL v2 13/23] linux-user: Make ipc syscall's third argument an abi_long

2014-08-19 Thread riku . voipio
From: Tom Musta 

For those target ABIs that use the ipc system call (e.g. POWER),
the third argument is used in the shmat path as a pointer.  It
therefore must be declared as an abi_long (versus int) so that
the address bits are not lost in truncation.  In fact, all arguments
to do_ipc should be declared as abit_long.

In fact, it makes more sense for all of the arguments to be declaried
as abi_long (except call).

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ba9dfc5..db40829 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3130,8 +3130,8 @@ static inline abi_long do_shmdt(abi_ulong shmaddr)
 #ifdef TARGET_NR_ipc
 /* ??? This only works with linear mappings.  */
 /* do_ipc() must return target values and target errnos. */
-static abi_long do_ipc(unsigned int call, int first,
-   int second, int third,
+static abi_long do_ipc(unsigned int call, abi_long first,
+   abi_long second, abi_long third,
abi_long ptr, abi_long fifth)
 {
 int version;
@@ -3153,9 +3153,9 @@ static abi_long do_ipc(unsigned int call, int first,
 /* The semun argument to semctl is passed by value, so dereference the
  * ptr argument. */
 abi_ulong atptr;
-get_user_ual(atptr, (abi_ulong)ptr);
+get_user_ual(atptr, ptr);
 ret = do_semctl(first, second, third,
-(union target_semun)(abi_ulong) atptr);
+(union target_semun) atptr);
 break;
 }
 
-- 
2.0.1




[Qemu-devel] [PULL v2 16/23] linux-user: Handle NULL sched_param argument to sched_*

2014-08-19 Thread riku . voipio
From: Tom Musta 

The sched_getparam, sched_setparam and sched_setscheduler system
calls take a pointer argument to a sched_param structure.  When
this pointer is null, errno should be set to EINVAL.

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d223cff..a0436da 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7806,6 +7806,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 struct sched_param *target_schp;
 struct sched_param schp;
 
+if (arg2 == 0) {
+return -TARGET_EINVAL;
+}
 if (!lock_user_struct(VERIFY_READ, target_schp, arg2, 1))
 goto efault;
 schp.sched_priority = tswap32(target_schp->sched_priority);
@@ -7817,6 +7820,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 {
 struct sched_param *target_schp;
 struct sched_param schp;
+
+if (arg2 == 0) {
+return -TARGET_EINVAL;
+}
 ret = get_errno(sched_getparam(arg1, &schp));
 if (!is_error(ret)) {
 if (!lock_user_struct(VERIFY_WRITE, target_schp, arg2, 0))
@@ -7830,6 +7837,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 {
 struct sched_param *target_schp;
 struct sched_param schp;
+if (arg3 == 0) {
+return -TARGET_EINVAL;
+}
 if (!lock_user_struct(VERIFY_READ, target_schp, arg3, 1))
 goto efault;
 schp.sched_priority = tswap32(target_schp->sched_priority);
-- 
2.0.1




[Qemu-devel] [PULL v2 10/23] linux-user: PPC64 semid_ds Doesnt Include _unused1 and _unused2

2014-08-19 Thread riku . voipio
From: Tom Musta 

The 64 bit PowerPC platforms eliminate the _unused1 and _unused2
elements of the semid_ds structure from .  So eliminate
these from the target_semid_ds structure.

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e5c3ebb..239e682 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2424,9 +2424,13 @@ struct target_semid_ds
 {
   struct target_ipc_perm sem_perm;
   abi_ulong sem_otime;
+#if !defined(TARGET_PPC64)
   abi_ulong __unused1;
+#endif
   abi_ulong sem_ctime;
+#if !defined(TARGET_PPC64)
   abi_ulong __unused2;
+#endif
   abi_ulong sem_nsems;
   abi_ulong __unused3;
   abi_ulong __unused4;
-- 
2.0.1




[Qemu-devel] [PULL v2 15/23] linux-user: Detect Negative Message Sizes in msgsnd System Call

2014-08-19 Thread riku . voipio
From: Tom Musta 

The msgsnd system call takes an argument that describes the message
size (msgsz) and is of type size_t.  The system call should set
errno to EINVAL in the event that a negative message size is passed.

Signed-off-by: Tom Musta 
Reviewed-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c9a9d3d..d223cff 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2879,12 +2879,16 @@ struct target_msgbuf {
 };
 
 static inline abi_long do_msgsnd(int msqid, abi_long msgp,
- unsigned int msgsz, int msgflg)
+ ssize_t msgsz, int msgflg)
 {
 struct target_msgbuf *target_mb;
 struct msgbuf *host_mb;
 abi_long ret = 0;
 
+if (msgsz < 0) {
+return -TARGET_EINVAL;
+}
+
 if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
 return -TARGET_EFAULT;
 host_mb = malloc(msgsz+sizeof(long));
-- 
2.0.1




[Qemu-devel] [PULL v2 20/23] linux-user: clock_nanosleep errno Handling on PPC

2014-08-19 Thread riku . voipio
From: Tom Musta 

The clock_nanosleep syscall is unusual in that it returns positive
numbers in error handling situations, versus returning -1 and setting
errno, or returning a negative errno value.  On POWER, the kernel will
set the SO bit of CR0 to indicate failure in a syscall.  QEMU has
generic handling to do this for syscalls with standard return values.

Add special case code for clock_nanosleep to handle CR0 properly.

Signed-off-by: Tom Musta 
Reviewed-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a24356d..e4be32c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9103,6 +9103,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 ret = get_errno(clock_nanosleep(arg1, arg2, &ts, arg4 ? &ts : NULL));
 if (arg4)
 host_to_target_timespec(arg4, &ts);
+
+#if defined(TARGET_PPC)
+/* clock_nanosleep is odd in that it returns positive errno values.
+ * On PPC, CR0 bit 3 should be set in such a situation. */
+if (ret) {
+((CPUPPCState *)cpu_env)->crf[0] |= 1;
+}
+#endif
 break;
 }
 #endif
-- 
2.0.1




[Qemu-devel] [PULL v2 04/23] linux-user: Fix conversion of sigevent argument to timer_create

2014-08-19 Thread riku . voipio
From: Peter Maydell 

There were a number of bugs in the conversion of the sigevent
argument to timer_create from target to host format:
 * signal number not converted from target to host
 * thread ID not copied across
 * sigev_value not copied across
 * we never unlocked the struct when we were done

Between them, these problems meant that SIGEV_THREAD_ID
timers (and the glibc-implemented SIGEV_THREAD timers which
depend on them) didn't work.

Fix these problems and clean up the code a little by pulling
the struct conversion out into its own function, in line with
how we convert various other structs. This allows the test
program in bug LP:1042388 to run.

Signed-off-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 38 ++
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index dd77673..fccf9f0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4912,6 +4912,32 @@ static inline abi_long 
host_to_target_itimerspec(abi_ulong target_addr,
 return 0;
 }
 
+static inline abi_long target_to_host_sigevent(struct sigevent *host_sevp,
+   abi_ulong target_addr)
+{
+struct target_sigevent *target_sevp;
+
+if (!lock_user_struct(VERIFY_READ, target_sevp, target_addr, 1)) {
+return -TARGET_EFAULT;
+}
+
+/* This union is awkward on 64 bit systems because it has a 32 bit
+ * integer and a pointer in it; we follow the conversion approach
+ * used for handling sigval types in signal.c so the guest should get
+ * the correct value back even if we did a 64 bit byteswap and it's
+ * using the 32 bit integer.
+ */
+host_sevp->sigev_value.sival_ptr =
+(void *)(uintptr_t)tswapal(target_sevp->sigev_value.sival_ptr);
+host_sevp->sigev_signo =
+target_to_host_signal(tswap32(target_sevp->sigev_signo));
+host_sevp->sigev_notify = tswap32(target_sevp->sigev_notify);
+host_sevp->_sigev_un._tid = tswap32(target_sevp->_sigev_un._tid);
+
+unlock_user_struct(target_sevp, target_addr, 1);
+return 0;
+}
+
 #if defined(TARGET_NR_stat64) || defined(TARGET_NR_newfstatat)
 static inline abi_long host_to_target_stat64(void *cpu_env,
  abi_ulong target_addr,
@@ -9403,7 +9429,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */
 
 struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL;
-struct target_sigevent *ptarget_sevp;
 struct target_timer_t *ptarget_timer;
 
 int clkid = arg1;
@@ -9415,14 +9440,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 timer_t *phtimer = g_posix_timers  + timer_index;
 
 if (arg2) {
-if (!lock_user_struct(VERIFY_READ, ptarget_sevp, arg2, 1)) {
-goto efault;
-}
-
-host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo);
-host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify);
-
 phost_sevp = &host_sevp;
+ret = target_to_host_sigevent(phost_sevp, arg2);
+if (ret != 0) {
+break;
+}
 }
 
 ret = get_errno(timer_create(clkid, phost_sevp, phtimer));
-- 
2.0.1




[Qemu-devel] [PULL v2 23/23] linux-user: check return value of malloc()

2014-08-19 Thread riku . voipio
From: zhanghailiang 

Signed-off-by: zhanghailiang 
Acked-by: Riku Voipio 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3b1beff..3aaed81 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2904,6 +2904,10 @@ static inline abi_long do_msgsnd(int msqid, abi_long 
msgp,
 if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
 return -TARGET_EFAULT;
 host_mb = malloc(msgsz+sizeof(long));
+if (!host_mb) {
+unlock_user_struct(target_mb, msgp, 0);
+return -TARGET_ENOMEM;
+}
 host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
 memcpy(host_mb->mtext, target_mb->mtext, msgsz);
 ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
-- 
2.0.1




[Qemu-devel] [PULL v2 22/23] linux-user: writev Partial Writes

2014-08-19 Thread riku . voipio
From: Tom Musta 

Although not technically not required by POSIX, the writev system call will
typically write out its buffers individually.  That is, if the first buffer
is written successfully, but the second buffer pointer is invalid, then
the first chuck will be written and its size is returned.

Signed-off-by: Tom Musta 
Reviewed-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c4f6454..3b1beff 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1803,6 +1803,7 @@ static struct iovec *lock_iovec(int type, abi_ulong 
target_addr,
 abi_ulong total_len, max_len;
 int i;
 int err = 0;
+bool bad_address = false;
 
 if (count == 0) {
 errno = 0;
@@ -1843,9 +1844,20 @@ static struct iovec *lock_iovec(int type, abi_ulong 
target_addr,
 vec[i].iov_base = 0;
 } else {
 vec[i].iov_base = lock_user(type, base, len, copy);
+/* If the first buffer pointer is bad, this is a fault.  But
+ * subsequent bad buffers will result in a partial write; this
+ * is realized by filling the vector with null pointers and
+ * zero lengths. */
 if (!vec[i].iov_base) {
-err = EFAULT;
-goto fail;
+if (i == 0) {
+err = EFAULT;
+goto fail;
+} else {
+bad_address = true;
+}
+}
+if (bad_address) {
+len = 0;
 }
 if (len > max_len - total_len) {
 len = max_len - total_len;
-- 
2.0.1




[Qemu-devel] [PULL v2 17/23] linux-user: Detect fault in sched_rr_get_interval

2014-08-19 Thread riku . voipio
From: Tom Musta 

Properly detect a fault when attempting to store into an invalid
struct timespec pointer.

Signed-off-by: Tom Musta 
Reviewed-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a0436da..a24356d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7864,7 +7864,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 struct timespec ts;
 ret = get_errno(sched_rr_get_interval(arg1, &ts));
 if (!is_error(ret)) {
-host_to_target_timespec(arg2, &ts);
+ret = host_to_target_timespec(arg2, &ts);
 }
 }
 break;
-- 
2.0.1




[Qemu-devel] [PULL v2 14/23] linux-user: Conditionally Pass Attribute Pointer to mq_open()

2014-08-19 Thread riku . voipio
From: Tom Musta 

The mq_open system call takes an optional struct mq_attr pointer
argument in the fourth position.  This pointer is used when O_CREAT
is specified in the flags (second) argument.  It may be NULL, in
which case the queue is created with implementation defined attributes.

Change the code to properly handle the case when NULL is passed in the
arg4 position.

Signed-off-by: Tom Musta 
Reviewed-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index db40829..c9a9d3d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9185,12 +9185,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
 case TARGET_NR_mq_open:
 {
-struct mq_attr posix_mq_attr;
+struct mq_attr posix_mq_attr, *attrp;
 
 p = lock_user_string(arg1 - 1);
-if (arg4 != 0)
+if (arg4 != 0) {
 copy_from_user_mq_attr (&posix_mq_attr, arg4);
-ret = get_errno(mq_open(p, arg2, arg3, &posix_mq_attr));
+attrp = &posix_mq_attr;
+} else {
+attrp = 0;
+}
+ret = get_errno(mq_open(p, arg2, arg3, attrp));
 unlock_user (p, arg1, 0);
 }
 break;
-- 
2.0.1




[Qemu-devel] [PULL v2 18/23] linux-user: Move get_ppc64_abi

2014-08-19 Thread riku . voipio
From: Tom Musta 

The get_ppc64_abi is used to determine the ELF ABI (i.e. V1 or V2). This
routine is currently implemented in the linux-user/elfload.c file but
is useful in other scenarios.  Move the routine to a more generally
available location (linux-user/ppc/target_cpu.h).

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/elfload.c|  9 -
 linux-user/ppc/target_cpu.h | 10 ++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 60777fe..bea803b 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -824,8 +824,6 @@ static uint32_t get_elf_hwcap2(void)
 NEW_AUX_ENT(AT_IGNOREPPC, AT_IGNOREPPC);\
 } while (0)
 
-static inline uint32_t get_ppc64_abi(struct image_info *infop);
-
 static inline void init_thread(struct target_pt_regs *_regs, struct image_info 
*infop)
 {
 _regs->gpr[1] = infop->start_stack;
@@ -1205,13 +1203,6 @@ static inline void init_thread(struct target_pt_regs 
*regs, struct image_info *i
 
 #include "elf.h"
 
-#ifdef TARGET_PPC
-static inline uint32_t get_ppc64_abi(struct image_info *infop)
-{
-  return infop->elf_flags & EF_PPC64_ABI;
-}
-#endif
-
 struct exec
 {
 unsigned int a_info;   /* Use macros N_MAGIC, etc for access */
diff --git a/linux-user/ppc/target_cpu.h b/linux-user/ppc/target_cpu.h
index 9cc0c3b..26f4ba2 100644
--- a/linux-user/ppc/target_cpu.h
+++ b/linux-user/ppc/target_cpu.h
@@ -38,4 +38,14 @@ static inline void cpu_set_tls(CPUPPCState *env, 
target_ulong newtls)
 #endif
 }
 
+#ifndef EF_PPC64_ABI
+#define EF_PPC64_ABI   0x3
+#endif
+
+static inline uint32_t get_ppc64_abi(struct image_info *infop)
+{
+  return infop->elf_flags & EF_PPC64_ABI;
+}
+
+
 #endif
-- 
2.0.1




Re: [Qemu-devel] Microcheckpointing: Memory-VCPU / Disk State consistency

2014-08-19 Thread Walid Nouri
Hi,
I have tried to find more information on how to use drive-mirror besides what 
is available on the wiki. This was not very satisfactory...

This may sound naive but are there some code examples in "c" or any other 
language, documentation of any kind, blog entries (developer), presentation 
videos or any other source of information to get started?

Walid


> Am 14.08.2014 um 12:58 schrieb "Dr. David Alan Gilbert" :
> 
> cc'ing in a couple of the COLOers.
> 
> * Michael R. Hines (mrhi...@linux.vnet.ibm.com) wrote:
>>> On 08/13/2014 10:03 PM, Walid Nouri wrote:
>>> 
>>> While looking to find some ideas for approaches to replicating block
>>> devices I have read the paper about the Remus implementation. I think MC
>>> can take a similar approach for local disk.
>> 
>> I agree.
>> 
>>> Here are the main facts that I have understood:
>>> 
>>> Local disk contents is viewed as internal state the primary and secondary.
>>> In the explanation they describe that for keeping disc semantics of the
>>> primary and to allow the primary to run speculatively all disc state
>>> changes are directly written to the disk. In parrallel and asynchronously
>>> send to the secondary. The secondary keeps the pending writing requests in
>>> two disk buffers. A speculation-disk-buffer and a write-out-buffer.
>>> 
>>> After the reception of the next checkpoint the secondary copies the
>>> speculation buffer to the write out buffer, commits the checkpoint and
>>> applies the write out buffer to its local disk.
>>> 
>>> When the primary fails the secondary must wait until write-out-buffer has
>>> been completely written to disk before before changing the execution mode
>>> to run as primary. In this case (failure of primary) the secondary
>>> discards pending disk writes in its speculation buffer. This protocol
>>> keeps the disc state consistent with the last checkpoint.
>>> 
>>> Remus uses the XEN specific blktap driver. As far as I know this can?t be
>>> used with QEMU (KVM).
>>> 
>>> I must see how drive-mirror can be used for this kind of protocol.
>> 
>> That's all correct. Theoretically, we would do exactly the same thing:
>> drive-mirror on the source would write immediately to disk but follow the
>> same commit semantics on the destination as Xen.
>> 
>>> 
>>> I have taken a look at COLO.
>> 
>>> IMHO there are two points. Custom changes of the TCP-Stack are a no-go for
>>> proprietary operating systems like Windows. It makes COLO application
>>> agnostic but not operating system agnostic. The other point is that with
>>> I/O intensive workloads COLO will tend to behave like MC. This is my point
>>> of view but i didn?t invest much time to understand everything in detail.
>> 
>> Actually, if I remember correctly, the TCP stack is only modified at the
>> hypervisor level - they are intercepting and translating TCP sequence
>> numbers "in-flight" to detect divergence of the source and destination -
>> which is not a big problem if the implementation is well-done.
> 
> The 2013 paper says:
>   'COLO modifies the guest OS’s TCP/IP stack in order to make the behavior
>more deterministic. '
> but does say that an alternative might be to have a
>  ' comparison function that operates transparently over re-assembled TCP 
> streams'
> 
>> My hope in the future was that the two approaches could be used in a
>> "Hybrid" manner - actually MC has much more of a performance hit for I/O
>> than COLO does because of its buffering requirements.
>> 
>> On the other hand, MC would perform better in a memory-intensive or
>> CPU-intensive situation - so maybe QEMU could "switch" between the two
>> mechanisms at different points in time when the resource bottleneck changes.
> 
> If the primary were to rate-limit the number of resynchronisations
> (and send the secondary a message as soon as it knew a resync was needed) that
> would get some of the way, but then the only difference from 
> microcheckpointing
> at that point is the secondary doing a wasteful copy and sending the packets 
> across;
> it seems it should be easy to disable those if it knew that a resync was 
> going to
> happen.
> 
> Dave
> 
>> - Michael
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *

2014-08-19 Thread Markus Armbruster
Markus Armbruster  writes:

> Jeff Cody  writes:
>
>> On Mon, Aug 18, 2014 at 06:10:43PM +0200, Markus Armbruster wrote:
>>> They clutter the code.  Unfortunately, I can't figure out how to make
>>> Coccinelle drop all of them, so I have to settle for common special
>>> cases:
>>> 
>>> @@
>>> type T;
>>> T *pt;
>>> void *pv;
>>> @@
>>> - pt = (T *)pv;
>>> + pt = pv;
>>> @@
>>> type T;
>>> @@
>>> - (T *)
>>>   (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\|
>>>  g_try_malloc\|g_try_malloc0\|g_try_realloc\|
>>>  g_try_new\|g_try_new0\|g_try_renew\)(...))
>>> 
>>> Signed-off-by: Markus Armbruster 
>>> ---
>>>  block/vhdx-log.c| 2 +-
>>>  block/vvfat.c   | 8 
>>>  hw/ide/microdrive.c | 2 +-
>>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
>>> index eb5c7a0..4267e60 100644
>>> --- a/block/vhdx-log.c
>>> +++ b/block/vhdx-log.c
>>> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, 
>>> BDRVVHDXState *s,
>>>  buffer = qemu_blockalign(bs, total_length);
>>>  memcpy(buffer, &new_hdr, sizeof(new_hdr));
>>>  
>>> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr));
>>> +new_desc = (buffer + sizeof(new_hdr));
>>
>> Agree with Max, in that the parenthesis could be removed.  Not worthy
>> of a respin normally, but since the point of this patch is to
>> unclutter the code, I guess it makes sense to fix it.
>
> Max and you are right.  I neglected to clean up the patch produced by
> spatch.

Cleaned up in v3.

>>>  data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE);
>>>  data_tmp = data;
>>>  
>>> diff --git a/block/vvfat.c b/block/vvfat.c
>>> index f877e85..401539d 100644
>>> --- a/block/vvfat.c
>>> +++ b/block/vvfat.c
>>> @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int 
>>> mapping_index)
>>> if(first_cluster == 0 && (is_dotdot || is_dot))
>>> continue;
>>>  
>>> -   buffer=(char*)g_malloc(length);
>>> +   buffer=g_malloc(length);
>>
>> You missed a spot to put spaces around the '=' here.  Nothing worthy
>> of a respin, of course - just a note in case you decide to respin.
>
> Right again.  Although perhaps vvfat should remain ugly, to warn unwary
> travellers ;)

Cleaned up in v3.

[...]



[Qemu-devel] [PULL v2 21/23] linux-user: Support target-to-host translation of mlockall argument

2014-08-19 Thread riku . voipio
From: Tom Musta 

The argument to the mlockall system call is not necessarily the same on
all platforms and thus may require translation prior to passing to the
host.

For example, PowerPC 64 bit platforms define values for MCL_CURRENT
(0x2000) and MCL_FUTURE (0x4000) which are different from Intel platforms
(0x1 and 0x2, respectively)

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/aarch64/syscall.h|  2 ++
 linux-user/alpha/syscall.h  |  2 ++
 linux-user/arm/syscall.h|  2 ++
 linux-user/cris/syscall.h   |  2 ++
 linux-user/i386/syscall.h   |  2 ++
 linux-user/m68k/syscall.h   |  2 ++
 linux-user/microblaze/syscall.h |  2 ++
 linux-user/mips/syscall.h   |  2 ++
 linux-user/mips64/syscall.h |  2 ++
 linux-user/openrisc/syscall.h   |  2 ++
 linux-user/ppc/syscall.h|  2 ++
 linux-user/s390x/syscall.h  |  2 ++
 linux-user/sh4/syscall.h|  2 ++
 linux-user/sparc/syscall.h  |  2 ++
 linux-user/sparc64/syscall.h|  2 ++
 linux-user/syscall.c| 17 -
 linux-user/unicore32/syscall.h  |  2 ++
 linux-user/x86_64/syscall.h |  2 ++
 18 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/linux-user/aarch64/syscall.h b/linux-user/aarch64/syscall.h
index d1f4823..dc72a15 100644
--- a/linux-user/aarch64/syscall.h
+++ b/linux-user/aarch64/syscall.h
@@ -9,3 +9,5 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "3.8.0"
 #define TARGET_CLONE_BACKWARDS
 #define TARGET_MINSIGSTKSZ   2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/alpha/syscall.h b/linux-user/alpha/syscall.h
index 3adedeb..245cff2 100644
--- a/linux-user/alpha/syscall.h
+++ b/linux-user/alpha/syscall.h
@@ -253,3 +253,5 @@ struct target_pt_regs {
 #define TARGET_UAC_NOFIX   2
 #define TARGET_UAC_SIGBUS  4
 #define TARGET_MINSIGSTKSZ  4096
+#define TARGET_MLOCKALL_MCL_CURRENT 0x2000
+#define TARGET_MLOCKALL_MCL_FUTURE  0x4000
diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h
index cdadb0c..3844a96 100644
--- a/linux-user/arm/syscall.h
+++ b/linux-user/arm/syscall.h
@@ -46,3 +46,5 @@ struct target_pt_regs {
 #define TARGET_CLONE_BACKWARDS
 
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/cris/syscall.h b/linux-user/cris/syscall.h
index a75bcc4..2957b0d 100644
--- a/linux-user/cris/syscall.h
+++ b/linux-user/cris/syscall.h
@@ -40,5 +40,7 @@ struct target_pt_regs {
 
 #define TARGET_CLONE_BACKWARDS2
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
 
 #endif
diff --git a/linux-user/i386/syscall.h b/linux-user/i386/syscall.h
index acf6856..906aaac 100644
--- a/linux-user/i386/syscall.h
+++ b/linux-user/i386/syscall.h
@@ -148,3 +148,5 @@ struct target_vm86plus_struct {
 
 #define TARGET_CLONE_BACKWARDS
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/m68k/syscall.h b/linux-user/m68k/syscall.h
index f8553f8..9218493 100644
--- a/linux-user/m68k/syscall.h
+++ b/linux-user/m68k/syscall.h
@@ -19,5 +19,7 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
 
 void do_m68k_simcall(CPUM68KState *, int);
diff --git a/linux-user/microblaze/syscall.h b/linux-user/microblaze/syscall.h
index 2a5e160..3c1ed27 100644
--- a/linux-user/microblaze/syscall.h
+++ b/linux-user/microblaze/syscall.h
@@ -50,5 +50,7 @@ struct target_pt_regs {
 
 #define TARGET_CLONE_BACKWARDS
 #define TARGET_MINSIGSTKSZ  2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
 
 #endif
diff --git a/linux-user/mips/syscall.h b/linux-user/mips/syscall.h
index 0b4662c..35ca23b 100644
--- a/linux-user/mips/syscall.h
+++ b/linux-user/mips/syscall.h
@@ -229,3 +229,5 @@ struct target_pt_regs {
 
 #define TARGET_CLONE_BACKWARDS
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h
index 39b8bed..6733107 100644
--- a/linux-user/mips64/syscall.h
+++ b/linux-user/mips64/syscall.h
@@ -226,3 +226,5 @@ struct target_pt_regs {
 
 #define TARGET_CLONE_BACKWARDS
 #define TARGET_MINSIGSTKSZ  2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/openrisc/syscall.h b/linux-user/openrisc/syscall.h
index e5e6180..8ac0365 100644
--- a/linux-user/openrisc/syscall.h
+++ b/linux-user/openrisc/syscall.h
@@ -25,3 +25,5 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_MINSIGSTKSZ 2048
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
diff --git a/linux-user/pp

[Qemu-devel] [PULL v2 19/23] linux-user: Minimum Sig Handler Stack Size for PPC64 ELF V2

2014-08-19 Thread riku . voipio
From: Tom Musta 

The ELF V2 ABI for PPC64 defines MINSIGSTKSZ as 4096 bytes whereas it was
2048 previously.

Signed-off-by: Tom Musta 
Signed-off-by: Riku Voipio 
---
 linux-user/aarch64/syscall.h|  1 +
 linux-user/alpha/syscall.h  |  1 +
 linux-user/arm/syscall.h|  2 ++
 linux-user/cris/syscall.h   |  1 +
 linux-user/i386/syscall.h   |  1 +
 linux-user/m68k/syscall.h   |  2 ++
 linux-user/microblaze/syscall.h |  1 +
 linux-user/mips/syscall.h   |  1 +
 linux-user/mips64/syscall.h |  1 +
 linux-user/openrisc/syscall.h   |  2 ++
 linux-user/ppc/syscall.h|  2 ++
 linux-user/s390x/syscall.h  |  1 +
 linux-user/sh4/syscall.h|  2 ++
 linux-user/signal.c | 12 +++-
 linux-user/sparc/syscall.h  |  1 +
 linux-user/sparc64/syscall.h|  1 +
 linux-user/unicore32/syscall.h  |  2 ++
 linux-user/x86_64/syscall.h |  1 +
 18 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/linux-user/aarch64/syscall.h b/linux-user/aarch64/syscall.h
index 18f44a8..d1f4823 100644
--- a/linux-user/aarch64/syscall.h
+++ b/linux-user/aarch64/syscall.h
@@ -8,3 +8,4 @@ struct target_pt_regs {
 #define UNAME_MACHINE "aarch64"
 #define UNAME_MINIMUM_RELEASE "3.8.0"
 #define TARGET_CLONE_BACKWARDS
+#define TARGET_MINSIGSTKSZ   2048
diff --git a/linux-user/alpha/syscall.h b/linux-user/alpha/syscall.h
index ed13d9a..3adedeb 100644
--- a/linux-user/alpha/syscall.h
+++ b/linux-user/alpha/syscall.h
@@ -252,3 +252,4 @@ struct target_pt_regs {
 #define TARGET_UAC_NOPRINT 1
 #define TARGET_UAC_NOFIX   2
 #define TARGET_UAC_SIGBUS  4
+#define TARGET_MINSIGSTKSZ  4096
diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h
index e0d2cc3..cdadb0c 100644
--- a/linux-user/arm/syscall.h
+++ b/linux-user/arm/syscall.h
@@ -44,3 +44,5 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS
+
+#define TARGET_MINSIGSTKSZ 2048
diff --git a/linux-user/cris/syscall.h b/linux-user/cris/syscall.h
index f5783c0..a75bcc4 100644
--- a/linux-user/cris/syscall.h
+++ b/linux-user/cris/syscall.h
@@ -39,5 +39,6 @@ struct target_pt_regs {
 };
 
 #define TARGET_CLONE_BACKWARDS2
+#define TARGET_MINSIGSTKSZ 2048
 
 #endif
diff --git a/linux-user/i386/syscall.h b/linux-user/i386/syscall.h
index 9bfc1ad..acf6856 100644
--- a/linux-user/i386/syscall.h
+++ b/linux-user/i386/syscall.h
@@ -147,3 +147,4 @@ struct target_vm86plus_struct {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS
+#define TARGET_MINSIGSTKSZ 2048
diff --git a/linux-user/m68k/syscall.h b/linux-user/m68k/syscall.h
index 889eaf7..f8553f8 100644
--- a/linux-user/m68k/syscall.h
+++ b/linux-user/m68k/syscall.h
@@ -18,4 +18,6 @@ struct target_pt_regs {
 #define UNAME_MACHINE "m68k"
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
+#define TARGET_MINSIGSTKSZ 2048
+
 void do_m68k_simcall(CPUM68KState *, int);
diff --git a/linux-user/microblaze/syscall.h b/linux-user/microblaze/syscall.h
index 5b5f6b4..2a5e160 100644
--- a/linux-user/microblaze/syscall.h
+++ b/linux-user/microblaze/syscall.h
@@ -49,5 +49,6 @@ struct target_pt_regs {
 };
 
 #define TARGET_CLONE_BACKWARDS
+#define TARGET_MINSIGSTKSZ  2048
 
 #endif
diff --git a/linux-user/mips/syscall.h b/linux-user/mips/syscall.h
index 5bc5696..0b4662c 100644
--- a/linux-user/mips/syscall.h
+++ b/linux-user/mips/syscall.h
@@ -228,3 +228,4 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS
+#define TARGET_MINSIGSTKSZ 2048
diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h
index a7f5a58..39b8bed 100644
--- a/linux-user/mips64/syscall.h
+++ b/linux-user/mips64/syscall.h
@@ -225,3 +225,4 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS
+#define TARGET_MINSIGSTKSZ  2048
diff --git a/linux-user/openrisc/syscall.h b/linux-user/openrisc/syscall.h
index c3b36da..e5e6180 100644
--- a/linux-user/openrisc/syscall.h
+++ b/linux-user/openrisc/syscall.h
@@ -23,3 +23,5 @@ struct target_pt_regs {
 
 #define UNAME_MACHINE "openrisc"
 #define UNAME_MINIMUM_RELEASE "2.6.32"
+
+#define TARGET_MINSIGSTKSZ 2048
diff --git a/linux-user/ppc/syscall.h b/linux-user/ppc/syscall.h
index db92bbe..5311cc6 100644
--- a/linux-user/ppc/syscall.h
+++ b/linux-user/ppc/syscall.h
@@ -69,3 +69,5 @@ struct target_revectored_struct {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS
+
+#define TARGET_MINSIGSTKSZ 2048
diff --git a/linux-user/s390x/syscall.h b/linux-user/s390x/syscall.h
index aaad512..b11a3b2 100644
--- a/linux-user/s390x/syscall.h
+++ b/linux-user/s390x/syscall.h
@@ -24,3 +24,4 @@ struct target_pt_regs {
 #define UNAME_MINIMUM_RELEASE "2.6.32"
 
 #define TARGET_CLONE_BACKWARDS2
+#define TARGET_MINSIGSTKSZ2048
diff --git a/linux-user/sh4/syscall.h b/linux-user/sh4/syscall.h
index ccd2216..285ecf3 100644
--- a/linux-user/sh4/sy

Re: [Qemu-devel] [PATCH] net: Forbid dealing with packets when VM is not running

2014-08-19 Thread Dr. David Alan Gilbert
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> On 2014/8/18 20:27, Dr. David Alan Gilbert wrote:
> >* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> >>For all NICs(except virtio-net) emulated by qemu,
> >>Such as e1000, rtl8139, pcnet and ne2k_pci,
> >>Qemu can still receive packets when VM is not running.
> >>If this happened in *migration's* last PAUSE VM stage,
> >>The new dirty RAM related to the packets will be missed,
> >>And this will lead serious network fault in VM.
> >
> >I'd like to understand more about when exactly this happens.
> >migration.c:migration_thread  in the last step, takes the iothread, puts
> >the runstate into RUN_STATE_FINISH_MIGRATE and only then calls 
> >qemu_savevm_state_complete
> >before unlocking the iothread.
> >
> 
> Hmm, Sorry, the description above may not be exact.
> 
> Actually, the action of receiving packets action happens after the
> migration thread unlock the iothread-lock(when VM is stopped), but
> before the end of the migration(to be exact, before close the socket
> fd, which is used to send data to destination).
> 
> I think before close the socket fd, we can not assure all the RAM of the
> VM has been copied to the memory of network card or has been send to
> the destination, we still have the chance to modify its content. (If i
> am wrong, please let me know, Thanks. ;) )
> 
> If the above situation happens, it will dirty parts of the RAM which
> will be send and parts of new dirty RAM page may be missed. As a result,
> The contents of memory in the destination is not integral, And this
> will lead network fault in VM.

Interesting; the only reason I can think that would happen, is because
arch_init.c:ram_save_page uses qemu_put_buffer_async to send most of the RAM 
pages
and that won't make sure the write happens until later.

This fix will probably help other migration code as well; postcopy
runs the migration for a lot longer after stopping the CPUs, and I am seeing
something that might be due to this very occasionally.

Dave

> Here i have made a test:
> (1) Download the new qemu.
> (2) Extend the time window between qemu_savevm_state_complete and
> migrate_fd_cleanup(where qemu_fclose(s->file) will be called), patch
> like(this will extend the chances of receiving packets between the time
> window):
> diff --git a/migration.c b/migration.c
> index 8d675b3..597abf9 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -614,7 +614,7 @@ static void *migration_thread(void *opaque)
>  qemu_savevm_state_complete(s->file);
>  }
>  qemu_mutex_unlock_iothread();
> -
> +sleep(2);/*extend the time window between stop vm and end
> migration*/
>  if (ret < 0) {
>  migrate_set_state(s, MIG_STATE_ACTIVE,
> MIG_STATE_ERROR);
>  break;
> (3) Start VM (suse11sp1) and in VM ping xxx -i 0.1
> (4) Migrate the VM to another host.
> 
> And the *PING* command in VM will very likely fail with message:
> 'Destination HOST Unreachable', the NIC in VM will stay unavailable
> unless you run 'service network restart'.(without step 2, you may have
> to migration lots of times between two hosts before that happens).
> 
> Thanks,
> zhanghailiang
> 
> >If that's true, how does this problem happen - can the net code
> >still receive packets with the iothread lock taken?
> >qemu_savevm_state_complete does a call to the RAM code, so I think this 
> >should get
> >any RAM changes that happened before the lock was taken.
> >
> >I'm gently worried about threading stuff as well - is all this net code
> >running off fd handlers on the main thread or are there separate threads?
> >
> >Dave
> >
> >>To avoid this, we forbid receiving packets in generic net code when
> >>VM is not running. Also, when the runstate changes back to running,
> >>we definitely need to flush queues to get packets flowing again.
> >>
> >>Here we implement this in the net layer:
> >>(1) Judge the vm runstate in qemu_can_send_packet
> >>(2) Add a member 'VMChangeStateEntry *vmstate' to struct NICState,
> >>Which will listen for VM runstate changes.
> >>(3) Register a handler function for VMstate change.
> >>When vm changes back to running, we flush all queues in the callback 
> >>function.
> >>(4) Remove checking vm state in virtio_net_can_receive
> >>
> >>Signed-off-by: zhanghailiang
> >>---
> >>  hw/net/virtio-net.c |  4 
> >>  include/net/net.h   |  2 ++
> >>  net/net.c   | 32 
> >>  3 files changed, 34 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>index 268eff9..287d762 100644
> >>--- a/hw/net/virtio-net.c
> >>+++ b/hw/net/virtio-net.c
> >>@@ -839,10 +839,6 @@ static int virtio_net_can_receive(NetClientState *nc)
> >>  VirtIODevice *vdev = VIRTIO_DEVICE(n);
> >>  VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> >>
> >>-if (!vdev->vm_running) {
> >>-return 0;
> >

Re: [Qemu-devel] Microcheckpointing: Memory-VCPU / Disk State consistency

2014-08-19 Thread Walid Nouri
Hi Paolo,
thanks for your hint. I missed your mail from last sunday.
I will take a look on that!

Walid

> Am 17.08.2014 um 11:52 schrieb Paolo Bonzini :
> 
> Il 11/08/2014 22:15, Michael R. Hines ha scritto:
>> Excellent question: QEMU does have a feature called "drive-mirror"
>> in block/mirror.c that was introduced a couple of years ago. I'm not
>> sure what the
>> adoption rate of the feature is, but I would start with that one.
> 
> block/mirror.c is asynchronous, and there's no support for communicating
> checkpoints back to the master.  However, the quorum disk driver could
> be what you need.
> 
> There's also a series on the mailing list that lets quorum read only
> from the primary, so that quorum can still do replication and fault
> tolerance, but skip fault detection.
> 
> Paolo
> 
>> There is also a second fault tolerance implementation that works a
>> little differently called
>> "COLO" - you may have seen those emails on the list too, but their
>> method does not require a disk replication solution, if I recall correctly.
> 



Re: [Qemu-devel] [PATCH V2 0/2] runner: Control test duration

2014-08-19 Thread Stefan Hajnoczi
On Tue, Aug 19, 2014 at 12:02:33AM +0400, Maria Kustova wrote:
> The first patch adds the '--duration SECONDS' argument. After the specified
> duration the runner allows to end the current test and then exits.
> 
> The second patch adds forced termination of a program under test, if the test
> execution takes more than 10 minutes to indicate program freezes.
> 
> If a program under test hangs, then the specified test duration can be overrun
> up to 10 minutes.
> 
> The patch series is based on https://github.com/stefanha/qemu/commits/block,
> commit 07a45925fa88376f8583a333e74f7eeb0f455685
> 
> v1 -> v2:
>  * Trivial fixes based on the review of Fam Zheng
>  * Increased time-out (in some cases 5 minutes interval returned false
>negatives)
> 
> Maria Kustova (2):
>   runner: Add an argument for test duration
>   runner: Kill a program under test by time-out
> 
>  tests/image-fuzzer/runner.py | 50 
> +---
>  1 file changed, 42 insertions(+), 8 deletions(-)

Reviewed-by: Stefan Hajnoczi 


pgpQnasBDCwmO.pgp
Description: PGP signature


Re: [Qemu-devel] [PULL] virtio-serial: avoid duplicate port names

2014-08-19 Thread Peter Maydell
On 18 August 2014 18:18, Amit Shah  wrote:
> Hi,
>
> Patches have been on list for a while..
>
> The following changes since commit 08ab59770da57648bfb8fc9be37f0ef7fb50b0f9:
>
>   Merge remote-tracking branch 'remotes/mcayland/qemu-sparc' into staging 
> (2014-08-18 12:55:02 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git for-2.2
>
> for you to fetch changes up to d0a0bfe6729ef6044d76ea49fafa07e29fa598bd:
>
>   virtio-serial: search for duplicate port names before adding new ports 
> (2014-08-18 22:42:49 +0530)
>
> 
> Amit Shah (2):
>   virtio-serial: create a linked list of all active devices
>   virtio-serial: search for duplicate port names before adding new ports
>
>  hw/char/virtio-serial-bus.c   | 32 
>  include/hw/virtio/virtio-serial.h |  2 ++
>  2 files changed, 34 insertions(+)

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH 3/3] layout: Add generators for refcount tbles and blocks

2014-08-19 Thread Fam Zheng
On Mon, 08/11 15:55, Maria Kustova wrote:
> Refcount structures are placed in clusters randomly selected from all not
> allocated host clusters.

s/not allocated/unallocated/

> 
> Signed-off-by: Maria Kustova 
> ---
>  tests/image-fuzzer/qcow2/layout.py | 136 
> -
>  1 file changed, 135 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/image-fuzzer/qcow2/layout.py 
> b/tests/image-fuzzer/qcow2/layout.py
> index 730c771..2239789 100644
> --- a/tests/image-fuzzer/qcow2/layout.py
> +++ b/tests/image-fuzzer/qcow2/layout.py
> @@ -102,6 +102,8 @@ class Image(object):
>  self.end_of_extension_area = FieldsList()
>  self.l2_tables = FieldsList()
>  self.l1_table = FieldsList()
> +self.refcount_table = FieldsList()
> +self.refcount_blocks = FieldsList()
>  self.ext_offset = 0
>  self.create_header(cluster_bits, backing_file_name)
>  self.set_backing_file_name(backing_file_name)
> @@ -113,7 +115,8 @@ class Image(object):
>  def __iter__(self):
>  return chain(self.header, self.backing_file_format,
>   self.feature_name_table, self.end_of_extension_area,
> - self.backing_file_name, self.l1_table, self.l2_tables)
> + self.backing_file_name, self.l1_table, self.l2_tables,
> + self.refcount_table, self.refcount_blocks)
>  
>  def create_header(self, cluster_bits, backing_file_name=None):
>  """Generate a random valid header."""
> @@ -330,6 +333,136 @@ class Image(object):
>  float(self.cluster_size**2)))
>  self.header['l1_table_offset'][0].value = l1_offset
>  
> +def create_refcount_structures(self):
> +"""Generate random refcount blocks and refcount table."""
> +def allocate_rfc_blocks(data, size):
> +"""Return indices of clusters allocated for recount blocks."""

s/recount/refcount/

> +cluster_ids = set()
> +diff = block_ids = set([x / size for x in data])
> +while len(diff) != 0:
> +# Allocate all yet not allocated clusters
> +new = self._get_available_clusters(data | cluster_ids,
> +   len(diff))
> +# Indices of new refcount blocks necessary to cover clusters
> +# in 'new'
> +diff = set([x / size for x in new]) - block_ids
> +cluster_ids |= new
> +block_ids |= diff
> +return cluster_ids, block_ids
> +
> +def allocate_rfc_table(data, init_blocks, block_size):
> +"""Return indices of clusters allocated for the refcount table
> +and updated indices of clusters allocated for blocks and indices
> +of blocks.
> +"""
> +blocks = set(init_blocks)
> +clusters = set()
> +# Number of entries in one cluster of the refcount table
> +size = self.cluster_size / UINT64_S
> +# Number of clusters necessary for the refcount table based on
> +# the current number of refcount blocks
> +table_size = int(ceil((max(blocks) + 1) / float(size)))
> +# Index of the first cluster of the refcount table
> +table_start = self._get_adjacent_clusters(data, table_size + 1)
> +# Clusters allocated for the current length of the refcount table
> +table_clusters = set(range(table_start, table_start + 
> table_size))
> +# Clusters allocated for the refcount table including
> +# last optional one for potential l1 growth
> +table_clusters_allocated = set(range(table_start, table_start +
> + table_size + 1))
> +# New refcount blocks necessary for clusters occupied by the
> +# refcount table
> +diff = set([c / block_size for c in table_clusters]) - blocks
> +blocks |= diff
> +while len(diff) != 0:
> +# Allocate clusters for new refcount blocks
> +new = self._get_available_clusters((data | clusters) |
> +   table_clusters_allocated,
> +   len(diff))
> +# Indices of new refcount blocks necessary to cover
> +# clusters in 'new'
> +diff = set([x / block_size for x in new]) - blocks
> +clusters |= new
> +blocks |= diff
> +# Check if the refcount table needs one more cluster
> +if int(ceil((max(blocks) + 1) / float(size))) > table_size:
> +new_block_id = (table_start + table_size) / block_size
> +# Check if the additional table cluster needs
> +# o

Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

2014-08-19 Thread Stefan Hajnoczi
On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote:
> diff --git a/tests/image-fuzzer/qcow2/fuzz.py 
> b/tests/image-fuzzer/qcow2/fuzz.py
> index 6e272c6..c652dc9 100644
> --- a/tests/image-fuzzer/qcow2/fuzz.py
> +++ b/tests/image-fuzzer/qcow2/fuzz.py
> @@ -123,7 +123,7 @@ def string_validator(current, strings):
>  return validator(current, random.choice, strings)
>  
>  
> -def selector(current, constraints, validate=int_validator):
> +def selector(current, constraints, validate=None):
>  """Select one value from all defined by constraints.
>  
>  Each constraint produces one random value satisfying to it. The function
> @@ -131,6 +131,9 @@ def selector(current, constraints, 
> validate=int_validator):
>  constraints overlaps).
>  """
>  
> +if validate is None:
> +validate = int_validator
> +
>  def iter_validate(c):
>  """Apply validate() only to constraints represented as lists.
>  

I don't understand the benefit of this change.  The default initializer
syntax did what was intended, now it has been made more complicated with
a None value that gets converted to int_validator later.


pgpNVPpauhild.pgp
Description: PGP signature


[Qemu-devel] [PATCH 3/3] pcie: using error_setg instead of impolite assert

2014-08-19 Thread arei.gonglei
From: Gonglei 

It's enough of reporting an error. Assert() is not acceptable
because the error is not a fatal error.

Signed-off-by: Gonglei 
---
 hw/pci/pcie.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index a123c01..7b46140 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -254,7 +254,11 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, 
DeviceState *dev,
  * Right now, only a device of function = 0 is allowed to be
  * hot plugged/unplugged.
  */
-assert(PCI_FUNC(pci_dev->devfn) == 0);
+if (PCI_FUNC(pci_dev->devfn) != 0) {
+error_setg(errp, "Unsupported device function %d for PCIe hotplugging, 
"
+   "only supported function 0", PCI_FUNC(pci_dev->devfn));
+return;
+}
 
 pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
PCI_EXP_SLTSTA_PDS);
-- 
1.7.12.4





[Qemu-devel] [PATCH 2/3] qdev: Refactor device_set_realized to avoid resource leak

2014-08-19 Thread arei.gonglei
From: Gonglei 

At present, the local variable local_err is reused at multi-places,
Which will cause resource leak in some scenarios.

Example:

1. Assuming that "dc->realize(dev, &local_err)" execute successful
   and local_err == NULL;
2. Executing device hotplug in hotplug_handler_plug(), but failed
  (It is prone to occur). Then local_err != NULL;
3. error_propagate(errp, local_err) and return. But the resources
 which been allocated in dc->realize() will be leaked.
 Simple backtrace:
  dc->realize()
   |->device_realize
|->pci_qdev_init()
|->do_pci_register_device()
|->etc.

To avoid the resource leak, using some different local error variables.
For different error conditions, release the corresponding resources.

Signed-off-by: Gonglei 
---
 hw/core/qdev.c | 75 +++---
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 3e7085e..b3a463b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -839,41 +839,58 @@ static void device_set_realized(Object *obj, bool value, 
Error **errp)
 dc->realize(dev, &local_err);
 }
 
-if (dev->parent_bus && dev->parent_bus->hotplug_handler &&
-local_err == NULL) {
-hotplug_handler_plug(dev->parent_bus->hotplug_handler,
- dev, &local_err);
-} else if (local_err == NULL &&
-   object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
-HotplugHandler *hotplug_ctrl;
-MachineState *machine = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-if (mc->get_hotplug_handler) {
-hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
-if (hotplug_ctrl) {
-hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
+if (local_err == NULL) {
+Error *hotplug_err = NULL;
+
+if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+hotplug_handler_plug(dev->parent_bus->hotplug_handler,
+ dev, &hotplug_err);
+} else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
+HotplugHandler *hotplug_ctrl;
+MachineState *machine = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+if (mc->get_hotplug_handler) {
+hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
+if (hotplug_ctrl) {
+hotplug_handler_plug(hotplug_ctrl, dev, &hotplug_err);
+}
 }
 }
-}
 
-if (qdev_get_vmsd(dev) && local_err == NULL) {
-vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
-   dev->instance_id_alias,
-   dev->alias_required_for_version);
-}
-if (local_err == NULL) {
-QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-object_property_set_bool(OBJECT(bus), true, "realized",
- &local_err);
-if (local_err != NULL) {
-break;
+if (hotplug_err == NULL) {
+Error *err = NULL;
+if (qdev_get_vmsd(dev)) {
+vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev),
+   dev, dev->instance_id_alias,
+   
dev->alias_required_for_version);
 }
+
+QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+object_property_set_bool(OBJECT(bus), true, "realized",
+ &err);
+if (err != NULL) {
+if (qdev_get_vmsd(dev)) {
+vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
+}
+
+break;
+}
+}
+
+if (dev->hotplugged && err == NULL) {
+device_reset(dev);
+}
+error_free(err);
+} else {
+if (dc->unrealize) {
+dc->unrealize(dev, NULL);
+}
+
+error_propagate(errp, hotplug_err);
+return;
 }
 }
-if (dev->hotplugged && local_err == NULL) {
-device_reset(dev);
-}
 dev->pending_deleted_event = false;
 } else if (!value && dev->realized) {
 QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-- 
1.7.12.4





[Qemu-devel] [PATCH 1/3] qdev: add missing error check

2014-08-19 Thread arei.gonglei
From: Gonglei 

If local_err is not null, the next code logic is useless.

Signed-off-by: Gonglei 
---
 hw/core/qdev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index da1ba48..3e7085e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -830,6 +830,11 @@ static void device_set_realized(Object *obj, bool value, 
Error **errp)
 g_free(name);
 }
 
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
+
 if (dc->realize) {
 dc->realize(dev, &local_err);
 }
-- 
1.7.12.4





[Qemu-devel] [PATCH 0/3] Refactor device_set_realized to avoid resource leak.

2014-08-19 Thread arei.gonglei
From: Gonglei 

after committing
  [PATCH v6 0/9] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug 
API

if devcie hotplgging failed, will casuse resource leak.

This patch series include address resouce leak and two other issuses.


BTW, for patch 2/3, checkpatch.py report a warning, but I have no idea how
to handle this probleam. Any ideas?

WARNING: line over 80 characters
#90: FILE: hw/core/qdev.c:866:
+   dev->alias_required_for_version);

total: 0 errors, 1 warnings, 87 lines checked

Please review, thanks in advance.

Gonglei (3):
  qdev: add missing error check
  qdev: Refactor device_set_realized to avoid resource
  pcie: using error_setg instead of impolite assert

 hw/core/qdev.c | 80 +-
 hw/pci/pcie.c  |  6 -
 2 files changed, 56 insertions(+), 30 deletions(-)

-- 
1.7.12.4





Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

2014-08-19 Thread Fam Zheng
On Tue, 08/19 02:00, Maria Kustova wrote:
> Signed-off-by: Maria Kustova 
> ---
>  tests/image-fuzzer/qcow2/fuzz.py | 15 ++--
>  tests/image-fuzzer/runner.py | 51 
> 
>  2 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/image-fuzzer/qcow2/fuzz.py 
> b/tests/image-fuzzer/qcow2/fuzz.py
> index 6e272c6..c652dc9 100644
> --- a/tests/image-fuzzer/qcow2/fuzz.py
> +++ b/tests/image-fuzzer/qcow2/fuzz.py
> @@ -123,7 +123,7 @@ def string_validator(current, strings):
>  return validator(current, random.choice, strings)
>  
>  
> -def selector(current, constraints, validate=int_validator):
> +def selector(current, constraints, validate=None):
>  """Select one value from all defined by constraints.
>  
>  Each constraint produces one random value satisfying to it. The function
> @@ -131,6 +131,9 @@ def selector(current, constraints, 
> validate=int_validator):
>  constraints overlaps).
>  """
>  
> +if validate is None:
> +validate = int_validator
> +
>  def iter_validate(c):
>  """Apply validate() only to constraints represented as lists.
>  
> @@ -337,9 +340,8 @@ def l1_entry(current):
>  constraints = UINT64_V
>  # Reserved bits are ignored
>  # Added a possibility when only flags are fuzzed
> -offset = 0x7fff & random.choice([selector(current,
> -  constraints),
> - current])
> +offset = 0x7fff & \
> + random.choice([selector(current, constraints), current])
>  is_cow = random.randint(0, 1)
>  return offset + (is_cow << UINT64_M)
>  
> @@ -349,9 +351,8 @@ def l2_entry(current):
>  constraints = UINT64_V
>  # Reserved bits are ignored
>  # Add a possibility when only flags are fuzzed
> -offset = 0x3ffe & random.choice([selector(current,
> -  constraints),
> - current])
> +offset = 0x3ffe & \
> + random.choice([selector(current, constraints), current])
>  is_compressed = random.randint(0, 1)
>  is_cow = random.randint(0, 1)
>  is_zero = random.randint(0, 1)
> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
> index fd97c40..b142577 100755
> --- a/tests/image-fuzzer/runner.py
> +++ b/tests/image-fuzzer/runner.py
> @@ -73,7 +73,7 @@ def run_app(fd, q_args):
>  """Exception for signal.alarm events."""
>  pass
>  
> -def handler(*arg):
> +def handler(*args):
>  """Notify that an alarm event occurred."""
>  raise Alarm
>  
> @@ -137,8 +137,8 @@ class TestEnv(object):
>  self.init_path = os.getcwd()
>  self.work_dir = work_dir
>  self.current_dir = os.path.join(work_dir, 'test-' + test_id)
> -self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\
> -  .strip().split(' ')
> +self.qemu_img = \
> +os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ')
>  self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' 
> ')
>  strings = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
> '%d%d%d%d', '%s%s%s%s', '%999s', '%08x', '%%20d',
> @@ -247,10 +247,8 @@ class TestEnv(object):
>  
>  os.chdir(self.current_dir)
>  backing_file_name, backing_file_fmt = self._create_backing_file()
> -img_size = image_generator.create_image('test.img',
> -backing_file_name,
> -backing_file_fmt,
> -fuzz_config)

Actually this one is fine, but the new way is good too.

> +img_size = image_generator.create_image(
> +'test.img', backing_file_name, backing_file_fmt, fuzz_config)
>  for item in commands:
>  shutil.copy('test.img', 'copy.img')
>  # 'off' and 'len' are multiple of the sector size
> @@ -263,7 +261,7 @@ class TestEnv(object):
>  elif item[0] == 'qemu-io':
>  current_cmd = list(self.qemu_io)
>  else:
> -multilog("Warning: test command '%s' is not defined.\n" \
> +multilog("Warning: test command '%s' is not defined.\n"
>   % item[0], sys.stderr, self.log, self.parent_log)
>  continue
>  # Replace all placeholders with their real values
> @@ -279,29 +277,30 @@ class TestEnv(object):
> "Backing file: %s\n" \
> % (self.seed, " ".join(current_cmd),
>self.current_dir, backing_file_name)
> -
>  temp_log = StringIO.StringIO()
>  try:
>  

Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

2014-08-19 Thread M.Kustova
On Tue, Aug 19, 2014 at 1:38 PM, Stefan Hajnoczi  wrote:
> On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote:
>> diff --git a/tests/image-fuzzer/qcow2/fuzz.py 
>> b/tests/image-fuzzer/qcow2/fuzz.py
>> index 6e272c6..c652dc9 100644
>> --- a/tests/image-fuzzer/qcow2/fuzz.py
>> +++ b/tests/image-fuzzer/qcow2/fuzz.py
>> @@ -123,7 +123,7 @@ def string_validator(current, strings):
>>  return validator(current, random.choice, strings)
>>
>>
>> -def selector(current, constraints, validate=int_validator):
>> +def selector(current, constraints, validate=None):
>>  """Select one value from all defined by constraints.
>>
>>  Each constraint produces one random value satisfying to it. The function
>> @@ -131,6 +131,9 @@ def selector(current, constraints, 
>> validate=int_validator):
>>  constraints overlaps).
>>  """
>>
>> +if validate is None:
>> +validate = int_validator
>> +
>>  def iter_validate(c):
>>  """Apply validate() only to constraints represented as lists.
>>
>
> I don't understand the benefit of this change.  The default initializer
> syntax did what was intended, now it has been made more complicated with
> a None value that gets converted to int_validator later.

It's a convention rule that allows keep the function signature
unchanged in case of modified function internals.
In other words, wrapper functions should not change in response of the
internal function change.

Maria.



Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

2014-08-19 Thread M.Kustova
On Tue, Aug 19, 2014 at 1:44 PM, Fam Zheng  wrote:
> On Tue, 08/19 02:00, Maria Kustova wrote:
>> Signed-off-by: Maria Kustova 
>> ---
>>  tests/image-fuzzer/qcow2/fuzz.py | 15 ++--
>>  tests/image-fuzzer/runner.py | 51 
>> 
>>  2 files changed, 34 insertions(+), 32 deletions(-)
>>
>> diff --git a/tests/image-fuzzer/qcow2/fuzz.py 
>> b/tests/image-fuzzer/qcow2/fuzz.py
>> index 6e272c6..c652dc9 100644
>> --- a/tests/image-fuzzer/qcow2/fuzz.py
>> +++ b/tests/image-fuzzer/qcow2/fuzz.py
>> @@ -123,7 +123,7 @@ def string_validator(current, strings):
>>  return validator(current, random.choice, strings)
>>
>>
>> -def selector(current, constraints, validate=int_validator):
>> +def selector(current, constraints, validate=None):
>>  """Select one value from all defined by constraints.
>>
>>  Each constraint produces one random value satisfying to it. The function
>> @@ -131,6 +131,9 @@ def selector(current, constraints, 
>> validate=int_validator):
>>  constraints overlaps).
>>  """
>>
>> +if validate is None:
>> +validate = int_validator
>> +
>>  def iter_validate(c):
>>  """Apply validate() only to constraints represented as lists.
>>
>> @@ -337,9 +340,8 @@ def l1_entry(current):
>>  constraints = UINT64_V
>>  # Reserved bits are ignored
>>  # Added a possibility when only flags are fuzzed
>> -offset = 0x7fff & random.choice([selector(current,
>> -  constraints),
>> - current])
>> +offset = 0x7fff & \
>> + random.choice([selector(current, constraints), current])
>>  is_cow = random.randint(0, 1)
>>  return offset + (is_cow << UINT64_M)
>>
>> @@ -349,9 +351,8 @@ def l2_entry(current):
>>  constraints = UINT64_V
>>  # Reserved bits are ignored
>>  # Add a possibility when only flags are fuzzed
>> -offset = 0x3ffe & random.choice([selector(current,
>> -  constraints),
>> - current])
>> +offset = 0x3ffe & \
>> + random.choice([selector(current, constraints), current])
>>  is_compressed = random.randint(0, 1)
>>  is_cow = random.randint(0, 1)
>>  is_zero = random.randint(0, 1)
>> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
>> index fd97c40..b142577 100755
>> --- a/tests/image-fuzzer/runner.py
>> +++ b/tests/image-fuzzer/runner.py
>> @@ -73,7 +73,7 @@ def run_app(fd, q_args):
>>  """Exception for signal.alarm events."""
>>  pass
>>
>> -def handler(*arg):
>> +def handler(*args):
>>  """Notify that an alarm event occurred."""
>>  raise Alarm
>>
>> @@ -137,8 +137,8 @@ class TestEnv(object):
>>  self.init_path = os.getcwd()
>>  self.work_dir = work_dir
>>  self.current_dir = os.path.join(work_dir, 'test-' + test_id)
>> -self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\
>> -  .strip().split(' ')
>> +self.qemu_img = \
>> +os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ')
>>  self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' 
>> ')
>>  strings = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
>> '%d%d%d%d', '%s%s%s%s', '%999s', '%08x', '%%20d',
>> @@ -247,10 +247,8 @@ class TestEnv(object):
>>
>>  os.chdir(self.current_dir)
>>  backing_file_name, backing_file_fmt = self._create_backing_file()
>> -img_size = image_generator.create_image('test.img',
>> -backing_file_name,
>> -backing_file_fmt,
>> -fuzz_config)
>
> Actually this one is fine, but the new way is good too.
>
>> +img_size = image_generator.create_image(
>> +'test.img', backing_file_name, backing_file_fmt, fuzz_config)
>>  for item in commands:
>>  shutil.copy('test.img', 'copy.img')
>>  # 'off' and 'len' are multiple of the sector size
>> @@ -263,7 +261,7 @@ class TestEnv(object):
>>  elif item[0] == 'qemu-io':
>>  current_cmd = list(self.qemu_io)
>>  else:
>> -multilog("Warning: test command '%s' is not defined.\n" \
>> +multilog("Warning: test command '%s' is not defined.\n"
>>   % item[0], sys.stderr, self.log, self.parent_log)
>>  continue
>>  # Replace all placeholders with their real values
>> @@ -279,29 +277,30 @@ class TestEnv(object):
>> "Backing file: %s\n" \
>> % (self.seed, " ".join(current_cmd),
>> 

Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code

2014-08-19 Thread Edgar E. Iglesias
On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote:
> Implement ARMv8 software single-step handling for A64 code:
> correctly update the single-step state machine and generate
> debug exceptions when stepping A64 code.
> 
> This patch has no behavioural change since MDSCR_EL1.SS can't
> be set by the guest yet.

Hi Peter,

> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/cpu.h   | 21 +++
>  target-arm/helper.h|  1 +
>  target-arm/internals.h |  6 +++
>  target-arm/op_helper.c |  5 +++
>  target-arm/translate-a64.c | 91 
> +++---
>  target-arm/translate.h | 12 ++
>  6 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 74f7b15..ac01524 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT)
>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3
> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)

Shouldn't these shifts/masks differ?



>  
>  /* some convenience accessor macros */
>  #define ARM_TBFLAG_AARCH64_STATE(F) \
> @@ -1235,6 +1239,10 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  (((F) & ARM_TBFLAG_AA64_EL_MASK) >> ARM_TBFLAG_AA64_EL_SHIFT)
>  #define ARM_TBFLAG_AA64_FPEN(F) \
>  (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT)
> +#define ARM_TBFLAG_AA64_SS_ACTIVE(F) \
> +(((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >> 
> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> +#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
> +(((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >> 
> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>  
>  static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>  target_ulong *cs_base, int *flags)
> @@ -1248,6 +1256,19 @@ static inline void cpu_get_tb_cpu_state(CPUARMState 
> *env, target_ulong *pc,
>  if (fpen == 3 || (fpen == 1 && arm_current_pl(env) != 0)) {
>  *flags |= ARM_TBFLAG_AA64_FPEN_MASK;
>  }
> +/* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
> + * states defined in the ARM ARM for software singlestep:
> + *  SS_ACTIVE   PSTATE.SS   State
> + * 0x   Inactive (the TB flag for SS is always 0)
> + * 10   Active-pending
> + * 11   Active-not-pending
> + */
> +if (arm_singlestep_active(env)) {
> +*flags |= ARM_TBFLAG_AA64_SS_ACTIVE_MASK;
> +if (env->pstate & PSTATE_SS) {
> +*flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
> +}
> +}
>  } else {
>  int privmode;
>  *pc = env->regs[15];
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index facfcd2..1d7003b 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -64,6 +64,7 @@ DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64)
>  DEF_HELPER_2(get_cp_reg64, i64, env, ptr)
>  
>  DEF_HELPER_3(msr_i_pstate, void, env, i32, i32)
> +DEF_HELPER_1(clear_pstate_ss, void, env)
>  DEF_HELPER_1(exception_return, void, env)
>  
>  DEF_HELPER_2(get_r13_banked, i32, env, i32)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 08fa697..53c2e3c 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -290,4 +290,10 @@ static inline uint32_t syn_data_abort(int same_el, int 
> ea, int cm, int s1ptw,
>  | (ea << 9) | (cm << 8) | (s1ptw << 7) | (wnr << 6) | fsc;
>  }
>  
> +static inline uint32_t syn_swstep(int same_el, int isv, int ex)
> +{
> +return (EC_SOFTWARESTEP << ARM_EL_EC_SHIFT) | (same_el << 
> ARM_EL_EC_SHIFT)
> +| (isv << 24) | (ex << 6) | 0x22;
> +}
> +
>  #endif
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 62cc07d..fe40358 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -369,6 +369,11 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
> uint32_t imm)
>  }
>  }
>  
> +void HELPER(clear_pstate_ss)(CPUARMState *env)
> +{
> +env->pstate &= ~PSTATE_SS;
> +}
> +
>  void HELPER(exception_return)(CPUARMState *env)
>  {
>  int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index aa731bf..21cb12b 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -203,10 +203,39 @@ static void gen_exception_insn(DisasContext *s, int 
> offset, int excp,
>  s->is_jmp = DISAS_EXC;
>  }
>  
> +static void gen_ss_advance(DisasContext *s)
> +{
> +/* I

Re: [Qemu-devel] [PATCH 3/3] layout: Add generators for refcount tbles and blocks

2014-08-19 Thread M.Kustova
On Tue, Aug 19, 2014 at 1:36 PM, Fam Zheng  wrote:
> On Mon, 08/11 15:55, Maria Kustova wrote:
>> Refcount structures are placed in clusters randomly selected from all not
>> allocated host clusters.
>
> s/not allocated/unallocated/
>
>>
>> Signed-off-by: Maria Kustova 
>> ---
>>  tests/image-fuzzer/qcow2/layout.py | 136 
>> -
>>  1 file changed, 135 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/image-fuzzer/qcow2/layout.py 
>> b/tests/image-fuzzer/qcow2/layout.py
>> index 730c771..2239789 100644
>> --- a/tests/image-fuzzer/qcow2/layout.py
>> +++ b/tests/image-fuzzer/qcow2/layout.py
>> @@ -102,6 +102,8 @@ class Image(object):
>>  self.end_of_extension_area = FieldsList()
>>  self.l2_tables = FieldsList()
>>  self.l1_table = FieldsList()
>> +self.refcount_table = FieldsList()
>> +self.refcount_blocks = FieldsList()
>>  self.ext_offset = 0
>>  self.create_header(cluster_bits, backing_file_name)
>>  self.set_backing_file_name(backing_file_name)
>> @@ -113,7 +115,8 @@ class Image(object):
>>  def __iter__(self):
>>  return chain(self.header, self.backing_file_format,
>>   self.feature_name_table, self.end_of_extension_area,
>> - self.backing_file_name, self.l1_table, self.l2_tables)
>> + self.backing_file_name, self.l1_table, self.l2_tables,
>> + self.refcount_table, self.refcount_blocks)
>>
>>  def create_header(self, cluster_bits, backing_file_name=None):
>>  """Generate a random valid header."""
>> @@ -330,6 +333,136 @@ class Image(object):
>>  
>> float(self.cluster_size**2)))
>>  self.header['l1_table_offset'][0].value = l1_offset
>>
>> +def create_refcount_structures(self):
>> +"""Generate random refcount blocks and refcount table."""
>> +def allocate_rfc_blocks(data, size):
>> +"""Return indices of clusters allocated for recount blocks."""
>
> s/recount/refcount/
>
>> +cluster_ids = set()
>> +diff = block_ids = set([x / size for x in data])
>> +while len(diff) != 0:
>> +# Allocate all yet not allocated clusters
>> +new = self._get_available_clusters(data | cluster_ids,
>> +   len(diff))
>> +# Indices of new refcount blocks necessary to cover clusters
>> +# in 'new'
>> +diff = set([x / size for x in new]) - block_ids
>> +cluster_ids |= new
>> +block_ids |= diff
>> +return cluster_ids, block_ids
>> +
>> +def allocate_rfc_table(data, init_blocks, block_size):
>> +"""Return indices of clusters allocated for the refcount table
>> +and updated indices of clusters allocated for blocks and indices
>> +of blocks.
>> +"""
>> +blocks = set(init_blocks)
>> +clusters = set()
>> +# Number of entries in one cluster of the refcount table
>> +size = self.cluster_size / UINT64_S
>> +# Number of clusters necessary for the refcount table based on
>> +# the current number of refcount blocks
>> +table_size = int(ceil((max(blocks) + 1) / float(size)))
>> +# Index of the first cluster of the refcount table
>> +table_start = self._get_adjacent_clusters(data, table_size + 1)
>> +# Clusters allocated for the current length of the refcount 
>> table
>> +table_clusters = set(range(table_start, table_start + 
>> table_size))
>> +# Clusters allocated for the refcount table including
>> +# last optional one for potential l1 growth
>> +table_clusters_allocated = set(range(table_start, table_start +
>> + table_size + 1))
>> +# New refcount blocks necessary for clusters occupied by the
>> +# refcount table
>> +diff = set([c / block_size for c in table_clusters]) - blocks
>> +blocks |= diff
>> +while len(diff) != 0:
>> +# Allocate clusters for new refcount blocks
>> +new = self._get_available_clusters((data | clusters) |
>> +   table_clusters_allocated,
>> +   len(diff))
>> +# Indices of new refcount blocks necessary to cover
>> +# clusters in 'new'
>> +diff = set([x / block_size for x in new]) - blocks
>> +clusters |= new
>> +blocks |= diff
>> +# Check if the refcount table needs one more cluster
>> +if int(ceil((max(blocks) + 1) / float(size))) > table_size:
>> +   

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

2014-08-19 Thread Juan Quintela
Juan Quintela  wrote:
> Hi
>
> Please, send any topic that you are interested in covering.
>
> People have complained on the past that I don't cancel the call until
> the very last minute.  So, what do you think that deadline for
> submitting topics is 23:00UTC on Monday?

As there are no topics, call gets cancelled.

Have a good week, Juan.



Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code

2014-08-19 Thread Peter Maydell
On 19 August 2014 10:56, Edgar E. Iglesias  wrote:
> On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote:
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState 
>> *env)
>>  #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT)
>>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
>>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << 
>> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
>> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3
>> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << 
>> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>
> Shouldn't these shifts/masks differ?

Oops. Yes, they certainly should.

-- PMM



Re: [Qemu-devel] [PATCH 0/3] image-fuzzer: Support refcount structures in the qcow2 image generator

2014-08-19 Thread Stefan Hajnoczi
On Mon, Aug 11, 2014 at 03:55:03PM +0400, Maria Kustova wrote:
> This patch series adds support of refcount tables and blocks to the qcow2 
> image
> generator.
> 
> This patch series was created for the 'block-next' branch and based on the 
> next
> series:
>  [PATCH V3] layout: Reduce number of generator functions in __init__
> 
> Maria Kustova (3):
>   docs: List all image elements currently supported by the fuzzer
>   fuzz: Add fuzzing functions for entries of refcount table and blocks
>   layout: Add generators for refcount tbles and blocks
> 
>  docs/image-fuzzer.txt  |   3 +-
>  tests/image-fuzzer/qcow2/fuzz.py   |  16 -
>  tests/image-fuzzer/qcow2/layout.py | 136 
> -
>  3 files changed, 152 insertions(+), 3 deletions(-)
> 
> -- 
> 1.9.3
> 

Reviewed-by: Stefan Hajnoczi 


pgpc49clynS3v.pgp
Description: PGP signature


[Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18

2014-08-19 Thread Paolo Bonzini
The following changes since commit 5a7348045091a2bc15d85bb177e5956aa6114e5a:

  Update version for v2.1.0-rc2 release (2014-07-15 18:55:37 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to f54bb15f9d373877954e44db3a8bb368aff45b42:

  mtree: remove write-only field (2014-08-18 12:06:21 +0200)


SCSI changes that enable sending vendor-specific commands via virtio-scsi.

Memory changes for QOMification and automatic tracking of MR lifetime.


Paolo Bonzini (15):
  scsi-bus: prepare scsi_req_new for introduction of parse_cdb
  scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo
  scsi-block: extract scsi_block_is_passthrough
  scsi-block, scsi-generic: implement parse_cdb
  virtio-scsi: implement parse_cdb
  qom: object: delete properties before calling instance_finalize
  qom: object: move unparenting to the child property's release callback
  sysbus: remove unused function sysbus_del_io
  vga: do not dynamically allocate chain4_alias
  nic: do not destroy memory regions in cleanup functions
  ioport: split deletion and destruction
  memory: convert memory_region_destroy to object_unparent
  memory: remove memory_region_destroy
  tpm_tis: remove instance_finalize callback
  mtree: remove write-only field

Peter Crosthwaite (5):
  loader: Abstract away ref to memory region names
  exec: Abstract away ref to memory region names
  memory: constify memory_region_name
  memory: Use memory_region_name for name access
  memory: Use canonical path component as the name

 backends/hostmem.c | 10 --
 docs/memory.txt| 15 ++---
 exec.c |  4 +--
 hw/audio/ac97.c|  9 -
 hw/audio/es1370.c  |  8 -
 hw/audio/intel-hda.c   |  1 -
 hw/block/nvme.c|  1 -
 hw/block/pflash_cfi01.c|  1 -
 hw/block/pflash_cfi02.c|  1 -
 hw/char/serial-pci.c   |  3 --
 hw/core/loader.c   |  2 +-
 hw/core/sysbus.c   |  5 ---
 hw/display/vga.c   | 24 ++
 hw/display/vga_int.h   |  3 +-
 hw/i386/kvm/pci-assign.c   |  8 -
 hw/i386/kvmvapic.c |  2 +-
 hw/ide/ahci.c  |  2 --
 hw/ide/cmd646.c|  5 ---
 hw/ide/piix.c  |  3 --
 hw/ide/via.c   |  3 --
 hw/ipack/tpci200.c | 13 
 hw/mips/gt64xxx_pci.c  |  2 +-
 hw/misc/ivshmem.c  |  3 --
 hw/misc/omap_gpmc.c|  2 +-
 hw/misc/pci-testdev.c  |  2 --
 hw/misc/vfio.c | 11 ++-
 hw/net/dp8393x.c   |  3 --
 hw/net/e1000.c |  2 --
 hw/net/eepro100.c  |  3 --
 hw/net/mcf_fec.c   |  3 --
 hw/net/ne2000.c|  1 -
 hw/net/pcnet-pci.c |  2 --
 hw/net/rtl8139.c   |  2 --
 hw/net/stellaris_enet.c|  8 -
 hw/net/vmxnet3.c   |  4 ---
 hw/pci-bridge/pci_bridge_dev.c |  2 --
 hw/pci/msix.c  |  4 ---
 hw/pci/pci.c   |  2 --
 hw/pci/pci_bridge.c|  8 -
 hw/pci/pcie_host.c |  1 -
 hw/pci/shpc.c  |  1 -
 hw/ppc/ppc4xx_devs.c   |  2 +-
 hw/scsi/esp-pci.c  |  1 -
 hw/scsi/lsi53c895a.c   | 10 --
 hw/scsi/megasas.c  |  3 --
 hw/scsi/scsi-bus.c | 74 +-
 hw/scsi/scsi-disk.c| 52 ++---
 hw/scsi/scsi-generic.c |  7 
 hw/scsi/virtio-scsi.c  | 25 ++
 hw/scsi/vmw_pvscsi.c   |  3 --
 hw/tpm/tpm_tis.c   |  9 -
 hw/usb/hcd-uhci.c  |  8 -
 hw/virtio/virtio-pci.c |  3 --
 hw/watchdog/wdt_i6300esb.c |  8 -
 hw/xen/xen_pt.c| 20 
 hw/xen/xen_pt_msi.c|  2 --
 include/exec/memory.h  | 12 +--
 include/hw/scsi/scsi.h |  7 
 include/hw/sysbus.h|  1 -
 ioport.c   | 11 +--
 memory.c   | 28 +---
 qom/object.c   | 16 +++--
 62 files changed, 183 insertions(+), 308 deletions(-)
-- 
1.8.3.1




[Qemu-devel] [PULL 03/20] scsi-block: extract scsi_block_is_passthrough

2014-08-19 Thread Paolo Bonzini
This will be used for both scsi_block_new_request and the scsi-block
implementation of parse_cdb.

Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-disk.c | 38 ++
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d47ecd6..81b7276 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2501,12 +2501,8 @@ static int scsi_block_initfn(SCSIDevice *dev)
 return scsi_initfn(&s->qdev);
 }
 
-static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
-   uint32_t lun, uint8_t *buf,
-   void *hba_private)
+static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-
 switch (buf[0]) {
 case READ_6:
 case READ_10:
@@ -2523,9 +2519,9 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
 case WRITE_VERIFY_12:
 case WRITE_VERIFY_16:
 /* If we are not using O_DIRECT, we might read stale data from the
-* host cache if writes were made using other commands than these
-* ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
-* O_DIRECT everything must go through SG_IO.
+ * host cache if writes were made using other commands than these
+ * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
+ * O_DIRECT everything must go through SG_IO.
  */
 if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) {
 break;
@@ -2542,13 +2538,31 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice 
*d, uint32_t tag,
  * just make scsi-block operate the same as scsi-generic for them.
  */
 if (s->qdev.type != TYPE_ROM) {
-return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
-  hba_private);
+return false;
 }
+break;
+
+default:
+break;
 }
 
-return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
-  hba_private);
+return true;
+}
+
+
+static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
+   uint32_t lun, uint8_t *buf,
+   void *hba_private)
+{
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+if (scsi_block_is_passthrough(s, buf)) {
+return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
+  hba_private);
+} else {
+return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
+  hba_private);
+}
 }
 #endif
 
-- 
1.8.3.1





[Qemu-devel] [PULL 05/20] virtio-scsi: implement parse_cdb

2014-08-19 Thread Paolo Bonzini
Enable passthrough of vendor-specific commands.

Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/virtio-scsi.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0eb069a..2dd9255 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -406,6 +406,30 @@ static void virtio_scsi_command_complete(SCSIRequest *r, 
uint32_t status,
 virtio_scsi_complete_cmd_req(req);
 }
 
+static int virtio_scsi_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+ uint8_t *buf, void *hba_private)
+{
+VirtIOSCSIReq *req = hba_private;
+
+if (cmd->len == 0) {
+cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE);
+memcpy(cmd->buf, buf, cmd->len);
+}
+
+/* Extract the direction and mode directly from the request, for
+ * host device passthrough.
+ */
+cmd->xfer = req->qsgl.size;
+if (cmd->xfer == 0) {
+cmd->mode = SCSI_XFER_NONE;
+} else if (iov_size(req->elem.in_sg, req->elem.in_num) > req->resp_size) {
+cmd->mode = SCSI_XFER_FROM_DEV;
+} else {
+cmd->mode = SCSI_XFER_TO_DEV;
+}
+return 0;
+}
+
 static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r)
 {
 VirtIOSCSIReq *req = r->hba_private;
@@ -658,6 +682,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 .change = virtio_scsi_change,
 .hotplug = virtio_scsi_hotplug,
 .hot_unplug = virtio_scsi_hot_unplug,
+.parse_cdb = virtio_scsi_parse_cdb,
 .get_sg_list = virtio_scsi_get_sg_list,
 .save_request = virtio_scsi_save_request,
 .load_request = virtio_scsi_load_request,
-- 
1.8.3.1





[Qemu-devel] [PULL 02/20] scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo

2014-08-19 Thread Paolo Bonzini
These callbacks will let devices do their own request parsing, or
defer it to the bus.  If the bus does not provide an implementation,
in turn, fall back to the default parsing routine.

Swap the first two arguments to scsi_req_parse, and rename it to
scsi_req_parse_cdb, for consistency.

Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 26 +++---
 include/hw/scsi/scsi.h |  6 ++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ca4e9f3..d97d18a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,7 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -54,6 +54,20 @@ static void scsi_device_destroy(SCSIDevice *s)
 }
 }
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+   void *hba_private)
+{
+SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+int rc;
+
+assert(cmd->len == 0);
+rc = scsi_req_parse_cdb(dev, cmd, buf);
+if (bus->info->parse_cdb) {
+rc = bus->info->parse_cdb(dev, cmd, buf, hba_private);
+}
+return rc;
+}
+
 static SCSIRequest *scsi_device_alloc_req(SCSIDevice *s, uint32_t tag, 
uint32_t lun,
   uint8_t *buf, void *hba_private)
 {
@@ -562,6 +576,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 {
 SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
 const SCSIReqOps *ops;
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
 SCSIRequest *req;
 SCSICommand cmd = { .len = 0 };
 int ret;
@@ -587,7 +602,12 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 ops = NULL;
 }
 
-ret = scsi_req_parse(&cmd, d, buf);
+if (ops != NULL || !sc->parse_cdb) {
+ret = scsi_req_parse_cdb(d, &cmd, buf);
+} else {
+ret = sc->parse_cdb(d, &cmd, buf, hba_private);
+}
+
 if (ret != 0) {
 trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
 req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
@@ -1190,7 +1210,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 return lba;
 }
 
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
 int rc;
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1adb549..4a0b860 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -76,6 +76,8 @@ typedef struct SCSIDeviceClass {
 DeviceClass parent_class;
 int (*init)(SCSIDevice *dev);
 void (*destroy)(SCSIDevice *s);
+int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ void *hba_private);
 SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
   uint8_t *buf, void *hba_private);
 void (*unit_attention_reported)(SCSIDevice *s);
@@ -131,6 +133,8 @@ struct SCSIReqOps {
 struct SCSIBusInfo {
 int tcq;
 int max_channel, max_target, max_lun;
+int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ void *hba_private);
 void (*transfer_data)(SCSIRequest *req, uint32_t arg);
 void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
 void (*cancel)(SCSIRequest *req);
@@ -244,6 +248,8 @@ void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+   void *hba_private);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.8.3.1





[Qemu-devel] [PULL 01/20] scsi-bus: prepare scsi_req_new for introduction of parse_cdb

2014-08-19 Thread Paolo Bonzini
The per-SCSIDevice parse_cdb callback must not be called if the
request will go through special SCSIReqOps, so detect the special
cases early enough.

Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 51 ++-
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4341754..ca4e9f3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -561,13 +561,38 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
   uint8_t *buf, void *hba_private)
 {
 SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
+const SCSIReqOps *ops;
 SCSIRequest *req;
-SCSICommand cmd;
+SCSICommand cmd = { .len = 0 };
+int ret;
+
+if ((d->unit_attention.key == UNIT_ATTENTION ||
+ bus->unit_attention.key == UNIT_ATTENTION) &&
+(buf[0] != INQUIRY &&
+ buf[0] != REPORT_LUNS &&
+ buf[0] != GET_CONFIGURATION &&
+ buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
+
+ /*
+  * If we already have a pending unit attention condition,
+  * report this one before triggering another one.
+  */
+ !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
+ops = &reqops_unit_attention;
+} else if (lun != d->lun ||
+   buf[0] == REPORT_LUNS ||
+   (buf[0] == REQUEST_SENSE && d->sense_len)) {
+ops = &reqops_target_command;
+} else {
+ops = NULL;
+}
 
-if (scsi_req_parse(&cmd, d, buf) != 0) {
+ret = scsi_req_parse(&cmd, d, buf);
+if (ret != 0) {
 trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
 req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
 } else {
+assert(cmd.len != 0);
 trace_scsi_req_parsed(d->id, lun, tag, buf[0],
   cmd.mode, cmd.xfer);
 if (cmd.lba != -1) {
@@ -577,25 +602,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 
 if (cmd.xfer > INT32_MAX) {
 req = scsi_req_alloc(&reqops_invalid_field, d, tag, lun, 
hba_private);
-} else if ((d->unit_attention.key == UNIT_ATTENTION ||
-   bus->unit_attention.key == UNIT_ATTENTION) &&
-  (buf[0] != INQUIRY &&
-   buf[0] != REPORT_LUNS &&
-   buf[0] != GET_CONFIGURATION &&
-   buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
-
-   /*
-* If we already have a pending unit attention condition,
-* report this one before triggering another one.
-*/
-   !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
-req = scsi_req_alloc(&reqops_unit_attention, d, tag, lun,
- hba_private);
-} else if (lun != d->lun ||
-   buf[0] == REPORT_LUNS ||
-   (buf[0] == REQUEST_SENSE && d->sense_len)) {
-req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
- hba_private);
+} else if (ops) {
+req = scsi_req_alloc(ops, d, tag, lun, hba_private);
 } else {
 req = scsi_device_alloc_req(d, tag, lun, buf, hba_private);
 }
@@ -1186,6 +1194,7 @@ static int scsi_req_parse(SCSICommand *cmd, SCSIDevice 
*dev, uint8_t *buf)
 {
 int rc;
 
+cmd->lba = -1;
 switch (buf[0] >> 5) {
 case 0:
 cmd->len = 6;
-- 
1.8.3.1





[Qemu-devel] [PULL 06/20] qom: object: delete properties before calling instance_finalize

2014-08-19 Thread Paolo Bonzini
This ensures that the children's unparent callback will still
have a usable parent.

Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 0e8267b..f301bc2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -418,8 +418,8 @@ static void object_finalize(void *data)
 Object *obj = data;
 TypeImpl *ti = obj->class->type;
 
-object_deinit(obj, ti);
 object_property_del_all(obj);
+object_deinit(obj, ti);
 
 g_assert(obj->ref == 0);
 if (obj->free) {
-- 
1.8.3.1





[Qemu-devel] [PULL 04/20] scsi-block, scsi-generic: implement parse_cdb

2014-08-19 Thread Paolo Bonzini
The callback lets the bus provide the direction and transfer count
for passthrough commands, enabling passthrough of vendor-specific
commands.

Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c |  3 +--
 hw/scsi/scsi-disk.c| 14 ++
 hw/scsi/scsi-generic.c |  7 +++
 include/hw/scsi/scsi.h |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index d97d18a..6f4462b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,6 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -1210,7 +1209,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 return lba;
 }
 
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
 int rc;
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 81b7276..d55521d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2564,6 +2564,19 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice 
*d, uint32_t tag,
   hba_private);
 }
 }
+
+static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
+  uint8_t *buf, void *hba_private)
+{
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+if (scsi_block_is_passthrough(s, buf)) {
+return scsi_bus_parse_cdb(&s->qdev, cmd, buf, hba_private);
+} else {
+return scsi_req_parse_cdb(&s->qdev, cmd, buf);
+}
+}
+
 #endif
 
 #define DEFINE_SCSI_DISK_PROPERTIES()\
@@ -2672,6 +2685,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 sc->init = scsi_block_initfn;
 sc->destroy  = scsi_destroy;
 sc->alloc_req= scsi_block_new_request;
+sc->parse_cdb= scsi_block_parse_cdb;
 dc->fw_name = "disk";
 dc->desc = "SCSI block device passthrough";
 dc->reset = scsi_disk_reset;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 3733d2c..0b2ff90 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -490,6 +490,12 @@ static Property scsi_generic_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static int scsi_generic_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+  uint8_t *buf, void *hba_private)
+{
+return scsi_bus_parse_cdb(dev, cmd, buf, hba_private);
+}
+
 static void scsi_generic_class_initfn(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -498,6 +504,7 @@ static void scsi_generic_class_initfn(ObjectClass *klass, 
void *data)
 sc->init = scsi_generic_initfn;
 sc->destroy  = scsi_destroy;
 sc->alloc_req= scsi_new_request;
+sc->parse_cdb= scsi_generic_parse_cdb;
 dc->fw_name = "disk";
 dc->desc = "pass through generic scsi device (/dev/sg*)";
 dc->reset = scsi_generic_reset;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 4a0b860..a7a28e6 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -250,6 +250,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.8.3.1





[Qemu-devel] [PULL 13/20] memory: remove memory_region_destroy

2014-08-19 Thread Paolo Bonzini
The function is empty after the previous patch, so remove it.

Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 backends/hostmem.c | 10 --
 hw/audio/ac97.c|  9 -
 hw/audio/es1370.c  |  8 
 hw/audio/intel-hda.c   |  1 -
 hw/block/nvme.c|  1 -
 hw/block/pflash_cfi01.c|  1 -
 hw/block/pflash_cfi02.c|  1 -
 hw/char/serial-pci.c   |  3 ---
 hw/i386/kvm/pci-assign.c   |  8 
 hw/ide/ahci.c  |  2 --
 hw/ide/cmd646.c|  5 -
 hw/ide/piix.c  |  3 ---
 hw/ide/via.c   |  3 ---
 hw/ipack/tpci200.c | 13 -
 hw/misc/ivshmem.c  |  3 ---
 hw/misc/pci-testdev.c  |  2 --
 hw/misc/vfio.c |  7 ---
 hw/net/e1000.c |  2 --
 hw/net/eepro100.c  |  3 ---
 hw/net/ne2000.c|  1 -
 hw/net/pcnet-pci.c |  2 --
 hw/net/rtl8139.c   |  2 --
 hw/net/stellaris_enet.c|  8 
 hw/net/vmxnet3.c   |  4 
 hw/pci-bridge/pci_bridge_dev.c |  2 --
 hw/pci/msix.c  |  4 
 hw/pci/pci.c   |  2 --
 hw/pci/pci_bridge.c|  8 
 hw/pci/pcie_host.c |  1 -
 hw/pci/shpc.c  |  1 -
 hw/scsi/esp-pci.c  |  1 -
 hw/scsi/lsi53c895a.c   | 10 --
 hw/scsi/megasas.c  |  3 ---
 hw/scsi/vmw_pvscsi.c   |  3 ---
 hw/tpm/tpm_tis.c   |  1 -
 hw/usb/hcd-uhci.c  |  8 
 hw/virtio/virtio-pci.c |  3 ---
 hw/watchdog/wdt_i6300esb.c |  8 
 hw/xen/xen_pt.c| 20 
 hw/xen/xen_pt_msi.c|  2 --
 include/exec/memory.h  |  9 -
 memory.c   |  5 -
 42 files changed, 193 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index ca10c51..e7eec37 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -257,15 +257,6 @@ static void host_memory_backend_init(Object *obj)
 host_memory_backend_set_policy, NULL, NULL, NULL);
 }
 
-static void host_memory_backend_finalize(Object *obj)
-{
-HostMemoryBackend *backend = MEMORY_BACKEND(obj);
-
-if (memory_region_size(&backend->mr)) {
-memory_region_destroy(&backend->mr);
-}
-}
-
 MemoryRegion *
 host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
 {
@@ -360,7 +351,6 @@ static const TypeInfo host_memory_backend_info = {
 .class_init = host_memory_backend_class_init,
 .instance_size = sizeof(HostMemoryBackend),
 .instance_init = host_memory_backend_init,
-.instance_finalize = host_memory_backend_finalize,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_USER_CREATABLE },
 { }
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 45cb118..0e22bb9 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1388,14 +1388,6 @@ static int ac97_initfn (PCIDevice *dev)
 return 0;
 }
 
-static void ac97_exitfn (PCIDevice *dev)
-{
-AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
-
-memory_region_destroy (&s->io_nam);
-memory_region_destroy (&s->io_nabm);
-}
-
 static int ac97_init (PCIBus *bus)
 {
 pci_create_simple (bus, -1, "AC97");
@@ -1413,7 +1405,6 @@ static void ac97_class_init (ObjectClass *klass, void 
*data)
 PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
 k->init = ac97_initfn;
-k->exit = ac97_exitfn;
 k->vendor_id = PCI_VENDOR_ID_INTEL;
 k->device_id = PCI_DEVICE_ID_INTEL_82801AA_5;
 k->revision = 0x01;
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 5dbf803..e67d1ea 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -1042,13 +1042,6 @@ static int es1370_initfn (PCIDevice *dev)
 return 0;
 }
 
-static void es1370_exitfn (PCIDevice *dev)
-{
-ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
-
-memory_region_destroy (&s->io);
-}
-
 static int es1370_init (PCIBus *bus)
 {
 pci_create_simple (bus, -1, "ES1370");
@@ -1061,7 +1054,6 @@ static void es1370_class_init (ObjectClass *klass, void 
*data)
 PCIDeviceClass *k = PCI_DEVICE_CLASS (klass);
 
 k->init = es1370_initfn;
-k->exit = es1370_exitfn;
 k->vendor_id = PCI_VENDOR_ID_ENSONIQ;
 k->device_id = PCI_DEVICE_ID_ENSONIQ_ES1370;
 k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index aa49b47..aba45fc 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1155,7 +1155,6 @@ static void intel_hda_exit(PCIDevice *pci)
 IntelHDAState *d = INTEL_HDA(pci);
 
 msi_uninit(&d->pci);
-memory_region_destroy(&d->mmio);
 }
 
 static int intel_hda_post_load(void *opaque, int version)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fd8f89..6d9a065 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -839,7 +83

[Qemu-devel] [PULL 07/20] qom: object: move unparenting to the child property's release callback

2014-08-19 Thread Paolo Bonzini
This ensures that the unparent callback is called automatically
when the parent object is finalized.

Note that there's no need to keep a reference neither in
object_unparent nor in object_finalize_child_property.  The
reference held by the child property itself will do.

Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 qom/object.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index f301bc2..1b00831 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -387,19 +387,9 @@ static void object_property_del_child(Object *obj, Object 
*child, Error **errp)
 
 void object_unparent(Object *obj)
 {
-if (!obj->parent) {
-return;
-}
-
-object_ref(obj);
-if (obj->class->unparent) {
-(obj->class->unparent)(obj);
-}
 if (obj->parent) {
 object_property_del_child(obj->parent, obj, NULL);
-obj->parent = NULL;
 }
-object_unref(obj);
 }
 
 static void object_deinit(Object *obj, TypeImpl *type)
@@ -1042,6 +1032,10 @@ static void object_finalize_child_property(Object *obj, 
const char *name,
 {
 Object *child = opaque;
 
+if (child->class->unparent) {
+(child->class->unparent)(child);
+}
+child->parent = NULL;
 object_unref(child);
 }
 
-- 
1.8.3.1





[Qemu-devel] [PULL 11/20] ioport: split deletion and destruction

2014-08-19 Thread Paolo Bonzini
Of the two functions portio_list_del and portio_list_destroy,
the latter is just freeing a memory area.  However, portio_list_del
is the logical equivalent of memory_region_del_subregion so
destruction of memory regions does not belong there.

Actually, neither of these APIs are in use; portio is mostly used by
ISA devices or VGAs, and neither of these is currently hot-unpluggable.

Signed-off-by: Paolo Bonzini 
---
 ioport.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ioport.c b/ioport.c
index 3d91e79..dce94a3 100644
--- a/ioport.c
+++ b/ioport.c
@@ -149,6 +149,14 @@ void portio_list_set_flush_coalesced(PortioList *piolist)
 
 void portio_list_destroy(PortioList *piolist)
 {
+MemoryRegionPortioList *mrpio;
+unsigned i;
+
+for (i = 0; i < piolist->nr; ++i) {
+mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
+memory_region_destroy(&mrpio->mr);
+g_free(mrpio);
+}
 g_free(piolist->regions);
 }
 
@@ -291,8 +299,5 @@ void portio_list_del(PortioList *piolist)
 for (i = 0; i < piolist->nr; ++i) {
 mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
 memory_region_del_subregion(piolist->address_space, &mrpio->mr);
-memory_region_destroy(&mrpio->mr);
-g_free(mrpio);
-piolist->regions[i] = NULL;
 }
 }
-- 
1.8.3.1





[Qemu-devel] [PULL 10/20] nic: do not destroy memory regions in cleanup functions

2014-08-19 Thread Paolo Bonzini
The memory regions should be destroyed in the unrealize function;
since these NICs are not even qdev-ified, they cannot be unplugged
and they do not have to do anything to destroy their memory regions.

Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/net/dp8393x.c | 3 ---
 hw/net/mcf_fec.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 789d385..7eab7ad 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -863,9 +863,6 @@ static void nic_cleanup(NetClientState *nc)
 {
 dp8393xState *s = qemu_get_nic_opaque(nc);
 
-memory_region_del_subregion(s->address_space, &s->mmio);
-memory_region_destroy(&s->mmio);
-
 timer_del(s->watchdog);
 timer_free(s->watchdog);
 
diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index 4bff3de..22cd7cf 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -443,9 +443,6 @@ static void mcf_fec_cleanup(NetClientState *nc)
 {
 mcf_fec_state *s = qemu_get_nic_opaque(nc);
 
-memory_region_del_subregion(s->sysmem, &s->iomem);
-memory_region_destroy(&s->iomem);
-
 g_free(s);
 }
 
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code

2014-08-19 Thread Peter Maydell
On 19 August 2014 11:25, Peter Maydell  wrote:
> On 19 August 2014 10:56, Edgar E. Iglesias  wrote:
>> On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote:
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState 
>>> *env)
>>>  #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT)
>>>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
>>>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
>>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
>>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << 
>>> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
>>> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3
>>> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << 
>>> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
>>
>> Shouldn't these shifts/masks differ?
>
> Oops. Yes, they certainly should.

The fix is just a simple s/3/4/ for the PSTATE_SS_SHIFT
define. Does anybody want a retransmit of the series for
this one-liner?

thanks
-- PMM



[Qemu-devel] [PULL 08/20] sysbus: remove unused function sysbus_del_io

2014-08-19 Thread Paolo Bonzini
Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/sysbus.c| 5 -
 include/hw/sysbus.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f4e760d..414e2a1 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -242,11 +242,6 @@ void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
 memory_region_add_subregion(get_system_io(), addr, mem);
 }
 
-void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem)
-{
-memory_region_del_subregion(get_system_io(), mem);
-}
-
 MemoryRegion *sysbus_address_space(SysBusDevice *dev)
 {
 return get_system_memory();
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index f5aaa05..0bb91a8 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -71,7 +71,6 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr 
addr,
  int priority);
 void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
MemoryRegion *mem);
-void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem);
 MemoryRegion *sysbus_address_space(SysBusDevice *dev);
 
 /* Legacy helper function for creating devices.  */
-- 
1.8.3.1





[Qemu-devel] [PULL 12/20] memory: convert memory_region_destroy to object_unparent

2014-08-19 Thread Paolo Bonzini
Explicitly call object_unparent in the few places where we
will re-create the memory region.  If the memory region is
simply being destroyed as part of device teardown, let QOM
handle it.

Signed-off-by: Paolo Bonzini 
---
 docs/memory.txt   | 15 ++-
 hw/display/vga.c  |  2 +-
 hw/i386/kvmvapic.c|  2 +-
 hw/mips/gt64xxx_pci.c |  2 +-
 hw/misc/omap_gpmc.c   |  2 +-
 hw/misc/vfio.c|  4 ++--
 hw/ppc/ppc4xx_devs.c  |  2 +-
 ioport.c  |  2 +-
 memory.c  |  1 -
 9 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/docs/memory.txt b/docs/memory.txt
index 5bdbdb3..b12f1f0 100644
--- a/docs/memory.txt
+++ b/docs/memory.txt
@@ -74,11 +74,16 @@ Region lifecycle
 
 
 A region is created by one of the constructor functions (memory_region_init*())
-and destroyed by the destructor (memory_region_destroy()).  In between,
-a region can be added to an address space by using 
memory_region_add_subregion()
-and removed using memory_region_del_subregion().  Region attributes may be
-changed at any point; they take effect once the region becomes exposed to the
-guest.
+and attached to an object.  It is then destroyed by object_unparent() or simply
+when the parent object dies.
+
+In between, a region can be added to an address space
+by using memory_region_add_subregion() and removed using
+memory_region_del_subregion().  Destroying the region implicitly
+removes the region from the address space.
+
+Region attributes may be changed at any point; they take effect once
+the region becomes exposed to the guest.
 
 Overlapping regions and priority
 
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 68cfee2..65dab8d 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -176,7 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
 
 if (s->has_chain4_alias) {
 memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias);
-memory_region_destroy(&s->chain4_alias);
+object_unparent(OBJECT(&s->chain4_alias));
 s->has_chain4_alias = false;
 s->plane_updated = 0xf;
 }
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index cb855c7..ee95963 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -584,7 +584,7 @@ static int vapic_map_rom_writable(VAPICROMState *s)
 
 if (s->rom_mapped_writable) {
 memory_region_del_subregion(as, &s->rom);
-memory_region_destroy(&s->rom);
+object_unparent(OBJECT(&s->rom));
 }
 
 /* grab RAM memory region (region @rom_paddr may still be pc.rom) */
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 22f63ce..1f2fe5f 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -297,7 +297,7 @@ static void gt64120_pci_mapping(GT64120State *s)
   if (s->PCI0IO_length)
   {
   memory_region_del_subregion(get_system_memory(), &s->PCI0IO_mem);
-  memory_region_destroy(&s->PCI0IO_mem);
+  object_unparent(OBJECT(&s->PCI0IO_mem));
   }
   /* Map new IO address */
   s->PCI0IO_start = s->regs[GT_PCI0IOLD] << 21;
diff --git a/hw/misc/omap_gpmc.c b/hw/misc/omap_gpmc.c
index cddea24..fbbe2ff 100644
--- a/hw/misc/omap_gpmc.c
+++ b/hw/misc/omap_gpmc.c
@@ -436,7 +436,7 @@ static void omap_gpmc_cs_unmap(struct omap_gpmc_s *s, int 
cs)
 }
 memory_region_del_subregion(get_system_memory(), &f->container);
 memory_region_del_subregion(&f->container, omap_gpmc_cs_memregion(s, cs));
-memory_region_destroy(&f->container);
+object_unparent(OBJECT(&f->container));
 }
 
 void omap_gpmc_reset(struct omap_gpmc_s *s)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 0b9eba0..af9ae1f 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2266,7 +2266,7 @@ static void vfio_vga_quirk_teardown(VFIODevice *vdev)
 while (!QLIST_EMPTY(&vdev->vga.region[i].quirks)) {
 VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga.region[i].quirks);
 memory_region_del_subregion(&vdev->vga.region[i].mem, &quirk->mem);
-memory_region_destroy(&quirk->mem);
+object_unparent(OBJECT(&quirk->mem));
 QLIST_REMOVE(quirk, next);
 g_free(quirk);
 }
@@ -2290,7 +2290,7 @@ static void vfio_bar_quirk_teardown(VFIODevice *vdev, int 
nr)
 while (!QLIST_EMPTY(&bar->quirks)) {
 VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks);
 memory_region_del_subregion(&bar->mem, &quirk->mem);
-memory_region_destroy(&quirk->mem);
+object_unparent(OBJECT(&quirk->mem));
 QLIST_REMOVE(quirk, next);
 g_free(quirk);
 }
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 07f9d00..405bbe7 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -422,7 +422,7 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram,
 &sdram->containers[n]);
 memory_region_del_subregion(&sdram->containers[n],
  

[Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

2014-08-19 Thread Paolo Bonzini
From: Peter Crosthwaite 

Rather than having the name as separate state. This prepares support
for creating a MemoryRegion dynamically (i.e. without
memory_region_init() and friends) and the MemoryRegion still getting
a usable name.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 include/exec/memory.h | 1 -
 memory.c  | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d165b27..10f73d9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -162,7 +162,6 @@ struct MemoryRegion {
 QTAILQ_HEAD(subregions, MemoryRegion) subregions;
 QTAILQ_ENTRY(MemoryRegion) subregions_link;
 QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
-const char *name;
 uint8_t dirty_log_mask;
 unsigned ioeventfd_nb;
 MemoryRegionIoeventfd *ioeventfds;
diff --git a/memory.c b/memory.c
index b6b208f..8da29af 100644
--- a/memory.c
+++ b/memory.c
@@ -915,7 +915,6 @@ void memory_region_init(MemoryRegion *mr,
 if (size == UINT64_MAX) {
 mr->size = int128_2_64();
 }
-mr->name = g_strdup(name);
 
 if (name) {
 object_property_add_child_array(owner, name, OBJECT(mr));
@@ -1260,7 +1259,6 @@ static void memory_region_finalize(Object *obj)
 assert(memory_region_transaction_depth == 0);
 mr->destructor(mr);
 memory_region_clear_coalescing(mr);
-g_free((char *)mr->name);
 g_free(mr->ioeventfds);
 }
 
@@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
 
 const char *memory_region_name(const MemoryRegion *mr)
 {
-return mr->name;
+return object_get_canonical_path_component(OBJECT(mr));
 }
 
 bool memory_region_is_ram(MemoryRegion *mr)
-- 
1.8.3.1





[Qemu-devel] [PULL 20/20] mtree: remove write-only field

2014-08-19 Thread Paolo Bonzini
ml->printed is never set to true.

Signed-off-by: Paolo Bonzini 
---
 memory.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/memory.c b/memory.c
index 8da29af..031ff51 100644
--- a/memory.c
+++ b/memory.c
@@ -1972,7 +1972,6 @@ typedef struct MemoryRegionList MemoryRegionList;
 
 struct MemoryRegionList {
 const MemoryRegion *mr;
-bool printed;
 QTAILQ_ENTRY(MemoryRegionList) queue;
 };
 
@@ -2002,7 +2001,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 
 /* check if the alias is already in the queue */
 QTAILQ_FOREACH(ml, alias_print_queue, queue) {
-if (ml->mr == mr->alias && !ml->printed) {
+if (ml->mr == mr->alias) {
 found = true;
 }
 }
@@ -2010,7 +2009,6 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 if (!found) {
 ml = g_new(MemoryRegionList, 1);
 ml->mr = mr->alias;
-ml->printed = false;
 QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
 }
 mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
@@ -2092,10 +2090,8 @@ void mtree_info(fprintf_function mon_printf, void *f)
 mon_printf(f, "aliases\n");
 /* print aliased regions */
 QTAILQ_FOREACH(ml, &ml_head, queue) {
-if (!ml->printed) {
-mon_printf(f, "%s\n", memory_region_name(ml->mr));
-mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
-}
+mon_printf(f, "%s\n", memory_region_name(ml->mr));
+mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
 }
 
 QTAILQ_FOREACH_SAFE(ml, &ml_head, queue, ml2) {
-- 
1.8.3.1




[Qemu-devel] [PULL 09/20] vga: do not dynamically allocate chain4_alias

2014-08-19 Thread Paolo Bonzini
Instead, add a boolean variable to indicate the presence of the region.
This avoids a repeated malloc/free (later we can also avoid the
add_child/unparent by changing the offset/size of the alias).

Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/display/vga.c | 24 ++--
 hw/display/vga_int.h |  3 ++-
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 4b089a3..68cfee2 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -168,15 +168,18 @@ static uint8_t expand4to8[16];
 
 static void vga_update_memory_access(VGACommonState *s)
 {
-MemoryRegion *region, *old_region = s->chain4_alias;
 hwaddr base, offset, size;
 
 if (s->legacy_address_space == NULL) {
 return;
 }
 
-s->chain4_alias = NULL;
-
+if (s->has_chain4_alias) {
+memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias);
+memory_region_destroy(&s->chain4_alias);
+s->has_chain4_alias = false;
+s->plane_updated = 0xf;
+}
 if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
 VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] & VGA_SR04_CHN_4M) {
 offset = 0;
@@ -201,18 +204,11 @@ static void vga_update_memory_access(VGACommonState *s)
 break;
 }
 base += isa_mem_base;
-region = g_malloc(sizeof(*region));
-memory_region_init_alias(region, memory_region_owner(&s->vram),
+memory_region_init_alias(&s->chain4_alias, 
memory_region_owner(&s->vram),
  "vga.chain4", &s->vram, offset, size);
 memory_region_add_subregion_overlap(s->legacy_address_space, base,
-region, 2);
-s->chain4_alias = region;
-}
-if (old_region) {
-memory_region_del_subregion(s->legacy_address_space, old_region);
-memory_region_destroy(old_region);
-g_free(old_region);
-s->plane_updated = 0xf;
+&s->chain4_alias, 2);
+s->has_chain4_alias = true;
 }
 }
 
@@ -1321,7 +1317,7 @@ static void vga_draw_text(VGACommonState *s, int 
full_update)
 s->font_offsets[1] = offset;
 full_update = 1;
 }
-if (s->plane_updated & (1 << 2) || s->chain4_alias) {
+if (s->plane_updated & (1 << 2) || s->has_chain4_alias) {
 /* if the plane 2 was modified since the last display, it
indicates the font may have been modified */
 s->plane_updated = 0;
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 5320abd..641f8f4 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -94,7 +94,8 @@ typedef struct VGACommonState {
 uint32_t vram_size;
 uint32_t vram_size_mb; /* property */
 uint32_t latch;
-MemoryRegion *chain4_alias;
+bool has_chain4_alias;
+MemoryRegion chain4_alias;
 uint8_t sr_index;
 uint8_t sr[256];
 uint8_t gr_index;
-- 
1.8.3.1





[Qemu-devel] [PULL 14/20] tpm_tis: remove instance_finalize callback

2014-08-19 Thread Paolo Bonzini
It is never used, since ISA device are not hot-unpluggable.

Reviewed-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/tpm/tpm_tis.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d398c16..82747ee 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj)
 &s->mmio);
 }
 
-static void tpm_tis_uninitfn(Object *obj)
-{
-TPMState *s = TPM(obj);
-
-memory_region_del_subregion(get_system_memory(), &s->mmio);
-}
-
 static void tpm_tis_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = {
 .parent = TYPE_ISA_DEVICE,
 .instance_size = sizeof(TPMState),
 .instance_init = tpm_tis_initfn,
-.instance_finalize = tpm_tis_uninitfn,
 .class_init  = tpm_tis_class_init,
 };
 
-- 
1.8.3.1





[Qemu-devel] [PULL 15/20] loader: Abstract away ref to memory region names

2014-08-19 Thread Paolo Bonzini
From: Peter Crosthwaite 

Use the function provided rather than spying on the struct.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..1a53f0f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -955,7 +955,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict)
 if (rom->mr) {
 monitor_printf(mon, "%s"
" size=0x%06zx name=\"%s\"\n",
-   rom->mr->name,
+   memory_region_name(rom->mr),
rom->romsize,
rom->name);
 } else if (!rom->fw_file) {
-- 
1.8.3.1





[Qemu-devel] [PULL 18/20] memory: Use memory_region_name for name access

2014-08-19 Thread Paolo Bonzini
From: Peter Crosthwaite 

Despite being local to memory.c, use the helper function. This prepares
support for fully QOMifiying the name field of MR (which will remove
this state from MR completely).

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 memory.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/memory.c b/memory.c
index 1cde6ff..b6b208f 100644
--- a/memory.c
+++ b/memory.c
@@ -2027,8 +2027,8 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
: '-',
-   mr->name,
-   mr->alias->name,
+   memory_region_name(mr),
+   memory_region_name(mr->alias),
mr->alias_offset,
mr->alias_offset
+ (int128_nz(mr->size) ?
@@ -2046,7 +2046,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
mr->romd_mode ? 'R' : '-',
!mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
: '-',
-   mr->name);
+   memory_region_name(mr));
 }
 
 QTAILQ_INIT(&submr_print_queue);
@@ -2095,7 +2095,7 @@ void mtree_info(fprintf_function mon_printf, void *f)
 /* print aliased regions */
 QTAILQ_FOREACH(ml, &ml_head, queue) {
 if (!ml->printed) {
-mon_printf(f, "%s\n", ml->mr->name);
+mon_printf(f, "%s\n", memory_region_name(ml->mr));
 mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head);
 }
 }
-- 
1.8.3.1





Re: [Qemu-devel] [Qemu-ppc] [PATCH V3] spapr: Fix stale HTAB during live migration

2014-08-19 Thread Alexander Graf


On 19.08.14 08:17, Samuel Mendoza-Jonas wrote:
> If a guest reboots during a running migration, changes to the
> hash page table are not necessarily updated on the destination.
> Opening a new file descriptor to the HTAB forces the migration
> handler to resend the entire table.
> 
> Signed-off-by: Samuel Mendoza-Jonas 
> ---
> Changes in v3: Pointed out by David, htab_save_iterate could
>   potentially try to read before htab_fd is open again.
>   Leave opening the fd to the functions trying to read.
> Changes in v2: Forgot check on kvmppc_get_htab_fd return value
>  hw/ppc/spapr.c | 25 +
>  include/hw/ppc/spapr.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3a6d26d..5b41318 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -997,6 +997,10 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>  /* Kernel handles htab, we don't need to allocate one */
>  spapr->htab_shift = shift;
>  kvmppc_kern_htab = true;
> +
> +/* Check if we are overlapping a migration */
> +if (spapr->htab_fd > 0)
> +spapr->need_reset = true;

A "need_reset" field in the sPAPR machine with semantics of "we
triggered a reset, so if there's a migration in flight please consider
the full HTAB as invalid and thus reopen the fd to fetch all entries
again" is not exactly obvious to me.

How about something like "htab_save_should_reopen_fd" or an even better
name you might come up with :). Alternatively you could save the last
time stamp of when the HTAB was allocated and compare to that.


Alex



[Qemu-devel] [PULL 16/20] exec: Abstract away ref to memory region names

2014-08-19 Thread Paolo Bonzini
From: Peter Crosthwaite 

Use the function provided rather than spying on the struct.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 5a2a25e..42688b6 100644
--- a/exec.c
+++ b/exec.c
@@ -1044,7 +1044,7 @@ static void *file_ram_alloc(RAMBlock *block,
 }
 
 /* Make name safe to use with mkstemp by replacing '/' with '_'. */
-sanitized_name = g_strdup(block->mr->name);
+sanitized_name = g_strdup(memory_region_name(block->mr));
 for (c = sanitized_name; *c != '\0'; c++) {
 if (*c == '/')
 *c = '_';
@@ -1242,7 +1242,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
 new_block->host = phys_mem_alloc(new_block->length);
 if (!new_block->host) {
 fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
-new_block->mr->name, strerror(errno));
+memory_region_name(new_block->mr), strerror(errno));
 exit(1);
 }
 memory_try_enable_merging(new_block->host, new_block->length);
-- 
1.8.3.1





[Qemu-devel] [PULL 17/20] memory: constify memory_region_name

2014-08-19 Thread Paolo Bonzini
From: Peter Crosthwaite 

It doesn't change the MR and some prospective call sites will have
const MRs at hand.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 include/exec/memory.h | 2 +-
 memory.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5bd10d1..d165b27 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -511,7 +511,7 @@ void memory_region_unregister_iommu_notifier(Notifier *n);
  *
  * @mr: the memory region being queried
  */
-const char *memory_region_name(MemoryRegion *mr);
+const char *memory_region_name(const MemoryRegion *mr);
 
 /**
  * memory_region_is_logging: return whether a memory region is logging writes
diff --git a/memory.c b/memory.c
index f11c035..1cde6ff 100644
--- a/memory.c
+++ b/memory.c
@@ -1308,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
 return int128_get64(mr->size);
 }
 
-const char *memory_region_name(MemoryRegion *mr)
+const char *memory_region_name(const MemoryRegion *mr)
 {
 return mr->name;
 }
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

2014-08-19 Thread Markus Armbruster
"M.Kustova"  writes:

> On Tue, Aug 19, 2014 at 1:44 PM, Fam Zheng  wrote:
>> On Tue, 08/19 02:00, Maria Kustova wrote:
[...]
>>> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
>>> index fd97c40..b142577 100755
>>> --- a/tests/image-fuzzer/runner.py
>>> +++ b/tests/image-fuzzer/runner.py
[...]
>>> @@ -279,29 +277,30 @@ class TestEnv(object):
>>> "Backing file: %s\n" \
>>> % (self.seed, " ".join(current_cmd),
>>>self.current_dir, backing_file_name)
>>> -
>>>  temp_log = StringIO.StringIO()
>>>  try:
>>>  retcode = run_app(temp_log, current_cmd)
>>>  except OSError, e:
>>> -multilog(test_summary + "Error: Start of '%s' failed. " \
>>> - "Reason: %s\n\n" % (os.path.basename(
>>> - current_cmd[0]), e[1]),
>>> +multilog(test_summary +
>>> + ("Error: Start of '%s' failed. Reason: %s\n\n"
>>> +  % (os.path.basename(current_cmd[0]), e[1])),
>>
>> I prefer the old one. I don't like the special case in python syntax: '(val)'
>> is just val, but '(val1, val2)' is a tuple.
>
> This 'tuple' format provides that '%' substitution will be applied
> only to the string in the parentheses,
> instead of an entire string (in this case including test summary).
> The same rationale for cases below.

What about something like

multilog("%sError: Start of '%s' failed. Reason: %s\n\n"
 % (test_summary, os.path.basename(current_cmd[0]),
e[1]),
 sys.stderr, self.log, self.parent_log)

[...]



Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

2014-08-19 Thread Alexander Graf


On 19.08.14 00:26, Joel Schopp wrote:
> 
> On 08/18/2014 05:11 PM, Peter Maydell wrote:
>> On 18 August 2014 22:54, Joel Schopp  wrote:
>>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque)
>>> +{
>>> +PlatformDevtreeData *data = opaque;
>>> +void *fdt = data->fdt;
>>> +const char *parent_node = data->node;
>>> +int compat_str_len;
>>> +char *nodename;
>>> +int i, ret;
>>> +uint32_t *irq_attr;
>>> +uint64_t *reg_attr;
>>> +uint64_t mmio_base;
>>> +uint64_t irq_number;
>>> +gchar mmio_base_prop[8];
>>> +gchar irq_number_prop[8];
>>> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
>>> +VFIODevice *vbasedev = &vdev->vbasedev;
>>> +Object *obj = OBJECT(sbdev);
>>> +
>>> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>>> +
>>> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
>>> +   vbasedev->name,
>>> +   mmio_base);
>>> +
>>> +qemu_fdt_add_subnode(fdt, nodename);
>>> +
>>> +compat_str_len = strlen(vdev->compat) + 1;
>> At this point you've already substituted the NULs in,
>> so you can't call strlen(), I think.
>>
>>> +qemu_fdt_setprop(fdt, nodename, "compatible",
>>> +vdev->compat, compat_str_len);
>>> +
>>> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>>> +
>>> +for (i = 0; i < vbasedev->num_regions; i++) {
>>> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i);
>>> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL);
>>> +reg_attr[2*i] = 1;
>>> +reg_attr[2*i+1] = mmio_base;
>>> +reg_attr[2*i+2] = 1;
>>> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem);
>>> +}
>>>
>>> This should be 4 instead of 2.
>>> Also, to support 64 bit systems I think this should be 2 instead of 1.
>> Actually it depends entirely on what the board has done to
>> create the device tree node that we're inserting this child
>> node into. For ARM boot.c sets both #address-cells and
>> #size-cells to 2 regardless of whether the system is 32
>> or 64 bits, for simplicity. I imagine PPC does something
>> different. If we're editing a dtb that the user passed in (which
>> I think would be pretty lunatic so we shouldn't do this)
>> we'd have to actually walk the dtb to try to figure out what
>> the semantics of the reg property should be.
> For the index [2*i],[2*i+1], etc is clearly a bug as when i = 1 it will
> overwrite two of the values.  Changing that to [4*i],[4*i+1],etc fixes it.
> 
> I think you are right on the size.  I also wonder if the user doesn't
> pass in a dtb if qemu should try to recreate the device-tree entry from
> the platform device entry in the host kernel?  If so would that best be
> done by recreating the values from /proc/device-tree ?
> 
> I also wish that qemu had a flag to output the generated dtb to a file
> much like lkvm (kvmtool) has.

It does. "qemu-system-foo -machine dumpdtb=mydtb.dtb" should dump the
generated dtb into a file called mydtb.dtb.


Alex



Re: [Qemu-devel] [PATCH v3 0/4] block: Use g_new() & friends more

2014-08-19 Thread Kevin Wolf
Am 19.08.2014 um 10:31 hat Markus Armbruster geschrieben:
> PATCH 1+2 convert some allocations.  While preparing them, I stumbled
> over dead error handling and some useless casts, which led to PATCH
> 3+4.
> 
> I posted a tree-wide version of PATCH 1 some time ago, and was told to
> split it up.  This is the block part, redone from scratch.  Other
> parts available on request.
> 
> v3:
> * Manual style cleanups in PATCH 4 [Max, Jeff]
> * PATCH 1-3 unchanged.
> v2:
> * Regenerated with spatch.
> * Because of that, I decided not to carry Max's R-by forward.
> 
> Markus Armbruster (4):
>   block: Use g_new() & friends where that makes obvious sense
>   block: Use g_new() & friends to avoid multiplying sizes
>   qemu-io-cmds: g_renew() can't fail, bury dead error handling
>   block: Drop some superfluous casts from void *

Thanks, applied all to the block branch.

Kevin



Re: [Qemu-devel] [PULL 0/3] QMP queue

2014-08-19 Thread Peter Maydell
On 18 August 2014 20:26, Luiz Capitulino  wrote:
> Three little birds.
>
> The following changes since commit 08ab59770da57648bfb8fc9be37f0ef7fb50b0f9:
>
>   Merge remote-tracking branch 'remotes/mcayland/qemu-sparc' into staging 
> (2014-08-18 12:55:02 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
>
> for you to fetch changes up to b3dd1b8c295636e64ceb14cdc4db6420d7319e38:
>
>   monitor: fix use after free (2014-08-18 14:39:10 -0400)
>
> 
> Chen Gang (1):
>   dump.c: Fix memory leak issue in cleanup processing for dump_init()
>
> Hani Benhabiles (1):
>   monitor: Remove hardcoded watchdog event names
>
> Michael S. Tsirkin (1):
>   monitor: fix use after free
>
>  dump.c| 18 +-
>  include/monitor/monitor.h |  2 +-
>  monitor.c | 19 ++-
>  stubs/fdset-remove-fd.c   |  3 +--
>  4 files changed, 17 insertions(+), 25 deletions(-)
>

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH V2 0/2] runner: Control test duration

2014-08-19 Thread Kevin Wolf
Am 19.08.2014 um 11:18 hat Stefan Hajnoczi geschrieben:
> On Tue, Aug 19, 2014 at 12:02:33AM +0400, Maria Kustova wrote:
> > The first patch adds the '--duration SECONDS' argument. After the specified
> > duration the runner allows to end the current test and then exits.
> > 
> > The second patch adds forced termination of a program under test, if the 
> > test
> > execution takes more than 10 minutes to indicate program freezes.
> > 
> > If a program under test hangs, then the specified test duration can be 
> > overrun
> > up to 10 minutes.
> > 
> > The patch series is based on https://github.com/stefanha/qemu/commits/block,
> > commit 07a45925fa88376f8583a333e74f7eeb0f455685
> > 
> > v1 -> v2:
> >  * Trivial fixes based on the review of Fam Zheng
> >  * Increased time-out (in some cases 5 minutes interval returned false
> >negatives)
> > 
> > Maria Kustova (2):
> >   runner: Add an argument for test duration
> >   runner: Kill a program under test by time-out
> > 
> >  tests/image-fuzzer/runner.py | 50 
> > +---
> >  1 file changed, 42 insertions(+), 8 deletions(-)
> 
> Reviewed-by: Stefan Hajnoczi 

Thanks, applied all to the block branch.

Kevin


pgp4gYglQGwAP.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL

2014-08-19 Thread Kevin Wolf
Am 18.08.2014 um 13:41 hat Michael Tokarev geschrieben:
> Just log to stderr unconditionally, like other similar code does.
> 
> Signed-off-by: Michael Tokarev 
> ---
>  block/vvfat.c |5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 70176b1..ea37023 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1082,11 +1082,6 @@ static int vvfat_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  vvv = s;
>  #endif
>  
> -DLOG(if (stderr == NULL) {
> -stderr = fopen("vvfat.log", "a");
> -setbuf(stderr, NULL);
> -})
> -
>  opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>  qemu_opts_absorb_qdict(opts, options, &local_err);
>  if (local_err) {

Michael, it's fine to merge trivial block patches through your tree, but
can you please keep Stefan and me CCed anyway?

This specific patch isn't as trivial as it might look at the first
sight (in other words: it's wrong). The part that you probably missed is
that stderr isn't the real one when DEBUG is set:

#undef stderr
#define stderr STDERR
FILE* stderr = NULL;

So now you have a lot of fprintf(NULL, ...), which obviously makes qemu
segfault as soon as you enable debugging for vvfat.

Kevin



Re: [Qemu-devel] [PULL v2 00/23] linux-user updates

2014-08-19 Thread Peter Maydell
On 19 August 2014 09:32,   wrote:
> From: Riku Voipio 
>
> The same as previous series, except the patch "make binfmt flag O require P"
> has been dropped.
>
> The following changes since commit 142f4ac5d5e024670ef4725e8943702b027e4218:
>
>   Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-2014-08-15' 
> into staging (2014-08-15 18:44:48 +0100)
>
> are available in the git repository at:
>
>   git://git.linaro.org/people/riku.voipio/qemu.git linux-user-for-upstream
>
> for you to fetch changes up to 8349027af60e861fcc401c928eb76f9f418815d7:

Hi. I'm afraid this doesn't build on my ARM board:

/root/qemu/linux-user/syscall.c: In function 'do_open_by_handle_at':
/root/qemu/linux-user/syscall.c:5475:16: error: 'p' may be used
uninitialized in this function [-Werror=uninitialized]

The compiler seems to be correct here -- we should
be passing 'fh' to unlock_user(), not 'p' (which seems
to be completely useless and should be deleted).

This function and name_to_handle_at() also both seem
to be missing the necessary guest-to-host byteswaps
on the fields in struct file_handle.

thanks
-- PMM



Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL

2014-08-19 Thread Peter Maydell
On 19 August 2014 12:58, Kevin Wolf  wrote:
> This specific patch isn't as trivial as it might look at the first
> sight (in other words: it's wrong). The part that you probably missed is
> that stderr isn't the real one when DEBUG is set:
>
> #undef stderr
> #define stderr STDERR
> FILE* stderr = NULL;

Yikes. Can we rip that out as well, then?

-- PMM



Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL

2014-08-19 Thread Eric Blake
On 08/19/2014 05:58 AM, Kevin Wolf wrote:
> Am 18.08.2014 um 13:41 hat Michael Tokarev geschrieben:
>> Just log to stderr unconditionally, like other similar code does.
>>

>>  
>> -DLOG(if (stderr == NULL) {
>> -stderr = fopen("vvfat.log", "a");
>> -setbuf(stderr, NULL);
>> -})
>> -

> 
> This specific patch isn't as trivial as it might look at the first
> sight (in other words: it's wrong). The part that you probably missed is
> that stderr isn't the real one when DEBUG is set:
> 
> #undef stderr
> #define stderr STDERR
> FILE* stderr = NULL;

Eeek, that's horrible.  I'd rather see code doing freopen("vvfat.log",
"a", stderr) than the current horrid mess of redefining stderr away from
its normal meaning.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code

2014-08-19 Thread Edgar E. Iglesias
On Tue, Aug 19, 2014 at 11:46:23AM +0100, Peter Maydell wrote:
> On 19 August 2014 11:25, Peter Maydell  wrote:
> > On 19 August 2014 10:56, Edgar E. Iglesias  wrote:
> >> On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote:
> >>> --- a/target-arm/cpu.h
> >>> +++ b/target-arm/cpu.h
> >>> @@ -1211,6 +1211,10 @@ static inline bool 
> >>> arm_singlestep_active(CPUARMState *env)
> >>>  #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT)
> >>>  #define ARM_TBFLAG_AA64_FPEN_SHIFT  2
> >>>  #define ARM_TBFLAG_AA64_FPEN_MASK   (1 << ARM_TBFLAG_AA64_FPEN_SHIFT)
> >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3
> >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << 
> >>> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
> >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3
> >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << 
> >>> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
> >>
> >> Shouldn't these shifts/masks differ?
> >
> > Oops. Yes, they certainly should.
> 
> The fix is just a simple s/3/4/ for the PSTATE_SS_SHIFT
> define. Does anybody want a retransmit of the series for
> this one-liner?

Hi,

AFAICT, the rest of the series looks good, RB on all.

Reviewed-by: Edgar E. Iglesias 

No need to repost for me.

Cheers,
Edgar



  1   2   3   >