date:20100121

Juan Quintela  writes:

> Gleb Natapov  wrote:
>> On Wed, Jan 20, 2010 at 09:14:03PM +0100, Juan Quintela wrote:
[...]
>>> diff --git a/block/vvfat.c b/block/vvfat.c
>>> index 063f731..df957e5 100644
>>> --- a/block/vvfat.c
>>> +++ b/block/vvfat.c
>>> @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
>>>  {
>>> direntry_t* entry=array_get_next(&(s->directory));
>>> entry->attributes=0x28; /* archive | volume label */
>>> -   snprintf((char*)entry->name,11,"QEMU VVFAT");
>>> +   memcpy(entry->name,"QEMU VVF",8);
>>> +   memcpy(entry->extension,"AT ",3);
>>>  }
>>> 
>> Before the change extension was initialized to "AT\0" after it is "AT "
>
> it was paolo who told to do that change.  entries are not 0 ended.
>
> that was his explanation.

Please mention this in the commit message.

[Qemu-devel] Re: Stop using "which" in ./configure

2010-01-21 Thread Juan Quintela

Loïc Minier  wrote:
> On Wed, Jan 20, 2010, Måns Rullgård wrote:
>> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>> Likewise if you set the value first.
>
>  Ok; see attached patches

I still think that path_of is a complication that we don't really need.

"type" command exist in posix, and althought its output is not defined,
if the output of a real command don't show the path that we need (in
this case /usr/bin/install), then type is pretty wrong in my humble
opinion.

With respect of the rest of the patch.  Only real difference is that
mine uses "prog_exists" vs yours use "has" name.

I obviosly think that prog_exists is a more descriptive name, but I
really don't care one way or another.

Can we agree that we don't need path_of, and then just stop playing with
IFS and friends?

Later, Juan.

Re: [Qemu-devel] [PATCH 1/8] virtio: Remove duplicate macro definition for max. virtqueues, bump up the max

Amit, what about renaming hw/virtio-serial.c to a less misleading name
now?

Re: [Qemu-devel] [PATCH 1/8] virtio: Remove duplicate macro definition for max. virtqueues, bump up the max

On (Thu) Jan 21 2010 [10:53:36], Markus Armbruster wrote:
> Amit, what about renaming hw/virtio-serial.c to a less misleading name
> now?

I'm going to push that upstream; Anthony in the past indicated he was ok
with it.

Amit

[Qemu-devel] [PATCH] virtio-console: Rename virtio-serial.c back to virtio-console.c

This file was renamed to ease the reviews of the recent changes
that went in.

Now that the changes are done, rename the file back to its original
name.

Signed-off-by: Amit Shah 
---
 Makefile.objs|2 +-
 hw/{virtio-serial.c => virtio-console.c} |0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename hw/{virtio-serial.c => virtio-console.c} (100%)

diff --git a/Makefile.objs b/Makefile.objs
index 77ff7f6..e791dd5 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -127,7 +127,7 @@ user-obj-y += cutils.o cache-utils.o
 
 hw-obj-y =
 hw-obj-y += loader.o
-hw-obj-y += virtio.o virtio-serial.o
+hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o
 hw-obj-y += watchdog.o
 hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/hw/virtio-serial.c b/hw/virtio-console.c
similarity index 100%
rename from hw/virtio-serial.c
rename to hw/virtio-console.c
-- 
1.6.2.5

[Qemu-devel] [PATCH] virtio-console: Automatically use virtio-serial-bus for the older -virtioconsole invocation

These hunks got dropped off mysteriously during the rebasing of my
virtio-serial series. Thanks go to Markus for noticing it.

Without these fixes, -virtioconsole doesn't actually have any effect.

Signed-off-by: Amit Shah 
Reported-by: Markus Armbruster 
---
 roms/seabios |2 +-
 vl.c |   15 +--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/roms/seabios b/roms/seabios
index 5da6833..8362699 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit 5da68339ecf44677b8f4f115cdf3cb1da46a9f6c
+Subproject commit 8362699269d7b3c816981be0e306d7531aa3ea1d
diff --git a/vl.c b/vl.c
index e070ec9..ae3ee0b 100644
--- a/vl.c
+++ b/vl.c
@@ -288,8 +288,9 @@ static struct {
 { .driver = "isa-parallel", .flag = &default_parallel  },
 { .driver = "isa-fdc",  .flag = &default_floppy},
 { .driver = "ide-drive",.flag = &default_cdrom },
-{ .driver = "virtio-console-pci",   .flag = &default_virtcon   },
-{ .driver = "virtio-console-s390",  .flag = &default_virtcon   },
+{ .driver = "virtio-serial-pci",.flag = &default_virtcon   },
+{ .driver = "virtio-serial-s390",   .flag = &default_virtcon   },
+{ .driver = "virtio-serial",.flag = &default_virtcon   },
 { .driver = "VGA",  .flag = &default_vga   },
 { .driver = "cirrus-vga",   .flag = &default_vga   },
 { .driver = "vmware-svga",  .flag = &default_vga   },
@@ -4623,6 +4624,7 @@ static int virtcon_parse(const char *devname)
 {
 static int index = 0;
 char label[32];
+QemuOpts *bus_opts, *dev_opts;
 
 if (strcmp(devname, "none") == 0)
 return 0;
@@ -4630,6 +4632,13 @@ static int virtcon_parse(const char *devname)
 fprintf(stderr, "qemu: too many virtio consoles\n");
 exit(1);
 }
+
+bus_opts = qemu_opts_create(&qemu_device_opts, NULL, 0);
+qemu_opt_set(bus_opts, "driver", "virtio-serial");
+
+dev_opts = qemu_opts_create(&qemu_device_opts, NULL, 0);
+qemu_opt_set(dev_opts, "driver", "virtconsole");
+
 snprintf(label, sizeof(label), "virtcon%d", index);
 virtcon_hds[index] = qemu_chr_open(label, devname, NULL);
 if (!virtcon_hds[index]) {
@@ -4637,6 +4646,8 @@ static int virtcon_parse(const char *devname)
 devname, strerror(errno));
 return -1;
 }
+qemu_opt_set(dev_opts, "chardev", label);
+
 index++;
 return 0;
 }
-- 
1.6.2.5

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

On (Wed) Jan 13 2010 [12:32:54], Amit Shah wrote:
> > 
> > I'd be very interested in the results of Sparc32 and Sparc64 analyses.
> 
> OK, I added the two targets to the run and got the following result:
> 
> http://amitshah.fedorapeople.org/clang-output/2010-01-13-1/
> 
> The bug count went up from 95 for just x86-64 to 131.

The count currently is at 107:

http://amitshah.fedorapeople.org/clang-output/2010-01-21-1/

A few new ones have been introduced.

Amit

[Qemu-devel] Re: Stop using "which" in ./configure

2010-01-21 Thread Måns Rullgård

Juan Quintela  writes:

> Loïc Minier  wrote:
>> On Wed, Jan 20, 2010, Måns Rullgård wrote:
>>> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>>> Likewise if you set the value first.
>>
>>  Ok; see attached patches
>
> I still think that path_of is a complication that we don't really need.
>
> "type" command exist in posix, and althought its output is not defined,
> if the output of a real command don't show the path that we need (in
> this case /usr/bin/install), then type is pretty wrong in my humble
> opinion.

I think that entire test is wrong, in fact.  It is perfectly possible
for someone on Solaris to install a working "install" command in
/usr/bin.  It is better, if possible, to test whatever "install"
command is in the path, and complain only if it actually fails.

-- 
Måns Rullgård
m...@mansr.com

[Qemu-devel] [PATCH 0/3] Even more read-write/read-only fixes and cleanup

A comment from Christoph Hellwig made me check (and fix) some stuff.
I'm afraid there are some more callers that needs to explicitly ask for 
read-write permissions.
I'd like to get comments about that.

P.S. The qcow2 bug (I introduced) in pre-allocate showed me there's a need to 
check return value.

Naphtali Sprei (3):
  No need anymore for bdrv_set_read_only
  Ask for read-write permissions when opening files where needed
  Read-only device changed to opens it's file for read-only.

 block.c   |7 ---
 block.h   |1 -
 block/bochs.c |6 ++
 block/parallels.c |6 ++
 block/qcow2.c |2 +-
 block/vvfat.c |2 +-
 qemu-img.c|4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 0/3] * SUBJECT HERE *

*** BLURB HERE ***

Naphtali Sprei (3):
  No need anymoe for bdrv_set_read_only
  Ask for read-write permissions when opening files
  Read-only device changed to opens it's file for read-only.

 block.c   |7 ---
 block.h   |1 -
 block/bochs.c |6 ++
 block/parallels.c |6 ++
 block/qcow2.c |2 +-
 block/vvfat.c |2 +-
 qemu-img.c|4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 0/3] * SUBJECT HERE *

*** BLURB HERE ***

Naphtali Sprei (3):
  No need anymore for bdrv_set_read_only
  Ask for read-write permissions when opening files where needed
  Read-only device changed to opens it's file for read-only.

 block.c   |7 ---
 block.h   |1 -
 block/bochs.c |6 ++
 block/parallels.c |6 ++
 block/qcow2.c |2 +-
 block/vvfat.c |2 +-
 qemu-img.c|4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 1/3] No need anymoe for bdrv_set_read_only


Signed-off-by: Naphtali Sprei 
---
 block.c |7 ---
 block.h |1 -
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 73c26ec..8378c18 100644
--- a/block.c
+++ b/block.c
@@ -1009,13 +1009,6 @@ int bdrv_is_read_only(BlockDriverState *bs)
 return bs->read_only;
 }
 
-int bdrv_set_read_only(BlockDriverState *bs, int read_only)
-{
-int ret = bs->read_only;
-bs->read_only = read_only;
-return ret;
-}
-
 int bdrv_is_sg(BlockDriverState *bs)
 {
 return bs->sg;
diff --git a/block.h b/block.h
index fd4e8dd..1aec453 100644
--- a/block.h
+++ b/block.h
@@ -147,7 +147,6 @@ int bdrv_get_type_hint(BlockDriverState *bs);
 int bdrv_get_translation_hint(BlockDriverState *bs);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
-int bdrv_set_read_only(BlockDriverState *bs, int read_only);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
-- 
1.6.3.3

[Qemu-devel] [PATCH 1/3] No need anymore for bdrv_set_read_only


Signed-off-by: Naphtali Sprei 
---
 block.c |7 ---
 block.h |1 -
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 73c26ec..8378c18 100644
--- a/block.c
+++ b/block.c
@@ -1009,13 +1009,6 @@ int bdrv_is_read_only(BlockDriverState *bs)
 return bs->read_only;
 }
 
-int bdrv_set_read_only(BlockDriverState *bs, int read_only)
-{
-int ret = bs->read_only;
-bs->read_only = read_only;
-return ret;
-}
-
 int bdrv_is_sg(BlockDriverState *bs)
 {
 return bs->sg;
diff --git a/block.h b/block.h
index fd4e8dd..1aec453 100644
--- a/block.h
+++ b/block.h
@@ -147,7 +147,6 @@ int bdrv_get_type_hint(BlockDriverState *bs);
 int bdrv_get_translation_hint(BlockDriverState *bs);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
-int bdrv_set_read_only(BlockDriverState *bs, int read_only);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/3] Ask for read-write permissions when opening files

Found some places that seems needs this explicitly, now that
read-write is not the default.

Signed-off-by: Naphtali Sprei 
---
 block/qcow2.c |2 +-
 block/vvfat.c |2 +-
 qemu-img.c|4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6622eba..91835f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -961,7 +961,7 @@ static int qcow_create2(const char *filename, int64_t 
total_size,
 if (prealloc) {
 BlockDriverState *bs;
 bs = bdrv_new("");
-bdrv_open(bs, filename, BDRV_O_CACHE_WB);
+bdrv_open(bs, filename, BDRV_O_CACHE_WB | BDRV_O_RDWR);
 preallocate(bs);
 bdrv_close(bs);
 }
diff --git a/block/vvfat.c b/block/vvfat.c
index 063f731..8e12f92 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2787,7 +2787,7 @@ static int enable_write_target(BDRVVVFATState *s)
 if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0)
return -1;
 s->qcow = bdrv_new("");
-if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0) < 0)
+if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, BDRV_O_RDWR) < 
0)
return -1;
 
 #ifndef _WIN32
diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..cbba4fc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1116,7 +1116,7 @@ static int img_rebase(int argc, char **argv)
 if (!bs)
 error("Not enough memory");
 
-flags = BRDV_O_FLAGS | (unsafe ? BDRV_O_NO_BACKING : 0);
+flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
 if (bdrv_open2(bs, filename, flags, NULL) < 0) {
 error("Could not open '%s'", filename);
 }
@@ -1157,7 +1157,7 @@ static int img_rebase(int argc, char **argv)
 }
 
 bs_new_backing = bdrv_new("new_backing");
-if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS,
+if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS | BDRV_O_RDWR,
 new_backing_drv))
 {
 error("Could not open new backing file '%s'", backing_name);
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/3] Ask for read-write permissions when opening files where needed

Found some places that seems needs this explicitly, now that
read-write is not the default.

Signed-off-by: Naphtali Sprei 
---
 block/qcow2.c |2 +-
 block/vvfat.c |2 +-
 qemu-img.c|4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6622eba..91835f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -961,7 +961,7 @@ static int qcow_create2(const char *filename, int64_t 
total_size,
 if (prealloc) {
 BlockDriverState *bs;
 bs = bdrv_new("");
-bdrv_open(bs, filename, BDRV_O_CACHE_WB);
+bdrv_open(bs, filename, BDRV_O_CACHE_WB | BDRV_O_RDWR);
 preallocate(bs);
 bdrv_close(bs);
 }
diff --git a/block/vvfat.c b/block/vvfat.c
index 063f731..8e12f92 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2787,7 +2787,7 @@ static int enable_write_target(BDRVVVFATState *s)
 if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0)
return -1;
 s->qcow = bdrv_new("");
-if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0) < 0)
+if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, BDRV_O_RDWR) < 
0)
return -1;
 
 #ifndef _WIN32
diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..cbba4fc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1116,7 +1116,7 @@ static int img_rebase(int argc, char **argv)
 if (!bs)
 error("Not enough memory");
 
-flags = BRDV_O_FLAGS | (unsafe ? BDRV_O_NO_BACKING : 0);
+flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
 if (bdrv_open2(bs, filename, flags, NULL) < 0) {
 error("Could not open '%s'", filename);
 }
@@ -1157,7 +1157,7 @@ static int img_rebase(int argc, char **argv)
 }
 
 bs_new_backing = bdrv_new("new_backing");
-if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS,
+if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS | BDRV_O_RDWR,
 new_backing_drv))
 {
 error("Could not open new backing file '%s'", backing_name);
-- 
1.6.3.3

[Qemu-devel] [PATCH 3/3] Read-only device changed to opens it's file for read-only.


Signed-off-by: Naphtali Sprei 
---
 block/bochs.c |6 ++
 block/parallels.c |6 ++
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 3489258..fb83594 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -116,11 +116,9 @@ static int bochs_open(BlockDriverState *bs, const char 
*filename, int flags)
 struct bochs_header bochs;
 struct bochs_header_v1 header_v1;
 
-fd = open(filename, O_RDWR | O_BINARY);
+fd = open(filename, O_RDONLY | O_BINARY);
 if (fd < 0) {
-fd = open(filename, O_RDONLY | O_BINARY);
-if (fd < 0)
-return -1;
+return -1;
 }
 
 bs->read_only = 1; // no write support yet
diff --git a/block/parallels.c b/block/parallels.c
index 63b6738..41b3a7c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -74,11 +74,9 @@ static int parallels_open(BlockDriverState *bs, const char 
*filename, int flags)
 int fd, i;
 struct parallels_header ph;
 
-fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
 if (fd < 0) {
-fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
-if (fd < 0)
-return -1;
+return -1;
 }
 
 bs->read_only = 1; // no write support yet
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH v2 3/8] monitor: New argument type 'b'

On Wed, 20 Jan 2010 17:08:17 +0100
Markus Armbruster  wrote:

> This is a double value with optional suffixes G, g, M, m, K, k.  We'll
> need this to get migrate_set_speed() QMP-ready.

 Nice, not only good for QMP: we're moving this kind of handling
from the handlers to common code, which is the right thing to do.

 The only possible issue is that, if we decide to move all this stuff
to json, such types will make the change complex. But that's something
for the future.

 Some comments follow.

> Signed-off-by: Markus Armbruster 
> ---
>  monitor.c |   58 ++
>  1 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 775fe3f..ce97e7b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -47,6 +47,7 @@
>  #include "kvm.h"
>  #include "acl.h"
>  #include "qint.h"
> +#include "qfloat.h"
>  #include "qlist.h"
>  #include "qdict.h"
>  #include "qbool.h"
> @@ -70,6 +71,10 @@
>   * 'l'  target long (32 or 64 bit)
>   * 'M'  just like 'l', except in user mode the value is
>   *  multiplied by 2^20 (think Mebibyte)
> + * 'b'  double
> + *  user mode accepts an optional G, g, M, m, K, k suffix,
> + *  which multiplies the value by 2^30 for suffixes G and
> + *  g, 2^20 for M and m, 2^10 for K and k
>   * '/'  optional gdb-like print format (like "/10x")
>   *
>   * '?'  optional type (for all types, except '/')
> @@ -3181,6 +3186,27 @@ static int get_expr(Monitor *mon, int64_t *pval, const 
> char **pp)
>  return 0;
>  }
>  
> +static int get_double(Monitor *mon, double *pval, const char **pp)
> +{
> +const char *p = *pp;
> +char *tailp;

 Better to init to NULL?

> +double d;
> +
> +errno = 0;
> +d = strtod(p, &tailp);
> +if (tailp == p) {
> +monitor_printf(mon, "Number expected\n");
> +return -1;
> +}
> +if (errno) {
> +monitor_printf(mon, "Bad number (%s)\n", strerror(errno));
> +return -1;
> +}

 Should we trust errno this way? The manpage only mentions ERANGE.

> +*pval = d;
> +*pp = tailp;
> +return 0;
> +}
> +
>  static int get_str(char *buf, int buf_size, const char **pp)
>  {
>  const char *p;
> @@ -3517,6 +3543,38 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
> *mon,
>  qdict_put(qdict, key, qint_from_int(val));
>  }
>  break;
> +case 'b':
> +{
> +double val;
> +
> +while (qemu_isspace(*p))
> +p++;
> +if (*typestr == '?') {
> +typestr++;
> +if (*p == '\0') {
> +break;
> +}
> +}
> +if (get_double(mon, &val, &p) < 0) {
> +goto fail;
> +}
> +if (*p) {
> +switch (*p) {
> +case 'K': case 'k':
> +val *= 1 << 10; p++; break;
> +case 'M': case 'm':
> +val *= 1 << 20; p++; break;
> +case 'G': case 'g':
> +val *= 1 << 30; p++; break;
> +}
> +}
> +if (*p && !qemu_isspace(*p)) {
> +monitor_printf(mon, "Unknown unit suffix\n");
> +goto fail;
> +}

 A good way to test if 'p' handling is correct, is to write a test
handler which has different types (say, 'foo:b,str:s,bla:i') and print
the values to see if they match what we expect or have hardcoded
to values in a specific test handler...

> +qdict_put(qdict, key, qfloat_from_double(val));
> +}
> +break;
>  case '-':
>  {
>  const char *tmp = p;

Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute

Christoph Hellwig wrote:
> Looking at the version of this that landed in git I don't think the
> read-only handling is entirely clean after this.

I fixed what I could, still I got some questions below.

> 
>  - we now normally set the read_only flag from bdrv_open2 when we do
>not have the O_RDWR flag set
>  - but the block drivers also mess with it:
>   o raw-posix superflously sets it when BDRV_O_RDWR is not in the
> open flags

Not sure where exactly is the issue. Can you please point the line ?

>   o bochs, cloop, dmg and parallels set it unconditionally given
> that they do not support writing at all.  But they do not
> bother to reject opens without BDRV_O_RDWR

I just changed bochs and parallels not to ask for read-write.
Should all of them test the flags for RDWR and returns failure ?

>   o vvfat as usual is a complete mess setting and clearing it in
> various places

Fixed one occurance. More places ?

>  - in addition to that bdrv_open2 also sets it after calling itself for
>the backing hd which seems superflous

Is this a problem ? I thought it's safer to mark it read-only, in case a write 
operation requested somehow.

>  - there also is a now unused bdrv_set_read_only helper to set it from
>outside block.c

Done. Removed.

> 
> 

Thanks,

  Naphtali

Re: [Qemu-devel] [PATCH v2 6/8] monitor: New argument type 'T'

On Wed, 20 Jan 2010 17:08:20 +0100
Markus Armbruster  wrote:

> This is a double value with optional suffixes ms, us, ns.  We'll need
> this to get migrate_set_downtime() QMP-ready.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  monitor.c |   16 +++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index ce97e7b..6dafe0b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -75,6 +75,9 @@
>   *  user mode accepts an optional G, g, M, m, K, k suffix,
>   *  which multiplies the value by 2^30 for suffixes G and
>   *  g, 2^20 for M and m, 2^10 for K and k
> + * 'T'  double
> + *  user mode accepts an optional ms, us, ns suffix,
> + *  which divides the value by 1e3, 1e6, 1e9, respectively
>   * '/'  optional gdb-like print format (like "/10x")
>   *
>   * '?'  optional type (for all types, except '/')
> @@ -3544,6 +3547,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
> *mon,
>  }
>  break;
>  case 'b':
> +case 'T':
>  {
>  double val;
>  
> @@ -3558,7 +3562,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
> *mon,
>  if (get_double(mon, &val, &p) < 0) {
>  goto fail;
>  }
> -if (*p) {
> +if (c == 'b' && *p) {
>  switch (*p) {
>  case 'K': case 'k':
>  val *= 1 << 10; p++; break;
> @@ -3568,6 +3572,16 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
> *mon,
>  val *= 1 << 30; p++; break;
>  }
>  }
> +if (c == 'T' && p[0] && p[1] == 's') {

 Is this indexing of 'p' really correct? What if the value you're interested
is at the of the string? Like:

.args_type = "str:s,value:b"

> +switch (*p) {
> +case 'm':
> +val /= 1e3; p += 2; break;
> +case 'u':
> +val /= 1e6; p += 2; break;
> +case 'n':
> +val /= 1e9; p += 2; break;
> +}
> +}
>  if (*p && !qemu_isspace(*p)) {
>  monitor_printf(mon, "Unknown unit suffix\n");
>  goto fail;

Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute

2010-01-21 Thread Christoph Hellwig

On Thu, Jan 21, 2010 at 03:19:28PM +0200, Naphtali Sprei wrote:
> 
> > 
> >  - we now normally set the read_only flag from bdrv_open2 when we do
> >not have the O_RDWR flag set
> >  - but the block drivers also mess with it:
> > o raw-posix superflously sets it when BDRV_O_RDWR is not in the
> >   open flags
> 
> Not sure where exactly is the issue. Can you please point the line ?

It's really just a now superflous place in the image driver that sets
the read_only flag.  Currently it's not clear who is supposed to set
the flag, we do it both from block.c and the image driver.

> > o bochs, cloop, dmg and parallels set it unconditionally given
> >   that they do not support writing at all.  But they do not
> >   bother to reject opens without BDRV_O_RDWR
> 
> I just changed bochs and parallels not to ask for read-write.
> Should all of them test the flags for RDWR and returns failure ?

That would be most logical, but might cause regressions for existing
setups that did not bother to specify the read-only option on the
command line.  Another options might be to allow the driver to return
EROFS and the retry a read-only open for the block layer for these.

> > o vvfat as usual is a complete mess setting and clearing it in
> >   various places
> 
> Fixed one occurance. More places ?

I mean the ->read_only flag setting and clearing.  As you've pulled
up the main place for setting it to the block layer the drivers
shouldn't mess with it anymore.

> >  - in addition to that bdrv_open2 also sets it after calling itself for
> >the backing hd which seems superflous
> 
> Is this a problem ? I thought it's safer to mark it read-only, in case a 
> write operation requested somehow.

It's superflous, bdrv_open2 always does it based on the argument, so
no need to do it a second time for the snapshot.

[Qemu-devel] Bug ARM NEON instrucion vcvt

2010-01-21 Thread André Bergner

Hi,

I don't know if this is already known. I found the ARM NEON vcvt instruction
mentioned in some fix in the mailing list archive, but not what has been
fixed.

However, I found a bug in the implementation of that instruction. I observe
that the type operands are emulated in the wrong order.
Example: "vcvt.s32.f32  q0,q0" should convert a float in q0 into an integer,
which works correctly on the CPU. With Qemu I have to change the order of
the
operands to get the correct result (i.e. "vcvt.f32.s32  q0,q0").
I did not test all combinations of conversions, just integer and float.

cheers,
André

Re: [Qemu-devel] [PATCH v2 0/5]: Convert pci_info() to QObject

On Wed, 20 Jan 2010 19:22:48 +
Blue Swirl  wrote:

> On Wed, Jan 20, 2010 at 6:11 PM, Luiz Capitulino  
> wrote:
> > On Wed, 20 Jan 2010 18:57:56 +0100
> > Markus Armbruster  wrote:
> >
> >> Luiz Capitulino  writes:
> >>
> >> >  Hi,
> >> >
> >> >  This new version addresses Markus's comments.
> >> >
> >> > changelog
> >> > -
> >> >
> >> > V1 -> V2
> >> >
> >> > - Make class_info's key 'desc' optional
> >> > - Better indentation
> >> > - Doc fixes
> >> >
> >> > V0 -> V1
> >> >
> >> > - Coding style fixes
> >> > - Make 'BAR' and 'IRQ' keys lowercase
> >> > - Add 'irq' key to the documentation
> >> >
> >> >  Thanks.
> >>
> >> Looks good, although one comment still applies: PATCH 3/5 regresses info
> >> pci, 4/5 and 5/5 fix it.  Do we care?  They're separate because they're
> >> untested.
> >
> >  There are two problems here, which apply for those whom emulate
> > the hardware:
> >
> > 1. 'info pci' output will brake with git bisect
> >
> > 2. As the code is untested, it might be broken
> >
> >  Only two 2 seems serious.
> >
> >  Michael, does the sparc image on qemu.org have the hardware
> > in question (pci bridge)?
> 
> Sparc64 has two Simba bridges, but currently they are broken so there
> are no devices behind them. In addition there should be a DEC 21154
> bridge.

 Can the DEC one have devices attached to it?

> There is no Sparc64 test image yet (very few Sparc32 machines did have
> any PCI and we don't emulate them), but you can test the output
> without any images:

 Oh man, thanks a lot!

 I got an abort() :)

Re: [Qemu-devel] [PATCH v2 6/8] monitor: New argument type 'T'

Luiz Capitulino  writes:

> On Wed, 20 Jan 2010 17:08:20 +0100
> Markus Armbruster  wrote:
>
>> This is a double value with optional suffixes ms, us, ns.  We'll need
>> this to get migrate_set_downtime() QMP-ready.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  monitor.c |   16 +++-
>>  1 files changed, 15 insertions(+), 1 deletions(-)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index ce97e7b..6dafe0b 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -75,6 +75,9 @@
>>   *  user mode accepts an optional G, g, M, m, K, k suffix,
>>   *  which multiplies the value by 2^30 for suffixes G and
>>   *  g, 2^20 for M and m, 2^10 for K and k
>> + * 'T'  double
>> + *  user mode accepts an optional ms, us, ns suffix,
>> + *  which divides the value by 1e3, 1e6, 1e9, respectively
>>   * '/'  optional gdb-like print format (like "/10x")
>>   *
>>   * '?'  optional type (for all types, except '/')
>> @@ -3544,6 +3547,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
>> *mon,
>>  }
>>  break;
>>  case 'b':
>> +case 'T':
>>  {
>>  double val;
>>  
>> @@ -3558,7 +3562,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
>> *mon,
>>  if (get_double(mon, &val, &p) < 0) {
>>  goto fail;
>>  }
>> -if (*p) {
>> +if (c == 'b' && *p) {
>>  switch (*p) {
>>  case 'K': case 'k':
>>  val *= 1 << 10; p++; break;
>> @@ -3568,6 +3572,16 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
>> *mon,
>>  val *= 1 << 30; p++; break;
>>  }
>>  }
>> +if (c == 'T' && p[0] && p[1] == 's') {
>
>  Is this indexing of 'p' really correct? What if the value you're interested
> is at the of the string? Like:
>
> .args_type = "str:s,value:b"

p points right behind the floating-point number.  If no characters
follow, it points to the string's terminating zero.  If exactly one
character follows, it points to that one, and the next one is the
terminating zero.  If more than one follow, p points to the first one.

"p[0] && p[1] == 's'" is safe, because p[0] is safe, and if p[0] != 0,
then p[1] is also safe.

Convinced?

>> +switch (*p) {
>> +case 'm':
>> +val /= 1e3; p += 2; break;
>> +case 'u':
>> +val /= 1e6; p += 2; break;
>> +case 'n':
>> +val /= 1e9; p += 2; break;
>> +}
>> +}
>>  if (*p && !qemu_isspace(*p)) {
>>  monitor_printf(mon, "Unknown unit suffix\n");
>>  goto fail;

Re: [Qemu-devel] [PATCH v2 3/8] monitor: New argument type 'b'

Luiz Capitulino  writes:

> On Wed, 20 Jan 2010 17:08:17 +0100
> Markus Armbruster  wrote:
>
>> This is a double value with optional suffixes G, g, M, m, K, k.  We'll
>> need this to get migrate_set_speed() QMP-ready.
>
>  Nice, not only good for QMP: we're moving this kind of handling
> from the handlers to common code, which is the right thing to do.
>
>  The only possible issue is that, if we decide to move all this stuff
> to json, such types will make the change complex. But that's something
> for the future.
>
>  Some comments follow.
>
>> Signed-off-by: Markus Armbruster 
>> ---
>>  monitor.c |   58 ++
>>  1 files changed, 58 insertions(+), 0 deletions(-)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index 775fe3f..ce97e7b 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -47,6 +47,7 @@
>>  #include "kvm.h"
>>  #include "acl.h"
>>  #include "qint.h"
>> +#include "qfloat.h"
>>  #include "qlist.h"
>>  #include "qdict.h"
>>  #include "qbool.h"
>> @@ -70,6 +71,10 @@
>>   * 'l'  target long (32 or 64 bit)
>>   * 'M'  just like 'l', except in user mode the value is
>>   *  multiplied by 2^20 (think Mebibyte)
>> + * 'b'  double
>> + *  user mode accepts an optional G, g, M, m, K, k suffix,
>> + *  which multiplies the value by 2^30 for suffixes G and
>> + *  g, 2^20 for M and m, 2^10 for K and k
>>   * '/'  optional gdb-like print format (like "/10x")
>>   *
>>   * '?'  optional type (for all types, except '/')
>> @@ -3181,6 +3186,27 @@ static int get_expr(Monitor *mon, int64_t *pval, 
>> const char **pp)
>>  return 0;
>>  }
>>  
>> +static int get_double(Monitor *mon, double *pval, const char **pp)
>> +{
>> +const char *p = *pp;
>> +char *tailp;
>
>  Better to init to NULL?

Not necessary, as strtod() sets tailp unconditionally.

>> +double d;
>> +
>> +errno = 0;
>> +d = strtod(p, &tailp);
>> +if (tailp == p) {
>> +monitor_printf(mon, "Number expected\n");
>> +return -1;
>> +}
>> +if (errno) {
>> +monitor_printf(mon, "Bad number (%s)\n", strerror(errno));
>> +return -1;
>> +}
>
>  Should we trust errno this way? The manpage only mentions ERANGE.

Unless we want to ignore errors other than the "Number expected" caught
above, we have to check errno.  strtod() doesn't have a distinct error
value.

I'm not particular about reporting strerror(errno).

>> +*pval = d;
>> +*pp = tailp;
>> +return 0;
>> +}
>> +
>>  static int get_str(char *buf, int buf_size, const char **pp)
>>  {
>>  const char *p;
>> @@ -3517,6 +3543,38 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
>> *mon,
>>  qdict_put(qdict, key, qint_from_int(val));
>>  }
>>  break;
>> +case 'b':
>> +{
>> +double val;
>> +
>> +while (qemu_isspace(*p))
>> +p++;
>> +if (*typestr == '?') {
>> +typestr++;
>> +if (*p == '\0') {
>> +break;
>> +}
>> +}
>> +if (get_double(mon, &val, &p) < 0) {
>> +goto fail;
>> +}
>> +if (*p) {
>> +switch (*p) {
>> +case 'K': case 'k':
>> +val *= 1 << 10; p++; break;
>> +case 'M': case 'm':
>> +val *= 1 << 20; p++; break;
>> +case 'G': case 'g':
>> +val *= 1 << 30; p++; break;
>> +}
>> +}
>> +if (*p && !qemu_isspace(*p)) {
>> +monitor_printf(mon, "Unknown unit suffix\n");
>> +goto fail;
>> +}
>
>  A good way to test if 'p' handling is correct, is to write a test
> handler which has different types (say, 'foo:b,str:s,bla:i') and print
> the values to see if they match what we expect or have hardcoded
> to values in a specific test handler...

Umm, what do you want me to do here?

>> +qdict_put(qdict, key, qfloat_from_double(val));
>> +}
>> +break;
>>  case '-':
>>  {
>>  const char *tmp = p;

[Qemu-devel] Luvalley-5 has been released (with whitepaper!): enables arbitrary OS to run VMs without any modification

2010-01-21 Thread Xiaodong Yi

Luvalley is a lightweight type-1 Virtual Machine Monitor (VMM).
Its part of source codes are derived from KVM to virtualize
CPU instructions and memory management unit (MMU). However, its
overall architecture is completely different from KVM, but somewhat
like Xen. Luvalley runs outside of Linux, just like Xen's architecture.
Any operating system, including Linux, could be used as
Luvalley's scheduler, memory manager, physical device driver provider
and virtual IO device
emulator. Currently, Luvalley supports Linux and Windows. That is to
say, one may run Luvalley to boot a Linux or Windows, and then run
multiple virtualized operating systems on such Linux or Windows.

>From the point of view of Qemu, Luvalley enables Qemu to utilize the
Intel's VT extension to gain much better performance.

If you are interested in Luvalley project, you may download the source
codes as well as the whitepaper from
  http://sourceforge.net/projects/luvalley/

The main changes of this release (Luvalley-5) are:

 * The code derived is updated from KVM-83 to KVM-88

 * Supports both Intel and AMD CPUs

 * Automatically identify Intel and AMD CPUs

This release (Luvalley-5) includes:

 * Luvalley whitepaper (the first edition)

 * Luvalley binary and source code tarball

 * Readme, changelog and release notes files

Re: [Qemu-devel] [PATCH v2 6/8] monitor: New argument type 'T'

On Thu, 21 Jan 2010 14:54:51 +0100
Markus Armbruster  wrote:

> Luiz Capitulino  writes:
> 
> > On Wed, 20 Jan 2010 17:08:20 +0100
> > Markus Armbruster  wrote:
> >
> >> This is a double value with optional suffixes ms, us, ns.  We'll need
> >> this to get migrate_set_downtime() QMP-ready.
> >> 
> >> Signed-off-by: Markus Armbruster 
> >> ---
> >>  monitor.c |   16 +++-
> >>  1 files changed, 15 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/monitor.c b/monitor.c
> >> index ce97e7b..6dafe0b 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -75,6 +75,9 @@
> >>   *  user mode accepts an optional G, g, M, m, K, k suffix,
> >>   *  which multiplies the value by 2^30 for suffixes G and
> >>   *  g, 2^20 for M and m, 2^10 for K and k
> >> + * 'T'  double
> >> + *  user mode accepts an optional ms, us, ns suffix,
> >> + *  which divides the value by 1e3, 1e6, 1e9, respectively
> >>   * '/'  optional gdb-like print format (like "/10x")
> >>   *
> >>   * '?'  optional type (for all types, except '/')
> >> @@ -3544,6 +3547,7 @@ static const mon_cmd_t 
> >> *monitor_parse_command(Monitor *mon,
> >>  }
> >>  break;
> >>  case 'b':
> >> +case 'T':
> >>  {
> >>  double val;
> >>  
> >> @@ -3558,7 +3562,7 @@ static const mon_cmd_t 
> >> *monitor_parse_command(Monitor *mon,
> >>  if (get_double(mon, &val, &p) < 0) {
> >>  goto fail;
> >>  }
> >> -if (*p) {
> >> +if (c == 'b' && *p) {
> >>  switch (*p) {
> >>  case 'K': case 'k':
> >>  val *= 1 << 10; p++; break;
> >> @@ -3568,6 +3572,16 @@ static const mon_cmd_t 
> >> *monitor_parse_command(Monitor *mon,
> >>  val *= 1 << 30; p++; break;
> >>  }
> >>  }
> >> +if (c == 'T' && p[0] && p[1] == 's') {
> >
> >  Is this indexing of 'p' really correct? What if the value you're interested
> > is at the of the string? Like:
> >
> > .args_type = "str:s,value:b"
> 
> p points right behind the floating-point number.  If no characters
> follow, it points to the string's terminating zero.  If exactly one
> character follows, it points to that one, and the next one is the
> terminating zero.  If more than one follow, p points to the first one.
> 
> "p[0] && p[1] == 's'" is safe, because p[0] is safe, and if p[0] != 0,
> then p[1] is also safe.
> 
> Convinced?

 Yes, looks fine.

> 
> >> +switch (*p) {
> >> +case 'm':
> >> +val /= 1e3; p += 2; break;
> >> +case 'u':
> >> +val /= 1e6; p += 2; break;
> >> +case 'n':
> >> +val /= 1e9; p += 2; break;
> >> +}
> >> +}
> >>  if (*p && !qemu_isspace(*p)) {
> >>  monitor_printf(mon, "Unknown unit suffix\n");
> >>  goto fail;

Re: [Qemu-devel] [PATCH v2 3/8] monitor: New argument type 'b'

On Thu, 21 Jan 2010 15:04:43 +0100
Markus Armbruster  wrote:

> Luiz Capitulino  writes:
> 
> > On Wed, 20 Jan 2010 17:08:17 +0100
> > Markus Armbruster  wrote:
> >
> >> This is a double value with optional suffixes G, g, M, m, K, k.  We'll
> >> need this to get migrate_set_speed() QMP-ready.
> >
> >  Nice, not only good for QMP: we're moving this kind of handling
> > from the handlers to common code, which is the right thing to do.
> >
> >  The only possible issue is that, if we decide to move all this stuff
> > to json, such types will make the change complex. But that's something
> > for the future.
> >
> >  Some comments follow.
> >
> >> Signed-off-by: Markus Armbruster 
> >> ---
> >>  monitor.c |   58 
> >> ++
> >>  1 files changed, 58 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/monitor.c b/monitor.c
> >> index 775fe3f..ce97e7b 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -47,6 +47,7 @@
> >>  #include "kvm.h"
> >>  #include "acl.h"
> >>  #include "qint.h"
> >> +#include "qfloat.h"
> >>  #include "qlist.h"
> >>  #include "qdict.h"
> >>  #include "qbool.h"
> >> @@ -70,6 +71,10 @@
> >>   * 'l'  target long (32 or 64 bit)
> >>   * 'M'  just like 'l', except in user mode the value is
> >>   *  multiplied by 2^20 (think Mebibyte)
> >> + * 'b'  double
> >> + *  user mode accepts an optional G, g, M, m, K, k suffix,
> >> + *  which multiplies the value by 2^30 for suffixes G and
> >> + *  g, 2^20 for M and m, 2^10 for K and k
> >>   * '/'  optional gdb-like print format (like "/10x")
> >>   *
> >>   * '?'  optional type (for all types, except '/')
> >> @@ -3181,6 +3186,27 @@ static int get_expr(Monitor *mon, int64_t *pval, 
> >> const char **pp)
> >>  return 0;
> >>  }
> >>  
> >> +static int get_double(Monitor *mon, double *pval, const char **pp)
> >> +{
> >> +const char *p = *pp;
> >> +char *tailp;
> >
> >  Better to init to NULL?
> 
> Not necessary, as strtod() sets tailp unconditionally.

 Ok.

> >> +double d;
> >> +
> >> +errno = 0;
> >> +d = strtod(p, &tailp);
> >> +if (tailp == p) {
> >> +monitor_printf(mon, "Number expected\n");
> >> +return -1;
> >> +}
> >> +if (errno) {
> >> +monitor_printf(mon, "Bad number (%s)\n", strerror(errno));
> >> +return -1;
> >> +}
> >
> >  Should we trust errno this way? The manpage only mentions ERANGE.
> 
> Unless we want to ignore errors other than the "Number expected" caught
> above, we have to check errno.  strtod() doesn't have a distinct error
> value.
> 
> I'm not particular about reporting strerror(errno).

 Ok, no big deal. I just tend to do strictly what the manpages says.

> >> +*pval = d;
> >> +*pp = tailp;
> >> +return 0;
> >> +}
> >> +
> >>  static int get_str(char *buf, int buf_size, const char **pp)
> >>  {
> >>  const char *p;
> >> @@ -3517,6 +3543,38 @@ static const mon_cmd_t 
> >> *monitor_parse_command(Monitor *mon,
> >>  qdict_put(qdict, key, qint_from_int(val));
> >>  }
> >>  break;
> >> +case 'b':
> >> +{
> >> +double val;
> >> +
> >> +while (qemu_isspace(*p))
> >> +p++;
> >> +if (*typestr == '?') {
> >> +typestr++;
> >> +if (*p == '\0') {
> >> +break;
> >> +}
> >> +}
> >> +if (get_double(mon, &val, &p) < 0) {
> >> +goto fail;
> >> +}
> >> +if (*p) {
> >> +switch (*p) {
> >> +case 'K': case 'k':
> >> +val *= 1 << 10; p++; break;
> >> +case 'M': case 'm':
> >> +val *= 1 << 20; p++; break;
> >> +case 'G': case 'g':
> >> +val *= 1 << 30; p++; break;
> >> +}
> >> +}
> >> +if (*p && !qemu_isspace(*p)) {
> >> +monitor_printf(mon, "Unknown unit suffix\n");
> >> +goto fail;
> >> +}
> >
> >  A good way to test if 'p' handling is correct, is to write a test
> > handler which has different types (say, 'foo:b,str:s,bla:i') and print
> > the values to see if they match what we expect or have hardcoded
> > to values in a specific test handler...
> 
> Umm, what do you want me to do here?

 Just a suggestion on how I would test this stuff.

> 
> >> +qdict_put(qdict, key, qfloat_from_double(val));
> >> +}
> >> +break;
> >>  case '-':
> >>  {
> >>  const char *tmp = p;

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

2010-01-21 Thread Andre Przywara

john cooper wrote:

Chris Wright wrote:

* Daniel P. Berrange (berra...@redhat.com) wrote:

To be honest all possible naming schemes for '-cpu ' are just as
unfriendly as each other. The only user friendly option is '-cpu host'.

IMHO, we should just pick a concise naming scheme & document it. Given
they are all equally unfriendly, the one that has consistency with vmware
naming seems like a mild winner.

Heh, I completely agree, and was just saying the same thing to John
earlier today. May as well be -cpu {foo,bar,baz} since the meaning for
those command line options must be well-documented in the man page.

I can appreciate the concern of wanting to get this
as "correct" as possible. But ultimately we just
need three unique tags which ideally have some relation
to their associated architectures. The diatribes
available from /proc/cpuinfo while generally accurate
don't really offer any more of a clue to the model
group, and in their unmodified form are rather unwieldy
as command line flags.
I agree. I'd underline that this patch is for migration purposes only,
so you don't want to specify an exact CPU, but more like a class of
CPUs. If you look into the available CPUID features in each CPU, you
will find that there are only a few groups, with currently three for
each vendor being a good guess.
/proc/cpuinfo just prints out marketing names, which have only a mild
relationship to a feature-related technical CPU model. Maybe we can use
a generation approach like the AMD Opteron ones for Intel, too.

These G1/G2/G3 names are just arbitrary and have no roots within AMD.

I think that an exact CPU model specification is out of scope for this
patch and maybe even for QEMU. One could create a database with CPU
names and associated CPUID flags and provide an external tool to
generate a QEMU command line out of this. Keeping this database
up-to-date (especially for desktop CPU models) is a burden that the QEMU
project does not want to bear.

This is from an EVC kb article[1]:

Here is a pointer to a more detailed version:

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1003212

We probably should also add an option to dump out the
full set of qemu-side cpuid flags for the benefit of
users and upper level tools.

You mean like this one?
http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg01228.html
Resending this patch set is on my plan for next week. What is the state
of this patch? Will it go in soon? Then I'd rebase my patch set on top
of it.

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

Re: [Qemu-devel] [PATCH v2 3/8] monitor: New argument type 'b'

Luiz Capitulino  writes:

> On Thu, 21 Jan 2010 15:04:43 +0100
> Markus Armbruster  wrote:
>
>> Luiz Capitulino  writes:
>> 
>> > On Wed, 20 Jan 2010 17:08:17 +0100
>> > Markus Armbruster  wrote:
>> >
>> >> This is a double value with optional suffixes G, g, M, m, K, k.  We'll
>> >> need this to get migrate_set_speed() QMP-ready.
>> >
>> >  Nice, not only good for QMP: we're moving this kind of handling
>> > from the handlers to common code, which is the right thing to do.
>> >
>> >  The only possible issue is that, if we decide to move all this stuff
>> > to json, such types will make the change complex. But that's something
>> > for the future.
>> >
>> >  Some comments follow.
>> >
>> >> Signed-off-by: Markus Armbruster 
>> >> ---
>> >>  monitor.c |   58 
>> >> ++
>> >>  1 files changed, 58 insertions(+), 0 deletions(-)
>> >> 
>> >> diff --git a/monitor.c b/monitor.c
>> >> index 775fe3f..ce97e7b 100644
>> >> --- a/monitor.c
>> >> +++ b/monitor.c
>> >> @@ -47,6 +47,7 @@
>> >>  #include "kvm.h"
>> >>  #include "acl.h"
>> >>  #include "qint.h"
>> >> +#include "qfloat.h"
>> >>  #include "qlist.h"
>> >>  #include "qdict.h"
>> >>  #include "qbool.h"
>> >> @@ -70,6 +71,10 @@
>> >>   * 'l'  target long (32 or 64 bit)
>> >>   * 'M'  just like 'l', except in user mode the value is
>> >>   *  multiplied by 2^20 (think Mebibyte)
>> >> + * 'b'  double
>> >> + *  user mode accepts an optional G, g, M, m, K, k suffix,
>> >> + *  which multiplies the value by 2^30 for suffixes G and
>> >> + *  g, 2^20 for M and m, 2^10 for K and k
>> >>   * '/'  optional gdb-like print format (like "/10x")
>> >>   *
>> >>   * '?'  optional type (for all types, except '/')
>> >> @@ -3181,6 +3186,27 @@ static int get_expr(Monitor *mon, int64_t *pval, 
>> >> const char **pp)
>> >>  return 0;
>> >>  }
>> >>  
>> >> +static int get_double(Monitor *mon, double *pval, const char **pp)
>> >> +{
>> >> +const char *p = *pp;
>> >> +char *tailp;
>> >
>> >  Better to init to NULL?
>> 
>> Not necessary, as strtod() sets tailp unconditionally.
>
>  Ok.
>
>> >> +double d;
>> >> +
>> >> +errno = 0;
>> >> +d = strtod(p, &tailp);
>> >> +if (tailp == p) {
>> >> +monitor_printf(mon, "Number expected\n");
>> >> +return -1;
>> >> +}
>> >> +if (errno) {
>> >> +monitor_printf(mon, "Bad number (%s)\n", strerror(errno));
>> >> +return -1;
>> >> +}
>> >
>> >  Should we trust errno this way? The manpage only mentions ERANGE.
>> 
>> Unless we want to ignore errors other than the "Number expected" caught
>> above, we have to check errno.  strtod() doesn't have a distinct error
>> value.
>> 
>> I'm not particular about reporting strerror(errno).
>
>  Ok, no big deal. I just tend to do strictly what the manpages says.

We might want to ignore underflow silently, and fail loudly for ininity
and NaN.  Opinions?

[...]

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

On 01/20/2010 07:18 PM, john cooper wrote:

Chris Wright wrote:

* Daniel P. Berrange (berra...@redhat.com) wrote:

To be honest all possible naming schemes for '-cpu' are just as
unfriendly as each other. The only user friendly option is '-cpu host'.

IMHO, we should just pick a concise naming scheme& document it. Given
they are all equally unfriendly, the one that has consistency with vmware
naming seems like a mild winner.

I can appreciate the concern of wanting to get this
as "correct" as possible.

This is the root of the trouble. At the qemu layer, we try to focus on
being correct.

Management tools are typically the layer that deals with being "correct".

A good compromise is making things user tunable which means that a
downstream can make "correctness" decisions without forcing those
decisions on upstream.

In this case, the idea would be to introduce a new option, say something
like -cpu-def. The syntax would be:

-cpu-def
name=coreduo,level=10,family=6,model=14,stepping=8,features=+vme+mtrr+clflush+mca+sse3+monitor,xlevel=0x8008,model_id="Genuine
Intel(R) CPU T2600 @ 2.16GHz"

Which is not that exciting since it just lets you do -cpu coreduo in a
much more complex way. However, if we take advantage of the current
config support, you can have:

[cpu-def]
name=coreduo
level=10
family=6
model=14
stepping=8
features="+vme+mtrr+clflush+mca+sse3.."
model_id="Genuine Intel..."

And that can be stored in a config file. We should then parse
/etc/qemu/target-.conf by default. We'll move the current
x86_defs table into this config file and then downstreams/users can
define whatever compatibility classes they want.

With this feature, I'd be inclined to take "correct" compatibility
classes like Nehalem as part of the default qemurc that we install
because it's easily overridden by a user. It then becomes just a
suggestion on our part verses a guarantee.

It should just be a matter of adding qemu_cpudefs_opts to
qemu-config.[ch], taking a new command line that parses the argument via
QemuOpts, then passing the parsed options to a target-specific function
that then builds the table of supported cpus.

Regards,

Anthony Liguori

Re: [Qemu-devel] Luvalley-5 has been released (with whitepaper!): enables arbitrary OS to run VMs without any modification

2010-01-21 Thread Alexander Graf

On 21.01.2010, at 15:19, Xiaodong Yi wrote:

> Luvalley is a lightweight type-1 Virtual Machine Monitor (VMM).
> Its part of source codes are derived from KVM to virtualize
> CPU instructions and memory management unit (MMU). However, its
> overall architecture is completely different from KVM, but somewhat
> like Xen. Luvalley runs outside of Linux, just like Xen's architecture.
> Any operating system, including Linux, could be used as
> Luvalley's scheduler, memory manager, physical device driver provider
> and virtual IO device
> emulator. Currently, Luvalley supports Linux and Windows. That is to
> say, one may run Luvalley to boot a Linux or Windows, and then run
> multiple virtualized operating systems on such Linux or Windows.
> 
> From the point of view of Qemu, Luvalley enables Qemu to utilize the
> Intel's VT extension to gain much better performance.
> 
> If you are interested in Luvalley project, you may download the source
> codes as well as the whitepaper from
>  http://sourceforge.net/projects/luvalley/
> 
> The main changes of this release (Luvalley-5) are:
> 
> * The code derived is updated from KVM-83 to KVM-88

It might be a better idea to use upstream kernel sources as basis. The KVM 
snapshots are rather deprecated FWIW.

Is the code to leverage Luvally vastly different from the accessors for KVM? 
Maybe it'd be enough to have a wrapper for kvm_ioctl() that sends ioctls off to 
Luvally instead of KVM to make the existing infrastructure work. That way 
upstream support should be a no-brainer and you get all the upstream qemu work 
for free.

Also, this would finally make the windows builds more useful again.

Alex

[Qemu-devel] [PATCH] qcow2: rename two QCowAIOCB members

2010-01-21 Thread Christoph Hellwig

The n member is not very descriptive and very hard to grep, rename it to
cur_nr_sectors to better indicate what it is used for.  Also rename
nb_sectors to remaining_sectors as that is what it is used for.

Signed-off-by: Christoph Hellwig 

Index: qemu/block/qcow2.c
===
--- qemu.orig/block/qcow2.c 2010-01-21 16:01:04.712004060 +0100
+++ qemu/block/qcow2.c  2010-01-21 16:07:56.059006498 +0100
@@ -332,8 +332,8 @@ typedef struct QCowAIOCB {
 QEMUIOVector *qiov;
 uint8_t *buf;
 void *orig_buf;
-int nb_sectors;
-int n;
+int remaining_sectors;
+int cur_nr_sectors;/* number of sectors in current iteration */
 uint64_t cluster_offset;
 uint8_t *cluster_data;
 BlockDriverAIOCB *hd_aiocb;
@@ -399,38 +399,38 @@ static void qcow_aio_read_cb(void *opaqu
 } else {
 if (s->crypt_method) {
 qcow2_encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
-acb->n, 0,
+acb->cur_nr_sectors, 0,
 &s->aes_decrypt_key);
 }
 }
 
-acb->nb_sectors -= acb->n;
-acb->sector_num += acb->n;
-acb->buf += acb->n * 512;
+acb->remaining_sectors -= acb->cur_nr_sectors;
+acb->sector_num += acb->cur_nr_sectors;
+acb->buf += acb->cur_nr_sectors * 512;
 
-if (acb->nb_sectors == 0) {
+if (acb->remaining_sectors == 0) {
 /* request completed */
 ret = 0;
 goto done;
 }
 
 /* prepare next AIO request */
-acb->n = acb->nb_sectors;
-acb->cluster_offset =
-qcow2_get_cluster_offset(bs, acb->sector_num << 9, &acb->n);
+acb->cur_nr_sectors = acb->remaining_sectors;
+acb->cluster_offset = qcow2_get_cluster_offset(bs, acb->sector_num << 9,
+   &acb->cur_nr_sectors);
 index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
 
 if (!acb->cluster_offset) {
 if (bs->backing_hd) {
 /* read from the base image */
 n1 = qcow2_backing_read1(bs->backing_hd, acb->sector_num,
-   acb->buf, acb->n);
+   acb->buf, acb->cur_nr_sectors);
 if (n1 > 0) {
 acb->hd_iov.iov_base = (void *)acb->buf;
-acb->hd_iov.iov_len = acb->n * 512;
+acb->hd_iov.iov_len = acb->cur_nr_sectors * 512;
 qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
 acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num,
-&acb->hd_qiov, acb->n,
+&acb->hd_qiov, acb->cur_nr_sectors,
qcow_aio_read_cb, acb);
 if (acb->hd_aiocb == NULL)
 goto done;
@@ -441,7 +441,7 @@ static void qcow_aio_read_cb(void *opaqu
 }
 } else {
 /* Note: in this case, no need to wait */
-memset(acb->buf, 0, 512 * acb->n);
+memset(acb->buf, 0, 512 * acb->cur_nr_sectors);
 ret = qcow_schedule_bh(qcow_aio_read_bh, acb);
 if (ret < 0)
 goto done;
@@ -450,8 +450,8 @@ static void qcow_aio_read_cb(void *opaqu
 /* add AIO support for compressed blocks ? */
 if (qcow2_decompress_cluster(s, acb->cluster_offset) < 0)
 goto done;
-memcpy(acb->buf,
-   s->cluster_cache + index_in_cluster * 512, 512 * acb->n);
+memcpy(acb->buf, s->cluster_cache + index_in_cluster * 512,
+   512 * acb->cur_nr_sectors);
 ret = qcow_schedule_bh(qcow_aio_read_bh, acb);
 if (ret < 0)
 goto done;
@@ -462,11 +462,12 @@ static void qcow_aio_read_cb(void *opaqu
 }
 
 acb->hd_iov.iov_base = (void *)acb->buf;
-acb->hd_iov.iov_len = acb->n * 512;
+acb->hd_iov.iov_len = acb->cur_nr_sectors * 512;
 qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
 acb->hd_aiocb = bdrv_aio_readv(s->hd,
 (acb->cluster_offset >> 9) + index_in_cluster,
-&acb->hd_qiov, acb->n, qcow_aio_read_cb, acb);
+&acb->hd_qiov, acb->cur_nr_sectors,
+qcow_aio_read_cb, acb);
 if (acb->hd_aiocb == NULL)
 goto done;
 }
@@ -500,8 +501,8 @@ static QCowAIOCB *qcow_aio_setup(BlockDr
 } else {
 acb->buf = (uint8_t *)qiov->iov->iov_base;
 }
-acb->nb_sectors = nb_sectors;
-acb->n = 0;
+acb->remaining_sectors = nb_sectors;
+acb->cur_nr_sectors = 0;
 acb->cluster_offset = 0;
 acb->l2meta.nb_clusters = 0;
 QLIST_INIT(&acb->l2meta.dependent_requests);
@@ -569,25 +570,26 @@ static void qcow_aio_write_cb(void *opaq
 if (ret < 0)
 goto done;
 
-acb->nb_sectors -= acb->n;
-acb->sect

[Qemu-devel] [PATCH v2 0/4] Reduce down time during migration without shared storage

This series of patches reduce the down time of the guest during a migration
without shared storage. It does that by start transfer dirty blocks in the 
iterative phase. In the current code transferring of dirty blocks begins only 
during the full phase while the guest is suspended. Therefore the guest will 
be suspended linear to the amount of data that was written to disk during
migration.

Changes from v1: - infer storage performance by get_clock()
- remove dirty max iterations - user is responsible for migration convergence
- remove trailing whitespaces
- minor cleanups

 block-migration.c |  244 +++--
 block.c   |   20 -
 block.h   |1 +
 block_int.h   |1 +
 4 files changed, 181 insertions(+), 85 deletions(-)

Signed-off-by: Liran Schour

[Qemu-devel] [PATCH v2 1/4] Remove unused code

blk_mig_save_bulked_block is never called with sync flag. Remove the sync
flag. Calculate bulk completion during blk_mig_save_bulked_block.

Changes from v1: remove trailing whitespaces and minor cleanups.

Signed-off-by: Liran Schour 
---
 block-migration.c |   59 +---
 1 files changed, 19 insertions(+), 40 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 258a88a..f9bb42c 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -72,6 +72,7 @@ typedef struct BlkMigState {
 int transferred;
 int64_t total_sector_sum;
 int prev_progress;
+int bulk_completed;
 } BlkMigState;
 
 static BlkMigState block_mig_state;
@@ -138,7 +139,7 @@ static void blk_mig_read_cb(void *opaque, int ret)
 }
 
 static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
-BlkMigDevState *bmds, int is_async)
+BlkMigDevState *bmds)
 {
 int64_t total_sectors = bmds->total_sectors;
 int64_t cur_sector = bmds->cur_sector;
@@ -175,27 +176,16 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
 blk->bmds = bmds;
 blk->sector = cur_sector;
 
-if (is_async) {
-blk->iov.iov_base = blk->buf;
-blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
-qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
-
-blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
-nr_sectors, blk_mig_read_cb, blk);
-if (!blk->aiocb) {
-goto error;
-}
-block_mig_state.submitted++;
-} else {
-if (bdrv_read(bs, cur_sector, blk->buf, nr_sectors) < 0) {
-goto error;
-}
-blk_send(f, blk);
+blk->iov.iov_base = blk->buf;
+blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
+qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
 
-qemu_free(blk->buf);
-qemu_free(blk);
+blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
+nr_sectors, blk_mig_read_cb, blk);
+if (!blk->aiocb) {
+goto error;
 }
-
+block_mig_state.submitted++;
 bdrv_reset_dirty(bs, cur_sector, nr_sectors);
 bmds->cur_sector = cur_sector + nr_sectors;
 
@@ -229,6 +219,7 @@ static void init_blk_migration(Monitor *mon, QEMUFile *f)
 block_mig_state.transferred = 0;
 block_mig_state.total_sector_sum = 0;
 block_mig_state.prev_progress = -1;
+block_mig_state.bulk_completed = 0;
 
 for (bs = bdrv_first; bs != NULL; bs = bs->next) {
 if (bs->type == BDRV_TYPE_HD) {
@@ -260,7 +251,7 @@ static void init_blk_migration(Monitor *mon, QEMUFile *f)
 }
 }
 
-static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f, int is_async)
+static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile *f)
 {
 int64_t completed_sector_sum = 0;
 BlkMigDevState *bmds;
@@ -269,7 +260,7 @@ static int blk_mig_save_bulked_block(Monitor *mon, QEMUFile 
*f, int is_async)
 
 QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
 if (bmds->bulk_completed == 0) {
-if (mig_save_device_bulk(mon, f, bmds, is_async) == 1) {
+if (mig_save_device_bulk(mon, f, bmds) == 1) {
 /* completed bulk section for this device */
 bmds->bulk_completed = 1;
 }
@@ -362,19 +353,8 @@ static void flush_blks(QEMUFile* f)
 
 static int is_stage2_completed(void)
 {
-BlkMigDevState *bmds;
-
-if (block_mig_state.submitted > 0) {
-return 0;
-}
-
-QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-if (bmds->bulk_completed == 0) {
-return 0;
-}
-}
-
-return 1;
+return (block_mig_state.submitted == 0 &&
+   block_mig_state.bulk_completed);
 }
 
 static void blk_mig_cleanup(Monitor *mon)
@@ -432,8 +412,9 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int 
stage, void *opaque)
 while ((block_mig_state.submitted +
 block_mig_state.read_done) * BLOCK_SIZE <
qemu_file_get_rate_limit(f)) {
-if (blk_mig_save_bulked_block(mon, f, 1) == 0) {
-/* no more bulk blocks for now */
+if (blk_mig_save_bulked_block(mon, f) == 0) {
+/* finish saving bulk on all devices */
+block_mig_state.bulk_completed = 1;
 break;
 }
 }
@@ -446,9 +427,7 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int 
stage, void *opaque)
 }
 
 if (stage == 3) {
-while (blk_mig_save_bulked_block(mon, f, 0) != 0) {
-/* empty */
-}
+/* we now for sure that save bulk is completed */
 
 blk_mig_save_dirty_blocks(mon, f);
 blk_mig_cleanup(mon);
-- 
1.6.0.4

[Qemu-devel] [PATCH v2 3/4] Count dirty blocks and expose an API to get dirty count

This will manage dirty counter for each device and will allow to get the
dirty counter from above.

Changes from v1: remove trailing whitespaces.

Signed-off-by: Liran Schour 
---
 block.c |   16 ++--
 block.h |1 +
 block_int.h |1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 30ae2b1..a6381ad 100644
--- a/block.c
+++ b/block.c
@@ -653,9 +653,15 @@ static void set_dirty_bitmap(BlockDriverState *bs, int64_t 
sector_num,
 bit = start % (sizeof(unsigned long) * 8);
 val = bs->dirty_bitmap[idx];
 if (dirty) {
-val |= 1 << bit;
+if (!(val & (1 << bit))) {
+bs->dirty_count++;
+val |= 1 << bit;
+}
 } else {
-val &= ~(1 << bit);
+if (val & (1 << bit)) {
+bs->dirty_count--;
+val &= ~(1 << bit);
+}
 }
 bs->dirty_bitmap[idx] = val;
 }
@@ -2116,6 +2122,7 @@ void bdrv_set_dirty_tracking(BlockDriverState *bs, int 
enable)
 {
 int64_t bitmap_size;
 
+bs->dirty_count = 0;
 if (enable) {
 if (!bs->dirty_bitmap) {
 bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS) +
@@ -2150,3 +2157,8 @@ void bdrv_reset_dirty(BlockDriverState *bs, int64_t 
cur_sector,
 {
 set_dirty_bitmap(bs, cur_sector, nr_sectors, 0);
 }
+
+int64_t bdrv_get_dirty_count(BlockDriverState *bs)
+{
+return bs->dirty_count;
+}
diff --git a/block.h b/block.h
index fa51ddf..1012303 100644
--- a/block.h
+++ b/block.h
@@ -201,4 +201,5 @@ void bdrv_set_dirty_tracking(BlockDriverState *bs, int 
enable);
 int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
 void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
   int nr_sectors);
+int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 #endif
diff --git a/block_int.h b/block_int.h
index 9a3b2e0..8d5d9bc 100644
--- a/block_int.h
+++ b/block_int.h
@@ -172,6 +172,7 @@ struct BlockDriverState {
 int type;
 char device_name[32];
 unsigned long *dirty_bitmap;
+int64_t dirty_count;
 BlockDriverState *next;
 void *private;
 };
-- 
1.6.0.4

[Qemu-devel] [PATCH v2 4/4] Try not to exceed max downtime on stage3

Move to stage3 only when remaining work can be done below max downtime.

Changes from v1: remove max iterations. Try to infer storage performance and by 
that calculate remaining work.

Signed-off-by: Liran Schour 
---
 block-migration.c |  136 +++--
 1 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 16df75f..5ef3eb8 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -17,6 +17,7 @@
 #include "qemu-queue.h"
 #include "monitor.h"
 #include "block-migration.h"
+#include "migration.h"
 #include 
 
 #define BLOCK_SIZE (BDRV_SECTORS_PER_DIRTY_CHUNK << BDRV_SECTOR_BITS)
@@ -60,6 +61,7 @@ typedef struct BlkMigBlock {
 QEMUIOVector qiov;
 BlockDriverAIOCB *aiocb;
 int ret;
+long double time;
 QSIMPLEQ_ENTRY(BlkMigBlock) entry;
 } BlkMigBlock;
 
@@ -74,11 +76,79 @@ typedef struct BlkMigState {
 int64_t total_sector_sum;
 int prev_progress;
 int bulk_completed;
-int dirty_iterations;
+long double total_time;
+int reads;
 } BlkMigState;
 
 static BlkMigState block_mig_state;
 
+static int64_t get_clock_realtime(void)
+{
+struct timeval tv;
+
+gettimeofday(&tv, NULL);
+return tv.tv_sec * 10LL + (tv.tv_usec * 1000);
+}
+
+#ifdef WIN32
+
+static int64_t clock_freq;
+
+static void init_get_clock(void)
+{
+LARGE_INTEGER freq;
+int ret;
+ret = QueryPerformanceFrequency(&freq);
+if (ret == 0) {
+fprintf(stderr, "Could not calibrate ticks\n");
+exit(1);
+}
+clock_freq = freq.QuadPart;
+}
+
+static int64_t get_clock(void)
+{
+LARGE_INTEGER ti;
+QueryPerformanceCounter(&ti);
+return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
+}
+
+#else
+
+static int use_rt_clock;
+
+static void init_get_clock(void)
+{
+use_rt_clock = 0;
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 
50) \
+|| defined(__DragonFly__) || defined(__FreeBSD_kernel__)
+{
+struct timespec ts;
+if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
+use_rt_clock = 1;
+}
+}
+#endif
+}
+
+static int64_t get_clock(void)
+{
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 
50) \
+   || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
+if (use_rt_clock) {
+struct timespec ts;
+clock_gettime(CLOCK_MONOTONIC, &ts);
+return ts.tv_sec * 10LL + ts.tv_nsec;
+} else
+#endif
+{
+/* XXX: using gettimeofday leads to problems if the date
+   changes, so it should be avoided. */
+return get_clock_realtime();
+}
+}
+#endif
+
 static void blk_send(QEMUFile *f, BlkMigBlock * blk)
 {
 int len;
@@ -127,12 +197,28 @@ uint64_t blk_mig_bytes_total(void)
 return sum << BDRV_SECTOR_BITS;
 }
 
+static inline void add_avg_read_time(long double time)
+{
+block_mig_state.reads++;
+block_mig_state.total_time += time;
+}
+
+static inline long double compute_read_bwidth(void)
+{
+assert(block_mig_state.total_time != 0);
+return  (block_mig_state.reads * BLOCK_SIZE)/ block_mig_state.total_time;
+}
+
 static void blk_mig_read_cb(void *opaque, int ret)
 {
 BlkMigBlock *blk = opaque;
 
 blk->ret = ret;
 
+blk->time = get_clock() - blk->time;
+
+add_avg_read_time(blk->time);
+
 QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
 
 block_mig_state.submitted--;
@@ -182,6 +268,8 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
 blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
 qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
 
+blk->time = get_clock();
+
 blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
 if (!blk->aiocb) {
@@ -223,6 +311,8 @@ static void init_blk_migration(Monitor *mon, QEMUFile *f)
 block_mig_state.total_sector_sum = 0;
 block_mig_state.prev_progress = -1;
 block_mig_state.bulk_completed = 0;
+block_mig_state.total_time = 0;
+block_mig_state.reads = 0;
 
 for (bs = bdrv_first; bs != NULL; bs = bs->next) {
 if (bs->type == BDRV_TYPE_HD) {
@@ -321,6 +411,8 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
 blk->iov.iov_base = blk->buf;
 blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
 qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
+
+   blk->time = get_clock();
 
 blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
@@ -403,10 +495,42 @@ static void flush_blks(QEMUFile* f)
 block_mig_state.transferred);
 }
 
+static int64_t get_remaining_dirty(void)
+{
+BlkMigDevState *bmds;
+int64_t dirty = 0;
+
+QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
+dirty +=

[Qemu-devel] [PATCH v2 2/4] Tranfer dirty blocks during iterative phase

Start transfer dirty blocks during the iterative stage. That will
reduce the time that the guest will be suspended

Changes from v1: remove trailing whitespaces and remove max iterations limit.

Signed-off-by: Liran Schour 
---
 block-migration.c |  135 +++--
 1 files changed, 99 insertions(+), 36 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index f9bb42c..16df75f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -45,6 +45,7 @@ typedef struct BlkMigDevState {
 int bulk_completed;
 int shared_base;
 int64_t cur_sector;
+int64_t cur_dirty;
 int64_t completed_sectors;
 int64_t total_sectors;
 int64_t dirty;
@@ -73,6 +74,7 @@ typedef struct BlkMigState {
 int64_t total_sector_sum;
 int prev_progress;
 int bulk_completed;
+int dirty_iterations;
 } BlkMigState;
 
 static BlkMigState block_mig_state;
@@ -186,6 +188,7 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
 goto error;
 }
 block_mig_state.submitted++;
+
 bdrv_reset_dirty(bs, cur_sector, nr_sectors);
 bmds->cur_sector = cur_sector + nr_sectors;
 
@@ -284,39 +287,88 @@ static int blk_mig_save_bulked_block(Monitor *mon, 
QEMUFile *f)
 return ret;
 }
 
-#define MAX_NUM_BLOCKS 4
-
-static void blk_mig_save_dirty_blocks(Monitor *mon, QEMUFile *f)
+static void blk_mig_reset_dirty_cursor(void)
 {
 BlkMigDevState *bmds;
-BlkMigBlock blk;
+
+QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
+bmds->cur_dirty = 0;
+}
+}
+
+static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
+ BlkMigDevState *bmds, int is_async)
+{
+BlkMigBlock *blk;
+int64_t total_sectors = bmds->total_sectors;
 int64_t sector;
+int nr_sectors;
 
-blk.buf = qemu_malloc(BLOCK_SIZE);
+for (sector = bmds->cur_dirty; sector < bmds->total_sectors;) {
+if (bdrv_get_dirty(bmds->bs, sector)) {
 
-QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-for (sector = 0; sector < bmds->cur_sector;) {
-if (bdrv_get_dirty(bmds->bs, sector)) {
-if (bdrv_read(bmds->bs, sector, blk.buf,
-  BDRV_SECTORS_PER_DIRTY_CHUNK) < 0) {
-monitor_printf(mon, "Error reading sector %" PRId64 "\n",
-   sector);
-qemu_file_set_error(f);
-qemu_free(blk.buf);
-return;
+if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) {
+nr_sectors = total_sectors - sector;
+} else {
+nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK;
+}
+blk = qemu_malloc(sizeof(BlkMigBlock));
+blk->buf = qemu_malloc(BLOCK_SIZE);
+blk->bmds = bmds;
+blk->sector = sector;
+
+if(is_async) {
+blk->iov.iov_base = blk->buf;
+blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
+qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
+
+blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
+nr_sectors, blk_mig_read_cb, blk);
+if (!blk->aiocb) {
+goto error;
+}
+block_mig_state.submitted++;
+} else {
+if (bdrv_read(bmds->bs, sector, blk->buf,
+  nr_sectors) < 0) {
+goto error;
 }
-blk.bmds = bmds;
-blk.sector = sector;
-blk_send(f, &blk);
+blk_send(f, blk);
 
-bdrv_reset_dirty(bmds->bs, sector,
- BDRV_SECTORS_PER_DIRTY_CHUNK);
+qemu_free(blk->buf);
+qemu_free(blk);
 }
-sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
+
+bdrv_reset_dirty(bmds->bs, sector, nr_sectors);
+break;
 }
+sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
+bmds->cur_dirty = sector;
 }
 
-qemu_free(blk.buf);
+return (bmds->cur_dirty >= bmds->total_sectors);
+
+ error:
+monitor_printf(mon, "Error reading sector %" PRId64 "\n", sector);
+qemu_file_set_error(f);
+qemu_free(blk->buf);
+qemu_free(blk);
+return 0;
+}
+
+static int blk_mig_save_dirty_block(Monitor *mon, QEMUFile *f, int is_async)
+{
+BlkMigDevState *bmds;
+int ret = 0;
+
+QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
+if(mig_save_device_dirty(mon, f, bmds, is_async) == 0) {
+ret = 1;
+break;
+}
+}
+
+return ret;
 }
 
 static void flush_blks(QEMUFile* f)
@@ -408,28 +460,39 @@ static int block_save_live(Monitor *mon, QEMUFile *f, int 
stage, void *opaque)
 return 0;
 }
 
-/* control the rate of trans

[Qemu-devel] regression between 0.12.1.2 and 0.12.2

2010-01-21 Thread Toralf Förster

Hi,

under a mostly stable Gentoo I observed this new msg :

tfoer...@n22 ~/virtual/kvm $ qemu -hda gentoo_kdevm.img -hdb 
portage_kdeprefix.img -hdd swap.img -smp 2 -m 768 -vga std -soundhw es1370  

 
BUG: kvm_dirty_pages_log_enable_slot: invalid parameters
 
BUG: kvm_dirty_pages_log_disable_slot: invalid parameters   
 
..

The kvm image can be derived from http://dev.gentooexperimental.org/~wired/kvm/ 
.

My system is a :
tfoer...@n22 ~/virtual/kvm $ uname -a
Linux n22 2.6.32.4 #1 SMP Mon Jan 18 20:20:38 CET 2010 i686 Intel(R) Core(TM)2 
Duo CPU P8600 @ 2.40GHz GenuineIntel GNU/Linux


-- 
MfG/Sincerely
Toralf Förster

Re: [Qemu-devel] [PATCH 1/3] qdev: Add help for device properties

Stefan Weil  writes:

> When called with property "?", a list of supported
> properties will be printed (instead of an error message).
>
> This is useful for command lines like
>   qemu -device e1000,?
> and was already standard for other options like model=?
>
> Signed-off-by: Stefan Weil 
> ---
>  hw/qdev-properties.c |   15 +--
>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 277ff9e..8547ad2 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -544,8 +544,19 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
> const char *value)
>  
>  prop = qdev_prop_find(dev, name);
>  if (!prop) {
> -fprintf(stderr, "property \"%s.%s\" not found\n",
> -dev->info->name, name);
> +if (strcmp(name, "?") != 0) {
> +fprintf(stderr, "property \"%s.%s\" not found\n",
> +dev->info->name, name);
> +} else {
> +fprintf(stderr, "supported properties:\n");
> +if (dev->info->props != NULL) {
> +Property *props = dev->info->props;
> +while (props->name) {
> +fprintf(stderr, "%s.%s\n", dev->info->name, props->name);
> +props++;
> +}
> +}
> +}
>  return -1;
>  }
>  if (!prop->info->parse) {

I like it.

Re: [Qemu-devel] [PATCH 2/3] qdev: Add help for property value

Stefan Weil  writes:

> When called with property value "?",
> a help text will be printed (instead of an error message).
>
> This is useful for command lines like
>   qemu -device e1000,mac=?
> and is already standard for other command line options.
>
> A better help text could be provided by extending
> the Property structure with a desc field.
>
> Signed-off-by: Stefan Weil 
> ---
>  hw/qdev-properties.c |9 +++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 8547ad2..f5ca05f 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -565,8 +565,13 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
> const char *value)
>  return -1;
>  }
>  if (prop->info->parse(dev, prop, value) != 0) {
> -fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
> -dev->info->name, name, value);
> +if (strcmp(value, "?") != 0) {
> +fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
> +dev->info->name, name, value);
> +} else {
> +fprintf(stderr, "%s.%s=%s\n",
> +dev->info->name, name, prop->info->name);
> +}
>  return -1;
>  }
>  return 0;

This one is problematic.  If prop->info->parse() accepts "?", help is
inaccessible.  PATCH 1/3 is immune to that problem, because we won't
name a property "?".

PATCH 1/3 already makes -device FOO,? print all property names.  I
figure it could just as well print all help along with names.

Re: [Qemu-devel] [PATCH 1/3] qdev: Add help for device properties

Markus Armbruster  writes:

> Stefan Weil  writes:
>
>> When called with property "?", a list of supported
>> properties will be printed (instead of an error message).
>>
>> This is useful for command lines like
>>  qemu -device e1000,?
>> and was already standard for other options like model=?
>>
>> Signed-off-by: Stefan Weil 
>> ---
>>  hw/qdev-properties.c |   15 +--
>>  1 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
>> index 277ff9e..8547ad2 100644
>> --- a/hw/qdev-properties.c
>> +++ b/hw/qdev-properties.c
>> @@ -544,8 +544,19 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
>> const char *value)
>>  
>>  prop = qdev_prop_find(dev, name);
>>  if (!prop) {
>> -fprintf(stderr, "property \"%s.%s\" not found\n",
>> -dev->info->name, name);
>> +if (strcmp(name, "?") != 0) {
>> +fprintf(stderr, "property \"%s.%s\" not found\n",
>> +dev->info->name, name);
>> +} else {
>> +fprintf(stderr, "supported properties:\n");
>> +if (dev->info->props != NULL) {
>> +Property *props = dev->info->props;
>> +while (props->name) {
>> +fprintf(stderr, "%s.%s\n", dev->info->name, 
>> props->name);
>> +props++;
>> +}
>> +}
>> +}
>>  return -1;
>>  }
>>  if (!prop->info->parse) {
>
> I like it.

One question, though: why print DRIVER.PROPNAME instead of just
PROPNAME?  You could still put DRIVER into the heading, say "Properties
of DRIVER:".

Re: [Qemu-devel] [PATCH] Documentation: Add missing documentation for qdev related command line options

Stefan Weil  writes:

> Markus Armbruster schrieb:
>> Stefan Weil  writes:
>>
>>> Markus Armbruster schrieb:
[...]
 While there, would you mind improving --help for -device a bit? It's
 too terse, and it doesn't start the help text in column 16 like the
 other options do.
>>> Hi Markus,
>>>
>>> this needs a little more work. I just had a look on the code,
>>> and there is no online help for the possible options (key, value).
>>
>> What I had in mind was just to bring it up to par with your patch to the
>> texi, but...
>>
>>> If you (and especially those who have commit rights) agree,
>>> I could provide these three additional patches:
>>>
>>> * Add online help for properties (qemu -device driver,?)
>>> * Add online help for property value (qemu -device driver,property=?)
>>> * Update documentation for command line option -device
>>
>> ... a patch to provide that is very desirable!
>>
>> I figure the best way to document available properties and there values
>> is a self-documenting struct PropertyInfo: add a doc member, extend
>> DEFINE_PROP() & friends to set it, fix up users to pass NULL, and so
>> forth. We can then replace the NULL by something useful at our leisure.
>>
>>> There is already an online help for the driver (qemu -device ?).
>
> I cannot spend too much time on this, but a very basic help
> for "?" is implemented by the patch series I just sent to the list.
>
> The new feature was already very helpful for me, but it still
> can be improved, of course: the driver list contains shows
> too many drivers and is not nicely formatted, the help text
> for the values could be more user friendly, ...

It's a start.  Many thanks!

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

2010-01-21 Thread john cooper

Anthony Liguori wrote:
> On 01/20/2010 07:18 PM, john cooper wrote: 
>> I can appreciate the concern of wanting to get this
>> as "correct" as possible.
>>
> 
> This is the root of the trouble.  At the qemu layer, we try to focus on
> being correct.
> 
> Management tools are typically the layer that deals with being "correct".
> 
> A good compromise is making things user tunable which means that a
> downstream can make "correctness" decisions without forcing those
> decisions on upstream.

Conceptually I agree with such a malleable approach -- actually
I prefer it.  I thought however it was too much infrastructure to
foist on the problem just to add a few more models into the mix.

The only reservation which comes to mind is that of logistics.
This may ruffle the code some and impact others such as Andre
who seem to have existing patches relative to the current structure.
Anyone have strong objections to this approach before I have a
look at an implementation?

Thanks,

-john

-- 
john.coo...@redhat.com

Re: [Qemu-devel] Re: Stop using "which" in ./configure

Måns Rullgård wrote:
>   If IFS is not set, the shell shall behave as if the value of IFS is
>   , , and 
> 
> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
> Likewise if you set the value first.

Remove the colon.  The above will wrongly change empty IFS, which
is not the same as unset IFS.

That said, nobody should expect ./configure to work if IFS has an
unusual value anyway.  Probably .configure should just set it to the
standard value at the start.

-- Jamie

Re: [Qemu-devel] possible qemu miscompilation by latest gcc

2010-01-21 Thread Richard Henderson


On 01/20/2010 10:15 PM, John Regehr wrote:

Hi folks-

Just wanted to let you know that perhaps the function
helper_neon_rshl_s8() is being miscompiled by the latest gcc.

I'm using qemu 0.12.2 and gcc rev 156103, which is a pre-version of gcc
4.5. This is on an x86 machine running Ubuntu 9.10.

At -O2 or higher this is the resulting object code:

2060 :
2060: 31 c0 xor %eax,%eax
2062: c3 ret

If this is not the intended result, then either the function has a
latent bug or else the compiler is doing something bad.


Confirmed.  The compiler is at fault.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42833


r~

Re: [Qemu-devel] [PATCH v2 0/5]: Convert pci_info() to QObject

2010-01-21 Thread Blue Swirl

On Thu, Jan 21, 2010 at 1:52 PM, Luiz Capitulino  wrote:
> On Wed, 20 Jan 2010 19:22:48 +
> Blue Swirl  wrote:
>
>> On Wed, Jan 20, 2010 at 6:11 PM, Luiz Capitulino  
>> wrote:
>> > On Wed, 20 Jan 2010 18:57:56 +0100
>> > Markus Armbruster  wrote:
>> >
>> >> Luiz Capitulino  writes:
>> >>
>> >> >  Hi,
>> >> >
>> >> >  This new version addresses Markus's comments.
>> >> >
>> >> > changelog
>> >> > -
>> >> >
>> >> > V1 -> V2
>> >> >
>> >> > - Make class_info's key 'desc' optional
>> >> > - Better indentation
>> >> > - Doc fixes
>> >> >
>> >> > V0 -> V1
>> >> >
>> >> > - Coding style fixes
>> >> > - Make 'BAR' and 'IRQ' keys lowercase
>> >> > - Add 'irq' key to the documentation
>> >> >
>> >> >  Thanks.
>> >>
>> >> Looks good, although one comment still applies: PATCH 3/5 regresses info
>> >> pci, 4/5 and 5/5 fix it.  Do we care?  They're separate because they're
>> >> untested.
>> >
>> >  There are two problems here, which apply for those whom emulate
>> > the hardware:
>> >
>> > 1. 'info pci' output will brake with git bisect
>> >
>> > 2. As the code is untested, it might be broken
>> >
>> >  Only two 2 seems serious.
>> >
>> >  Michael, does the sparc image on qemu.org have the hardware
>> > in question (pci bridge)?
>>
>> Sparc64 has two Simba bridges, but currently they are broken so there
>> are no devices behind them. In addition there should be a DEC 21154
>> bridge.
>
>  Can the DEC one have devices attached to it?

It should, but sun4u machine does not even create it yet. There are
two DEC 21154 models, located in grackle_pci.c and unin_pci.c (PPC).
They should be unified and given a file of their own so they could be
used also for Sparc64.

>> There is no Sparc64 test image yet (very few Sparc32 machines did have
>> any PCI and we don't emulate them), but you can test the output
>> without any images:
>
>  Oh man, thanks a lot!
>
>  I got an abort() :)
>

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

2010-01-21 Thread Blue Swirl

On Thu, Jan 21, 2010 at 2:39 PM, Andre Przywara  wrote:
> john cooper wrote:
>>
>> Chris Wright wrote:
>>>
>>> * Daniel P. Berrange (berra...@redhat.com) wrote:

 To be honest all possible naming schemes for '-cpu ' are just as
 unfriendly as each other. The only user friendly option is '-cpu host'.
 IMHO, we should just pick a concise naming scheme & document it. Given
 they are all equally unfriendly, the one that has consistency with
 vmware
 naming seems like a mild winner.
>>>
>>> Heh, I completely agree, and was just saying the same thing to John
>>> earlier today.  May as well be -cpu {foo,bar,baz} since the meaning for
>>> those command line options must be well-documented in the man page.
>>
>> I can appreciate the concern of wanting to get this
>> as "correct" as possible.  But ultimately we just
>> need three unique tags which ideally have some relation
>> to their associated architectures.  The diatribes
>> available from /proc/cpuinfo while generally accurate
>> don't really offer any more of a clue to the model
>> group, and in their unmodified form are rather unwieldy
>> as command line flags.
>
> I agree. I'd underline that this patch is for migration purposes only, so
> you don't want to specify an exact CPU, but more like a class of CPUs. If
> you look into the available CPUID features in each CPU, you will find that
> there are only a few groups, with currently three for each vendor being a
> good guess.
> /proc/cpuinfo just prints out marketing names, which have only a mild
> relationship to a feature-related technical CPU model. Maybe we can use a
> generation approach like the AMD Opteron ones for Intel, too.
> These G1/G2/G3 names are just arbitrary and have no roots within AMD.
>
> I think that an exact CPU model specification is out of scope for this patch
> and maybe even for QEMU. One could create a database with CPU names and
> associated CPUID flags and provide an external tool to generate a QEMU
> command line out of this. Keeping this database up-to-date (especially for
> desktop CPU models) is a burden that the QEMU project does not want to bear.
>
>>
>>> This is from an EVC kb article[1]:
>>
>> Here is a pointer to a more detailed version:
>>
>>
>> http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1003212
>>
>>
>> We probably should also add an option to dump out the
>> full set of qemu-side cpuid flags for the benefit of
>> users and upper level tools.
>
> You mean like this one?
> http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg01228.html
> Resending this patch set is on my plan for next week. What is the state of
> this patch? Will it go in soon? Then I'd rebase my patch set on top of it.

FYI, a similar CPU flag mechanism has been implemented for Sparc and
x86, unifying these would be cool.

Re: [Qemu-devel] [PATCH 1/3] qdev: Add help for device properties

2010-01-21 Thread Gerd Hoffmann


On 01/21/10 17:44, Markus Armbruster wrote:

Markus Armbruster  writes:


-fprintf(stderr, "property \"%s.%s\" not found\n",
-dev->info->name, name);



+fprintf(stderr, "%s.%s\n", dev->info->name, props->name);
+props++;



One question, though: why print DRIVER.PROPNAME instead of just
PROPNAME?  You could still put DRIVER into the heading, say "Properties
of DRIVER:".


Probably for consistency with the error messages.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

malc  writes:

> On Tue, 29 Dec 2009, Jamie Lokier wrote:
>
>> malc wrote:
>> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
>> > 
>> > > Aurelien Jarno wrote:
>> > > > This fixes the loading of a stripped kernel with zero malloc disabled.
>> > > 
>> > > *Raises an eyebrow*
>> > > 
>> > > Even though there's different perspectives over whether qemu_malloc(0)
>> > > should be allowed, inherited from ambiguity over malloc(0),
>> > > realloc(p,0) has always had a standard, well-defined meaning.
>> > 
>> > No.
>> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
>> 
>> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
>> rationale is not consistent with itself on the subject, and differs
>> from C90 where the "standard, well-defined meaning" I referred to was
>> defined.
>
> Yep.

No, this is a misinterpretation of the C99 standard, made possible by
its poor wording.  The C99 Rationale is perfectly clear, though:

7.20.3.4 The realloc function

A null first argument is permissible.  If the first argument is not
null, and the second argument is 0, then the call frees the memory
pointed to by the first argument, and a null argument may be
returned; [...]

This is hardly surprising, because anything else would break working C89
programs, and that would squarely contradict the standard's mission, as
explained in the Rationale:

Avoid "quiet changes."  Any change to widespread practice altering
the meaning of existing code causes problems.  Changes that cause
code to be so ill-formed as to require diagnostic messages are at
least easy to detect.  As much as seemed possible consistent with its
other goals, the C89 Committee avoided changes that quietly alter
one valid program to another with different semantics, that cause a
working program to work differently without notice.  In important
places where this principle is violated, both the C89 Rationale and
this Rationale point out a QUIET CHANGE.

The only quiet change in the Rationale regarding malloc() & friends is
this one:

  QUIET CHANGE IN C89

A program which relies on size-zero allocation requests returning a
non-null pointer will behave differently.

For what it's worth, I checked my facts with a member of the C
committee.

>> See also http://c-faq.com/malloc/reallocnull.html which says "and the
>> related realloc(..., 0), which frees" and has references at the end.
>> See, it's not just me :-)
>
> Nope not just you, even glibc still uses C90 behaviour.

Because it reads the standard correctly, just like the comp.lang.c FAQ
Aurelien quoted.

[...]

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

john cooper wrote:
> kvm itself can modify flags exported from qemu to a guest.

I would hope for an option to request that qemu doesn't run if the
guest won't get the cpuid flags requested on the command line.

-- Jamie

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

john cooper wrote:
> > I foresee wanting to iterate over the models and pick the latest one
> > which a host supports - on the grounds that you have done the hard
> > work of ensuring it is a reasonably good performer, while "probably"
> > working on another host of similar capability when a new host is made
> > available.
> 
> That's a fairly close use case to that of safe migration
> which was one of the primary motivations to identify
> the models being discussed.  Although presentation and
> administration of such was considered the domain of management
> tools.

My hypothetical script which iterates over models in that way is a
"management tool", and would use qemu to help do its job.

Do you mean that more powerful management tools to support safe
migration will maintain _their own_ processor model tables, and
perform their calculations using their own tables instead of querying
qemu, and therefore not have any need of qemu's built in table?

If so, I favour more strongly Anthony's suggestion that the processor
model table lives in a config file (eventually), as that file could be
shared between management tools and qemu itself without duplication.

-- Jamie

Re: [Qemu-devel] [PATCH v2 4/4] Try not to exceed max downtime on stage3

2010-01-21 Thread Pierre Riteau

On 21 janv. 2010, at 16:24, Liran Schour wrote:

> Move to stage3 only when remaining work can be done below max downtime.
> 
> Changes from v1: remove max iterations. Try to infer storage performance and 
> by that calculate remaining work.
> 
> Signed-off-by: Liran Schour 
> ---
> block-migration.c |  136 +++--
> 1 files changed, 132 insertions(+), 4 deletions(-)
> 
> diff --git a/block-migration.c b/block-migration.c
> index 16df75f..5ef3eb8 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -17,6 +17,7 @@
> #include "qemu-queue.h"
> #include "monitor.h"
> #include "block-migration.h"
> +#include "migration.h"
> #include 
> 
> #define BLOCK_SIZE (BDRV_SECTORS_PER_DIRTY_CHUNK << BDRV_SECTOR_BITS)
> @@ -60,6 +61,7 @@ typedef struct BlkMigBlock {
> QEMUIOVector qiov;
> BlockDriverAIOCB *aiocb;
> int ret;
> +long double time;
> QSIMPLEQ_ENTRY(BlkMigBlock) entry;
> } BlkMigBlock;
> 
> @@ -74,11 +76,79 @@ typedef struct BlkMigState {
> int64_t total_sector_sum;
> int prev_progress;
> int bulk_completed;
> -int dirty_iterations;
> +long double total_time;
> +int reads;
> } BlkMigState;
> 
> static BlkMigState block_mig_state;
> 
> +static int64_t get_clock_realtime(void)
> +{
> +struct timeval tv;
> +
> +gettimeofday(&tv, NULL);
> +return tv.tv_sec * 10LL + (tv.tv_usec * 1000);
> +}
> +
> +#ifdef WIN32
> +
> +static int64_t clock_freq;
> +
> +static void init_get_clock(void)
> +{
> +LARGE_INTEGER freq;
> +int ret;
> +ret = QueryPerformanceFrequency(&freq);
> +if (ret == 0) {
> +fprintf(stderr, "Could not calibrate ticks\n");
> +exit(1);
> +}
> +clock_freq = freq.QuadPart;
> +}
> +
> +static int64_t get_clock(void)
> +{
> +LARGE_INTEGER ti;
> +QueryPerformanceCounter(&ti);
> +return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
> +}
> +
> +#else
> +
> +static int use_rt_clock;
> +
> +static void init_get_clock(void)
> +{
> +use_rt_clock = 0;
> +#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 
> 50) \
> +|| defined(__DragonFly__) || defined(__FreeBSD_kernel__)
> +{
> +struct timespec ts;
> +if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
> +use_rt_clock = 1;
> +}
> +}
> +#endif
> +}
> +
> +static int64_t get_clock(void)
> +{
> +#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 
> 50) \
> + || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
> +if (use_rt_clock) {
> +struct timespec ts;
> +clock_gettime(CLOCK_MONOTONIC, &ts);
> +return ts.tv_sec * 10LL + ts.tv_nsec;
> +} else
> +#endif
> +{
> +/* XXX: using gettimeofday leads to problems if the date
> +   changes, so it should be avoided. */
> +return get_clock_realtime();
> +}
> +}
> +#endif
> +
> static void blk_send(QEMUFile *f, BlkMigBlock * blk)
> {
> int len;
> @@ -127,12 +197,28 @@ uint64_t blk_mig_bytes_total(void)
> return sum << BDRV_SECTOR_BITS;
> }
> 
> +static inline void add_avg_read_time(long double time)
> +{
> +block_mig_state.reads++;
> +block_mig_state.total_time += time;
> +}
> +
> +static inline long double compute_read_bwidth(void)
> +{
> +assert(block_mig_state.total_time != 0);
> +return  (block_mig_state.reads * BLOCK_SIZE)/ block_mig_state.total_time;
> +}
> +
> static void blk_mig_read_cb(void *opaque, int ret)
> {
> BlkMigBlock *blk = opaque;
> 
> blk->ret = ret;
> 
> +blk->time = get_clock() - blk->time;
> +
> +add_avg_read_time(blk->time);
> +
> QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
> 
> block_mig_state.submitted--;
> @@ -182,6 +268,8 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
> blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
> qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
> 
> +blk->time = get_clock();
> +
> blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
> nr_sectors, blk_mig_read_cb, blk);
> if (!blk->aiocb) {
> @@ -223,6 +311,8 @@ static void init_blk_migration(Monitor *mon, QEMUFile *f)
> block_mig_state.total_sector_sum = 0;
> block_mig_state.prev_progress = -1;
> block_mig_state.bulk_completed = 0;
> +block_mig_state.total_time = 0;
> +block_mig_state.reads = 0;
> 
> for (bs = bdrv_first; bs != NULL; bs = bs->next) {
> if (bs->type == BDRV_TYPE_HD) {
> @@ -321,6 +411,8 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile 
> *f,
> blk->iov.iov_base = blk->buf;
> blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
> qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
> +
> + blk->time = get_clock();
> 
> blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
>

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

2010-01-21 Thread malc

On Thu, 21 Jan 2010, Markus Armbruster wrote:

> malc  writes:
> 
> > On Tue, 29 Dec 2009, Jamie Lokier wrote:
> >
> >> malc wrote:
> >> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
> >> > 
> >> > > Aurelien Jarno wrote:
> >> > > > This fixes the loading of a stripped kernel with zero malloc 
> >> > > > disabled.
> >> > > 
> >> > > *Raises an eyebrow*
> >> > > 
> >> > > Even though there's different perspectives over whether qemu_malloc(0)
> >> > > should be allowed, inherited from ambiguity over malloc(0),
> >> > > realloc(p,0) has always had a standard, well-defined meaning.
> >> > 
> >> > No.
> >> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
> >> 
> >> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
> >> rationale is not consistent with itself on the subject, and differs
> >> from C90 where the "standard, well-defined meaning" I referred to was
> >> defined.
> >
> > Yep.
> 
> No, this is a misinterpretation of the C99 standard, made possible by
> its poor wording.  The C99 Rationale is perfectly clear, though:

You have to show the flaw in Hallvard B Furuseth's analysis to claim
that it's a misinterpretation. And unlike the standard rationale is
non normative.

[..snip..]

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

john cooper wrote:
> I can appreciate the argument above, however the goal was
> choosing names with some basis in reality.  These were
> recommended by our contacts within Intel, are used by VmWare
> to describe their similar cpu models, and arguably have fallen
> to defacto usage as evidenced by such sources as:
> 
> http://en.wikipedia.org/wiki/Conroe_(microprocessor)
> http://en.wikipedia.org/wiki/Penryn_(microprocessor)
> http://en.wikipedia.org/wiki/Nehalem_(microarchitecture)

(Aside: I can confirm they haven't fallen into de facto usage anywhere
in my vicinity :-) I wonder if the contact within Intel are living in
a bit of a bubble where these names are more familiar than the outside
world.)

I think we can all agree that there is no point looking for a familiar
-cpu naming scheme because there aren't any familiar and meaningful names
these days.

> used by VmWare to describe their similar cpu models

If the same names are being used, I see some merit in qemu's list
matching VMware's cpu models *exactly* (in capabilities, not id
strings), to aid migration from VMware.  Is that feasible?  Do they
match already?

> I suspect whatever we choose of reasonable length as a model
> tag for "-cpu" some further detail is going to be required.
> That was the motivation to augment the table as above with
> an instance of a LCD for that associated class.
>  
> > I'm not a typical user: I know quite a lot about x86 architecture;
> > I just haven't kept up to date enough to know the code/model names.
> > Typical users will know less about them.
> 
> Understood.


> One thought I had to further clarify what is going on under the hood
> was to dump the cpuid flags for each model as part of (or in
> addition to) the above table.  But this seems a bit extreme and kvm
> itself can modify flags exported from qemu to a guest.

Here's another idea.

It would be nice if qemu could tell the user which of the built-in
-cpu choices is the most featureful subset of their own host.  With
-cpu host implemented, finding that is probably quite easy.

Users with multiple hosts will get a better feel for what the -cpu
names mean that way, probably better than any documentation would give
them, because they probably have not much idea what CPU families they
have anyway.  (cat /proc/cpuinfo doesn't clarify, as I found).

And it would give a simple, effective, quick indication of what they
must choose if they want an VM image that runs on more than one of
their hosts without a management tool.

-- Jamie

Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform

2010-01-21 Thread Stefan Weil

identifier scorpio schrieb:
> Thank Mr. Weil for your reply.
>  
> >
> > Maybe you can also try the TCG interpreter (TCI) from
> > http://repo.or.cz/w/qemu/ar7.git.
> > In theory, it supports any host architecture with or
> > without native TCG
> > support.
> >
> > It was tested successful with some basic tests on x86,
> > mips, ppc and arm,
> > so I hope it will run on alpha, too.
> >
>
> so that means i have to learn another set of interface?
> is TCI more simple or straightforward  than TCG?
>
> Dong Weiyu.
>
>
> 
> 好玩贺卡等你发，邮箱贺卡全新上线！
> 


Hello,

No, you don't have to learn a new interface.
Just use it:
configure --enable-tcg-interpreter
make
and run it on alpha. It will be a little slower
than a native tcg implementation for alpha
because tcg code is interpreted at runtime.

Stefan Weil

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

Markus Armbruster wrote:
> malc  writes:
> 
> > On Tue, 29 Dec 2009, Jamie Lokier wrote:
> >
> >> malc wrote:
> >> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
> >> > 
> >> > > Aurelien Jarno wrote:
> >> > > > This fixes the loading of a stripped kernel with zero malloc 
> >> > > > disabled.
> >> > > 
> >> > > *Raises an eyebrow*
> >> > > 
> >> > > Even though there's different perspectives over whether qemu_malloc(0)
> >> > > should be allowed, inherited from ambiguity over malloc(0),
> >> > > realloc(p,0) has always had a standard, well-defined meaning.
> >> > 
> >> > No.
> >> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
> >> 
> >> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
> >> rationale is not consistent with itself on the subject, and differs
> >> from C90 where the "standard, well-defined meaning" I referred to was
> >> defined.
> >
> > Yep.
> 
> No, this is a misinterpretation of the C99 standard, made possible by
> its poor wording.  The C99 Rationale is perfectly clear, though:
> 
> 7.20.3.4 The realloc function
> 
> A null first argument is permissible.  If the first argument is not
> null, and the second argument is 0, then the call frees the memory
> pointed to by the first argument, and a null argument may be
> returned; [...]

The rationale above does not match C89 behaviour.  It says the call
frees the memory, but it does not forbid the call from then proceeding
to do the same as malloc(0) and return a non-NULL pointer.  It's quite
explicit: a null argument *may* be returned.  Which means the
rationale does not require realloc(p,0) to do the same as C89, which
always frees the memory and doesn't allocate anything.

> This is hardly surprising, because anything else would break working C89
> programs, and that would squarely contradict the standard's mission,

Understood.  But it doesn't really matter what's intended or what's
misinterpreted.  If there are any significant implementations out
there based on the "misinterpretation", or even based on the
rationale, that's enough of a reason to not depend on realloc(p,0).

-- Jamie

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

2010-01-21 Thread malc

On Thu, 21 Jan 2010, Jamie Lokier wrote:

> Markus Armbruster wrote:
> > malc  writes:
> > 
> > > On Tue, 29 Dec 2009, Jamie Lokier wrote:
> > >
> > >> malc wrote:
> > >> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
> > >> > 
> > >> > > Aurelien Jarno wrote:
> > >> > > > This fixes the loading of a stripped kernel with zero malloc 
> > >> > > > disabled.
> > >> > > 
> > >> > > *Raises an eyebrow*
> > >> > > 
> > >> > > Even though there's different perspectives over whether 
> > >> > > qemu_malloc(0)
> > >> > > should be allowed, inherited from ambiguity over malloc(0),
> > >> > > realloc(p,0) has always had a standard, well-defined meaning.
> > >> > 
> > >> > No.
> > >> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
> > >> 
> > >> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
> > >> rationale is not consistent with itself on the subject, and differs
> > >> from C90 where the "standard, well-defined meaning" I referred to was
> > >> defined.
> > >
> > > Yep.
> > 
> > No, this is a misinterpretation of the C99 standard, made possible by
> > its poor wording.  The C99 Rationale is perfectly clear, though:
> > 
> > 7.20.3.4 The realloc function
> > 
> > A null first argument is permissible.  If the first argument is not
> > null, and the second argument is 0, then the call frees the memory
> > pointed to by the first argument, and a null argument may be
> > returned; [...]
> 
> The rationale above does not match C89 behaviour.  It says the call
> frees the memory, but it does not forbid the call from then proceeding
> to do the same as malloc(0) and return a non-NULL pointer.  It's quite
> explicit: a null argument *may* be returned.  Which means the
> rationale does not require realloc(p,0) to do the same as C89, which
> always frees the memory and doesn't allocate anything.
> 
> > This is hardly surprising, because anything else would break working C89
> > programs, and that would squarely contradict the standard's mission,
> 
> Understood.  But it doesn't really matter what's intended or what's
> misinterpreted.  If there are any significant implementations out
> there based on the "misinterpretation", or even based on the
> rationale, that's enough of a reason to not depend on realloc(p,0).
> 

My sentiment exactly.

An example:

Dinkum Unabridged Library was certified by Perennial 
(http://peren.com/pages/aboutus_set.htm) to conform
to ISO/IEC 9899:1999.

Documentation for realloc:
http://www.dinkumware.com/manuals/?manual=compleat&Search=realloc&page=stdlib.html#realloc

Hallvard B Furuseth analysis fully applies i believe...

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

2010-01-21 Thread john cooper

Jamie Lokier wrote:

> Do you mean that more powerful management tools to support safe
> migration will maintain _their own_ processor model tables, and
> perform their calculations using their own tables instead of querying
> qemu, and therefore not have any need of qemu's built in table?

I would expect so.  IIRC that is what the libvirt folks have
in mind for example.  But we're also trying to simplify the use
case of the lonesome user at one with the qemu CLI.

-john

-- 
john.coo...@redhat.com

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

2010-01-21 Thread john cooper

Jamie Lokier wrote:

> I think we can all agree that there is no point looking for a familiar
> -cpu naming scheme because there aren't any familiar and meaningful names
> these days.

Even if we dismiss the Intel coined names as internal
code names, there is still VMW's use of them in this
space which we can either align with or attempt to
displace.   All considered I don't see any motivation
nor gain in doing the latter.  Anyway it doesn't appear
likely we're going to resolve this to our collective
satisfaction with a hard-wired naming scheme.   

> It would be nice if qemu could tell the user which of the built-in
> -cpu choices is the most featureful subset of their own host.  With
> -cpu host implemented, finding that is probably quite easy.

This should be doable although it may not be as simple
as traversing a hierarchy of features and picking one
with the most host flags present.  In any case this
should be fairly detachable from settling the immediate
issue.

-john

-- 
john.coo...@redhat.com

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

Jamie Lokier  writes:

> Markus Armbruster wrote:
>> malc  writes:
>> 
>> > On Tue, 29 Dec 2009, Jamie Lokier wrote:
>> >
>> >> malc wrote:
>> >> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
>> >> > 
>> >> > > Aurelien Jarno wrote:
>> >> > > > This fixes the loading of a stripped kernel with zero malloc 
>> >> > > > disabled.
>> >> > > 
>> >> > > *Raises an eyebrow*
>> >> > > 
>> >> > > Even though there's different perspectives over whether qemu_malloc(0)
>> >> > > should be allowed, inherited from ambiguity over malloc(0),
>> >> > > realloc(p,0) has always had a standard, well-defined meaning.
>> >> > 
>> >> > No.
>> >> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
>> >> 
>> >> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
>> >> rationale is not consistent with itself on the subject, and differs
>> >> from C90 where the "standard, well-defined meaning" I referred to was
>> >> defined.
>> >
>> > Yep.
>> 
>> No, this is a misinterpretation of the C99 standard, made possible by
>> its poor wording.  The C99 Rationale is perfectly clear, though:
>> 
>> 7.20.3.4 The realloc function
>> 
>> A null first argument is permissible.  If the first argument is not
>> null, and the second argument is 0, then the call frees the memory
>> pointed to by the first argument, and a null argument may be
>> returned; [...]
>
> The rationale above does not match C89 behaviour.  It says the call
> frees the memory, but it does not forbid the call from then proceeding
> to do the same as malloc(0) and return a non-NULL pointer.  It's quite
> explicit: a null argument *may* be returned.  Which means the
> rationale does not require realloc(p,0) to do the same as C89, which
> always frees the memory and doesn't allocate anything.

I didn't claim there's *no* difference between C89 and C99.  In fact,
the Rationale nicely documents the change:

A new feature of C99: the realloc function was changed to make it
clear that the pointed-to object is deallocated, a new object is
allocated, and the content of the new object is the same as that of
the old object up to the lesser of the two sizes.  C89 attempted to
specify that the new object was the same object as the old object
but might have a different address.  This conflicts with other parts
of the Standard that assume that the address of an object is
constant during its lifetime.  Also, implementations that support an
actual allocation when the size is zero do not necessarily return a
null pointer for this case.  C89 appeared to require a null return
value, and the Committee felt that this was too restrictive.

So C99 permits realloc(p, 0) to return a non-null value.  Regardless, it
still *requires* it to free(p).

>> This is hardly surprising, because anything else would break working C89
>> programs, and that would squarely contradict the standard's mission,
>
> Understood.  But it doesn't really matter what's intended or what's
> misinterpreted.  If there are any significant implementations out
> there based on the "misinterpretation", or even based on the
> rationale, that's enough of a reason to not depend on realloc(p,0).

There are none.

I don't really care how scared QEMU is of realloc(p, 0).  I just want to
correct the misinformation on the standard being spread on this list.

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

malc  writes:

> On Thu, 21 Jan 2010, Markus Armbruster wrote:
>
>> malc  writes:
>> 
>> > On Tue, 29 Dec 2009, Jamie Lokier wrote:
>> >
>> >> malc wrote:
>> >> > On Mon, 28 Dec 2009, Jamie Lokier wrote:
>> >> > 
>> >> > > Aurelien Jarno wrote:
>> >> > > > This fixes the loading of a stripped kernel with zero malloc 
>> >> > > > disabled.
>> >> > > 
>> >> > > *Raises an eyebrow*
>> >> > > 
>> >> > > Even though there's different perspectives over whether qemu_malloc(0)
>> >> > > should be allowed, inherited from ambiguity over malloc(0),
>> >> > > realloc(p,0) has always had a standard, well-defined meaning.
>> >> > 
>> >> > No.
>> >> > http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b
>> >> 
>> >> Wow, thanks for that.  It's a real surprise.  Looks like C99's own
>> >> rationale is not consistent with itself on the subject, and differs
>> >> from C90 where the "standard, well-defined meaning" I referred to was
>> >> defined.
>> >
>> > Yep.
>> 
>> No, this is a misinterpretation of the C99 standard, made possible by
>> its poor wording.  The C99 Rationale is perfectly clear, though:
>
> You have to show the flaw in Hallvard B Furuseth's analysis to claim
> that it's a misinterpretation. And unlike the standard rationale is
> non normative.
>
> [..snip..]

I did.  If that doesn't convince you, I'll gladly wait for the Technical
Corrigendum that'll put this rather absurd misreading to rest.

[Qemu-devel] [PATCH 2/4] Load global config files by default

A new option, -nodefconfig is introduced to prevent loading from the default
config location.  Otherwise, two configuration files will be searched for,
qemu.conf and target-.conf.

The ordering is a little troubling.  Command line options are parsed before
loading the default configs which means that the command line configs will be
loaded before the default config.  The effect is that the default config will
override -readconfig directives.

It's unclear the best way to handle this.

Signed-off-by: Anthony Liguori 
---
 qemu-options.hx |7 +++
 vl.c|   23 +++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index ee60d8a..57f453d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1965,6 +1965,13 @@ STEXI
 @item -writeconfig @var{file}
 Write device configuration to @var{file}.
 ETEXI
+DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig,
+"-nodefconfig\n"
+"do not load default config file\n")
+STEXI
+...@item -nodefconfig
+Do not load default config files.
+ETEXI
 
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
diff --git a/vl.c b/vl.c
index e070ec9..cf12ab0 100644
--- a/vl.c
+++ b/vl.c
@@ -4690,6 +4690,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 CPUState *env;
 int show_vnc_port = 0;
+int defconfig = 1;
 
 init_clocks();
 
@@ -5457,6 +5458,9 @@ int main(int argc, char **argv, char **envp)
 fclose(fp);
 break;
 }
+case QEMU_OPTION_nodefconfig:
+defconfig=0;
+break;
 }
 }
 }
@@ -5471,6 +5475,25 @@ int main(int argc, char **argv, char **envp)
 data_dir = CONFIG_QEMU_SHAREDIR;
 }
 
+if (defconfig) {
+FILE *fp;
+fp = fopen(CONFIG_QEMU_CONFDIR "/qemu.conf", "r");
+if (fp) {
+if (qemu_config_parse(fp) != 0) {
+exit(1);
+}
+fclose(fp);
+}
+
+fp = fopen(CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf", "r");
+if (fp) {
+if (qemu_config_parse(fp) != 0) {
+exit(1);
+}
+fclose(fp);
+}
+}
+
 /*
  * Default to max_cpus = smp_cpus, in case the user doesn't
  * specify a max_cpus value.
-- 
1.6.5.2

[Qemu-devel] [PATCH 0/4] Introduce global config and default devices

This series introduces global config files stored in /etc/qemu.  There is both
a common config (qemu.conf) and a per-target config (target-.conf).

To demonstrate what can be done with global config, I've also made some
enhancements to the default device code that allows many of the builtin qemu
defaults to be overridden.  For instance, the following config file would always
disable the monitor on stdio and use virtio-net with tun/tap instead of
slirp.

/etc/qemu/qemu.conf:

[default]
  monitor = "stdio"

[net]
 type = "nic"
 default = "on"
 model = "virtio"

[net]
 type = "tap"
 default = "on"

[Qemu-devel] [PATCH 3/4] Add -defaults option to allow default devices to be overridden

This option can be used to toggle whether each default device is enabled or
disabled.  For character devices, the default backend can also be overridden.

For devices, we'll have to take a different approach to changing the defaults
which will be covered in the next patch.

N.B. I took special care with -nographic.  Now -nographic pretty clearly acts
as a mechanism to override the default backend devices.

Signed-off-by: Anthony Liguori 
---
 qemu-config.c   |   45 +
 qemu-config.h   |1 +
 qemu-options.hx |7 +
 vl.c|   75 +--
 4 files changed, 109 insertions(+), 19 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..82ca399 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -242,6 +242,50 @@ QemuOptsList qemu_mon_opts = {
 },
 };
 
+QemuOptsList qemu_default_opts = {
+.name = "default",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_default_opts.head),
+.desc = {
+{
+.name = "serial",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "parallel",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "virtcon",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "monitor",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "vga",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "net",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "floppy",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "cdrom",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "sdcard",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end if list */ }
+},
+};
+
 static QemuOptsList *lists[] = {
 &qemu_drive_opts,
 &qemu_chardev_opts,
@@ -250,6 +294,7 @@ static QemuOptsList *lists[] = {
 &qemu_net_opts,
 &qemu_rtc_opts,
 &qemu_global_opts,
+&qemu_default_opts,
 &qemu_mon_opts,
 NULL,
 };
diff --git a/qemu-config.h b/qemu-config.h
index dd89ae4..14ed67b 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -9,6 +9,7 @@ extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_rtc_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
+extern QemuOptsList qemu_default_opts;
 
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
diff --git a/qemu-options.hx b/qemu-options.hx
index 57f453d..e81ecb5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1919,6 +1919,13 @@ STEXI
 Don't create default devices.
 ETEXI
 
+DEF("default", HAS_ARG, QEMU_OPTION_default, \
+"-default argspecify default devices\n")
+STEXI
+...@item -defaults
+Override builtin default devices
+ETEXI
+
 #ifndef _WIN32
 DEF("chroot", HAS_ARG, QEMU_OPTION_chroot, \
 "-chroot dir chroot to dir just before starting the VM\n")
diff --git a/vl.c b/vl.c
index cf12ab0..6a3529e 100644
--- a/vl.c
+++ b/vl.c
@@ -280,6 +280,11 @@ static int default_floppy = 1;
 static int default_cdrom = 1;
 static int default_sdcard = 1;
 
+static const char *default_serial_opt = "vc:80Cx24C";
+static const char *default_parallel_opt = "vc:80Cx24C";
+static const char *default_monitor_opt = "vc:80Cx24C";
+static const char *default_virtcon_opt = "vc:80Cx24C";
+
 static struct {
 const char *driver;
 int *flag;
@@ -4658,6 +4663,32 @@ static int debugcon_parse(const char *devname)
 return 0;
 }
 
+static int default_opt_init(QemuOpts *opts, void *dummy)
+{
+const char *opt;
+
+if ((opt = qemu_opt_get(opts, "serial"))) {
+default_serial_opt = opt;
+}
+if ((opt = qemu_opt_get(opts, "parallel"))) {
+default_parallel_opt = opt;
+}
+if ((opt = qemu_opt_get(opts, "virtcon"))) {
+default_virtcon_opt = opt;
+}
+if ((opt = qemu_opt_get(opts, "monitor"))) {
+default_monitor_opt = opt;
+}
+
+default_vga = qemu_opt_get_bool(opts, "vga", default_vga);
+default_net = qemu_opt_get_bool(opts, "net", default_net);
+default_floppy = qemu_opt_get_bool(opts, "floppy", default_floppy);
+default_cdrom = qemu_opt_get_bool(opts, "cdrom", default_cdrom);
+default_sdcard = qemu_opt_get_bool(opts, "sdcard", default_sdcard);
+
+return 0;
+}
+
 int main(int argc, char **argv, char **envp)
 {
 const char *gdbstub_dev = NULL;
@@ -5409,6 +5440,13 @@ int main(int argc, char **argv, char **envp)
 default_cdrom = 0;
 default_sdcard = 0;
 break;
+case QEMU_OPTION_default:
+opts = qemu_opts_parse(&qemu_default_opts, optarg, NULL);
+if (!opts) {
+fprintf(stderr, "parse error: %s\n", optarg);
+exit(1);
+}
+break;
 #ifndef _WIN32
 case QEMU_OPTION_chroot:

[Qemu-devel] [PATCH 4/4] Allow default network type to be overridden

Introduce a default option to the network device which specifies that this is
a default network device.  This approach should generalize to any other device.

The meaning of a default device is as follows: a default device is added to a
machine IIF defaults aren't disable (via -default or -nodefaults) and a
non-default device of this type hasn't been added.

Signed-off-by: Anthony Liguori 
---
 net.c |   42 +-
 1 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index 6ef93e6..1aa3c77 100644
--- a/net.c
+++ b/net.c
@@ -833,6 +833,10 @@ static int net_init_nic(QemuOpts *opts,
 .name = "name",\
 .type = QEMU_OPT_STRING,   \
 .help = "identifier for monitor commands", \
+ }, {  \
+.name = "default", \
+.type = QEMU_OPT_BOOL, \
+.help = "act as default network device",   \
  }
 
 typedef int (*net_client_init_func)(QemuOpts *opts,
@@ -1056,6 +1060,15 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int 
is_netdev)
 const char *type;
 int i;
 
+/* Do not create default network devices if no defaults */
+if (!default_net) {
+const char *opt = qemu_opt_get(opts, "default");
+
+if (opt && strcmp(opt, "on") == 0) {
+return 0;
+}
+}
+
 type = qemu_opt_get(opts, "type");
 
 if (!is_netdev) {
@@ -1320,10 +1333,22 @@ static int net_init_netdev(QemuOpts *opts, void *dummy)
 int net_init_clients(void)
 {
 if (default_net) {
-/* if no clients, we use a default config */
-qemu_opts_set(&qemu_net_opts, NULL, "type", "nic");
+QemuOpts *opts;
+
+opts = qemu_opts_create(&qemu_net_opts, NULL, 1);
+if (opts == NULL) {
+return -1;
+}
+qemu_opt_set(opts, "type", "nic");
+qemu_opt_set(opts, "default", "on");
+
 #ifdef CONFIG_SLIRP
-qemu_opts_set(&qemu_net_opts, NULL, "type", "user");
+opts = qemu_opts_create(&qemu_net_opts, NULL, 1);
+if (opts == NULL) {
+return -1;
+}
+qemu_opt_set(opts, "type", "user");
+qemu_opt_set(opts, "default", "on");
 #endif
 }
 
@@ -1344,6 +1369,8 @@ int net_init_clients(void)
 
 int net_client_parse(QemuOptsList *opts_list, const char *optarg)
 {
+QemuOpts *opts;
+const char *opt;
 #if defined(CONFIG_SLIRP)
 int ret;
 if (net_slirp_parse_legacy(opts_list, optarg, &ret)) {
@@ -1351,10 +1378,15 @@ int net_client_parse(QemuOptsList *opts_list, const 
char *optarg)
 }
 #endif
 
-if (!qemu_opts_parse(opts_list, optarg, "type")) {
+opts = qemu_opts_parse(opts_list, optarg, "type");
+if (opts == NULL) {
 return -1;
 }
 
-default_net = 0;
+opt = qemu_opt_get(opts, "default");
+if (!opt || strcmp(opt, "off") == 0) {
+default_net = 0;
+}
+
 return 0;
 }
-- 
1.6.5.2

[Qemu-devel] [PATCH 1/4] Support --confdir in configure to specify path to configuration files

The default value is ${prefix}/etc/qemu.  --confdir can be used to override the
default to an absolute path.  The expectation is that when installed to /usr,
 --confdir=/etc/qemu will be used.

Signed-off-by: Anthony Liguori 
---
 configure |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 5631bbb..e86b27e 100755
--- a/configure
+++ b/configure
@@ -32,6 +32,7 @@ cpu=""
 prefix=""
 interp_prefix="/usr/gnemul/qemu-%M"
 static="no"
+confdir=""
 sparc_cpu=""
 cross_prefix=""
 cc="gcc"
@@ -453,6 +454,8 @@ for opt do
   ;;
   --static) static="yes"
   ;;
+  --confdir) confdir="$optarg"
+  ;;
   --disable-sdl) sdl="no"
   ;;
   --enable-sdl) sdl="yes"
@@ -686,6 +689,7 @@ echo "  --extra-ldflags=LDFLAGS  append extra linker flags 
LDFLAGS"
 echo "  --make=MAKE  use specified make [$make]"
 echo "  --install=INSTALLuse specified install [$install]"
 echo "  --static enable static build [$static]"
+echo "  --confdir=PATH   install config in PATH"
 echo "  --enable-debug-tcg   enable TCG debugging"
 echo "  --disable-debug-tcg  disable TCG debugging (default)"
 echo "  --enable-debug   enable common debug build options"
@@ -1828,6 +1832,7 @@ if test "$mingw32" = "yes" ; then
   fi
   mansuffix=""
   datasuffix=""
+  confsuffix=""
   docsuffix=""
   binsuffix=""
 else
@@ -1838,6 +1843,9 @@ else
   datasuffix="/share/qemu"
   docsuffix="/share/doc/qemu"
   binsuffix="/bin"
+  if test -z "$confdir" ; then
+  confdir="${prefix}/etc/qemu"
+  fi
 fi
 
 echo "Install prefix$prefix"
@@ -1914,6 +1922,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak
 echo >> $config_host_mak
 
 echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
+echo "CONFIG_QEMU_CONFDIR=\"$confdir\"" >> $config_host_mak
 
 case "$cpu" in
   
i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64)
@@ -2159,6 +2168,7 @@ echo "prefix=$prefix" >> $config_host_mak
 echo "bindir=\${prefix}$binsuffix" >> $config_host_mak
 echo "mandir=\${prefix}$mansuffix" >> $config_host_mak
 echo "datadir=\${prefix}$datasuffix" >> $config_host_mak
+echo "confdir=$confdir" >> $config_host_mak
 echo "docdir=\${prefix}$docsuffix" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
 echo "INSTALL=$install" >> $config_host_mak
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..


On 01/21/2010 10:43 AM, john cooper wrote:

Anthony Liguori wrote:
   

On 01/20/2010 07:18 PM, john cooper wrote:
 

I can appreciate the concern of wanting to get this
as "correct" as possible.

   

This is the root of the trouble.  At the qemu layer, we try to focus on
being correct.

Management tools are typically the layer that deals with being "correct".

A good compromise is making things user tunable which means that a
downstream can make "correctness" decisions without forcing those
decisions on upstream.
 

Conceptually I agree with such a malleable approach -- actually
I prefer it.  I thought however it was too much infrastructure to
foist on the problem just to add a few more models into the mix.
   


See list for patches.  I didn't do the cpu bits but it should be very 
obvious how to do that now.


Regards,

Anthony Liguori


The only reservation which comes to mind is that of logistics.
This may ruffle the code some and impact others such as Andre
who seem to have existing patches relative to the current structure.
Anyone have strong objections to this approach before I have a
look at an implementation?

Thanks,

-john

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present

2010-01-21 Thread malc

On Thu, 21 Jan 2010, Markus Armbruster wrote:

> malc  writes:
> 
> > On Thu, 21 Jan 2010, Markus Armbruster wrote:

[..snip..]

> >> No, this is a misinterpretation of the C99 standard, made possible by
> >> its poor wording.  The C99 Rationale is perfectly clear, though:
> >
> > You have to show the flaw in Hallvard B Furuseth's analysis to claim
> > that it's a misinterpretation. And unlike the standard rationale is
> > non normative.
> >
> > [..snip..]
> 
> I did.  If that doesn't convince you, I'll gladly wait for the Technical
> Corrigendum that'll put this rather absurd misreading to rest.

If you did, then, i guess, i've missed it, here's the whole analysis,
please point what and where is wrong:

[quote: 
http://groups.google.com/group/comp.std.c/browse_thread/thread/4e9af8847613d71f/6f75ad22e0768a0b?q=realloc++group:comp.std.c#6f75ad22e0768a0b]

C90 said realloc(non-null, 0) did free().  C99 inverted that, saying it
does not:

The only place where 7.20.3.4 (The realloc function) mentions that
realloc may free the old object, is in the case where it returns another
object.  7.20.3 (Memory management functions) says zero-sized allocation
returns NULL, but 7.20.3.4 overrides that.

Could we have the original behavior back, please?  I've seen people say
the current definition is unintentional.  And it conflicts with the C99
Rationale:

   7.20.3.4 The realloc function

   (...)  If the first argument is not null, and the second argument is
   0, then the call frees the memory pointed to by the first argument,

though that goes on with

   and a null argument may be returned; C99 is consistent with the
   policy of not allowing zero-sized objects.

Is that supposed to mean no new object is returned but the return value
is indeterminate, or does it mean that it might free the old object and
return an inaccessible new object like malloc(0)?

Repeating from old realloc(non-null, 0) discussions:

In the latter case a program which sees a NULL return from
realloc(non-null, 0) cannot know if the old object was freed or not -
i.e. it cannot know if the NULL was a failure return (from allocating
the new object) or a success return (after freeing the old object).

Which is exactly the situation for a portable program which sees such a
NULL return today - it cannot know if it was a C99 failure return or a
C90 success return.  Even if the language is C99, the library might be
C90. 

[end quote]

-- 
mailto:av1...@comtv.ru

[Qemu-devel] Re: Stop using "which" in ./configure

2010-01-21 Thread Paolo Bonzini

On 01/21/2010 05:53 PM, Jamie Lokier wrote:

>  If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>  Likewise if you set the value first.

Remove the colon.  The above will wrongly change empty IFS, which
is not the same as unset IFS.

local_ifs would never be unset anyway, so it would never trigger.

After all, Loic's code can be considered okay as it was in the first 
place (sorry).  Instead, we should just make sure that no code ever 
unsets IFS.  I committed this recommendation to the Autoconf manual's 
shell portability section, so it's not necessary to add any comment here.

Paolo

[Qemu-devel] [RFC 00/11]: QMP feature negotiation support

 Feature negotiation allows clients to enable QMP capabilities they are
interested in using. This allows QMP to envolve without breaking old clients.

 A capability is a new QMP feature and/or protocol change which is not part of
the core protocol as defined in the QMP spec.

 Feature negotiation is implemented by defining a set of rules and adding
mode-oriented support.

 The set of rules are:

o All QMP capabilities are disabled by default
o All QMP capabilities must be advertised in the capabilities array
o Commands to enable/disable capabilities must be provided

NOTE: Asynchronous messages are now considered a capability.

 Mode-oriented support adds the following to QMP:

o Two modes: handshake and operational
o By default all QMP Monitors start in handshake mode
o In handshake mode only commands to query/enable/disable QMP capabilities are
  allowed (there are few exceptions)
o Clients can switch to the operational mode at any time
o In Operational mode most commands are allowed and QMP capabilities changes
  made in handshake mode take effect

 Also note that each QMP Monitor has its own mode state and set of capabilities,
this means that if QEMU is started with N QMP Monitors protocol setup done in
one of them doesn't affect the others.

 Session example:

"""
{"QMP": {"capabilities": ["async messages"]}}

{ "execute": "query-qmp-mode" }
{"return": {"mode": "handshake"}}

{ "execute": "change", "arguments": { "device": "vnc", "target": "password", 
"arg": "1234" } }
{"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is 
invalid in this mode", "data": {}}}

{ "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
{"return": {}}

{ "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }
{"return": {}}

{ "execute": "query-qmp-mode" }
{"return": {"mode": "operational"}}

{ "execute": "change", "arguments": { "device": "vnc", "target": "password", 
"arg": "1234" } }
{"return": {}}
"""

 TODO:

- Update the spec
- Test more and fix some known issues
- Improve the changelog a bit

[Qemu-devel] [PATCH 01/11] QMP: Initial mode-oriented bits

In order to support feature negotiation in QMP, the Monitor
has to be modified to support different modes of operation.

We need two modes:

o Handshake: where features are negotiated. Only commands
which deal with protocol configuration are allowed, async
messages (as any other protocol capability) are disabled
by default

o Operational: regular Monitor operations. All handlers
(except the protocol configuration ones) are allowed and
async messages enabled in 'handshake' mode are delivered

This commit adds the initial infrastructure to implement this
design, basically it does:

1. Adds the QMPMode data type to MonitorControl and sets
QMODE_HANDSHAKE as the default one

2. Grant permission to 'query-version' and 'query-commands'
to run on handshake mode

Note, however, that these changes are not visable yet and
thus QMP's behavior is still the same.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index cadf422..4b7067a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -76,11 +76,16 @@
  *
  */
 
+/* Handler flags */
+#define HANDLER_HANDSHAKE  0x01 /* allowed to run on handshake mode */
+#define HANDLER_HANDSHAKE_ONLY 0x02 /* can ONLY run on handshake mode */
+
 typedef struct mon_cmd_t {
 const char *name;
 const char *args_type;
 const char *params;
 const char *help;
+unsigned int flags;
 void (*user_print)(Monitor *mon, const QObject *data);
 union {
 void (*info)(Monitor *mon);
@@ -98,8 +103,14 @@ struct mon_fd_t {
 QLIST_ENTRY(mon_fd_t) next;
 };
 
+typedef enum QMPMode {
+QMODE_OPERATIONAL,
+QMODE_HANDSHAKE,
+} QMPMode;
+
 typedef struct MonitorControl {
 QObject *id;
+QMPMode mode;
 int print_enabled;
 JSONMessageParser parser;
 } MonitorControl;
@@ -2364,6 +2375,7 @@ static const mon_cmd_t info_cmds[] = {
 .args_type  = "",
 .params = "",
 .help   = "show the version of QEMU",
+.flags  = HANDLER_HANDSHAKE,
 .user_print = do_info_version_print,
 .mhandler.info_new = do_info_version,
 },
@@ -2372,6 +2384,7 @@ static const mon_cmd_t info_cmds[] = {
 .args_type  = "",
 .params = "",
 .help   = "list QMP available commands",
+.flags  = HANDLER_HANDSHAKE,
 .user_print = monitor_user_noop,
 .mhandler.info_new = do_info_commands,
 },
@@ -4264,6 +4277,7 @@ void monitor_init(CharDriverState *chr, int flags)
 
 if (monitor_ctrl_mode(mon)) {
 mon->mc = qemu_mallocz(sizeof(MonitorControl));
+mon->mc->mode = QMODE_HANDSHAKE;
 /* Control mode requires special handlers */
 qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read,
   monitor_control_event, mon);
-- 
1.6.6

[Qemu-devel] [PATCH 02/11] QMP: Introduce 'query-qmp-mode' command

Only valid in QMP and allowed to run in both "handshake" and
"operational" modes.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   42 ++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 4b7067a..c475a38 100644
--- a/monitor.c
+++ b/monitor.c
@@ -586,6 +586,39 @@ static QObject *get_cmd_dict(const char *name)
 }
 
 /**
+ * do_info_qmp_mode(): Show current QMP mode
+ *
+ * Return a QDict with the following key:
+ *
+ * - "mode": either "handshake" or "operational"
+ *
+ * Example:
+ *
+ * { "mode": "handshake" }
+ */
+static void do_info_qmp_mode(Monitor *mon, QObject **ret_data)
+{
+const char *mode;
+
+if (!monitor_ctrl_mode(mon)) {
+return;
+}
+
+switch (mon->mc->mode) {
+case QMODE_HANDSHAKE:
+mode = "handshake";
+break;
+case QMODE_OPERATIONAL:
+mode = "operational";
+break;
+default:
+abort();
+}
+
+*ret_data = qobject_from_jsonf("{ 'mode': %s }", mode);
+}
+
+/**
  * do_info_commands(): List QMP available commands
  *
  * Each command is represented by a QDict, the returned QObject is a QList
@@ -2619,6 +2652,15 @@ static const mon_cmd_t info_cmds[] = {
 .mhandler.info_new = do_info_migrate,
 },
 {
+.name   = "qmp-mode",
+.args_type  = "",
+.params = "",
+.help   = "show current mode",
+.flags  = HANDLER_HANDSHAKE,
+.user_print = monitor_user_noop,
+.mhandler.info_new = do_info_qmp_mode,
+},
+{
 .name   = "balloon",
 .args_type  = "",
 .params = "",
-- 
1.6.6

[Qemu-devel] [PATCH 03/11] QError: Add QMP mode-oriented errors

Two new errors:

- QERR_QMP_INVALID_MODE_NAME
- QERR_QMP_INVALID_MODE_TRANSACTION

Signed-off-by: Luiz Capitulino 
---
 qerror.c |8 
 qerror.h |6 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 5f8fc5d..9dd729a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -105,6 +105,14 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Bad QMP input object",
 },
 {
+.error_fmt = QERR_QMP_INVALID_MODE_NAME,
+.desc  = "Mode name %(name) is invalid",
+},
+{
+.error_fmt = QERR_QMP_INVALID_MODE_TRANSACTION,
+.desc  = "Invalid mode transaction",
+},
+{
 .error_fmt = QERR_SET_PASSWD_FAILED,
 .desc  = "Could not set password",
 },
diff --git a/qerror.h b/qerror.h
index 9e220d6..9982d57 100644
--- a/qerror.h
+++ b/qerror.h
@@ -88,6 +88,12 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT \
 "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
 
+#define QERR_QMP_INVALID_MODE_NAME \
+"{ 'class': 'QMPInvalidModeName', 'data': { 'name': %s } }"
+
+#define QERR_QMP_INVALID_MODE_TRANSACTION \
+"{ 'class': 'QMPInvalidModeTransaction', 'data': {} }"
+
 #define QERR_SET_PASSWD_FAILED \
 "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
-- 
1.6.6

[Qemu-devel] [PATCH 04/11] QMP: Introduce qmp_switch_mode command

It will be used to switch between "handshake" and "operational"
modes. Currently it doesn't have any practical effect, as
mode-oriented support is not enforced yet.

Usage example:

{ "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }

Signed-off-by: Luiz Capitulino 
---
 monitor.c   |   21 +
 qemu-monitor.hx |   15 +++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index c475a38..fc6a1ed 100644
--- a/monitor.c
+++ b/monitor.c
@@ -618,6 +618,27 @@ static void do_info_qmp_mode(Monitor *mon, QObject 
**ret_data)
 *ret_data = qobject_from_jsonf("{ 'mode': %s }", mode);
 }
 
+static void do_qmp_switch_mode(Monitor *mon, const QDict *qdict,
+   QObject **ret_data)
+{
+const char *mode;
+
+if (!monitor_ctrl_mode(mon)) {
+return;
+}
+
+mode = qdict_get_str(qdict, "mode");
+
+if (mon->mc->mode != QMODE_HANDSHAKE || !strcmp(mode, "handshake")) {
+/* only handshake -> operational is allowed */
+qemu_error_new(QERR_QMP_INVALID_MODE_TRANSACTION);
+} else if (!strcmp(mode, "operational")) {
+mon->mc->mode = QMODE_OPERATIONAL;
+} else {
+qemu_error_new(QERR_QMP_INVALID_MODE_NAME, mode);
+}
+}
+
 /**
  * do_info_commands(): List QMP available commands
  *
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 1aa7818..eebea09 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1062,6 +1062,21 @@ STEXI
 Set the encrypted device @var{device} password to @var{password}
 ETEXI
 
+{
+.name   = "qmp_switch_mode",
+.args_type  = "mode:s",
+.params = "qmp_switch_mode name",
+.help   = "switch QMP mode",
+.flags  = HANDLER_HANDSHAKE_ONLY,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_qmp_switch_mode,
+},
+
+STEXI
+...@item qmp_switch_mode @var{mode}
+Switch QMP to @var{mode}
+ETEXI
+
 STEXI
 @end table
 ETEXI
-- 
1.6.6

[Qemu-devel] [PATCH 05/11] QMP: advertise asynchronous messages

With feature negotiation support asynchronous messages are
going to behave like any protocol capability, that is, it
is disabled by default, it can be negotiated and has to be
advertised.

TODO: update spec.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   18 +-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index fc6a1ed..70a59c7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4251,6 +4251,21 @@ void monitor_resume(Monitor *mon)
 readline_show_prompt(mon->rs);
 }
 
+/* XXX: Do we need anything fancier here? */
+static QObject *qmp_capabilities(void)
+{
+int i;
+QList *qmp_caps;
+const char *capabilities[] = { "async messages", NULL };
+
+qmp_caps = qlist_new();
+for (i = 0; capabilities[i]; i++) {
+qlist_append(qmp_caps, qstring_from_str(capabilities[i]));
+}
+
+return QOBJECT(qmp_caps);
+}
+
 /**
  * monitor_control_event(): Print QMP gretting
  */
@@ -4262,7 +4277,8 @@ static void monitor_control_event(void *opaque, int event)
 
 json_message_parser_init(&mon->mc->parser, handle_qmp_command);
 
-data = qobject_from_jsonf("{ 'QMP': { 'capabilities': [] } }");
+data = qobject_from_jsonf("{ 'QMP': { 'capabilities': %p } }",
+  qmp_capabilities()); 
 assert(data != NULL);
 
 monitor_json_emitter(mon, data);
-- 
1.6.6

[Qemu-devel] [PATCH 06/11] QMP: Array-based async messages

This commit moves the asynchronous messages names to an array
of structs, so that it can be indexed and searched.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   67 +++--
 1 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/monitor.c b/monitor.c
index 70a59c7..61c0273 100644
--- a/monitor.c
+++ b/monitor.c
@@ -336,6 +336,38 @@ static void timestamp_put(QDict *qdict)
 qdict_put_obj(qdict, "timestamp", obj);
 }
 
+typedef struct MonitorEventName {
+const char *name;
+} MonitorEventName;
+
+static const MonitorEventName monitor_events_names[] = {
+[QEVENT_DEBUG] = {
+.name  = "DEBUG",
+},
+[QEVENT_SHUTDOWN] = {
+.name  = "SHUTDOWN",
+},
+[QEVENT_RESET] = {
+.name  = "RESET",
+},
+[QEVENT_POWERDOWN] = {
+.name  = "POWERDOWN",
+},
+[QEVENT_STOP] = {
+.name  = "STOP",
+},
+[QEVENT_VNC_CONNECTED] = {
+.name  = "VNC_CONNECTED",
+},
+[QEVENT_VNC_INITIALIZED] = {
+.name  = "VNC_INITIALIZED",
+},
+[QEVENT_VNC_DISCONNECTED] = {
+.name  = "VNC_DISCONNECTED",
+},
+{ },
+};
+
 /**
  * monitor_protocol_event(): Generate a Monitor event
  *
@@ -344,44 +376,13 @@ static void timestamp_put(QDict *qdict)
 void monitor_protocol_event(MonitorEvent event, QObject *data)
 {
 QDict *qmp;
-const char *event_name;
 Monitor *mon;
 
-assert(event < QEVENT_MAX);
-
-switch (event) {
-case QEVENT_DEBUG:
-event_name = "DEBUG";
-break;
-case QEVENT_SHUTDOWN:
-event_name = "SHUTDOWN";
-break;
-case QEVENT_RESET:
-event_name = "RESET";
-break;
-case QEVENT_POWERDOWN:
-event_name = "POWERDOWN";
-break;
-case QEVENT_STOP:
-event_name = "STOP";
-break;
-case QEVENT_VNC_CONNECTED:
-event_name = "VNC_CONNECTED";
-break;
-case QEVENT_VNC_INITIALIZED:
-event_name = "VNC_INITIALIZED";
-break;
-case QEVENT_VNC_DISCONNECTED:
-event_name = "VNC_DISCONNECTED";
-break;
-default:
-abort();
-break;
-}
+assert(event >= 0 && event < QEVENT_MAX);
 
 qmp = qdict_new();
 timestamp_put(qmp);
-qdict_put(qmp, "event", qstring_from_str(event_name));
+qdict_put(qmp, "event", 
qstring_from_str(monitor_events_names[event].name));
 if (data) {
 qobject_incref(data);
 qdict_put_obj(qmp, "data", data);
-- 
1.6.6

[Qemu-devel] [PATCH 07/11] QError: New QERR_ASYNC_MSG_NOT_FOUND

Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 9dd729a..12dcc8f 100644
--- a/qerror.c
+++ b/qerror.c
@@ -41,6 +41,10 @@ static const QType qerror_type = {
  */
 static const QErrorStringTable qerror_table[] = {
 {
+.error_fmt = QERR_ASYNC_MSG_NOT_FOUND,
+.desc  = "Asynchronous message %(name) has not been found",
+},
+{
 .error_fmt = QERR_COMMAND_NOT_FOUND,
 .desc  = "The command %(name) has not been found",
 },
diff --git a/qerror.h b/qerror.h
index 9982d57..8e16b35 100644
--- a/qerror.h
+++ b/qerror.h
@@ -40,6 +40,9 @@ QError *qobject_to_qerror(const QObject *obj);
 /*
  * QError class list
  */
+#define QERR_ASYNC_MSG_NOT_FOUND \
+"{ 'class': 'AsyncMsgNotFound', 'data': { 'name': %s } }"
+
 #define QERR_COMMAND_NOT_FOUND \
 "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
 
-- 
1.6.6

[Qemu-devel] [PATCH 08/11] QMP: Asynchronous messages enable/disable support

This commit disables asynchronous messages by default and
introduces two new QMP commands: async_msg_enable and
async_msg_disable.

Each QMP Monitor has its own set of asynchronous messages,
so for example, if QEMU is run with two QMP Monitors async
messages setup in one of them doesn't affect the other.

To implement this design a bitmap is introduced to the
Monitor struct, each async message is represented by one bit.

Signed-off-by: Luiz Capitulino 
---
 monitor.c   |   49 -
 qemu-monitor.hx |   30 ++
 2 files changed, 78 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 61c0273..321bc3a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -108,11 +108,14 @@ typedef enum QMPMode {
 QMODE_HANDSHAKE,
 } QMPMode;
 
+#define EVENTS_BITMAP_SIZE (QEVENT_MAX / 8)
+
 typedef struct MonitorControl {
 QObject *id;
 QMPMode mode;
 int print_enabled;
 JSONMessageParser parser;
+uint8_t events_bitmap[EVENTS_BITMAP_SIZE];
 } MonitorControl;
 
 struct Monitor {
@@ -318,6 +321,23 @@ static void monitor_protocol_emitter(Monitor *mon, QObject 
*data)
 QDECREF(qmp);
 }
 
+static void qevent_set(const Monitor *mon, int64_t event)
+{
+assert(event >= 0 && event < QEVENT_MAX);
+mon->mc->events_bitmap[event / 8] |= (1 << (event % 8));
+}
+
+static void qevent_unset(const Monitor *mon, int64_t event)
+{
+assert(event >= 0 && event < QEVENT_MAX);
+mon->mc->events_bitmap[event / 8] &= ~(1 << (event % 8));
+}
+
+static int qevent_enabled(const Monitor *mon, int64_t event)
+{
+return (mon->mc->events_bitmap[event / 8] & (1 << (event % 8)));
+}
+
 static void timestamp_put(QDict *qdict)
 {
 int err;
@@ -389,7 +409,8 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 }
 
 QLIST_FOREACH(mon, &mon_list, entry) {
-if (monitor_ctrl_mode(mon)) {
+if (monitor_ctrl_mode(mon) &&
+qevent_enabled(mon, event)) {
 monitor_json_emitter(mon, QOBJECT(qmp));
 }
 }
@@ -465,6 +486,32 @@ static void do_commit(Monitor *mon, const QDict *qdict)
 }
 }
 
+static void qevent_set_value(Monitor *mon, const QDict *qdict, int value)
+{
+int i;
+const char *name = qdict_get_str(qdict, "name");
+
+for (i = 0; monitor_events_names[i].name != NULL; i++) {
+if (!strcmp(monitor_events_names[i].name, name)) {
+return (value ? qevent_set(mon, i) : qevent_unset(mon, i));
+}
+}
+
+qemu_error_new(QERR_ASYNC_MSG_NOT_FOUND, name);
+}
+
+static void do_async_msg_enable(Monitor *mon, const QDict *qdict,
+QObject **ret_data)
+{
+qevent_set_value(mon, qdict, 1);
+}
+
+static void do_async_msg_disable(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+qevent_set_value(mon, qdict, 0);
+}
+
 static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const mon_cmd_t *cmd;
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index eebea09..0f3dfde 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1077,6 +1077,36 @@ STEXI
 Switch QMP to @var{mode}
 ETEXI
 
+{
+.name   = "async_msg_enable",
+.args_type  = "name:s",
+.params = "async_msg_enable name",
+.help   = "enable an asynchronous message",
+.flags  = HANDLER_HANDSHAKE_ONLY,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_async_msg_enable,
+},
+
+STEXI
+...@item async_msg_enable @var{name}
+Enable the asynchronous message @var{name}
+ETEXI
+
+{
+.name   = "async_msg_disable",
+.args_type  = "name:s",
+.params = "async_msg_disable name",
+.help   = "disable an asynchronous message",
+.flags  = HANDLER_HANDSHAKE_ONLY,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_async_msg_disable,
+},
+
+STEXI
+...@item async_msg_disable @var{name}
+Disable the asynchronous message @var{name}
+ETEXI
+
 STEXI
 @end table
 ETEXI
-- 
1.6.6

[Qemu-devel] [PATCH 09/11] Monitor: Introduce find_info_cmd()

Move the code used to locate an info command entry to its
own function, as it's going to be used by other functions.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   21 +++--
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index 321bc3a..34df1cc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -512,6 +512,19 @@ static void do_async_msg_disable(Monitor *mon, const QDict 
*qdict,
 qevent_set_value(mon, qdict, 0);
 }
 
+static const mon_cmd_t *monitor_find_info_command(const char *name)
+{
+const mon_cmd_t *cmd;
+
+for (cmd = info_cmds; cmd->name != NULL; cmd++) {
+if (compare_cmd(name, cmd->name)) {
+return cmd;
+}
+}
+
+return NULL;
+}
+
 static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const mon_cmd_t *cmd;
@@ -522,12 +535,8 @@ static void do_info(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 goto help;
 }
 
-for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-if (compare_cmd(item, cmd->name))
-break;
-}
-
-if (cmd->name == NULL) {
+cmd = monitor_find_info_command(item);
+if (!cmd) {
 if (monitor_ctrl_mode(mon)) {
 qemu_error_new(QERR_COMMAND_NOT_FOUND, item);
 return;
-- 
1.6.6

[Qemu-devel] [PATCH 10/11] QError: New QERR_QMP_INVALID_MODE_COMMAND

Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 12dcc8f..526 100644
--- a/qerror.c
+++ b/qerror.c
@@ -109,6 +109,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Bad QMP input object",
 },
 {
+.error_fmt = QERR_QMP_INVALID_MODE_COMMAND,
+.desc  = "The issued command is invalid in this mode",
+},
+{
 .error_fmt = QERR_QMP_INVALID_MODE_NAME,
 .desc  = "Mode name %(name) is invalid",
 },
diff --git a/qerror.h b/qerror.h
index 8e16b35..c0deacc 100644
--- a/qerror.h
+++ b/qerror.h
@@ -91,6 +91,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT \
 "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
 
+#define QERR_QMP_INVALID_MODE_COMMAND \
+"{ 'class': 'QMPInvalidModeCommad', 'data': {} }"
+
 #define QERR_QMP_INVALID_MODE_NAME \
 "{ 'class': 'QMPInvalidModeName', 'data': { 'name': %s } }"
 
-- 
1.6.6

[Qemu-devel] [PATCH 11/11] QMP: Enable feature negotiation support

This is done by enforcing the following mode-oriented rules:

- QMP is started in handshake mode
- In handshake mode all protocol capabilities are disabled
  and (apart of a few exceptions) only commands which
  query/enable/disable them are allowed
- Asynchronous messages are now considered a capability
- Clients can change to the operational mode (where capabilities'
  changes take effect and most commands are allowed) at any
  time
- Each QMP Monitor has its own set of capabilities, changes
  made to one of them don't affect the others

Also note that all these changes should have no effect in
the user Monitor.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   43 ++-
 1 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 34df1cc..0ca0941 100644
--- a/monitor.c
+++ b/monitor.c
@@ -410,7 +410,8 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 
 QLIST_FOREACH(mon, &mon_list, entry) {
 if (monitor_ctrl_mode(mon) &&
-qevent_enabled(mon, event)) {
+qevent_enabled(mon, event) &&
+mon->mc->mode == QMODE_OPERATIONAL) {
 monitor_json_emitter(mon, QOBJECT(qmp));
 }
 }
@@ -4166,12 +4167,38 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, 
QDict *args)
 return err;
 }
 
+static int qmp_mode_invalid(const Monitor *mon, unsigned int cmd_flags)
+{
+switch (mon->mc->mode) {
+case QMODE_OPERATIONAL:
+if (cmd_flags & HANDLER_HANDSHAKE_ONLY) {
+goto mode_error;
+}
+break;
+case QMODE_HANDSHAKE:
+if (!((cmd_flags & HANDLER_HANDSHAKE) ||
+(cmd_flags & HANDLER_HANDSHAKE_ONLY))) {
+goto mode_error;
+}
+break;
+default:
+abort();
+}
+
+return 0;
+
+mode_error:
+qemu_error_new(QERR_QMP_INVALID_MODE_COMMAND);
+return 1;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
 int err;
 QObject *obj;
 QDict *input, *args;
 const mon_cmd_t *cmd;
+unsigned int cmd_flags;
 Monitor *mon = cur_mon;
 const char *cmd_name, *info_item;
 
@@ -4213,6 +4240,15 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
 goto err_input;
 } else if (strstart(cmd_name, "query-", &info_item)) {
+/* check it exists and get its flags */
+cmd = monitor_find_info_command(info_item);
+if (!cmd) {
+qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
+goto err_input;
+}
+cmd_flags = cmd->flags;
+
+/* setup 'info' to call it */
 cmd = monitor_find_command("info");
 qdict_put_obj(input, "arguments",
   qobject_from_jsonf("{ 'item': %s }", info_item));
@@ -4222,6 +4258,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
 goto err_input;
 }
+cmd_flags = cmd->flags;
+}
+
+if (qmp_mode_invalid(mon, cmd_flags)) {
+goto err_input;
 }
 
 obj = qdict_get(input, "arguments");
-- 
1.6.6

[Qemu-devel] [PATCH v3 0/4]: Convert pci_info() to QObject

 Hi,

 This new version fixes the printing of PCI bridge information,
as I was able to test it after some help from Blue Swirl.

 Only the printing of PCI bridge attached devices remains
untested, but according to Blue Swirl attaching devices to it
is broken so I don't think it matters much.

changelog
-

V2 -> V3

- Fix issues with the printing of PCI bridge info
- Merge PCI bridge info patch to the main conversion one
- Clarify changelog

V1 -> V2

- Make class_info's key 'desc' optional
- Better indentation
- Doc fixes

V0 -> V1

- Coding style fixes
- Make 'BAR' and 'IRQ' keys lowercase
- Add 'irq' key to the documentation

 Thanks.

[Qemu-devel] [PATCH 1/4] QList: Introduce QLIST_FOREACH_ENTRY()

Iterate over QList entries, it's needed to call qlist_entry_obj()
to retrieve the stored QObject.

I'm not sure if it's ok to have this, because it's not as easy as
qlist_iter() and the QListEntry data type is now exposed to the
users, which means we have one more struct to be maintained when
we have libqmp.

Adding anyway, as it's more compact and people are asking for it.

Signed-off-by: Luiz Capitulino 
---
 qlist.h |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/qlist.h b/qlist.h
index afdc446..a3261e1 100644
--- a/qlist.h
+++ b/qlist.h
@@ -29,6 +29,16 @@ typedef struct QList {
 #define qlist_append(qlist, obj) \
 qlist_append_obj(qlist, QOBJECT(obj))
 
+#define QLIST_FOREACH_ENTRY(qlist, var) \
+for ((var) = ((qlist)->head.tqh_first); \
+(var);  \
+(var) = ((var)->next.tqe_next))
+
+static inline QObject *qlist_entry_obj(const QListEntry *entry)
+{
+return entry->value;
+}
+
 QList *qlist_new(void);
 QList *qlist_copy(QList *src);
 void qlist_append_obj(QList *qlist, QObject *obj);
-- 
1.6.6

[Qemu-devel] [PATCH 2/4] QDict: Introduce qdict_get_qdict()

A helper to retrieve a QDict from a QDict.

Signed-off-by: Luiz Capitulino 
---
 qdict.c |   13 +
 qdict.h |1 +
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index ba8eef0..c6a5a42 100644
--- a/qdict.c
+++ b/qdict.c
@@ -216,6 +216,19 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_get_qdict(): Get the QDict mapped by 'key'
+ *
+ * This function assumes that 'key' exists and it stores a
+ * QDict object.
+ *
+ * Return QDict mapped by 'key'.
+ */
+QDict *qdict_get_qdict(const QDict *qdict, const char *key)
+{
+return qobject_to_qdict(qdict_get_obj(qdict, key, QTYPE_QDICT));
+}
+
+/**
  * qdict_get_str(): Get a pointer to the stored string mapped
  * by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 5fef1ea..2eaf6d5 100644
--- a/qdict.h
+++ b/qdict.h
@@ -40,6 +40,7 @@ void qdict_iter(const QDict *qdict,
 int64_t qdict_get_int(const QDict *qdict, const char *key);
 int qdict_get_bool(const QDict *qdict, const char *key);
 QList *qdict_get_qlist(const QDict *qdict, const char *key);
+QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
   int64_t err_value);
-- 
1.6.6

[Qemu-devel] [PATCH 3/4] PCI: Convert pci_info() to QObject

The returned QObject is a QList of all buses. Each bus is
represented by a QDict, which has a key with a QList of all
PCI devices attached to it. Each device is represented by
a QDict.

As has happended to other complex conversions, it's hard to
split this commit as part of it are new functions which are
called by each other.

IMPORTANT: support for printing PCI bridge attached devices
is NOT part of this commit, it's going to be added by the
next commit, as it's untested.

Signed-off-by: Luiz Capitulino 
---
 hw/pci.c  |  370 
 hw/pci.h  |4 +-
 monitor.c |3 +-
 3 files changed, 301 insertions(+), 76 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 2bdf27e..ffe4cfe 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -27,6 +27,7 @@
 #include "net.h"
 #include "sysemu.h"
 #include "loader.h"
+#include "qemu-objects.h"
 
 //#define DEBUG_PCI
 #ifdef DEBUG_PCI
@@ -1076,119 +1077,340 @@ static const pci_class_desc pci_class_descriptions[] =
 { 0, NULL}
 };
 
-static void pci_info_device(PCIBus *bus, PCIDevice *d)
+static void pci_for_each_device_under_bus(PCIBus *bus,
+  void (*fn)(PCIBus *b, PCIDevice *d))
 {
-Monitor *mon = cur_mon;
-int i, class;
-PCIIORegion *r;
-const pci_class_desc *desc;
+PCIDevice *d;
+int devfn;
 
-monitor_printf(mon, "  Bus %2d, device %3d, function %d:\n",
-   pci_bus_num(d->bus),
-   PCI_SLOT(d->devfn), PCI_FUNC(d->devfn));
-class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+for(devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
+d = bus->devices[devfn];
+if (d) {
+fn(bus, d);
+}
+}
+}
+
+void pci_for_each_device(PCIBus *bus, int bus_num,
+ void (*fn)(PCIBus *b, PCIDevice *d))
+{
+bus = pci_find_bus(bus, bus_num);
+
+if (bus) {
+pci_for_each_device_under_bus(bus, fn);
+}
+}
+
+static void pci_device_print(Monitor *mon, QDict *device)
+{
+QDict *qdict;
+QListEntry *entry;
+uint64_t addr, size;
+
+monitor_printf(mon, "  Bus %2" PRId64 ", ", qdict_get_int(device, "bus"));
+monitor_printf(mon, "device %3" PRId64 ", function %" PRId64 ":\n",
+qdict_get_int(device, "slot"),
+qdict_get_int(device, "function"));
 monitor_printf(mon, "");
-desc = pci_class_descriptions;
-while (desc->desc && class != desc->class)
-desc++;
-if (desc->desc) {
-monitor_printf(mon, "%s", desc->desc);
+
+qdict = qdict_get_qdict(device, "class_info");
+if (qdict_haskey(qdict, "desc")) {
+monitor_printf(mon, "%s", qdict_get_str(qdict, "desc"));
 } else {
-monitor_printf(mon, "Class %04x", class);
+monitor_printf(mon, "Class %04" PRId64, qdict_get_int(qdict, "class"));
 }
-monitor_printf(mon, ": PCI device %04x:%04x\n",
-   pci_get_word(d->config + PCI_VENDOR_ID),
-   pci_get_word(d->config + PCI_DEVICE_ID));
 
-if (d->config[PCI_INTERRUPT_PIN] != 0) {
-monitor_printf(mon, "  IRQ %d.\n",
-   d->config[PCI_INTERRUPT_LINE]);
+qdict = qdict_get_qdict(device, "id");
+monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
+qdict_get_int(qdict, "device"),
+qdict_get_int(qdict, "vendor"));
+
+if (qdict_haskey(device, "irq")) {
+monitor_printf(mon, "  IRQ %" PRId64 ".\n",
+qdict_get_int(device, "irq"));
 }
-if (class == 0x0604) {
-uint64_t base;
-uint64_t limit;
 
-monitor_printf(mon, "  BUS %d.\n", d->config[0x19]);
-monitor_printf(mon, "  secondary bus %d.\n",
-   d->config[PCI_SECONDARY_BUS]);
-monitor_printf(mon, "  subordinate bus %d.\n",
-   d->config[PCI_SUBORDINATE_BUS]);
+if (qdict_haskey(device, "pci_bridge")) {
+QDict *info;
+
+qdict = qdict_get_qdict(device, "pci_bridge");
+
+info = qdict_get_qdict(qdict, "bus");
+monitor_printf(mon, "  BUS %" PRId64 ".\n",
+qdict_get_int(info, "number"));
+monitor_printf(mon, "  secondary bus %" PRId64 ".\n",
+qdict_get_int(info, "secondary"));
+monitor_printf(mon, "  subordinate bus %" PRId64 ".\n",
+qdict_get_int(info, "subordinate"));
 
-base = pci_bridge_get_base(d, PCI_BASE_ADDRESS_SPACE_IO);
-limit = pci_bridge_get_limit(d, PCI_BASE_ADDRESS_SPACE_IO);
+info = qdict_get_qdict(qdict, "io_range");
 monitor_printf(mon, "  IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
-   base, limit);
+   qdict_get_int(info, "base"),
+   qdict_get_int(info, "limit"));
 
-base = pci_brid

[Qemu-devel] [PATCH 4/4] PCI: do_pci_info(): PCI bridge devices support

This commit completes the do_pci_info() conversion to
QObject by adding support to PCI bridge devices.

This is done by recursively adding devices in the
"pci_bridge" key.

IMPORTANT: This code is being added separately because I could
NOT test it properly. According to Michael Tsirkin, it depends
on ultrasparc and it would take time to do the proper setup.

Signed-off-by: Luiz Capitulino 
---
 hw/pci.c |   23 ---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index ffe4cfe..b83fd53 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1183,7 +1183,15 @@ static void pci_device_print(Monitor *mon, QDict *device)
 
 monitor_printf(mon, "  id \"%s\"\n", qdict_get_str(device, "qdev_id"));
 
-/* TODO: PCI bridge devices */
+if (qdict_haskey(device, "pci_bridge")) {
+qdict = qdict_get_qdict(device, "pci_bridge");
+if (qdict_haskey(qdict, "devices")) {
+QListEntry *dev;
+QLIST_FOREACH_ENTRY(qdict_get_qlist(qdict, "devices"), dev) {
+pci_device_print(mon, qobject_to_qdict(qlist_entry_obj(dev)));
+}
+}
+}
 }
 
 void do_pci_info_print(Monitor *mon, const QObject *data)
@@ -1261,7 +1269,9 @@ static QObject *pci_get_regions_list(const PCIDevice *dev)
 return QOBJECT(regions_list);
 }
 
-static QObject *pci_get_dev_dict(PCIDevice *dev, int bus_num)
+static QObject *pci_get_devices_list(PCIBus *bus, int bus_num);
+
+static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
 {
 int class;
 QObject *obj;
@@ -1300,6 +1310,12 @@ static QObject *pci_get_dev_dict(PCIDevice *dev, int 
bus_num)
 pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
 PCI_BASE_ADDRESS_MEM_PREFETCH));
 
+if (dev->config[0x19] != 0) {
+qdict = qobject_to_qdict(pci_bridge);
+qdict_put_obj(qdict, "devices",
+  pci_get_devices_list(bus, dev->config[0x19]));
+}
+
 qdict = qobject_to_qdict(obj);
 qdict_put_obj(qdict, "pci_bridge", pci_bridge);
 }
@@ -1318,7 +1334,7 @@ static QObject *pci_get_devices_list(PCIBus *bus, int 
bus_num)
 for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
 dev = bus->devices[devfn];
 if (dev) {
-qlist_append_obj(dev_list, pci_get_dev_dict(dev, bus_num));
+qlist_append_obj(dev_list, pci_get_dev_dict(dev, bus, bus_num));
 }
 }
 
@@ -1371,6 +1387,7 @@ static QObject *pci_get_bus_dict(PCIBus *bus, int bus_num)
  *  - "io_range": a QDict with memory range information
  *  - "memory_range": a QDict with memory range information
  *  - "prefetchable_range": a QDict with memory range information
+ *  - "devices": a QList of PCI devices if there's any attached (optional)
  * - "regions": a QList of QDicts, each QDict represents a
  *   memory region of this device
  *
-- 
1.6.6

[Qemu-devel] [PATCH] Tell users about out-of-memory errors

2010-01-21 Thread Stefan Weil

Aborting without an error message when memory is short
is not helpful, so print the reason for the abort.

Try
qemu -m 100
or
qemu -m 2000 (win32)

to force an out-of-memory error.

v2:
* Fix error message for win32.
* Fix error message for posix_memalign.

Thanks to malc for the hints.

Signed-off-by: Stefan Weil 
---
 osdep.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/osdep.c b/osdep.c
index 1310684..9b47066 100644
--- a/osdep.c
+++ b/osdep.c
@@ -52,6 +52,11 @@
 static void *oom_check(void *ptr)
 {
 if (ptr == NULL) {
+#if defined(_WIN32)
+fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
+#else
+fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
+#endif
 abort();
 }
 return ptr;
@@ -91,8 +96,11 @@ void *qemu_memalign(size_t alignment, size_t size)
 int ret;
 void *ptr;
 ret = posix_memalign(&ptr, alignment, size);
-if (ret != 0)
+if (ret != 0) {
+fprintf(stderr, "Failed to allocate %zu B: %s\n",
+size, strerror(ret));
 abort();
+}
 return ptr;
 #elif defined(CONFIG_BSD)
 return oom_check(valloc(size));
-- 
1.6.5

Re: [Qemu-devel] [PATCH] loader: don't call realloc(O) when no symbols are present