Re: [Qemu-devel] [PATCH] ahci: fix device detect emulation.

2011-06-24 Thread Gerd Hoffmann

On 06/23/11 15:59, Gerd Hoffmann wrote:

AHCI specs say about the device detection field:

Device Detection (DET): Indicates the interface device detection and Phy
state.

0h - No device detected and Phy communication not established
1h - Device presence detected but Phy communication not established
3h - Device presence detected and Phy communication established
4h - Phy in offline mode as a result of the interface being disabled or
  running in a BIST loopback mode

The "Software Initilaization" section also mentions "If PxSSTS.DET
returns a value of 1h or 3h when read, then system software shall
continue to the next step, ..."

This makes me think that 1h means "tried to detect device but didn't
found one" and 0h means "device detection not finished yet".


/me was thinking wrong, scratch that, qemu behavior is fine.

cheers,
  Gerd




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Peter Maydell
On 24 June 2011 03:44, Max Filippov  wrote:

> Please note how the current instruction in gdb differ from what
> was said in OUT. This lea corrupts stack pointer and the next
> callq generates segfault.
> Could please anyone familiar with TCG take a look at this, or
> suggest where I should look myself?

You don't say which target you're compiling code for, or what
the input assembly was which triggered this.

My first guess is that the target's front end might have a bug
where it wrongly bakes in assumptions about bits of the CPUState.
QEMU will occasionally retranslate-in-place a TB (if a load in
the TB causes an exception) so if the frontend generates different
code the second time around things will go wrong...

You should be able to find out what's stomping on the code
with the aid of a debugger and some watchpoints.

-- PMM



Re: [Qemu-devel] Kemari status?

2011-06-24 Thread OHMURA Kei
2011/6/24 Christian Brunner :
> Does anyone know what happened to kemari?
>
> Back in March it was on the list for a possible merge in qemu 0.15. In
> April the last update was sent to this list. After that everything
> remained silent.
>
> I think it's a really interesting project and I wonder why it isn't picked up.
>
> Thanks,
> Christian
>
>

Hi, I'm Kei, developing Kemari with Yoshi.

We add patches to follow git tree.
git://kemari.git.sourceforge.net/gitroot/kemari/kemari next

> I think it's a really interesting project and I wonder why it isn't picked up.

Thanks for your cheering!
We need acked-by to the whole series to get it merged.
Although some people have kindly volunteered for reviewing, we haven't
received for all patches yet.

We would be glad if more people can take a look, and hopefully get it
improved and merged into the mainline soon :)


Thanks,
Kei



Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Laurent Desnogues
On Fri, Jun 24, 2011 at 4:44 AM, Max Filippov  wrote:
> Hello guys.
>
> I'm running qemu on x86_64 host.
> It's clean build from git sources dated 2011.05.19, commit 
> 1fddfba129f5435c80eda14e8bc23fdb888c7187
> I have the following output from "log trace,op,out_asm":
>
> Trace 0x4000a310 [d0026c92]
> OP:
>   0xd0c0
>  movi_i32 tmp1,$0xfff4
>  add_i32 tmp0,ar9,tmp1
>  qemu_ld32 ar1,tmp0,$0x0
>
>   0xd0c3
>  movi_i32 tmp1,$0xfff0
>  add_i32 tmp0,ar9,tmp1
>  qemu_ld32 ar0,tmp0,$0x0
>
> [...snip...]
[...]
> 0x4000a360:  xor%esi,%esi
> 0x4000a362:  callq  0x52edc2
[...]
> (gdb) x/25i 0x4000a330
[...]
>   0x4000a360:  mov    $0x1,%esi
>   0x4000a365:  callq  0x52edc2 <__ldl_mmu>
>   0x4000a36a:  mov    %eax,%ebp
>   0x4000a36c:  sub    $0x44,%al
> => 0x4000a36e:  lea    -0x10(%rbx),%esp
>   0x4000a371:  mov    %ebp,0xc(%r14)
>   0x4000a375:  mov    %r12d,%esi
>   0x4000a378:  mov    %r12d,%edi
>
> Please note how the current instruction in gdb differ from what was said in 
> OUT. This lea corrupts stack pointer and the next callq generates segfault.
> Could please anyone familiar with TCG take a look at this, or suggest where I 
> should look myself?

As Peter hinted, you're not looking at the code you think :-)
Note how your original TCG code does loads:

   qemu_ld32 ar1,tmp0,$0x0

That $0x0 will end up in %RSI.  It's the mem index used to
distinguish from user and privileged level accesses.  In your
examples of host code, in one case it is 0 and in the other
it is 1, so you're definitely not really looking at the same
block in the same running conditions.

HTH,

Laurent



[Qemu-devel] [PATCH v2 00/12] Adding VMDK monolithic flat support

2011-06-24 Thread famcool
From: Fam Zheng 

VMDK multiple file images can not be recognized for now. This patch series is
adding monolithic flat support to it, that is the image type with two
files, one text descriptor file and a plain data file. This type of
image can be created in VMWare, with the options "allocate all disk
space now" and "store virtual disk as a single file" checked.

A VmdkExtent structure is introduced to hold the image "extent"
information, which makes further adding multi extents support of VMDK
easy. An image creating option "flat" is added for creating flat
(preallocated) image.

Fam Zheng (12):
  VMDK: introduce VmdkExtent
  VMDK: bugfix, align offset to cluster in get_whole_cluster
  VMDK: probe for monolithicFlat images
  VMDK: separate vmdk_open by format version
  VMDK: add field BDRVVmdkState.desc_offset
  VMDK: flush multiple extents
  VMDK: move 'static' cid_update flag to bs field
  VMDK: change get_cluster_offset return type
  VMDK: open/read/write for monolithicFlat image
  VMDK: create different subformats
  VMDK: fix coding style
  BlockDriver: add bdrv_get_allocated_file_size() operation

 block.c   |   19 +
 block.h   |1 +
 block/raw-posix.c |   21 +
 block/raw-win32.c |   29 ++
 block/vmdk.c  | 1360 +
 block_int.h   |2 +
 qemu-img.c|   31 +--
 7 files changed, 1026 insertions(+), 437 deletions(-)




[Qemu-devel] [PATCH v2 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster

2011-06-24 Thread famcool
From: Fam Zheng 

In get_whole_cluster, the offset is not aligned to cluster when reading
from backing_hd. When the first write to child is not at the cluster
boundary, wrong address data from parent is copied to child.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index c7246f0..0540ec5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -485,21 +485,23 @@ static int get_whole_cluster(BlockDriverState *bs,
 /* 128 sectors * 512 bytes each = grain size 64KB */
 uint8_t  whole_grain[extent->cluster_sectors * 512];
 
-// we will be here if it's first write on non-exist grain(cluster).
-// try to read from parent image, if exist
+/* we will be here if it's first write on non-exist grain(cluster).
+ * try to read from parent image, if exist */
 if (bs->backing_hd) {
 int ret;
 
 if (!vmdk_is_cid_valid(bs))
 return -1;
 
+/* floor offset to cluster */
+offset -= offset % (extent->cluster_sectors * 512);
 ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
 extent->cluster_sectors);
 if (ret < 0) {
 return -1;
 }
 
-//Write grain only into the active image
+/* Write grain only into the active image */
 ret = bdrv_write(extent->file, cluster_offset, whole_grain,
 extent->cluster_sectors);
 if (ret < 0) {



[Qemu-devel] [PATCH v2 08/12] VMDK: change get_cluster_offset return type

2011-06-24 Thread famcool
From: Fam Zheng 

The return type of get_cluster_offset was an offset that use 0 to denote
'not allocated', this will be no longer true for flat extents, as we see
flat extent file as a single huge cluster whose offset is 0 and length
is the whole file length.
So now we use int return value, 0 means success and otherwise offset
invalid.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   73 +++---
 1 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 070a06d..9a4df98 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -646,18 +646,23 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
*m_data)
 return 0;
 }
 
-static uint64_t get_cluster_offset(BlockDriverState *bs,
+static int get_cluster_offset(BlockDriverState *bs,
 VmdkExtent *extent,
 VmdkMetaData *m_data,
-uint64_t offset, int allocate)
+uint64_t offset,
+int allocate,
+uint64_t *cluster_offset)
 {
 unsigned int l1_index, l2_offset, l2_index;
 int min_index, i, j;
 uint32_t min_count, *l2_table, tmp = 0;
-uint64_t cluster_offset;
 
 if (m_data)
 m_data->valid = 0;
+if (extent->flat) {
+*cluster_offset = 0;
+return 0;
+}
 
 l1_index = (offset >> 9) / extent->l1_entry_sectors;
 if (l1_index >= extent->l1_size) {
@@ -702,21 +707,22 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 extent->l2_cache_counts[min_index] = 1;
  found:
 l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
-cluster_offset = le32_to_cpu(l2_table[l2_index]);
+*cluster_offset = le32_to_cpu(l2_table[l2_index]);
 
-if (!cluster_offset) {
-if (!allocate)
-return 0;
+if (!*cluster_offset) {
+if (!allocate) {
+return -1;
+}
 
 // Avoid the L2 tables update for the images that have snapshots.
-cluster_offset = bdrv_getlength(extent->file);
+*cluster_offset = bdrv_getlength(extent->file);
 bdrv_truncate(
 extent->file,
-cluster_offset + (extent->cluster_sectors << 9)
+*cluster_offset + (extent->cluster_sectors << 9)
 );
 
-cluster_offset >>= 9;
-tmp = cpu_to_le32(cluster_offset);
+*cluster_offset >>= 9;
+tmp = cpu_to_le32(*cluster_offset);
 l2_table[l2_index] = tmp;
 
 /* First of all we write grain itself, to avoid race condition
@@ -725,8 +731,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
  * or inappropriate VM shutdown.
  */
 if (get_whole_cluster(
-bs, extent, cluster_offset, offset, allocate) == -1)
-return 0;
+bs, extent, *cluster_offset, offset, allocate) == -1)
+return -1;
 
 if (m_data) {
 m_data->offset = tmp;
@@ -736,8 +742,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 m_data->valid = 1;
 }
 }
-cluster_offset <<= 9;
-return cluster_offset;
+*cluster_offset <<= 9;
+return 0;
 }
 
 static VmdkExtent *find_extent(BDRVVmdkState *s,
@@ -761,7 +767,6 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t 
sector_num,
  int nb_sectors, int *pnum)
 {
 BDRVVmdkState *s = bs->opaque;
-
 int64_t index_in_cluster, n, ret;
 uint64_t offset;
 VmdkExtent *extent;
@@ -770,15 +775,13 @@ static int vmdk_is_allocated(BlockDriverState *bs, 
int64_t sector_num,
 if (!extent) {
 return 0;
 }
-if (extent->flat) {
-n = extent->end_sector - sector_num;
-ret = 1;
-} else {
-offset = get_cluster_offset(bs, extent, NULL, sector_num * 512, 0);
-index_in_cluster = sector_num % extent->cluster_sectors;
-n = extent->cluster_sectors - index_in_cluster;
-ret = offset ? 1 : 0;
-}
+ret = get_cluster_offset(bs, extent, NULL,
+sector_num * 512, 0, &offset);
+/* get_cluster_offset returning 0 means success */
+ret = !ret;
+
+index_in_cluster = sector_num % extent->cluster_sectors;
+n = extent->cluster_sectors - index_in_cluster;
 if (n > nb_sectors)
 n = nb_sectors;
 *pnum = n;
@@ -799,14 +802,15 @@ static int vmdk_read(BlockDriverState *bs, int64_t 
sector_num,
 if (!extent) {
 return -EIO;
 }
-cluster_offset = get_cluster_offset(
-bs, extent, NULL, sector_num << 9, 0);
+ret = get_cluster_offset(
+bs, extent, NULL,
+sector_num << 9, 0, &cluster_offset);
 index_in_cluster = sector_num % extent->cluster_sectors;
 n = exte

[Qemu-devel] [PATCH v2 01/12] VMDK: introduce VmdkExtent

2011-06-24 Thread famcool
From: Fam Zheng 

Introduced VmdkExtent array into BDRVVmdkState, enable holding multiple
image extents for multiple file image support.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  321 --
 1 files changed, 222 insertions(+), 99 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 922b23d..c7246f0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -60,7 +60,11 @@ typedef struct {
 
 #define L2_CACHE_SIZE 16
 
-typedef struct BDRVVmdkState {
+typedef struct VmdkExtent {
+BlockDriverState *file;
+bool flat;
+int64_t sectors;
+int64_t end_sector;
 int64_t l1_table_offset;
 int64_t l1_backup_table_offset;
 uint32_t *l1_table;
@@ -74,7 +78,12 @@ typedef struct BDRVVmdkState {
 uint32_t l2_cache_counts[L2_CACHE_SIZE];
 
 unsigned int cluster_sectors;
+} VmdkExtent;
+
+typedef struct BDRVVmdkState {
+int num_extents;
 uint32_t parent_cid;
+VmdkExtent *extents;
 } BDRVVmdkState;
 
 typedef struct VmdkMetaData {
@@ -156,8 +165,8 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t 
cid)
 
 static int vmdk_is_cid_valid(BlockDriverState *bs)
 {
-#ifdef CHECK_CID
 BDRVVmdkState *s = bs->opaque;
+#ifdef CHECK_CID
 BlockDriverState *p_bs = bs->backing_hd;
 uint32_t cur_pcid;
 
@@ -363,6 +372,7 @@ static int vmdk_open(BlockDriverState *bs, int flags)
 BDRVVmdkState *s = bs->opaque;
 uint32_t magic;
 int l1_size, i;
+VmdkExtent *extent = NULL;
 
 if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
 goto fail;
@@ -370,31 +380,45 @@ static int vmdk_open(BlockDriverState *bs, int flags)
 magic = be32_to_cpu(magic);
 if (magic == VMDK3_MAGIC) {
 VMDK3Header header;
-
-if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != 
sizeof(header))
+s->extents = qemu_mallocz(sizeof(VmdkExtent));
+s->num_extents = 1;
+if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+!= sizeof(header)) {
 goto fail;
-s->cluster_sectors = le32_to_cpu(header.granularity);
-s->l2_size = 1 << 9;
-s->l1_size = 1 << 6;
-bs->total_sectors = le32_to_cpu(header.disk_sectors);
-s->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
-s->l1_backup_table_offset = 0;
-s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
+}
+extent = s->extents;
+extent->flat = false;
+extent->file = bs->file;
+extent->cluster_sectors = le32_to_cpu(header.granularity);
+extent->l2_size = 1 << 9;
+extent->l1_size = 1 << 6;
+extent->sectors = le32_to_cpu(header.disk_sectors);
+extent->end_sector = le32_to_cpu(header.disk_sectors);
+extent->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
+extent->l1_backup_table_offset = 0;
+extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
 } else if (magic == VMDK4_MAGIC) {
 VMDK4Header header;
-
-if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != 
sizeof(header))
+s->extents = qemu_mallocz(sizeof(VmdkExtent));
+s->num_extents = 1;
+if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+!= sizeof(header)) {
 goto fail;
-bs->total_sectors = le64_to_cpu(header.capacity);
-s->cluster_sectors = le64_to_cpu(header.granularity);
-s->l2_size = le32_to_cpu(header.num_gtes_per_gte);
-s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
-if (s->l1_entry_sectors <= 0)
+}
+extent = s->extents;
+extent->file = bs->file;
+extent->sectors = le64_to_cpu(header.capacity);
+extent->end_sector = le64_to_cpu(header.capacity);
+extent->cluster_sectors = le64_to_cpu(header.granularity);
+extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
+extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
+if (extent->l1_entry_sectors <= 0) {
 goto fail;
-s->l1_size = (bs->total_sectors + s->l1_entry_sectors - 1)
-/ s->l1_entry_sectors;
-s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
-s->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
+}
+extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
+/ extent->l1_entry_sectors;
+extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
+extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
 
 // try to open parent images, if exist
 if (vmdk_parent_open(bs) != 0)
@@ -405,41 +429,61 @@ static int vmdk_open(BlockDriverState *bs, int flags)
 goto fail;
 }
 
+/* sum up the total sectors */
+bs->total_sectors = 0;
+for (i = 0; i < s->num_extents; i++) {
+bs->total_sec

[Qemu-devel] [PATCH v2 07/12] VMDK: move 'static' cid_update flag to bs field

2011-06-24 Thread famcool
From: Fam Zheng 

Cid_update is the flag for updating CID on first write after opening the
image. This should be per image open rather than per program life cycle,
so change it from static var of vmdk_write to a field in BDRVVmdkState.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 41a18a1..070a06d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -82,6 +82,7 @@ typedef struct VmdkExtent {
 
 typedef struct BDRVVmdkState {
 int desc_offset;
+bool cid_updated;
 int num_extents;
 uint32_t parent_cid;
 VmdkExtent *extents;
@@ -834,7 +835,6 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 int n;
 int64_t index_in_cluster;
 uint64_t cluster_offset;
-static int cid_update = 0;
 VmdkMetaData m_data;
 
 if (sector_num > bs->total_sectors) {
@@ -881,9 +881,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 buf += n * 512;
 
 // update CID on the first write every time the virtual disk is opened
-if (!cid_update) {
+if (!s->cid_updated) {
 vmdk_write_cid(bs, time(NULL));
-cid_update++;
+s->cid_updated = true;
 }
 }
 return 0;



[Qemu-devel] [PATCH v2 09/12] VMDK: open/read/write for monolithicFlat image

2011-06-24 Thread famcool
From: Fam Zheng 

Parse vmdk decriptor file and open mono flat image.
Read/write the flat extent.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  190 ++
 1 files changed, 177 insertions(+), 13 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 9a4df98..1783f6b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -65,6 +65,7 @@ typedef struct VmdkExtent {
 bool flat;
 int64_t sectors;
 int64_t end_sector;
+int64_t flat_start_offset;
 int64_t l1_table_offset;
 int64_t l1_backup_table_offset;
 uint32_t *l1_table;
@@ -381,9 +382,10 @@ fail:
 static int vmdk_parent_open(BlockDriverState *bs)
 {
 char *p_name;
-char desc[DESC_SIZE];
+char desc[DESC_SIZE + 1];
 BDRVVmdkState *s = bs->opaque;
 
+desc[DESC_SIZE] = '\0';
 if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
 return -1;
 }
@@ -565,6 +567,162 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int 
flags)
 return ret;
 }
 
+/* find an option value out of descriptor file */
+static int vmdk_parse_description(const char *desc, const char *opt_name,
+char *buf, int buf_size)
+{
+char *opt_pos = strstr(desc, opt_name);
+int r;
+const char *end = desc + strlen(desc);
+
+if (!opt_pos) {
+return -1;
+}
+opt_pos += strlen(opt_name) + 2;
+if (opt_pos >= end) {
+return -1;
+}
+r = sscanf(opt_pos, "%[^\"]s", buf);
+return r <= 0;
+}
+
+static int vmdk_parse_extents(const char *desc, VmdkExtent extents[],
+const char *desc_file_path)
+{
+int ret = 0;
+int r;
+char access[11];
+char type[11];
+char fname[512];
+VmdkExtent *extent;
+const char *p = desc;
+int64_t sectors = 0;
+int64_t last_end_sector = 0;
+int64_t flat_offset;
+
+while (*p) {
+if (strncmp(p, "RW", strlen("RW"))) {
+goto next_line;
+}
+/* parse extent line:
+ * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
+ * or
+ * RW [size in sectors] SPARSE "file-name.vmdk"
+ */
+flat_offset = -1;
+sscanf(p, "%10s %lld %10s %512s",
+access, §ors, type, fname);
+if (!strcmp(type, "FLAT")) {
+sscanf(p, "%10s %lld %10s %512s %lld",
+access, §ors, type, fname, &flat_offset);
+if (flat_offset == -1) {
+return 0;
+}
+}
+
+/* trim the quotation marks around */
+if (fname[0] == '"') {
+memmove(fname, fname + 1, strlen(fname) + 1);
+if (fname[strlen(fname) - 1] == '"') {
+fname[strlen(fname) - 1] = '\0';
+}
+}
+if (!(strlen(access) && sectors && strlen(type) && strlen(fname))) {
+goto next_line;
+}
+if (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) {
+goto next_line;
+}
+if (strcmp(access, "RW")) {
+goto next_line;
+}
+ret++;
+if (!extents) {
+goto next_line;
+}
+
+/* save to extents array */
+if (!strcmp(type, "FLAT")) {
+/* FLAT extent */
+char extent_path[PATH_MAX];
+path_combine(extent_path, sizeof(extent_path),
+desc_file_path, fname);
+extent = &extents[ret - 1];
+extent->flat = true;
+extent->flat_start_offset = flat_offset;
+extent->sectors = sectors;
+extent->end_sector = last_end_sector + sectors;
+last_end_sector += sectors;
+extent->cluster_sectors = sectors;
+extent->file = bdrv_new("");
+BlockDriver *drv = bdrv_find_format("file");
+if (!drv) {
+return -EINVAL;
+}
+r = bdrv_open(extent->file, extent_path,
+BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
+if (r) {
+return -EINVAL;
+}
+} else {
+/* SPARSE extent, not supported for now */
+fprintf(stderr,
+"VMDK: Not supported extent type \"%s\""".\n", type);
+return -ENOTSUP;
+}
+next_line:
+/* move to next line */
+while (*p && *p != '\n') {
+p++;
+}
+p++;
+}
+return ret;
+}
+
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
+{
+int ret;
+char buf[2048];
+char ct[128];
+VmdkExtent *extent;
+BDRVVmdkState *s = bs->opaque;
+
+ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+if (ret < 0) {
+return ret;
+}
+if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
+return -EINVAL;
+}
+if (strcmp(ct, "monolithicFlat")) {
+fprintf(stderr,
+"VMDK: Not supported image type

[Qemu-devel] [PATCH v2 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation

2011-06-24 Thread famcool
From: Fam Zheng 

qemu-img.c wants to count allocated file size of image. Previously it
counts a single bs->file by 'stat' or Window API. As VMDK introduces
multiple file support, the operation becomes format specific with
platform specific meanwhile.

The functions are moved to block/raw-{posix,win32}.c and qemu-img.c calls
bdrv_get_allocated_file_size to count the bs. And also added VMDK code
to count his own extents.

Signed-off-by: Fam Zheng 
---
 block.c   |   19 +++
 block.h   |1 +
 block/raw-posix.c |   21 +
 block/raw-win32.c |   29 +
 block/vmdk.c  |   21 +
 block_int.h   |1 +
 qemu-img.c|   31 +--
 7 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/block.c b/block.c
index 24a25d5..9549b9e 100644
--- a/block.c
+++ b/block.c
@@ -1147,6 +1147,25 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
 }
 
 /**
+ * Length of a allocated file in bytes. Sparse files are counted by actual
+ * allocated space. Return < 0 if error or unknown.
+ */
+int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+if (!drv) {
+return -ENOMEDIUM;
+}
+if (drv->bdrv_get_allocated_file_size) {
+return drv->bdrv_get_allocated_file_size(bs);
+}
+if (bs->file) {
+return bdrv_get_allocated_file_size(bs->file);
+}
+return -ENOTSUP;
+}
+
+/**
  * Length of a file in bytes. Return < 0 if error or unknown.
  */
 int64_t bdrv_getlength(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 859d1d9..59cc410 100644
--- a/block.h
+++ b/block.h
@@ -89,6 +89,7 @@ int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
 const uint8_t *buf, int nb_sectors);
 int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
+int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int 
*psecs);
 int bdrv_commit(BlockDriverState *bs);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4cd7d7a..911cc0d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -791,6 +791,17 @@ static int64_t raw_getlength(BlockDriverState *bs)
 }
 #endif
 
+static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
+{
+struct stat st;
+BDRVRawState *s = bs->opaque;
+
+if (fstat(s->fd, &st) < 0) {
+return -errno;
+}
+return (int64_t)st.st_blocks * 512;
+}
+
 static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
 int fd;
@@ -886,6 +897,8 @@ static BlockDriver bdrv_file = {
 
 .bdrv_truncate = raw_truncate,
 .bdrv_getlength = raw_getlength,
+.bdrv_get_allocated_file_size
+= raw_get_allocated_file_size,
 
 .create_options = raw_create_options,
 };
@@ -1154,6 +1167,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_read  = raw_read,
 .bdrv_write = raw_write,
 .bdrv_getlength= raw_getlength,
+.bdrv_get_allocated_file_size
+= raw_get_allocated_file_size,
 
 /* generic scsi device */
 #ifdef __linux__
@@ -1269,6 +1284,8 @@ static BlockDriver bdrv_host_floppy = {
 .bdrv_read  = raw_read,
 .bdrv_write = raw_write,
 .bdrv_getlength= raw_getlength,
+.bdrv_get_allocated_file_size
+= raw_get_allocated_file_size,
 
 /* removable device support */
 .bdrv_is_inserted   = floppy_is_inserted,
@@ -1366,6 +1383,8 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_read  = raw_read,
 .bdrv_write = raw_write,
 .bdrv_getlength = raw_getlength,
+.bdrv_get_allocated_file_size
+= raw_get_allocated_file_size,
 
 /* removable device support */
 .bdrv_is_inserted   = cdrom_is_inserted,
@@ -1489,6 +1508,8 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_read  = raw_read,
 .bdrv_write = raw_write,
 .bdrv_getlength = raw_getlength,
+.bdrv_get_allocated_file_size
+= raw_get_allocated_file_size,
 
 /* removable device support */
 .bdrv_is_inserted   = cdrom_is_inserted,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 56bd719..91067e7 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -213,6 +213,31 @@ static int64_t raw_getlength(BlockDriverState *bs)
 return l.QuadPart;
 }
 
+static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
+{
+typedef DWORD (WINAPI * get_compressed_t)(const char *filename,
+  DWORD * high);
+get_compressed_t get_compressed;
+struct _stati64 st;
+const char *filename = bs->filename;
+/* WinNT support GetCompressedFileSize to determine allocate size */
+  

[Qemu-devel] [PATCH v2 10/12] VMDK: create different subformats

2011-06-24 Thread famcool
From: Fam Zheng 

Add create option 'format', with enums:
monolithicSparse
monolithicFlat
twoGbMaxExtentSparse
twoGbMaxExtentFlat
Each creates a subformat image file. The default is monolithiSparse.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  563 ++
 block_int.h  |1 +
 2 files changed, 333 insertions(+), 231 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1783f6b..bbc9299 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -145,8 +145,8 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 #define CHECK_CID 1
 
 #define SECTOR_SIZE 512
-#define DESC_SIZE 20*SECTOR_SIZE   // 20 sectors of 512 bytes each
-#define HEADER_SIZE 512// first sector of 512 bytes
+#define DESC_SIZE (20 * SECTOR_SIZE)/* 20 sectors of 512 bytes each */
+#define HEADER_SIZE 512 /* first sector of 512 bytes */
 
 static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
 {
@@ -218,167 +218,6 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
 return 1;
 }
 
-static int vmdk_snapshot_create(const char *filename, const char *backing_file)
-{
-int snp_fd, p_fd;
-int ret;
-uint32_t p_cid;
-char *p_name, *gd_buf, *rgd_buf;
-const char *real_filename, *temp_str;
-VMDK4Header header;
-uint32_t gde_entries, gd_size;
-int64_t gd_offset, rgd_offset, capacity, gt_size;
-char p_desc[DESC_SIZE], s_desc[DESC_SIZE], hdr[HEADER_SIZE];
-static const char desc_template[] =
-"# Disk DescriptorFile\n"
-"version=1\n"
-"CID=%x\n"
-"parentCID=%x\n"
-"createType=\"monolithicSparse\"\n"
-"parentFileNameHint=\"%s\"\n"
-"\n"
-"# Extent description\n"
-"RW %u SPARSE \"%s\"\n"
-"\n"
-"# The Disk Data Base \n"
-"#DDB\n"
-"\n";
-
-snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | 
O_LARGEFILE, 0644);
-if (snp_fd < 0)
-return -errno;
-p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE);
-if (p_fd < 0) {
-close(snp_fd);
-return -errno;
-}
-
-/* read the header */
-if (lseek(p_fd, 0x0, SEEK_SET) == -1) {
-ret = -errno;
-goto fail;
-}
-if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) {
-ret = -errno;
-goto fail;
-}
-
-/* write the header */
-if (lseek(snp_fd, 0x0, SEEK_SET) == -1) {
-ret = -errno;
-goto fail;
-}
-if (write(snp_fd, hdr, HEADER_SIZE) == -1) {
-ret = -errno;
-goto fail;
-}
-
-memset(&header, 0, sizeof(header));
-memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
-
-if (ftruncate(snp_fd, header.grain_offset << 9)) {
-ret = -errno;
-goto fail;
-}
-if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
-ret = -errno;
-goto fail;
-}
-if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) {
-ret = -errno;
-goto fail;
-}
-
-if ((p_name = strstr(p_desc,"CID")) != NULL) {
-p_name += sizeof("CID");
-sscanf(p_name,"%x",&p_cid);
-}
-
-real_filename = filename;
-if ((temp_str = strrchr(real_filename, '\\')) != NULL)
-real_filename = temp_str + 1;
-if ((temp_str = strrchr(real_filename, '/')) != NULL)
-real_filename = temp_str + 1;
-if ((temp_str = strrchr(real_filename, ':')) != NULL)
-real_filename = temp_str + 1;
-
-snprintf(s_desc, sizeof(s_desc), desc_template, p_cid, p_cid, backing_file,
- (uint32_t)header.capacity, real_filename);
-
-/* write the descriptor */
-if (lseek(snp_fd, 0x200, SEEK_SET) == -1) {
-ret = -errno;
-goto fail;
-}
-if (write(snp_fd, s_desc, strlen(s_desc)) == -1) {
-ret = -errno;
-goto fail;
-}
-
-gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table
-rgd_offset = header.rgd_offset * SECTOR_SIZE;   // offset of RGD table
-capacity = header.capacity * SECTOR_SIZE;   // Extent size
-/*
- * Each GDE span 32M disk, means:
- * 512 GTE per GT, each GTE points to grain
- */
-gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * 
SECTOR_SIZE;
-if (!gt_size) {
-ret = -EINVAL;
-goto fail;
-}
-gde_entries = (uint32_t)(capacity / gt_size);  // number of gde/rgde
-gd_size = gde_entries * sizeof(uint32_t);
-
-/* write RGD */
-rgd_buf = qemu_malloc(gd_size);
-if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) {
-ret = -errno;
-goto fail_rgd;
-}
-if (read(p_fd, rgd_buf, gd_size) != gd_size) {
-ret = -errno;
-goto fail_rgd;
-}
-if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) {
-ret = -errno;
-goto fail_rgd;
-}
-if (write(snp_fd, rgd_buf, gd_size) == -1) {
-ret = -errno;
-goto fail_rgd;
-}
-
-/* write GD */
-gd_buf = qemu_mallo

[Qemu-devel] [PATCH v2 05/12] VMDK: add field BDRVVmdkState.desc_offset

2011-06-24 Thread famcool
From: Fam Zheng 

There are several occurrence of magic number 0x200 as the descriptor
offset within mono sparse image file. This is not the case for images
with separate descriptor file. So a field is added to BDRVVmdkState to
hold the correct value.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   26 --
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8d55e5f..e1b94c2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -81,6 +81,7 @@ typedef struct VmdkExtent {
 } VmdkExtent;
 
 typedef struct BDRVVmdkState {
+int desc_offset;
 int num_extents;
 uint32_t parent_cid;
 VmdkExtent *extents;
@@ -151,10 +152,11 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int 
parent)
 uint32_t cid;
 const char *p_name, *cid_str;
 size_t cid_str_size;
+BDRVVmdkState *s = bs->opaque;
 
-/* the descriptor offset = 0x200 */
-if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
+if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
 return 0;
+}
 
 if (parent) {
 cid_str = "parentCID";
@@ -176,10 +178,11 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t 
cid)
 {
 char desc[DESC_SIZE], tmp_desc[DESC_SIZE];
 char *p_name, *tmp_str;
+BDRVVmdkState *s = bs->opaque;
 
-/* the descriptor offset = 0x200 */
-if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
-return -1;
+if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
+return -EIO;
+}
 
 tmp_str = strstr(desc,"parentCID");
 pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str);
@@ -189,8 +192,9 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t 
cid)
 pstrcat(desc, sizeof(desc), tmp_desc);
 }
 
-if (bdrv_pwrite_sync(bs->file, 0x200, desc, DESC_SIZE) < 0)
-return -1;
+if (bdrv_pwrite_sync(bs->file, s->desc_offset, desc, DESC_SIZE) < 0) {
+return -EIO;
+}
 return 0;
 }
 
@@ -274,7 +278,6 @@ static int vmdk_snapshot_create(const char *filename, const 
char *backing_file)
 ret = -errno;
 goto fail;
 }
-/* the descriptor offset = 0x200 */
 if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
 ret = -errno;
 goto fail;
@@ -378,10 +381,11 @@ static int vmdk_parent_open(BlockDriverState *bs)
 {
 char *p_name;
 char desc[DESC_SIZE];
+BDRVVmdkState *s = bs->opaque;
 
-/* the descriptor offset = 0x200 */
-if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
+if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
 return -1;
+}
 
 if ((p_name = strstr(desc,"parentFileNameHint")) != NULL) {
 char *end_name;
@@ -479,6 +483,7 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
 goto fail;
 }
 s->num_extents = 1;
+s->desc_offset = 0x200;
 ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
 if (ret != sizeof(header)) {
 ret = ret < 0 ? ret : -EIO;
@@ -520,6 +525,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
 goto fail;
 }
 s->num_extents = 1;
+s->desc_offset = 0x200;
 ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
 if (ret != sizeof(header)) {
 ret = ret < 0 ? ret : -EIO;



[Qemu-devel] [V11 06/15] virtio-9p: Create support in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Add both chroot worker & qemu side interfaces to create regular files in
chroot environment

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   36 
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |5 +++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index 40a54b3..581bfa95a 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -93,6 +93,36 @@ static int chroot_do_open(V9fsFileObjectRequest *request)
 return fd;
 }
 
+/*
+ * Helper routine to create a file and return the file descriptor
+ */
+static int chroot_do_create(V9fsFileObjectRequest *request)
+{
+uid_t cur_uid;
+gid_t cur_gid;
+int fd = -1;
+
+cur_uid = geteuid();
+cur_gid = getegid();
+
+if (setfsuid(request->data.uid) < 0) {
+return -errno;
+}
+if (setfsgid(request->data.gid) < 0) {
+fd = -errno;
+goto unset_uid;
+}
+
+fd = open(request->path.path, request->data.flags, request->data.mode);
+if (fd < 0) {
+fd = -errno;
+}
+setfsgid(cur_gid);
+unset_uid:
+setfsuid(cur_uid);
+return fd;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -184,6 +214,12 @@ int v9fs_chroot(FsContext *fs_ctx)
 valid_fd = 1;
 }
 break;
+case T_CREATE:
+retval = chroot_do_create(&request);
+if (retval >= 0) {
+valid_fd = 1;
+}
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 326238d..d5c3f37 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -4,6 +4,7 @@
 #include "qemu_socket.h"
 /* types for V9fsFileObjectRequest */
 #define T_OPEN  1
+#define T_CREATE2
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2ae4317..2ae08e8 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -455,8 +455,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath 
*dir_path, const char *name,
 serrno = errno;
 goto err_end;
 }
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 fd = open(rpath(fs_ctx, path, buffer), flags, credp->fc_mode);
 if (fd == -1) {
 err = fd;
@@ -467,6 +466,8 @@ static int local_open2(FsContext *fs_ctx, V9fsPath 
*dir_path, const char *name,
 serrno = errno;
 goto err_end;
 }
+} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+fd = passthrough_request(fs_ctx, NULL, path, flags, credp, T_CREATE);
 }
 err = fd;
 goto out;
-- 
1.7.5.4




[Qemu-devel] [V11 08/15] virtio-9p: Removing file or directory in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Support for removing file or directory in chroot environment. Add
interfaces to remove file/directory in chroot worker and qemu side.

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |4 
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index 10f290d..f269c7b 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -169,6 +169,21 @@ unset_uid:
 return retval;
 }
 
+/*
+ * Remove a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_remove(V9fsFileObjectRequest *request)
+{
+int retval;
+
+retval = remove(request->path.path);
+if (retval < 0) {
+return -errno;
+}
+return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -272,6 +287,9 @@ int v9fs_chroot(FsContext *fs_ctx)
 case T_MKNOD:
 retval = chroot_do_create_special(&request);
 break;
+case T_REMOVE:
+retval = chroot_do_remove(&request);
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 9075361..9d69739 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -9,6 +9,7 @@
 #define T_MKNOD 4
 #define T_SYMLINK   5
 #define T_LINK  6
+#define T_REMOVE7
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index a1eff5f..f2d66e1 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -617,6 +617,10 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path,
 static int local_remove(FsContext *ctx, const char *path)
 {
 char buffer[PATH_MAX];
+
+if (ctx->fs_sm == SM_PASSTHROUGH) {
+return passthrough_request(ctx, NULL, path, 0, NULL, T_REMOVE);
+}
 return remove(rpath(ctx, path, buffer));
 }
 
-- 
1.7.5.4




[Qemu-devel] [PATCH v2 06/12] VMDK: flush multiple extents

2011-06-24 Thread famcool
From: Fam Zheng 

Flush all the file that referenced by the image.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index e1b94c2..41a18a1 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1061,7 +1061,14 @@ static void vmdk_close(BlockDriverState *bs)
 
 static int vmdk_flush(BlockDriverState *bs)
 {
-return bdrv_flush(bs->file);
+int i, ret;
+BDRVVmdkState *s = bs->opaque;
+
+ret = bdrv_flush(bs->file);
+for (i = 0; i < s->num_extents; i++) {
+ret |= bdrv_flush(s->extents[i].file);
+}
+return ret;
 }
 
 



[Qemu-devel] [V11 12/15] virtio-9p: chown in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Add support to do chown in chroot process

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |9 +
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index d297b50..8ca4805 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -213,6 +213,21 @@ static int chroot_do_chmod(V9fsFileObjectRequest *request)
 return 0;
 }
 
+/*
+ * Change ownership of a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_chown(V9fsFileObjectRequest *request)
+{
+int retval;
+
+retval = lchown(request->path.path, request->data.uid, request->data.gid);
+if (retval < 0) {
+return -errno;
+}
+return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -325,6 +340,9 @@ int v9fs_chroot(FsContext *fs_ctx)
 case T_CHMOD:
 retval = chroot_do_chmod(&request);
 break;
+case T_CHOWN:
+retval = chroot_do_chown(&request);
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index fc7a134..07c6627 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -12,6 +12,7 @@
 #define T_REMOVE7
 #define T_RENAME8
 #define T_CHMOD 9
+#define T_CHOWN 10
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2b22f5d..ea7d954 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -587,16 +587,17 @@ static int local_chown(FsContext *fs_ctx, V9fsPath 
*fs_path, FsCred *credp)
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
-if ((credp->fc_uid == -1 && credp->fc_gid == -1) ||
-(fs_ctx->fs_sm == SM_PASSTHROUGH)) {
+if (fs_ctx->fs_sm != SM_PASSTHROUGH &&
+(credp->fc_uid == -1 && credp->fc_gid == -1)) {
 return lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
 credp->fc_gid);
 } else if (fs_ctx->fs_sm == SM_MAPPED) {
 return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 return lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
 credp->fc_gid);
+} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHOWN);
 }
 return -1;
 }
-- 
1.7.5.4




[Qemu-devel] [PATCH v2 11/12] VMDK: fix coding style

2011-06-24 Thread famcool
From: Fam Zheng 

Conform coding style in vmdk.c to pass scripts/checkpatch.pl checks.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   79 +++--
 1 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index bbc9299..4e1931f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -101,8 +101,9 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 {
 uint32_t magic;
 
-if (buf_size < 4)
+if (buf_size < 4) {
 return 0;
+}
 magic = be32_to_cpu(*(uint32_t *)buf);
 if (magic == VMDK3_MAGIC ||
 magic == VMDK4_MAGIC) {
@@ -168,9 +169,10 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int 
parent)
 cid_str_size = sizeof("CID");
 }
 
-if ((p_name = strstr(desc,cid_str)) != NULL) {
+p_name = strstr(desc, cid_str);
+if (p_name != NULL) {
 p_name += cid_str_size;
-sscanf(p_name,"%x",&cid);
+sscanf(p_name, "%x", &cid);
 }
 
 return cid;
@@ -186,9 +188,10 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t 
cid)
 return -EIO;
 }
 
-tmp_str = strstr(desc,"parentCID");
+tmp_str = strstr(desc, "parentCID");
 pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str);
-if ((p_name = strstr(desc,"CID")) != NULL) {
+p_name = strstr(desc, "CID");
+if (p_name != NULL) {
 p_name += sizeof("CID");
 snprintf(p_name, sizeof(desc) - (p_name - desc), "%x\n", cid);
 pstrcat(desc, sizeof(desc), tmp_desc);
@@ -208,13 +211,14 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
 uint32_t cur_pcid;
 
 if (p_bs) {
-cur_pcid = vmdk_read_cid(p_bs,0);
-if (s->parent_cid != cur_pcid)
-// CID not valid
+cur_pcid = vmdk_read_cid(p_bs, 0);
+if (s->parent_cid != cur_pcid) {
+/* CID not valid */
 return 0;
+}
 }
 #endif
-// CID valid
+/* CID valid */
 return 1;
 }
 
@@ -229,14 +233,18 @@ static int vmdk_parent_open(BlockDriverState *bs)
 return -1;
 }
 
-if ((p_name = strstr(desc,"parentFileNameHint")) != NULL) {
+p_name = strstr(desc, "parentFileNameHint");
+if (p_name != NULL) {
 char *end_name;
 
 p_name += sizeof("parentFileNameHint") + 1;
-if ((end_name = strchr(p_name,'\"')) == NULL)
+end_name = strchr(p_name, '\"');
+if (end_name == NULL) {
 return -1;
-if ((end_name - p_name) > sizeof (bs->backing_file) - 1)
+}
+if ((end_name - p_name) > sizeof(bs->backing_file) - 1) {
 return -1;
+}
 
 pstrcpy(bs->backing_file, end_name - p_name + 1, p_name);
 }
@@ -594,8 +602,9 @@ static int get_whole_cluster(BlockDriverState *bs,
 if (bs->backing_hd) {
 int ret;
 
-if (!vmdk_is_cid_valid(bs))
+if (!vmdk_is_cid_valid(bs)) {
 return -1;
+}
 
 /* floor offset to cluster */
 offset -= offset % (extent->cluster_sectors * 512);
@@ -654,8 +663,9 @@ static int get_cluster_offset(BlockDriverState *bs,
 int min_index, i, j;
 uint32_t min_count, *l2_table, tmp = 0;
 
-if (m_data)
+if (m_data) {
 m_data->valid = 0;
+}
 if (extent->flat) {
 *cluster_offset = extent->flat_start_offset;
 return 0;
@@ -711,7 +721,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 return -1;
 }
 
-// Avoid the L2 tables update for the images that have snapshots.
+/* Avoid the L2 tables update for the images that have snapshots. */
 *cluster_offset = bdrv_getlength(extent->file);
 bdrv_truncate(
 extent->file,
@@ -728,8 +738,9 @@ static int get_cluster_offset(BlockDriverState *bs,
  * or inappropriate VM shutdown.
  */
 if (get_whole_cluster(
-bs, extent, *cluster_offset, offset, allocate) == -1)
+bs, extent, *cluster_offset, offset, allocate) == -1) {
 return -1;
+}
 
 if (m_data) {
 m_data->offset = tmp;
@@ -779,8 +790,9 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t 
sector_num,
 
 index_in_cluster = sector_num % extent->cluster_sectors;
 n = extent->cluster_sectors - index_in_cluster;
-if (n > nb_sectors)
+if (n > nb_sectors) {
 n = nb_sectors;
+}
 *pnum = n;
 return ret;
 }
@@ -804,16 +816,19 @@ static int vmdk_read(BlockDriverState *bs, int64_t 
sector_num,
 sector_num << 9, 0, &cluster_offset);
 index_in_cluster = sector_num % extent->cluster_sectors;
 n = extent->cluster_sectors - index_in_cluster;
-if (n > nb_sectors)
+if (n > nb_sectors) {
 n = nb_sectors;
+}
 if (ret) {
 /* if not allocated, try to read from parent image, if exist */
 if (bs->backing_hd)

[Qemu-devel] [V11 07/15] virtio-9p: Creating special files in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Add both chroot worker and qemu side interfaces to create special files
(directory, device nodes, links and symbolic links)

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   52 +
 hw/9pfs/virtio-9p-chroot.h|5 +++
 hw/9pfs/virtio-9p-local.c |   37 +-
 3 files changed, 75 insertions(+), 19 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index 581bfa95a..10f290d 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -123,6 +123,52 @@ unset_uid:
 return fd;
 }
 
+/*
+ * Create directory, symbolic link, link, device node and regular files
+ * Similar to create, but it does not return the fd of created object
+ * Returns 0 as file descriptor on success and -errno on failure
+ */
+static int chroot_do_create_special(V9fsFileObjectRequest *request)
+{
+int cur_uid, cur_gid;
+int retval = -1;
+
+cur_uid = geteuid();
+cur_gid = getegid();
+
+if (setfsuid(request->data.uid) < 0) {
+return -errno;
+}
+if (setfsgid(request->data.gid) < 0) {
+retval = -errno;
+goto unset_uid;
+}
+
+switch (request->data.type) {
+case T_MKDIR:
+retval = mkdir(request->path.path, request->data.mode);
+break;
+case T_SYMLINK:
+retval = symlink(request->path.old_path, request->path.path);
+break;
+case T_LINK:
+retval = link(request->path.old_path, request->path.path);
+break;
+default:
+retval = mknod(request->path.path, request->data.mode,
+request->data.dev);
+break;
+}
+
+if (retval < 0) {
+retval = -errno;
+}
+setfsgid(cur_gid);
+unset_uid:
+setfsuid(cur_uid);
+return retval;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -220,6 +266,12 @@ int v9fs_chroot(FsContext *fs_ctx)
 valid_fd = 1;
 }
 break;
+case T_MKDIR:
+case T_SYMLINK:
+case T_LINK:
+case T_MKNOD:
+retval = chroot_do_create_special(&request);
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index d5c3f37..9075361 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -5,6 +5,10 @@
 /* types for V9fsFileObjectRequest */
 #define T_OPEN  1
 #define T_CREATE2
+#define T_MKDIR 3
+#define T_MKNOD 4
+#define T_SYMLINK   5
+#define T_LINK  6
 
 #define V9FS_FD_VALID INT_MAX
 
@@ -37,5 +41,6 @@ typedef struct V9fsFileObjectRequest
 
 int v9fs_chroot(FsContext *fs_ctx);
 int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or);
+int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2ae08e8..a1eff5f 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -327,8 +327,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath 
*dir_path,
 serrno = errno;
 goto err_end;
 }
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 err = mknod(rpath(fs_ctx, path, buffer), credp->fc_mode,
 credp->fc_rdev);
 if (err == -1) {
@@ -339,6 +338,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath 
*dir_path,
 serrno = errno;
 goto err_end;
 }
+} else {
+err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKNOD);
 }
 goto out;
 
@@ -375,8 +376,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath 
*dir_path,
 serrno = errno;
 goto err_end;
 }
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 err = mkdir(rpath(fs_ctx, path, buffer), credp->fc_mode);
 if (err == -1) {
 goto out;
@@ -386,6 +386,8 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath 
*dir_path,
 serrno = errno;
 goto err_end;
 }
+} else {
+err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKDIR);
 }
 goto out;
 
@@ -524,25 +526,17 @@ static int local_symlink(FsContext *fs_ctx, const char 
*oldpath,
 serrno = errno;
 goto err_end;
 }
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 err = symlink(oldpath, rpath(fs_ctx, newpath, buffer));
 if (err) {
 goto out;
 }
 err = lchown(rpath(fs_ctx, newpath, buffer), credp->fc_uid,
   

[Qemu-devel] [V11 11/15] virtio-9p: chmod in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Add support to do chmod operation in chroot process.

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |5 +++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index 9bb94f2..d297b50 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -198,6 +198,21 @@ static int chroot_do_rename(V9fsFileObjectRequest *request)
 return 0;
 }
 
+/*
+ * Change permissions of a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_chmod(V9fsFileObjectRequest *request)
+{
+int retval;
+
+retval = chmod(request->path.path, request->data.mode);
+if (retval < 0) {
+return -errno;
+}
+return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -307,6 +322,9 @@ int v9fs_chroot(FsContext *fs_ctx)
 case T_RENAME:
 retval = chroot_do_rename(&request);
 break;
+case T_CHMOD:
+retval = chroot_do_chmod(&request);
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index fda60af..fc7a134 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -11,6 +11,7 @@
 #define T_LINK  6
 #define T_REMOVE7
 #define T_RENAME8
+#define T_CHMOD 9
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2645da0..2b22f5d 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -288,9 +288,10 @@ static int local_chmod(FsContext *fs_ctx, V9fsPath 
*fs_path, FsCred *credp)
 
 if (fs_ctx->fs_sm == SM_MAPPED) {
 return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 return chmod(rpath(fs_ctx, path, buffer), credp->fc_mode);
+} else {
+return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHMOD);
 }
 return -1;
 }
-- 
1.7.5.4




[Qemu-devel] [V11 13/15] virtio-9p: stat in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 


Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   52 -
 hw/9pfs/virtio-9p-chroot.c|   59 -
 hw/9pfs/virtio-9p-chroot.h|3 ++
 hw/9pfs/virtio-9p-local.c |   30 +++
 4 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index 8ca4805..c4ebb96 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -66,6 +66,29 @@ static void chroot_sendfd(int sockfd, int fd, int fd_valid)
 }
 }
 
+/* Send special information and error status to qemu process */
+static void chroot_sendspecial(int sockfd, void *buff, int size, int error)
+{
+int retval;
+
+/* If there is an error, send NULL buff also set status to error */
+if (error) {
+memset(buff, 0, size);
+}
+do {
+retval = send(sockfd, &error, sizeof(error), 0);
+} while (retval < 0 && errno == EINTR);
+if (retval < 0) {
+_exit(1);
+}
+do {
+retval = send(sockfd, buff, size, 0);
+} while (retval < 0 && errno == EINTR);
+if (retval < 0) {
+_exit(1);
+}
+}
+
 /* Read V9fsFileObjectRequest sent by QEMU process */
 static int chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
 {
@@ -267,6 +290,7 @@ int v9fs_chroot(FsContext *fs_ctx)
 pid_t pid;
 uint32_t code;
 int retval, valid_fd;
+char *buff;
 
 if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) {
 error_report("socketpair %s", strerror(errno));
@@ -346,7 +370,33 @@ int v9fs_chroot(FsContext *fs_ctx)
 default:
 retval = -1;
 break;
+case T_LSTAT:
+buff = qemu_malloc(request.data.size);
+retval = lstat(request.path.path, (struct stat *)buff);
+if (retval  < 0) {
+retval = -errno;
+}
+break;
+}
+
+/* Send the results */
+switch (request.data.type) {
+case T_OPEN:
+case T_CREATE:
+case T_MKDIR:
+case T_SYMLINK:
+case T_LINK:
+case T_MKNOD:
+case T_REMOVE:
+case T_RENAME:
+case T_CHMOD:
+case T_CHOWN:
+chroot_sendfd(chroot_sock, retval, valid_fd);
+break;
+case T_LSTAT:
+chroot_sendspecial(chroot_sock, buff, request.data.size, retval);
+qemu_free(buff);
+break;
 }
-chroot_sendfd(chroot_sock, retval, valid_fd);
 }
 }
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index f5b3abc..9480ce0 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -77,6 +77,37 @@ static int v9fs_receivefd(int sockfd, int *sock_error)
 return -ENFILE; /* Ancillary data sent but not received */
 }
 
+static int qemu_recv(int sockfd, void *data, size_t size, int flags)
+{
+int retval;
+do {
+retval = recv(sockfd, data, size, 0);
+} while (retval < 0 && errno == EINTR);
+return retval;
+}
+
+/*
+ * Return received struct stat on success and -errno on failure.
+ * sock_error is set to 1 whenever there is error in socket IO
+ */
+static int v9fs_special_receive(int sockfd, int *sock_error, char *buff,
+int size)
+{
+int retval, status;
+if (qemu_recv(sockfd, &status, sizeof(status), 0) <= 0) {
+*sock_error = 1;
+return -EIO;
+}
+
+retval = qemu_recv(sockfd, buff, size, 0);
+if (retval > 0) {
+return status;
+} else {
+*sock_error = 1;
+return -EIO;
+}
+}
+
 /*
  * V9fsFileObjectRequest is written into the socket by QEMU process.
  * Then this request is read by chroot process using v9fs_read_request function
@@ -94,7 +125,7 @@ static int v9fs_write_request(int sockfd, 
V9fsFileObjectRequest *request)
 /* Return opened file descriptor on success or -errno on error */
 int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request)
 {
-int fd, sock_error;
+int fd, sock_error = 0;
 qemu_mutex_lock(&fs_ctx->chroot_mutex);
 if (fs_ctx->chroot_socket == -1) {
 goto error;
@@ -114,3 +145,29 @@ error:
 qemu_mutex_unlock(&fs_ctx->chroot_mutex);
 return -EIO;
 }
+
+/* Return special information on success or -errno on error */
+int v9fs_special(FsContext *fs_ctx, V9fsFileObjectRequest *request,
+char *buff, int size)
+{
+int retval, sock_error = 0;
+qemu_mutex_lock(&fs_ctx->chroot_mutex);
+if (fs_ctx->chroot_socket == -1) {
+goto error;
+}
+if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
+goto error;
+}
+retval = v9fs_special_receive(fs_ctx->chroot_socket, &sock_error, buff,
+size);
+if (retval < 0 && sock_error) {
+goto error;
+}
+qemu_mutex_unlock(&fs_ctx->ch

[Qemu-devel] [V11 01/15] Implement qemu_read_full

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Signed-off-by: M. Mohan Kumar 
---
 osdep.c   |   32 
 qemu-common.h |2 ++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/osdep.c b/osdep.c
index 56e6963..5a4d670 100644
--- a/osdep.c
+++ b/osdep.c
@@ -126,6 +126,38 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
count)
 }
 
 /*
+ * A variant of read(2) which handles interrupted read.
+ * Simlar to qemu_write_full function
+ *
+ * Return the number of bytes read.
+ *
+ * This function does not work with non-blocking fd's.
+ * errno is set if fewer than `count' bytes are read because of any
+ * error
+ */
+ssize_t qemu_read_full(int fd, void *buf, size_t count)
+{
+ssize_t ret = 0;
+ssize_t total = 0;
+
+while (count) {
+ret = read(fd, buf, count);
+if (ret <= 0) {
+if (errno == EINTR) {
+continue;
+}
+break;
+}
+
+count -= ret;
+buf += ret;
+total += ret;
+}
+
+return total;
+}
+
+/*
  * Opens a socket with FD_CLOEXEC set
  */
 int qemu_socket(int domain, int type, int protocol)
diff --git a/qemu-common.h b/qemu-common.h
index 39fabc9..ea74ead 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -216,6 +216,8 @@ void qemu_mutex_unlock_iothread(void);
 int qemu_open(const char *name, int flags, ...);
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 QEMU_WARN_UNUSED_RESULT;
+ssize_t qemu_read_full(int fd, void *buf, size_t count)
+QEMU_WARN_UNUSED_RESULT;
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32
-- 
1.7.5.4




[Qemu-devel] [V11 00/15] virtio-9p: Use chroot to safely access files in passthrough security model

2011-06-24 Thread M. Mohan Kumar
In passthrough security model, following symbolic links in the server
side could result in TOCTTOU vulnerabilities.
(http://en.wikipedia.org/wiki/Time-of-check-to-time-of-use)

This patchset resolves this issue by creating a dedicated process which
chroots into the share path and all file object access is done in the
chroot environment.

This patchset implements chroot enviroment, provides necessary functions
that can be used by the passthrough function calls.

This patchset is rebased on top of 9p coroutines patches posted to
qemu-devel list
http://lists.nongnu.org/archive/html/qemu-devel/2011-05/msg02796.html

Changes from version V10:
* Added support to do lstat and readlink from chroot process
* Fixed an issue with dealing fds when qemu process reached maxfds limit

Changes from version V9:
* Error handling in special file object creation in virtio-9p-local.c

Changes from version V8:
* Make chmod and chown also operate under chroot process
* Check for invalid path requests, minor cleanups

Changes from version V7:
* Add two chroot methods remove and rename
* Minor cleanups like consolidating functions

Changes from version V6:
* Send only fd/errno in socket operations instead of FdInfo structure
* Minor cleanups

Changes from version V5:
* Return errno on failure instead of setting errno
* Minor cleanups like updated comments, enable CONFIG_THREAD if
  CONFIG_VIRTFS is enabled

Changes from version V4:
* Avoid using malloc/free inside chroot process
* Seperate chroot server and client functions

Changes from version V3
* Return EIO incase of socket read/write fail instead of exiting
* Changed data types as suggested by Blue Swirl
* Chroot process reports error through qemu process

Changes from version V2
* Treat socket IO errors as fatal, ie qemu will exit
* Split patchset based on chroot side (server) and qemu side(client)
  functionalities

M. Mohan Kumar (15):
  Implement qemu_read_full
  virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
  virtio-9p: Provide chroot worker side interfaces
  virtio-9p: Add qemu side interfaces for chroot environment
  virtio-9p: Add support to open a file in chroot environment
  virtio-9p: Create support in chroot environment
  virtio-9p: Support for creating special files
  virtio-9p: Add support for removing file or directory
  virtio-9p: Add support to rename
  virtio-9p: Move file post creation changes to none security model
  virtio-9p: Add support for chmod
  virtio-9p: Add support for chown
  virtio-9p: Chroot environment for other functions
  virtio-9p: Add stat functionality to chroot
  virtio-9p: Add readlink support to chroot

 Makefile.objs |1 +
 configure |1 +
 fsdev/file-op-9p.h|3 +
 hw/9pfs/virtio-9p-chroot-worker.c |  418 +
 hw/9pfs/virtio-9p-chroot.c|  174 +++
 hw/9pfs/virtio-9p-chroot.h|   54 +
 hw/9pfs/virtio-9p-device.c|   24 ++
 hw/9pfs/virtio-9p-local.c |  248 ++
 osdep.c   |   32 +++
 qemu-common.h |2 +
 10 files changed, 910 insertions(+), 47 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot-worker.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.h

-- 
1.7.5.1




[Qemu-devel] [V11 03/15] virtio-9p: Provide chroot worker side interfaces

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Implement chroot worker side interfaces like sending the file
descriptor to qemu process, reading the object request from socket etc.
Also add chroot main function and other helper routines.

Signed-off-by: M. Mohan Kumar 
[mala...@us.ibm.com: Do not send fd as part of data, instead a special
value is sent as part of data when fd is sent as part of ancillary data]
---
 Makefile.objs |1 +
 fsdev/file-op-9p.h|3 +
 hw/9pfs/virtio-9p-chroot-worker.c |  193 +
 hw/9pfs/virtio-9p-chroot.h|   39 
 hw/9pfs/virtio-9p-device.c|   24 +
 5 files changed, 260 insertions(+), 0 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot-worker.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.h

diff --git a/Makefile.objs b/Makefile.objs
index e38ca15..588eae2 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -304,6 +304,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o virtio-9p-handle.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-worker.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 5d088d4..66dac07 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "qemu-thread.h"
 
 #define SM_LOCAL_MODE_BITS0600
 #define SM_LOCAL_DIR_MODE_BITS0700
@@ -63,6 +64,8 @@ typedef struct FsContext
 struct xattr_operations **xops;
 /* fs driver specific data */
 void *private;
+QemuMutex chroot_mutex;
+int chroot_socket;
 } FsContext;
 
 typedef struct V9fsPath {
diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
new file mode 100644
index 000..40a54b3
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -0,0 +1,193 @@
+/*
+ * Virtio 9p chroot environment for contained access to the exported path
+ * Code path handles chroot worker side interfaces
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * M. Mohan Kumar 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the copying file in the top-level directory
+ *
+ */
+
+#include 
+#include 
+#include 
+#include "qemu_socket.h"
+#include "qemu-thread.h"
+#include "qerror.h"
+#include "virtio-9p.h"
+#include "virtio-9p-chroot.h"
+
+/* Send file descriptor and error status to qemu process */
+static void chroot_sendfd(int sockfd, int fd, int fd_valid)
+{
+struct msghdr msg = { };
+struct iovec iov;
+struct cmsghdr *cmsg;
+int retval, data;
+union MsgControl msg_control;
+
+iov.iov_base = &data;
+iov.iov_len = sizeof(data);
+
+memset(&msg, 0, sizeof(msg));
+msg.msg_iov = &iov;
+msg.msg_iovlen = 1;
+/* No ancillary data on error */
+if (!fd_valid) {
+/*
+ * fd is really negative errno if the request failed. Or simply
+ * zero if the request is successful and it doesn't need a file
+ * descriptor.
+ */
+data = fd;
+} else {
+data = V9FS_FD_VALID;
+msg.msg_control = &msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+cmsg = &msg_control.cmsg;
+cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
+cmsg->cmsg_level = SOL_SOCKET;
+cmsg->cmsg_type = SCM_RIGHTS;
+memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
+}
+
+do {
+retval = sendmsg(sockfd, &msg, 0);
+} while (retval < 0 && errno == EINTR);
+if (retval < 0) {
+_exit(1);
+}
+if (fd_valid) {
+close(fd);
+}
+}
+
+/* Read V9fsFileObjectRequest sent by QEMU process */
+static int chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
+{
+int retval;
+retval = qemu_read_full(sockfd, request, sizeof(*request));
+if (retval != sizeof(*request)) {
+if (errno == EBADF) {
+_exit(1);
+}
+return -EIO;
+}
+return 0;
+}
+
+/*
+ * Helper routine to open a file
+ */
+static int chroot_do_open(V9fsFileObjectRequest *request)
+{
+int fd;
+fd = open(request->path.path, request->data.flags);
+if (fd < 0) {
+fd = -errno;
+}
+return fd;
+}
+
+static void chroot_daemonize(int chroot_sock)
+{
+sigset_t sigset;
+struct rlimit nr_fd;
+int fd;
+
+/* Block all signals for this process */
+sigfillset(&sigset);
+sigprocmask(SIG_SETMASK, &sigset, NULL);
+
+/* Close other file descriptors */
+getrlimit(RLIMIT_NOFILE, &nr_fd);
+for (fd = 0; fd < nr_fd.rlim_cur; fd++) {
+if (fd != chroot_sock) {
+close(fd);
+}
+}
+chdir("/");
+/* Create files with mode as per request */
+umask(0);

[Qemu-devel] [V11 15/15] virtio-9p: Chroot environment for other functions

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Add chroot functionality for system calls that can operate on a file using
relative directory file descriptor.

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-local.c |   41 +++--
 1 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index e865cc8..5847d43 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -608,7 +608,25 @@ static int local_truncate(FsContext *ctx, V9fsPath 
*fs_path, off_t size)
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
-return truncate(rpath(ctx, path, buffer), size);
+if (ctx->fs_sm == SM_PASSTHROUGH) {
+int fd, retval, serrno;
+fd = passthrough_request(ctx, NULL, path, O_RDWR, NULL, T_OPEN);
+if (fd < 0) {
+errno = -fd;
+return -1;
+}
+retval = ftruncate(fd, size);
+if (retval < 0) {
+serrno = errno;
+}
+close(fd);
+if (retval < 0) {
+errno = serrno;
+}
+return retval;
+} else {
+return truncate(rpath(ctx, path, buffer), size);
+}
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
@@ -648,8 +666,27 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path,
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
-return qemu_utimensat(AT_FDCWD, rpath(s, path, buffer), buf,
+if (s->fs_sm == SM_PASSTHROUGH) {
+int fd, retval, serrno = 0;
+fd = passthrough_request(s, NULL, path,
+O_RDONLY | O_NONBLOCK | O_NOFOLLOW, NULL, T_OPEN);
+if (fd < 0) {
+errno = -fd;
+return -1;
+}
+retval = futimens(fd, buf);
+if (retval < 0) {
+serrno = errno;
+}
+close(fd);
+if (retval < 0) {
+errno = serrno;
+}
+return retval;
+} else {
+return qemu_utimensat(AT_FDCWD, rpath(s, path, buffer), buf,
   AT_SYMLINK_NOFOLLOW);
+}
 }
 
 static int local_remove(FsContext *ctx, const char *path)
-- 
1.7.5.4




Re: [Qemu-devel] Image streaming and live block copy

2011-06-24 Thread Stefan Hajnoczi
On Sun, Jun 19, 2011 at 5:02 PM, Dor Laor  wrote:
> On 06/18/2011 12:17 PM, Stefan Hajnoczi wrote:
>>
>> On Sat, Jun 18, 2011 at 10:15 AM, Stefan Hajnoczi
>>  wrote:
>>>
>>> On Fri, Jun 17, 2011 at 1:31 PM, Marcelo Tosatti
>>>  wrote:

 On Thu, Jun 16, 2011 at 04:30:18PM +0100, Stefan Hajnoczi wrote:
>
> On Thu, Jun 16, 2011 at 11:52:43AM -0300, Marcelo Tosatti wrote:
> This approach does not use the backing file feature?
>
>> blkstream block driver:
>>
>> - Maintain in memory whether given block is allocated in local image,
>> if not, read from remote, write to local. Set block as local.
>> Local and remote simply two block drivers from image streaming driver
>> POV.
>> - Once all blocks are local, notify mgmt so it can switch to local
>> copy.
>> - Writes are mirrored to source and destination, minding guest writes
>> over copy writes.
>
> We open the remote file read-only for image streaming and do not want
> to
> mirror writes.

 Why not? Is there any disadvantage of mirroring writes?
>>>
>>> Think of the use case with a Fedora master image over NFS.  You want a
>>> local clone of that master image and use the stream command to copy
>>> the data from the master image into the local clone.
>>>
>>> You cannot modify that master image because other VMs are using it too
>>> and/or you want to be able to clone new VMs from it in the future.
>>
>> BTW the workaround is to create two local images:
>> 1. Local clone with master image as a backing file.  This is the live
>> block copy source image.
>> 2. Local image without a backing file.  This is the live block copy
>> destination image.
>>
>> But this is not very elegant.  Writes get mirrored so that crash recovery
>> works.
>
> There is an easier work around for image streaming using live block copy
> (mirror approach):
>  - Create the dst VM as an empty new COW image of the src (even over
>    the non shared storage, use some protocol tag for the src location
>    like nbd://original_path/src_file_name)

Migration and non-shared storage has come up a few times in this
discussion.  But both live block copy and image streaming need access
to source and destination - they do not have explicit non-shared
storage support.  I think non-shared and using nbd:// is orthogonal to
the discussion.  Just want to check that you agree and I haven't
missed something?

>  - Run the usual live block copy of src image (master read only OS
>    template) to the destination.
>    - Use a -src-read-only flag that will make the copy skip the src
>      writing.
>
> Voila - no duplicate writes, crash recovery works since we reference the
> original image and we share the code.

So the running guest is using the destination image since the source
is read-only?

This approach makes sense to me.

Stefan



[Qemu-devel] [V11 02/15] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

9p Chroot environment needs APIs defined in qemu-thread.c, so enable
CONFIG_THREAD if virtfs is enabled

Signed-off-by: M. Mohan Kumar 
---
 configure |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index df63403..888ac87 100755
--- a/configure
+++ b/configure
@@ -2964,6 +2964,7 @@ fi
 if test "$linux" = "yes" ; then
   if test "$attr" = "yes" ; then
 echo "CONFIG_VIRTFS=y" >> $config_host_mak
+echo "CONFIG_THREAD=y" >> $config_host_mak
   fi
 fi
 if test "$blobs" = "yes" ; then
-- 
1.7.5.4




[Qemu-devel] [PATCH v2 04/12] VMDK: separate vmdk_open by format version

2011-06-24 Thread famcool
From: Fam Zheng 

Separate vmdk_open by subformats to:
* vmdk_open_vmdk3
* vmdk_open_vmdk4

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  225 ++
 1 files changed, 147 insertions(+), 78 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 71d81bc..8d55e5f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -398,84 +398,26 @@ static int vmdk_parent_open(BlockDriverState *bs)
 return 0;
 }
 
-static int vmdk_open(BlockDriverState *bs, int flags)
+static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
 {
-BDRVVmdkState *s = bs->opaque;
-uint32_t magic;
+int ret;
 int l1_size, i;
-VmdkExtent *extent = NULL;
-
-if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
-goto fail;
-
-magic = be32_to_cpu(magic);
-if (magic == VMDK3_MAGIC) {
-VMDK3Header header;
-s->extents = qemu_mallocz(sizeof(VmdkExtent));
-s->num_extents = 1;
-if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
-!= sizeof(header)) {
-goto fail;
-}
-extent = s->extents;
-extent->flat = false;
-extent->file = bs->file;
-extent->cluster_sectors = le32_to_cpu(header.granularity);
-extent->l2_size = 1 << 9;
-extent->l1_size = 1 << 6;
-extent->sectors = le32_to_cpu(header.disk_sectors);
-extent->end_sector = le32_to_cpu(header.disk_sectors);
-extent->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
-extent->l1_backup_table_offset = 0;
-extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
-} else if (magic == VMDK4_MAGIC) {
-VMDK4Header header;
-s->extents = qemu_mallocz(sizeof(VmdkExtent));
-s->num_extents = 1;
-if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
-!= sizeof(header)) {
-goto fail;
-}
-extent = s->extents;
-extent->file = bs->file;
-extent->sectors = le64_to_cpu(header.capacity);
-extent->end_sector = le64_to_cpu(header.capacity);
-extent->cluster_sectors = le64_to_cpu(header.granularity);
-extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
-extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
-if (extent->l1_entry_sectors <= 0) {
-goto fail;
-}
-extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
-/ extent->l1_entry_sectors;
-extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
-extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
-
-// try to open parent images, if exist
-if (vmdk_parent_open(bs) != 0)
-goto fail;
-// write the CID once after the image creation
-s->parent_cid = vmdk_read_cid(bs,1);
-} else {
-goto fail;
-}
-
-/* sum up the total sectors */
-bs->total_sectors = 0;
-for (i = 0; i < s->num_extents; i++) {
-bs->total_sectors += s->extents[i].sectors;
-}
 
 /* read the L1 table */
 l1_size = extent->l1_size * sizeof(uint32_t);
 extent->l1_table = qemu_malloc(l1_size);
-if (bdrv_pread(bs->file,
-extent->l1_table_offset,
-extent->l1_table,
-l1_size)
-!= l1_size) {
+if (!extent->l1_table) {
+ret = -ENOMEM;
 goto fail;
 }
+ret = bdrv_pread(bs->file,
+extent->l1_table_offset,
+extent->l1_table,
+l1_size);
+if (ret != l1_size) {
+ret = ret < 0 ? ret : -EIO;
+goto fail_l1;
+}
 for (i = 0; i < extent->l1_size; i++) {
 le32_to_cpus(&extent->l1_table[i]);
 }
@@ -490,21 +432,148 @@ static int vmdk_open(BlockDriverState *bs, int flags)
 goto fail;
 }
 for (i = 0; i < extent->l1_size; i++) {
+if (!extent->l1_backup_table) {
+ret = -ENOMEM;
+goto fail_l1;
+}
+}
+ret = bdrv_pread(bs->file,
+extent->l1_backup_table_offset,
+extent->l1_backup_table,
+l1_size);
+if (ret != l1_size) {
+ret = ret < 0 ? ret : -EIO;
+goto fail;
+}
+for (i = 0; i < extent->l1_size; i++) {
 le32_to_cpus(&extent->l1_backup_table[i]);
 }
 }
 
 extent->l2_cache =
 qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
+if (!extent->l2_cache) {
+ret = -ENOMEM;
+goto fail_l1b;
+}
 return 0;
+ fail_l1b:
+qemu_free(extent->l1_backup_table);
+ fail_l1:
+qemu_free(extent->l1_table);
  fail:
-for (i = 0; i < s->num_extents; i++) {
-qemu_free(s->extents[i].l1_backup_table);
-qemu_free(s->extents[i].l1_tab

[Qemu-devel] [V11 14/15] virtio-9p: readlink in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 


Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   17 ++---
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |   14 --
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index c4ebb96..0b87017 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -367,9 +367,6 @@ int v9fs_chroot(FsContext *fs_ctx)
 case T_CHOWN:
 retval = chroot_do_chown(&request);
 break;
-default:
-retval = -1;
-break;
 case T_LSTAT:
 buff = qemu_malloc(request.data.size);
 retval = lstat(request.path.path, (struct stat *)buff);
@@ -377,6 +374,19 @@ int v9fs_chroot(FsContext *fs_ctx)
 retval = -errno;
 }
 break;
+case T_READLINK:
+buff = qemu_malloc(request.data.size);
+retval = readlink(request.path.path, buff, request.data.size);
+if (retval < 0) {
+retval = -errno;
+} else {
+buff[retval] = '\0';
+retval = 0;
+}
+break;
+default:
+retval = -1;
+break;
 }
 
 /* Send the results */
@@ -394,6 +404,7 @@ int v9fs_chroot(FsContext *fs_ctx)
 chroot_sendfd(chroot_sock, retval, valid_fd);
 break;
 case T_LSTAT:
+case T_READLINK:
 chroot_sendspecial(chroot_sock, buff, request.data.size, retval);
 qemu_free(buff);
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 9ed3f4d..ebf04b5 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -14,6 +14,7 @@
 #define T_CHMOD 9
 #define T_CHOWN 10
 #define T_LSTAT 11
+#define T_READLINK  12
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index eab0854..e865cc8 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -96,6 +96,12 @@ static int passthrough_special_request(FsContext *fs_ctx, 
const char *path,
 request.data.size = size;
 if (type == T_LSTAT) {
 retval = v9fs_special(fs_ctx, &request, buff, size);
+} else if (type == T_READLINK) {
+retval = v9fs_special(fs_ctx, &request, buff, size);
+/* readlink needs to return the number of bytes placed in buff */
+if (retval >= 0) {
+retval = size;
+}
 } else {
 return -1;
 }
@@ -203,6 +209,11 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath 
*fs_path,
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
+if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+return passthrough_special_request(fs_ctx, path, buf, bufsz,
+T_READLINK);
+}
+
 if (fs_ctx->fs_sm == SM_MAPPED) {
 int fd;
 fd = open(rpath(fs_ctx, path, buffer), O_RDONLY);
@@ -214,8 +225,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath 
*fs_path,
 } while (tsize == -1 && errno == EINTR);
 close(fd);
 return tsize;
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 tsize = readlink(rpath(fs_ctx, path, buffer), buf, bufsz);
 }
 return tsize;
-- 
1.7.5.4




Re: [Qemu-devel] Image streaming gives live block copy for free (and vice versa)

2011-06-24 Thread Stefan Hajnoczi
On Sun, Jun 19, 2011 at 5:12 PM, Dor Laor  wrote:
> On 06/17/2011 08:53 AM, Stefan Hajnoczi wrote:
>>
>> Perhaps someone has been saying this all along but I want to spell it
>> out that image streaming and live block copy are equivalent in theory.
>> I just realized this last night.  In practice we might choose one
>> implementation or two different ones for performance reasons.
>>
>> If any of these are unclear please let me know and I'll try to post
>> diagrams.
>>
>> Live block copy using image streaming
>> -
>>
>> 1. Create the destination image file and use the source image as the
>> backing file.
>> 2. Quiesce I/O and pause VM.
>> 3. Switch to destination image.
>> 4. Resume VM.
>> 5. Start streaming destination image in order to copy source image
>> data into destination file.
>> 6. Streaming completes and disables the backing file, leaving the live
>> copied destination image that no longer depends on the source image.
>
> Well streaming is copying just using post-copy approach.
> Both pre and post are required:
>  - post-copy (aka streaming) for fast live block migration of the VM
>   for cpu load balance.
>  - pre-copy (aka live block copy)
>   If you manage to get a network outage between the source and the
>   destination storage systems, you will manage to keep running the VM
>   on the source side.

Out of interest, are you brainstorming using live block copy and image
streaming for pre- and post-copy live migration or do you have
concrete plans to update libvirt to use these mechanisms?

So far I've been focussing on the fast provisioning use case where
image streaming helps, but eventually I would like to improve the
state of storage migration too.

Stefan



[Qemu-devel] [V11 10/15] virtio-9p: Move file post creation changes to none security model

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

After creating a file object, its permission and ownership details are updated
as per 9p client's request for both passthrough and none security model.
But with chrooted environment its not required for passthrough security model.
Move all post file creation changes to none security model.

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-local.c |   25 +
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 864a464..2645da0 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -152,23 +152,16 @@ static int local_set_xattr(const char *path, FsCred 
*credp)
 return 0;
 }
 
-static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
- FsCred *credp)
+static int local_post_create_none(const char *path, FsCred *credp)
 {
-char buffer[PATH_MAX];
+int retval;
 
-if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 0) < 0) {
+if (chmod(path, credp->fc_mode & 0) < 0) {
 return -1;
 }
-if (lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
-credp->fc_gid) < 0) {
-/*
- * If we fail to change ownership and if we are
- * using security model none. Ignore the error
- */
-if (fs_ctx->fs_sm != SM_NONE) {
-return -1;
-}
+retval = lchown(path, credp->fc_uid, credp->fc_gid);
+if (retval < 0) {
+/* Ignore return value for none security model */
 }
 return 0;
 }
@@ -333,7 +326,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath 
*dir_path,
 if (err == -1) {
 goto out;
 }
-err = local_post_create_passthrough(fs_ctx, path, credp);
+err = local_post_create_none(buffer, credp);
 if (err == -1) {
 serrno = errno;
 goto err_end;
@@ -381,7 +374,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath 
*dir_path,
 if (err == -1) {
 goto out;
 }
-err = local_post_create_passthrough(fs_ctx, path, credp);
+err = local_post_create_none(buffer, credp);
 if (err == -1) {
 serrno = errno;
 goto err_end;
@@ -463,7 +456,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath 
*dir_path, const char *name,
 err = fd;
 goto out;
 }
-err = local_post_create_passthrough(fs_ctx, path, credp);
+err = local_post_create_none(buffer, credp);
 if (err == -1) {
 serrno = errno;
 goto err_end;
-- 
1.7.5.4




[Qemu-devel] [PATCH v2 03/12] VMDK: probe for monolithicFlat images

2011-06-24 Thread famcool
From: Fam Zheng 

Probe as the same behavior as VMware does.
Recognize image as monolithicFlat descriptor file when the file is text
and the first effective line (not '#' leaded comment or space line) is
either 'version=1' or 'version=2'. No space or upper case charactors
accepted.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   37 ++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 0540ec5..71d81bc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -102,10 +102,41 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 0;
 magic = be32_to_cpu(*(uint32_t *)buf);
 if (magic == VMDK3_MAGIC ||
-magic == VMDK4_MAGIC)
+magic == VMDK4_MAGIC) {
 return 100;
-else
-return 0;
+} else {
+const char *p = (const char *)buf;
+const char *end = p + buf_size;
+int version = 0;
+while (*p && p < end) {
+if (*p == '#') {
+/* skip comment line */
+while (*p != '\n' && p < end) {
+p++;
+}
+p++;
+continue;
+}
+if (*p == ' ') {
+while (*p == ' ' && p < end) {
+p++;
+}
+/* only accept blank lines before 'version=' line */
+if (*p != '\n') {
+return 0;
+}
+p++;
+continue;
+}
+sscanf(p, "version=%d", &version);
+if (version == 1 || version == 2) {
+return 100;
+} else {
+return 0;
+}
+}
+}
+return 0;
 }
 
 #define CHECK_CID 1



[Qemu-devel] [V11 05/15] virtio-9p: Support for opening a file in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

This patch adds both chroot worker and qemu side support to open a file/
directory in the chroot environment

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot.c |   29 
 hw/9pfs/virtio-9p-chroot.h |2 +-
 hw/9pfs/virtio-9p-local.c  |   78 ++--
 3 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index 63de410..f5b3abc 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -91,13 +91,26 @@ static int v9fs_write_request(int sockfd, 
V9fsFileObjectRequest *request)
 return 0;
 }
 
-/*
- * This patch adds v9fs_receivefd and v9fs_write_request functions,
- * but there is no caller. To avoid compiler warning message,
- * refer these two functions
- */
-void chroot_dummy(void)
+/* Return opened file descriptor on success or -errno on error */
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request)
 {
-(void)v9fs_receivefd;
-(void)v9fs_write_request;
+int fd, sock_error;
+qemu_mutex_lock(&fs_ctx->chroot_mutex);
+if (fs_ctx->chroot_socket == -1) {
+goto error;
+}
+if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
+goto error;
+}
+fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
+if (fd < 0 && sock_error) {
+goto error;
+}
+qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+return fd;
+error:
+close(fs_ctx->chroot_socket);
+fs_ctx->chroot_socket = -1;
+qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+return -EIO;
 }
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index a817bcf..326238d 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -35,6 +35,6 @@ typedef struct V9fsFileObjectRequest
 } V9fsFileObjectRequest;
 
 int v9fs_chroot(FsContext *fs_ctx);
-void chroot_dummy(void);
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 1b6c323..2ae4317 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -14,6 +14,9 @@
 #include "hw/virtio.h"
 #include "virtio-9p.h"
 #include "virtio-9p-xattr.h"
+#include "qemu_socket.h"
+#include "fsdev/qemu-fsdev.h"
+#include "virtio-9p-chroot.h"
 #include 
 #include 
 #include 
@@ -21,6 +24,63 @@
 #include 
 #include 
 
+/* Helper routine to fill V9fsFileObjectRequest structure */
+static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
+const char *oldpath, const char *path, int flags,
+FsCred *credp, int type)
+{
+if (oldpath && strlen(oldpath) >= PATH_MAX) {
+return -ENAMETOOLONG;
+}
+/* path can't be NULL */
+if (!path) {
+return -EFAULT;
+}
+
+if (strlen(path) >= PATH_MAX) {
+return -ENAMETOOLONG;
+}
+strcpy(request->path.path, path);
+if (oldpath) {
+strcpy(request->path.old_path, oldpath);
+} else {
+request->path.old_path[0] = '\0';
+}
+
+memset(&request->data, 0, sizeof(request->data));
+if (credp) {
+request->data.mode = credp->fc_mode;
+request->data.uid = credp->fc_uid;
+request->data.gid = credp->fc_gid;
+request->data.dev = credp->fc_rdev;
+}
+
+request->data.flags = flags;
+request->data.type = type;
+return 0;
+}
+
+static int passthrough_request(FsContext *fs_ctx, const char *old_path,
+const char *path, int flags, FsCred *credp, int type)
+{
+V9fsFileObjectRequest request;
+int retval;
+
+retval = fill_fileobjectrequest(&request, old_path, path, flags, credp,
+type);
+if (retval < 0) {
+errno = -retval;
+return -1;
+}
+
+retval = v9fs_request(fs_ctx, &request);
+if (retval < 0) {
+errno = -retval;
+retval = -1;
+}
+return retval;
+}
+
 static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat 
*stbuf)
 {
 int err;
@@ -153,7 +213,11 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path, 
int flags)
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
-return open(rpath(ctx, path, buffer), flags);
+if (ctx->fs_sm == SM_PASSTHROUGH) {
+return passthrough_request(ctx, NULL, path, flags, NULL, T_OPEN);
+} else {
+return open(rpath(ctx, path, buffer), flags);
+}
 }
 
 static DIR *local_opendir(FsContext *ctx, V9fsPath *fs_path)
@@ -161,7 +225,16 @@ static DIR *local_opendir(FsContext *ctx, V9fsPath 
*fs_path)
 char buffer[PATH_MAX];
 char *path = fs_path->data;
 
-return opendir(rpath(ctx, path, buffer));
+if (ctx->fs_sm == SM_PASSTHROUGH) {
+int fd;
+fd = passthrough_request(ctx, NULL, path, O_DIRECTORY, NULL, T_OPEN);
+if (fd < 0) {
+return NULL;
+}
+return fdopendir(fd);
+

Re: [Qemu-devel] [PATCH v2 00/12] Adding VMDK monolithic flat support

2011-06-24 Thread Fam Zheng
On Fri, Jun 24, 2011 at 4:18 PM,   wrote:
> From: Fam Zheng 
>
> VMDK multiple file images can not be recognized for now. This patch series is
> adding monolithic flat support to it, that is the image type with two
> files, one text descriptor file and a plain data file. This type of
> image can be created in VMWare, with the options "allocate all disk
> space now" and "store virtual disk as a single file" checked.
>
> A VmdkExtent structure is introduced to hold the image "extent"
> information, which makes further adding multi extents support of VMDK
> easy. An image creating option "flat" is added for creating flat
> (preallocated) image.
Oops, this comment is obsolete, the flag is adjusted to a string
option "format". See patch [10/12].
>
> Fam Zheng (12):
>  VMDK: introduce VmdkExtent
>  VMDK: bugfix, align offset to cluster in get_whole_cluster
>  VMDK: probe for monolithicFlat images
>  VMDK: separate vmdk_open by format version
>  VMDK: add field BDRVVmdkState.desc_offset
>  VMDK: flush multiple extents
>  VMDK: move 'static' cid_update flag to bs field
>  VMDK: change get_cluster_offset return type
>  VMDK: open/read/write for monolithicFlat image
>  VMDK: create different subformats
>  VMDK: fix coding style
>  BlockDriver: add bdrv_get_allocated_file_size() operation
>
>  block.c           |   19 +
>  block.h           |    1 +
>  block/raw-posix.c |   21 +
>  block/raw-win32.c |   29 ++
>  block/vmdk.c      | 1360 
> +
>  block_int.h       |    2 +
>  qemu-img.c        |   31 +--
>  7 files changed, 1026 insertions(+), 437 deletions(-)
>
>



-- 
Best regards!
Fam Zheng



Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Laurent Desnogues
On Fri, Jun 24, 2011 at 10:35 AM, Max Filippov  wrote:
[...]
> Yes, I've noticed it (however, after I sent this mail).
> But (1) quoted OUT is the last OUT for this host address range in the log and 
> (2) in gdb I set "b tlb_fill if retaddr == 0x4000a369" and made some steps.
> You mean that I should look at previous OUTs for this address range?

That should be the last block matching the address in the
log output if you run *under gdb* with -d out_asm.

BTW you say this is a clean build, but as far as I could see
it looks like your emulated target is not part of standard
QEMU;  it seems to have a register named 'ar9'.  Did I
miss something or is it some non public target? ia64?


Laurent



[Qemu-devel] [V11 09/15] virtio-9p: Rename in chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

Support renaming a file or directory in chroot envirnoment. Add
interfaces for renaming in chroot worker and qemu side.

Signed-off-by: M. Mohan Kumar 
---
 hw/9pfs/virtio-9p-chroot-worker.c |   17 +
 hw/9pfs/virtio-9p-chroot.h|1 +
 hw/9pfs/virtio-9p-local.c |3 +++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c 
b/hw/9pfs/virtio-9p-chroot-worker.c
index f269c7b..9bb94f2 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -184,6 +184,20 @@ static int chroot_do_remove(V9fsFileObjectRequest *request)
 return 0;
 }
 
+/*
+ * Rename a file object
+ */
+static int chroot_do_rename(V9fsFileObjectRequest *request)
+{
+int retval;
+
+retval = rename(request->path.old_path, request->path.path);
+if (retval < 0) {
+return -errno;
+}
+return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
 sigset_t sigset;
@@ -290,6 +304,9 @@ int v9fs_chroot(FsContext *fs_ctx)
 case T_REMOVE:
 retval = chroot_do_remove(&request);
 break;
+case T_RENAME:
+retval = chroot_do_rename(&request);
+break;
 default:
 retval = -1;
 break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 9d69739..fda60af 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -10,6 +10,7 @@
 #define T_SYMLINK   5
 #define T_LINK  6
 #define T_REMOVE7
+#define T_RENAME8
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index f2d66e1..864a464 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -582,6 +582,9 @@ static int local_rename(FsContext *ctx, const char *oldpath,
 {
 char buffer[PATH_MAX], buffer1[PATH_MAX];
 
+if (ctx->fs_sm == SM_PASSTHROUGH) {
+return passthrough_request(ctx, oldpath, newpath, 0, NULL, T_RENAME);
+}
 return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath, buffer1));
 }
 
-- 
1.7.5.4




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Max Filippov
> > Please note how the current instruction in gdb differ from what
> > was said in OUT. This lea corrupts stack pointer and the next
> > callq generates segfault.
> > Could please anyone familiar with TCG take a look at this, or
> > suggest where I should look myself?
> 
> You don't say which target you're compiling code for, or what
> the input assembly was which triggered this.

I thought it doesn't matter. It's target-xtensa that I've been developing, 
input assembly is the following:

d0c0 <_WindowUnderflow8>:
d0c0:   09d910  l32ea1, a9, -12
d0c3:   09c900  l32ea0, a9, -16
d0c6:   09d170  l32ea7, a1, -12
d0c9:   09e920  l32ea2, a9, -8
d0cc:   09f930  l32ea3, a9, -4
d0cf:   098740  l32ea4, a7, -32
d0d2:   099750  l32ea5, a7, -28
d0d5:   09a760  l32ea6, a7, -24
d0d8:   09b770  l32ea7, a7, -20
d0db:   003500  rfwu

> My first guess is that the target's front end might have a bug
> where it wrongly bakes in assumptions about bits of the CPUState.
> QEMU will occasionally retranslate-in-place a TB (if a load in
> the TB causes an exception) so if the frontend generates different
> code the second time around things will go wrong...
> 
> You should be able to find out what's stomping on the code
> with the aid of a debugger and some watchpoints.

I just thought that "lea-0x10(%rbx),%esp" may not appear in generated code 
at all, and in the OUT section (which is for different MMU mode, as I can see 
now) it is "lea-0x10(%rbx),%r12d".
The instruction itself looks odd: it writes to esp and the sizes of the 
registers it operates on are different.

Thanks.
-- Max



Re: [Qemu-devel] [PATCH v2 03/12] VMDK: probe for monolithicFlat images

2011-06-24 Thread Stefan Hajnoczi
On Fri, Jun 24, 2011 at 9:18 AM,   wrote:
> +        const char *p = (const char *)buf;
> +        const char *end = p + buf_size;
> +        int version = 0;
> +        while (*p && p < end) {

These short-circuit comparisions need to be the other way around.  If
p >= end then p is out-of-range and we cannot dereference it.
Therefore:
while (p < end && *p) {

Although what is the point of checking for the NUL byte?  This buffer
isn't a C string and NULs don't matter.

Please fix the other loops too.

> +            if (*p == '#') {
> +                /* skip comment line */
> +                while (*p != '\n' && p < end) {
> +                    p++;
> +                }
> +                p++;
> +                continue;
> +            }
> +            if (*p == ' ') {
> +                while (*p == ' ' && p < end) {
> +                    p++;
> +                }
> +                /* only accept blank lines before 'version=' line */
> +                if (*p != '\n') {
> +                    return 0;
> +                }

What about Windows line endings (\r\n)?

> +                p++;
> +                continue;
> +            }
> +            sscanf(p, "version=%d", &version);

This function cannot be used on p because the buffer is not
NUL-terminated.  sscanf(3) may run off the end of the buffer.

How about this:

/* Match supported versions, Windows and UNIX line endings */
if (strncmp("version=1\r\n", p, end - p) == 0 ||
strncmp("version=1\n", p, end - p) == 0 ||
strncmp("version=2\r\n", p, end - p) == 0) ||
strncmp("version=2\n", p, end - p) == 0) {
return 100;
}

Stefan



[Qemu-devel] [V11 04/15] virtio-9p: qemu interfaces for chroot environment

2011-06-24 Thread M. Mohan Kumar
From: "M. Mohan Kumar" 

QEMU side interfaces to communicate with chroot worker process.

Signed-off-by: M. Mohan Kumar 
[mala...@us.ibm.com: Handle when qemu process can not receive fd because
it already reached max fds]
---
 Makefile.objs  |2 +-
 hw/9pfs/virtio-9p-chroot.c |  103 
 hw/9pfs/virtio-9p-chroot.h |1 +
 3 files changed, 105 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot.c

diff --git a/Makefile.objs b/Makefile.objs
index 588eae2..9ff332f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -304,7 +304,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o virtio-9p-handle.o
-9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-worker.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-worker.o virtio-9p-chroot.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
new file mode 100644
index 000..63de410
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -0,0 +1,103 @@
+/*
+ * Virtio 9p chroot environment for contained access to exported path
+ * Code handles qemu side interfaces to communicate with chroot worker process
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * M. Mohan Kumar 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the copying file in the top-level directory
+ *
+ */
+
+#include 
+#include 
+#include 
+#include "qemu_socket.h"
+#include "qemu-thread.h"
+#include "qerror.h"
+#include "virtio-9p.h"
+#include "virtio-9p-chroot.h"
+
+/*
+ * Return received file descriptor on success and -errno on failure.
+ * sock_error is set to 1 whenever there is error in socket IO
+ */
+static int v9fs_receivefd(int sockfd, int *sock_error)
+{
+struct msghdr msg = { };
+struct iovec iov;
+union MsgControl msg_control;
+struct cmsghdr *cmsg;
+int retval, data, fd;
+
+iov.iov_base = &data;
+iov.iov_len = sizeof(data);
+
+*sock_error = 0;
+memset(&msg, 0, sizeof(msg));
+msg.msg_iov = &iov;
+msg.msg_iovlen = 1;
+msg.msg_control = &msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+do {
+retval = recvmsg(sockfd, &msg, 0);
+} while (retval < 0 && errno == EINTR);
+if (retval <= 0) {
+*sock_error = 1;
+return -EIO;
+}
+
+/*
+ * data is set to V9FS_FD_VALID, if ancillary data is sent.  If this
+ * request doesn't need ancillary data (fd) or an error occurred,
+ * data is set to negative errno value.
+ */
+if (data != V9FS_FD_VALID) {
+return data;
+}
+
+/*
+ * File descriptor (fd) is sent in the ancillary data. Check if we
+ * indeed received it. One of the reasons to fail to receive it is if
+ * we exceeded the maximum number of file descriptors!
+ */
+for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+cmsg->cmsg_level != SOL_SOCKET ||
+cmsg->cmsg_type != SCM_RIGHTS) {
+continue;
+}
+fd = *((int *)CMSG_DATA(cmsg));
+return fd;
+}
+
+return -ENFILE; /* Ancillary data sent but not received */
+}
+
+/*
+ * V9fsFileObjectRequest is written into the socket by QEMU process.
+ * Then this request is read by chroot process using v9fs_read_request function
+ */
+static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
+{
+int retval;
+retval = qemu_write_full(sockfd, request, sizeof(*request));
+if (retval != sizeof(*request)) {
+return -EIO;
+}
+return 0;
+}
+
+/*
+ * This patch adds v9fs_receivefd and v9fs_write_request functions,
+ * but there is no caller. To avoid compiler warning message,
+ * refer these two functions
+ */
+void chroot_dummy(void)
+{
+(void)v9fs_receivefd;
+(void)v9fs_write_request;
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index c2a4a6e..a817bcf 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -35,5 +35,6 @@ typedef struct V9fsFileObjectRequest
 } V9fsFileObjectRequest;
 
 int v9fs_chroot(FsContext *fs_ctx);
+void chroot_dummy(void);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
-- 
1.7.5.4




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Max Filippov
> > Hello guys.
> >
> > I'm running qemu on x86_64 host.
> > It's clean build from git sources dated 2011.05.19, commit 
> > 1fddfba129f5435c80eda14e8bc23fdb888c7187
> > I have the following output from "log trace,op,out_asm":
> >
> > Trace 0x4000a310 [d0026c92]
> > OP:
> >   0xd0c0
> >  movi_i32 tmp1,$0xfff4
> >  add_i32 tmp0,ar9,tmp1
> >  qemu_ld32 ar1,tmp0,$0x0
> >
> >   0xd0c3
> >  movi_i32 tmp1,$0xfff0
> >  add_i32 tmp0,ar9,tmp1
> >  qemu_ld32 ar0,tmp0,$0x0
> >
> > [...snip...]
> [...]
> > 0x4000a360:  xor%esi,%esi
> > 0x4000a362:  callq  0x52edc2
> [...]
> > (gdb) x/25i 0x4000a330
> [...]
> >   0x4000a360:  mov$0x1,%esi
> >   0x4000a365:  callq  0x52edc2 <__ldl_mmu>
> >   0x4000a36a:  mov%eax,%ebp
> >   0x4000a36c:  sub$0x44,%al
> > => 0x4000a36e:  lea-0x10(%rbx),%esp
> >   0x4000a371:  mov%ebp,0xc(%r14)
> >   0x4000a375:  mov%r12d,%esi
> >   0x4000a378:  mov%r12d,%edi
> >
> > Please note how the current instruction in gdb differ from what was said in 
> > OUT. This lea corrupts stack pointer and the next callq generates segfault.
> > Could please anyone familiar with TCG take a look at this, or suggest where 
> > I should look myself?
> 
> As Peter hinted, you're not looking at the code you think :-)
> Note how your original TCG code does loads:
> 
>qemu_ld32 ar1,tmp0,$0x0
> 
> That $0x0 will end up in %RSI.  It's the mem index used to
> distinguish from user and privileged level accesses.  In your
> examples of host code, in one case it is 0 and in the other
> it is 1, so you're definitely not really looking at the same
> block in the same running conditions.

Yes, I've noticed it (however, after I sent this mail).
But (1) quoted OUT is the last OUT for this host address range in the log and 
(2) in gdb I set "b tlb_fill if retaddr == 0x4000a369" and made some steps.
You mean that I should look at previous OUTs for this address range?

Thanks.
-- Max



Re: [Qemu-devel] [PATCH v2 06/12] VMDK: flush multiple extents

2011-06-24 Thread Stefan Hajnoczi
On Fri, Jun 24, 2011 at 9:18 AM,   wrote:
> From: Fam Zheng 
>
> Flush all the file that referenced by the image.
>
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |    9 -
>  1 files changed, 8 insertions(+), 1 deletions(-)

Please see my comments on the previous version of this patch.

Stefan



Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix compilation for non-POSIX system (w64)

2011-06-24 Thread Stefan Hajnoczi
On Thu, Jun 23, 2011 at 09:19:53PM +0200, Stefan Weil wrote:
> compatfd.h is only needed with CONFIG_POSIX.
> Compilation fails in compatfd.h for at least one non-POSIX systems (w64).
> 
> Signed-off-by: Stefan Weil 
> ---
>  cpus.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Thanks!  Jan's patch which is similar but uses #ifndef _WIN32 is already
queued.  The cpus.c qemu_signalfd() usage is #ifndef _WIN32, so I think
there's a logic to keeping that patch.

Stefan



Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Max Filippov
> Here are my rules of thumb for generating code where the code
> generated might change based on some bit of CPU state:
>
> When you are generating code, if the code you generate will
> change based on the contents of something in the CPUState struct,
> then the bit of CPUState you are looking at has to be one of:
>  (1) encoded in the TB flags (or tb->pc or tb->cs_base)
>      (and gen_intermediate_code_internal() must read and
>      use the value in tb->tb_flags, not the one in env)

So if changing a bit of context does not cause TB invalidation then it
must be captured in cpu_get_tb_cpu_state and
gen_intermediate_code_internal must use that captured value?

>  (2) always constant for the life of the simulation
>      (eg env->features if you have some sort of
>      "target feature support" flags)
>  (3) specially handled so that when it changes then
>      all TBs or at least all affected TBs are flushed
>      (env->breakpoints is in this category), and also
>      if the change is the result of some instruction then
>      you must terminate the TB after that instruction.
>      This is often used for things that rarely change and/or
>      where you can't fit the whole value into tb_flags.
> The reason for this is that the CPUState you're passed in
> is not guaranteed to be the state of the CPU at the PC
> which you are starting translation from.
>
> This is the xtensa port at
> http://jcmvbkbc.spb.ru/git/?p=dumb/qemu-xtensa.git;a=shortlog;h=refs/heads/xtensa
> right?

Yes.

> It looks like you're breaking these rules with a lot of
> the fields in your DisasContext. (Most obviously, you
> need to translate code from tb->pc, not env->pc, and
> xtensa_get_ring() and xtensa_get_cring() should not read
> from env->sregs[PS]. But you should be clear for every
> field in DisasContext which category it falls into.)

Thank you Peter. Will go rearrange that mess.

-- 
Thanks.
-- Max



[Qemu-devel] [PATCH 03/13] usb-linux: track inflight iso urb count

2011-06-24 Thread Gerd Hoffmann
Track the number of iso urbs which are currently in flight.
Log a message in case the count goes down to zero.  Also
warn in case many urbs are returned at the same time.

Signed-off-by: Gerd Hoffmann 
---
 usb-linux.c |   26 +-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index f65dcb7..42baafe 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -100,6 +100,7 @@ struct endp_data {
 int iso_urb_idx;
 int iso_buffer_used;
 int max_packet_size;
+int inflight;
 };
 
 struct USBAutoFilter {
@@ -184,7 +185,19 @@ static void clear_iso_started(USBHostDevice *s, int ep)
 
 static void set_iso_started(USBHostDevice *s, int ep)
 {
-get_endp(s, ep)->iso_started = 1;
+struct endp_data *e = get_endp(s, ep);
+if (!e->iso_started) {
+e->iso_started = 1;
+e->inflight = 0;
+}
+}
+
+static int change_iso_inflight(USBHostDevice *s, int ep, int value)
+{
+struct endp_data *e = get_endp(s, ep);
+
+e->inflight += value;
+return e->inflight;
 }
 
 static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
@@ -282,6 +295,7 @@ static void async_complete(void *opaque)
 {
 USBHostDevice *s = opaque;
 AsyncURB *aurb;
+int urbs = 0;
 
 while (1) {
 USBPacket *p;
@@ -289,6 +303,9 @@ static void async_complete(void *opaque)
 int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
 if (r < 0) {
 if (errno == EAGAIN) {
+if (urbs > 2) {
+fprintf(stderr, "husb: %d iso urbs finished at once\n", 
urbs);
+}
 return;
 }
 if (errno == ENODEV && !s->closing) {
@@ -306,10 +323,16 @@ static void async_complete(void *opaque)
 /* If this is a buffered iso urb mark it as complete and don't do
anything else (it is handled further in usb_host_handle_iso_data) */
 if (aurb->iso_frame_idx == -1) {
+int inflight;
 if (aurb->urb.status == -EPIPE) {
 set_halt(s, aurb->urb.endpoint & 0xf);
 }
 aurb->iso_frame_idx = 0;
+urbs++;
+inflight = change_iso_inflight(s, aurb->urb.endpoint & 0xf, -1);
+if (inflight == 0 && is_iso_started(s, aurb->urb.endpoint & 0xf)) {
+fprintf(stderr, "husb: out of buffers for iso stream\n");
+}
 continue;
 }
 
@@ -670,6 +693,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, 
USBPacket *p, int in)
 break;
 }
 aurb[i].iso_frame_idx = -1;
+change_iso_inflight(s, p->devep, +1);
 }
 }
 }
-- 
1.7.1




[Qemu-devel] [PATCH 02/13] usb-linux: make iso urb count contigurable

2011-06-24 Thread Gerd Hoffmann
Add a qdev property for the number of iso urbs which
usb-linux keeps in flight, so it can be configured at
runtime.  Make it default to four (old hardcoded value
used to be three).

Signed-off-by: Gerd Hoffmann 
---
 usb-linux.c |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 3c6156a..f65dcb7 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -85,7 +85,6 @@ static int usb_fs_type;
 
 /* endpoint association data */
 #define ISO_FRAME_DESC_PER_URB 32
-#define ISO_URB_COUNT 3
 #define INVALID_EP_TYPE 255
 
 /* devio.c limits single requests to 16k */
@@ -120,6 +119,7 @@ typedef struct USBHostDevice {
 int   configuration;
 int   ninterfaces;
 int   closing;
+uint32_t  iso_urb_count;
 Notifier  exit;
 
 struct endp_data endp_table[MAX_ENDPOINTS];
@@ -505,8 +505,8 @@ static AsyncURB *usb_host_alloc_iso(USBHostDevice *s, 
uint8_t ep, int in)
 AsyncURB *aurb;
 int i, j, len = get_max_packet_size(s, ep);
 
-aurb = qemu_mallocz(ISO_URB_COUNT * sizeof(*aurb));
-for (i = 0; i < ISO_URB_COUNT; i++) {
+aurb = qemu_mallocz(s->iso_urb_count * sizeof(*aurb));
+for (i = 0; i < s->iso_urb_count; i++) {
 aurb[i].urb.endpoint  = ep;
 aurb[i].urb.buffer_length = ISO_FRAME_DESC_PER_URB * len;
 aurb[i].urb.buffer= qemu_malloc(aurb[i].urb.buffer_length);
@@ -536,7 +536,7 @@ static void usb_host_stop_n_free_iso(USBHostDevice *s, 
uint8_t ep)
 return;
 }
 
-for (i = 0; i < ISO_URB_COUNT; i++) {
+for (i = 0; i < s->iso_urb_count; i++) {
 /* in flight? */
 if (aurb[i].iso_frame_idx == -1) {
 ret = ioctl(s->fd, USBDEVFS_DISCARDURB, &aurb[i]);
@@ -554,7 +554,7 @@ static void usb_host_stop_n_free_iso(USBHostDevice *s, 
uint8_t ep)
 async_complete(s);
 }
 
-for (i = 0; i < ISO_URB_COUNT; i++) {
+for (i = 0; i < s->iso_urb_count; i++) {
 qemu_free(aurb[i].urb.buffer);
 }
 
@@ -639,7 +639,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, 
USBPacket *p, int in)
 }
 aurb[i].iso_frame_idx++;
 if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
-i = (i + 1) % ISO_URB_COUNT;
+i = (i + 1) % s->iso_urb_count;
 set_iso_urb_idx(s, p->devep, i);
 }
 } else {
@@ -652,7 +652,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, 
USBPacket *p, int in)
 
 if (is_iso_started(s, p->devep)) {
 /* (Re)-submit all fully consumed / filled urbs */
-for (i = 0; i < ISO_URB_COUNT; i++) {
+for (i = 0; i < s->iso_urb_count; i++) {
 if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
 ret = ioctl(s->fd, USBDEVFS_SUBMITURB, &aurb[i]);
 if (ret < 0) {
@@ -1233,6 +1233,7 @@ static struct USBDeviceInfo usb_host_dev_info = {
 DEFINE_PROP_STRING("hostport", USBHostDevice, match.port),
 DEFINE_PROP_HEX32("vendorid",  USBHostDevice, match.vendor_id,  0),
 DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0),
+DEFINE_PROP_UINT32("isobufs",  USBHostDevice, iso_urb_count,4),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
-- 
1.7.1




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Peter Maydell
On 24 June 2011 09:34, Max Filippov  wrote:
> I thought it doesn't matter. It's target-xtensa that I've been
> developing

Ah...

>> My first guess is that the target's front end might have a bug
>> where it wrongly bakes in assumptions about bits of the CPUState.
>> QEMU will occasionally retranslate-in-place a TB (if a load in
>> the TB causes an exception) so if the frontend generates different
>> code the second time around things will go wrong...
>>
>> You should be able to find out what's stomping on the code
>> with the aid of a debugger and some watchpoints.
>
> I just thought that "lea    -0x10(%rbx),%esp" may not appear
> in generated code at all, and in the OUT section (which is for
> different MMU mode, as I can see now) it is
> "lea    -0x10(%rbx),%r12d".
> The instruction itself looks odd: it writes to esp and the sizes
> of the registers it operates on are different.

Yes, typically when we retranslate code in place then the second
time around we will stop before the end of the TB, so if there
is a mismatch in the generated code this usually manifests as
a corrupted instruction where half of it got rewritten.

Here are my rules of thumb for generating code where the code
generated might change based on some bit of CPU state:

When you are generating code, if the code you generate will
change based on the contents of something in the CPUState struct,
then the bit of CPUState you are looking at has to be one of:
 (1) encoded in the TB flags (or tb->pc or tb->cs_base)
  (and gen_intermediate_code_internal() must read and
  use the value in tb->tb_flags, not the one in env)
 (2) always constant for the life of the simulation
  (eg env->features if you have some sort of
  "target feature support" flags)
 (3) specially handled so that when it changes then
  all TBs or at least all affected TBs are flushed
  (env->breakpoints is in this category), and also
  if the change is the result of some instruction then
  you must terminate the TB after that instruction.
  This is often used for things that rarely change and/or
  where you can't fit the whole value into tb_flags.

The reason for this is that the CPUState you're passed in
is not guaranteed to be the state of the CPU at the PC
which you are starting translation from.

This is the xtensa port at
http://jcmvbkbc.spb.ru/git/?p=dumb/qemu-xtensa.git;a=shortlog;h=refs/heads/xtensa
right?

It looks like you're breaking these rules with a lot of
the fields in your DisasContext. (Most obviously, you
need to translate code from tb->pc, not env->pc, and
xtensa_get_ring() and xtensa_get_cring() should not read
from env->sregs[PS]. But you should be clear for every
field in DisasContext which category it falls into.)

-- PMM



[Qemu-devel] [PATCH 01/13] usb-linux: add get_endp()

2011-06-24 Thread Gerd Hoffmann
Add a helper function to get the endpoint data structure
and put it into use.

Signed-off-by: Gerd Hoffmann 
---
 usb-linux.c |   39 +--
 1 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 5d2ec5c..3c6156a 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -142,74 +142,79 @@ static void usb_host_auto_check(void *unused);
 static int usb_host_read_file(char *line, size_t line_size,
 const char *device_file, const char *device_name);
 
+static struct endp_data *get_endp(USBHostDevice *s, int ep)
+{
+return s->endp_table + ep - 1;
+}
+
 static int is_isoc(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].type == USBDEVFS_URB_TYPE_ISO;
+return get_endp(s, ep)->type == USBDEVFS_URB_TYPE_ISO;
 }
 
 static int is_valid(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].type != INVALID_EP_TYPE;
+return get_endp(s, ep)->type != INVALID_EP_TYPE;
 }
 
 static int is_halted(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].halted;
+return get_endp(s, ep)->halted;
 }
 
 static void clear_halt(USBHostDevice *s, int ep)
 {
-s->endp_table[ep - 1].halted = 0;
+get_endp(s, ep)->halted = 0;
 }
 
 static void set_halt(USBHostDevice *s, int ep)
 {
-s->endp_table[ep - 1].halted = 1;
+get_endp(s, ep)->halted = 1;
 }
 
 static int is_iso_started(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].iso_started;
+return get_endp(s, ep)->iso_started;
 }
 
 static void clear_iso_started(USBHostDevice *s, int ep)
 {
-s->endp_table[ep - 1].iso_started = 0;
+get_endp(s, ep)->iso_started = 0;
 }
 
 static void set_iso_started(USBHostDevice *s, int ep)
 {
-s->endp_table[ep - 1].iso_started = 1;
+get_endp(s, ep)->iso_started = 1;
 }
 
 static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
 {
-s->endp_table[ep - 1].iso_urb = iso_urb;
+get_endp(s, ep)->iso_urb = iso_urb;
 }
 
 static AsyncURB *get_iso_urb(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].iso_urb;
+return get_endp(s, ep)->iso_urb;
 }
 
 static void set_iso_urb_idx(USBHostDevice *s, int ep, int i)
 {
-s->endp_table[ep - 1].iso_urb_idx = i;
+get_endp(s, ep)->iso_urb_idx = i;
 }
 
 static int get_iso_urb_idx(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].iso_urb_idx;
+return get_endp(s, ep)->iso_urb_idx;
 }
 
 static void set_iso_buffer_used(USBHostDevice *s, int ep, int i)
 {
-s->endp_table[ep - 1].iso_buffer_used = i;
+get_endp(s, ep)->iso_buffer_used = i;
 }
 
 static int get_iso_buffer_used(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].iso_buffer_used;
+return get_endp(s, ep)->iso_buffer_used;
 }
 
 static void set_max_packet_size(USBHostDevice *s, int ep, uint8_t *descriptor)
@@ -223,14 +228,12 @@ static void set_max_packet_size(USBHostDevice *s, int ep, 
uint8_t *descriptor)
 case 2:  microframes = 3; break;
 default: microframes = 1; break;
 }
-DPRINTF("husb: max packet size: 0x%x -> %d x %d\n",
-raw, microframes, size);
-s->endp_table[ep - 1].max_packet_size = size * microframes;
+get_endp(s, ep)->max_packet_size = size * microframes;
 }
 
 static int get_max_packet_size(USBHostDevice *s, int ep)
 {
-return s->endp_table[ep - 1].max_packet_size;
+return get_endp(s, ep)->max_packet_size;
 }
 
 /*
-- 
1.7.1




[Qemu-devel] [PATCH 08/13] usb: Add a speedmask to devices

2011-06-24 Thread Gerd Hoffmann
From: Hans de Goede 

This is used to indicate at which speed[s] the device can operate,
so that this can be checked to match the ports capabilities when it gets
attached to a bus.

Note that currently all usb1 emulated device claim to be fullspeed, this
seems to not cause any problems, but still seems wrong, because with real
hardware keyboards, mice and tablets usually are lo-speed, so reporting these
as fullspeed devices seems wrong.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-ccid.c |1 +
 hw/usb-desc.c |   10 ++
 hw/usb.h  |3 +++
 usb-bsd.c |2 ++
 usb-linux.c   |1 +
 5 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c
index 59c6431..30bb4d6 100644
--- a/hw/usb-ccid.c
+++ b/hw/usb-ccid.c
@@ -1271,6 +1271,7 @@ static int ccid_initfn(USBDevice *dev)
 s->migration_target_ip = 0;
 s->migration_target_port = 0;
 s->dev.speed = USB_SPEED_FULL;
+s->dev.speedmask = USB_SPEED_MASK_FULL;
 s->notify_slot_change = false;
 s->powered = true;
 s->pending_answers_num = 0;
diff --git a/hw/usb-desc.c b/hw/usb-desc.c
index e4a4680..0b9d351 100644
--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -242,7 +242,17 @@ static void usb_desc_setdefaults(USBDevice *dev)
 
 void usb_desc_init(USBDevice *dev)
 {
+const USBDesc *desc = dev->info->usb_desc;
+
+assert(desc != NULL);
 dev->speed = USB_SPEED_FULL;
+dev->speedmask = 0;
+if (desc->full) {
+dev->speedmask |= USB_SPEED_MASK_FULL;
+}
+if (desc->high) {
+dev->speedmask |= USB_SPEED_MASK_HIGH;
+}
 usb_desc_setdefaults(dev);
 }
 
diff --git a/hw/usb.h b/hw/usb.h
index 06ce058..a6b311f 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -168,7 +168,10 @@ struct USBDevice {
 char *port_path;
 void *opaque;
 
+/* Actual connected speed */
 int speed;
+/* Supported speeds, not in info because it may be variable (hostdevs) */
+int speedmask;
 uint8_t addr;
 char product_desc[32];
 int auto_attach;
diff --git a/usb-bsd.c b/usb-bsd.c
index c1bcc4a..3b97eb4 100644
--- a/usb-bsd.c
+++ b/usb-bsd.c
@@ -367,8 +367,10 @@ USBDevice *usb_host_device_open(const char *devname)
 
 if (dev_info.udi_speed == 1) {
 dev->dev.speed = USB_SPEED_LOW - 1;
+dev->dev.speedmask = USB_SPEED_MASK_LOW;
 } else {
 dev->dev.speed = USB_SPEED_FULL - 1;
+dev->dev.speedmask = USB_SPEED_MASK_FULL;
 }
 
 if (strncmp(dev_info.udi_product, "product", 7) != 0) {
diff --git a/usb-linux.c b/usb-linux.c
index 9b6f2be..4d22c9c 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1167,6 +1167,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
 }
 }
 dev->dev.speed = speed;
+dev->dev.speedmask = (1 << speed);
 
 printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
 
-- 
1.7.1




[Qemu-devel] [PATCH] w32: Remove redundant definitions of PRI*64

2011-06-24 Thread Stefan Weil
The PRI*64 macros are defined in MinGW's inttypes.h since 2002,
so they are not needed in qemu-common.h (which includes inttypes.h).

Signed-off-by: Stefan Weil 
---
 qemu-common.h |5 -
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/qemu-common.h b/qemu-common.h
index 109498d..abd7a75 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -112,11 +112,6 @@ static inline char *realpath(const char *path, char 
*resolved_path)
 _fullpath(resolved_path, path, _MAX_PATH);
 return resolved_path;
 }
-
-#define PRId64 "I64d"
-#define PRIx64 "I64x"
-#define PRIu64 "I64u"
-#define PRIo64 "I64o"
 #endif
 
 /* FIXME: Remove NEED_CPU_H.  */
-- 
1.7.0.4




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Max Filippov
> That should be the last block matching the address in the
> log output if you run *under gdb* with -d out_asm.

This is the case.

> BTW you say this is a clean build, but as far as I could see
> it looks like your emulated target is not part of standard
> QEMU;  it seems to have a register named 'ar9'.  Did I
> miss something or is it some non public target? ia64?

Clean build w.r.t. TCG/other core code. The target is target-xtensa,
http://jcmvbkbc.spb.ru/git/?p=dumb/qemu-xtensa.git;a=shortlog;h=refs/heads/xtensa

-- 
Thanks.
-- Max



[Qemu-devel] [PATCH 07/13] usb: Proper error propagation for usb_device_attach errors

2011-06-24 Thread Gerd Hoffmann
From: Hans de Goede 

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-bus.c |   25 +
 hw/usb-msd.c |5 +++--
 usb-linux.c  |6 +-
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0a49921..fc72018 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -75,7 +75,7 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
 QLIST_INIT(&dev->strings);
 rc = dev->info->init(dev);
 if (rc == 0 && dev->auto_attach)
-usb_device_attach(dev);
+rc = usb_device_attach(dev);
 return rc;
 }
 
@@ -121,7 +121,7 @@ USBDevice *usb_create(USBBus *bus, const char *name)
 bus = usb_bus_find(-1);
 if (!bus)
 return NULL;
-fprintf(stderr, "%s: no bus specified, using \"%s\" for \"%s\"\n",
+error_report("%s: no bus specified, using \"%s\" for \"%s\"\n",
 __FUNCTION__, bus->qbus.name, name);
 }
 #endif
@@ -171,20 +171,20 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
 bus->nfree--;
 }
 
-static void do_attach(USBDevice *dev)
+static int do_attach(USBDevice *dev)
 {
 USBBus *bus = usb_bus_from_device(dev);
 USBPort *port;
 
 if (dev->attached) {
-fprintf(stderr, "Warning: tried to attach usb device %s twice\n",
+error_report("Error: tried to attach usb device %s twice\n",
 dev->product_desc);
-return;
+return -1;
 }
 if (bus->nfree == 0) {
-fprintf(stderr, "Warning: tried to attach usb device %s to a bus with 
no free ports\n",
+error_report("Error: tried to attach usb device %s to a bus with no 
free ports\n",
 dev->product_desc);
-return;
+return -1;
 }
 if (dev->port_path) {
 QTAILQ_FOREACH(port, &bus->free, next) {
@@ -193,9 +193,9 @@ static void do_attach(USBDevice *dev)
 }
 }
 if (port == NULL) {
-fprintf(stderr, "Warning: usb port %s (bus %s) not found\n",
+error_report("Error: usb port %s (bus %s) not found\n",
 dev->port_path, bus->qbus.name);
-return;
+return -1;
 }
 } else {
 port = QTAILQ_FIRST(&bus->free);
@@ -209,6 +209,8 @@ static void do_attach(USBDevice *dev)
 
 QTAILQ_INSERT_TAIL(&bus->used, port, next);
 bus->nused++;
+
+return 0;
 }
 
 int usb_device_attach(USBDevice *dev)
@@ -220,8 +222,7 @@ int usb_device_attach(USBDevice *dev)
(unless a physical port location is specified). */
 usb_create_simple(bus, "usb-hub");
 }
-do_attach(dev);
-return 0;
+return do_attach(dev);
 }
 
 int usb_device_detach(USBDevice *dev)
@@ -230,7 +231,7 @@ int usb_device_detach(USBDevice *dev)
 USBPort *port;
 
 if (!dev->attached) {
-fprintf(stderr, "Warning: tried to detach unattached usb device %s\n",
+error_report("Error: tried to detach unattached usb device %s\n",
 dev->product_desc);
 return -1;
 }
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index c59797b..a75948b 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -497,8 +497,9 @@ static void usb_msd_password_cb(void *opaque, int err)
 MSDState *s = opaque;
 
 if (!err)
-usb_device_attach(&s->dev);
-else
+err = usb_device_attach(&s->dev);
+
+if (err)
 qdev_unplug(&s->dev.qdev);
 }
 
diff --git a/usb-linux.c b/usb-linux.c
index 42baafe..9b6f2be 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1178,10 +1178,14 @@ static int usb_host_open(USBHostDevice *dev, int 
bus_num,
 prod_name);
 }
 
+ret = usb_device_attach(&dev->dev);
+if (ret) {
+goto fail;
+}
+
 /* USB devio uses 'write' flag to check for async completions */
 qemu_set_fd_handler(dev->fd, NULL, async_complete, dev);
 
-usb_device_attach(&dev->dev);
 return 0;
 
 fail:
-- 
1.7.1




[Qemu-devel] [PATCH 13/13] usb: ignore USB_DT_DEBUG

2011-06-24 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 hw/usb-desc.c |4 
 hw/usb.h  |1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/usb-desc.c b/hw/usb-desc.c
index 0b9d351..bc6858f 100644
--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -385,6 +385,10 @@ int usb_desc_get_descriptor(USBDevice *dev, int value, 
uint8_t *dest, size_t len
 trace_usb_desc_other_speed_config(dev->addr, index, len, ret);
 break;
 
+case USB_DT_DEBUG:
+/* ignore silently */
+break;
+
 default:
 fprintf(stderr, "%s: %d unknown type %d (len %zd)\n", __FUNCTION__,
 dev->addr, type, len);
diff --git a/hw/usb.h b/hw/usb.h
index a6b311f..076e2ff 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -130,6 +130,7 @@
 #define USB_DT_ENDPOINT0x05
 #define USB_DT_DEVICE_QUALIFIER 0x06
 #define USB_DT_OTHER_SPEED_CONFIG   0x07
+#define USB_DT_DEBUG0x0A
 #define USB_DT_INTERFACE_ASSOC  0x0B
 
 #define USB_ENDPOINT_XFER_CONTROL  0
-- 
1.7.1




[Qemu-devel] [PATCH 10/13] usb-bus: Don't allow speed mismatch while attaching devices

2011-06-24 Thread Gerd Hoffmann
From: Hans de Goede 

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-bus.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index fc72018..2abce12 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -200,6 +200,11 @@ static int do_attach(USBDevice *dev)
 } else {
 port = QTAILQ_FIRST(&bus->free);
 }
+if (!(port->speedmask & dev->speedmask)) {
+error_report("Warning: speed mismatch trying to attach usb device %s 
to bus %s\n",
+dev->product_desc, bus->qbus.name);
+return -1;
+}
 
 dev->attached++;
 QTAILQ_REMOVE(&bus->free, port, next);
-- 
1.7.1




Re: [Qemu-devel] Actual TB code doesn't look like what was intended (TCG issue)?

2011-06-24 Thread Peter Maydell
On 24 June 2011 11:08, Max Filippov  wrote:
>> Here are my rules of thumb for generating code where the code
>> generated might change based on some bit of CPU state:
>>
>> When you are generating code, if the code you generate will
>> change based on the contents of something in the CPUState struct,
>> then the bit of CPUState you are looking at has to be one of:
>>  (1) encoded in the TB flags (or tb->pc or tb->cs_base)
>>      (and gen_intermediate_code_internal() must read and
>>      use the value in tb->tb_flags, not the one in env)
>
> So if changing a bit of context does not cause TB invalidation then it
> must be captured in cpu_get_tb_cpu_state and
> gen_intermediate_code_internal must use that captured value?

Yes. (The other option is to arrange to not change the code you
generate based on that bit of context, for instance you can generate
code which loads an env field and passes it to a helper function.)

-- PMM



Re: [Qemu-devel] [PATCH v2 09/12] VMDK: open/read/write for monolithicFlat image

2011-06-24 Thread Stefan Hajnoczi
On Fri, Jun 24, 2011 at 9:18 AM,   wrote:
> From: Fam Zheng 
>
> Parse vmdk decriptor file and open mono flat image.
> Read/write the flat extent.
>
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |  190 
> ++
>  1 files changed, 177 insertions(+), 13 deletions(-)

Please see my comments on the previous version.

Stefan



Re: [Qemu-devel] [PATCH 0/4] A few cleanups of qdev users

2011-06-24 Thread Markus Armbruster
Ping?

Markus Armbruster  writes:

> Markus Armbruster (4):
>   usb-ccid: Drop unused CCIDCardInfo callback print()
>   virtio-serial: Clean up virtser_bus_dev_print() output
>   virtio-serial: Turn props any virtio-serial-bus device must have into
> bus props
>   ide: Turn properties any IDE device must have into bus properties
>
>  hw/ccid.h  |1 -
>  hw/ide/qdev.c  |5 -
>  hw/usb-ccid.c  |   11 ---
>  hw/virtio-console.c|4 
>  hw/virtio-serial-bus.c |   18 ++
>  5 files changed, 14 insertions(+), 25 deletions(-)



Re: [Qemu-devel] [Qemu-trivial] [PATCH] w32: Remove redundant definitions of PRI*64

2011-06-24 Thread Stefan Hajnoczi
On Fri, Jun 24, 2011 at 12:15:49PM +0200, Stefan Weil wrote:
> The PRI*64 macros are defined in MinGW's inttypes.h since 2002,
> so they are not needed in qemu-common.h (which includes inttypes.h).
> 
> Signed-off-by: Stefan Weil 
> ---
>  qemu-common.h |5 -
>  1 files changed, 0 insertions(+), 5 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



[Qemu-devel] [PULL] usb patch queue

2011-06-24 Thread Gerd Hoffmann
  Hi,

Here comes the USB patch queue.  Nothing major, just a bunch of little
fixes and improvements.

please pull,
  Gerd

The following changes since commit 48e2faf222cbf4abab7c8e4b3f44229ec98eae7f:

  net: Warn about "-net nic" options which were ignored (2011-06-22 07:18:39 
-0500)

are available in the git repository at:
  git://git.kraxel.org/qemu usb.17

Gerd Hoffmann (6):
  usb-linux: add get_endp()
  usb-linux: make iso urb count contigurable
  usb-linux: track inflight iso urb count
  ehci: add freq + maxframes properties
  ehci: switch to nanoseconds
  usb: ignore USB_DT_DEBUG

Hans de Goede (5):
  usb-bus: Don't allow attaching a device to a bus with no free ports
  usb: Proper error propagation for usb_device_attach errors
  usb: Add a speedmask to devices
  usb-linux: allow "compatible" high speed devices to connect at fullspeed
  usb-bus: Don't allow speed mismatch while attaching devices

Markus Armbruster (1):
  usb-storage: Turn drive serial into a qdev property usb-storage.serial

Peter Maydell (1):
  hw/usb-ohci.c: Fix handling of remote wakeup corner cases

 hw/usb-bus.c  |   31 ++-
 hw/usb-ccid.c |1 +
 hw/usb-desc.c |   14 ++
 hw/usb-ehci.c |   43 +++-
 hw/usb-msd.c  |   19 ++--
 hw/usb-ohci.c |   17 ++-
 hw/usb.h  |4 ++
 usb-bsd.c |2 +
 usb-linux.c   |  124 +
 9 files changed, 191 insertions(+), 64 deletions(-)



Re: [Qemu-devel] unix domain socket communication with guests

2011-06-24 Thread Joel Uckelman
On Fri, Jun 24, 2011 at 4:54 AM, Amit Shah  wrote:
>>
>> I guess this means I need to get networking running on the guest so
>> that it has a port visible to the host on which my server can listen.
>> Is there a guide somewhere for doing that? I've not had any success in
>> an afternoon of searching and trying.
>
> Can't say I understand what you're trying to accomplish.  If you want
> to just pass data between a guest and host w/o networking,
> virtio-serial is one way of doing it.  If you have networking enabled
> and the host and guest can talk to each other, TCP or UDP
> communication will work as usual.
>
>                Amit

I have a server which I want to run on the guest, and one or more
clients which I want to run on the host. So, I need something I can
bind(2) to on the guest side, and something I can connect(2) to on the
host side. Is that clearer? Is this possible to do with virtio-serial?



[Qemu-devel] [PATCH 04/13] ehci: add freq + maxframes properties

2011-06-24 Thread Gerd Hoffmann
Add properties for the wakeup rate and the max number of frames ehci
will process at once.

The wakeup rate defaults to 1000 which equals the usb frame rate.  This
can be reduced to make qemu wake up less often when ehci is active.

In case the wakeup rate is reduced or the ehci timer is delayed due to
latency issues elsewhere in qemu ehci will process multiple frames at
once.  The maxframes property specifies the upper limit for this.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-ehci.c |   14 --
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e33e546..fa9792e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -373,6 +373,11 @@ struct EHCIState {
 target_phys_addr_t mem_base;
 int mem;
 int num_ports;
+
+/* properties */
+uint32_t freq;
+uint32_t maxframes;
+
 /*
  *  EHCI spec version 1.0 Section 2.3
  *  Host Controller Operational Registers
@@ -2048,7 +2053,7 @@ static void ehci_frame_timer(void *opaque)
 
 
 t_now = qemu_get_clock_ns(vm_clock);
-expire_time = t_now + (get_ticks_per_sec() / FRAME_TIMER_FREQ);
+expire_time = t_now + (get_ticks_per_sec() / ehci->freq);
 if (expire_time == t_now) {
 expire_time++;
 }
@@ -2073,7 +2078,7 @@ static void ehci_frame_timer(void *opaque)
 ehci->sofv &= 0x03ff;
 }
 
-if (frames - i > 10) {
+if (frames - i > ehci->maxframes) {
 skipped_frames++;
 } else {
 ehci_advance_periodic_state(ehci);
@@ -2146,6 +2151,11 @@ static PCIDeviceInfo ehci_info = {
 .device_id= PCI_DEVICE_ID_INTEL_82801D,
 .revision = 0x10,
 .class_id = PCI_CLASS_SERIAL_USB,
+.qdev.props   = (Property[]) {
+DEFINE_PROP_UINT32("freq",  EHCIState, freq, FRAME_TIMER_FREQ),
+DEFINE_PROP_UINT32("maxframes", EHCIState, maxframes, 128),
+DEFINE_PROP_END_OF_LIST(),
+},
 };
 
 static int usb_ehci_initfn(PCIDevice *dev)
-- 
1.7.1




[Qemu-devel] [PATCH 06/13] usb-bus: Don't allow attaching a device to a bus with no free ports

2011-06-24 Thread Gerd Hoffmann
From: Hans de Goede 

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-bus.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 480956d..0a49921 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -181,6 +181,11 @@ static void do_attach(USBDevice *dev)
 dev->product_desc);
 return;
 }
+if (bus->nfree == 0) {
+fprintf(stderr, "Warning: tried to attach usb device %s to a bus with 
no free ports\n",
+dev->product_desc);
+return;
+}
 if (dev->port_path) {
 QTAILQ_FOREACH(port, &bus->free, next) {
 if (strcmp(port->path, dev->port_path) == 0) {
-- 
1.7.1




[Qemu-devel] [PATCH 09/13] usb-linux: allow "compatible" high speed devices to connect at fullspeed

2011-06-24 Thread Gerd Hoffmann
From: Hans de Goede 

Some usb2 highspeed devices, like usb-msd devices, work fine when redirected
to a usb1 virtual controller. Allow this to avoid the new speedhecks causing
regressions for users who do not enable the new experimental ehci code.

Signed-off-by: Gerd Hoffmann 
---
 usb-linux.c |   39 +++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 4d22c9c..1a2deb3 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1088,6 +1088,42 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
 return 0;
 }
 
+/*
+ * Check if we can safely redirect a usb2 device to a usb1 virtual controller,
+ * this function assumes this is safe, if:
+ * 1) There are no isoc endpoints
+ * 2) There are no interrupt endpoints with a max_packet_size > 64
+ * Note bulk endpoints with a max_packet_size > 64 in theory also are not
+ * usb1 compatible, but in practice this seems to work fine.
+ */
+static int usb_linux_full_speed_compat(USBHostDevice *dev)
+{
+int i, packet_size;
+
+/*
+ * usb_linux_update_endp_table only registers info about ep in the current
+ * interface altsettings, so we need to parse the descriptors again.
+ */
+for (i = 0; (i + 5) < dev->descr_len; i += dev->descr[i]) {
+if (dev->descr[i + 1] == USB_DT_ENDPOINT) {
+switch (dev->descr[i + 3] & 0x3) {
+case 0x00: /* CONTROL */
+break;
+case 0x01: /* ISO */
+return 0;
+case 0x02: /* BULK */
+break;
+case 0x03: /* INTERRUPT */
+packet_size = dev->descr[i + 4] + (dev->descr[i + 5] << 8);
+if (packet_size > 64)
+return 0;
+break;
+}
+}
+}
+return 1;
+}
+
 static int usb_host_open(USBHostDevice *dev, int bus_num,
 int addr, char *port, const char *prod_name, int speed)
 {
@@ -1168,6 +1204,9 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
 }
 dev->dev.speed = speed;
 dev->dev.speedmask = (1 << speed);
+if (dev->dev.speed == USB_SPEED_HIGH && usb_linux_full_speed_compat(dev)) {
+dev->dev.speedmask |= USB_SPEED_MASK_FULL;
+}
 
 printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
 
-- 
1.7.1




[Qemu-devel] [PATCH 05/13] ehci: switch to nanoseconds

2011-06-24 Thread Gerd Hoffmann
Make ehci use nanoseconds everywhere.
Simplifies time calculations.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb-ehci.c |   29 +++--
 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index fa9792e..91fb7de 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -130,7 +130,7 @@
 #define PORTSC_CONNECT   (1 << 0) // Current Connect Status
 
 #define FRAME_TIMER_FREQ 1000
-#define FRAME_TIMER_USEC (100 / FRAME_TIMER_FREQ)
+#define FRAME_TIMER_NS   (10 / FRAME_TIMER_FREQ)
 
 #define NB_MAXINTRATE8// Max rate at which controller issues ints
 #define NB_PORTS 4// Number of downstream ports
@@ -348,7 +348,8 @@ struct EHCIQueue {
 EHCIState *ehci;
 QTAILQ_ENTRY(EHCIQueue) next;
 bool async_schedule;
-uint32_t seen, ts;
+uint32_t seen;
+uint64_t ts;
 
 /* cached data from guest - needs to be flushed
  * when guest removes an entry (doorbell, handshake sequence)
@@ -418,12 +419,11 @@ struct EHCIState {
 uint8_t ibuffer[BUFF_SIZE];
 int isoch_pause;
 
-uint32_t last_run_usec;
-uint32_t frame_end_usec;
+uint64_t last_run_ns;
 };
 
 #define SET_LAST_RUN_CLOCK(s) \
-(s)->last_run_usec = qemu_get_clock_ns(vm_clock) / 1000;
+(s)->last_run_ns = qemu_get_clock_ns(vm_clock);
 
 /* nifty macros from Arnon's EHCI version  */
 #define get_field(data, field) \
@@ -690,10 +690,10 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
 QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
 if (q->seen) {
 q->seen = 0;
-q->ts = ehci->last_run_usec;
+q->ts = ehci->last_run_ns;
 continue;
 }
-if (ehci->last_run_usec < q->ts + 25) {
+if (ehci->last_run_ns < q->ts + 25000) {
 /* allow 0.25 sec idle */
 continue;
 }
@@ -2045,23 +2045,16 @@ static void ehci_frame_timer(void *opaque)
 {
 EHCIState *ehci = opaque;
 int64_t expire_time, t_now;
-int usec_elapsed;
+uint64_t ns_elapsed;
 int frames;
-int usec_now;
 int i;
 int skipped_frames = 0;
 
-
 t_now = qemu_get_clock_ns(vm_clock);
 expire_time = t_now + (get_ticks_per_sec() / ehci->freq);
-if (expire_time == t_now) {
-expire_time++;
-}
 
-usec_now = t_now / 1000;
-usec_elapsed = usec_now - ehci->last_run_usec;
-frames = usec_elapsed / FRAME_TIMER_USEC;
-ehci->frame_end_usec = usec_now + FRAME_TIMER_USEC - 10;
+ns_elapsed = t_now - ehci->last_run_ns;
+frames = ns_elapsed / FRAME_TIMER_NS;
 
 for (i = 0; i < frames; i++) {
 if ( !(ehci->usbsts & USBSTS_HALT)) {
@@ -2084,7 +2077,7 @@ static void ehci_frame_timer(void *opaque)
 ehci_advance_periodic_state(ehci);
 }
 
-ehci->last_run_usec += FRAME_TIMER_USEC;
+ehci->last_run_ns += FRAME_TIMER_NS;
 }
 
 #if 0
-- 
1.7.1




[Qemu-devel] [PATCH 11/13] hw/usb-ohci.c: Fix handling of remote wakeup corner cases

2011-06-24 Thread Gerd Hoffmann
From: Peter Maydell 

Correct a number of minor errors in the OHCI wakeup implementation:
 * when the port is suspended but the controller is not, raise RHSC
 * when the controller is suspended but the port is not, raise RD
 * when the controller is suspended, move it to resume state

These fix some edge cases where a USB device might not successfully get
the attention of the guest OS if it tried to do so at the wrong time.

Signed-off-by: Peter Maydell 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb-ohci.c |   17 ++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 5d2ae01..1c29b9f 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -373,14 +373,25 @@ static void ohci_wakeup(USBDevice *dev)
 OHCIState *s = container_of(bus, OHCIState, bus);
 int portnum = dev->port->index;
 OHCIPort *port = &s->rhport[portnum];
+uint32_t intr = 0;
 if (port->ctrl & OHCI_PORT_PSS) {
 DPRINTF("usb-ohci: port %d: wakeup\n", portnum);
 port->ctrl |= OHCI_PORT_PSSC;
 port->ctrl &= ~OHCI_PORT_PSS;
-if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
-ohci_set_interrupt(s, OHCI_INTR_RD);
-}
+intr = OHCI_INTR_RHSC;
+}
+/* Note that the controller can be suspended even if this port is not */
+if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
+DPRINTF("usb-ohci: remote-wakeup: SUSPEND->RESUME\n");
+/* This is the one state transition the controller can do by itself */
+s->ctl &= ~OHCI_CTL_HCFS;
+s->ctl |= OHCI_USB_RESUME;
+/* In suspend mode only ResumeDetected is possible, not RHSC:
+ * see the OHCI spec 5.1.2.3.
+ */
+intr = OHCI_INTR_RD;
 }
+ohci_set_interrupt(s, intr);
 }
 
 /* Reset the controller */
-- 
1.7.1




Re: [Qemu-devel] [PATCH 06/11] exec.c: refactor cpu_physical_memory_map

2011-06-24 Thread Stefano Stabellini
On Thu, 23 Jun 2011, Peter Maydell wrote:
> On 23 June 2011 18:56, Stefano Stabellini
>  wrote:
> > Thanks for the detailed explanation of the problem, I think I understand
> > what I have to do to fix.
> > However I would like to have a repro of the bug before sending any
> > patches, so that I am sure that the solution works correctly.
> > However I am not very familiar with ARM emulation in Qemu: could you
> > please let me know which target I have to compile, which machine I have
> > to emulate and what other steps do I have to take in order to see this
> > issue?
> 
> You'll need this not-yet-applied patch which adds cp15 perf reg support:
>  http://patchwork.ozlabs.org/patch/92423/
> I've temporarily put a test kernel and initrd here:
> http://people.linaro.org/~pmaydell/linaro-20110228.tar.gz
> 
> configure qemu with --target-list=arm-softmmu
> and then you can run:
> 
> ./arm-softmmu/qemu-system-arm -kernel
> linaro-20110228/vmlinuz-2.6.37-1003-linaro-vexpress -initrd
> linaro-20110228/initrd.img-2.6.37-1003-linaro-vexpress  -M vexpress-a9
> -cpu cortex-a9 -serial stdio -m 1024  -append 'root=/dev/mmcblk0p2 rw
> mem=1024M raid=noautodetect console=ttyAMA0,38400n8 rootwait
> vmalloc=256MB devtmpfs.mount=0'
> 
> At the moment this aborts when the kernel tries to initialise the
> display; if you fix the problem it ought to display a penguin on
> the graphics window (although it will drop you into an initramfs
> prompt because you don't have a root filesystem sd card image).
 
Thanks, I managed to test my fix succesfully.
See the patch "qemu_ram_ptr_length: take ram_addr_t as arguments" I have
just sent.




[Qemu-devel] [PATCH 12/13] usb-storage: Turn drive serial into a qdev property usb-storage.serial

2011-06-24 Thread Gerd Hoffmann
From: Markus Armbruster 

It needs to be a qdev property, because it belongs to the drive's
guest part.  Precedence: commit a0fef654 and 6ced55a5.

Bonus: info qtree now shows the serial number.

Signed-off-by: Markus Armbruster 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb-msd.c |   14 +++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index a75948b..86582cc 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -51,6 +51,7 @@ typedef struct {
 SCSIRequest *req;
 SCSIBus bus;
 BlockConf conf;
+char *serial;
 SCSIDevice *scsi_dev;
 uint32_t removable;
 int result;
@@ -532,9 +533,15 @@ static int usb_msd_initfn(USBDevice *dev)
 bdrv_detach(bs, &s->dev.qdev);
 s->conf.bs = NULL;
 
-dinfo = drive_get_by_blockdev(bs);
-if (dinfo && dinfo->serial) {
-usb_desc_set_string(dev, STR_SERIALNUMBER, dinfo->serial);
+if (!s->serial) {
+/* try to fall back to value set with legacy -drive serial=... */
+dinfo = drive_get_by_blockdev(bs);
+if (*dinfo->serial) {
+s->serial = strdup(dinfo->serial);
+}
+}
+if (s->serial) {
+usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
 }
 
 usb_desc_init(dev);
@@ -633,6 +640,7 @@ static struct USBDeviceInfo msd_info = {
 .usbdevice_init = usb_msd_init,
 .qdev.props = (Property[]) {
 DEFINE_BLOCK_PROPERTIES(MSDState, conf),
+DEFINE_PROP_STRING("serial", MSDState, serial),
 DEFINE_PROP_BIT("removable", MSDState, removable, 0, false),
 DEFINE_PROP_END_OF_LIST(),
 },
-- 
1.7.1




[Qemu-devel] [PATCH v2] qxl: set mm_time in vga update

2011-06-24 Thread Alon Levy
This fixes a problem where on windows 7 startup phase, before the qxl driver
is loaded, the drawables are sufficiently large and video like to trigger a
stream, but the lack of a filled mm time field triggers a warning in spice-gtk.
---
 ui/spice-display.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index bb1e4a7..93ebc19 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -182,6 +182,7 @@ static SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 QXLCommand *cmd;
 uint8_t *src, *dst;
 int by, bw, bh;
+struct timespec time_space;
 
 if (qemu_spice_rect_is_empty(&ssd->dirty)) {
 return NULL;
@@ -208,6 +209,10 @@ static SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 drawable->surfaces_dest[0] = -1;
 drawable->surfaces_dest[1] = -1;
 drawable->surfaces_dest[2] = -1;
+clock_gettime(CLOCK_MONOTONIC, &time_space);
+/* time in milliseconds from epoch. */
+drawable->mm_time = time_space.tv_sec * 1000
+  + time_space.tv_nsec / 1000 / 1000;
 
 drawable->u.copy.rop_descriptor  = SPICE_ROPD_OP_PUT;
 drawable->u.copy.src_bitmap  = (intptr_t)image;
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: interface_get_command: fix reported mode

2011-06-24 Thread Alon Levy
report correct mode when in undefined mode.
introduces qxl_mode_to_string, and uses it in another place that looks
helpful (qxl_post_load)
---
 hw/qxl.c |   23 ---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e45f33a..e2b07dd 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -335,6 +335,21 @@ static void interface_get_init_info(QXLInstance *sin, 
QXLDevInitInfo *info)
 info->n_surfaces = NUM_SURFACES;
 }
 
+static const char *qxl_mode_to_string(int mode)
+{
+switch (mode) {
+case QXL_MODE_COMPAT:
+return "compat";
+case QXL_MODE_NATIVE:
+return "native";
+case QXL_MODE_UNDEFINED:
+return "undefined";
+case QXL_MODE_VGA:
+return "vga";
+}
+return "INVALID";
+}
+
 /* called from spice server thread context only */
 static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
 {
@@ -357,18 +372,19 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 }
 qemu_mutex_unlock(&qxl->ssd.lock);
 if (ret) {
+dprint(qxl, 2, "%s %s\n", __FUNCTION__, 
qxl_mode_to_string(qxl->mode));
 qxl_log_command(qxl, "vga", ext);
 }
 return ret;
 case QXL_MODE_COMPAT:
 case QXL_MODE_NATIVE:
 case QXL_MODE_UNDEFINED:
-dprint(qxl, 2, "%s: %s\n", __FUNCTION__,
-   qxl->cmdflags ? "compat" : "native");
+dprint(qxl, 4, "%s: %s\n", __FUNCTION__, 
qxl_mode_to_string(qxl->mode));
 ring = &qxl->ram->cmd_ring;
 if (SPICE_RING_IS_EMPTY(ring)) {
 return false;
 }
+dprint(qxl, 2, "%s: %s\n", __FUNCTION__, 
qxl_mode_to_string(qxl->mode));
 SPICE_RING_CONS_ITEM(ring, cmd);
 ext->cmd  = *cmd;
 ext->group_id = MEMSLOT_GROUP_GUEST;
@@ -1500,7 +1516,8 @@ static int qxl_post_load(void *opaque, int version)
 
 d->modes = (QXLModes*)((uint8_t*)d->rom + d->rom->modes_offset);
 
-dprint(d, 1, "%s: restore mode\n", __FUNCTION__);
+dprint(d, 1, "%s: restore mode (%s)\n", __FUNCTION__,
+qxl_mode_to_string(d->mode));
 newmode = d->mode;
 d->mode = QXL_MODE_UNDEFINED;
 switch (newmode) {
-- 
1.7.5.4




[Qemu-devel] [PATCH] qemu_ram_ptr_length: take ram_addr_t as arguments

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

qemu_ram_ptr_length should take ram_addr_t as argument rather than
target_phys_addr_t because is doing comparisons with RAMBlock addresses.

cpu_physical_memory_map should create a ram_addr_t address to pass to
qemu_ram_ptr_length from PhysPageDesc phys_offset.

Remove code after abort() in qemu_ram_ptr_length.

Signed-off-by: Stefano Stabellini 
---
 cpu-common.h |2 +-
 exec.c   |   18 +++---
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index 085aacb..ceaa631 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -64,7 +64,7 @@ void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
 /* This should only be used for ram local to a device.  */
 void *qemu_get_ram_ptr(ram_addr_t addr);
-void *qemu_ram_ptr_length(target_phys_addr_t addr, target_phys_addr_t *size);
+void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size);
 /* Same but slower, to use for migration, where the order of
  * RAMBlocks must not change. */
 void *qemu_safe_ram_ptr(ram_addr_t addr);
diff --git a/exec.c b/exec.c
index aebb23b..5b9390e 100644
--- a/exec.c
+++ b/exec.c
@@ -3115,7 +3115,7 @@ void *qemu_safe_ram_ptr(ram_addr_t addr)
 
 /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
  * but takes a size argument */
-void *qemu_ram_ptr_length(target_phys_addr_t addr, target_phys_addr_t *size)
+void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size)
 {
 if (xen_mapcache_enabled())
 return qemu_map_cache(addr, *size, 1);
@@ -3132,9 +3132,6 @@ void *qemu_ram_ptr_length(target_phys_addr_t addr, 
target_phys_addr_t *size)
 
 fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
 abort();
-
-*size = 0;
-return NULL;
 }
 }
 
@@ -4000,7 +3997,9 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 target_phys_addr_t page;
 unsigned long pd;
 PhysPageDesc *p;
-target_phys_addr_t addr1 = addr;
+ram_addr_t addr1 = addr;
+ram_addr_t rlen;
+void *raddr;
 
 while (len > 0) {
 page = addr & TARGET_PAGE_MASK;
@@ -4028,13 +4027,18 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 *plen = l;
 return bounce.buffer;
 }
+if (!todo) {
+addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
+}
 
 len -= l;
 addr += l;
 todo += l;
 }
-*plen = todo;
-return qemu_ram_ptr_length(addr1, plen);
+rlen = todo;
+raddr = qemu_ram_ptr_length(addr1, &rlen);
+*plen = rlen;
+return raddr;
 }
 
 /* Unmaps a memory region previously mapped by cpu_physical_memory_map().
-- 
1.7.2.3




[Qemu-devel] [PATCH v2] qxl-logger: add timestamp to command log

2011-06-24 Thread Alon Levy
---
 hw/qxl-logger.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/qxl-logger.c b/hw/qxl-logger.c
index 76f43e6..74cadba 100644
--- a/hw/qxl-logger.c
+++ b/hw/qxl-logger.c
@@ -19,6 +19,7 @@
  * along with this program; if not, see .
  */
 
+#include "qemu-timer.h"
 #include "qxl.h"
 
 static const char *qxl_type[] = {
@@ -223,7 +224,8 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, 
QXLCommandExt *ext)
 if (!qxl->cmdlog) {
 return;
 }
-fprintf(stderr, "qxl-%d/%s:", qxl->id, ring);
+fprintf(stderr, "%ld qxl-%d/%s:", qemu_get_clock_ns(vm_clock),
+qxl->id, ring);
 fprintf(stderr, " cmd @ 0x%" PRIx64 " %s%s", ext->cmd.data,
 qxl_name(qxl_type, ext->cmd.type),
 compat ? "(compat)" : "");
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: allow QXL_IO_LOG also in vga

2011-06-24 Thread Alon Levy
The driver may change us to vga mode and still issue a QXL_IO_LOG,
which we can easily support.
---
 hw/qxl.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 5266707..96f9c55 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1001,9 +1001,9 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_MEMSLOT_ADD_ASYNC:
 case QXL_IO_MEMSLOT_DEL:
 case QXL_IO_CREATE_PRIMARY:
-case QXL_IO_LOG:
 case QXL_IO_CREATE_PRIMARY_ASYNC:
 case QXL_IO_UPDATE_IRQ:
+case QXL_IO_LOG:
 break;
 default:
 if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
-- 
1.7.5.4




Re: [Qemu-devel] [PULL] usb patch queue

2011-06-24 Thread Hans de Goede

Hi,

Entire series looks good to me, including my own patches ;)

Ack series.

Regards,

Hans


On 06/24/2011 12:59 PM, Gerd Hoffmann wrote:

   Hi,

Here comes the USB patch queue.  Nothing major, just a bunch of little
fixes and improvements.

please pull,
   Gerd

The following changes since commit 48e2faf222cbf4abab7c8e4b3f44229ec98eae7f:

   net: Warn about "-net nic" options which were ignored (2011-06-22 07:18:39 
-0500)

are available in the git repository at:
   git://git.kraxel.org/qemu usb.17

Gerd Hoffmann (6):
   usb-linux: add get_endp()
   usb-linux: make iso urb count contigurable
   usb-linux: track inflight iso urb count
   ehci: add freq + maxframes properties
   ehci: switch to nanoseconds
   usb: ignore USB_DT_DEBUG

Hans de Goede (5):
   usb-bus: Don't allow attaching a device to a bus with no free ports
   usb: Proper error propagation for usb_device_attach errors
   usb: Add a speedmask to devices
   usb-linux: allow "compatible" high speed devices to connect at fullspeed
   usb-bus: Don't allow speed mismatch while attaching devices

Markus Armbruster (1):
   usb-storage: Turn drive serial into a qdev property usb-storage.serial

Peter Maydell (1):
   hw/usb-ohci.c: Fix handling of remote wakeup corner cases

  hw/usb-bus.c  |   31 ++-
  hw/usb-ccid.c |1 +
  hw/usb-desc.c |   14 ++
  hw/usb-ehci.c |   43 +++-
  hw/usb-msd.c  |   19 ++--
  hw/usb-ohci.c |   17 ++-
  hw/usb.h  |4 ++
  usb-bsd.c |2 +
  usb-linux.c   |  124 +
  9 files changed, 191 insertions(+), 64 deletions(-)





[Qemu-devel] [PATCH v2] qxl: add io_port_to_string

2011-06-24 Thread Alon Levy
---
 hw/qxl.c |   64 +-
 1 files changed, 63 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 3d5c823..b4bc376 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -350,6 +350,67 @@ static const char *qxl_mode_to_string(int mode)
 return "INVALID";
 }
 
+static const char *io_port_to_string(uint32_t io_port)
+{
+if (io_port >= QXL_IO_RANGE_SIZE) {
+return "out of range";
+}
+switch(io_port) {
+case QXL_IO_NOTIFY_CMD:
+return "QXL_IO_NOTIFY_CMD";
+case QXL_IO_NOTIFY_CURSOR:
+return "QXL_IO_NOTIFY_CURSOR";
+case QXL_IO_UPDATE_AREA:
+return "QXL_IO_UPDATE_AREA";
+case QXL_IO_UPDATE_IRQ:
+return "QXL_IO_UPDATE_IRQ";
+case QXL_IO_NOTIFY_OOM:
+return "QXL_IO_NOTIFY_OOM";
+case QXL_IO_RESET:
+return "QXL_IO_RESET";
+case QXL_IO_SET_MODE:
+return "QXL_IO_SET_MODE";
+case QXL_IO_LOG:
+return "QXL_IO_LOG";
+case QXL_IO_MEMSLOT_ADD:
+return "QXL_IO_MEMSLOT_ADD";
+case QXL_IO_MEMSLOT_DEL:
+return "QXL_IO_MEMSLOT_DEL";
+case QXL_IO_DETACH_PRIMARY:
+return "QXL_IO_DETACH_PRIMARY";
+case QXL_IO_ATTACH_PRIMARY:
+return "QXL_IO_ATTACH_PRIMARY";
+case QXL_IO_CREATE_PRIMARY:
+return "QXL_IO_CREATE_PRIMARY";
+case QXL_IO_DESTROY_PRIMARY:
+return "QXL_IO_DESTROY_PRIMARY";
+case QXL_IO_DESTROY_SURFACE_WAIT:
+return "QXL_IO_DESTROY_SURFACE_WAIT";
+case QXL_IO_DESTROY_ALL_SURFACES:
+return "QXL_IO_DESTROY_ALL_SURFACES";
+case QXL_IO_UPDATE_AREA_ASYNC:
+return "QXL_IO_UPDATE_AREA_ASYNC";
+case QXL_IO_NOTIFY_OOM_ASYNC:
+return "QXL_IO_NOTIFY_OOM_ASYNC";
+case QXL_IO_MEMSLOT_ADD_ASYNC:
+return "QXL_IO_MEMSLOT_ADD_ASYNC";
+case QXL_IO_CREATE_PRIMARY_ASYNC:
+return "QXL_IO_CREATE_PRIMARY_ASYNC";
+case QXL_IO_DESTROY_PRIMARY_ASYNC:
+return "QXL_IO_DESTROY_PRIMARY_ASYNC";
+case QXL_IO_DESTROY_SURFACE_ASYNC:
+return "QXL_IO_DESTROY_SURFACE_ASYNC";
+case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+return "QXL_IO_DESTROY_ALL_SURFACES_ASYNC";
+case QXL_IO_FLUSH_SURFACES:
+return "QXL_IO_FLUSH_SURFACES";
+case QXL_IO_FLUSH_RELEASE:
+return "QXL_IO_FLUSH_RELEASE";
+}
+// not reached?
+return "error in io_port_to_string";
+}
+
 /* called from spice server thread context only */
 static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
 {
@@ -1009,7 +1070,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 default:
 if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
 break;
-dprint(d, 1, "%s: unexpected port 0x%x in vga mode\n", __FUNCTION__, 
io_port);
+dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
+__FUNCTION__, io_port, io_port_to_string(io_port));
 /* be nice to buggy guest drivers */
 if (io_port >= QXL_IO_UPDATE_AREA_ASYNC &&
 io_port <= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: abort on panic instead of exit

2011-06-24 Thread Alon Levy
---
 hw/qxl.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.h b/hw/qxl.h
index 6dd38e6..584f02b 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -87,7 +87,7 @@ typedef struct PCIQXLDevice {
 
 #define PANIC_ON(x) if ((x)) { \
 printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
-exit(-1);  \
+abort();  \
 }
 
 #define dprint(_qxl, _level, _fmt, ...) \
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: update and add debug prints

2011-06-24 Thread Alon Levy
---
 hw/qxl.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 96f9c55..865e985 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -361,7 +361,7 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 
 switch (qxl->mode) {
 case QXL_MODE_VGA:
-dprint(qxl, 2, "%s: vga\n", __FUNCTION__);
+dprint(qxl, 4, "%s: vga\n", __FUNCTION__);
 ret = false;
 qemu_mutex_lock(&qxl->ssd.lock);
 if (qxl->ssd.update != NULL) {
@@ -700,7 +700,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
 qemu_spice_create_host_memslot(&d->ssd);
 qxl_soft_reset(d);
 
-dprint(d, 1, "%s: done\n", __FUNCTION__);
+dprint(d, 1, "%s: done (num_free_res %d, %p)\n", __FUNCTION__,
+d->num_free_res, d->last_release);
 }
 
 static void qxl_reset_handler(DeviceState *dev)
@@ -1040,6 +1041,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 }
 pthread_yield();
 if (!SPICE_RING_IS_EMPTY(&d->ram->release_ring)) {
+dprint(d, 1, "QXL_IO_NOTIFY_OOM (return after pthread_yield)\n");
 break;
 }
 d->oom_running = 1;
@@ -1057,7 +1059,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 }
 break;
 case QXL_IO_RESET:
-dprint(d, 1, "QXL_IO_RESET\n");
+dprint(d, 1, "QXL_IO_RESET %d (%p)\n", d->num_free_res,
+   d->last_release);
 qxl_hard_reset(d, 0);
 break;
 case QXL_IO_MEMSLOT_ADD:
@@ -1073,6 +1076,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 break;
 }
 case QXL_IO_MEMSLOT_DEL:
+dprint(d, 1, "QXL_IO_MEMSLOT_DEL %d\n", val);
 qxl_del_memslot(d, val);
 break;
 case QXL_IO_CREATE_PRIMARY:
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: update revision to QXL_REVISION_STABLE_V10

2011-06-24 Thread Alon Levy
also errors if provided revision is wrong. 0 is reserved for experimental
revision, the other valid values are as before:
 1 - V04
 2 - V06
And the new
 3 - V10
---
 hw/qxl.c |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b4bc376..969a984 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1424,18 +1424,24 @@ static int qxl_init_common(PCIQXLDevice *qxl)
 qxl->num_surfaces = NUM_SURFACES;
 
 switch (qxl->revision) {
-case 1: /* spice 0.4 -- qxl-1 */
+case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
 pci_device_id  = QXL_DEVICE_ID_STABLE;
 pci_device_rev = QXL_REVISION_STABLE_V04;
 break;
-case 2: /* spice 0.6 -- qxl-2 */
+case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
 pci_device_id  = QXL_DEVICE_ID_STABLE;
 pci_device_rev = QXL_REVISION_STABLE_V06;
 break;
-default: /* experimental */
+case QXL_REVISION_STABLE_V10: /* spice 1.0 -- qxl-3 */
+pci_device_id  = QXL_DEVICE_ID_STABLE;
+pci_device_rev = QXL_REVISION_STABLE_V10;
+break;
+case 0: /* experimental */
 pci_device_id  = QXL_DEVICE_ID_DEVEL;
 pci_device_rev = 1;
 break;
+default:
+error_report("bad revision paramter");
 }
 
 pci_config_set_vendor_id(config, REDHAT_PCI_VENDOR_ID);
@@ -1723,7 +1729,7 @@ static PCIDeviceInfo qxl_info_secondary = {
 .qdev.props = (Property[]) {
 DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 
* 1024),
 DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, 64 * 1024 * 
1024),
-DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
+DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 
QXL_REVISION_STABLE_V10),
 DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
 DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
 DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: add mode to debugprint on destroy primary

2011-06-24 Thread Alon Levy
---
 hw/qxl.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e2b07dd..5266707 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1088,7 +1088,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 }
 case QXL_IO_DESTROY_PRIMARY:
 PANIC_ON(val != 0);
-dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%d)\n", d->mode);
+dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", 
qxl_mode_to_string(d->mode));
 if (d->mode != QXL_MODE_UNDEFINED) {
 d->mode = QXL_MODE_UNDEFINED;
 qemu_spice_destroy_primary_surface(&d->ssd, 0);
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] S3&S4 support

2011-06-24 Thread Alon Levy
This is a rewrite of v1, doesn't use any new api, reuses 
stop+start+destroy_surfaces
instead of the previously introduced update_mem spice server function. stop does
a flush of all commands, updating all the surfaces, and with destroy surfaces 
the
result is the same - everything has been pushed either to the release ring or to
the last_release list.

I've sent a bunch of patches that have previously appeared on the list, 
annotated below.

The before last patch actually does the implementation of the two new io's to 
be added
to spice-protocol V10, QXL_IO_FLUSH_SURFACES and QXL_IO_FLUSH_RELEASE.

The last patch requires some more attention - see it's commit message.

Alon Levy (12):
  qxl: set mm_time in vga update
   - previously sent as an RfC
  qxl: interface_get_command: fix reported mode
   - fixed with Gerd's comments (print INVALID if not know, used where it 
seemed appropriate)
  qxl: add mode to debugprint on destroy primary
  qxl: allow QXL_IO_LOG also in vga
  qxl: abort on panic instead of exit
  qxl-logger: add timestamp to command log
  qxl: update and add debug prints
  qxl: add dev id to guest prints
  qxl: add io_port_to_string
  qxl: update revision to QXL_REVISION_STABLE_V10
  qxl: add QXL_IO_FLUSH_{SURFACES,RELEASE} for guest S3&S4 support
  qxl: add primary_created state, change mode lifetimes

 hw/qxl-logger.c|4 +-
 hw/qxl.c   |  180 ---
 hw/qxl.h   |3 +-
 ui/spice-display.c |5 ++
 4 files changed, 165 insertions(+), 27 deletions(-)

-- 
1.7.5.4




[Qemu-devel] [PATCH] usb-hid: Fix 0/0 position for Windows in tablet mode

2011-06-24 Thread Jan Kiszka
For unknown reasons, Windows drivers (tested with XP and Win7) ignore
usb-tablet events that move the pointer to 0/0. So always set bit 0 of
the coordinates.

Signed-off-by: Jan Kiszka 
---
 hw/usb-hid.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index d711b5c..2b9a451 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -457,8 +457,10 @@ static void usb_pointer_event_combine(USBPointerEvent *e, 
int xyrel,
 e->xdx += x1;
 e->ydy += y1;
 } else {
-e->xdx = x1;
-e->ydy = y1;
+/* Windows drivers do not like the 0/0 position and ignore such
+ * events. */
+e->xdx = x1 | 1;
+e->ydy = y1 | 1;
 }
 e->dz += z1;
 }



[Qemu-devel] [PATCH v2] qxl: add dev id to guest prints

2011-06-24 Thread Alon Levy
---
 hw/qxl.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 865e985..3d5c823 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1055,7 +1055,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 break;
 case QXL_IO_LOG:
 if (d->guestdebug) {
-fprintf(stderr, "qxl/guest: %s", d->ram->log_buf);
+fprintf(stderr, "qxl/guest-%d: %ld: %s", d->id,
+qemu_get_clock_ns(vm_clock), d->ram->log_buf);
 }
 break;
 case QXL_IO_RESET:
-- 
1.7.5.4




[Qemu-devel] [PATCH 2/2] libcacard: replace copy_string with strndup

2011-06-24 Thread Christophe Fergeau
copy_string reimplements strndup, this commit removes it and
replaces all copy_string uses with strndup.

Signed-off-by: Christophe Fergeau 
---
 libcacard/vcard_emul_nss.c |   23 ++-
 1 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 2a20bd6..de324ba 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -925,17 +925,6 @@ vcard_emul_replay_insertion_events(void)
 /*
  *  Silly little functions to help parsing our argument string
  */
-static char *
-copy_string(const char *str, int str_len)
-{
-char *new_str;
-
-new_str = qemu_malloc(str_len+1);
-memcpy(new_str, str, str_len);
-new_str[str_len] = 0;
-return new_str;
-}
-
 static int
 count_tokens(const char *str, char token, char token_end)
 {
@@ -1054,18 +1043,18 @@ vcard_emul_options(const char *args)
 }
 opts->vreader = vreaderOpt;
 vreaderOpt = &vreaderOpt[opts->vreader_count];
-vreaderOpt->name = copy_string(name, name_length);
-vreaderOpt->vname = copy_string(vname, vname_length);
+vreaderOpt->name = qemu_strndup(name, name_length);
+vreaderOpt->vname = qemu_strndup(vname, vname_length);
 vreaderOpt->card_type = type;
 vreaderOpt->type_params =
-copy_string(type_params, type_params_length);
+qemu_strndup(type_params, type_params_length);
 count = count_tokens(args, ',', ')') + 1;
 vreaderOpt->cert_count = count;
 vreaderOpt->cert_name = (char **)qemu_malloc(count*sizeof(char *));
 for (i = 0; i < count; i++) {
 const char *cert = args;
 args = strpbrk(args, ",)");
-vreaderOpt->cert_name[i] = copy_string(cert, args - cert);
+vreaderOpt->cert_name[i] = qemu_strndup(cert, args - cert);
 args = strip(args+1);
 }
 if (*args == ')') {
@@ -1092,7 +1081,7 @@ vcard_emul_options(const char *args)
 args = strip(args+10);
 params = args;
 args = find_blank(args);
-opts->hw_type_params = copy_string(params, args-params);
+opts->hw_type_params = qemu_strndup(params, args-params);
 /* db="/data/base/path" */
 } else if (strncmp(args, "db=", 3) == 0) {
 const char *db;
@@ -1103,7 +1092,7 @@ vcard_emul_options(const char *args)
 args++;
 db = args;
 args = strpbrk(args, "\"\n");
-opts->nss_db = copy_string(db, args-db);
+opts->nss_db = qemu_strndup(db, args-db);
 if (*args != 0) {
 args++;
 }
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] qxl: add primary_created state, change mode lifetimes

2011-06-24 Thread Alon Levy
Currently we define the following relation between modes and primary surface
existance:

 QXL_MODE_COMPAT
 QXL_MODE_NATIVE  primary exists
 QXL_MODE_UNDEFINED
 QXL_MODE_VGA primary doesn't exist

This, coupled with disallowing various IO's when not in QXL_MODE_NATIVE or 
COMPAT
means that the S3 implementation gets more complex - This is wrong:
 DESTROY_SURFACES
 SURFACE_RELEASE
 .. wakeup
 PRIMARY_CREATE <-- error, we are already in NATIVE_MODE, not allowed!

This patch adds an explicit primary_created status, changed by:
CREATE_PRIMARY, DESTROY_PRIMARY, DESTROY_SURFACES

This patch also changes the lifetime of QXL_MODE_NATIVE.
 CREATE_PRIMARY - enter QXL_MODE_NATIVE
 vga io - exit QXL_MODE_NATIVE

anything else doesn't effect it, specifically DESTROY_PRIMARY.
---
 hw/qxl.c |   38 +++---
 hw/qxl.h |1 +
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index fab0208..51a5270 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -666,6 +666,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
 return;
 }
 dprint(d, 1, "%s\n", __FUNCTION__);
+d->primary_created = 1;
 qemu_spice_create_host_primary(&d->ssd);
 d->mode = QXL_MODE_VGA;
 memset(&d->ssd.dirty, 0, sizeof(d->ssd.dirty));
@@ -677,10 +678,9 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
 return;
 }
 dprint(d, 1, "%s\n", __FUNCTION__);
-if (d->mode != QXL_MODE_UNDEFINED) {
-d->mode = QXL_MODE_UNDEFINED;
-qemu_spice_destroy_primary_surface(&d->ssd, 0);
-}
+d->mode = QXL_MODE_UNDEFINED;
+d->primary_created = 0;
+qemu_spice_destroy_primary_surface(&d->ssd, 0);
 }
 
 static void qxl_set_irq(PCIQXLDevice *d)
@@ -780,7 +780,9 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 dprint(qxl, 1, "%s\n", __FUNCTION__);
 if (qxl->mode != QXL_MODE_UNDEFINED) {
 qxl->mode = QXL_MODE_UNDEFINED;
-qemu_spice_destroy_primary_surface(&qxl->ssd, 0);
+if (qxl->primary_created) {
+qemu_spice_destroy_primary_surface(&qxl->ssd, 0);
+}
 }
 qxl_soft_reset(qxl);
 }
@@ -912,8 +914,10 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, 
int loadvm,
 {
 QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
 
-assert(qxl->mode != QXL_MODE_NATIVE);
-qxl_exit_vga_mode(qxl);
+if (qxl->mode != QXL_MODE_NATIVE) {
+qxl_exit_vga_mode(qxl);
+}
+assert(!qxl->primary_created);
 
 dprint(qxl, 1, "%s: %dx%d\n", __FUNCTION__,
le32_to_cpu(sc->width), le32_to_cpu(sc->height));
@@ -933,6 +937,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int 
loadvm,
 surface->flags |= QXL_SURF_FLAG_KEEP_DATA;
 }
 
+qxl->primary_created = 1;
 qxl->mode = QXL_MODE_NATIVE;
 qxl->cmdflags = 0;
 
@@ -1156,15 +1161,19 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_DESTROY_PRIMARY:
 PANIC_ON(val != 0);
 dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", 
qxl_mode_to_string(d->mode));
-if (d->mode != QXL_MODE_UNDEFINED) {
-d->mode = QXL_MODE_UNDEFINED;
+if (d->primary_created) {
+d->primary_created = 0;
 qemu_spice_destroy_primary_surface(&d->ssd, 0);
 }
 break;
 case QXL_IO_DESTROY_SURFACE_WAIT:
+if (val == 0) {
+d->primary_created = 0;
+}
 qemu_spice_destroy_surface_wait(&d->ssd, val);
 break;
 case QXL_IO_DESTROY_ALL_SURFACES:
+d->primary_created = 0;
 qemu_spice_destroy_surfaces(&d->ssd);
 break;
 case QXL_IO_FLUSH_SURFACES:
@@ -1209,8 +1218,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_DESTROY_PRIMARY_ASYNC:
 PANIC_ON(val != 0);
 dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC\n");
-if (d->mode != QXL_MODE_UNDEFINED) {
-d->mode = QXL_MODE_UNDEFINED;
+if (d->primary_created) {
+d->primary_created = 0;
 async = qemu_mallocz(sizeof(*async));
 goto async_common;
 } else {
@@ -1218,13 +1227,20 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 break;
 }
 case QXL_IO_UPDATE_AREA_ASYNC:
+dprint(d, 1, "QXL_IO_UPDATE_AREA_ASYNC %d\n", val);
 async = qemu_mallocz(sizeof(*async));
 async->update_area = d->ram->update_area;
 async->update_surface = d->ram->update_surface;
 goto async_common;
 case QXL_IO_NOTIFY_OOM_ASYNC:
+goto async_common;
 case QXL_IO_DESTROY_SURFACE_ASYNC:
+if (val == 0) {
+d->primary_created = 0;
+}
+goto async_common;
 case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+d->primary_created = 0;
 async = qemu_mallocz(sizeof(*async));
 async_common:
 async->port = io_

[Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

When disk is a cdrom and the drive is empty the "params" node in
xenstore might be missing completely: cope with it instead of
segfaulting.

Signed-off-by: Stefano Stabellini 
---
 hw/xen_disk.c |   16 +++-
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 096d1c9..801da58 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -616,11 +616,13 @@ static int blk_init(struct XenDevice *xendev)
 {
 struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
 int index, qflags, have_barriers, info = 0;
-char *h;
+char *h = NULL;
 
 /* read xenstore entries */
 if (blkdev->params == NULL) {
 blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
+if (blkdev->params != NULL)
+h = strchr(blkdev->params, ':');
 h = strchr(blkdev->params, ':');
 if (h != NULL) {
 blkdev->fileproto = blkdev->params;
@@ -672,11 +674,15 @@ static int blk_init(struct XenDevice *xendev)
 /* setup via xenbus -> create new block driver instance */
 xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
 blkdev->bs = bdrv_new(blkdev->dev);
-if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
-  bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) {
-bdrv_delete(blkdev->bs);
-return -1;
+if (blkdev->bs) {
+if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
+bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) 
{
+bdrv_delete(blkdev->bs);
+blkdev->bs = NULL;
+}
 }
+if (!blkdev->bs)
+return -1;
 } else {
 /* setup via qemu cmdline -> already setup for us */
 xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline 
setup)\n");
-- 
1.7.2.3




[Qemu-devel] [PATCH] xen_disk: treat "aio" as "raw"

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

Sometimes the toolstack uses "aio" without an additional format
identifier, in such cases use "raw".

Signed-off-by: Stefano Stabellini 
---
 hw/xen_disk.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 801da58..745127d 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -633,6 +633,8 @@ static int blk_init(struct XenDevice *xendev)
 blkdev->filename  = blkdev->params;
 }
 }
+if (!strcmp("aio", blkdev->fileproto))
+blkdev->fileproto = "raw";
 if (blkdev->mode == NULL) {
 blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
 }
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH 09/14] exec: last_first_tb was only used in !ONLY_USER case

2011-06-24 Thread Stefan Hajnoczi
On Thu, Jun 02, 2011 at 01:53:44PM +0200, Juan Quintela wrote:
> Once there, use a better variable name.
> 
> Signed-off-by: Juan Quintela 
> ---
>  exec.c |   10 +++---
>  1 files changed, 7 insertions(+), 3 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



[Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support

2011-06-24 Thread Alon Levy
Add two new IOs.
 QXL_IO_FLUSH_SURFACES - equivalent to update area for all surfaces, used
  to reduce vmexits from NumSurfaces to 1 on guest S3, S4 and resolution change 
(windows
  driver implementation is such that this is done on each of those occasions).
 QXL_IO_FLUSH_RELEASE - used to ensure anything on last_release is put on the 
release ring
  for the client to free.

Cc: Yonit Halperin 
---
 hw/qxl.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 969a984..fab0208 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1167,6 +1167,30 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_DESTROY_ALL_SURFACES:
 qemu_spice_destroy_surfaces(&d->ssd);
 break;
+case QXL_IO_FLUSH_SURFACES:
+dprint(d, 1, "QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, res#=%d)\n",
+val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+d->num_free_res);
+qemu_spice_stop(&d->ssd);
+qemu_spice_start(&d->ssd);
+dprint(d, 1, "QXL_IO_FLUSH_SURFACES exit (%s, s#=%d, res#=%d,%p)\n",
+qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+d->num_free_res, d->last_release);
+break;
+case QXL_IO_FLUSH_RELEASE: {
+QXLReleaseRing *ring = &d->ram->release_ring;
+if (ring->prod - ring->cons + 1 == ring->num_items) {
+// TODO - "return" a value to the guest and let it loop?
+fprintf(stderr,
+"ERROR: no flush, full release ring [p%d,%dc]\n",
+ring->prod, ring->cons);
+}
+qxl_push_free_res(d, 1 /* flush */);
+dprint(d, 1, "QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n",
+qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+d->num_free_res, d->last_release);
+break;
+}
 case QXL_IO_MEMSLOT_ADD_ASYNC:
 PANIC_ON(val >= NUM_MEMSLOTS);
 PANIC_ON(d->guest_slots[val].active);
-- 
1.7.5.4




[Qemu-devel] [PATCH 0/2] xen: enable PV backends for HVM guests

2011-06-24 Thread Stefano Stabellini
Hi all,
this small patch series enables console, disk and kbd backends for HVM
guests, for the benefit of Linux and Windows PV on HVM drivers.

Stefano Stabellini (2):
  xen: enable console and disk backend in HVM mode
  xen: add vkbd support for PV on HVM guests

 hw/xenfb.c |   19 ---
 xen-all.c  |9 +
 2 files changed, 21 insertions(+), 7 deletions(-)

Cheers,

Stefano



[Qemu-devel] [PATCH 1/2] xen: enable console and disk backend in HVM mode

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

Initialize the Xen console backend and the Xen disk backend even when
running in HVM mode so that PV on HVM drivers can connect to them.

Signed-off-by: Stefano Stabellini 
---
 xen-all.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index b0b2f10..93fa2ee 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -862,6 +862,14 @@ int xen_hvm_init(void)
 cpu_register_phys_memory_client(&state->client);
 state->log_for_dirtybit = NULL;
 
+/* Initialize backend core & drivers */
+if (xen_be_init() != 0) {
+fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
+exit(1);
+}
+xen_be_register("console", &xen_console_ops);
+xen_be_register("qdisk", &xen_blkdev_ops);
+
 return 0;
 }
 
-- 
1.7.2.3




[Qemu-devel] [PATCH 2/2] xen: add vkbd support for PV on HVM guests

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

Register the vkbd backend even when running as device emulator for HVM
guests: it is useful because it doesn't need a frequent timer like usb.

Check whether the XenInput DisplayState has been set in the initialise
state, rather than the input state.
In case the DisplayState hasn't been set and there is no vfb for this
domain, then set the XenInput DisplayState to the default one.

Signed-off-by: Stefano Stabellini 
---
 hw/xenfb.c |   19 ---
 xen-all.c  |1 +
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/xenfb.c b/hw/xenfb.c
index a3301ac..13b9b1b 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -347,13 +347,6 @@ static void xenfb_mouse_event(void *opaque,
 
 static int input_init(struct XenDevice *xendev)
 {
-struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
-
-if (!in->c.ds) {
-xen_be_printf(xendev, 1, "ds not set (yet)\n");
-   return -1;
-}
-
 xenstore_write_be_int(xendev, "feature-abs-pointer", 1);
 return 0;
 }
@@ -363,6 +356,18 @@ static int input_initialise(struct XenDevice *xendev)
 struct XenInput *in = container_of(xendev, struct XenInput, c.xendev);
 int rc;
 
+if (!in->c.ds) {
+char *vfb = xenstore_read_str(NULL, "device/vfb");
+if (vfb == NULL) {
+/* there is no vfb, run vkbd on its own */
+in->c.ds = get_displaystate();
+} else {
+free(vfb);
+xen_be_printf(xendev, 1, "ds not set (yet)\n");
+return -1;
+}
+}
+
 rc = common_bind(&in->c);
 if (rc != 0)
return rc;
diff --git a/xen-all.c b/xen-all.c
index 93fa2ee..6099bff 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -868,6 +868,7 @@ int xen_hvm_init(void)
 exit(1);
 }
 xen_be_register("console", &xen_console_ops);
+xen_be_register("vkbd", &xen_kbdmouse_ops);
 xen_be_register("qdisk", &xen_blkdev_ops);
 
 return 0;
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH 2/2] libcacard: replace copy_string with strndup

2011-06-24 Thread Alon Levy
On Fri, Jun 24, 2011 at 04:37:40PM +0200, Christophe Fergeau wrote:
> copy_string reimplements strndup, this commit removes it and
> replaces all copy_string uses with strndup.
> 

Reviewed-by: Alon Levy 

> Signed-off-by: Christophe Fergeau 
> ---
>  libcacard/vcard_emul_nss.c |   23 ++-
>  1 files changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> index 2a20bd6..de324ba 100644
> --- a/libcacard/vcard_emul_nss.c
> +++ b/libcacard/vcard_emul_nss.c
> @@ -925,17 +925,6 @@ vcard_emul_replay_insertion_events(void)
>  /*
>   *  Silly little functions to help parsing our argument string
>   */
> -static char *
> -copy_string(const char *str, int str_len)
> -{
> -char *new_str;
> -
> -new_str = qemu_malloc(str_len+1);
> -memcpy(new_str, str, str_len);
> -new_str[str_len] = 0;
> -return new_str;
> -}
> -
>  static int
>  count_tokens(const char *str, char token, char token_end)
>  {
> @@ -1054,18 +1043,18 @@ vcard_emul_options(const char *args)
>  }
>  opts->vreader = vreaderOpt;
>  vreaderOpt = &vreaderOpt[opts->vreader_count];
> -vreaderOpt->name = copy_string(name, name_length);
> -vreaderOpt->vname = copy_string(vname, vname_length);
> +vreaderOpt->name = qemu_strndup(name, name_length);
> +vreaderOpt->vname = qemu_strndup(vname, vname_length);
>  vreaderOpt->card_type = type;
>  vreaderOpt->type_params =
> -copy_string(type_params, type_params_length);
> +qemu_strndup(type_params, type_params_length);
>  count = count_tokens(args, ',', ')') + 1;
>  vreaderOpt->cert_count = count;
>  vreaderOpt->cert_name = (char **)qemu_malloc(count*sizeof(char 
> *));
>  for (i = 0; i < count; i++) {
>  const char *cert = args;
>  args = strpbrk(args, ",)");
> -vreaderOpt->cert_name[i] = copy_string(cert, args - cert);
> +vreaderOpt->cert_name[i] = qemu_strndup(cert, args - cert);
>  args = strip(args+1);
>  }
>  if (*args == ')') {
> @@ -1092,7 +1081,7 @@ vcard_emul_options(const char *args)
>  args = strip(args+10);
>  params = args;
>  args = find_blank(args);
> -opts->hw_type_params = copy_string(params, args-params);
> +opts->hw_type_params = qemu_strndup(params, args-params);
>  /* db="/data/base/path" */
>  } else if (strncmp(args, "db=", 3) == 0) {
>  const char *db;
> @@ -1103,7 +1092,7 @@ vcard_emul_options(const char *args)
>  args++;
>  db = args;
>  args = strpbrk(args, "\"\n");
> -opts->nss_db = copy_string(db, args-db);
> +opts->nss_db = qemu_strndup(db, args-db);
>  if (*args != 0) {
>  args++;
>  }
> -- 
> 1.7.5.4
> 
> 



Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node

2011-06-24 Thread Peter Maydell
On 24 June 2011 15:50,   wrote:
>     /* read xenstore entries */
>     if (blkdev->params == NULL) {
>         blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
> +        if (blkdev->params != NULL)
> +            h = strchr(blkdev->params, ':');
>         h = strchr(blkdev->params, ':');

This adds the if () statement but it looks like you forgot to remove
the strchr that is outside the if(), so this will still segfault...
(Also, coding style demands braces.)

You could also make that "char *h" local to this 'if' block.

> @@ -672,11 +674,15 @@ static int blk_init(struct XenDevice *xendev)
>         /* setup via xenbus -> create new block driver instance */
>         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
>         blkdev->bs = bdrv_new(blkdev->dev);
> -        if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> -                      bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) 
> {
> -            bdrv_delete(blkdev->bs);
> -            return -1;
> +        if (blkdev->bs) {
> +            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> +                        bdrv_find_whitelisted_format(blkdev->fileproto)) != 
> 0) {
> +                bdrv_delete(blkdev->bs);
> +                blkdev->bs = NULL;
> +            }
>         }
> +        if (!blkdev->bs)
> +            return -1;

Doesn't this error return leak the strings allocated by
xenstore_read_be_str() ?

-- PMM



Re: [Qemu-devel] [PATCH v4 2/2] Add support for Zipit Z2 machine

2011-06-24 Thread Peter Maydell
On 17 June 2011 11:04, Vasily Khoruzhick  wrote:
> Zipit Z2 is small PXA270 based handheld.
>
> Signed-off-by: Vasily Khoruzhick 

These patches are affected by the bug in current qemu master
which breaks cpu_physical_memory_map() so I haven't tested them yet.
Some minor nitpicks (nearly there now):

> +            if (z->cur_reg == 0x22 && val == 0x) {
> +                z->enabled = 1;
> +                printf("%s: LCD enabled\n", __func__);
> +            } else if (z->cur_reg == 0x10 && val == 0x) {
> +                z->enabled = 0;
> +                printf("%s: LCD disabled\n", __func__);
> +            }

Drop or use DPRINTF for these printfs, please.

> +            break;
> +        default:
> +            fprintf(stderr, "%s: unknown command!\n", __func__);

Ditto.

> +static VMStateDescription vmstate_zipit_lcd_state = {
> +    .name = "zipit-lcd",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT32(enabled, ZipitLCD),
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};

This is missing fields for selected, buf[] and cur_reg.

> +    if (s->len++ > 2) {
> +        fprintf(stderr, "%s: message too long (%i bytes)\n",
> +            __func__, s->len);
> +        return 1;
> +    }

DPRINTF.

> +    case I2C_START_RECV:
> +        if (s->len != 1) {
> +            fprintf(stderr, "%s: short message!?\n", __func__);
> +        }
> +        break;

Ditto.

> +static VMStateDescription vmstate_aer915_state = {
> +    .name = "aer915",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};

Missing fields for len and buf[].

Looks ok otherwise. (Patch 1 looks ok too.)

Have you tried vmload/vmsave, by the way? (I don't know if all the
devices the pxa2xx uses have save/load support implemented, it
would be interesting to check if you haven't already.)

-- PMM



Re: [Qemu-devel] [PATCH] qemu-kvm: fix pulseaudio detection in configure

2011-06-24 Thread Marc-Antoine Perennou
On 10 June 2011 09:14, Markus Armbruster  wrote:
> Peter Maydell  writes:
>
>> On 9 June 2011 18:44, Andreas Färber  wrote:
>>> Am 09.06.2011 um 17:52 schrieb Marc-Antoine Perennou:
 Manually including stddef.h or replacing NULL by 0 or (void*)0 makes it
 work.
>>>
>>> Then please submit a new patch that explicit includes that header with a
>>> comment explaining why, so that it doesn't get removed by accident.
>>
>> I think the original patch is fine. This configure test isn't
>> trying to test whether we correctly found some definition of
>> NULL, it's testing pulseaudio presence. The test code should
>> be the simplest and most stand-alone code that achieves that.
>> There's no need to go dragging in extra headers when we don't
>> even need to be using NULL here anyhow.
>>
>> We already have configure tests which use unadorned 0 for
>> pointers (eg the pthread test), it's legal C, and it works.
>> I think we should call this a trivial patch and just apply it.
>>
>> Reviewed-by: Peter Maydell 
>
> I agree with your reasoning.
>

If you want me to provide a patch with the extra include instead, I
can, otherwise, could this trivial one get merged so qemu-kvm can be
built on several systems on which it actually can't ?



Re: [Qemu-devel] [PATCH 2/2] xen: add vkbd support for PV on HVM guests

2011-06-24 Thread Peter Maydell
On 24 June 2011 15:54,   wrote:

> +    if (!in->c.ds) {
> +        char *vfb = xenstore_read_str(NULL, "device/vfb");
> +        if (vfb == NULL) {
> +            /* there is no vfb, run vkbd on its own */
> +            in->c.ds = get_displaystate();
> +        } else {
> +            free(vfb);

xenstore_read_str() returns a pointer from a qemu_strdup()
so strictly this should be a qemu_free(), I think?

-- PMM



[Qemu-devel] [PATCH 1/2] libcacard: fix soft=... parsing in vcard_emul_options

2011-06-24 Thread Christophe Fergeau
The previous parser had copy and paste errors when computing
vname_length and type_params_length, "name" was used instead
of respectively vname and type_params. This led to length that could
be bigger than the input string, and to access out of the array
bounds when trying to copy these strings. valgrind rightfully
complained about this. It also didn't handle empty fields correctly,
and there were some args = strip(args++); which also didn't do what
was expected.

Since the token parsing is always the same, I factored all the
repetitive code in a NEXT_TOKEN macro.

Signed-off-by: Christophe Fergeau 
---
 libcacard/vcard_emul_nss.c |   90 +++-
 1 files changed, 39 insertions(+), 51 deletions(-)

diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index f3db657..2a20bd6 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -975,13 +975,31 @@ find_blank(const char *str)
 static VCardEmulOptions options;
 #define READER_STEP 4
 
+/* Expects "args" to be at the beginning of a token (ie right after the ','
+ * ending the previous token), and puts the next token start in "token",
+ * and its length in "token_length". "token" will not be nul-terminated.
+ * After calling the macro, "args" will be advanced to the beginning of
+ * the next token.
+ * This macro may call continue or break.
+ */
+#define NEXT_TOKEN(token) \
+(token) = args; \
+args = strpbrk(args, ",)"); \
+if (*args == 0) { \
+break; \
+} \
+if (*args == ')') { \
+args++; \
+continue; \
+} \
+(token##_length) = args - (token); \
+args = strip(args+1);
+
 VCardEmulOptions *
 vcard_emul_options(const char *args)
 {
 int reader_count = 0;
 VCardEmulOptions *opts;
-char type_str[100];
-int type_len;
 
 /* Allow the future use of allocating the options structure on the fly */
 memcpy(&options, &default_options, sizeof(options));
@@ -996,63 +1014,32 @@ vcard_emul_options(const char *args)
  *   cert_2,cert_3...) */
 if (strncmp(args, "soft=", 5) == 0) {
 const char *name;
+size_t name_length;
 const char *vname;
+size_t vname_length;
 const char *type_params;
+size_t type_params_length;
+char type_str[100];
 VCardEmulType type;
-int name_length, vname_length, type_params_length, count, i;
+int count, i;
 VirtualReaderOptions *vreaderOpt = NULL;
 
 args = strip(args + 5);
 if (*args != '(') {
 continue;
 }
-name = args;
-args = strpbrk(args + 1, ",)");
-if (*args == 0) {
-break;
-}
-if (*args == ')') {
-args++;
-continue;
-}
-args = strip(args+1);
-name_length = args - name - 2;
-vname = args;
-args = strpbrk(args + 1, ",)");
-if (*args == 0) {
-break;
-}
-if (*args == ')') {
-args++;
-continue;
-}
-vname_length = args - name - 2;
 args = strip(args+1);
-type_len = strpbrk(args, ",)") - args;
-assert(sizeof(type_str) > type_len);
-strncpy(type_str, args, type_len);
-type_str[type_len] = 0;
+
+NEXT_TOKEN(name)
+NEXT_TOKEN(vname)
+NEXT_TOKEN(type_params)
+type_params_length = MIN(type_params_length, sizeof(type_str)-1);
+strncpy(type_str, type_params, type_params_length);
+type_str[type_params_length] = 0;
 type = vcard_emul_type_from_string(type_str);
-args = strpbrk(args, ",)");
-if (*args == 0) {
-break;
-}
-if (*args == ')') {
-args++;
-continue;
-}
-args = strip(args++);
-type_params = args;
-args = strpbrk(args + 1, ",)");
-if (*args == 0) {
-break;
-}
-if (*args == ')') {
-args++;
-continue;
-}
-type_params_length = args - name;
-args = strip(args++);
+
+NEXT_TOKEN(type_params)
+
 if (*args == 0) {
 break;
 }
@@ -1072,13 +1059,14 @@ vcard_emul_options(const char *args)
 vreaderOpt->card_type = type;
 vreaderOpt->type_params =
 copy_string(type_params, type_params_length);
-count = count_tokens(args, ',', ')');
+count = count_tokens(args, ',', ')') + 1;
 vreaderOpt->cert_count = count;
 vreaderOpt->cert

[Qemu-devel] [PATCH] xen_console: fix memory leak

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

con_init leaks the string "type", fix it.

Signed-off-by: Stefano Stabellini 
---
 hw/xen_console.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/xen_console.c b/hw/xen_console.c
index e88714f..519d5f5 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -180,6 +180,7 @@ static int con_init(struct XenDevice *xendev)
 {
 struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
 char *type, *dom;
+int ret = 0;
 
 /* setup */
 dom = xs_get_domain_path(xenstore, con->xendev.dom);
@@ -189,7 +190,8 @@ static int con_init(struct XenDevice *xendev)
 type = xenstore_read_str(con->console, "type");
 if (!type || strcmp(type, "ioemu") != 0) {
xen_be_printf(xendev, 1, "not for me (type=%s)\n", type);
-   return -1;
+ret = -1;
+goto out;
 }
 
 if (!serial_hds[con->xendev.dev])
@@ -198,7 +200,9 @@ static int con_init(struct XenDevice *xendev)
 else
 con->chr = serial_hds[con->xendev.dev];
 
-return 0;
+out:
+qemu_free(type);
+return ret;
 }
 
 static int con_initialise(struct XenDevice *xendev)
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH] qemu-kvm: fix pulseaudio detection in configure

2011-06-24 Thread Stefan Hajnoczi
On Fri, Apr 29, 2011 at 05:59:19PM +0200, Marc-Antoine Perennou wrote:
> pulse/simple.h does not include stdlib.h
> We cannot use NULL since it may not be defined
> Use 0 instead
> 
> Signed-off-by: Marc-Antoine Perennou 
> ---
>  configure |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

There has been discussion but I hope we've arrived at consensus based on
the fact that 0 is already used for pthreads and it's valid C.

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



Re: [Qemu-devel] [PATCH 2/2] xen: add vkbd support for PV on HVM guests

2011-06-24 Thread Stefano Stabellini
On Fri, 24 Jun 2011, Peter Maydell wrote:
> On 24 June 2011 15:54,   wrote:
> 
> > +    if (!in->c.ds) {
> > +        char *vfb = xenstore_read_str(NULL, "device/vfb");
> > +        if (vfb == NULL) {
> > +            /* there is no vfb, run vkbd on its own */
> > +            in->c.ds = get_displaystate();
> > +        } else {
> > +            free(vfb);
> 
> xenstore_read_str() returns a pointer from a qemu_strdup()
> so strictly this should be a qemu_free(), I think?
 
Yes, it should be, I'll send a second version of the patch.

Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node

2011-06-24 Thread Stefano Stabellini
On Fri, 24 Jun 2011, Peter Maydell wrote:
> On 24 June 2011 15:50,   wrote:
> >     /* read xenstore entries */
> >     if (blkdev->params == NULL) {
> >         blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
> > +        if (blkdev->params != NULL)
> > +            h = strchr(blkdev->params, ':');
> >         h = strchr(blkdev->params, ':');
> 
> This adds the if () statement but it looks like you forgot to remove
> the strchr that is outside the if(), so this will still segfault...
> (Also, coding style demands braces.)
> 
> You could also make that "char *h" local to this 'if' block.

Thank you very much for the review, I'll make the changes.

> 
> > @@ -672,11 +674,15 @@ static int blk_init(struct XenDevice *xendev)
> >         /* setup via xenbus -> create new block driver instance */
> >         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus 
> > setup)\n");
> >         blkdev->bs = bdrv_new(blkdev->dev);
> > -        if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> > -                      bdrv_find_whitelisted_format(blkdev->fileproto)) != 
> > 0) {
> > -            bdrv_delete(blkdev->bs);
> > -            return -1;
> > +        if (blkdev->bs) {
> > +            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> > +                        bdrv_find_whitelisted_format(blkdev->fileproto)) 
> > != 0) {
> > +                bdrv_delete(blkdev->bs);
> > +                blkdev->bs = NULL;
> > +            }
> >         }
> > +        if (!blkdev->bs)
> > +            return -1;
> 
> Doesn't this error return leak the strings allocated by
> xenstore_read_be_str() ?

Another very good point, I'll introduce an out_error label and free
everything there.

[Qemu-devel] [PATCH v2] xen_disk: cope with missing xenstore "params" node

2011-06-24 Thread stefano.stabellini
From: Stefano Stabellini 

When disk is a cdrom and the drive is empty the "params" node in
xenstore might be missing completely: cope with it instead of
segfaulting.


Updated in v2:

- actually removed the strchr(blkdev->params, ':') that caused the
segfault;

- free all the allocated strings from xenstore before returning;

Signed-off-by: Stefano Stabellini 
---
 hw/xen_disk.c |   29 ++---
 1 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 096d1c9..eec05dd 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -616,12 +616,14 @@ static int blk_init(struct XenDevice *xendev)
 {
 struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
 int index, qflags, have_barriers, info = 0;
-char *h;
 
 /* read xenstore entries */
 if (blkdev->params == NULL) {
+char *h = NULL;
 blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
-h = strchr(blkdev->params, ':');
+if (blkdev->params != NULL) {
+h = strchr(blkdev->params, ':');
+}
 if (h != NULL) {
 blkdev->fileproto = blkdev->params;
 blkdev->filename  = h+1;
@@ -649,7 +651,7 @@ static int blk_init(struct XenDevice *xendev)
 blkdev->mode == NULL   ||
 blkdev->type == NULL   ||
 blkdev->dev == NULL) {
-return -1;
+goto out_error;
 }
 
 /* read-only ? */
@@ -672,10 +674,15 @@ static int blk_init(struct XenDevice *xendev)
 /* setup via xenbus -> create new block driver instance */
 xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
 blkdev->bs = bdrv_new(blkdev->dev);
-if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
-  bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) {
-bdrv_delete(blkdev->bs);
-return -1;
+if (blkdev->bs) {
+if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
+bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) 
{
+bdrv_delete(blkdev->bs);
+blkdev->bs = NULL;
+}
+}
+if (!blkdev->bs) {
+goto out_error;
 }
 } else {
 /* setup via qemu cmdline -> already setup for us */
@@ -704,6 +711,14 @@ static int blk_init(struct XenDevice *xendev)
 xenstore_write_be_int(&blkdev->xendev, "sectors",
   blkdev->file_size / blkdev->file_blk);
 return 0;
+
+out_error:
+qemu_free(blkdev->params);
+qemu_free(blkdev->mode);
+qemu_free(blkdev->type);
+qemu_free(blkdev->dev);
+qemu_free(blkdev->devtype);
+return -1;
 }
 
 static int blk_connect(struct XenDevice *xendev)
-- 
1.7.2.3




  1   2   >